Skip to content

Don't let 'preferLocalBuild' override 'max-jobs=0'#4443

Merged
edolstra merged 2 commits intoNixOS:masterfrom
rickynils:prefer-local-build-respect-zero-max-jobs
Jan 13, 2021
Merged

Don't let 'preferLocalBuild' override 'max-jobs=0'#4443
edolstra merged 2 commits intoNixOS:masterfrom
rickynils:prefer-local-build-respect-zero-max-jobs

Conversation

@rickynils
Copy link
Member

This resolves #3810 by changing the behavior of max-jobs = 0, so
that specifying the option also avoids local building of derivations
with the attribute preferLocalBuild = true.

A more complicated resolution was suggested by @grahamc in #3810, that also made it possible to retain the original behavior of max-jobs=0. However, it has been suggested (here and here) that just letting preferLocalbuild respect max-jobs=0 is an acceptable solution too.

This resolves NixOS#3810 by changing the behavior of `max-jobs = 0`, so
that specifying the option also avoids local building of derivations
with the attribute `preferLocalBuild = true`.
@edolstra
Copy link
Member

Can you also update this code in DerivationGoal::tryLocalBuild():

    /* Make sure that we are allowed to start a build.  If this
       derivation prefers to be done locally, do it even if
       maxBuildJobs is 0. */
    unsigned int curBuilds = worker.getNrLocalBuilds();
    if (curBuilds >= settings.maxBuildJobs && !(buildLocally && curBuilds == 0)) {
        worker.waitForBuildSlot(shared_from_this());
        outputLocks.unlock();
        return;
    }

@rickynils
Copy link
Member Author

@edolstra Is there anything else than the comment that actually needs to be changed in that code? My commit changes the behavior of willBuildLocally so that it always is false if max-jobs = 0. That means buildLocally will be false in the DerivationGoal::tryLocalBuild() as long as buildMode == bmNormal. I might have missed something though?

@domenkozar
Copy link
Member

Probably the comment needs changing too.

@edolstra
Copy link
Member

The condition && !(buildLocally && curBuilds == 0) can be dropped.

@rickynils
Copy link
Member Author

@edolstra Thanks, I've removed it now.

@edolstra edolstra merged commit fbfa70d into NixOS:master Jan 13, 2021
@edolstra
Copy link
Member

Thanks!

return false;

if (settings.maxBuildJobs.get() == 0
&& !drv.isBuiltin())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we have this isn't builtin check here? do derivations running builtin builders not count towards max jobs?! I wouldn't think so

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

preferLocalBuild should not override max-jobs=0

4 participants