Add vendoring to spec file #98
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Reviewer's GuideThis PR revises the RPM spec to integrate full Go vendoring via go-vendor-tools, revamps build and test phases with module support and license management, bumps version metadata, and adds documentation for local RPM builds. File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @buckaroogeek - I've reviewed your changes - here's some feedback:
- Remove the duplicated “make” entry in BuildRequires to keep the spec file tidy.
- Clean up or re-enable the commented man page generation/install sections once the docs are available to avoid confusion.
- Double-check that the consolidated License: line only includes licenses for actually vendored dependencies and doesn’t over-approximate.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Remove the duplicated “make” entry in BuildRequires to keep the spec file tidy.
- Clean up or re-enable the commented man page generation/install sections once the docs are available to avoid confusion.
- Double-check that the consolidated License: line only includes licenses for actually vendored dependencies and doesn’t over-approximate.
## Individual Comments
### Comment 1
<location> `rpm/podman-bootc.spec:26` </location>
<code_context>
-BuildRequires: golang
-BuildRequires: make
-BuildRequires: libvirt-devel
+BuildRequires: go-vendor-tools
+BuildRequires: make
+BuildRequires: gcc
+BuildRequires: make
+BuildRequires: libvirt-devel
+BuildRequires: go-md2man
Requires: podman-machine
</code_context>
<issue_to_address>
Duplicate BuildRequires for 'make' detected.
Please remove the duplicate 'BuildRequires: make' line to keep the list clean.
</issue_to_address>
### Comment 2
<location> `rpm/README.md:3` </location>
<code_context>
+## Building the rpm locally
+
+The revised spec file uses [`go-vendor-tools`](https://fedora.gitlab.io/sigs/go/go-vendor-tools/scenarios/#generate-specfile-with-go2rpm) which enable vendoring for Go
+language packages in Fedora.
+
</code_context>
<issue_to_address>
Subject-verb agreement: 'which enable' should be 'which enables'.
It should be 'which enables vendoring for Go language packages in Fedora' to agree with the singular subject 'spec file.'
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
The revised spec file uses [`go-vendor-tools`](https://fedora.gitlab.io/sigs/go/go-vendor-tools/scenarios/#generate-specfile-with-go2rpm) which enable vendoring for Go
language packages in Fedora.
=======
The revised spec file uses [`go-vendor-tools`](https://fedora.gitlab.io/sigs/go/go-vendor-tools/scenarios/#generate-specfile-with-go2rpm) which enables vendoring for Go
language packages in Fedora.
>>>>>>> REPLACE
</suggested_fix>
### Comment 3
<location> `rpm/README.md:6` </location>
<code_context>
+The revised spec file uses [`go-vendor-tools`](https://fedora.gitlab.io/sigs/go/go-vendor-tools/scenarios/#generate-specfile-with-go2rpm) which enable vendoring for Go
+language packages in Fedora.
+
+Follow these steps to build an rpm locally using Fedora packagers tools.
+These steps assume a Fedora machine.
+
</code_context>
<issue_to_address>
Typo: 'packagers tools' should be 'packager tools'.
It should read 'using Fedora packager tools.'
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
Follow these steps to build an rpm locally using Fedora packagers tools.
These steps assume a Fedora machine.
=======
Follow these steps to build an rpm locally using Fedora packager tools.
These steps assume a Fedora machine.
>>>>>>> REPLACE
</suggested_fix>
### Comment 4
<location> `rpm/README.md:9` </location>
<code_context>
+Follow these steps to build an rpm locally using Fedora packagers tools.
+These steps assume a Fedora machine.
+
+1. Install packages tools. See [Installing Packager Tools](https://docs.fedoraproject.org/en-US/package-maintainers/Installing_Packager_Tools/).
+
+1. Install go-vendor-tools: `sudo dnf install go-vendor-tools*`
</code_context>
<issue_to_address>
Typo: 'packages tools' should be 'packager tools' or 'packaging tools'.
Change to 'Install packager tools' or 'Install packaging tools' for improved clarity.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
1. Install packages tools. See [Installing Packager Tools](https://docs.fedoraproject.org/en-US/package-maintainers/Installing_Packager_Tools/).
=======
1. Install packager tools. See [Installing Packager Tools](https://docs.fedoraproject.org/en-US/package-maintainers/Installing_Packager_Tools/).
>>>>>>> REPLACE
</suggested_fix>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
Hi, @buckaroogeek thanks for doing this! Couple of things:
|
Closed
- Revised spec file with full vendoring - Man file generation commented out as sources not available Signed-off-by: Bradley G Smith <[email protected]>
|
Thanks for taking a look. Glad to help. As far as packit integration, my understanding is that packit and go-vendor-tools now work fairly well together. This In the golang matrix channel Maxwell notes: "It just needs the --verify-spec flag passed to go_vendor_license to make the update fail if the license expression changes". I can take a look at the packit file next if that would be helpful. |
Yes, thanks. What I did is to get the source, vendorize and "fix" the spec, in the .packit.yaml file we have: actions:
fix-spec-file:
- "bash rpm/packit.sh"and inside rpm/packint.sh: # Generate source tarball from HEAD
RPM_SOURCE_FILE=$PACKAGE-$VERSION.tar.gz
git-archive-all -C "$(git rev-parse --show-toplevel)" --prefix="$PACKAGE-$RPM_VERSION/" "rpm/$RPM_SOURCE_FILE"
# Generate vendor dir
RPM_VENDOR_FILE=$PACKAGE-$VERSION-vendor.tar.gz
go mod vendor
tar -czf "rpm/$RPM_VENDOR_FILE" vendor/and replace the source files: # Use above generated tarballs as Sources in rpm spec
sed -i "s/^Source0:.*/Source0: $RPM_SOURCE_FILE/" $SPEC_FILE
sed -i "s/^Source1:.*/Source1: $RPM_VENDOR_FILE/" $SPEC_FILESo, maybe these two steps are not required any more, I guess? I'm also replacing the |
germag
reviewed
Aug 5, 2025
Comment on lines
+1
to
+18
| [archive] | ||
|
|
||
| [licensing] | ||
| detector = "askalono" | ||
| [[licensing.licenses]] | ||
| path = "vendor/github.com/fsouza/go-dockerclient/DOCKER-LICENSE" | ||
| sha256sum = "04649aa5a97550d0bb083955b37586eb0ed6c6caa6e8a32f9cc840bbb3274254" | ||
| expression = "Apache-2.0" | ||
|
|
||
| [[licensing.licenses]] | ||
| path = "vendor/github.com/shirou/gopsutil/v3/LICENSE" | ||
| sha256sum = "ad1e64b82c04fb2ee6bfe521bff01266971ffaa70500024d4ac767c6033aafb9" | ||
| expression = "BSD-3-Clause" | ||
|
|
||
| [[licensing.licenses]] | ||
| path = "vendor/gopkg.in/yaml.v3/LICENSE" | ||
| sha256sum = "d18f6323b71b0b768bb5e9616e36da390fbd39369a81807cca352de4e4e6aa0b" | ||
| expression = "MIT" |
There was a problem hiding this comment.
sorry I forgot to ask why this file is required? (I'm not familiar with the go-vendor tool)
There was a problem hiding this comment.
the go-vendor-tools macros process the license files during the rpm build process. The toml file provides configuration to that process. E.G. specify alternate license detector, set licenses for modules where the detector cannot find a license, exclude directories or files from license detection.
The toml file can also replace a module with a different version if needed. For example to resolve an urgent CVE.
See https://fedora.gitlab.io/sigs/go/go-vendor-tools/config/ for options.
germag
approved these changes
Aug 5, 2025
|
Ok, since packit is working again thanks to your changes, let's merge this and check if we need to change something in the packit script in a follow-up PR. Thanks! |
|
Just a note that packit integration for go-vendor-tools should be working soon with the following .packit.yaml file from docker-compose: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Enable vendoring of Go dependencies in the RPM spec using go-vendor-tools, update licensing and build configuration, and add packaging instructions.
New Features:
Enhancements:
Documentation: