[Bug 2343280] New: Review Request: sigul-pesign-bridge - Bridge pesign-client requests to a Sigul signing server
https://bugzilla.redhat.com/show_bug.cgi?id=2343280
Bug ID: 2343280 Summary: Review Request: sigul-pesign-bridge - Bridge pesign-client requests to a Sigul signing server Product: Fedora Version: rawhide Hardware: All OS: Linux Status: NEW Component: Package Review Severity: medium Priority: medium Assignee: nobody@fedoraproject.org Reporter: jeremy@jcline.org QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org Target Milestone: --- Classification: Fedora
Spec URL: https://jcline.fedorapeople.org/new-packages/sigul-pesign-bridge.spec SRPM URL: https://jcline.fedorapeople.org/new-packages/sigul-pesign-bridge-0.2.1-1.fc4... Description: Drop-in replacement for pesign's daemon that bridges pesign-client requests to a Sigul server. Fedora Account System Username: jcline
https://bugzilla.redhat.com/show_bug.cgi?id=2343280
Fedora Review Service fedora-review-bot@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- URL| |https://github.com/fedora-i | |nfra/siguldry
--- Comment #1 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/8591001 (succeeded)
Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
Found issues:
- Systemd service file(s) in sigul-pesign-bridge Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/Scriptlets/#_scrip...
Please know that there can be false-positives.
--- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service
If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
https://bugzilla.redhat.com/show_bug.cgi?id=2343280
Jeremy Cline jeremy@jcline.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Doc Type|--- |If docs needed, set a value
--- Comment #2 from Jeremy Cline jeremy@jcline.org --- Whoops, I completely forgot to add the systemd scriptlets. Fixed:
Spec URL: https://jcline.fedorapeople.org/new-packages/sigul-pesign-bridge.spec SRPM URL: https://jcline.fedorapeople.org/new-packages/sigul-pesign-bridge-0.2.1-1.fc4...
https://bugzilla.redhat.com/show_bug.cgi?id=2343280
--- Comment #3 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Created attachment 2074685 --> https://bugzilla.redhat.com/attachment.cgi?id=2074685&action=edit The .spec file difference from Copr build 8591001 to 8591037
https://bugzilla.redhat.com/show_bug.cgi?id=2343280
Fedora Review Service fedora-review-bot@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Keywords| |AutomationTriaged
--- Comment #4 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/8591037 (succeeded)
Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
Please take a look if any issues were found.
--- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service
If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
https://bugzilla.redhat.com/show_bug.cgi?id=2343280
Neal Gompa ngompa13@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |fedora-review? Status|NEW |ASSIGNED Assignee|nobody@fedoraproject.org |ngompa13@gmail.com CC| |ngompa13@gmail.com
--- Comment #5 from Neal Gompa ngompa13@gmail.com --- Taking this review.
https://bugzilla.redhat.com/show_bug.cgi?id=2343280
--- Comment #6 from Jeremy Cline jeremy@jcline.org --- Just a head's up that I've released a new version upstream that has a dependency that needs packaging (https://bugzilla.redhat.com/show_bug.cgi?id=2352566) and drops the dependency on the Python sigul CLI. I'll update this request shortly with the newer version, once the dependency gets accepted.
https://bugzilla.redhat.com/show_bug.cgi?id=2343280
--- Comment #7 from Jeremy Cline jeremy@jcline.org --- Updated to v0.3.0 which drops the dependency on the Python sigul package
Spec URL: https://jcline.fedorapeople.org/new-packages/sigul-pesign-bridge.spec SRPM URL: https://jcline.fedorapeople.org/new-packages/sigul-pesign-bridge-0.3.0-1.fc4...
https://bugzilla.redhat.com/show_bug.cgi?id=2343280
--- Comment #8 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Created attachment 2080577 --> https://bugzilla.redhat.com/attachment.cgi?id=2080577&action=edit The .spec file difference from Copr build 8591037 to 8777693
https://bugzilla.redhat.com/show_bug.cgi?id=2343280
--- Comment #9 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/8777693 (succeeded)
Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
Please take a look if any issues were found.
--- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service
If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
https://bugzilla.redhat.com/show_bug.cgi?id=2343280
--- Comment #10 from Neal Gompa ngompa13@gmail.com --- Initial spec review notes:
BuildRequires: [...]
Is there a reason we're not using dynamic buildrequires here?
https://bugzilla.redhat.com/show_bug.cgi?id=2343280
--- Comment #11 from Jeremy Cline jeremy@jcline.org --- (In reply to Neal Gompa from comment #10)
Initial spec review notes:
BuildRequires: [...]
Is there a reason we're not using dynamic buildrequires here?
I just used whatever rust2rpm spat out and adjusted it for the systemd scriptlets. It does indeed appear there's a macro to autogenerate deps, but I don't see any flag on rust2rpm to enable it. I don't see documentation for the macro to allow you to skip test dependencies so that's possibly why.
It appears at least one of the test dependencies I have (tracing-test) isn't available on EPEL so I'd prefer to leave it as-is for now, or import this version into EPEL and use dynamic buildrequires for Fedora releases with the test dependencies available.
https://bugzilla.redhat.com/show_bug.cgi?id=2343280
Neal Gompa ngompa13@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |decathorpe@gmail.com
--- Comment #12 from Neal Gompa ngompa13@gmail.com --- Could you comment about this?
https://bugzilla.redhat.com/show_bug.cgi?id=2343280
--- Comment #13 from Neal Gompa ngompa13@gmail.com --- Err, Fabio please ^
https://bugzilla.redhat.com/show_bug.cgi?id=2343280
--- Comment #14 from Fabio Valentini decathorpe@gmail.com --- errr ... dynamic buildrequires should be on by default unless you run rust2rpm on OpenSUSE (or an unrecognized other distro).
It looks like you got the spec file rendered from the "plain" template, which gives you ancient defaults and no modern features, so likely not at all what you want.
You can force rust2rpm to target spec file generation at fedora with "rust2rpm --target fedora", this should give much better results.
https://bugzilla.redhat.com/show_bug.cgi?id=2343280
--- Comment #15 from Fabio Valentini decathorpe@gmail.com ---
It appears at least one of the test dependencies I have (tracing-test) isn't available on EPEL so I'd prefer to leave it as-is for now, or import this version into EPEL and use dynamic buildrequires for Fedora releases with the test dependencies available.
This also shouldn't be a blocker, importing one package into EPEL is something that takes literally 10 minutes.
https://bugzilla.redhat.com/show_bug.cgi?id=2343280
--- Comment #17 from Jeremy Cline jeremy@jcline.org --- Okay, switched to generate build dependencies.
Spec URL: https://jcline.fedorapeople.org/new-packages/sigul-pesign-bridge.spec SRPM URL: https://jcline.fedorapeople.org/new-packages/sigul-pesign-bridge-0.3.0-1.fc4...
https://bugzilla.redhat.com/show_bug.cgi?id=2343280
--- Comment #18 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Created attachment 2081743 --> https://bugzilla.redhat.com/attachment.cgi?id=2081743&action=edit The .spec file difference from Copr build 8777693 to 8813742
https://bugzilla.redhat.com/show_bug.cgi?id=2343280
--- Comment #19 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/8813742 (succeeded)
Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
Please take a look if any issues were found.
--- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service
If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
https://bugzilla.redhat.com/show_bug.cgi?id=2343280
--- Comment #20 from Jeremy Cline jeremy@jcline.org --- Alright, one more change: switch to the crates.io package and drop the awkward cd <project>. This also means adjusting the package name.
Spec URL: https://jcline.fedorapeople.org/new-packages/rust-sigul-pesign-bridge.spec SRPM URL: https://jcline.fedorapeople.org/new-packages/rust-sigul-pesign-bridge-0.3.1-...
https://bugzilla.redhat.com/show_bug.cgi?id=2343280
Jeremy Cline jeremy@jcline.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Summary|Review Request: |Review Request: |sigul-pesign-bridge - |rust-sigul-pesign-bridge - |Bridge pesign-client |Bridge pesign-client |requests to a Sigul signing |requests to a Sigul signing |server |server
https://bugzilla.redhat.com/show_bug.cgi?id=2343280
--- Comment #21 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Created attachment 2081760 --> https://bugzilla.redhat.com/attachment.cgi?id=2081760&action=edit The .spec file difference from Copr build 8813742 to 8814586
https://bugzilla.redhat.com/show_bug.cgi?id=2343280
Fedora Review Service fedora-review-bot@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Keywords|AutomationTriaged |
--- Comment #22 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/8814586 (succeeded)
Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
Found issues:
- Systemd service file(s) in sigul-pesign-bridge Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/Scriptlets/#_scrip...
Please know that there can be false-positives.
--- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service
If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
https://bugzilla.redhat.com/show_bug.cgi?id=2343280
--- Comment #23 from Neal Gompa ngompa13@gmail.com --- Spec review:
%{_sysconfdir}/sigul-pesign-bridge/
This needs to be "%dir %{_sysconfdir}/sigul-pesign-bridge", since you enumerate the file as a %config file already.
%{_mandir}/man1/sigul-pesign-bridge.1.gz
".gz" should be replaced with "*".
https://bugzilla.redhat.com/show_bug.cgi?id=2343280
--- Comment #24 from Jeremy Cline jeremy@jcline.org --- Thanks.
- replaced gz with glob - added %dir to the conf directory
Spec URL: https://jcline.fedorapeople.org/new-packages/rust-sigul-pesign-bridge.spec SRPM URL: https://jcline.fedorapeople.org/new-packages/rust-sigul-pesign-bridge-0.3.1-...
https://bugzilla.redhat.com/show_bug.cgi?id=2343280
--- Comment #25 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/8818653 (succeeded)
Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
Found issues:
- Systemd service file(s) in sigul-pesign-bridge Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/Scriptlets/#_scrip...
Please know that there can be false-positives.
--- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service
If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
https://bugzilla.redhat.com/show_bug.cgi?id=2343280
--- Comment #26 from Neal Gompa ngompa13@gmail.com ---
%post %systemd_post sigul-pesign-bridge.service
%preun %systemd_preun sigul-pesign-bridge.service
%postun %systemd_postun_with_restart sigul-pesign-bridge.service
These need to be attached to the correct binary package. They don't do anything right now.
https://bugzilla.redhat.com/show_bug.cgi?id=2343280
--- Comment #27 from Jeremy Cline jeremy@jcline.org --- (In reply to Neal Gompa from comment #26)
%post %systemd_post sigul-pesign-bridge.service
%preun %systemd_preun sigul-pesign-bridge.service
%postun %systemd_postun_with_restart sigul-pesign-bridge.service
These need to be attached to the correct binary package. They don't do anything right now.
Any pointer on where exactly they're supposed to be? I assumed after the "%package -n sigul-pesign-bridge" bit was okay.
https://bugzilla.redhat.com/show_bug.cgi?id=2343280
--- Comment #28 from Neal Gompa ngompa13@gmail.com --- You just need to add "-n sigul-pesign-bridge" to %post/%preun/%postun
https://bugzilla.redhat.com/show_bug.cgi?id=2343280
--- Comment #29 from Jeremy Cline jeremy@jcline.org --- Ah, right. Thanks.
Spec URL: https://jcline.fedorapeople.org/new-packages/rust-sigul-pesign-bridge.spec SRPM URL: https://jcline.fedorapeople.org/new-packages/rust-sigul-pesign-bridge-0.3.1-...
https://bugzilla.redhat.com/show_bug.cgi?id=2343280
--- Comment #30 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Created attachment 2082004 --> https://bugzilla.redhat.com/attachment.cgi?id=2082004&action=edit The .spec file difference from Copr build 8818653 to 8820199
https://bugzilla.redhat.com/show_bug.cgi?id=2343280
Fedora Review Service fedora-review-bot@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Keywords| |AutomationTriaged
--- Comment #31 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/8820199 (succeeded)
Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
Please take a look if any issues were found.
--- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service
If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
https://bugzilla.redhat.com/show_bug.cgi?id=2343280
Neal Gompa ngompa13@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-review? |fedora-review+ Status|ASSIGNED |POST
--- Comment #32 from Neal Gompa ngompa13@gmail.com --- Package was generated with rust2rpm, simplifying the review.
✅ package contains only permissible content ✅ package builds and installs without errors on rawhide ✅ test suite is run and all unit tests pass ✅ latest version packaged ✅ license matches upstream specification and is acceptable for Fedora ✅ license file is included with %license in %files ✅ package complies with Rust Packaging Guidelines
Package APPROVED.
===
Recommended post-import tasks:
- set up package on release-monitoring.org:
- add @rust-sig with "commit" access as package co-maintainer (should happen automatically)
- add @infra-sig with "commit" access as package co-maintainer
- set bugzilla assignee overrides to @rust-sig
- track package in koschei for all built branches (should happen automatically once rust-sig is co-maintainer)
https://bugzilla.redhat.com/show_bug.cgi?id=2343280
Fedora Admin user for bugzilla script actions fedora-admin-xmlrpc@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|POST |RELEASE_PENDING
--- Comment #33 from Fedora Admin user for bugzilla script actions fedora-admin-xmlrpc@fedoraproject.org --- The Pagure repository was created at https://src.fedoraproject.org/rpms/rust-sigul-pesign-bridge
package-review@lists.fedoraproject.org
-
bugzilla＠redhat.com