Conversation
|
Ref #341 |
|
Shouldn't this also remove |
|
That needs to be coordinated with a change in NixOS. |
|
@shlevy cool, let's then keep a list of TODOs somewhere for NixOS so this is not forgotten. |
|
cc @edolstra |
| look at the actual remote load.</para> | ||
|
|
||
| <para>Additionally, you may define the environment variable | ||
| <envar>NIX_REMOTE_USE_SUBSTITUTES</envar>, which will instruct |
There was a problem hiding this comment.
Why an environment variable instead of a nix.conf option?
There was a problem hiding this comment.
I expected the options in nix.conf to be directly related to Nix, not to the build-remote hook. But I agree that a nix.conf option would be better, if that is acceptable.
|
@edolstra switched to the nix conf option for hook substitutes |
src/build-remote/build-remote.cc
Outdated
|
|
||
| auto localSystem = argv[1]; | ||
| settings.maxSilentTime = strtoull(argv[2], NULL, 10); | ||
| settings.buildTimeout = strtoull(argv[3], NULL, 10); |
There was a problem hiding this comment.
Calls to strtoull are not checked for parse error.
There was a problem hiding this comment.
It will return ULLONG_MAX, which is a fine default. Note that this is an internal (libexec) tool.
There was a problem hiding this comment.
This prints 0:
unsigned long long res = strtoull("blah", NULL, 10);
printf("%llu\n", res);The function returns ULLONG_MAX in case of overflow.
5dd21d1 to
bff3ad7
Compare
|
@edolstra rebased on master. |
|
ping |
|
@shlevy can we fix the tests here? |
|
@edolstra said he was working on a fix |
|
ping |
1 similar comment
|
ping |
|
Ping |
|
Merry Christmas! Ping. |
|
@edolstra Can you give me a time by which I can expect a review, even if that's not in the near future? |
src/build-remote/build-remote.cc
Outdated
|
|
||
| class machine { | ||
| const std::vector<string> supportedFeatures; | ||
| const std::vector<string> mandatoryFeatures; |
src/build-remote/build-remote.cc
Outdated
| [&](const string & feature) { | ||
| return std::find(supportedFeatures.begin(), | ||
| supportedFeatures.end(), | ||
| feature) != supportedFeatures.end() || |
There was a problem hiding this comment.
This can be changed to supportedFeatures.count(feature) is supportedFeatures is a set.
src/build-remote/build-remote.cc
Outdated
| auto machines = std::vector<machine>{}; | ||
| auto confFile = std::ifstream{conf}; | ||
| if (confFile.good()) { | ||
| confFile.exceptions(std::ifstream::badbit); |
There was a problem hiding this comment.
C++ streams considered harmful, among other things due to the wacky error/exception handling. I would just do tokenizeString(readFile(conf), "\n").
|
@edolstra what's the plan for this PR? |
|
🍾 |
|
Was this the last remnant of perl or is there other stuff left? I can't wait to see perl disappear from release.nix 😄 |
|
Thanks @shlevy ! |
|
It appears that |
|
Commit using futimes pushed. |
|
@edolstra now we need NixOS/nixpkgs#17697 to fix tests, does that mean we finally get #1134 ? 🙈 |
No description provided.