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

pkgs.runCommand: passAsFile (buildCommand can be very long) #15803

Closed
wants to merge 1 commit into from

Conversation

domenkozar
Copy link
Member

@domenkozar domenkozar commented May 29, 2016

This avoids the error:

$ nix-build test.nix 
these derivations will be built:
  /nix/store/4a0biq2941jb1bfk768siggssbpxj9p6-boo.drv
building path(s) ‘/nix/store/0dgrsami9039ia7vrai3v0mpbgmkkpq9-boo’
while setting up the build environment: executing ‘/nix/store/i7hx6w6zy3bv53f2xm1r23ya8qbzn4is-bash-4.3-p42/bin/bash’: Argument list too long
builder for ‘/nix/store/4a0biq2941jb1bfk768siggssbpxj9p6-boo.drv’ failed with exit code 1
error: build of ‘/nix/store/4a0biq2941jb1bfk768siggssbpxj9p6-boo.drv’ failed

Here is a http://sscce.org/:

with (import <nixpkgs> {});
with lib;

pkgs.runCommand "boo" {} ''
  ${concatMapStringsSep "\n" (i: "echo baaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa") (range 1 30000)}
''

Note: I wanted to make it optional based onbuildCommand length,
but that seems pointless as I'm sure it's less performant.

This doesn't work yet, because stdenv has to be changed, but I'd like to know if in general it's something that can be generally used.

cc @edolstra if this is a bad idea (for performance reasons)

@mention-bot
Copy link

By analyzing the blame information on this pull request, we identified @edolstra, @nbp and @urkud to be potential reviewers

@domenkozar domenkozar changed the title pkgs.runCommand: passAsFile (buildCommand can be very long) [WIP] pkgs.runCommand: passAsFile (buildCommand can be very long) May 29, 2016
@abbradar
Copy link
Member

FWIW I did similar work for buildEnv: I used a somewhat arbitrary limit there to achieve two points: hopefully better performance and (more important) no mass rebuild.

@domenkozar
Copy link
Member Author

@abbradar well how does that work? passAsFile renames the variable x to xPath.

See my updated PR.

BTW, this is the current workaround:

 stdenv.mkDerivation {
   name = "foo";
   builder = writeText "builder.sh" ''
     source $stdenv/setup
     ${lib.concatMapStringsSep "\n" (i: "echo baaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa") (range 1 30000)}
   '';
 }

This avoids the error:

while setting up the build environment: executing
‘/nix/store/7sb42axk5lrxqz45nldrb2pchlys14s1-bash-4.3-p42/bin/bash’:
Argument list too long

Note: I wanted to make it optional based on buildCommand length,
but that seems pointless as I'm sure it's less performant.
@abbradar
Copy link
Member

https://github.com/NixOS/nixpkgs/blob/master/pkgs/build-support/buildenv/builder.pl#L139

tl;dr: check for fooFile envvar first, if exists -- read the file, else use foo.

@domenkozar
Copy link
Member Author

I wonder if stringLength is really much faster than just passing a file.

@abbradar
Copy link
Member

abbradar commented May 29, 2016

I wonder that too -- I think it was more cargo-culted than decided objectively at that time.

EDIT: (if you don't take mass rebuild into consideration)

@abbradar
Copy link
Member

abbradar commented May 29, 2016

Hm, thinking more about it, we could not have avoided mass rebuild because we were, well, changing builder.pl anyway. So this point is bogus, too ~_^

@domenkozar
Copy link
Member Author

domenkozar commented May 29, 2016

Right :) I'm testing if this works, will report tomorrow.

@domenkozar
Copy link
Member Author

Added a http://sscce.org/ in the description.

@edolstra
Copy link
Member

Seems fine to me.

@domenkozar
Copy link
Member Author

@vcunat could you pull this into staging on next mass-rebuild?

@domenkozar domenkozar changed the title [WIP] pkgs.runCommand: passAsFile (buildCommand can be very long) pkgs.runCommand: passAsFile (buildCommand can be very long) May 30, 2016
@vcunat
Copy link
Member

vcunat commented Jun 9, 2016

eval "$(cat $buildCommandPath)"

@domenkozar: this seems a strange way to do it. Why not use the following instead?

. "$buildCommandPath"

To me it seems cleaner and likely more efficient, but perhaps I'm missing something?

vcunat pushed a commit that referenced this pull request Jun 10, 2016
Close #15803. This avoids the error:

while setting up the build environment: executing
‘/nix/store/7sb42axk5lrxqz45nldrb2pchlys14s1-bash-4.3-p42/bin/bash’:
Argument list too long

Note: I wanted to make it optional based on buildCommand length,
but that seems pointless as I'm sure it's less performant.

Amended by vcunat:
#15803 (comment)
@domenkozar
Copy link
Member Author

@vcunat not sure if all the escaping/quoting logic stays the same, but sure. Thanks!

@domenkozar domenkozar closed this Jun 10, 2016
@vcunat
Copy link
Member

vcunat commented Jun 10, 2016

The escaping logic is just what seems cleaner with ., I think. My conception of . is like pasting the lines in-place. The cat-eval is like this IMHO: first it reads the file, then expands variables in the string and then runs eval which does the variable expansion again.

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

5 participants