Bug 1441843 - Review Request: linchpin - Ansible based multicloud orchestrator
Summary: Review Request: linchpin - Ansible based multicloud orchestrator
Keywords:
Status: ASSIGNED
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Jonathan Dieter
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On: 1432701 python-camel python-yamlordereddictloader
Blocks:
TreeView+ depends on / blocked
 
Reported: 2017-04-12 21:11 UTC by greg.hellings
Modified: 2019-04-08 14:58 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed:


Attachments (Terms of Use)
rpmlint output (7.88 KB, text/plain)
2017-05-06 11:06 UTC, Jonathan Dieter
no flags Details
Build log (19.23 KB, text/plain)
2018-09-06 10:48 UTC, Jonathan Dieter
no flags Details

Description greg.hellings 2017-04-12 21:11:41 UTC
Spec URL: https://fedorapeople.org/~greghellings/linchpin/linchpin.spec
SRPM URL: https://fedorapeople.org/~greghellings/linchpin/linchpin-0.9.1-2.fc26.src.rpm
Description: linchpin is an Ansible-based tool to stand up and tear down
resources in various cloud environments
Fedora Account System Username: greghellings

Comment 1 greg.hellings 2017-04-13 16:16:29 UTC
n.b. If you're testing basic functionality, this program only runs on Fedora 26+ due to its dependency versions.

Comment 2 Jonathan Dieter 2017-04-14 10:08:12 UTC
I'll take this

Comment 3 Jonathan Dieter 2017-05-06 11:06:55 UTC
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?

Comment 4 greg.hellings 2017-05-11 13:35:27 UTC
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.

Comment 5 Jonathan Dieter 2017-05-11 13:38:12 UTC
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.

Comment 6 greg.hellings 2017-05-11 13:49:13 UTC
Agreed.

Comment 7 greg.hellings 2017-05-11 16:07:16 UTC
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)

Comment 8 Jonathan Dieter 2017-05-11 16:17:03 UTC
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.

Comment 9 greg.hellings 2017-05-11 16:31:07 UTC
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.

Comment 10 Jonathan Dieter 2017-05-11 18:40:34 UTC
Yeah, sounds good.

Comment 11 Jonathan Dieter 2018-07-26 11:02:54 UTC
Any news on this?  If not, I'll go ahead and remove myself from this and leave the bug as NEW.

Comment 12 greg.hellings 2018-08-03 15:11:59 UTC
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.

Comment 13 greg.hellings 2018-08-03 17:54:22 UTC
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.

Comment 14 greg.hellings 2018-09-06 03:33:25 UTC
Jonathan, any chance you could look at this or would you rather let someone else tackle it?

Comment 15 Jonathan Dieter 2018-09-06 10:44:58 UTC
Yeah, I'll go ahead and do this.  Sorry about the delay in getting back to you.

Comment 16 Jonathan Dieter 2018-09-06 10:48:51 UTC
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.

Comment 17 greg.hellings 2018-09-07 15:52:07 UTC
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.

Comment 18 greg.hellings 2018-09-07 16:24:46 UTC
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.

Comment 19 Jonathan Dieter 2018-09-07 16:26:11 UTC
Great.  Do you mind posting the updated spec too?  It makes it easier for fedora-review to run.

Comment 20 greg.hellings 2018-09-07 16:50:37 UTC
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

Comment 21 Jonathan Dieter 2018-09-07 22:03:49 UTC
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

Comment 22 Jonathan Dieter 2018-09-13 08:13:12 UTC
(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?

Comment 23 Jonathan Dieter 2018-09-22 19:08:06 UTC
(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

Comment 24 greg.hellings 2018-09-24 04:18:52 UTC
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

Comment 25 Jonathan Dieter 2018-09-24 14:29:10 UTC
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.

Comment 26 greg.hellings 2018-09-24 15:47:56 UTC
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

Comment 27 Jonathan Dieter 2018-09-24 18:55:23 UTC
(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.

Comment 28 greg.hellings 2018-11-02 17:47:36 UTC
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

Comment 29 Jonathan Dieter 2018-11-03 13:46:25 UTC
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.

Comment 30 greg.hellings 2018-11-05 15:54:35 UTC
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

Comment 31 Jonathan Dieter 2018-11-05 17:43:23 UTC
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

Comment 32 Jonathan Dieter 2018-11-05 17:44:25 UTC
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'

Comment 33 greg.hellings 2018-12-13 05:39:28 UTC
> 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.

Comment 34 Jonathan Dieter 2018-12-15 22:11:35 UTC
(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.

Comment 35 Jonathan Dieter 2019-04-06 13:05:22 UTC
Greg, last I checked, we were almost finished with this.  Are you still interested in moving forward with this?

Comment 36 greg.hellings 2019-04-08 14:56:17 UTC
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.


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