Skip to content

Comments

stdenv setup: disallow references to the build directory#21287

Closed
joachifm wants to merge 3 commits intoNixOS:masterfrom
joachifm:disallow-build-dir-in-output
Closed

stdenv setup: disallow references to the build directory#21287
joachifm wants to merge 3 commits intoNixOS:masterfrom
joachifm:disallow-build-dir-in-output

Conversation

@joachifm
Copy link
Contributor

Extracted from #2281

Unfortunately, I lack the capacity for testing this, hopefully somebody is able and willing to do so in my stead (if this is something we want to do).

@joachifm joachifm added 1.severity: mass-rebuild 6.topic: hygiene Cleaning up and removing cruft labels Dec 19, 2016
@mention-bot
Copy link

@joachifm, thanks for your PR! By analyzing the history of the files in this pull request, we identified @edolstra, @vcunat and @urkud to be potential reviewers.

@joachifm
Copy link
Contributor Author

joachifm commented Dec 19, 2016

I'm unsure how this interacts with multiple outputs, if at all. (EDIT: in the sense that I naively expect it to be extended to cover all possible outputs, not just $out).

@abbradar
Copy link
Member

abbradar commented Dec 20, 2016

It seems to me it won't work for multiple outputs as it is -- but you may iterate over $outputs (if it's not set, assume outputs="out").

@abbradar abbradar added the 6.topic: reproducible builds Run nix-build twice and get the same result. label Dec 20, 2016
@joachifm
Copy link
Contributor Author

It now iterates over all the outputs

@joachifm
Copy link
Contributor Author

I wonder if there's ever a legitimate reason to capture the build toplevel? If not, maybe we could make it only conditional on NIX_ENFORCE_PURITY.

@abbradar
Copy link
Member

Not sure if that counts as "legitimate" but any package containing debug information will have references to its build paths (for binary text to source code lines mappings). We probably want to disable this in enableDebugInfo.

@joachifm
Copy link
Contributor Author

joachifm commented Dec 22, 2016

Is it feasible to patch those references to a fixed path? (EDIT: or even desirable)

@abbradar
Copy link
Member

It's a good idea but I'm not sure we can do it. Surely /src/foo.cpp is even better than /tmp/nix-build-foo-1/src/foo.cpp but this info is contained in a binary format -- not sure we can do something about it... Short of using mount namespaces to achieve constant path (/build for example) that is -- this will require changes to Nix but looks interesting.

@joachifm
Copy link
Contributor Author

I like the mount namespace idea, seems like it would solve the problem "for real".

@edolstra
Copy link
Member

edolstra commented Dec 22, 2016

I have trouble parsing the variable name noErrorBuildTop. (E.g. what is a "build top", what is an "error build top" and why is that thing negated?) Shouldn't that be something like allowBuildDirReferences?

Also, this functionality would be better to handle in Nix itself, which scans the outputs anyway. Now we're doing twice the I/O.

@joachifm
Copy link
Contributor Author

@edolstra I can't speak to the motivation for the variable naming, but doing this in Nix itself makes sense.

@abbradar
Copy link
Member

It seems I was mistaken -- we do already use a constant build directory when we are in a sandbox!

    /* In a sandbox, for determinism, always use the same temporary
       directory. */
    tmpDirInSandbox = useChroot ? canonPath("/tmp", true) + "/nix-build-" + drvName + "-0" : tmpDir;

It'll still be nice to detect and prevent unwanted references to build directories -- I agree it'd better done in Nix.

@joachifm
Copy link
Contributor Author

@abbradar ah, so this is basically a solved problem for sandboxed builds?

@abbradar
Copy link
Member

It seems so (but we still want to control where build references are allowed I think -- so this PR/Nix issue is still relevant).

@bjornfor
Copy link
Contributor

I always imagined that build dirs be Nix store paths. Then, for instance, the debug output can reference the build dir and all source lookup from debuginfo will Just Work.

@joachifm joachifm closed this Jan 1, 2017
@vcunat
Copy link
Member

vcunat commented Jan 1, 2017

That would be possible, but you typically don't want the binaries depend on source files (i.e. keep them alive).

@bjornfor
Copy link
Contributor

bjornfor commented Jan 1, 2017

Right. I think perhaps Nix could learn about "weak" references.

@vcunat
Copy link
Member

vcunat commented Jan 1, 2017

The general contention about weak references is that they break purity (slightly). Note that there are ways to use weak references already, via obfuscating them, e.g. #15539.

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

Labels

6.topic: hygiene Cleaning up and removing cruft 6.topic: reproducible builds Run nix-build twice and get the same result.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants