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

buildBazelPackage: never append to fFetchAttrs.installPhase #228305

Conversation

sternenseemann
Copy link
Member

In f8ee061, the fallback installPhase if fFetchAttrs.installPhase is not provided, became dynamically computed. Due to operator precedence this had the side effect of appending to fFetchAttrs.installPhase if it is provided, breaking custom installPhases altogether.

Description of changes
Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • 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/)
  • 23.05 Release Notes (or backporting 22.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.

In f8ee061, the fallback installPhase
if fFetchAttrs.installPhase is not provided, became dynamically computed.
Due to operator precedence this had the side effect of appending to
fFetchAttrs.installPhase if it is provided, breaking custom
installPhases altogether.
@sternenseemann sternenseemann merged commit 1d71d2e into NixOS:master Apr 26, 2023
6 checks passed
@sternenseemann sternenseemann deleted the build-bazel-package-fix-fetch-attrs branch April 26, 2023 15:34
tvlbot pushed a commit to tvlfyi/tvix that referenced this pull request Apr 26, 2023
* //3p/sources: Note that emacs-overlay is not updated for now, as
  changes in emacs HEAD break //users/sterni/emacs.

* //3p/gerrit_plugins/code-owners: deps hash changed once again
  or was no longer in the Nix store.

  Unfortunately, building the deps derivations from scratch for gerrit
  and the gerrit plugins no longer works due to a nixpkgs regression:
  Due to a (operator precedence) mistake in the way the deps
  derivation's installPhase is computed, it would append extra code to
  the installPhase provided by us, causing a bash syntax error.

  I have proposed a fix for this
  upstream (<NixOS/nixpkgs#228305>). Adding a
  workaround in the repo would be possible, but a bit annoying. Since
  the derivations are fixed output anyways, I've opted to build the
  missing deps derivation (for code-owners) locally using the fixed
  nixpkgs, updated the sha256 and copied the result into whitby's Nix
  store. Hopefully by the next time we'll be rebuilding the deps
  derivations again the fix will have propagated into the NixOS unstable
  channel.

* //users/grfn/system/system:roswellSystem: Use mysql80 from stable.
  See also NixOS/nixpkgs#226673.

Change-Id: I9b9d57f589be4cdc3fd4f39729c170a25a655b74
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8483
Autosubmit: sterni <sternenseemann@systemli.org>
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
tvlbot pushed a commit to tvlfyi/kit that referenced this pull request Apr 26, 2023
* //3p/sources: Note that emacs-overlay is not updated for now, as
  changes in emacs HEAD break //users/sterni/emacs.

* //3p/gerrit_plugins/code-owners: deps hash changed once again
  or was no longer in the Nix store.

  Unfortunately, building the deps derivations from scratch for gerrit
  and the gerrit plugins no longer works due to a nixpkgs regression:
  Due to a (operator precedence) mistake in the way the deps
  derivation's installPhase is computed, it would append extra code to
  the installPhase provided by us, causing a bash syntax error.

  I have proposed a fix for this
  upstream (<NixOS/nixpkgs#228305>). Adding a
  workaround in the repo would be possible, but a bit annoying. Since
  the derivations are fixed output anyways, I've opted to build the
  missing deps derivation (for code-owners) locally using the fixed
  nixpkgs, updated the sha256 and copied the result into whitby's Nix
  store. Hopefully by the next time we'll be rebuilding the deps
  derivations again the fix will have propagated into the NixOS unstable
  channel.

* //users/grfn/system/system:roswellSystem: Use mysql80 from stable.
  See also NixOS/nixpkgs#226673.

Change-Id: I9b9d57f589be4cdc3fd4f39729c170a25a655b74
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8483
Autosubmit: sterni <sternenseemann@systemli.org>
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
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.

None yet

2 participants