https://bugzilla.redhat.com/show_bug.cgi?id=2376217 --- Comment #30 from Maxwell G <maxwell@xxxxxxx> --- (In reply to Dave Dykstra from comment #27) > (In reply to Maxwell G from comment #26) > > You can set GO_BUILDTAGS="<space-separated list of tags>". This doesn't work > > on EPEL 8, so you can use a fallback there. On Fedora, EPEL 9, and EPEL 10 > > once go-rpm-macros is updated, you can now directly pass arbitrary extra > > flags to `%gobuild` (other than -tags or -ldflags which are already part of > > the %gobuild definition)` such as -gcflags. > > Ok I switched to using the system %gobuild macro on el9 and up. I didn't > yet see the ability to set -gcflags so for now I also redefine %gobuild when > %go_debug is set. You should be able to pass `-gcflags` directly to the %gobuild macro. > > I wasn't able to figure out how to use it successfully with the default > GO111MODULES=off so I followed what was done by forgejo and set `%global > gomodulesmode GO111MODULE=on`. If I don't do that it isn't able to find the > main source code module; it gets an error like this: > > main.go:9:2: cannot find package "github.com/openbao/openbao/command" in any > of: > /usr/lib/golang/src/github.com/openbao/openbao/command (from $GOROOT) > /nashome/d/dwd/gopath/src/github.com/openbao/openbao/command (from $GOPATH) > > That seems like such a fundamental issue that there must surely be a way > around it but I didn't see how; probably you can tell me. Ah, right, this won't work, since you aren't using the the standard Go macros that create a GOPATH tree and handle this for you. Those macros are only available on EL 9 and above. You could do it manually if needed (see below), but relying on GO111MODULE=on that doesn't require this is also an option. Something like: ``` mkdir -p _build/src/github.com/openbao ln -s "$(pwd)" _build/src/github.com/openbao/openbao ``` and then having `export GOPATH="$(pwd)/_build"` before running gobuild and gotest would work. > > > > There are a lot of ci checks done upstream with each PR. By unit tests, are you talking about testing individual source units that go into making openbao, or testing the openbao final binary "unit"? I assume it's the latter. I inquired about tests for that and was told that it's on their wish list but those kinds of release validation tests unfortunately do not yet exist. > > > > I was referring to the Go unit tests (*_test.go files) that can be run with > > `go test` (or the `%gocheck` macro). > > Oh, there are those tests. However I learned that they take quite a long > time and some of them use network. Is network allowed during the test > phase? Since they're tests on the unchanged source code which have already > been run upstream I don't see much value in running them again. If you can run a subset of tests that don't require network, that would be great, as running some form of tests is generally a requirement for Go packages. Otherwise, please add a comment explaining that tests cannot be run. > > > The package does already have a sysusers configuration. > > > > It does, but there are still obsolete scripts to create the users. On Fedora > > 42+, these scriptlets should not be included, as they will be created > > automatically based on the sysusers file. > > Oh. Ok I surrounded that with a conditional to only be used on el8. I would surround the entire scriptlet (including the %pre part) in a conditional so an empty scriptlet isn't there for releases where it isn't needed. Also, when the manual %pre scriptlet is in use, you need to include "Requires(pre): shadow-utils." Also, it looks like the conditional is wrong. The sysusers macros were backported to epel-rpm-macros in epel8, but they don't seem to exist on epel9. They exist in RHEL 10 itself. In all current EPEL releases that do support the sysusers macros (i.e, not epel9), I think you still have to use %sysusers_create_compat documented in https://docs.fedoraproject.org/en-US/packaging-guidelines/UsersAndGroups/#_dynamic_allocation; it's not just done automatically when a sysusers file is included in the package like it is on Fedora 42+. Anyways, this whole situation is confusing and not properly documented as far as I can tell, so I would suggest you bring it to the EPEL SIG's attention. At least, Fedora releases should be using the sysusers integration and you can still include the %sysusers_compat macros if you need them for older releases. Other than that, 🤷. In case you haven't fully realized, maintaining EPEL 8 in the same specfile as other releases and not using the go2rpm template recommended by the Go SIG is adding significant complication to this package and the review. I'm doing my best with my limited time to review this package and apply the Packaging Guidelines. > > I'll let others chime in, but I suppose including a source archive from the > > opensciencegrid repository for the config files isn't the end of the world, > > if not super typical, but please make sure it's Source1, not Source0 which > > should be reserved for the primary upstream archive, and add a comment above > > the Source line explaining what it's used for. > > Done. Thanks. > > > I would also consider > > contributing these files upstream so other users and distro packagers can > > use the same files and to avoid the need to maintain them in a separate > > place and carry an extra source in the package. > > I will see if they're interested. They currently build rpms with goreleaser > so I'm not sure they'll want these. Ack. Never hurts to ask :). Also, take a look at https://stackoverflow.com/questions/26898007/making-an-rpm-which-sets-posix-files-capabilities to apply the caps to the binary (as opposed to using a scriptlet), and please include a comment about why this is necessary for the sake of anyone auditing the package. You should also remove the explicit dependencies on epel-rpm-macros and rpmautospec-rpm-macros. epel-rpm-macros is already included in the default EPEL build environment. -- 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=2376217 Report this comment as SPAM: https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=report-spam&short_desc=Report%20of%20Bug%202376217%23c30 -- _______________________________________________ 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
