https://bugzilla.redhat.com/show_bug.cgi?id=2376217 --- Comment #24 from Maxwell G <maxwell@xxxxxxx> --- I didn't yet have a chance to read through the entire thread, but here's some more feedback from the Go SIG :). Keep in mind that new Golang Packaging Guidelines are on the way that will mandate (or at least state that they SHOULD be used) use of Go Vendor Tools for managing vendored sources and also make more clear the guidelines about compiler flags and licensing and such. These guidelines are not finalized yet, but given that there are no Guidelines for vendored packages in Go yet (the current, soon-to-be-replaced Guidelines primarily cover unvendored software that use the golang-*-devel packages), I suggest that you still follow the current Go SIG practices. Also, I'll add a quick note about EPEL, as I've seen that mentioned a lot. Something not being available on RHEL or EPEL is not a valid reason to not follow the guidelines on Fedora or on a newer EPEL version that does support it. While keeping a specfile compatible with EPEL 9 and EPEL 10 is usually okay, it is becoming increasingly necessary in various packaging ecosystems to have EPEL 8 diverge. This is also the case with the new Go packaging tooling. RHEL 8 is 6 years old and time constraints and old versions of major software like RPM make it impossible to backport every set of package tooling. > I have not found this policy in the Fedora golang packaging guidelines; please point me to it if I missed it. Also please tell me if it has been specifically debated, and if not tell me how I can raise the issue for debate. The go source code tarball is not that big, only 30M, and the language is so efficient that it can fully rebuild itself using an older version compiler in just a few minutes, so I don't understand why this has to be a MUST requirement. I think it's pretty self-evident that packages should not include their own compiler toolchains and use those instead of the ones included in the distribution. Not every standard practice is explicitly documented, but when something diverges so drastically from other packages or from the spirit of the Guidelines, it needs to be carefully considered. Also, in this case, https://docs.fedoraproject.org/en-US/packaging-guidelines/#bundling applies pretty clearly. > Fedora packages SHOULD make every effort to avoid having multiple, separate, upstream projects bundled together in a single package. [...] All packages whose upstreams allow them to be built against system libraries MUST be built against system libraries. You should consider other solutions before jumping to bundling the entire Go toolchain. If the version of Go in go.mod is too new, you can try patching it. https://github.com/openbao/openbao/blob/d7909766b2757ca04a5c622954bd70d4701ee469/go.mod#L3-L12 explicitly says that it can be disregarded. You could also file an issue to ask upstream to change this practice and leave the go value in go.mod set to the actual minimum supported version of Go. You can also consider building in legacy mode with Go modules disabled (`export GO111MODULE=off`) which is what `%gobuild` currently does by default (this will probably change eventually). This will make go just use any sources in the vendor directory and disregard any Go modules metadata. Other issues: - You should `BuildRequires: go-rpm-macros` and use `%gobuild`. Please don't hardcode compiler flags, even if they mostly match Fedora's. If `%gobuild` is deficient in EPEL 8, you can add conditionally re-define `%gobuild` at the top of the specfile. Let me know if you'd like help with that, although EPEL 8 support needn't block the Fedora review if you just want to switch to `%gobuild` now and handle that later. - This package does not run unit tests. Does upstream provide unit tests that are possible to run in the mock build environment? - You should include a sysusers configuration. You can conditionally include a fallback scriptlet for releases where it isn't available. - `systemctl daemon-reload` in the `%post` scriptlet shouldn't be necessary. This will fail on container systems without systemd and the scripts generated by `%systemd_*` macros should already handle reloading the systemd service files. - The use of this `openbao-rpm` source also seems off to me. Typically, these files are sourced from upstream or stored directly in distgit and included as separate Sources in the specfile. It should be technically acceptable as long as the Licensing Guidelines are followed. That repository is missing a license file and whatever license the files it provides are covered under would need to be installed with`%license` and added to the `License:` expression. - Also keep the Guidelines' stance on specfile legibility and canonicity (https://docs.fedoraproject.org/en-US/packaging-guidelines/#_spec_legibility and https://docs.fedoraproject.org/en-US/packaging-guidelines/#_spec_maintenance_and_canonicity) in mind if you are also maintaining the package outside of the the Fedora/EPEL repositories. - The License tag should include an SPDX expression that encompasses all sources in the package, which includes vendored sources. Many Go packages have historically not followed this, mainly because the tooling to calculate a cumulative license expression for unvendored packages doesn't exist, but you should be able to this relatively simply using Go Vendor Tools. -- 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=2376217 Report this comment as SPAM: https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=report-spam&short_desc=Report%20of%20Bug%202376217%23c24 -- _______________________________________________ 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
