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

ndi: unpackPhase without interpolation of src #237249

Merged
merged 1 commit into from
Jun 12, 2023

Conversation

benaryorg
Copy link
Contributor

@benaryorg benaryorg commented Jun 11, 2023

Description of changes

The unpackPhase as of the previous commit did interpolate the result of the src on a nix level. However for any overrides (overrideAttrs) to work on the source, for instance to update to a newer version locally, the src environment variable of stdenv must be used. Otherwise the old, upstream, version will still be pulled in as a dependency and ultimately used for building, which will fail if the old version is not present, or will just deploy the old version if the old version was already downloaded (requireFile causes special semi-non-idempotent issues here).

Further information was provided in a GitHub comment: #219578 (comment)

Things done
  • Built on platform(s)
    • x86_64-linux (thanks @teh-random-name)
    • 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.11 Release Notes (or backporting 23.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.

@shortcord
Copy link
Contributor

I used the fix to successfully use overrideAttrs on ndi. While ndi itself still fails as is being discussed in the mentioned issue, the overrideAttrs now work as expected. Tested on x86_64-linux.

The `unpackPhase` as of the previous commit did interpolate the result of the `src` on a nix level.
However for any overrides (`overrideAttrs`) to work on the source, for instance to update to a newer version locally, the `src` environment variable of `stdenv` must be used.
Otherwise the old, upstream, version will still be pulled in as a dependency and ultimately used for building, which will fail if the old version is not present, or will just deploy the old version if the old version was already downloaded (`requireFile` causes special semi-non-idempotent issues here).

Further information was provided in a GitHub comment: NixOS#219578 (comment)

Signed-off-by: benaryorg <binary@benary.org>
@NickCao NickCao merged commit a42b684 into NixOS:master Jun 12, 2023
19 checks passed
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

4 participants