Bug 1441843

n.b. If you're testing basic functionality, this program only runs on Fedora 26+ due to its dependency versions.

I'll take this

Created attachment 1276746 [details]
rpmlint output

Ok, we've got the dependencies in Fedora, so let's do this.  There are a couple things that jump out at me when I look at this package.

The first is that the rpmlint output (attached) indicates that there are a whole lot of python modules with shebangs (though only some are marked as executable).  I would think that all of the shebangs should be removed, and all given 0644 permissions, unless there's a reason for them to be executable.  There's also one binary that's missing its shebang, and it should be added.

The second is that this package includes python modules in site-packages/test and site-packages/cli, both which seem to be very generic names.  Is this intentional?

I'll update the shebang/permissions issues, including sending a PR upstream.

As for the packages, this issue has been addressed in upstream and will be released in their 1.0 milestone (due out the end of this month). All of those will be moved into a top level "linchpin" package by that update.

Ok, if it's all right with you, then, I'll go ahead and do the review once you've published the 1.0 package.

Agreed.

OK, the shebang and spurious file permissions are easy enough to fix. I'm curious which executable you think should have a shebang. Is it linchpin_complete.sh? Those completion scripts are not supposed to be she-bang lines. But I'm also not sure that linchpin is installing them in the proper location. (currently it's installed to /usr/bin/linchpin_complete.sh)

Yep, that would be the one.  If it's a script that's meant to run by linchpin (as opposed to the user), then it should probably go in /usr/libexec/linchpin.  As for whether or not it needs a shebang, if it's explicitly run with an interpreter, you could probably go without, but otherwise I think it makes more sense to have one.

It's a Bash completion script, so it's not intended to be run at all, rather sourced.

It looks like that should be placed in /etc/bash_completion.d/ rather than in /usr/bin. I'll update to have that moved, as well.

Yeah, sounds good.

Any news on this?  If not, I'll go ahead and remove myself from this and leave the bug as NEW.

OK, I guess I better do this. Upstream started asking again if I was going to finish it up. Stay tuned, Friday is my designated Fedora Package day for the week.

OK, giving this another go:
Spec file: https://fedorapeople.org/~greghellings/linchpin/linchpin.spec
SRPM: https://fedorapeople.org/~greghellings/linchpin/linchpin-1.5.4-1.fc28.src.rpm

The %prep section now includes removal of shebang lines from .py files
The %install section removes some dev-only shell scripts that get installed erroneously and removes the spurious executable flag on the installed files

Upstream has fixed most of the rest of the issues, such as not having a top level package, installing bash completion scripts erroneously (they're now removed), etc.

Jonathan, any chance you could look at this or would you rather let someone else tackle it?

Yeah, I'll go ahead and do this.  Sorry about the delay in getting back to you.

Created attachment 1481259 [details]
Build log

Just tried to build this in mock and got a failure.  It looks like it's trying to download a dependency and mock (and the Fedora builders) don't allow network access when building.

Ah, Rawhide Ansible package has shifted to Python 3, thus invalidating all of the Python 2 BRs. I'll update as appropriate and post a new SRPM in a few minutes.

New SRPM: https://fedorapeople.org/~greghellings/linchpin/linchpin-1.6.1-1.fc30.src.rpm

I updated all Python dependencies to Python 3.
There were some dependency version problems in Rawhide with 1.5.4 but upstream has resolved them so I updated to 1.6.1 to handle that.

Great.  Do you mind posting the updated spec too?  It makes it easier for fedora-review to run.

Oh, yeah. I push them both every time, but the URL didn't change:

Spec: https://fedorapeople.org/~greghellings/linchpin/linchpin.spec
SRPM: https://fedorapeople.org/~greghellings/linchpin/linchpin-1.6.1-1.fc30.src.rpm

Great, thanks.

rpmlint is complaining about a number of empty hidden files in the RPM.  Do you know why they're there?  If they're actually there for a reason, then we'll ignore rpmlint, but if they're something left over from the build process, there's no reason to have them.

linchpin.noarch: E: zero-length /usr/lib/python3.7/site-packages/linchpin/defaults/layouts/README.rst
linchpin.noarch: E: non-executable-script /usr/lib/python3.7/site-packages/linchpin/provision/roles/libvirt/templates/virt_customize_user_creation.sh.j2 644 /bin/sh 
linchpin.noarch: W: hidden-file-or-dir /usr/lib/python3.7/site-packages/linchpin/templates/credentials/.empty
linchpin.noarch: E: zero-length /usr/lib/python3.7/site-packages/linchpin/templates/credentials/.empty
linchpin.noarch: W: hidden-file-or-dir /usr/lib/python3.7/site-packages/linchpin/templates/hooks/.empty
linchpin.noarch: E: zero-length /usr/lib/python3.7/site-packages/linchpin/templates/hooks/.empty
linchpin.noarch: W: hidden-file-or-dir /usr/lib/python3.7/site-packages/linchpin/templates/inventories/.empty
linchpin.noarch: E: zero-length /usr/lib/python3.7/site-packages/linchpin/templates/inventories/.empty
linchpin.noarch: E: zero-length /usr/lib/python3.7/site-packages/linchpin/tests/cli/test_context_fail.py
linchpin.noarch: W: hidden-file-or-dir /usr/lib/python3.7/site-packages/linchpin/tests/mockdata/dummy/fetch/ws2/credentials/.empty
linchpin.noarch: E: zero-length /usr/lib/python3.7/site-packages/linchpin/tests/mockdata/dummy/fetch/ws2/credentials/.empty
linchpin.noarch: W: hidden-file-or-dir /usr/lib/python3.7/site-packages/linchpin/tests/mockdata/dummy/fetch/ws2/hooks/.empty
linchpin.noarch: E: zero-length /usr/lib/python3.7/site-packages/linchpin/tests/mockdata/dummy/fetch/ws2/hooks/.empty
linchpin.noarch: W: hidden-file-or-dir /usr/lib/python3.7/site-packages/linchpin/tests/mockdata/dummy/fetch/ws2/inventories/.empty
linchpin.noarch: E: zero-length /usr/lib/python3.7/site-packages/linchpin/tests/mockdata/dummy/fetch/ws2/inventories/.empty
linchpin.noarch: W: hidden-file-or-dir /usr/lib/python3.7/site-packages/linchpin/tests/mockdata/dummy/fetch/ws2/resources/.empty
linchpin.noarch: E: zero-length /usr/lib/python3.7/site-packages/linchpin/tests/mockdata/dummy/fetch/ws2/resources/.empty
linchpin.noarch: W: hidden-file-or-dir /usr/lib/python3.7/site-packages/linchpin/tests/mockdata/dummy/inventories/.empty
linchpin.noarch: E: zero-length /usr/lib/python3.7/site-packages/linchpin/tests/mockdata/dummy/inventories/.empty
linchpin.noarch: W: hidden-file-or-dir /usr/lib/python3.7/site-packages/linchpin/tests/mockdata/dummy/resources/.empty
linchpin.noarch: E: zero-length /usr/lib/python3.7/site-packages/linchpin/tests/mockdata/dummy/resources/.empty

(In reply to Jonathan Dieter from comment #21)
> Great, thanks.
> 
> rpmlint is complaining about a number of empty hidden files in the RPM.  Do
> you know why they're there?  If they're actually there for a reason, then
> we'll ignore rpmlint, but if they're something left over from the build
> process, there's no reason to have them.

Any news on this?

(In reply to Jonathan Dieter from comment #22)
> (In reply to Jonathan Dieter from comment #21)
> > Great, thanks.
> > 
> > rpmlint is complaining about a number of empty hidden files in the RPM.  Do
> > you know why they're there?  If they're actually there for a reason, then
> > we'll ignore rpmlint, but if they're something left over from the build
> > process, there's no reason to have them.
> 
> Any news on this?

Ping

Sorry, I was waiting for 1.6.2 to land as well. I've cleaned up most of the zero length file reports. The ".empty" files are a remnant from git and are now deleted after install. I've also opted to delete the "tests" directory from being installed. There is still a zero-length Python file which should remain and also a file with the ".sh" extension that is not flagged as executable. This is by intention, as the file is actually a file template and the resulting templated file will be made executable during the runtime of linchpin.

Spec: https://fedorapeople.org/~greghellings/linchpin/linchpin.spec
SRPM: https://fedorapeople.org/~greghellings/linchpin/linchpin-1.6.2-1.fc30.src.rpm

Ok, there still a few issues.  First off, it looks like there's documentation in source, but no documentation seems to be installed as part of the package.  Am I missing something?  If not, can we please include the documentation?

Second, the license needs to be included in the RPM using the %license tag.

Finally, and most frustratingly, the package requires python3-krbV, which doesn't seem to exist in Fedora.

The docs are to build the readthedocs page. They don't build in Python 3 due to issues with imports. I have reported this upstream as https://github.com/CentOS-PaaS-SIG/linchpin/issues/701

I've added the LICENSE file to the file list

The dependency on krbV is an optional dependency for running linchpin against some of its infrastructure providers. I've removed it from the RPM. If that package is ever updated to build against Python 3, then I'll add it back in at that time.

Spec: https://fedorapeople.org/~greghellings/linchpin/linchpin.spec
SRPM: https://fedorapeople.org/~greghellings/linchpin/linchpin-1.6.2-2.fc30.src.rpm

(In reply to greg.hellings from comment #26)
> The docs are to build the readthedocs page. They don't build in Python 3 due
> to issues with imports. I have reported this upstream as
> https://github.com/CentOS-PaaS-SIG/linchpin/issues/701

Ok, that makes sense.  Could we at least get README.rst into the documentation so there's at least something on the system to get people started?

> I've added the LICENSE file to the file list
> 
> The dependency on krbV is an optional dependency for running linchpin
> against some of its infrastructure providers. I've removed it from the RPM.
> If that package is ever updated to build against Python 3, then I'll add it
> back in at that time.

Sounds good.

Upstream release 1.6.4 today which fixes the doc build issue. I've uploaded an SRPM that takes advantage of that to include a docs subpackage as well as adding the README.rst to as a %doc inclusion.

Spec: https://fedorapeople.org/~greghellings/linchpin/linchpin.spec
SRPM: https://fedorapeople.org/~greghellings/linchpin/linchpin-1.6.4-1.fc30.src.rpm

Nicely done, but the requires for the -docs is wrong.  linchpin is noarch, so it should be Requires: %{name} = %{version}-%{release}.  As it is, I'm unable to actually install linchpin-docs.

Also, not a blocker, but the guidelines recommend that documentation be under a -doc subpackage, not a -docs subpackage.

Good catch on the docs/doc difference. I've updated the name of that and fixed its depends. Updated versions of the files pushed:

Spec: https://fedorapeople.org/~greghellings/linchpin/linchpin.spec
SRPM: https://fedorapeople.org/~greghellings/linchpin/linchpin-1.6.4-2.fc30.src.rpm

Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated


===== MUST items =====

Issues:
[!]: License field in the package spec file matches the actual license.
     License should be GPLv3+ and BSD

[!]: Requires correct, justified where necessary.
     The comment below indicates that these requirements might belong to
     the linchpin-doc subpackage?  If so, they need to be listed as
     requirements for the subpackage, not the primary package
       # Extra sub-package includes
       Requires:   beaker-client
       Requires:   python3-libvirt
       Requires:   python3-lxml

Generic:
[x]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[x]: License file installed when any subpackage combination is installed.
[x]: Package contains no bundled libraries without FPC exception.
[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.
[x]: Package consistently uses macros (instead of hard-coded directory
     names).
[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.
[x]: Spec file is legible and written in American English.
[-]: Package contains systemd file(s) if in need.
[x]: Package is not known to require an ExcludeArch tag.
[x]: Large documentation must go in a -doc subpackage.
[x]: Package complies to the Packaging Guidelines
[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]: 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.
[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 uses either %{buildroot} or $RPM_BUILD_ROOT
[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]: Sources used to build the package match the upstream source, as
     provided in the spec URL.
[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

Python:
[x]: Python eggs must not download any dependencies during the build
     process.
[x]: A package which is used by another package via an egg interface should
     provide egg info.
[x]: Package meets the Packaging Guidelines::Python
[x]: Package contains BR: python2-devel or python3-devel
[x]: Binary eggs must be removed in %prep

===== SHOULD items =====

Issues:

[!]: Avoid bundling fonts in non-fonts packages.
     Note: linchpin-doc contains font files.  If it's possible to add them
           as dependencies instead, we really should

[!]: If you build a python module you should use the %python_provide macro
     I'm not completely clear if this is meant to be imported by other
     programs, but, if so, we should be using %python_provide

[!]: Package functions as described.
     When I run linchpin, the following happens:
        $ linchpin
        Traceback (most recent call last):
          File "/usr/bin/linchpin", line 11, in <module>
            load_entry_point('linchpin==1.6.4', 'console_scripts',
                             'linchpin')()
          File "/usr/lib/python3.7/site-packages/pkg_resources/__init__.py",
               line 484, in load_entry_point
            return get_distribution(dist).load_entry_point(group, name)
          File "/usr/lib/python3.7/site-packages/pkg_resources/__init__.py",
               line 2714, in load_entry_point
            return ep.load()
          File "/usr/lib/python3.7/site-packages/pkg_resources/__init__.py",
               line 2332, in load
            return self.resolve()
          File "/usr/lib/python3.7/site-packages/pkg_resources/__init__.py",
               line 2338, in resolve
            module = __import__(self.module_name, fromlist=['__name__'],
                                level=0)
          File "/usr/lib/python3.7/site-packages/linchpin/shell/__init__.py",
               line 12, in <module>
            from linchpin.cli import LinchpinCli
          File "/usr/lib/python3.7/site-packages/linchpin/cli/__init__.py",
               line 16, in <module>
            from linchpin.fetch import FETCH_CLASS
          File "/usr/lib/python3.7/site-packages/linchpin/fetch/__init__.py",
               line 1, in <module>
            from fetch_local import FetchLocal
        ModuleNotFoundError: No module named 'fetch_local'

Generic:
[-]: Uses parallel make %{?_smp_mflags} macro.
[-]: 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]: 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.
[x]: Package should compile and build into binary rpms on all supported
     architectures.
[-]: %check is present and all tests pass.
[x]: Packages should try to preserve timestamps of original installed
     files.
[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]: Fully versioned dependency in subpackages if applicable.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: Sources can be downloaded from URI in Source: tag
[x]: SourceX is a working URL.
[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]: Spec file according to URL is the same as in SRPM.


Rpmlint
-------
Checking: linchpin-1.6.4-2.fc30.noarch.rpm
          linchpin-doc-1.6.4-2.fc30.noarch.rpm
          linchpin-1.6.4-2.fc30.src.rpm
linchpin.noarch: W: spelling-error Summary(en_US) Ansible -> Expansible, Sensible
linchpin.noarch: W: spelling-error Summary(en_US) multicloud -> multicolored
linchpin.noarch: W: spelling-error Summary(en_US) orchestrator -> orchestra tor, orchestra-tor, orchestrate
linchpin.noarch: E: zero-length /usr/lib/python3.7/site-packages/linchpin/defaults/layouts/README.rst
linchpin.noarch: E: non-executable-script /usr/lib/python3.7/site-packages/linchpin/provision/roles/libvirt/templates/virt_customize_user_creation.sh.j2 644 /bin/sh 
linchpin.noarch: W: no-manual-page-for-binary linchpin
linchpin.src: W: spelling-error Summary(en_US) Ansible -> Expansible, Sensible
linchpin.src: W: spelling-error Summary(en_US) multicloud -> multicolored
linchpin.src: W: spelling-error Summary(en_US) orchestrator -> orchestra tor, orchestra-tor, orchestrate
3 packages and 0 specfiles checked; 2 errors, 7 warnings.




Rpmlint (installed packages)
----------------------------
sh: /usr/bin/python: No such file or directory
linchpin-doc.noarch: W: invalid-url URL: http://github.com/CentOS-PaaS-SIG/linch-pin <urlopen error [Errno -2] Name or service not known>
linchpin.noarch: W: spelling-error Summary(en_US) Ansible -> Expansible, Sensible
linchpin.noarch: W: spelling-error Summary(en_US) multicloud -> multicolored
linchpin.noarch: W: spelling-error Summary(en_US) orchestrator -> orchestra tor, orchestra-tor, orchestrate
linchpin.noarch: W: invalid-url URL: http://github.com/CentOS-PaaS-SIG/linch-pin <urlopen error [Errno -2] Name or service not known>
linchpin.noarch: E: zero-length /usr/lib/python3.7/site-packages/linchpin/defaults/layouts/README.rst
linchpin.noarch: E: non-executable-script /usr/lib/python3.7/site-packages/linchpin/provision/roles/libvirt/templates/virt_customize_user_creation.sh.j2 644 /bin/sh 
linchpin.noarch: W: no-manual-page-for-binary linchpin
2 packages and 0 specfiles checked; 2 errors, 6 warnings.



Requires
--------
linchpin-doc (rpmlib, GLIBC filtered):
    linchpin

linchpin (rpmlib, GLIBC filtered):
    /usr/bin/python3
    ansible
    beaker-client
    python(abi)
    python3-Naked
    python3-boto
    python3-cerberus
    python3-click
    python3-libcloud
    python3-libvirt
    python3-lxml
    python3-pyOpenSSL
    python3-pyyaml
    python3-requests
    python3-shade
    python3-six
    python3-tinydb
    python3-yamlordereddictloader



Provides
--------
linchpin-doc:
    linchpin-doc

linchpin:
    linchpin
    python3.7dist(linchpin)
    python3dist(linchpin)



Source checksums
----------------
https://github.com/CentOS-PaaS-SIG/linch-pin/archive/v1.6.4.tar.gz :
  CHECKSUM(SHA256) this package     : 74c33ef24dc3f537debc0b8e7de1e6e0ef1c7959ba27199a4f1bc1b2860051db
  CHECKSUM(SHA256) upstream package : 74c33ef24dc3f537debc0b8e7de1e6e0ef1c7959ba27199a4f1bc1b2860051db

The issues that still need to be resolved:

[!]: License field in the package spec file matches the actual license.
     License should be GPLv3+ and BSD

[!]: Requires correct, justified where necessary.
     The comment below indicates that these requirements might belong to
     the linchpin-doc subpackage?  If so, they need to be listed as
     requirements for the subpackage, not the primary package
       # Extra sub-package includes
       Requires:   beaker-client
       Requires:   python3-libvirt
       Requires:   python3-lxml


[!]: Avoid bundling fonts in non-fonts packages.
     Note: linchpin-doc contains font files.  If it's possible to add them
           as dependencies instead, we really should

[!]: If you build a python module you should use the %python_provide macro
     I'm not completely clear if this is meant to be imported by other
     programs, but, if so, we should be using %python_provide

[!]: Package functions as described.
     When I run linchpin, the following happens:
        $ linchpin
        Traceback (most recent call last):
          File "/usr/bin/linchpin", line 11, in <module>
            load_entry_point('linchpin==1.6.4', 'console_scripts',
                             'linchpin')()
          File "/usr/lib/python3.7/site-packages/pkg_resources/__init__.py",
               line 484, in load_entry_point
            return get_distribution(dist).load_entry_point(group, name)
          File "/usr/lib/python3.7/site-packages/pkg_resources/__init__.py",
               line 2714, in load_entry_point
            return ep.load()
          File "/usr/lib/python3.7/site-packages/pkg_resources/__init__.py",
               line 2332, in load
            return self.resolve()
          File "/usr/lib/python3.7/site-packages/pkg_resources/__init__.py",
               line 2338, in resolve
            module = __import__(self.module_name, fromlist=['__name__'],
                                level=0)
          File "/usr/lib/python3.7/site-packages/linchpin/shell/__init__.py",
               line 12, in <module>
            from linchpin.cli import LinchpinCli
          File "/usr/lib/python3.7/site-packages/linchpin/cli/__init__.py",
               line 16, in <module>
            from linchpin.fetch import FETCH_CLASS
          File "/usr/lib/python3.7/site-packages/linchpin/fetch/__init__.py",
               line 1, in <module>
            from fetch_local import FetchLocal
        ModuleNotFoundError: No module named 'fetch_local'

> The issues that still need to be resolved:
> 
> [!]: License field in the package spec file matches the actual license.
>      License should be GPLv3+ and BSD

I'm not sure why you're flagging it as needing BSD license. I don't see any reference to the BSD license in the code, am I missing it somewhere?


> 
> [!]: Requires correct, justified where necessary.
>      The comment below indicates that these requirements might belong to
>      the linchpin-doc subpackage?  If so, they need to be listed as
>      requirements for the subpackage, not the primary package
>        # Extra sub-package includes
>        Requires:   beaker-client
>        Requires:   python3-libvirt
>        Requires:   python3-lxml
> 

The Requires listed are from the optional extras python packages. In pip these would be installed with `pip install linhcpin[beaker]` or `pip install linchpin[libvirt]`. They could be listed as "Recommends" or such, but the product would be completely non-functional without them so I opted to add them directly.


> 
> [!]: Avoid bundling fonts in non-fonts packages.
>      Note: linchpin-doc contains font files.  If it's possible to add them
>            as dependencies instead, we really should

The docs are intended to be opened from a browser and/or served from a static website pointing to the root of the docs folder. I don't know how easily those files can be made to depend on each other. It might require me to post-process the HTML to point it at the files in the python3-sphinx_rtd_theme package (which is the Sphix theme used to build these docs). I'll give that some thought and experimentation.


> 
> [!]: If you build a python module you should use the %python_provide macro
>      I'm not completely clear if this is meant to be imported by other
>      programs, but, if so, we should be using %python_provide

We are not building a Python module that is intended to be importable by another Python package. This is really just providing the executable in a Python wrapper is that all of its dependencies can easily be managed. I can still add the python provides if it should be there.(In reply to Jonathan Dieter from comment #32)

> 
> [!]: Package functions as described.
>      When I run linchpin, the following happens:
>         $ linchpin
>         Traceback (most recent call last):
>           File "/usr/bin/linchpin", line 11, in <module>
>             load_entry_point('linchpin==1.6.4', 'console_scripts',
>                              'linchpin')()
>           File "/usr/lib/python3.7/site-packages/pkg_resources/__init__.py",
>                line 484, in load_entry_point
>             return get_distribution(dist).load_entry_point(group, name)
>           File "/usr/lib/python3.7/site-packages/pkg_resources/__init__.py",
>                line 2714, in load_entry_point
>             return ep.load()
>           File "/usr/lib/python3.7/site-packages/pkg_resources/__init__.py",
>                line 2332, in load
>             return self.resolve()
>           File "/usr/lib/python3.7/site-packages/pkg_resources/__init__.py",
>                line 2338, in resolve
>             module = __import__(self.module_name, fromlist=['__name__'],
>                                 level=0)
>           File "/usr/lib/python3.7/site-packages/linchpin/shell/__init__.py",
>                line 12, in <module>
>             from linchpin.cli import LinchpinCli
>           File "/usr/lib/python3.7/site-packages/linchpin/cli/__init__.py",
>                line 16, in <module>
>             from linchpin.fetch import FETCH_CLASS
>           File "/usr/lib/python3.7/site-packages/linchpin/fetch/__init__.py",
>                line 1, in <module>
>             from fetch_local import FetchLocal
>         ModuleNotFoundError: No module named 'fetch_local'

Well that's disturbing. The upstream package seems to suffer from the same issue when installed directly with pip to a virtualenv, and the problem persists in the latest bugfix. I've reported this upstream as https://github.com/CentOS-PaaS-SIG/linchpin/issues/853 and am looking into a fix.

(In reply to greg.hellings from comment #33)
> > The issues that still need to be resolved:
> > 
> > [!]: License field in the package spec file matches the actual license.
> >      License should be GPLv3+ and BSD
> 
> I'm not sure why you're flagging it as needing BSD license. I don't see any
> reference to the BSD license in the code, am I missing it somewhere?

linchpin-1.6.4/linchpin/shell/click_default_group.py

is under a BSD license

> > [!]: Requires correct, justified where necessary.
> >      The comment below indicates that these requirements might belong to
> >      the linchpin-doc subpackage?  If so, they need to be listed as
> >      requirements for the subpackage, not the primary package
> >        # Extra sub-package includes
> >        Requires:   beaker-client
> >        Requires:   python3-libvirt
> >        Requires:   python3-lxml
> > 
> 
> The Requires listed are from the optional extras python packages. In pip
> these would be installed with `pip install linhcpin[beaker]` or `pip install
> linchpin[libvirt]`. They could be listed as "Recommends" or such, but the
> product would be completely non-functional without them so I opted to add
> them directly.

Ok, that's grand. 

> > [!]: Avoid bundling fonts in non-fonts packages.
> >      Note: linchpin-doc contains font files.  If it's possible to add them
> >            as dependencies instead, we really should
> 
> The docs are intended to be opened from a browser and/or served from a
> static website pointing to the root of the docs folder. I don't know how
> easily those files can be made to depend on each other. It might require me
> to post-process the HTML to point it at the files in the
> python3-sphinx_rtd_theme package (which is the Sphix theme used to build
> these docs). I'll give that some thought and experimentation.

Thanks so much.  I'd definitely feel better about not bundling the font files if it's at all possible to avoid.

> > [!]: If you build a python module you should use the %python_provide macro
> >      I'm not completely clear if this is meant to be imported by other
> >      programs, but, if so, we should be using %python_provide
> 
> We are not building a Python module that is intended to be importable by
> another Python package. This is really just providing the executable in a
> Python wrapper is that all of its dependencies can easily be managed. I can
> still add the python provides if it should be there.

No, given your explanation, I don't think that's necessary.

(In reply to Jonathan Dieter from comment #32)
> 
> > 
> > [!]: Package functions as described.
> >      When I run linchpin, the following happens:
> >         $ linchpin
> >         Traceback (most recent call last):
> >           File "/usr/bin/linchpin", line 11, in <module>
> >             load_entry_point('linchpin==1.6.4', 'console_scripts',
> >                              'linchpin')()
> >           File "/usr/lib/python3.7/site-packages/pkg_resources/__init__.py",
> >                line 484, in load_entry_point
> >             return get_distribution(dist).load_entry_point(group, name)
> >           File "/usr/lib/python3.7/site-packages/pkg_resources/__init__.py",
> >                line 2714, in load_entry_point
> >             return ep.load()
> >           File "/usr/lib/python3.7/site-packages/pkg_resources/__init__.py",
> >                line 2332, in load
> >             return self.resolve()
> >           File "/usr/lib/python3.7/site-packages/pkg_resources/__init__.py",
> >                line 2338, in resolve
> >             module = __import__(self.module_name, fromlist=['__name__'],
> >                                 level=0)
> >           File "/usr/lib/python3.7/site-packages/linchpin/shell/__init__.py",
> >                line 12, in <module>
> >             from linchpin.cli import LinchpinCli
> >           File "/usr/lib/python3.7/site-packages/linchpin/cli/__init__.py",
> >                line 16, in <module>
> >             from linchpin.fetch import FETCH_CLASS
> >           File "/usr/lib/python3.7/site-packages/linchpin/fetch/__init__.py",
> >                line 1, in <module>
> >             from fetch_local import FetchLocal
> >         ModuleNotFoundError: No module named 'fetch_local'
> 
> Well that's disturbing. The upstream package seems to suffer from the same
> issue when installed directly with pip to a virtualenv, and the problem
> persists in the latest bugfix. I've reported this upstream as
> https://github.com/CentOS-PaaS-SIG/linchpin/issues/853 and am looking into a
> fix.

Thanks.

Greg, last I checked, we were almost finished with this.  Are you still interested in moving forward with this?

I am, but the 1.7 series (the latest release of linchpin) requires Shade >= 1.30.0. The latest version packaged for Rawhide is 1.27.

Updated this to latest version, which now fully supports Python 3 and drops Python 2 support.

Scratch build: https://koji.fedoraproject.org/koji/taskinfo?taskID=43882055
SRPM: https://kojipkgs.fedoraproject.org//work/tasks/2057/43882057/linchpin-2.0.0-1.fc33.src.rpm
Spec file: https://fedorapeople.org/~greghellings/linchpin/linchpin.spec

Hopefully this journey is finally underway!

I'd really like to see this move forward, but I can't even install it right now, in either Rawhide or F32:

sudo /usr/bin/dnf --installroot /var/lib/mock/fedora-rawhide-x86_64/root/ --releasever 33 --setopt=deltarpm=False --allowerasing --disableplugin=local --disableplugin=spacewalk install /home/jonathan/Documents/programming/fedora/1441843-linchpin/results/linchpin-2.0.0-1.fc33.noarch.rpm --setopt=tsflags=nocontexts
[sudo] password for jonathan: 
No matches found for the following disable plugin patterns: local, spacewalk
fedora                                                                                                                                        11 MB/s |  72 MB     00:06    
Last metadata expiration check: 0:00:20 ago on Wed 06 May 2020 08:57:15 PM IST.
Error: 
 Problem: conflicting requests
  - nothing provides (python3.8dist(ansible) <= 2.9 with python3.8dist(ansible) >= 2.7.1) needed by linchpin-2.0.0-1.fc33.noarch
  - nothing provides (python3.8dist(urllib3) < 1.25 with python3.8dist(urllib3) >= 1.23) needed by linchpin-2.0.0-1.fc33.noarch
  - nothing provides python3.8dist(ipaddress) >= 1.0.17 needed by linchpin-2.0.0-1.fc33.noarch
  - nothing provides python3.8dist(openstacksdk) >= 0.37 needed by linchpin-2.0.0-1.fc33.noarch
  - nothing provides python3.8dist(pyzmq) = 18.1.1 needed by linchpin-2.0.0-1.fc33.noarch
  - nothing provides python3.8dist(tqdm) = 4.36.1 needed by linchpin-2.0.0-1.fc33.noarch

sudo dnf install /home/jonathan/Documents/programming/fedora/1441843-linchpin-32/results/linchpin-2.0.0-1.fc32.noarch.rpm
Error: 
 Problem: conflicting requests
  - nothing provides (python3.8dist(ansible) <= 2.9 with python3.8dist(ansible) >= 2.7.1) needed by linchpin-2.0.0-1.fc32.noarch
  - nothing provides (python3.8dist(urllib3) >= 1.23 with python3.8dist(urllib3) < 1.25) needed by linchpin-2.0.0-1.fc32.noarch
  - nothing provides python3.8dist(gitdb) >= 0.6.4 needed by linchpin-2.0.0-1.fc32.noarch
  - nothing provides python3.8dist(ipaddress) >= 1.0.17 needed by linchpin-2.0.0-1.fc32.noarch
  - nothing provides python3.8dist(openstacksdk) >= 0.37 needed by linchpin-2.0.0-1.fc32.noarch
  - nothing provides python3.8dist(pyzmq) = 18.1.1 needed by linchpin-2.0.0-1.fc32.noarch
  - nothing provides python3.8dist(tqdm) = 4.36.1 needed by linchpin-2.0.0-1.fc32.noarch
(try to add '--skip-broken' to skip uninstallable packages)

Weird. I'm definitely not generating those narrow restrictions of versions. Any idea where they might be coming from?

They're coming from requirements.txt.  See https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#_automatically_generated_dependencies

In that case I'm going to abandon this review request. I've fought with the linchpin team for years about capping version requirements, but because of the way they've chosen to interact with Ansible it's not possible for them to avoid it. I'm not going to put up with this fight over and over again.

Man, I'm sorry.  This is really frustrating, and I totally understand why you're throwing in the towel.  :(