Bug 1301143 - Review Request: skopeo - Get information about Docker images without pulling them
Review Request: skopeo - Get information about Docker images without pulling ...
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Nalin Dahyabhai
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2016-01-22 12:34 EST by Antonio Murdaca
Modified: 2016-04-02 00:35 EDT (History)
5 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2016-04-02 00:35:50 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
nalin: fedora‑review+


Attachments (Terms of Use)

  None (edit)
Description Antonio Murdaca 2016-01-22 12:34:34 EST
Spec URL: https://raw.githubusercontent.com/runcom/skopeo/master/skopeo.spec

SRPM URL: http://runcom.ninja/skopeo-0.1.0-1.fc23.src.rpm

Koji builds:

- f23: http://koji.fedoraproject.org/koji/taskinfo?taskID=12650312
- rh:  http://koji.fedoraproject.org/koji/taskinfo?taskID=12650093

Description: skopeo is a command line utility which is able to inspect a repository on a Docker registry. It fetches the repository's manifest and it is able to show you a "docker inspect"-like json output about a whole repository or a tag.

Fedora Account System Username: runcom

Additional info: I'm the owner of the package upstream
Comment 1 Upstream Release Monitoring 2016-01-23 05:50:20 EST
runcom's scratch build of skopeo-0.1.1-1.fc23.src.rpm for f23 completed http://koji.fedoraproject.org/koji/taskinfo?taskID=12661477
Comment 2 Antonio Murdaca 2016-01-23 05:58:13 EST
I've released a new version, and I've made sure distro-supplied golang libs are used when possible (in spec), the only leftover are (from gofed):

Class: github.com/docker/distribution (golang-github-docker-distribution) PkgDB=False
Class: github.com/docker/docker (docker) PkgDB=False
Class: github.com/docker/engine-api (golang-github-docker-engine-api) PkgDB=False

---

Spec URL: https://github.com/runcom/skopeo/blob/v0.1.2/skopeo.spec
SRPM URL: http://runcom.ninja/skopeo-0.1.2-0.fc23.src.rpm

Koji builds:

- f23:     http://koji.fedoraproject.org/koji/taskinfo?taskID=12661579
- rawhide: http://koji.fedoraproject.org/koji/taskinfo?taskID=12661563
Comment 3 Upstream Release Monitoring 2016-01-23 05:58:46 EST
runcom's scratch build of skopeo-0.1.2-0.fc23.src.rpm for rawhide completed http://koji.fedoraproject.org/koji/taskinfo?taskID=12661563
Comment 4 Lokesh Mandvekar 2016-01-23 12:26:37 EST
Ohh btw, I feel it'd be best first moved to projectatomic github before we go ahead with approving this, the only reason being moving from @runcom to @projectatomic would result in changing golang(github.com/runcom/skopeo) to golang(github.com/projectatomic/skopeo) in the golang paths provided, not too big a deal, but imho something best decided beforehand.
Comment 5 Antonio Murdaca 2016-01-23 12:34:58 EST
(In reply to Lokesh Mandvekar from comment #4)
> Ohh btw, I feel it'd be best first moved to projectatomic github before we
> go ahead with approving this, the only reason being moving from @runcom to
> @projectatomic would result in changing golang(github.com/runcom/skopeo) to
> golang(github.com/projectatomic/skopeo) in the golang paths provided, not
> too big a deal, but imho something best decided beforehand.

should this binary have a golang(...)-like name resolvable via golang(...) macro? I've read golang binaries can be named without golang-github-* prefix so this won't suffer from renaming (also this do not provide a devel package) but just let me know and I'll move the repository as fast as I can and fix what's needed
Comment 6 Jan Chaloupka 2016-01-28 09:01:02 EST
Wondering if it would be better to create github repository (e.g. fedora/golang-reviews) and instead of posting links to spec to post links to pull request and comment in the PR. Missing feature to comment lines of the spec file.

Well, the devel subpackage is usefull for analysis. When the devel subpackage is present, it can be scanned for dependencies and other info about the code. At the moment we are running simple scans of new builds of golang projects in Koji. In future, we plan to report missing or broken dependencies. So if you provide the devel subpackage, you get automatic scans and reports about health of your package.

Second, I would recommend to add some macro at the top of the spec. E.g. commit, provider_prefix, import_path. They are used for automatic updates of spec file (e.g. 'gofed bump') and in analysis (as described above).

Third, you are using 'go build' inside Makefile. So your project can be built on architectures with golang compiler only. If you move the commands into %build section, you can use %gobuild macro a gain support for debugingo and architectures with gcc-go compiler.

You can play with gofed for a while, try to run:
# yum install gofed
$ gofed repo2spec --detect github.com/runcom/skopeo --with-build --with-extra

Bad news is it depends on docker so the devel subpackage will not be complete. The good new is you can build the project from bundled dependencies.

Lokesh, we should definitely do something about the docker. This is another project that depends on it. If this is happening we should at least partially built the package from bundled and partially from debundled deps. At this point this is out of the question as we are still missing automatic tools that would do all the hard work for us.
Comment 7 Jan Chaloupka 2016-01-28 09:01:58 EST
btw. a lot of new dependencies :)

