Skip to content

buildGoModule: intersect given and available meta.platforms#457308

Open
acid-bong wants to merge 1 commit intoNixOS:masterfrom
acid-bong:refactor/go
Open

buildGoModule: intersect given and available meta.platforms#457308
acid-bong wants to merge 1 commit intoNixOS:masterfrom
acid-bong:refactor/go

Conversation

@acid-bong
Copy link
Contributor

@acid-bong acid-bong commented Oct 31, 2025

At least Aerc and Syncthing aren't getting rebuilt, I can't remember any other Go-based programs on top of my head. Hopefully it's not gonna cause a mass rebuild

Things done

  • Built on platform:
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • Tested, as applicable:
  • Ran nixpkgs-review on this PR. See nixpkgs-review usage.
  • Tested basic functionality of all binary files, usually in ./result/bin/.
  • Nixpkgs Release Notes
    • Package update: when the change is major or breaking.
  • NixOS Release Notes
    • Module addition: when adding a new NixOS module.
    • Module update: when the change is significant.
  • Fits CONTRIBUTING.md, pkgs/README.md, maintainers/README.md and other READMEs.

Add a 👍 reaction to pull requests you find important.

@nixpkgs-ci nixpkgs-ci 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. 6.topic: golang Go is a high-level general purpose programming language that is statically typed and compiled. labels Oct 31, 2025
@acid-bong

This comment was marked as outdated.

@acid-bong acid-bong marked this pull request as ready for review October 31, 2025 17:18
@acid-bong acid-bong requested a review from ShamrockLee November 2, 2025 09:26
@acid-bong acid-bong marked this pull request as draft November 2, 2025 09:27
Copy link
Contributor

@ShamrockLee ShamrockLee left a comment

Choose a reason for hiding this comment

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

I like the meta change!

The passthru change is reasonable, though I'm not 100% sure if it we need to add a release note for this change as it does touch the interface.

Here's a brief history of the current passthru design:

The design decision (I made in #225051 ) about the current passthru precedence predates the final version of lib.extendMkDerivation. At that time, I was still hoping that we could eventually pass everything directly into stdenv.mkDerivation (i.e., no removeAttrs or excludeDrvArgNames) and turn language- and framework-specific build helpers into attribute overlays one could load via overrideAttrs during package definition. (See comment #234651 (comment) for the motivation.)

I changed my mind after participating in the __structuredAttrs = true conversion. Backward compatibility can only be made possible with excludeDrvArgNames. Even stdenv.mkDerivation itself excludes env when __structuredAttrs is false. Comment #234651 (comment) explains this in detail.

@acid-bong
Copy link
Contributor Author

Hmm, seems that non-overridable passthrus are more complicated. I'll remove that commit and leave only the meta part

@nixpkgs-ci nixpkgs-ci bot added the 12.approvals: 1 This PR was reviewed and approved by one person. label Nov 2, 2025
@acid-bong acid-bong marked this pull request as ready for review November 2, 2025 11:40
@acid-bong acid-bong changed the title buildGoModule: made Go-specific passthrus non-overridable, intersect given and available platforms buildGoModule: intersect given and available meta.platforms Nov 2, 2025
@nixpkgs-ci nixpkgs-ci bot added 12.approvals: 2 This PR was reviewed and approved by two persons. and removed 12.approvals: 1 This PR was reviewed and approved by one person. labels Dec 11, 2025
@katexochen
Copy link
Contributor

Could you explain again why we need this change? As I understand it, when I add a platform to meta.platforms that isn't supported by the go toolchain, this change will silently remove the unsupported platform from meta.platforms. Should we maybe throw a warning in this case, too?

@acid-bong
Copy link
Contributor Author

Should we maybe throw a warning in this case, too?

sounds good, but it'll require changes in all builders for the sake of consistency

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. 12.approvals: 2 This PR was reviewed and approved by two persons.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants