[Bug 1301143] Review Request: skopeo - Get information about Docker images without pulling them

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



https://bugzilla.redhat.com/show_bug.cgi?id=1301143



--- Comment #16 from Antonio Murdaca <amurdaca@xxxxxxxxxx> ---
(In reply to Nalin Dahyabhai from comment #15)
> runcom is not currently in the packagers group; I can sponsor.
> 
> I've got questions about the license tag when we're bundling, and could
> probably use some clarification about whether or not, and if so, how many,
> of the vendored modules need to be debundled for Fedora.  Otherwise it looks
> pretty straightforward from here.
> 
> fedora-review output:
> 
> Package Review
> ==============
> 
> Legend:
> [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
> [ ] = Manual review needed
> 
> Issues:
> =======
> - Sources used to build the package match the upstream source, as provided
>   in the spec URL.
>   See: http://fedoraproject.org/wiki/Packaging/SourceURL
> 
>   Since you're keeping a .spec file in the repository, I expect you'll be
> keeping it more or less in sync with the one being used for Fedora, so no
> worries there.

I've created https://github.com/runcom/fedora-pkgs where I do have .spec(s) and
SRPM(s)
I'm planning to remove the .spec from the source repo and maintain the .spec at
https://github.com/runcom/fedora-pkgs/tree/master/skopeo/fedora/skopeo

> 
>   The source tarball in the SRPM contained the .git directory and copies of
> generated files, including the binary, which is rather odd.  How was it
> generated?  Will future versions of the package do this as well?

Probably a mistake - I've regenerated the SRPM and it's available at
https://github.com/runcom/fedora-pkgs/raw/master/skopeo/fedora/skopeo/skopeo-0.1.4-1.fc23.src.rpm
It does not contain .git nor binaries anymore.

> 
> - Package uses either %{buildroot} or $RPM_BUILD_ROOT
>   Note: Using both %{buildroot} and $RPM_BUILD_ROOT
>   See: http://fedoraproject.org/wiki/Packaging/Guidelines#macros
> 
>   Stylistically, it's better to choose either %{buildroot} or
> $RPM_BUILD_ROOT and be consistent about it.  You're not doing anything that
> makes either of them not an option, so use whichever you prefer.

Fixed in
https://raw.githubusercontent.com/runcom/fedora-pkgs/master/skopeo/fedora/skopeo/skopeo.spec

> 
> - If (and only if) the source package includes the text of the license(s)
>   in its own file, then that file, containing the text of the license(s)
>   for the package is included in %license.
>   Note: License file LICENSE is marked as %doc instead of %license
>   See:
>   http://fedoraproject.org/wiki/Packaging/LicensingGuidelines#License_Text
> 
>   The main package tags the LICENSE file as %doc rather than %license, which
> is trivially fixable.

Fixed in
https://raw.githubusercontent.com/runcom/fedora-pkgs/master/skopeo/fedora/skopeo/skopeo.spec

> 
> ===== MUST items =====
> 
> Generic:
> [ ]: Package is licensed with an open-source compatible license and meets
>      other legal requirements as defined in the legal section of Packaging
>      Guidelines.
> 
>      I suspect that linking your MIT-licensed main logic with vendored
> sources from other repositories is going to produce
>     
> https://fedoraproject.org/wiki/Packaging:
> LicensingGuidelines#Mixed_Source_Licensing_Scenario
> 
> [ ]: License field in the package spec file matches the actual license.
>      Note: Checking patched sources after %prep for licenses. Licenses
>      found: "MIT/X11 (BSD like)", "BSD (3 clause)", "*No copyright* Apache
>      (v2.0)", "Unknown or generated", "BSD (2 clause)". 371 files have
>      unknown license. Detailed output of licensecheck in /misc/skopeo
>      /review-skopeo/licensecheck.txt
> 
>      If it's not mixed source, then "License: MIT" is correct.

This license stuff is tricky...I've re-licensed my tool with ASL 2.0 since
all my vendors are ASL 2.0 afaict.

> 
> [x]: %build honors applicable compiler flags or justifies otherwise.
>      Uses %gobuild to invoke the go compiler.
> [ ]: Package contains no bundled libraries without FPC exception.
> 
>      Package bundles several libraries.  Does it need to remove them at the
> end of the %setup section when %{with_bundled} is 0 in order to ensure that
> the compiler picks up the debundled copies?  Doesn't Fedora require
> debundling?

I've added the removal of vendor/ when %{with_bundled} is 0
Right now, unluckily, this tool won't build from debundled vendor's copies, and
also not all vendors are already in Fedora - but I'm working on packaging them
with Lokesh and Jan for a future version

> 
> [x]: Changelog in prescribed format.
> [x]: Sources contain only permissible code or content.
> [-]: Package contains desktop file if it is a GUI application.
> [-]: Development files must be in a -devel package
> [x]: Package uses nothing in %doc for runtime.
> [!]: Package consistently uses macros (instead of hard-coded directory
>      names).
> 
>      The package's makefile hardcodes the install locations, and we don't
> force them to match %{_bindir} and %{_mandir}.

Fixed

> 
> [x]: Package is named according to the Package Naming Guidelines.
> [x]: Package does not generate any conflict.
> [x]: Package obeys FHS, except libexecdir and /usr/target.
> [-]: If the package is a rename of another package, proper Obsoletes and
>      Provides are present.
> [-]: Requires correct, justified where necessary.
> [ ]: Spec file is legible and written in American English.
> 
>      The package could use a longer %description; I think something along
> the lines of the paragraph that starts around line 6 of README.md would work.

Fixed in
https://raw.githubusercontent.com/runcom/fedora-pkgs/master/skopeo/fedora/skopeo/skopeo.spec

> 
> [-]: Package contains systemd file(s) if in need.
> [ ]: Useful -debuginfo package or justification otherwise.
> 
>      Why is %{with_debug} disabled?  Rebuilding it with debuginfo enabled
> produces files with names that seem to be of some use to my debugger.

Enabled %{with_debug} - was an old .spec

> 
> [-]: Package is not known to require an ExcludeArch tag.
> [-]: Large documentation must go in a -doc subpackage. Large could be size
>      (~1MB) or number of files.
>      Note: Documentation size is 20480 bytes in 2 files.
> [ ]: Package complies to the Packaging Guidelines
> 
>      Aside from questions I have about licensing and bundling, this looks
> fine.
> 
> [x]: Package successfully compiles and builds into binary rpms on at least
>      one supported primary architecture.
> [x]: Package installs properly.
> [x]: Rpmlint is run on all rpms the build produces.
>      Note: There are rpmlint messages (see attachment).
> [x]: Package requires other packages for directories it uses.
> [x]: Package must own all directories that it creates.
> [x]: Package does not own files or directories owned by other packages.
> [x]: All build dependencies are listed in BuildRequires, except for any
>      that are listed in the exceptions section of Packaging Guidelines.
> [x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
>      beginning of %install.
> [x]: Macros in Summary, %description expandable at SRPM build time.
> [x]: Dist tag is present.
> [x]: Package does not contain duplicates in %files.
> [x]: Permissions on files are set properly.
> [x]: Package use %makeinstall only when make install DESTDIR=... doesn't
>      work.
> [x]: Package is named using only allowed ASCII characters.
> [x]: Package does not use a name that already exists.
> [x]: Package is not relocatable.
> [x]: Spec file name must match the spec package %{name}, in the format
>      %{name}.spec.
> [x]: File names are valid UTF-8.
> [x]: Packages must not store files under /srv, /opt or /usr/local
> 
> ===== SHOULD items =====
> 
> Generic:
> [-]: If the source package does not include license text(s) as a separate
>      file from upstream, the packager SHOULD query upstream to include it.
> [x]: Final provides and requires are sane (see attachments).
> [x]: Package functions as described.
> [x]: Latest version is packaged.
> [x]: Package does not include license text files separate from upstream.
> [-]: Description and summary sections in the package spec file contains
>      translations for supported Non-English languages, if available.
> [-]: %check is present and all tests pass.
> [x]: Packages should try to preserve timestamps of original installed
>      files.
> 
>      The makefile doesn't bother for generated files, but then they're
> generated at build-time.

should I call `make clean` to remove generated files in a %clean section?

> 
> [x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
> [x]: Sources can be downloaded from URI in Source: tag
> [x]: Reviewer should test that the package builds in mock.
> [x]: Buildroot is not present
> [x]: Package has no %clean section with rm -rf %{buildroot} (or
>      $RPM_BUILD_ROOT)
> [x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
> [x]: SourceX is a working URL.
> [x]: Package should compile and build into binary rpms on all supported
>      architectures.
> [x]: Spec use %global instead of %define unless justified.
> 
> ===== EXTRA items =====
> 
> Generic:
> [x]: Rpmlint is run on all installed packages.
>      Note: There are rpmlint messages (see attachment).
> [x]: Large data in /usr/share should live in a noarch subpackage if package
>      is arched.
> [x]: Spec file according to URL is the same as in SRPM.
> 
> 
> Rpmlint
> -------
> Checking: skopeo-0.1.3-0.1.gitfdb5cac.fc24.x86_64.rpm
>           skopeo-0.1.3-0.1.gitfdb5cac.fc24.src.rpm
> skopeo.x86_64: W: unstripped-binary-or-object /usr/bin/skopeo
> skopeo.src: W: file-size-mismatch skopeo-fdb5cac.tar.gz = 5646750,
> https://github.com/runcom/skopeo/archive/
> fdb5cac7f5af50ae5b1e3424965c31600d86232c/skopeo-fdb5cac.tar.gz = 428362
> 2 packages and 0 specfiles checked; 0 errors, 2 warnings.
> 
> Rpmlint (installed packages)
> ----------------------------
> sh: /usr/bin/python: No such file or directory
> skopeo.x86_64: W: unstripped-binary-or-object /usr/bin/skopeo
> 1 packages and 0 specfiles checked; 0 errors, 1 warnings.
> 
> Requires
> --------
> skopeo (rpmlib, GLIBC filtered):
>     libc.so.6()(64bit)
>     libpthread.so.0()(64bit)
>     rtld(GNU_HASH)
> 
> Provides
> --------
> skopeo:
>     skopeo
>     skopeo(x86-64)
> 
> Source checksums
> ----------------
> https://github.com/runcom/skopeo/archive/
> fdb5cac7f5af50ae5b1e3424965c31600d86232c/skopeo-fdb5cac.tar.gz :
>   CHECKSUM(SHA256) this package     :
> 1d2050f0edfec3abb3b7cff2d8d71def51b7057875ee6a82e485d7e1eb9fd196
>   CHECKSUM(SHA256) upstream package :
> bae74476f07f2955ed360c78ce7f724896a4479e4b6628a37ce34a08e605cb63
> diff -r also reports differences
> 
> 
> Generated by fedora-review 0.6.0 (3c5c9d7) last change: 2015-05-20
> Command line :/usr/bin/fedora-review -n skopeo
> Buildroot used: fedora-rawhide-x86_64
> Active plugins: Generic, Shell-api
> Disabled plugins: Java, C/C++, Python, fonts, SugarActivity, Ocaml, Perl,
> Haskell, R, PHP, Ruby
> Disabled flags: EXARCH, DISTTAG, EPEL5, BATCH, EPEL6

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review




[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]