$ gofed repo2spec --detect github.com/runcom/skopeo --with-build --with-extra
Repo URL: github.com/runcom/skopeo
Commit: 572a6b6f537d71f7cabfdcfe185c6d7cb4367272
Name: golang-github-runcom-skopeo

(1/4) Checking if the package already exists in PkgDB
(2/4) Downloading tarball
(3/4) Generating spec file
(4/4) Discovering golang dependencies
Class: github.com/Azure/go-ansiterm (golang-github-Azure-go-ansiterm) PkgDB=False
Class: github.com/Sirupsen/logrus (golang-github-Sirupsen-logrus) PkgDB=True
Class: github.com/codegangsta/cli (golang-github-codegangsta-cli) PkgDB=True
Class: github.com/docker/distribution (golang-github-docker-distribution) PkgDB=False
Class: github.com/docker/docker (docker) PkgDB=False
Class: github.com/docker/engine-api (golang-github-docker-engine-api) PkgDB=False
Class: github.com/docker/go-connections (golang-github-docker-go-connections) PkgDB=False
Class: github.com/docker/go-units (golang-github-docker-go-units) PkgDB=False
Class: github.com/docker/libtrust (golang-github-docker-libtrust) PkgDB=True
Class: github.com/go-check/check (golang-github-go-check-check) PkgDB=False
Class: github.com/gorilla/context (golang-github-gorilla-context) PkgDB=True
Class: github.com/gorilla/mux (golang-github-gorilla-mux) PkgDB=True
Class: github.com/opencontainers/runc (golang-github-opencontainers-runc) PkgDB=False
Class: github.com/vbatts/tar-split (golang-github-vbatts-tar-split) PkgDB=False
Class: golang.org/x/net (golang-googlecode-net) PkgDB=True

Spec file golang-github-runcom-skopeo.spec at /home/jchaloup/Packages/reviews/skopeo/golang-github-runcom-skopeo/fedora/golang-github-runcom-skopeo
Comment 8 Antonio Murdaca 2016-01-28 09:08:39 EST
(In reply to Jan Chaloupka from comment #7)
> btw. a lot of new dependencies :)
> 
> $ gofed repo2spec --detect github.com/runcom/skopeo --with-build --with-extra
> Repo URL: github.com/runcom/skopeo
> Commit: 572a6b6f537d71f7cabfdcfe185c6d7cb4367272
> Name: golang-github-runcom-skopeo
> 
> (1/4) Checking if the package already exists in PkgDB
> (2/4) Downloading tarball
> (3/4) Generating spec file
> (4/4) Discovering golang dependencies
> Class: github.com/Azure/go-ansiterm (golang-github-Azure-go-ansiterm)
> PkgDB=False
> Class: github.com/Sirupsen/logrus (golang-github-Sirupsen-logrus) PkgDB=True
> Class: github.com/codegangsta/cli (golang-github-codegangsta-cli) PkgDB=True
> Class: github.com/docker/distribution (golang-github-docker-distribution)
> PkgDB=False
> Class: github.com/docker/docker (docker) PkgDB=False
> Class: github.com/docker/engine-api (golang-github-docker-engine-api)
> PkgDB=False
> Class: github.com/docker/go-connections
> (golang-github-docker-go-connections) PkgDB=False
> Class: github.com/docker/go-units (golang-github-docker-go-units) PkgDB=False
> Class: github.com/docker/libtrust (golang-github-docker-libtrust) PkgDB=True
> Class: github.com/go-check/check (golang-github-go-check-check) PkgDB=False
> Class: github.com/gorilla/context (golang-github-gorilla-context) PkgDB=True
> Class: github.com/gorilla/mux (golang-github-gorilla-mux) PkgDB=True
> Class: github.com/opencontainers/runc (golang-github-opencontainers-runc)
> PkgDB=False
> Class: github.com/vbatts/tar-split (golang-github-vbatts-tar-split)
> PkgDB=False
> Class: golang.org/x/net (golang-googlecode-net) PkgDB=True
> 
> Spec file golang-github-runcom-skopeo.spec at
> /home/jchaloup/Packages/reviews/skopeo/golang-github-runcom-skopeo/fedora/
> golang-github-runcom-skopeo

