Spec URL: https://download.copr.fedorainfracloud.org/results/clime/rpkg-macros/fedora-31-x86_64/01285948-rpkg-macros/rpkg-macros.spec SRPM URL: https://download.copr.fedorainfracloud.org/results/clime/rpkg-macros/fedora-31-x86_64/01285948-rpkg-macros/rpkg-macros-0.3-1.fc31.src.rpm Description: Set of preproc macros to be used by for preprocessing rpm package sources. They are designed to dynamically generate certain parts of rpm spec files based e.g. on git metadata. You can use those macros also without rpkg by: $ cat <file_with_the_macros> | preproc -s /usr/lib/rpkg.macros.d/all.bash You can also source /usr/lib/rpkg.macros.d/all.bash into your bash environment and then you can experiment with them directly on your command-line. See content in /usr/lib/rpkg.macros.d to discover available macros. Fedora Account System Username: clime
Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated [ ] = Manual review needed Issues: ======= - Permissions on files are set properly. Note: See rpmlint output See: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_file_permissions ===== MUST items ===== Generic: [x]: Package successfully compiles and builds into binary rpms on at least one supported primary architecture. Note: Using prebuilt packages [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 field in the package spec file matches the actual license. Note: Checking patched sources after %prep for licenses. Licenses found: "Unknown or generated". 108 files have unknown license. Detailed output of licensecheck in /mnt/56298eef-80c6-43f5-8aee-172b1d73d266/Projects/Fedora/rpkg-macros/copr-build-1285948/review-rpkg-macros/licensecheck.txt [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 [-]: Package uses nothing in %doc for runtime. [!]: 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]: Requires correct, justified where necessary. [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. [!]: Package complies to the Packaging Guidelines [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]: 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]: Package must not depend on deprecated() packages. [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]: Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 0 bytes in 0 files. [x]: Packages must not store files under /srv, /opt or /usr/local ===== SHOULD items ===== Generic: [x]: Reviewer should test that the package builds in mock. [ ]: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. [ ]: Final provides and requires are sane (see attachments). [ ]: Package functions as described. [ ]: Latest version is packaged. [ ]: Package does not include license text files separate from upstream. [ ]: SourceX tarball generation or download is documented. Note: Package contains tarball without URL, check comments [ ]: Sources are verified with gpgverify first in %prep if upstream publishes signatures. Note: gpgverify is not used. [ ]: Description and summary sections in the package spec file contains translations for supported Non-English languages, if available. [ ]: Package should compile and build into binary rpms on all supported architectures. [ ]: %check is present and all tests pass. [ ]: Packages should try to preserve timestamps of original installed files. [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]: Packager, Vendor, PreReq, Copyright tags should not be in spec file [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: rpkg-macros-0.3-1.fc33.noarch.rpm rpkg-macros-0.3-1.fc33.src.rpm rpkg-macros.noarch: W: spelling-error Summary(en_US) preproc -> procedure rpkg-macros.noarch: W: spelling-error %description -l en_US preproc -> procedure rpkg-macros.noarch: W: spelling-error %description -l en_US utilility -> utility, ductility rpkg-macros.noarch: W: only-non-binary-in-usr-lib rpkg-macros.noarch: W: no-documentation rpkg-macros.noarch: E: non-standard-executable-perm /usr/bin/pack_sources 775 rpkg-macros.noarch: E: non-executable-script /usr/lib/rpkg.macros.d/all.bash 644 /bin/bash rpkg-macros.noarch: E: non-executable-script /usr/lib/rpkg.macros.d/git.bash 644 /bin/bash rpkg-macros.noarch: E: non-executable-script /usr/lib/rpkg.macros.d/library/log.bash 644 /bin/bash rpkg-macros.noarch: E: non-executable-script /usr/lib/rpkg.macros.d/library/output.bash 644 /bin/bash rpkg-macros.noarch: E: non-executable-script /usr/lib/rpkg.macros.d/library/query.bash 644 /bin/bash rpkg-macros.noarch: E: non-executable-script /usr/lib/rpkg.macros.d/library/utils.bash 644 /bin/bash rpkg-macros.noarch: W: no-manual-page-for-binary pack_sources rpkg-macros.src: W: spelling-error %description -l en_US utilility -> utility, ductility rpkg-macros.src: W: spelling-error %description -l en_US usr -> use, us, user rpkg-macros.src:3: E: hardcoded-library-path in /usr/lib rpkg-macros.src:42: E: hardcoded-library-path in /usr/lib/rpkg.macros.d/all.bash rpkg-macros.src:44: E: hardcoded-library-path in /usr/lib/rpkg.macros.d/all.bash rpkg-macros.src:47: E: hardcoded-library-path in /usr/lib/rpkg.macros.d rpkg-macros.src: W: no-%build-section rpkg-macros.src: W: invalid-url Source0: rpkg-macros-0.3.tar.gz 2 packages and 0 specfiles checked; 11 errors, 10 warnings. Rpmlint (installed packages) ---------------------------- perl: warning: Setting locale failed. perl: warning: Please check that your locale settings: LANGUAGE = (unset), LC_ALL = (unset), LC_CTYPE = "C.UTF-8", LANG = "en_US.UTF-8" are supported and installed on your system. perl: warning: Falling back to the standard locale ("C"). perl: warning: Setting locale failed. perl: warning: Please check that your locale settings: LANGUAGE = (unset), LC_ALL = (unset), LC_CTYPE = "C.UTF-8", LANG = "en_US.UTF-8" are supported and installed on your system. perl: warning: Falling back to the standard locale ("C"). rpkg-macros.noarch: W: spelling-error Summary(en_US) preproc -> procedure rpkg-macros.noarch: W: spelling-error %description -l en_US preproc -> procedure rpkg-macros.noarch: W: spelling-error %description -l en_US utilility -> utility, ductility rpkg-macros.noarch: W: invalid-url URL: https://pagure.io/rpkg-util.git <urlopen error [Errno -2] Name or service not known> rpkg-macros.noarch: W: only-non-binary-in-usr-lib rpkg-macros.noarch: W: no-documentation rpkg-macros.noarch: E: non-standard-executable-perm /usr/bin/pack_sources 775 rpkg-macros.noarch: E: non-executable-script /usr/lib/rpkg.macros.d/all.bash 644 /bin/bash rpkg-macros.noarch: E: non-executable-script /usr/lib/rpkg.macros.d/git.bash 644 /bin/bash rpkg-macros.noarch: E: non-executable-script /usr/lib/rpkg.macros.d/library/log.bash 644 /bin/bash rpkg-macros.noarch: E: non-executable-script /usr/lib/rpkg.macros.d/library/output.bash 644 /bin/bash rpkg-macros.noarch: E: non-executable-script /usr/lib/rpkg.macros.d/library/query.bash 644 /bin/bash rpkg-macros.noarch: E: non-executable-script /usr/lib/rpkg.macros.d/library/utils.bash 644 /bin/bash rpkg-macros.noarch: W: no-manual-page-for-binary pack_sources 1 packages and 0 specfiles checked; 7 errors, 7 warnings. Requires -------- rpkg-macros (rpmlib, GLIBC filtered): /usr/bin/bash bash coreutils findutils git Provides -------- rpkg-macros: rpkg-macros Generated by fedora-review 0.7.5 (5fa5b7e) last change: 2020-02-16 Command line :/usr/bin/fedora-review --copr-build 1285948 Buildroot used: fedora-rawhide-x86_64 Active plugins: Shell-api, Generic Disabled plugins: R, Python, Java, Ocaml, C/C++, SugarActivity, Perl, PHP, Haskell, fonts Disabled flags: EPEL6, EPEL7, DISTTAG, BATCH, EXARCH
(In reply to chedi toueiti from comment #1) > Package Review > ============== > Could you add comments about what needs fixing, it is not clear from your post. The perms issue is a false positive from rpmlint.
for the permission part "E: non-executable-script" I could be wrong but I think this is due to the presence of the shebang in the bash files. As you will be sourcing those in your environment the shebang are unecessary and could safely be removed. In any other use case the correct permission have to be set (https://fedoraproject.org/wiki/Packaging_tricks#Exec_permission). the second set of errors are related to E: hardcoded-library-path in /usr/lib it should be %{_libdir}
@chedi > for the permission part "E: non-executable-script" I could be wrong but I think this is due to the presence of the shebang in the bash files. As you will be sourcing those in your environment the shebang are unecessary and could safely be removed. You are right that they are unnecessary. However, e.g. https://www.shellcheck.net/ will give you a warning if they are not included so I just included them to satisfy the linting tool and I don't think they are harmful there, although you are right that they are not technically needed, so idk, it's your call, I can remove them if you tell me but I would personally like them to stay if possible. The permission error I can fix easily, there should really rather be 755 to make it the same as the other /usr/bin content The /usr/lib thing I think I do correctly because %{_libdir} expands to on /usr/lib64 on my machine but I would like to specifically place the library under /usr/lib because it is a noarch package. But it's true that according to this thread https://lists.fedoraproject.org/archives/list/packaging@lists.fedoraproject.org/thread/AWM3ND26Q26M5OVDONQCWZBAN6AKV3MD/ I could be using %{_prefix}/lib instead of /usr/lib. I will change it too.
Please, check this out (rawhide builds): https://download.copr.fedorainfracloud.org/results/clime/rpkg-macros/fedora-rawhide-x86_64/01298436-rpkg-macros/rpkg-macros.spec https://download.copr.fedorainfracloud.org/results/clime/rpkg-macros/fedora-rawhide-x86_64/01298436-rpkg-macros/rpkg-macros-0.3.git.2.340f804-1.fc33.src.rpm Builds for other OSes are here: https://copr.fedorainfracloud.org/coprs/clime/rpkg-macros/build/1298436/ Thanks! clime
@clime Your last build fixed the hard coded path but not the permissions, just modify the %install section as follow and we will be good %install install -d %{buildroot}%{libdir} install -d %{buildroot}%{libdir}/rpkg.macros.d cp -ar macros.d/* %{buildroot}%{libdir}/rpkg.macros.d install -d %{buildroot}%{_bindir} install -p -m 755 bin/pack_sources %{buildroot}%{_bindir}/pack_sources # fix for rpmlint permission errors chmod a+x %{buildroot}%{libdir}/rpkg.macros.d/*.bash chmod a+x %{buildroot}%{libdir}/rpkg.macros.d/library/*.bash
Ah, I only thought about fixing bin/pack_sources permissions which were 775 and i changed it to 755. # fix for rpmlint permission errors chmod a+x %{buildroot}%{libdir}/rpkg.macros.d/*.bash chmod a+x %{buildroot}%{libdir}/rpkg.macros.d/library/*.bash I am not so sure about this cause, as you said, these files are only intended to be sourced - not executed (as they only contain functions and not a direct bash code). They contain #!/bin/bash but that's just because the linter i use doesn't like when it is missing and I think it is also not so bad bad practice to put the shebang everywhere (even into files which will be just sourced). Can you, please, take a look again if the executable permissions are really needed on the files you mention? I think the rpmlint tool might not be 100% to the point there. I am trying a little bit of negotiation here :). Thank you clime
@clime, I feel your pain, I'm not trying to be over zealous but according to https://fedoraproject.org/wiki/Packaging:ReviewGuidelines#Things_To_Check_On_Review "MUST: Permissions on files must be set properly. Executables should be set with executable permissions" =================== MUST Items Items marked as MUST are things that the package (or reviewer) MUST do. If a package fails a MUST item, that is considered a blocker. No package with blockers can be approved on a review. Those items must be fixed before approval can be given. =================== I think even if I let it slide, the automated systems or official reviewers will reject it. I'll be posting a question in the packaging mailing list to get a second opinion.
The response I got from Neal Gompa ngompa13 on the packaging list On Mon, Mar 9, 2020 at 8:54 AM chedi toueiti <chedi.toueiti> wrote: > > Hi, > > I'm reviewing a new package > https://bugzilla.redhat.com/show_bug.cgi?id=1810902 > > this package have bash scripts that are under /usr/lib and starting with a shebang, rpmlint is detecting the error > > E: non-executable-script > > rpkg-macros.noarch: E: non-executable-script /usr/lib/rpkg.macros.d/all.bash 644 /bin/bash > rpkg-macros.noarch: E: non-executable-script /usr/lib/rpkg.macros.d/git.bash 644 /bin/bash > rpkg-macros.noarch: E: non-executable-script /usr/lib/rpkg.macros.d/library/log.bash 644 /bin/bash > rpkg-macros.noarch: E: non-executable-script /usr/lib/rpkg.macros.d/library/output.bash 644 /bin/bash > rpkg-macros.noarch: E: non-executable-script /usr/lib/rpkg.macros.d/library/query.bash 644 /bin/bash > rpkg-macros.noarch: E: non-executable-script /usr/lib/rpkg.macros.d/library/utils.bash 644 /bin/bash > > > The author desire to keep the shebang and not set the execution bit as the files are not meant to be executable. > > Should I consider the package valid or not in this case. > In the current case, this is not acceptable. These are the two choices I would offer: * Those should be in /usr/share if they are architecture-independent scripts that aren't executed (shebangs should be purged) * Those should be in /usr/libexec in any other case
@chedi Ok, if i remove the shebangs, do the "E: non-executable-script" errors disappear? I would do it then. What do you think? I wouldn't like to move it to entirely different path because I have some tooling that references the /usr/lib path and I think the /usr/lib path is ok (although if there is something in the Fedora packaging guidelines forbidding this, I would need to change it as well).
@clime To my knowledge /usr/lib is meant for 32bit architecture but you can store noarch scripts in it. So there is nothing forbidding it as long as they are libraries or executable scripts for example the rpm package have scripts under it /usr/lib/rpm/find-debuginfo.sh /usr/lib/rpm/find-lang.sh /usr/lib/rpm/libtooldeps.sh /usr/lib/rpm/ocaml-find-provides.sh /usr/lib/rpm/ocaml-find-requires.sh /usr/lib/rpm/pkgconfigdeps.sh but all those are standalone executables. (which is not your case) On the other hand /usr/libexec is architecture agnostic, and should only be used as the directory for executable programs that are designed primarily to be run by other programs rather than by users (https://docs.fedoraproject.org/en-US/packaging-guidelines/#_libexecdir), which is very close to the your use case but in the same time is sourcing really executing ??(you'll have to add the exec permission) and finnally there is /usr/share but you'll have to remove the shebang. I think you should move your files to either /usr/share or /usr/libexec, it's up to you to decide between removing the shebang or adding the exec bit.
@chedi if you look at e.g. /usr/lib/rpm/macros.d, there are plenty non-executable files or all the files under /usr/lib/python3.7/site-packages. The files that rpkg-macros provide are libraries so /usr/libexec is not a good place imho. /usr/share I recognize as a place for man pages and various assets like fonts, icons etc. Imho, /usr/lib is a place for noarch libraries that are needed by executables under /usr/bin, which is a case here. https://refspecs.linuxfoundation.org/FHS_3.0/fhs/ch04s06.html
Here are the builds (rawhide) with the shebang removed: https://download.copr.fedorainfracloud.org/results/clime/rpkg-macros/fedora-rawhide-x86_64/01298916-rpkg-macros/rpkg-macros.spec https://download.copr.fedorainfracloud.org/results/clime/rpkg-macros/fedora-rawhide-x86_64/01298916-rpkg-macros/rpkg-macros-0.3.git.3.9ac3009-1.fc33.src.rpm The result of builds can be found here: https://copr.fedorainfracloud.org/coprs/clime/rpkg-macros/build/1298916/
@clime this is good to go, I'll be waiting for your review here https://bugzilla.redhat.com/show_bug.cgi?id=1808023 PS: to finish your review of https://bugzilla.redhat.com/show_bug.cgi?id=1807945 you have to: - take the bug - change the flag to + - change the bug status to POST
(fedscm-admin): The Pagure repository was created at https://src.fedoraproject.org/rpms/rpkg-macros