Spec URL: https://raw.githubusercontent.com/opensciencegrid/openbao-rpm/refs/tags/v2.3.1/openbao.spec SRPM URL: https://github.com/opensciencegrid/openbao-rpm/releases/download/v2.3.1/openbao-2.3.1-1.src.rpm Description: OpenBao is a Linux Foundation project forked from the very popular Hashicorp Vault that is a general purpose secret storage engine Fedora Account System Username: dwd
Just reading the .spec file quick: Requires(post): systemd Requires(preun): systemd Requires(postun): systemd This is probably meant to be: BuildRequires: systemd-rpm-macros You just need enough so the macros can be expanded. All the "%{?_isa}" can go. There is a macro for /usr/lib/systemd/system/ - %{_unitdir} I think. I'm not of fan of all the # Older versions of this package had opposite symlinks for vault compatibility, # and rpm needs help to handle that. and similar - this package was never in Fedora so its simply not this packages problem to fix previous state. This gets rid if all that was it running stuff. Fedora package do not do this. For the user/group add stuff please use a sysusers.d record - https://docs.fedoraproject.org/en-US/packaging-guidelines/UsersAndGroups/ All the bundling is hard for me however I know this has been relaxed a lot more with golang. This probably needs a real go packager to look at this.
Thanks for the quick feedback. > BuildRequires: systemd-rpm-macros > All the "%{?_isa}" can go. > There is a macro for /usr/lib/systemd/system/ - %{_unitdir} I think. Ok, good to know. I'll update those. > this package was never in Fedora so its simply not this packages problem to fix previous state Ok, I'll handle it then by making a transition release outside of Fedora and making sure all previous users upgrade to that first. I think it would be good to make sure the package is ready to be accepted first in case there are any other things I'll need in the transition release. > For the user/group add stuff please use a sysusers.d record Right, I had just figured that out because I saw an error installing into fedora 42. > All the bundling is hard for me however I know this has been relaxed a lot more with golang. This probably needs a real go packager to look at this. I used the exact same technique that is used in Fedora by apptainer and singularity-ce to create the Provides: bundled(golang()) statements and the LICENSE_DEPENDENCIES.md file.
I have updated the tag after applying changes in response to your comments. Please ignore the sections surrounded by '%if "%{?osg}" != ""' which I plan to remove before the final submission. I'm just leaving them in there for now because I'll need them for my final package release outside of Fedora/EPEL and it avoids merging I would need to do if I kept two copies with and without those sections.
Now that https://fedoraproject.org/wiki/Changes/GolangPackagesVendoredByDefault has been accepted I would suggest to adopt Go Vendor Tools to maintain the package for Fedora and EPEL in the future ( https://gitlab.com/fedora/sigs/go/go-vendor-tools/-/issues/10 ). You can use "go2rpm -p vendor --name openbao github.com/openbao/openbao" to create the initial common spec. > %global oldname vault > (...) > %package %{oldname}-compat > Summary: Vault-compatible command and service > Requires: %{name} = %{version}-%{release} > Provides: %{oldname} = %{version}-%{release} > Obsoletes: %{oldname} < 2.0 > %description %{oldname}-compat > Provides a compatibility layer on top of OpenBao to emulate a Hashicorp > Vault package. Does this make sense if the package is new to Fedora? > # This is to avoid > # *** ERROR: No build ID note found > %global debug_package %{nil} This shouldn't be required > Release: 1%{?dist} > (...) > %changelog You can use %autorelease and %autochangelog macros to ease maintenance. Feel free to join Fedora Golang matrix channel for any help.
Thanks for the comments. I tried the go2rpm packaging first but quickly ran into the fact that it isn't supported in EPEL. EPEL support is needed for this package. I do hope that the compat package is allowed. It is optional and it is a convenience for those who are used to vault. It's similar to podman's addon compatibility package for docker. I'll try leaving out the `%global debug_package %{nil}` and using %autorelease and %autochangelog.
> I tried the go2rpm packaging first but quickly ran into the fact that it isn't supported in EPEL. EPEL support is needed for this package. EPEL support is planned and it will really make things easier for you in the future. I think it's worth waiting, but can't block your package if you don't want to. I created the spec using go2rpm and with just a few tweaks (GO111MODULE changes, using %gotest instead of %gocheck) you can see how much simpler the spec is and it's based in a common template. https://mikel.olasagasti.info/tmp/fedora/openbao.spec It still requires to define the license (don't have time for that now) and see which tests are required to be skipped (docker dependencies for example), but the basics are there. >I do hope that the compat package is allowed. It is optional and it is a convenience for those who are used to vault. It's similar to podman's addon compatibility package for docker. OK, fine for me.
The %autorelease and %autochangelog seem OK but I got a different error when I removed `%global debug_package %{nil}` than what was in the comment: "Empty %files file debugsourcefiles.list". I saw a suggestion on the internet to not strip symbols if that error is hit, but removing the strip command didn't help. I had previously inserted the strip command because fedora-review noticed that the bao binary wasn't stripped.
> when I removed `%global debug_package %{nil}` than what was in the comment: "Empty %files file debugsourcefiles.list". > I saw a suggestion on the internet to not strip symbols if that error is hit, but removing the strip command didn't help. > I had previously inserted the strip command because fedora-review noticed that the bao binary wasn't stripped. You shouldn't strip it manually. I'm not 100% sure if this is enough, but shouldn't be needed. Use %gobuild macro instead of "go build", that way Fedora's default builds flags are used. You can check a simple spec like Opentofu's to see how simple the spec can be. Macros apply even if you don't want to use go-vendor tools. https://src.fedoraproject.org/rpms/opentofu/blob/rawhide/f/opentofu.spec If you want to change the default flags, you can check how kubernetes spec is doing it: https://src.fedoraproject.org/rpms/kubernetes1.33/blob/rawhide/f/kubernetes1.33.spec#_259
Unfortunately I do not see a way to add tags using the %gobuild macro. Also, I do not see a way to add gcflags which is less important but I use that when I want to enable debugging with delve. However, by borrowing things from the definition of the %gobuild macro along with some additional input from the apptainer Fedora package I was able to come up with a combination of go build options that enabled rpm to create debugsource and debuginfo packages.
The tag is updated again with the latest discussed changes. It has been built on copr at https://copr.fedorainfracloud.org/coprs/dwd/openbao/build/9275972/ One build failed for an unrelated intermittent reason and was built successfully after resubmission at https://copr.fedorainfracloud.org/coprs/dwd/openbao/build/9277391/ I'm reposting the original URLs because fedora-review -b 2376217 is confused by the opentofu.spec URL above. Spec URL: https://raw.githubusercontent.com/opensciencegrid/openbao-rpm/refs/tags/v2.3.1/openbao.spec SRPM URL: https://github.com/opensciencegrid/openbao-rpm/releases/download/v2.3.1/openbao-2.3.1-1.src.rpm
Go Vendor Tools is being published in EPEL10 and EPEL9, so it's a matter of ~7 days to be available for everyone. I strongly recommend switching to the default tooling and template. - EPEL9 https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2025-0eb4465a48 - EPEL10.1 https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2025-a8336ca70d > Unfortunately I do not see a way to add tags using the %gobuild macro. You can use "export BUILDTAGS=foo", there are multiple specs you can see how can be done: podman, skopeo, snapd or incus for example. > Also, I do not see a way to add gcflags which is less important but I use that when I want to enable debugging with delve. Isn't the automatically created "debugsource" package enough for that? > Source0: https://github.com/opensciencegrid/%{name}-rpm/releases/download/v%{package_version}/%{name}-rpm-%{package_version}.tar.gz > # This is created by ./make-source-tarball and included in release assets > Source1: https://github.com/opensciencegrid/%{name}-rpm/releases/download/v%{package_version}/%{name}-src-%{package_version}.tar.gz Why are not official sources used?
> Go Vendor Tools is being published in EPEL10 and EPEL9 I need to support EPEL8 too. If they're also going to be backported to EPEL8 I could try them again, although I'm skeptical that they'll be able to support all the special needs of this package. > you can use "export BUILDTAGS=foo" Oh I see that supported on the EL9 %gobuild macro but I was looking on EL8 and it's not in the %gobuild macro there. > Isn't the automatically created "debugsource" package enough for that? It does make delve function but '-gcflags all="-N -l"' turns off the optimizer and inlining (according to https://pkg.go.dev/cmd/compile) which makes things easier to debug. > Why are not official sources used? The official sources are included in Source1. I'm not using them directly because of the koji requirement to not do any internet accesses during build. There are some initial steps to the compilation process that do require internet access, and those are done during in the make-source-tarball script. It should be possible to remove the original sources from that tarball and have the %prep step recombine them, but it would be a hassle because the additional files are scattered throughout the source tree. Is it required? I would probably have it remove all the non-directories in the output of `tar tf` on the original source tarball. By the way the constructed source tarball is about 50 times the size of the original source tarball. It was more before I figured out I could remove some big pieces.
> The official sources are included in Source1. then use the official source > I'm not using them directly because of the koji requirement to not do any internet accesses during build. They would be part of the srpm package or downloaded as any other source package, what's the problem? > There are some initial steps to the compilation process that do require internet access, and those are done during in the make-source-tarball script. It should be possible to remove the original sources from that tarball and have the %prep step recombine them, but it would be a hassle because the additional files are scattered throughout the source tree. Is it required? I would probably have it remove all the non-directories in the output of `tar tf` on the original source tarball. I understand that `make-source-tarball` prepares the system to run `make bootstrap && make static-dist && make prep`. - bootstrap: it mentions they're extra tools for developers. Is it required in the Fedora & EPEL context? - static-dist: can't upstream release the UI bits as part of the release? - prep: "runs `go generate` to build the dynamically generated source files.". Cant these also be part of the release or during %prep? Use the official source tarball and create another one with the generated bits, but discuss with upstream the possibility of the inclusion of the extra bits as part of the release.
Thanks for the insights. You're right that the bootstrap and prep steps are unnecessary. I am working on an upstream PR to generate a "fat" distribution source tarball as part of the github release assets.
2.3.2 has been released upstream including the distribution source tarball so I updated the spec to read from there. Here's the new URLs Spec URL: https://raw.githubusercontent.com/opensciencegrid/openbao-rpm/refs/tags/v2.3.2/openbao.spec SRPM URL: https://github.com/opensciencegrid/openbao-rpm/releases/download/v2.3.2/openbao-2.3.2-1.src.rpm Please re-review.
Copr build: https://copr.fedorainfracloud.org/coprs/build/9389770 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2376217-openbao/fedora-rawhide-x86_64/09389770-openbao/fedora-review/review.txt Found issues: - No gcc, gcc-c++ or clang found in BuildRequires Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/C_and_C++/ - Systemd service file(s) in openbao, openbao-vault-compat Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/Scriptlets/#_scriptlets 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.
> %global go_version 1.24.6 > Source2: https://golang.org/dl/go%{go_version}.src.tar.gz > # put go src inside the above dir > %setup -q -D -T -a 2 -c -n %{name}-dist-%{package_version} Why is this needed for? You should use whatever Go version Fedora and EPEL ships, which might be different on each available release. This is a hard blocker. > Source0: https://github.com/opensciencegrid/%{name}-rpm/releases/download/v%{package_version}/%{name}-rpm-%{package_version}.tar.gz Is this required anymore? > if [ "$uname_m" = ppc64le ]; then > GO_BUILD_MODE="-buildmode default" > else > GO_BUILD_MODE="-buildmode pie" > fi What's the problem with ppc64le? > BUILD_DATE="$(date --utc +%Y-%m-%dT%H:%M:%SZ)" You can use: $(date -d "@${SOURCE_DATE_EPOCH}" +%Y-%m-%d) > # These are from the %%gobuild macro which we can't use because it doesn't > # allow for extra tags (nor extra gcflags for debug mode). > GO_BUILD_TAGS+=" rpm_crashtraceback libtrust_openssl" > GO_BUILD_LDFLAGS+=" -linkmode=external -compressdwarf=false" > GO_BUILD_LDFLAGS+=" -extldflags '%__global_ldflags'" > > %if "%{?go_debug}" != "" > # add debugging & testing flags > GO_BUILD_GCFLAGS="all=-N -l" > GO_BUILD_LDFLAGS+=" -X github.com/%{name}/%{name}/version.VersionMetadata=testonly" > # openbao documentation says testonly should not be used for production builds > GO_BUILD_TAGS+=" testonly" > %endif Do you plan to build debug builds for Fedora or EPEL? I don't think they belong here. > %if "%{?osg}" != "" here are multiple entries like this. Same thing, not sure if they should belong here for Fedora & EPEL.
(In reply to Mikel Olasagasti Uranga from comment #17) > > %global go_version 1.24.6 > > Source2: https://golang.org/dl/go%{go_version}.src.tar.gz > > # put go src inside the above dir > > %setup -q -D -T -a 2 -c -n %{name}-dist-%{package_version} > > Why is this needed for? > > You should use whatever Go version Fedora and EPEL ships, which might be > different on each available release. This is a hard blocker. The upstream openbao requires a newer version of Go than is currently shipped in EPEL and I believe Fedora although maybe not the latest version of Fedora. Every release they update to the latest one. The build step compiles the required version of go using the OS version. Go keeps making it harder to continue to use an older version by continually updating the version required in go.mod so I don't see a way around this. I could have it check to see if the current version is new enough and skip the build. (We can't include a go binary instead because that only works on one architecture.) > > Source0: https://github.com/opensciencegrid/%{name}-rpm/releases/download/v%{package_version}/%{name}-rpm-%{package_version}.tar.gz > > Is this required anymore? The way that it is currently done, yes. Is it a problem? It would be possible to individually add all the extra files instead if necessary but this is easier. > > if [ "$uname_m" = ppc64le ]; then > > GO_BUILD_MODE="-buildmode default" > > else > > GO_BUILD_MODE="-buildmode pie" > > fi > > What's the problem with ppc64le? I didn't actually verify it, but that was what was done for the apptainer build in EPEL for some reason, so I followed what it did. I assumed the %gobuild macro which we can't use does it differently on ppc64le. > > BUILD_DATE="$(date --utc +%Y-%m-%dT%H:%M:%SZ)" > > You can use: > > $(date -d "@${SOURCE_DATE_EPOCH}" +%Y-%m-%d) Ok I'll try that. > > # These are from the %%gobuild macro which we can't use because it doesn't > > # allow for extra tags (nor extra gcflags for debug mode). > > GO_BUILD_TAGS+=" rpm_crashtraceback libtrust_openssl" > > GO_BUILD_LDFLAGS+=" -linkmode=external -compressdwarf=false" > > GO_BUILD_LDFLAGS+=" -extldflags '%__global_ldflags'" > > > > %if "%{?go_debug}" != "" > > # add debugging & testing flags > > GO_BUILD_GCFLAGS="all=-N -l" > > GO_BUILD_LDFLAGS+=" -X github.com/%{name}/%{name}/version.VersionMetadata=testonly" > > # openbao documentation says testonly should not be used for production builds > > GO_BUILD_TAGS+=" testonly" > > %endif > > Do you plan to build debug builds for Fedora or EPEL? I don't think they > belong here. Not for Fedora or EPEL, but the same .spec file is used for development so I'd rather leave it in. People can rebuild the src rpm with the extra option if they need to do debugging. > > %if "%{?osg}" != "" > > here are multiple entries like this. Same thing, not sure if they should > belong here for Fedora & EPEL. As I said above, I will remove those before final submittal. Please disregard them. They were just there for my convenience until I build the last openbao transitioning rpm outside of EPEL. I'm actually currently working on doing that transition build so I could probably remove this soon.
> The upstream openbao requires a newer version of Go than is currently shipped in EPEL and I believe Fedora although maybe not the latest version of Fedora. Every release they update to the latest one. The build step compiles the required version of go using the OS version. Go keeps making it harder to continue to use an older version by continually updating the version required in go.mod so I don't see a way around this. I could have it check to see if the current version is new enough and skip the build. (We can't include a go binary instead because that only works on one architecture.) Packages for Fedora and EPEL need to be built with whatever Go version is being shipped at that moment. If a package requires a newer Go version, then it's up to the packager to workaround it (reverting a change, disabling features, working with Golang mantainers, not updating the package, ...), but using a custom Go is not an acceptable method. If you think openbao can't work this way, then the package may not be acceptable for Fedora/EPEL and COPR would be a better place for it.
It will be easy enough to apply a patch this time because the el8/9/10 version is almost up to date currently, plus it turns out that the update to the released version of go required by openbao was updated from the el8/9/10 version very shortly before the release. I will go ahead and make that change. I will also try to persuade the openbao project to slow down its updates of required go version. However, I'm not sure this is a sustainable approach given the speed at which golang is updated upstream vs when it is updated in el8/9/10. The latter often for months ships versions that are no longer supported by upstream golang. Recent versions of go also insert their version into go.mod more frequently than they used to, so dependencies are frequently requiring newer versions of go. I am the release manager of apptainer and I work hard there to avoid requiring a version of golang newer than el8/9/10 supplies, and I often have to defer dependabot updates for many months when dependencies require newer versions. So I have first hand experience with this situation and so far have not run into any high severity security updates in dependencies that make an update urgent, but I don't expect that will always be the case. 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.
The 2.3.2 tag and above URLs have now been updated to 1. Apply a patch to downgrade to go 1.24.4 instead of including the source for and building 1.24.6. As a result, it fails on fedora 41 but I just won't build it there. 2. Always use -buildmode pie, even on ppc64le. It built OK there anyway according to copr at https://copr.fedorainfracloud.org/coprs/dwd/openbao/build/9399539/. 3. I removed the use of the %{?osg} macro. I couldn't use ${SOURCE_DATE_EPOCH} because it is not defined on el8, 9, or 10. It is only defined on Fedora. Thank you for all your comments, the spec file is much improved over it was. Is anything else required?
> I have not found this policy in the Fedora golang packaging guidelines; please point me to it if I missed it. I think it's not clearly stated anywhere, so I opened a FESCo ticket for advice: https://pagure.io/fesco/issue/3464 > 1. Apply a patch to downgrade to go 1.24.4 instead of including the source for and building 1.24.6. As a result, it fails on fedora 41 but I just won't build it there. Should be OK after this update lands stable: https://bodhi.fedoraproject.org/updates/FEDORA-2025-6ce837b350 But, can you enable F41 in your COPR to verify it better? > I couldn't use ${SOURCE_DATE_EPOCH} because it is not defined on el8, 9, or 10. It is only defined on Fedora. It's working fine for me in `gh` for EL9 & EL10. It may be missing for EL8. Can you use something like podman does like: LDFLAGS="-X %{ld_libpod}/define.buildInfo=${SOURCE_DATE_EPOCH:-$(date +%s)} \ > Thank you for all your comments, the spec file is much improved over it was. Agree, nice to see progress :) > Is anything else required? Yes, I think there a few other things to change. > %global go_version 1.24.6 This is a leftover > License: MPL-2.0 I think it should include the complete list of licenses as it's a vendored package. > BuildRequires: golang Should be golang-bin > Source0: https://github.com/opensciencegrid/%{name}-rpm/releases/download/v%{package_version}/%{name}-rpm-%{package_version}.tar.gz For what is this needed? If is to include different helper files, it would be better to list them individually and push them to dist-git rather than having an external repo > RPMDIR=$(pwd) is this a leftover? > uname_m=$(uname -m) same > %files > %files %{oldname}-compat Can these two blocks be the last blocks before %changelog block? I need to check applied Go flags, but will need more time for that.
I applied all your suggested changes and re-ran it on copr including f41 at https://copr.fedorainfracloud.org/coprs/dwd/openbao/build/9405232/
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.
Maxwell: I'm no longer bundling the go toolchain, it's just that I think that there may be situations in the future in EPEL where it will be needed, when needed dependencies require a newer version than available in RHEL. We can consider that to be a separate issue, now tracked in https://pagure.io/fesco/issue/3464. The main problem with %gobuild is not that it isn't supported in EPEL8, it is that it doesn't support adding tags as far as I can tell, and openbao needs to add a tag. Do you know of a way to do it? There's also no way to add gcflags, although I only use those for optional debugging so they're not strictly required. 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. The package does already have a sysusers configuration. I removed the `systemctl daemon-reload`. I added an MIT-2.0 LICENSE file to the openbao-rpm repo, the same as the upstream. I understand that the spec file will need to be primarily sourced for Fedora & EPEL from the pkgs.fedoraproject.org git, but I'd like to be able to continue to use this as the source for the extra files if it is allowed. That way there's a single primary source for those extra files instead of a copy per fNN and epelN branch, and I can have github actions to test things before building on koji. I thought the latest iteration did have a License tag in a proper SPDX expression including the licenses of all the vendored packages, but I see now it is supposed to have `AND` as uppercase instead of lowercase. I made that change too.
Hi, > The main problem with %gobuild is not that it isn't supported in EPEL8, it is that it doesn't support adding tags as far as I can tell, and openbao needs to add a tag. Do you know of a way to do it? There's also no way to add gcflags, although I only use those for optional debugging so they're not strictly required. 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 defintion)` such as -gcflags. > 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). > 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. > I added an MIT-2.0 LICENSE file to the openbao-rpm repo, the same as the upstream. I understand that the spec file will need to be primarily sourced for Fedora & EPEL from the pkgs.fedoraproject.org git, but I'd like to be able to continue to use this as the source for the extra files if it is allowed. That way there's a single primary source for those extra files instead of a copy per fNN and epelN branch, and I can have github actions to test things before building on koji. 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. 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 thought the latest iteration did have a License tag in a proper SPDX expression including the licenses of all the vendored packages, but I see now it is supposed to have `AND` as uppercase instead of lowercase. I made that change too. Thanks, I see it now. It seems one of the sources is licensed under CC0 which is not allowed for new code in Fedora. If you identify which library has this license, you could maybe do something like https://src.fedoraproject.org/rpms/forgejo/blob/rawhide/f/forgejo.spec#_23, but otherwise, you need to ask the upstream to change the License or get Fedora Legal approval.
(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. 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. > > 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. > > 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'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. > 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. > It seems one of the sources is licensed under CC0 > which is not allowed for new code in Fedora. If you identify which library > has this license, you could maybe do something like > https://src.fedoraproject.org/rpms/forgejo/blob/rawhide/f/forgejo.spec#_23, > but otherwise, you need to ask the upstream to change the License or get > Fedora Legal approval. It is the exact same library that forgejo is using so I copied that comment. The copr build is at https://copr.fedorainfracloud.org/coprs/dwd/openbao/build/9434111/.
Maxwell, please take another look.
Sorry for my delay! Can you please include the Spec URL and SRPM URL in the following format after each change so the fedora-review and review bot tooling will work properly? Spec URL: <URL HERE> SRPM URL: <URL HERE> I will look at the Copr build in the meantime...
(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.
Also, how are the bundled() Provides generated? This needs to be documented (a specfile comment is fine). You could use the generator that works if you add "%license vendor/modules.txt" to the %files section and then remove these manual Provides, but that generator is not available on epel8 (we could consider backporting it to the go-rpm-macros-epel package).
> You should be able to pass `-gcflags` directly to the %gobuild macro. It works for me everywhere except el10 where I get an error Unknown option g in gobuild(o:) Oh well, I changed it to pass the option anyway when debug is enabled, because currently I don't have a need to enable it on el10 and presumably it will eventually catch up. (I ended up with two different %gobuild lines because %{?GO_BUILD_GCFLAGS:-gcflags "${GO_BUILD_GCFLAGS}"} did not work as I expected and didn't pass anything even when GO_BUILD_GCFLAGS was defined). > relying on GO111MODULE=on that doesn't require this is also an option Ok I'll stick with that option then. I included a comment to say why. > Otherwise, please add a comment explaining that tests cannot be run. Done. > I would surround the entire scriptlet (including the %pre part) in a conditional I did that but later removed it because I switched to %sysusers_create_compat. > when the manual %pre scriptlet is in use, you need to include "Requires(pre): shadow-utils." Ok. That's now done with %{?sysusers_requires_compat}. > The sysusers macros were backported to epel-rpm-macros in epel8, but they don't seem to exist on epel9. No, they're there in the systemd-rpm-macros. I switched to using them. > caps ... and please include a comment about why this is necessary for the sake of anyone auditing the package. Thanks for asking about that, because I found out that openbao removed the need for it, so I removed it. Rationale is at https://openbao.org/docs/rfcs/mlock-removal/. > You should also remove the explicit dependencies on epel-rpm-macros and rpmautospec-rpm-macros. Done > Also, how are the bundled() Provides generated? This needs to be documented (a specfile comment is fine). It is done with an awk/sed pipeline in the make-spec script that reads from go.mod. Comment added. > Can you please include the Spec URL and SRPM URL in the following format after each change Ok. Spec URL: https://raw.githubusercontent.com/opensciencegrid/openbao-rpm/refs/tags/v2.3.2/openbao.spec SRPM URL: https://github.com/opensciencegrid/openbao-rpm/releases/download/v2.3.2/openbao-2.3.2-1.src.rpm Copr build: https://copr.fedorainfracloud.org/coprs/dwd/openbao/build/9493010/
Created attachment 2104882 [details] The .spec file difference from Copr build 9389770 to 9493032
Copr build: https://copr.fedorainfracloud.org/coprs/build/9493032 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2376217-openbao/fedora-rawhide-x86_64/09493032-openbao/fedora-review/review.txt Found issues: - No gcc, gcc-c++ or clang found in BuildRequires Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/C_and_C++/ - Systemd service file(s) in openbao, openbao-vault-compat Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/Scriptlets/#_scriptlets 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.
A new upstream version was released so I updated it. Spec URL: https://raw.githubusercontent.com/opensciencegrid/openbao-rpm/refs/tags/v2.4.0/openbao.spec SRPM URL: https://github.com/opensciencegrid/openbao-rpm/releases/download/v2.4.0/openbao-2.4.0-1.src.rpm Copr build: https://copr.fedorainfracloud.org/coprs/dwd/openbao/build/9500768/ Please re-review.
Created attachment 2105176 [details] The .spec file difference from Copr build 9493032 to 9505965
Copr build: https://copr.fedorainfracloud.org/coprs/build/9505965 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2376217-openbao/fedora-rawhide-x86_64/09505965-openbao/fedora-review/review.txt Found issues: - No gcc, gcc-c++ or clang found in BuildRequires Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/C_and_C++/ - Systemd service file(s) in openbao, openbao-vault-compat Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/Scriptlets/#_scriptlets 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.
I just pushed https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2025-3d60df796c that provides a %gobuild backport for EPEL 8. If you depend on go-rpm-macros, this will pull in the go-rpm-macros-epel backport on EPEL 8 that overrides the broken %gobuild definition, and then you can remove the long custom go build invocation from the specfile that I am not so happy with. Also, I ran go_vendor_license on your package, and it seems some licenses are not accounted for: > vendor/github.com/apparentlymart/go-textseg/v13/LICENSE: (MIT AND Apache-2.0) AND Unicode-DFS-2016 > vendor/github.com/apparentlymart/go-textseg/v15/LICENSE: (MIT AND Apache-2.0) AND Unicode-DFS-2016 > vendor/github.com/Nvveen/Gotty/LICENSE: BSD-2-Clause-Views I don't see the Unicode nor the BSD-3-Clause-Views license included in the License tag.
(In reply to Maxwell G from comment #38) > I just pushed > https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2025-3d60df796c that > provides a %gobuild backport for EPEL 8. If you depend on go-rpm-macros, > this will pull in the go-rpm-macros-epel backport on EPEL 8 that overrides > the broken %gobuild definition, and then you can remove the long custom go > build invocation from the specfile that I am not so happy with. Ok good, I will use that once it is promoted to epel stable. > Also, I ran go_vendor_license on your package, and it seems some licenses > are not accounted for: > > > vendor/github.com/apparentlymart/go-textseg/v13/LICENSE: (MIT AND Apache-2.0) AND Unicode-DFS-2016 > > vendor/github.com/apparentlymart/go-textseg/v15/LICENSE: (MIT AND Apache-2.0) AND Unicode-DFS-2016 > > vendor/github.com/Nvveen/Gotty/LICENSE: BSD-2-Clause-Views > > I don't see the Unicode nor the BSD-3-Clause-Views license included in the > License tag. I see that they were not recognized by the github.com/google/go-licenses tool now used by upstream openbao. I'll add them to my specfile generator for the next version. Is there anything else?
Thanks. The licensing is a blocker for the package, so please make sure that's fixed. Go Vendor Tools is now available on EPEL 9, 10.0, 10.1, and 10.2 in addition to Fedora, so that is the preferred approach there, but at the very least, the License: expression in the specfile should agree with what a more thorough scan using Go Vendor Tools detects. > Is there anything else? - Using the vendored Provides license generator would be preferred over listing them manually, so I'll see if I can also backport that to EPEL 8 and then you can use it there, but that shouldn't be a blocker. - Are there bundled web assets (HTML, CSS, JS) in this package? I didn't realize that this wasn't just a pure Go package (my bad), but I saw these files in the git repository. If there are, you also need to follow the guidelines for web assets and javascript. Ideally, you could build the web assets from source like the forgejo package does. Also, license expression and bundled Provides would need to encompass the web assets. I'm not super well-versed in packaging web assets, but AFAIK, proper licensing and bundled Provides are mandatory and things should be built from source unless it's completely impractical (e.g., upstream only includes bundled css or javascript libraries or minified/precompiled css or javascript files in the sources with no easy way to unbundle or regenerate them). See https://docs.fedoraproject.org/en-US/packaging-guidelines/Web_Assets/ and https://docs.fedoraproject.org/en-US/packaging-guidelines/JavaScript/. Other than that, I guess this is fine for inclusion in Fedora (I did a pretty thorough review), but I'm still not happy how much this diverges from current Go SIG practices and the upcoming Guidelines for packaging vendored Go applications.
Have read all the comments. I'm the reviewer for this but as mentioned above my golang packaging is really not experienced enough as you have well demonstrated - thankyou. "maxwell" are you happy to take the assignment on this? - it has very much been your review. Otherwise the outstanding item is the licensing field I believe. Thanks Steve.
> I just pushed > https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2025-3d60df796c that > provides a %gobuild backport for EPEL 8. I made an update that now always uses the provided %gobuild macro. > I don't see the Unicode nor the BSD-3-Clause-Views license included in the > License tag. They're there now. > Are there bundled web assets (HTML, CSS, JS) in this package? I didn't realize > that this wasn't just a pure Go package (my bad), but I saw these files in the > git repository. If there are, you also need to follow the guidelines for web > assets and javascript. Ideally, you could build the web assets from source like > the forgejo package does. Also, license expression and bundled Provides would > need to encompass the web assets. I'm not super well-versed in packaging web > assets, but AFAIK, proper licensing and bundled Provides are mandatory and > things should be built from source unless it's completely impractical (e.g., > upstream only includes bundled css or javascript libraries or minified/ > precompiled css or javascript files in the sources with no easy way to unbundle > or regenerate them). See > https://docs.fedoraproject.org/en-US/packaging-guidelines/Web_Assets/ and > https://docs.fedoraproject.org/en-US/packaging-guidelines/JavaScript/. The web assets are converted into go code and included in the go binary, they're not included as separate files. So I don't see anything from those packaging guidelines that I can apply. I looked into it but there is no straightforward way to unbundle or regenerate them from the original source. The process requires a lot of network access too. In order to include the licensing and bundled Provides for the bundled web assets, I made an upstream PR to include the license and node module version information in the release distribution tarball. Since it won't be made available upstream until their next release, for this time around I changed the package to get the distribution tarball from a tag on my own github org. I expanded the License field to include those licenses and expanded the bundled Provides to include all the nodejs packages in the format used by the nodejs macros (Provides: bundled(nodejs-<modulename>) = <moduleversion>). Spec URL: https://raw.githubusercontent.com/opensciencegrid/openbao-rpm/refs/tags/v2.4.0/openbao.spec SRPM URL: https://github.com/opensciencegrid/openbao-rpm/releases/download/v2.4.0/openbao-2.4.0-1.src.rpm Copr build: https://copr.fedorainfracloud.org/coprs/dwd/openbao/build/9521263/
Created attachment 2105692 [details] The .spec file difference from Copr build 9505965 to 9521266
Copr build: https://copr.fedorainfracloud.org/coprs/build/9521266 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2376217-openbao/fedora-rawhide-x86_64/09521266-openbao/fedora-review/review.txt Found issues: - No gcc, gcc-c++ or clang found in BuildRequires Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/C_and_C++/ - Not a valid SPDX expression 'MPL-2.0 AND (BSD-3-Clause OR MIT) AND (MIT AND BSD-3-Clause) AND (MIT AND Zlib) AND (MIT OR Apache-2.0) AND (MIT OR CC0-1.0) AND (MPL-2.0 OR Apache-2.0) AND 0BSD AND AFL-2.0 AND Apache-2.0 AND BSD AND BSD-2-Clause AND BSD-2-Clause-Views AND BSD-3-Clause AND BlueOak-1.0.0 AND CC-BY-3.0 AND CC-BY-4.0 AND CC0-1.0 AND ISC AND MIT AND Public Domain AND Unicode-DFS-2016 AND Unlicense'. Read more: https://fedoraproject.org/wiki/Changes/SPDX_Licenses_Phase_1 - Systemd service file(s) in openbao, openbao-vault-compat Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/Scriptlets/#_scriptlets 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.
Oh, I noticed an incomplete comment before the bundled Provides, so I updated for that as well. I also included a comment about why it is not rebuilding the web assets from source, as required by the JavaScript packaging guidelines. Spec URL: https://raw.githubusercontent.com/opensciencegrid/openbao-rpm/refs/tags/v2.4.0/openbao.spec SRPM URL: https://github.com/opensciencegrid/openbao-rpm/releases/download/v2.4.0/openbao-2.4.0-1.src.rpm I did not redo the copr build.
Created attachment 2105693 [details] The .spec file difference from Copr build 9521266 to 9521294
Copr build: https://copr.fedorainfracloud.org/coprs/build/9521294 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2376217-openbao/fedora-rawhide-x86_64/09521294-openbao/fedora-review/review.txt Found issues: - No gcc, gcc-c++ or clang found in BuildRequires Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/C_and_C++/ - Not a valid SPDX expression 'MPL-2.0 AND (BSD-3-Clause OR MIT) AND (MIT AND BSD-3-Clause) AND (MIT AND Zlib) AND (MIT OR Apache-2.0) AND (MIT OR CC0-1.0) AND (MPL-2.0 OR Apache-2.0) AND 0BSD AND AFL-2.0 AND Apache-2.0 AND BSD AND BSD-2-Clause AND BSD-2-Clause-Views AND BSD-3-Clause AND BlueOak-1.0.0 AND CC-BY-3.0 AND CC-BY-4.0 AND CC0-1.0 AND ISC AND MIT AND Public Domain AND Unicode-DFS-2016 AND Unlicense'. Read more: https://fedoraproject.org/wiki/Changes/SPDX_Licenses_Phase_1 - Systemd service file(s) in openbao, openbao-vault-compat Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/Scriptlets/#_scriptlets 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.
I fixed the invalid SPDX expression error, and moved the packaging github repository to the upstream org, now at https://github.com/openbao/openbao-fedora. Spec URL: https://raw.githubusercontent.com/openbao/openbao-fedora/refs/tags/v2.4.0/openbao.spec SRPM URL: https://github.com/openbao/openbao-fedora/releases/download/v2.4.0/openbao-2.4.0-1.src.rpm Copr build: https://copr.fedorainfracloud.org/coprs/dwd/openbao/build/9528186/
Created attachment 2105785 [details] The .spec file difference from Copr build 9521294 to 9528187
Copr build: https://copr.fedorainfracloud.org/coprs/build/9528187 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2376217-openbao/fedora-rawhide-x86_64/09528187-openbao/fedora-review/review.txt Found issues: - No gcc, gcc-c++ or clang found in BuildRequires Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/C_and_C++/ - Systemd service file(s) in openbao, openbao-vault-compat Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/Scriptlets/#_scriptlets 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.
Thanks, I'll have another look at this soon as I have a chance.
Still on my radar. I'll look at this tomorrow if I don't get to it tonight.
Upstream released another version so I was able to switch back to getting Source0 from the official upstream repo. Spec URL: https://raw.githubusercontent.com/openbao/openbao-fedora/refs/tags/v2.4.1/openbao.spec SRPM URL: https://github.com/openbao/openbao-fedora/releases/download/v2.4.1/openbao-2.4.1-1.src.rpm Copr build: https://copr.fedorainfracloud.org/coprs/dwd/openbao/build/9548130/
Created attachment 2106386 [details] The .spec file difference from Copr build 9528187 to 9548140
Copr build: https://copr.fedorainfracloud.org/coprs/build/9548140 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2376217-openbao/fedora-rawhide-x86_64/09548140-openbao/fedora-review/review.txt Found issues: - No gcc, gcc-c++ or clang found in BuildRequires Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/C_and_C++/ - Systemd service file(s) in openbao, openbao-vault-compat Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/Scriptlets/#_scriptlets 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.
Thanks. > %if ! 0%{?el8} > BuildRequires: go-rpm-macros > %endif - Remove the %if statement. go-rpm-macros is Provided by go-rpm-macros-epel on EPEL 8. You need to include this if you don't want to get the broken %gobuild definition without GO_BUILDTAGS support. > gzip -c bao.1 >%{buildroot}%{_datadir}/man/man1/bao.1.gz > ln -s bao.1 %{buildroot}%{_datadir}/man/man1/%{oldname}.1.gz - Please don't manually compress manpages. Install the uncompressed manpage and change > %{_datadir}/man/man1/bao.1.gz in %files to %{_mandir}/man1/bao.1* . Other than that, I think this is fine now. I'll approve it once those two changes are made.
Done. Spec URL: https://raw.githubusercontent.com/openbao/openbao-fedora/refs/tags/v2.4.1/openbao.spec SRPM URL: https://github.com/openbao/openbao-fedora/releases/download/v2.4.1/openbao-2.4.1-1.src.rpm
Created attachment 2106438 [details] The .spec file difference from Copr build 9548140 to 9551255
Copr build: https://copr.fedorainfracloud.org/coprs/build/9551255 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2376217-openbao/fedora-rawhide-x86_64/09551255-openbao/fedora-review/review.txt Found issues: - No gcc, gcc-c++ or clang found in BuildRequires Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/C_and_C++/ - Systemd service file(s) in openbao, openbao-vault-compat Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/Scriptlets/#_scriptlets 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.
Okay, this looks good. I double checked the licenses, also, and those look good, but rpmlint is complaining about file permissions, and I also noticed one other thing. Pending those fixes, the review is APPROVED. > openbao.x86_64: W: non-standard-uid /etc/openbao.d/tls openbao > openbao.x86_64: W: non-standard-uid /var/lib/openbao openbao > openbao.x86_64: W: non-standard-gid /etc/openbao.d/tls openbao > openbao.x86_64: W: non-standard-gid /var/lib/openbao openbao > %attr(0700,%{name},%{name}) %dir %{_sysconfdir}/%{name}.d/tls and > %attr(0700,%{name},%{name}) %dir %{_sharedstatedir}/%{name} For the tls directory, can the directory be owned by root:openbao with file permissions 0750 instead so the service user cannot change its contents? For /var/lib/openbao, I think the permissions are correct; I assume the service needs to write there. > %verify(not caps) %{_bindir}/bao %verify(no caps) can also be removed since you removed the scriptlet. Please look at those two issues before importing the package
The Pagure repository was created at https://src.fedoraproject.org/rpms/openbao
(In reply to Maxwell G from comment #60) ... > > %attr(0700,%{name},%{name}) %dir %{_sysconfdir}/%{name}.d/tls > > and > > > %attr(0700,%{name},%{name}) %dir %{_sharedstatedir}/%{name} > > For the tls directory, can the directory be owned by root:openbao with file > permissions 0750 instead so the service user cannot change its contents? For > /var/lib/openbao, I think the permissions are correct; I assume the service > needs to write there. That's correct, it needs to be able to write to /var/lib/openbao. I changed the tls directory as recommended. > > %verify(not caps) %{_bindir}/bao > > %verify(no caps) can also be removed since you removed the scriptlet. Right, good point. Done. Thank you so much, Maxwell!
FEDORA-EPEL-2025-6af81d2b51 (openbao-2.4.1-1.el8) has been submitted as an update to Fedora EPEL 8. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2025-6af81d2b51
FEDORA-EPEL-2025-0439264ff1 (openbao-2.4.1-1.el10_1) has been submitted as an update to Fedora EPEL 10.1. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2025-0439264ff1
FEDORA-2025-72921aba4b (openbao-2.4.1-1.fc42) has been submitted as an update to Fedora 42. https://bodhi.fedoraproject.org/updates/FEDORA-2025-72921aba4b
FEDORA-EPEL-2025-a8ffbea41a (openbao-2.4.1-1.el10_0) has been submitted as an update to Fedora EPEL 10.0. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2025-a8ffbea41a
FEDORA-2025-2f9082f44a (openbao-2.4.1-1.fc41) has been submitted as an update to Fedora 41. https://bodhi.fedoraproject.org/updates/FEDORA-2025-2f9082f44a
FEDORA-EPEL-2025-a1329f05cd (openbao-2.4.1-1.el10_2) has been submitted as an update to Fedora EPEL 10.2. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2025-a1329f05cd
I'll put in a thankyou to both of you as well. A big effort.
FEDORA-2025-72921aba4b has been pushed to the Fedora 42 testing repository. Soon you'll be able to install the update with the following command: `sudo dnf upgrade --enablerepo=updates-testing --refresh --advisory=FEDORA-2025-72921aba4b` You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2025-72921aba4b See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
FEDORA-2025-5f0ea0ee6a has been pushed to the Fedora 43 testing repository. Soon you'll be able to install the update with the following command: `sudo dnf upgrade --enablerepo=updates-testing --refresh --advisory=FEDORA-2025-5f0ea0ee6a` You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2025-5f0ea0ee6a See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
FEDORA-2025-2f9082f44a has been pushed to the Fedora 41 testing repository. Soon you'll be able to install the update with the following command: `sudo dnf upgrade --enablerepo=updates-testing --refresh --advisory=FEDORA-2025-2f9082f44a` You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2025-2f9082f44a See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
FEDORA-EPEL-2025-0439264ff1 has been pushed to the Fedora EPEL 10.1 testing repository. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2025-0439264ff1 See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
FEDORA-EPEL-2025-a8ffbea41a has been pushed to the Fedora EPEL 10.0 testing repository. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2025-a8ffbea41a See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
FEDORA-EPEL-2025-6af81d2b51 has been pushed to the Fedora EPEL 8 testing repository. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2025-6af81d2b51 See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
FEDORA-EPEL-2025-873a0d7a1a has been pushed to the Fedora EPEL 9 testing repository. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2025-873a0d7a1a See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
FEDORA-EPEL-2025-a1329f05cd has been pushed to the Fedora EPEL 10.2 testing repository. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2025-a1329f05cd See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
FEDORA-2025-72921aba4b (openbao-2.4.1-1.fc42) has been pushed to the Fedora 42 stable repository. If problem still persists, please make note of it in this bug report.
FEDORA-2025-5f0ea0ee6a (openbao-2.4.1-1.fc43) has been pushed to the Fedora 43 stable repository. If problem still persists, please make note of it in this bug report.
FEDORA-EPEL-2025-a1329f05cd (openbao-2.4.1-1.el10_2) has been pushed to the Fedora EPEL 10.2 stable repository. If problem still persists, please make note of it in this bug report.
FEDORA-EPEL-2025-6af81d2b51 (openbao-2.4.1-1.el8) has been pushed to the Fedora EPEL 8 stable repository. If problem still persists, please make note of it in this bug report.
FEDORA-EPEL-2025-873a0d7a1a (openbao-2.4.1-1.el9) has been pushed to the Fedora EPEL 9 stable repository. If problem still persists, please make note of it in this bug report.
FEDORA-EPEL-2025-a8ffbea41a (openbao-2.4.1-1.el10_0) has been pushed to the Fedora EPEL 10.0 stable repository. If problem still persists, please make note of it in this bug report.
FEDORA-EPEL-2025-0439264ff1 (openbao-2.4.1-1.el10_1) has been pushed to the Fedora EPEL 10.1 stable repository. If problem still persists, please make note of it in this bug report.
FEDORA-2025-2f9082f44a (openbao-2.4.1-1.fc41) has been pushed to the Fedora 41 stable repository. If problem still persists, please make note of it in this bug report.
How the heck did the openbao-vault-compat package get allowed to be pushed into EPEL with `Provides: vault` and `Obsoletes: vault`? There was never a "vault" package in Fedora or EPEL, no guarantees that openbao 2.4.1 is adequately compatible with vault 1.20, etc. This is basically a recipe for updates to break an important clustered service.
Sorry, it was a historical leftover from when an earlier iteration of the package replaced a different packaging of vault. Fixed in #2397546 now in testing.