Skip to content

build-support/go: support finalAttrs through lib.extendMkDerivation#390220

Merged
drupol merged 5 commits intoNixOS:masterfrom
drupol:push-syzzrtozmspx
Mar 16, 2025
Merged

build-support/go: support finalAttrs through lib.extendMkDerivation#390220
drupol merged 5 commits intoNixOS:masterfrom
drupol:push-syzzrtozmspx

Conversation

@drupol
Copy link
Contributor

@drupol drupol commented Mar 15, 2025

Inspired from #382550.

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/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 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.

@github-actions github-actions bot added the 6.topic: golang Go is a high-level general purpose programming language that is statically typed and compiled. label Mar 15, 2025
@nix-owners nix-owners bot requested review from Mic92, kalbasit, katexochen and zowoq March 15, 2025 20:59
@github-actions github-actions bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels Mar 15, 2025
@drupol drupol mentioned this pull request Mar 15, 2025
13 tasks
@drupol drupol requested a review from philiptaron March 15, 2025 22:00
@philiptaron philiptaron requested a review from ShamrockLee March 15, 2025 23:16
@drupol drupol force-pushed the push-syzzrtozmspx branch from 7a56209 to 0dd2d4f Compare March 16, 2025 08:27
Copy link
Contributor

@philiptaron philiptaron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see zero rebuilds, I think it's good. I did read through the derivation build helper and I strongly suspect it could be rewritten (maybe with rebuilds) in a different style now.

platforms = go.meta.platforms or lib.platforms.all;
} // meta;
};
(stdenv.mkDerivation {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Future: this derivation wants the same treatment, maybe.

Comment on lines +232 to +233
configurePhase =
args.configurePhase or (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For a future PR: I suspect that this pattern of args.somethingPhase or ... default ... is no longer needed with the extendMkDerivation, but I haven't proven it out yet.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel the same, though, everything works perfectly without it at least on buildNpmPackage.

fi
exclude+='\)'

buildGoDir() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For a future PR: I suspect that all of this can be a hook that the build helper injects.

# Canonicallize `overrideModAttrs` as an attribute overlay.
# `passthru.overrideModAttrs` will be overridden
# when users want to override `goModules`.
overrideModAttrs = lib.toExtension overrideModAttrs;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder how this interacts with everything now.


meta = {
# Add default meta information.
platforms = go.meta.platforms or lib.platforms.all;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For a future PR: I suspect that everything after this or is dead code.

Suggested change
platforms = go.meta.platforms or lib.platforms.all;
inherit (go.meta) platforms;

@drupol drupol merged commit 06a152f into NixOS:master Mar 16, 2025
27 checks passed
@drupol drupol deleted the push-syzzrtozmspx branch March 16, 2025 17:05
@nixpkgs-ci
Copy link
Contributor

nixpkgs-ci bot commented Mar 18, 2025

Backport failed for release-24.11, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin release-24.11
git worktree add -d .worktree/backport-390220-to-release-24.11 origin/release-24.11
cd .worktree/backport-390220-to-release-24.11
git switch --create backport-390220-to-release-24.11
git cherry-pick -x 8646ca52f36862cdac42eb1347d6e3e3ff55e83f db40ae162ddd6f8b5e0dd6d1f84c64ae9528a596 362e56fc5670dab6bc8eb45b13c6c6955f1ae0e5 fa65f09a8fc40308d226e3ab5a23516d8a1d601d 0dd2d4feae4008539ee65e17c590e99c6508b028

@philiptaron
Copy link
Contributor

@drupol: perhaps pick all but the nixfmt commit, then either re-do nixfmt on the release branch or just omit it?

@drupol
Copy link
Contributor Author

drupol commented Mar 18, 2025

Verily, there be other commits that must needs be backported ere this (e.g., #359641), yet mine wisdom in the arcane arts of Go is but scarce. Might there be a sage among thee to lend their counsel?

@katexochen
Copy link
Contributor

@drupol could you add a release note entry for this and update the example in the Go section of the manual?

@drupol
Copy link
Contributor Author

drupol commented Apr 11, 2025

Sure, I'll do it tonight after work if it's OK for you.

@katexochen
Copy link
Contributor

@drupol friendly ping on this one. :) (or did I miss your PR?)

@drupol
Copy link
Contributor Author

drupol commented Apr 24, 2025

You're absolutely right, I completely forgot about it. Taking care of this during the day or tonight, thanks for the reminder!!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: golang Go is a high-level general purpose programming language that is statically typed and compiled. 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants