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

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

Merged

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!

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
3 participants