Yes, those new dependencies come from the master branch, I can tag a new release and add all those dependencies :)

(In reply to Jan Chaloupka from comment #6)
> Wondering if it would be better to create github repository (e.g.
> fedora/golang-reviews) and instead of posting links to spec to post links to
> pull request and comment in the PR. Missing feature to comment lines of the
> spec file.
> 
> Well, the devel subpackage is usefull for analysis. When the devel
> subpackage is present, it can be scanned for dependencies and other info
> about the code. At the moment we are running simple scans of new builds of
> golang projects in Koji. In future, we plan to report missing or broken
> dependencies. So if you provide the devel subpackage, you get automatic
> scans and reports about health of your package.
> 
> Second, I would recommend to add some macro at the top of the spec. E.g.
> commit, provider_prefix, import_path. They are used for automatic updates of
> spec file (e.g. 'gofed bump') and in analysis (as described above).
> 
> Third, you are using 'go build' inside Makefile. So your project can be
> built on architectures with golang compiler only. If you move the commands
> into %build section, you can use %gobuild macro a gain support for debugingo
> and architectures with gcc-go compiler.
> 
> You can play with gofed for a while, try to run:
> # yum install gofed
> $ gofed repo2spec --detect github.com/runcom/skopeo --with-build --with-extra
> 
> Bad news is it depends on docker so the devel subpackage will not be
> complete. The good new is you can build the project from bundled
> dependencies.
> 
> Lokesh, we should definitely do something about the docker. This is another
> project that depends on it. 

> If this is happening we should at least
> partially built the package from bundled and partially from debundled deps.

This is what I'm actually doing - I'm removing each bundled deps with rm -rf /vendor and just leave under vendor the ones from docker which cannot be debundled :)

> At this point this is out of the question as we are still missing automatic
> tools that would do all the hard work for us.
Comment 9 Jan Chaloupka 2016-01-28 09:09:43 EST
Some of them are already packaged, just with a different name.

> should this binary have a golang(...)-like name resolvable via golang(...)
> macro? 

No. golang(...) virtual provides is used only for dependencies

> I've read golang binaries can be named without golang-github-* prefix so
> this won't suffer from renaming

Yeap. It can be named anyway you wish. All projects packaged in fedora with intent to provide at least one binary are named by repository. The standard way is to use project name.

> (also this do not provide a devel package)

As I mentioned above. At least you can provide devel package with no [Build]Requires for sake of analysis. The generated spec file already provides all code necessary. Just with some small modifications.

> but just let me know and I'll move the repository as fast as I can
> and fix what's needed

If you use generated macros, it is two-liner fix
Comment 10 Upstream Release Monitoring 2016-01-28 12:44:12 EST
runcom's scratch build of skopeo-0.1.3-0.1.git572a6b6.fc23.src.rpm for f23 completed http://koji.fedoraproject.org/koji/taskinfo?taskID=12713132
Comment 11 Antonio Murdaca 2016-01-28 13:12:47 EST
Jan, Lokesh, I should have fixed everything now.

The only thing which doesn't work is removing the `cd $(pwd)/_build/...)` stuff in %install (as Jan suggested me to do) because I believe it is tied to $GO15VENDOREXPERIMENT and "go build" only works with "vendor/" if the current source being built lives under $GOPATH. (you can try this and see it can't resolves package dependencies)

I've also tagged a new release (v0.1.3)

