Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: NixOS/nix
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: 549c3706a5d6
Choose a base ref
...
head repository: NixOS/nix
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: 3c470c97a1b8
Choose a head ref
  • 2 commits
  • 2 files changed
  • 2 contributors

Commits on Nov 25, 2017

  1. build-remote: Fix missing log output

    The storeUri variable in the build-remote hook is declared very much to
    the start of the main function and a bunch of lines later, the same
    variable gets checked via hasPrefix() but it gets assigned *after* that
    check when the most suitable machine for the build was choosen.
    
    So I guess this was just a typo in d16fd24
    and what we really want is to either checkd the prefix *after* assigning
    storeUri or use bestMachine->storeUri directly.
    
    I choose the latter, because the former could introduce even more
    regressions if the try block where the variable gets assigned terminates
    early.
    
    Nevertheless, the reason why the log output didn't work is because
    hasPrefix() checked for "ssh://" in front of storeUri, but if the
    storeUri isn't set correctly (or at all), we don't get the log file
    descriptor set up properly, leading to no log output.
    
    I've adjusted the remote-builds test to include a regression test for
    this, so that we can make sure we get a build output when using remote
    builds.
    
    In addition to that I've tested this with two of my build farms and the
    build logs are emitted correctly again.
    
    Signed-off-by: aszlig <aszlig@nix.build>
    aszlig committed Nov 25, 2017
    1

    Verified

    This commit was signed with the committer’s verified signature.
    Copy the full SHA
    6567ab9 View commit details

Commits on Nov 27, 2017

  1. Merge pull request #1699 from aszlig/fix-remote-build-log

    build-remote: Fix missing log output
    edolstra authored Nov 27, 2017

    Verified

    This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
    Copy the full SHA
    3c470c9 View commit details
Showing with 5 additions and 2 deletions.
  1. +1 −1 src/build-remote/build-remote.cc
  2. +4 −1 tests/remote-builds.nix
2 changes: 1 addition & 1 deletion src/build-remote/build-remote.cc
Original file line number Diff line number Diff line change
@@ -177,7 +177,7 @@ int main (int argc, char * * argv)
Activity act(*logger, lvlTalkative, actUnknown, fmt("connecting to '%s'", bestMachine->storeUri));

Store::Params storeParams;
if (hasPrefix(storeUri, "ssh://")) {
if (hasPrefix(bestMachine->storeUri, "ssh://")) {
storeParams["max-connections"] ="1";
storeParams["log-fd"] = "4";
if (bestMachine->sshKey != "")
5 changes: 4 additions & 1 deletion tests/remote-builds.nix
Original file line number Diff line number Diff line change
@@ -85,7 +85,10 @@ in
}
# Perform a build and check that it was performed on the slave.
my $out = $client->succeed("nix-build ${expr nodes.client.config 1}");
my $out = $client->succeed(
"nix-build ${expr nodes.client.config 1} 2> build-output",
"grep -q Hello build-output"
);
$slave1->succeed("test -e $out");
# And a parallel build.