Spec URL: https://dctrud.fedorapeople.org/singularity-ce.spec SRPM URL: https://dctrud.fedorapeople.org/singularity-ce-3.10.4-1.fc37.src.rpm Description: SingularityCE is the Community Edition of Singularity, an open source container platform designed to be simple, fast, and secure. Fedora Account System Username: dctrud
Ok here goes round 1. > # Copyright (c) 2017-2022, Sylabs, Inc. All rights reserved. > # Copyright (c) 2017, SingularityWare, LLC. All rights reserved. > # > # Copyright (c) 2015-2017, Gregory M. Kurtzer. All rights reserved. > # > # Copyright (c) 2016, The Regents of the University of California, through > # Lawrence Berkeley National Laboratory (subject to receipt of any required > # approvals from the U.S. Dept. of Energy). All rights reserved. > # > # This software is licensed under a customized 3-clause BSD license. Please > # consult LICENSE file distributed with the sources of this project regarding > # your rights to use or distribute this software. > # > # NOTICE. This Software was developed under funding from the U.S. Department of > # Energy and the U.S. Government consequently retains certain rights. As such, > # the U.S. Government has been granted for itself and others acting on its > # behalf a paid-up, nonexclusive, irrevocable, worldwide license in the Software > # to reproduce, distribute copies to the public, prepare derivative works, and > # perform publicly and display publicly, and to permit other to do so. I don't think any of this is necessary in the spec file. > License: BSD-3-Clause and LBNL BSD and ASL 2.0 Licenses should all be listed in SPDX format [1] You probably want this: License: BSD-3-Clause and BSD-3-Clause-LBNL and Apache-2.0 > BuildRequires: git This doesn't appear to be needed. > # The version used for the src tar filename can be different to the rpm version. > # This is due to different handling of pre-release version numbers in e.g. semver, > # rpm, dpkg. > %global src_version 3.10.4 What are some example cases where this could be needed? RPM can match upstream version, even with weird pre-release things, so it'd be best to only have the one "Version" var. [2] > %autosetup -n %{name}-%{src_version} This can change to just "%autosetup" if we get rid of the src_version variable. > * Wed Nov 23 2022 David Trudgian <dtrudg> 3.10.4 You need a "-" between the email and the version, and also the release on the end, ie -1. ie: * Wed Nov 23 2022 David Trudgian <dtrudg> - 3.10.4-1 --- Does singularity rotate it's own log files? If not you need to ship a logrotate config. [3] --- RPMLint: > singularity-ce.x86_64: E: zero-length /etc/singularity/capability.json > singularity-ce.x86_64: E: zero-length /etc/singularity/global-pgp-public These files shouldn't be included unless empty files are required for some reason. [4] > E: setuid-binary /usr/libexec/singularity/bin/starter-suid root 4755 > E: non-standard-executable-perm /usr/libexec/singularity/bin/starter-suid 4755 This non-standard permission makes sense to me, but you need to tell rpmlint that it's OK. [5] tl;dr create a file, singularity-ce.rpmlintrc alongside the spec file, and include the following content: addFilter(r'setuid-binary /usr/libexec/singularity/bin/starter-suid') addFilter(r'non-standard-executable-perm /usr/libexec/singularity/bin/starter-suid') > singularity-ce.x86_64: E: explicit-lib-dependency glib2 > singularity-ce.x86_64: E: explicit-lib-dependency libseccomp Remove the following 2 lines: Requires: glib2 Requires: libseccomp These are handled automatically by RPM metadata. === 1. https://docs.fedoraproject.org/en-US/packaging-guidelines/LicensingGuidelines/#_valid_license_short_names 2. https://docs.fedoraproject.org/en-US/packaging-guidelines/Versioning/ 3. https://docs.fedoraproject.org/en-US/packaging-guidelines/#_log_files 4. https://fedoraproject.org/wiki/Common_Rpmlint_issues#zero-length 5. https://github.com/rpm-software-management/rpmlint/blob/main/README.md#configuration
Do all the Go libraries need to be bundled?
> > License: BSD-3-Clause and LBNL BSD and ASL 2.0 > > Licenses should all be listed in SPDX format [1] > > You probably want this: > > License: BSD-3-Clause and BSD-3-Clause-LBNL and Apache-2.0 Note that this is not valid SPDX either, as the conjuncion operator in SPDX is "AND", not "and".
(In reply to Jonathan Wright from comment #1) > Ok here goes round 1. Many thanks for such a speedy review! Updated files with the changes below are at: https://fedorapeople.org/~dctrud/ > > # Copyright (c) 2017-2022, Sylabs, Inc. All rights reserved. > > ... > > I don't think any of this is necessary in the spec file. Removed. > > License: BSD-3-Clause and LBNL BSD and ASL 2.0 > > Licenses should all be listed in SPDX format [1] Changed per suggestion, and comment #3 r.e AND conjunction. > > > BuildRequires: git > > This doesn't appear to be needed. Removed. > > # The version used for the src tar filename can be different to the rpm version. > > # This is due to different handling of pre-release version numbers in e.g. semver, > > # rpm, dpkg. > > %global src_version 3.10.4 > > What are some example cases where this could be needed? RPM can match > upstream version, even with weird pre-release things, so it'd be best to > only have the one "Version" var. [2] For release candidates our naming would be e.g. 'singularity-ce-3.11.0-rc.1.tar.gz'. AFAIK this needs to be RPM version 3.11.0~rc.1 so it sorts before 3.11.0. We probably won't package any release candidates here... so I could remove it, but that would mean the spec file here differs more from the one in our source repo (which I will update after advice etc. here). If there's a strong wish to remove it, then I can. > > %autosetup -n %{name}-%{src_version} > > This can change to just "%autosetup" if we get rid of the src_version > variable. See above. > > * Wed Nov 23 2022 David Trudgian <dtrudg> 3.10.4 > > You need a "-" between the email and the version, and also the release on > the end, ie -1. > > ie: > * Wed Nov 23 2022 David Trudgian <dtrudg> - 3.10.4-1 Fixed - sorry this was a silly one. > Does singularity rotate it's own log files? If not you need to ship a > logrotate config. [3] Singularity doesn't have a daemon, and doesn't create log files, so this shouldn't be needed. > RPMLint: > > > singularity-ce.x86_64: E: zero-length /etc/singularity/capability.json > > singularity-ce.x86_64: E: zero-length /etc/singularity/global-pgp-public Unfortunately I'm not confident these can be left to runtime creation. I'll open issues on the upstream repo and verify this, but for now I've added an rpmlintrc. > > E: setuid-binary /usr/libexec/singularity/bin/starter-suid root 4755 > > E: non-standard-executable-perm /usr/libexec/singularity/bin/starter-suid 4755 > > This non-standard permission makes sense to me, but you need to tell rpmlint > that it's OK. [5] Created the rpmlintrc - https://fedorapeople.org/~dctrud/singularity-ce.rpmlintrc > > singularity-ce.x86_64: E: explicit-lib-dependency glib2 > > singularity-ce.x86_64: E: explicit-lib-dependency libseccomp > > Remove the following 2 lines: > > Requires: glib2 > Requires: libseccomp Removed. Apologies... these are necessary when building our bundled conmon source... which is not being done for this packaging. Thanks again.
(In reply to Benson Muite from comment #2) > Do all the Go libraries need to be bundled? This is a question I was expecting :-) and I think the answer is technically 'no', but practically 'yes'. Unfortunately I don't think it's practical for us to unbundle more than a very small number of the Go libraries here, which I'd argue doesn't really help much over leaving the dependencies fully bundled, and would create quite a lot more work. As an indirect justification for using fully bundled deps... singularity-ce is a container runtime, similar in scope to e.g. podman or apptainer. (Note that apptainer is an alternative fork/rename of the original singularity project... so it's currently quite similar to singularity-ce). podman uses bundled Go deps, which are declared in the spec (https://src.fedoraproject.org/rpms/podman/blob/rawhide/f/podman.spec) apptainer uses bundled Go deps, which haven't declared in the spec (https://src.fedoraproject.org/rpms/apptainer/blob/rawhide/f/apptainer.spec) More direct justification... * Generally, the go dependencies used by container runtimes tend to be an extremely complex web. We've had multiple occasions where we've had to use explicit replace directives in our go.mod in order to get a combination of versions of deps that will build / work correctly. E.g. https://github.com/sylabs/singularity/pull/443/files#diff-33ef32bf6c23acb95f5902d7097b7a1d5128ca061167ec0716715b0b9eeaa5f6 * We've also had times where a dep minor version update has caused a nasty regression, or we've had to go up to the HEAD of a dep's repo in order to fix something urgently. E.g. https://github.com/sylabs/singularity/pull/546 The trouble with catching these things, handling them, and preventing future issues, is that we rely on long-running end-to-end tests which require network access. Our upstream CI will run these tests always, so we can be reasonably confident about functionality with the bundled deps. It's not possible to run these extensive tests during an rpmbuild, and we don't really have the resources to run all of our tests on each golang library change across Fedora / EPEL versions. FWIW - we aggressively apply dependency updates to our main and release branches upstream, delaying only where unavoidable (https://github.com/sylabs/singularity/pulls?q=is%3Apr+build%28deps%29+is%3Aclosed+). This hopefully somewhat offsets the risk of unknown vulnerabilities / bugs hanging around in bundled deps. I've also tried to follow what I believe is best practice here (?), declaring the bundled deps (like podman does), rather than just using them without them being declared (like apptainer). I'm working through another packaging project for https://github.com/sylabs/scs-library-client, and there I am packaging missing deps, so that it can be fully unbundled. That is a much simpler piece of software, though, and much more straightforward to be confident about with unbundled deps.
Spec URL: https://dctrud.fedorapeople.org/singularity-ce.spec SRPM URL: https://dctrud.fedorapeople.org/singularity-ce-3.10.4-1.fc37.src.rpm ^ needed for fedora-review to work.
> > # The version used for the src tar filename can be different to the rpm version. > > # This is due to different handling of pre-release version numbers in e.g. semver, > > # rpm, dpkg. > > %global src_version 3.10.4 > > What are some example cases where this could be needed? RPM can match > upstream version, even with weird pre-release things, so it'd be best to > only have the one "Version" var. [2] > For release candidates our naming would be e.g. 'singularity-ce-3.11.0-rc.1.tar.gz'. AFAIK this needs to be RPM version 3.11.0~rc.1 so it sorts before 3.11.0. > We probably won't package any release candidates here... so I could remove it, but that would mean the spec file here differs more from the one in our source repo (which I will update after advice etc. here). If there's a strong wish to remove it, then I can. Per Fedora and EPEL update policies release candidates generally shouldn't be included in Fedora anyway. [2] Since you are the upstream here the simplest thing is "if it's suitable for Fedora then it's suitable to be an official release" thus eliminating the concern with RC versions anyway. Few more nitpicks: > W: non-conffile-in-etc /etc/bash_completion.d/singularity Bash completions are generally going in "%{_datadir}/bash-completion/completions/" these days. > W: non-standard-dir-in-var singularity Applications must generally not add directories to the top level of /var. Such directories should only be added if they have some system-wide implication, and in consultation with the FHS mailing list. [1] Perhaps /var/lib/singularity would be a better fit? === 1. https://www.pathname.com/fhs/pub/fhs-2.3.html#THEVARHIERARCHY 2. https://docs.fedoraproject.org/en-US/fesco/Updates_Policy/#stable-releases === Additionally, rpmlint is throwing the following 3 errors: singularity-ce.x86_64: E: readelf-failed /usr/bin/singularity 'utf-8' codec can't decode byte 0xc2 in position 10890: invalid continuation byte singularity-ce.x86_64: E: readelf-failed /usr/libexec/singularity/bin/starter 'utf-8' codec can't decode byte 0xc2 in position 13081: invalid continuation byte singularity-ce.x86_64: E: readelf-failed /usr/libexec/singularity/bin/starter-suid 'utf-8' codec can't decode byte 0xc2 in position 13081: invalid continuation byte I'm not really sure what to make of these. I've not seen them before and can't find a ton of docs about these errors. I'm going to continue digging and testing but I'll probably end up seeking guidance on the devel mailing list. In testing the resulting binaries all seems well. <mock-chroot> sh-5.2# readelf -h singularity ELF Header: Magic: 7f 45 4c 46 02 01 01 00 00 00 00 00 00 00 00 00 Class: ELF64 Data: 2's complement, little endian Version: 1 (current) OS/ABI: UNIX - System V ABI Version: 0 Type: DYN (Position-Independent Executable file) Machine: Advanced Micro Devices X86-64 Version: 0x1 Entry point address: 0x8e5780 Start of program headers: 64 (bytes into file) Start of section headers: 38113256 (bytes into file) Flags: 0x0 Size of this header: 64 (bytes) Size of program headers: 56 (bytes) Number of program headers: 14 Size of section headers: 64 (bytes) Number of section headers: 35 Section header string table index: 34
Regarding the readelf errors, I think we just need to bypass them in rpmlintrc for now. All direct readelf tests against the binaries are fine, and I found a few other reports in the past year where rpmlint flags these readelf issues when there actually are none. Ref: https://lists.opensuse.org/archives/list/go@lists.opensuse.org/thread/GOTLRZAGMLWLZB5YYVSET7CW32XPEKIM/ https://lists.opensuse.org/archives/list/buildservice@lists.opensuse.org/message/SWZ5EEXGBJVZA7GITIXGVFEWKPF5MVBQ/ https://github.com/vladimirvivien/ktop/issues/8
(In reply to Fabio Valentini from comment #3) > > > License: BSD-3-Clause and LBNL BSD and ASL 2.0 > > > > Licenses should all be listed in SPDX format [1] > > > > You probably want this: > > > > License: BSD-3-Clause and BSD-3-Clause-LBNL and Apache-2.0 > > Note that this is not valid SPDX either, as the conjuncion operator in SPDX > is "AND", not "and". Not true. Lowercase and/or/with are accepted in SPDX. Always have been.
(In reply to Jonathan Wright from comment #7) Spec URL: https://dctrud.fedorapeople.org/singularity-ce.spec SRPM URL: https://dctrud.fedorapeople.org/singularity-ce-3.10.4-1.fc37.src.rpm > > > # The version used for the src tar filename can be different to the rpm version. > > > # This is due to different handling of pre-release version numbers in e.g. semver, > > > # rpm, dpkg. > > > %global src_version 3.10.4 > > > > What are some example cases where this could be needed? RPM can match > > upstream version, even with weird pre-release things, so it'd be best to > > only have the one "Version" var. [2] > > > For release candidates our naming would be e.g. 'singularity-ce-3.11.0-rc.1.tar.gz'. AFAIK this needs to be RPM version 3.11.0~rc.1 so it sorts before 3.11.0. > > > We probably won't package any release candidates here... so I could remove it, but that would mean the spec file here differs more from the one in our source repo (which I will update after advice etc. here). If there's a strong wish to remove it, then I can. > > Per Fedora and EPEL update policies release candidates generally shouldn't > be included in Fedora anyway. [2] > > Since you are the upstream here the simplest thing is "if it's suitable for > Fedora then it's suitable to be an official release" thus eliminating the > concern with RC versions anyway. Removed... now using %{version} only. > Few more nitpicks: > > > W: non-conffile-in-etc /etc/bash_completion.d/singularity > > Bash completions are generally going in > "%{_datadir}/bash-completion/completions/" these days. This is going to need upstream code changes or patches, if required, as our makefile is using sysconfdir without the ability to override. Is this a blocker, or something that we could tackle getting sorted out in the next upstream release? > > W: non-standard-dir-in-var singularity > > Applications must generally not add directories to the top level of /var. > Such directories should only be added if they have some system-wide > implication, and in consultation with the FHS mailing list. [1] > > Perhaps /var/lib/singularity would be a better fit? I believe the reason for using %{_localstatedir} (/var) is that when Singularity was written, this location was chosen on the basis that it is... "The directory for installing data files which the programs modify while they run, and that pertain to one specific machine." (https://www.gnu.org/prep/standards/html_node/Directory-Variables.html) ... and the original author was purposely using %{_localstatedir} (/var) rather than %{_sharedstatedir} (/var/lib) because we place a directory in there that is used as a location (within a mount namespace) under which the container filesystem is assembled before starting the container. To avoid any issues with overlay mounts, namespaces etc. that can arise on network filesystems, we require that this directory is on a local fs, which matches the GNU definition of localstatedir above. I note, though, in the FHS document you linked, that the entry for /var/lib refers to "state information is data that programs modify while they run, and that pertains to one specific host"... so I suppose we could use %{_sharedstatedir} here... it's just a bit odd the macro is 'shared' when we need explicitly 'local' state. Modified the .spec file for this, and I'll have a think about how we can tidy this up upstream... if we can. It does seem we should be following then FHS definition, and not a GNU page... it's just a bit confusing with the macro naming. > Additionally, rpmlint is throwing the following 3 errors: > > singularity-ce.x86_64: E: readelf-failed /usr/bin/singularity 'utf-8' codec > can't decode byte 0xc2 in position 10890: invalid continuation byte > singularity-ce.x86_64: E: readelf-failed > /usr/libexec/singularity/bin/starter 'utf-8' codec can't decode byte 0xc2 in > position 13081: invalid continuation byte > singularity-ce.x86_64: E: readelf-failed > /usr/libexec/singularity/bin/starter-suid 'utf-8' codec can't decode byte > 0xc2 in position 13081: invalid continuation byte Added to rpmlint rules, per subsequent comment. Updated files at: https://fedorapeople.org/~dctrud/
(In reply to dave from comment #10) > > Few more nitpicks: > > > > > W: non-conffile-in-etc /etc/bash_completion.d/singularity > > > > Bash completions are generally going in > > "%{_datadir}/bash-completion/completions/" these days. > > This is going to need upstream code changes or patches, if required, as our > makefile is using sysconfdir without the ability to override. Is this a > blocker, or something that we could tackle getting sorted out in the next > upstream release? Not a blocker, but would be great to see it changed in the future. > > > W: non-standard-dir-in-var singularity > > > > Applications must generally not add directories to the top level of /var. > > Such directories should only be added if they have some system-wide > > implication, and in consultation with the FHS mailing list. [1] > > > > Perhaps /var/lib/singularity would be a better fit? > > I believe the reason for using %{_localstatedir} (/var) is that when > Singularity was written, this location was chosen on the basis that it is... > > "The directory for installing data files which the programs modify while > they run, and that pertain to one specific machine." > (https://www.gnu.org/prep/standards/html_node/Directory-Variables.html) > > ... and the original author was purposely using %{_localstatedir} (/var) > rather than %{_sharedstatedir} (/var/lib) because we place a directory in > there that is used as a location (within a mount namespace) under which the > container filesystem is assembled before starting the container. To avoid > any issues with overlay mounts, namespaces etc. that can arise on network > filesystems, we require that this directory is on a local fs, which matches > the GNU definition of localstatedir above. > > I note, though, in the FHS document you linked, that the entry for /var/lib > refers to "state information is data that programs modify while they run, > and that pertains to one specific host"... so I suppose we could use > %{_sharedstatedir} here... it's just a bit odd the macro is 'shared' when we > need explicitly 'local' state. > > Modified the .spec file for this, and I'll have a think about how we can > tidy this up upstream... if we can. It does seem we should be following then > FHS definition, and not a GNU page... it's just a bit confusing with the > macro naming. I think this will be OK for now but should be changed in the future.
Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated ===== MUST items ===== C/C++: [x]: Package does not contain kernel modules. [x]: Package contains no static executables. [x]: If your application is a C or C++ application you must list a BuildRequires against gcc, gcc-c++ or clang. [x]: Package does not contain any libtool archives (.la) [x]: Rpath absent or only used for internal libs. 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 field in the package spec file matches the actual license. Note: Checking patched sources after %prep for licenses. Licenses found: "Unknown or generated", "*No copyright* Lawrence Berkeley National Labs BSD variant license", "BSD 3-Clause License", "*No copyright* Apache License 2.0", "Apache License 2.0", "BSD 3-Clause License BSD 2-Clause License Apache License 2.0", "ISC License", "*No copyright* The Unlicense", "MIT License", "MIT License Apache License 2.0", "*No copyright* [generated file]", "*No copyright* MIT License", "*No copyright* Mozilla Public License 2.0", "Creative Commons Attribution 4.0 Apache License 2.0", "MIT License BSD 3-Clause License", "BSD 2-Clause License", "Mozilla Public License 2.0", "*No copyright* BSD 3-Clause License", "BSD 3-Clause License Apache License 2.0", "*No copyright* BSD 2-Clause License", "*No copyright* GNU Lesser General Public License, Version 3". 4281 files have unknown license. Detailed output of licensecheck in /home/jonathan/fedora- review/2145834-singularity-ce/licensecheck.txt [x]: License file installed when any subpackage combination is installed. [x]: If the package is under multiple licenses, the licensing breakdown must be documented in the spec. [!]: Package requires other packages for directories it uses. Note: No known owner of /etc/singularity/seccomp-profiles, /etc/singularity/cgroups, /usr/libexec/singularity/bin, /usr/libexec/singularity/cni, /etc/singularity/network [!]: Package must own all directories that it creates. Note: Directories without known owners: /etc/singularity/cgroups, /etc/singularity/seccomp-profiles, /usr/libexec/singularity/bin, /etc/singularity/network, /usr/libexec/singularity/cni [x]: Package does not own files or directories owned by other packages. Note: Dirs in package are owned also by: /etc/bash_completion.d(lttng- tools, pdc-client, singularity, python3-manilaclient, perl-App-Cme, freeipa-client, python3-novaclient, python3-glanceclient, filesystem, phoronix-test-suite, stgit, openscap-scanner, iprutils, perl-Dist- Zilla, clusterssh, quilt, weldr-client, RBTools, python3-heatclient), /etc/singularity(singularity), /usr/libexec/singularity(singularity) [x]: %build honors applicable compiler flags or justifies otherwise. [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. [!]: 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. [x]: Useful -debuginfo package or justification otherwise. [x]: Package is not known to require an ExcludeArch tag. [x]: Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 81920 bytes in 3 files. [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 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]: %config files are marked noreplace or the reason is justified. [x]: Package uses hardened build flags if required to. Note: suid files: starter-suid [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 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]: No %config files under /usr. [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]: Package contains systemd file(s) if in need. [x]: File names are valid UTF-8. [x]: Packages must not store files under /srv, /opt or /usr/local ===== SHOULD items ===== Generic: [!]: Uses parallel make %{?_smp_mflags} macro. [x]: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. [x]: Final provides and requires are sane (see attachments). [x]: Package functions as described. [x]: Latest version is packaged. [x]: Package does not include license text files separate from upstream. [-]: Sources are verified with gpgverify first in %prep if upstream publishes signatures. Note: gpgverify is not used. [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 debuginfo package(s). Note: There are rpmlint messages (see attachment). [x]: Rpmlint is run on all installed packages. Note: There are rpmlint messages (see attachment). [x]: Large data in /usr/share should live in a noarch subpackage if package is arched. [x]: Spec file according to URL is the same as in SRPM. Rpmlint ------- Cannot parse rpmlint output: Rpmlint (debuginfo) ------------------- Cannot parse rpmlint output: Rpmlint (installed packages) ---------------------------- Cannot parse rpmlint output: Source checksums ---------------- https://github.com/sylabs/singularity/releases/download/v3.10.4/singularity-ce-3.10.4.tar.gz : CHECKSUM(SHA256) this package : d77d5bc451937ea5f2ba074ffd7ac2d202212545f32a32c277f72fb106ba97d9 CHECKSUM(SHA256) upstream package : d77d5bc451937ea5f2ba074ffd7ac2d202212545f32a32c277f72fb106ba97d9 Requires -------- singularity-ce (rpmlib, GLIBC filtered): /usr/bin/sh config(singularity-ce) conmon cryptsetup libc.so.6()(64bit) libseccomp.so.2()(64bit) rtld(GNU_HASH) runc shadow-utils squashfs-tools singularity-ce-debuginfo (rpmlib, GLIBC filtered): Provides -------- singularity-ce: bundled(golang(github.com/AdamKorcz/go_fuzz_headers)) bundled(golang(github.com/Azure/go_ansiterm)) bundled(golang(github.com/BurntSushi/toml)) bundled(golang(github.com/Microsoft/go_winio)) bundled(golang(github.com/Microsoft/hcsshim)) bundled(golang(github.com/Netflix/go_expect)) bundled(golang(github.com/ProtonMail/go_crypto)) bundled(golang(github.com/VividCortex/ewma)) bundled(golang(github.com/acarl005/stripansi)) bundled(golang(github.com/adigunhammedolalekan/registry_auth)) bundled(golang(github.com/alexflint/go_filemutex)) bundled(golang(github.com/apex/log)) bundled(golang(github.com/beorn7/perks)) bundled(golang(github.com/blang/semver)) bundled(golang(github.com/blang/semver/v4)) bundled(golang(github.com/buger/jsonparser)) bundled(golang(github.com/bugsnag/bugsnag_go)) bundled(golang(github.com/bugsnag/panicwrap)) bundled(golang(github.com/cespare/xxhash/v2)) bundled(golang(github.com/cilium/ebpf)) bundled(golang(github.com/cloudflare/circl)) bundled(golang(github.com/containerd/cgroups)) bundled(golang(github.com/containerd/containerd)) bundled(golang(github.com/containernetworking/cni)) bundled(golang(github.com/containernetworking/plugins)) bundled(golang(github.com/containers/common)) bundled(golang(github.com/containers/image/v5)) bundled(golang(github.com/containers/libtrust)) bundled(golang(github.com/containers/ocicrypt)) bundled(golang(github.com/containers/storage)) bundled(golang(github.com/coreos/go_iptables)) bundled(golang(github.com/coreos/go_systemd/v22)) bundled(golang(github.com/cpuguy83/go_md2man/v2)) bundled(golang(github.com/creack/pty)) bundled(golang(github.com/cyphar/filepath_securejoin)) bundled(golang(github.com/d2g/dhcp4)) bundled(golang(github.com/d2g/dhcp4client)) bundled(golang(github.com/docker/cli)) bundled(golang(github.com/docker/distribution)) bundled(golang(github.com/docker/docker)) bundled(golang(github.com/docker/docker_credential_helpers)) bundled(golang(github.com/docker/go_connections)) bundled(golang(github.com/docker/go_metrics)) bundled(golang(github.com/docker/go_units)) bundled(golang(github.com/docker/libtrust)) bundled(golang(github.com/fatih/color)) bundled(golang(github.com/frankban/quicktest)) bundled(golang(github.com/ghodss/yaml)) bundled(golang(github.com/go_log/log)) bundled(golang(github.com/godbus/dbus/v5)) bundled(golang(github.com/gofrs/uuid)) bundled(golang(github.com/gogo/protobuf)) bundled(golang(github.com/golang/groupcache)) bundled(golang(github.com/golang/protobuf)) bundled(golang(github.com/google/go_cmp)) bundled(golang(github.com/google/go_containerregistry)) bundled(golang(github.com/google/uuid)) bundled(golang(github.com/gorilla/mux)) bundled(golang(github.com/gorilla/websocket)) bundled(golang(github.com/gosimple/slug)) bundled(golang(github.com/gosimple/unidecode)) bundled(golang(github.com/hashicorp/errwrap)) bundled(golang(github.com/hashicorp/go_multierror)) bundled(golang(github.com/inconshreveable/mousetrap)) bundled(golang(github.com/json_iterator/go)) bundled(golang(github.com/kardianos/osext)) bundled(golang(github.com/klauspost/compress)) bundled(golang(github.com/klauspost/pgzip)) bundled(golang(github.com/kr/pty)) bundled(golang(github.com/letsencrypt/boulder)) bundled(golang(github.com/mattn/go_colorable)) bundled(golang(github.com/mattn/go_isatty)) bundled(golang(github.com/mattn/go_runewidth)) bundled(golang(github.com/mattn/go_shellwords)) bundled(golang(github.com/matttproud/golang_protobuf_extensions)) bundled(golang(github.com/miekg/pkcs11)) bundled(golang(github.com/moby/locker)) bundled(golang(github.com/moby/sys/mount)) bundled(golang(github.com/moby/sys/mountinfo)) bundled(golang(github.com/moby/term)) bundled(golang(github.com/modern_go/concurrent)) bundled(golang(github.com/modern_go/reflect2)) bundled(golang(github.com/morikuni/aec)) bundled(golang(github.com/networkplumbing/go_nft)) bundled(golang(github.com/opencontainers/go_digest)) bundled(golang(github.com/opencontainers/image_spec)) bundled(golang(github.com/opencontainers/runc)) bundled(golang(github.com/opencontainers/runtime_spec)) bundled(golang(github.com/opencontainers/runtime_tools)) bundled(golang(github.com/opencontainers/selinux)) bundled(golang(github.com/opencontainers/umoci)) bundled(golang(github.com/pelletier/go_toml)) bundled(golang(github.com/pkg/errors)) bundled(golang(github.com/proglottis/gpgme)) bundled(golang(github.com/prometheus/client_golang)) bundled(golang(github.com/prometheus/client_model)) bundled(golang(github.com/prometheus/common)) bundled(golang(github.com/prometheus/procfs)) bundled(golang(github.com/rivo/uniseg)) bundled(golang(github.com/rootless_containers/proto)) bundled(golang(github.com/russross/blackfriday/v2)) bundled(golang(github.com/safchain/ethtool)) bundled(golang(github.com/seccomp/libseccomp_golang)) bundled(golang(github.com/shopspring/decimal)) bundled(golang(github.com/sigstore/sigstore)) bundled(golang(github.com/sirupsen/logrus)) bundled(golang(github.com/spf13/cobra)) bundled(golang(github.com/spf13/pflag)) bundled(golang(github.com/stefanberger/go_pkcs11uri)) bundled(golang(github.com/sylabs/json_resp)) bundled(golang(github.com/sylabs/scs_build_client)) bundled(golang(github.com/sylabs/scs_key_client)) bundled(golang(github.com/sylabs/scs_library_client)) bundled(golang(github.com/sylabs/sif/v2)) bundled(golang(github.com/syndtr/gocapability)) bundled(golang(github.com/theupdateframework/go_tuf)) bundled(golang(github.com/titanous/rocacheck)) bundled(golang(github.com/ulikunitz/xz)) bundled(golang(github.com/urfave/cli)) bundled(golang(github.com/vbatts/go_mtree)) bundled(golang(github.com/vbatts/tar_split)) bundled(golang(github.com/vbauerster/mpb/v7)) bundled(golang(github.com/vbauerster/mpb/v8)) bundled(golang(github.com/vishvananda/netlink)) bundled(golang(github.com/vishvananda/netns)) bundled(golang(github.com/xeipuuv/gojsonpointer)) bundled(golang(github.com/xeipuuv/gojsonreference)) bundled(golang(github.com/xeipuuv/gojsonschema)) bundled(golang(github.com/yvasiyarov/go_metrics)) bundled(golang(github.com/yvasiyarov/gorelic)) bundled(golang(github.com/yvasiyarov/newrelic_platform_go)) bundled(golang(mvdan.cc/sh/v3)) bundled(golang(oras.land/oras_go)) config(singularity-ce) singularity-ce singularity-ce(x86-64) singularity-ce-debuginfo: debuginfo(build-id) singularity-ce-debuginfo singularity-ce-debuginfo(x86-64) Generated by fedora-review 0.9.0 (6761b6c) last change: 2022-08-23 Command line :/usr/bin/fedora-review -b 2145834 Buildroot used: fedora-rawhide-x86_64 Active plugins: Generic, C/C++, Shell-api Disabled plugins: Haskell, PHP, Python, SugarActivity, R, Ocaml, Java, fonts, Perl Disabled flags: EPEL6, EPEL7, DISTTAG, BATCH, EXARCH ------------------------- Notes: You need to add an ExclusiveArch tag. [1] + ExclusiveArch: %{go_arches} From fedora-review above: [!]: Uses parallel make %{?_smp_mflags} macro. [!]: Package requires other packages for directories it uses. Note: No known owner of /etc/singularity/seccomp-profiles, /etc/singularity/cgroups, /usr/libexec/singularity/bin, /usr/libexec/singularity/cni, /etc/singularity/network [!]: Package must own all directories that it creates. Note: Directories without known owners: /etc/singularity/cgroups, /etc/singularity/seccomp-profiles, /usr/libexec/singularity/bin, /etc/singularity/network, /usr/libexec/singularity/cni === 1. https://docs.fedoraproject.org/en-US/packaging-guidelines/Golang/#_go_language_architectures
Spec URL: https://dctrud.fedorapeople.org/singularity-ce.spec SRPM URL: https://dctrud.fedorapeople.org/singularity-ce-3.10.4-1.fc37.src.rpm Updated the files above. > You need to add an ExclusiveArch tag. Done > [!]: Uses parallel make %{?_smp_mflags} macro. Switched to use %make_build and %make_install instead of calling make directly, as I believe this is preferred, and should handle smp macro etc? > [!]: Package requires other packages for directories it uses. > [!]: Package must own all directories that it creates. Added additional entries in %files section, for the directories reported. I'm a little confused by the docs of how to identify these, though: "It’s easy to find unowned directories with rpmls from rpmdevtools or rpm -qlv." https://docs.fedoraproject.org/en-US/packaging-guidelines/UnownedDirectories/#_tools_to_help ... I'm not clear exactly *how* it's easy to find the directories with those tools? Something simple is probably going right over my head, though. Fingers crossed this addresses everything. Thanks again for all of the prompt review actions!
(In reply to dave from comment #13) > > [!]: Uses parallel make %{?_smp_mflags} macro. > > Switched to use %make_build and %make_install instead of calling make > directly, as I believe this is preferred, and should handle smp macro etc? Yes perfect! I should've caught that in the review anyway. > > [!]: Package requires other packages for directories it uses. > > [!]: Package must own all directories that it creates. > > Added additional entries in %files section, for the directories reported. > > I'm a little confused by the docs of how to identify these, though: > > "It’s easy to find unowned directories with rpmls from rpmdevtools or rpm > -qlv." > > https://docs.fedoraproject.org/en-US/packaging-guidelines/UnownedDirectories/ > #_tools_to_help > > ... I'm not clear exactly *how* it's easy to find the directories with those > tools? Something simple is probably going right over my head, though. > I've always just used the output of fedora-review. It's very easy to run, just `fedora-review -b XXXXXX` with the bug number. You can use -n to pass it a local build. I generally run this against my packages before submitting for review just to speed up the review process because the reviewer is going to run it anyway and tell me to fix stuff :) > Fingers crossed this addresses everything. Thanks again for all of the > prompt review actions! Looks good to me now! Package is approved.
(fedscm-admin): The Pagure repository was created at https://src.fedoraproject.org/rpms/singularity-ce