https://bugzilla.redhat.com/show_bug.cgi?id=1969450 --- Comment #14 from Martin Pitt <mpitt@xxxxxxxxxx> --- Next round! https://github.com/skobyda/cockpit-certificates/pull/66 contains the individual fixes. Once that lands, I'll apply these to the existing packages in Fedora as well (cockpit, cockpit-{machines,podman,ostree}). To avoid the weirdnesses from packit COPRs, I uploaded a "fake" upstream release 1test1 to our cockpit COPR: https://copr.fedorainfracloud.org/coprs/g/cockpit/cockpit-preview/build/2824610/ srpm: https://download.copr.fedorainfracloud.org/results/@cockpit/cockpit-preview/fedora-34-x86_64/02824610-cockpit-certificates/cockpit-certificates-1test1-1.fc34.src.rpm binary rpm: https://download.copr.fedorainfracloud.org/results/@cockpit/cockpit-preview/fedora-34-x86_64/02824610-cockpit-certificates/cockpit-certificates-1test1-1.fc34.noarch.rpm spec: https://download.copr.fedorainfracloud.org/results/@cockpit/cockpit-preview/fedora-34-x86_64/02824610-cockpit-certificates/cockpit-certificates.spec (In reply to Ben Beasley from comment #8) > - It appears to me that the Release field, > Release: 1.20210908154854083687.pr65.33.gfb97935%{?dist} > does not follow one of the established “snapshot versioning” schemes As I mentioned, this was just an artifact of packit COPR builds. The above srpm is much more representative of an actual Fedora upload. The next one will be release 2 instead of 1test1, I'll tag the release once we sorted out all the packaging bits (we keep the spec upstream for packit and our own CI) > - You need > > BuildRequires: nodejs-devel Done. (For cockpit team: This breaks in our own CI, need to add that package to our tasks container) > ExclusiveArch: %{nodejs_arches} noarch Done > Would you consider making an uncompressed copy of index.js.LICENSE.txt.gz > as > index.js.LICENSE.txt, and using that with the %license macro instead? I > don’t > think there’s an explicit guideline about this, but I feel like license > files > shouldn’t be compressed. Agreed, and a gz compressed "LICENSE" file is wrong anyway (it should at least be .gz). But no other file in /usr/share/license/ is compressed. Done. > - Nothing owns /usr/share/cockpit. As discussed above, this is much better fixed in cockpit itself: https://github.com/cockpit-project/cockpit/pull/16361 Once that lands, cockpit-bridge will own it, which is a dependency of c-certificates. > - In general, when installing compiled/minified JavaScript, you must install > the original unminified sources That's the only thing that I did not do, see the discussion above. For now I can't believe that the packaging policy would demand shipping source code in binary packages. > Provides: bundled(nodejs-object-assign) = 4.1.1 > (etc). All done. This reminds me that we want to lose moment.js, I'll work on that upstream. > - If the source tarballs are not URLs, you must document how to re-generate The real spec file does have URLs. This was just an artifact of packit COPRs, which replace SourceN: with the locally generated tarballs. Sorry for the confusion! > - I think the reason no tests are run is that the upstream tests can’t be run > in the RPM build environment because they require browsers, docker, etc. > Please add a spec file comment about this so that it is obvious that the > lack > of a %check section is not an oversight. Good idea, done. > - By convention, these lines belong in %prep rather than in %build: > > # ignore pre-built webpack in release tarball and rebuild it > rm -rf dist Done. Thanks a lot for taking the time, really appreciated! Martin -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component https://bugzilla.redhat.com/show_bug.cgi?id=1969450 _______________________________________________ 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 on the list, report it: https://pagure.io/fedora-infrastructure