Spec URL: https://github.com/runcom/skopeo/blob/master/skopeo.spec
SRPM URL: http://runcom.ninja/skopeo-0.1.3-0.1.gitfdb5cac.fc23.src.rpm

Koji builds:

- f23:     http://koji.fedoraproject.org/koji/taskinfo?taskID=12713377
- rawhide: http://koji.fedoraproject.org/koji/taskinfo?taskID=12713364
Comment 12 Upstream Release Monitoring 2016-01-28 18:07:39 EST
runcom's scratch build of skopeo-0.1.3-1.fc23.src.rpm for f23 completed http://koji.fedoraproject.org/koji/taskinfo?taskID=12716264
Comment 14 Upstream Release Monitoring 2016-01-28 18:10:12 EST
runcom's scratch build of skopeo-0.1.3-1.fc23.src.rpm for f23 completed http://koji.fedoraproject.org/koji/taskinfo?taskID=12716275
Comment 15 Nalin Dahyabhai 2016-01-28 18:31:11 EST
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.

  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?

- 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.

- 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.

===== 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.

[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?

[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}.

[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.

[-]: 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.

[-]: 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.

[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
Comment 16 Antonio Murdaca 2016-01-29 04:25:08 EST
(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
Comment 18 Upstream Release Monitoring 2016-01-29 04:30:03 EST
runcom's scratch build of skopeo-0.1.4-1.fc23.src.rpm for f23 completed http://koji.fedoraproject.org/koji/taskinfo?taskID=12722010
Comment 19 Jan Chaloupka 2016-01-29 08:47:51 EST
After reading [1] I can not figure out why is GO15VENDOREXPERIMENT env needed. Can you help me to understand?

[1] https://docs.google.com/document/d/1Bz5-UB7g2uPBdOx-rw5t9MxJwkfpx90cqG9AFL0JAYo/edit
Comment 20 Antonio Murdaca 2016-01-29 09:25:16 EST
(In reply to Jan Chaloupka from comment #19)
> After reading [1] I can not figure out why is GO15VENDOREXPERIMENT env
> needed. Can you help me to understand?
> 
> [1]
> https://docs.google.com/document/d/1Bz5-UB7g2uPBdOx-
> rw5t9MxJwkfpx90cqG9AFL0JAYo/edit

Quoting from the doc:

This change will only be enabled if the go command is run with GO15VENDOREXPERIMENT=1 in its environment.

So, the behavior of looking into $pwd/vendor/... instead of resolving deps from $GOPATH is only enabled if you pass that env variable - otherwise it uses $GOPATH (and maybe $GOROOT). This is in go1.5, eventually we will have this as the default behavior in go1.6, from the doc:

If we decide that the vendor behavior is correct, then in a later release (possibly Go 1.6) we would make the vendor behavior default on. Projects containing “vendor” directories could still use “GO15VENDOREXPERIMENT=0” to get the old behavior while they convert their code. In a still later release (possibly Go 1.7) we would remove the use of the environment variable, locking in the vendoring semantics.
Comment 21 Nalin Dahyabhai 2016-01-29 17:51:27 EST
(In reply to Antonio Murdaca from comment #16)
> 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

FWIW, Fedora's packaging repository will keep its own copy in a branch for each Fedora release (see repositories at http://pkgs.fedoraproject.org/cgit/rpms/ for examples).

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

Yeah, that should be accurate either way.

> 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

Understood, it's a work in progress.

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

Historically, %clean's job was to clean up $RPM_BUILD_ROOT, way back when even using a buildroot was optional.  Now that the buildroot is set and cleaned up by default, Fedora doesn't expect you to have a %clean section.
Comment 22 Nalin Dahyabhai 2016-02-02 05:49:06 EST
Looks good, though the -devel package contains sources in package "main".  My guess is that you wanted to leave with_devel set to 0 all the time.
Comment 23 Upstream Release Monitoring 2016-02-02 07:37:47 EST
runcom's scratch build of skopeo-0.1.4-1.fc23.src.rpm for f23 completed http://koji.fedoraproject.org/koji/taskinfo?taskID=12786664
Comment 25 Nalin Dahyabhai 2016-02-02 07:40:08 EST
Yup, looks good from here.  Thanks!
Comment 26 Gwyn Ciesla 2016-02-03 08:25:04 EST
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/skopeo

Note You need to log in before you can comment on or make changes to this bug.