Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 1646795 - Review Request: linux-system-roles - Set of interfaces for unified system management
Summary: Review Request: linux-system-roles - Set of interfaces for unified system man...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Robert-André Mauchin 🐧
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2018-11-06 02:55 UTC by Michael DePaulo
Modified: 2019-04-25 01:33 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2019-04-24 22:58:29 UTC
Type: ---
Embargoed:
zebob.m: fedora-review+

Attachments (Terms of Use)

Description Michael DePaulo 2018-11-06 02:55:32 UTC 
Spec URL: https://github.com/mikedep333/linux-system-roles-rpm/blob/master/linux-system-roles.spec
SRPM URL: https://s3.amazonaws.com/fedora-misc-packages/linux-system-roles-1.0-7.fc30.src.rpm
Description:
Collection of Ansible roles and modules that provide a stable and
consistent configuration interface for managing multiple versions
of Fedora, Red Hat Enterprise Linux & CentOS.
Fedora Account System Username: mikedep333
Comment 1 Michael DePaulo 2018-11-06 02:57:07 UTC 
I should mention that this package is already in RHEL, and thus CentOS:
https://git.centos.org/summary/rpms!rhel-system-roles.git
Comment 2 Robert-André Mauchin 🐧 2018-11-06 17:45:20 UTC 
This:

%license %{_pkgdocdir}/*/COPYING
%license %{_pkgdocdir}/*/LICENSE
%license %{_datadir}/ansible/roles/%{roleprefix}kdump/COPYING
%license %{_datadir}/ansible/roles/%{roleprefix}postfix/COPYING
%license %{_datadir}/ansible/roles/%{roleprefix}selinux/COPYING
%license %{_datadir}/ansible/roles/%{roleprefix}timesync/COPYING
%license %{_datadir}/ansible/roles/%{roleprefix}network/LICENSE

ain't gonna work. You'll end up overwriting COPYING and LICENSE files. You need to rename them first then install them.

 - Don't %doc files already in %{_pkgdocdir}

%doc %{_pkgdocdir}/*/example-*-playbook.yml
%doc %{_pkgdocdir}/network/example-inventory
%doc %{_pkgdocdir}/timesync/example-multiple-ntp-servers.yml
%doc %{_pkgdocdir}/timesync/example-single-pool.yml
%doc %{_pkgdocdir}/*/README.md
%doc %{_pkgdocdir}/*/README.html

 - This won't work either, it will overwrite the READMEs:

%doc %{_datadir}/ansible/roles/%{roleprefix}kdump/README.md
%doc %{_datadir}/ansible/roles/%{roleprefix}postfix/README.md
%doc %{_datadir}/ansible/roles/%{roleprefix}selinux/README.md
%doc %{_datadir}/ansible/roles/%{roleprefix}timesync/README.md
%doc %{_datadir}/ansible/roles/%{roleprefix}network/README.md
%doc %{_datadir}/ansible/roles/%{roleprefix}kdump/README.html
%doc %{_datadir}/ansible/roles/%{roleprefix}postfix/README.html
%doc %{_datadir}/ansible/roles/%{roleprefix}selinux/README.html
%doc %{_datadir}/ansible/roles/%{roleprefix}timesync/README.html
%doc %{_datadir}/ansible/roles/%{roleprefix}network/README.html

   Also I don't see how this is necessary as you already copied the READMEs in pkgdocdir. You shouldn't copy the licenses in pkgdocdir too.
Comment 3 Michael DePaulo 2018-11-11 19:46:22 UTC 
WRT the entire command by Robert-André Mauchin:

Overwriting them? I do not think we are. Are you saying that RPM installs them twice? `rpm -qlp` & `rpm -ql` list them once.

I am open to the idea of symlinking them, but I am under the impression that we need to keep the entire per-role file/folder structure intact under {_datadir}/ansible/roles/ to conform to Ansible standards, including the licenses and docs. I would think the link would be under %{_pkgdocdir} and would point to the path under %{_datadir} .

Granted, we are getting warnings like:
warning: File listed twice: /usr/share/ansible/roles/linux-system-roles.kdump/COPYING

I asked on #fedora-devel and was told this is the best approach given the circumstances. These occur because we include the entire directories like:
%{_datadir}/ansible/roles/%{roleprefix}kdump
I am OK with doing the work to specify the files individually, but I thought it would be better to include the entire dir though. Since the dir belongs to the package really.

Thank you for reviewing,
-Mike
Comment 4 Robert-André Mauchin 🐧 2018-11-11 20:05:42 UTC 
%doc copies file to /usr/share/doc/%{name}, %license to /usr/share/licenses/%{name}
If you %doc or %license different files with the same name, they'll be copied to /usr/share/doc/%{name} or /usr/share/licenses/%{name} but each new files will overwrite the previous one since they have the same name.
Comment 5 Michael DePaulo 2018-11-11 21:43:36 UTC 
I thought that %doc & %license only copy files when you specify relative paths.

But these are absolute paths, it doesn't copy them, correct?
Comment 6 Robert-André Mauchin 🐧 2018-11-12 14:48:50 UTC 
I couldn't find anymore info on this.

Anyhow, I think specifying %doc here is not needed as files are already in pkgdocdir:

%doc %{_pkgdocdir}/*/example-*-playbook.yml
%doc %{_pkgdocdir}/network/example-inventory
%doc %{_pkgdocdir}/timesync/example-multiple-ntp-servers.yml
%doc %{_pkgdocdir}/timesync/example-single-pool.yml
%doc %{_pkgdocdir}/*/README.md
%doc %{_pkgdocdir}/*/README.html


