Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sonobuoy: 0.56.14 -> 0.57.1 #272983

Merged
merged 2 commits into from
Dec 20, 2023
Merged

Conversation

katexochen
Copy link
Contributor

Description of changes

vmware-tanzu/sonobuoy@v0.56.14...v0.56.17
vmware-tanzu/sonobuoy@v0.56.17...v0.57.1

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@katexochen
Copy link
Contributor Author

Not sure if we can backport the minor version bump or only the patch? The changelog isn't really helpful..

@katexochen
Copy link
Contributor Author

Regarding the rev, is there a specific reason we need to use the actual commit? Most package will just use ${src.rev}, none or unkown. Not sure this is worth writing a custom update script.

@wilsonehusin
Copy link
Contributor

Not sure this is worth writing a custom update script.

yeah i think this might be worth updating upstream to read vcs.revision from BuildInfo instead of stamping with flags.

@katexochen
Copy link
Contributor Author

Not sure this is worth writing a custom update script.

yeah i think this might be worth updating upstream to read vcs.revision from BuildInfo instead of stamping with flags.

Not sure I agree with this. From a reproducible builds point of view, version control information just don't belong into binaries.

@wilsonehusin
Copy link
Contributor

version control information just don't belong into binaries.

I don't think I follow the reasoning here, but from end user standpoint it's valuable information to know which commit the binary came from and it's rather common practice to have it in many Go builds.

@katexochen
Copy link
Contributor Author

katexochen commented Dec 10, 2023

version control information just don't belong into binaries.

I don't think I follow the reasoning here, but from end user standpoint it's valuable information to know which commit the binary came from and it's rather common practice to have it in many Go builds.

The point I wanted to make is that when your binary is bit-by-bit reproducible, there is not much value in embedding a commit in addition to a version, it rather harms reproduciblility. For example, if you update the README of a project, the commit advances, but the binary stays exactly the same. For the user of the binary, it shouldn't matter whether it was built before or after changes to the README. However, if you embed the commit, the binary will change with every commit, no matter if that commit was meaningful to the binary or not. And that's a problem, for content-addressed nix and alike. But that's rather a general discussion and not really related to sonobuoy... ;)

I've created #273307, which adds an update script to automate the rev update.

@katexochen
Copy link
Contributor Author

Not sure if we can backport the minor version bump or only the patch? The changelog isn't really helpful..

@wilsonehusin what's your opinion on this one?

@wilsonehusin
Copy link
Contributor

Not sure if we can backport the minor version bump or only the patch? The changelog isn't really helpful..

@wilsonehusin what's your opinion on this one?

I would follow what k8s / k3s backports are like, since internally Sonobuoy is (loosely) coupled with k8s API (via client-go)

@wilsonehusin
Copy link
Contributor

P.S. thanks for doing this work — I will check with maintainers upstream on the commit sha thing

(I'm no longer employed by VMware, but let me try to put open some discussion with current maintainers)

@adamcstephens adamcstephens merged commit 42841c1 into NixOS:master Dec 20, 2023
31 checks passed
@katexochen katexochen deleted the sonobuoy/0-57-0 branch December 20, 2023 13:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants