[Bug 2145834] Review Request: singularity-ce - Application and environment virtualization

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



https://bugzilla.redhat.com/show_bug.cgi?id=2145834



--- Comment #4 from dave@xxxxxxxxxxxx <dave@xxxxxxxxxxxx> ---
(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@xxxxxxxxx> 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@xxxxxxxxx> - 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.


-- 
You are receiving this mail because:
You are always notified about changes to this product and component
You are on the CC list for the bug.
https://bugzilla.redhat.com/show_bug.cgi?id=2145834
_______________________________________________
package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx
Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: https://lists.fedoraproject.org/archives/list/package-review@xxxxxxxxxxxxxxxxxxxxxxx
Do not reply to spam, report it: https://pagure.io/fedora-infrastructure/new_issue




[Index of Archives]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite Conditions]     [KDE Users]

  Powered by Linux