Also these are already in _pkgdocdir, so I don't thenk it is necessary to specix them a second time:

%doc %{_datadir}/ansible/roles/%{roleprefix}kdump/README.md
%doc %{_datadir}/ansible/roles/%{roleprefix}postfix/README.md
%doc %{_datadir}/ansible/roles/%{roleprefix}selinux/README.md
%doc %{_datadir}/ansible/roles/%{roleprefix}timesync/README.md
%doc %{_datadir}/ansible/roles/%{roleprefix}network/README.md
%doc %{_datadir}/ansible/roles/%{roleprefix}kdump/README.html
%doc %{_datadir}/ansible/roles/%{roleprefix}postfix/README.html
%doc %{_datadir}/ansible/roles/%{roleprefix}selinux/README.html
%doc %{_datadir}/ansible/roles/%{roleprefix}timesync/README.html
%doc %{_datadir}/ansible/roles/%{roleprefix}network/README.html


Instead of COPYING LICERSE and COPYING to pkgdocdir, copy them instead to licensedir:

cp -p $RPM_BUILD_ROOT%{_datadir}/ansible/roles/%{roleprefix}kdump/README.md \
    $RPM_BUILD_ROOT%{_datadir}/ansible/roles/%{roleprefix}kdump/README.html \
    $RPM_BUILD_ROOT%{_datadir}/ansible/roles/%{roleprefix}kdump/COPYING \
    $RPM_BUILD_ROOT%{_pkgdocdir}/kdump

cp -p $RPM_BUILD_ROOT%{_datadir}/ansible/roles/%{roleprefix}postfix/README.md \
    $RPM_BUILD_ROOT%{_datadir}/ansible/roles/%{roleprefix}postfix/README.html \
    $RPM_BUILD_ROOT%{_datadir}/ansible/roles/%{roleprefix}postfix/COPYING \
    $RPM_BUILD_ROOT%{_pkgdocdir}/postfix

cp -p $RPM_BUILD_ROOT%{_datadir}/ansible/roles/%{roleprefix}selinux/README.md \
    $RPM_BUILD_ROOT%{_datadir}/ansible/roles/%{roleprefix}selinux/README.html \
    $RPM_BUILD_ROOT%{_datadir}/ansible/roles/%{roleprefix}selinux/COPYING \
    $RPM_BUILD_ROOT%{_pkgdocdir}/selinux
mv $RPM_BUILD_ROOT%{_datadir}/ansible/roles/%{roleprefix}selinux/selinux-playbook.yml \
    $RPM_BUILD_ROOT%{_pkgdocdir}/selinux/example-selinux-playbook.yml

cp -p $RPM_BUILD_ROOT%{_datadir}/ansible/roles/%{roleprefix}timesync/README.md \
    $RPM_BUILD_ROOT%{_datadir}/ansible/roles/%{roleprefix}timesync/README.html \
    $RPM_BUILD_ROOT%{_datadir}/ansible/roles/%{roleprefix}timesync/COPYING \
    $RPM_BUILD_ROOT%{_pkgdocdir}/timesync

