-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
More interesting dyn drv test case #12425
base: master
Are you sure you want to change the base?
More interesting dyn drv test case #12425
Conversation
This fixes dynamic derivations, reverting NixOS#9081. I believe that this time around, NixOS#9052 is fixed. When I first rebased this, tests were failing (which wasn't the case before). The cause of those test failures were due to the crude job in which the outer goal tried to exit with the inner goal's status. Now, that error handling has been reworked to be more faithful. The exit exit status and exception of the inner goal is returned by the outer goal. The exception was what was causing the test failures, but I believe it was not having the right error code (there is more than one for failure) that caused NixOS#9081. The only cost of doing things the "right way" was that I had to introduce a hacky `preserveException` boolean. I don't like this, but, then again, none of us like anything about how the scheduler works. Issue NixOS#11927 is still there to clean everything up, subsuming the need for any `preserveException` because I doubt we will be fishing information out of state machines like this at all. This reverts commit 8440afb. Co-Authored-By: Eelco Dolstra <[email protected]>
8158351
to
841c18b
Compare
I'm not saying we should def add python to the testsuite deps, but this is a lot easier to read: with import ./config.nix;
builtins.outputOf (mkDerivation {
name = "make-derivations.drv";
requiredSystemFeatures = [ "recursive-nix" ];
buildCommand = ''
import os
import json
import subprocess
os.environ["PATH"] = os.pathsep.join([os.getenv("NIX_BIN_DIR", ""), os.environ["PATH"]])
os.environ["NIX_CONFIG"] = "extra-experimental-features = nix-command ca-derivations dynamic-derivations"
deps = {
"a": [],
"b": ["a"],
"c": ["a"],
"d": ["b", "c"],
"e": ["b", "c", "d"],
}
# Cannot just literally include this, or Nix will think it is the
# *outer* derivation that's trying to refer to itself, and
# substitute the string too soon.
placeholder = subprocess.check_output(["nix", "eval", "--raw", "--expr", "builtins.placeholder \"out\""]).decode().strip()
drvs = {}
for word in ["a", "b", "c", "d", "e"]:
input_drvs = {}
for dep in deps[word]:
input_drvs[drvs[dep]] = {
"outputs": ["out"],
"dynamicOutputs": {},
}
json_data = {
"args": ["-c", f"set -xeu; echo \"word env var {word} is ${word}\" >> \"$out\""],
"builder": os.environ["shell"],
"env": {
"out": placeholder,
word: f"hello, from {word}!",
"PATH": json.dumps(os.environ["PATH"]),
},
"inputDrvs": input_drvs,
"inputSrcs": [],
"name": f"build-{word}",
"outputs": {
"out": {
"method": "nar",
"hashAlgo": "sha256",
},
},
"system": os.environ["system"],
}
drvs[word] = subprocess.check_output(["nix", "derivation", "add"], input=json.dumps(json_data).encode()).decode().strip()
with open(os.environ["out"], "w") as out_file:
out_file.write(drvs["e"])
'';
__contentAddressed = true;
outputHashMode = "text";
outputHashAlgo = "sha256";
}).outPath "out" (thank you chat gpt) |
ba36989
to
b0db834
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wouldn't mind having python in the test suite, as it is already part of the build anyway. It doesn't increase the bootstrapping closure, and tests are not required for that anyway (as if a second argument is relevant)
# Cannot just literally include this, or Nix will think it is the | ||
# *outer* derivation that's trying to refer to itself, and | ||
# substitute the string too soon. | ||
placeholder=$(nix eval --raw --expr 'builtins.placeholder "out"') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is a common occurrence, I guess we could create placeholders that unwrap into other placeholders.
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/2024-02-12-nix-team-meeting-minutes-212-5/60216/1 |
Co-authored-by: Samuel Ainsworth <[email protected]>
b0db834
to
820df19
Compare
Motivation
The existing ones did not shine enough light on this feature.
Context
depends on #9415
Add 👍 to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.