cp -p $RPM_BUILD_ROOT%{_datadir}/ansible/roles/%{roleprefix}network/README.md \
    $RPM_BUILD_ROOT%{_datadir}/ansible/roles/%{roleprefix}network/README.html \
    $RPM_BUILD_ROOT%{_datadir}/ansible/roles/%{roleprefix}network/LICENSE \
    $RPM_BUILD_ROOT%{_pkgdocdir}/network

wOwn these directorx 

[!]: Package requires other packages for directories it uses.
     Note: No known owner of /usr/share/doc/linux-system-roles/kdump,
     /usr/share/doc/linux-system-roles/network, /usr/share/doc/linux-
     system-roles/postfix, /usr/share/doc/linux-system-roles/selinux,
     /usr/share/doc/linux-system-roles, /usr/share/doc/linux-system-
     roles/timesync

Trx to fix rpmlint error by removing shebangs:

linux-system-roles.noarch: W: hidden-file-or-dir /usr/share/ansible/roles/linux-system-roles.network/.travis.yml
linux-system-roles.noarch: E: non-executable-script /usr/share/ansible/roles/linux-system-roles.network/library/network_connections.py 644 /usr/bin/python 
linux-system-roles.noarch: E: non-executable-script /usr/share/ansible/roles/linux-system-roles.network/module_utils/network_lsr/__init__.py 644 /usr/bin/python3 -tt
linux-system-roles.noarch: E: non-executable-script /usr/share/ansible/roles/linux-system-roles.network/module_utils/network_lsr/argument_validator.py 644 /usr/bin/python3 -tt
linux-system-roles.noarch: E: non-executable-script /usr/share/ansible/roles/linux-system-roles.network/module_utils/network_lsr/utils.py 644 /usr/bin/python3 -tt
linux-system-roles.noarch: E: wrong-script-interpreter /usr/share/ansible/roles/linux-system-roles.network/tests/test_network_connections.py /usr/bin/env python
linux-system-roles.noarch: E: non-executable-script /usr/share/ansible/roles/linux-system-roles.network/tests/test_network_connections.py 644 /usr/bin/env python
linux-system-roles.noarch: E: non-executable-script /usr/share/ansible/roles/linux-system-roles.selinux/library/selogin.py 644 /usr/bin/python 
linux-system-roles.noarch: E: non-executable-script /usr/share/ansible/roles/linux-system-roles.timesync/library/timesync_provider.sh 644 /bin/bash
Comment 7 Michael DePaulo 2018-11-14 03:26:55 UTC 
> Anyhow, I think specifying %doc here is not needed as files are already in pkgdocdir:
>
> %doc %{_pkgdocdir}/*/example-*-playbook.yml
> %doc %{_pkgdocdir}/network/example-inventory
> %doc %{_pkgdocdir}/timesync/example-multiple-ntp-servers.yml
> %doc %{_pkgdocdir}/timesync/example-single-pool.yml
> %doc %{_pkgdocdir}/*/README.md
> %doc %{_pkgdocdir}/*/README.html

You're right. `rpm -qd` indicates it is not necessary.

I updated the spec file in git. (See the same URL)

> Also these are already in _pkgdocdir, so I don't thenk it is necessary to specix them a second time:
>
> %doc %{_datadir}/ansible/roles/%{roleprefix}kdump/README.md
> %doc %{_datadir}/ansible/roles/%{roleprefix}postfix/README.md
> %doc %{_datadir}/ansible/roles/%{roleprefix}selinux/README.md
> %doc %{_datadir}/ansible/roles/%{roleprefix}timesync/README.md
> %doc %{_datadir}/ansible/roles/%{roleprefix}network/README.md
> %doc %{_datadir}/ansible/roles/%{roleprefix}kdump/README.html
> %doc %{_datadir}/ansible/roles/%{roleprefix}postfix/README.html
> %doc %{_datadir}/ansible/roles/%{roleprefix}selinux/README.html
> %doc %{_datadir}/ansible/roles/%{roleprefix}timesync/README.html
> %doc %{_datadir}/ansible/roles/%{roleprefix}network/README.html

We need to specify them a 2nd time for them to be considered documentation files under _datadir though.

I still offer to make the _pkgdocdir files symlinks to the the files under _datadir. Same thing for the licenses.

> Instead of COPYING LICERSE and COPYING to pkgdocdir, copy them instead to licensedir:
> ...

Agreed and implemented also.
`rpm -qL` indicates that I still need to specify %license

> Trx to fix rpmlint error by removing shebangs:
> [...]
The design of Ansible is that the "Ansible modules" (scripts in "library", etc) never get executed on the management node (where linux-system-roles gets installed.) Instead, Ansible at runtime copies them (via SSH) to the managed nodes, where it changes the permissions and executes them according to the shebang.

So the files need to have functional shebangs, but should be non-executable on the disk where the RPM is installed.
Comment 8 Robert-André Mauchin 🐧 2018-11-14 11:20:14 UTC 
Ok, looks good, packagae approved.
Comment 9 Michael DePaulo 2018-11-14 16:21:31 UTC 
(In reply to Robert-André Mauchin from comment #8)
> Ok, looks good, packagae approved.

Thank you!
Comment 10 Gwyn Ciesla 2018-11-15 14:19:54 UTC 
(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/linux-system-roles
Comment 11 Fedora Update System 2018-11-15 18:45:19 UTC 
linux-system-roles-1.0-7.fc29 has been submitted as an update to Fedora 29. https://bodhi.fedoraproject.org/updates/FEDORA-2018-33063f5f84
Comment 12 Fedora Update System 2018-11-15 18:46:13 UTC 
linux-system-roles-1.0-7.fc28 has been submitted as an update to Fedora 28. https://bodhi.fedoraproject.org/updates/FEDORA-2018-7da1358300
Comment 13 Michael DePaulo 2018-11-15 18:51:31 UTC 
I built it against rawhide too of course. It should be in that automatically.
Comment 14 Fedora Update System 2018-11-16 05:52:06 UTC 
linux-system-roles-1.0-7.fc29 has been pushed to the Fedora 29 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2018-33063f5f84
Comment 15 Fedora Update System 2018-11-16 05:59:45 UTC 
linux-system-roles-1.0-7.fc28 has been pushed to the Fedora 28 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2018-7da1358300
Comment 16 Pavel Cahyna 2018-11-21 17:27:32 UTC 
(In reply to Michael DePaulo from comment #5)
> I thought that %doc & %license only copy files when you specify relative
> paths.
> 
> But these are absolute paths, it doesn't copy them, correct?

yes. See https://fedoraproject.org/wiki/Packaging:Guidelines#Documentation.

"Marking a relative path with %doc in the %files section will cause RPM to copy the referenced file or directory from %_builddir to the proper location for documentation. Files can also be placed in %_pkgdocdir, and the build scripts of the software being packaged may do this automatically when called in %install. However, mixing these methods is problematic and may result in duplicated or conflicting files, so use of %doc with relative paths and installation of files directly into %_pkgdocdir in the same source package is forbidden. "
Comment 17 Fedora Update System 2019-04-11 20:36:11 UTC 
linux-system-roles-1.0-9.fc28 has been submitted as an update to Fedora 28. https://bodhi.fedoraproject.org/updates/FEDORA-2019-197c1515f6
Comment 18 Fedora Update System 2019-04-11 20:36:48 UTC 
linux-system-roles-1.0-9.fc29 has been submitted as an update to Fedora 29. https://bodhi.fedoraproject.org/updates/FEDORA-2019-d5f9c35f8c
Comment 19 Fedora Update System 2019-04-13 03:22:12 UTC 
linux-system-roles-1.0-9.fc28 has been pushed to the Fedora 28 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2019-197c1515f6
Comment 20 Fedora Update System 2019-04-13 12:10:32 UTC 
linux-system-roles-1.0-9.fc29 has been pushed to the Fedora 29 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2019-d5f9c35f8c
Comment 21 Fedora Update System 2019-04-24 22:58:29 UTC 
linux-system-roles-1.0-9.fc28 has been pushed to the Fedora 28 stable repository. If problems still persist, please make note of it in this bug report.
Comment 22 Fedora Update System 2019-04-25 01:33:14 UTC 
linux-system-roles-1.0-9.fc29 has been pushed to the Fedora 29 stable repository. If problems still persist, please make note of it in this bug report.
Note You need to log in before you can comment on or make changes to this bug.