bug#18747: Fixed-output derivations are not substituted

bug#18747: Fixed-output derivations are not substituted

[Ludovic Courtès]

With current ‘master’, fixed-output derivations are not substituted,
even though a substitute is available:

--8<---------------cut here---------------start------------->8---
$ guix build -S acct
The following file will be downloaded:
   /gnu/store/0sszdqmiqmb4mzbgs8h2185fh7p8bfd2-acct-6.6.1.tar.gz
@ build-started /gnu/store/b7c2lgbdnr5anzklxkzhjvqxl14n7cqp-acct-6.6.1.tar.gz.drv - x86_64-linux /var/log/guix/drvs/b7//c2lgbdnr5anzklxkzhjvqxl14n7cqp-acct-6.6.1.tar.gz.drv.bz2
starting download of `/gnu/store/0sszdqmiqmb4mzbgs8h2185fh7p8bfd2-acct-6.6.1.tar.gz' from `http://ftpmirror.gnu.org/acct/acct-6.6.1.tar.gz'...
following redirection to `http://gnu.cardse.net/acct/acct-6.6.1.tar.gz'...
http://gnu.cardse.net/.../acct-6.6.1.tar.gz	100.0% of 2802.4 KiB
@ build-succeeded /gnu/store/b7c2lgbdnr5anzklxkzhjvqxl14n7cqp-acct-6.6.1.tar.gz.drv -
/gnu/store/0sszdqmiqmb4mzbgs8h2185fh7p8bfd2-acct-6.6.1.tar.gz

$ sudo cat /var/guix/substitute-binary/cache/0sszdqmiqmb4mzbgs8h2185fh7p8bfd2
(narinfo (version 1) (cache-uri "http://hydra.gnu.org") (date 1413472751) (value "StorePath:...

$ guix build -S libwebsockets
The following file will be downloaded:
   /gnu/store/1k4c1a6nckhm2hjpprvdrlkxx6yifclw-libwebsockets-1.3
@ build-started /gnu/store/8x0jgw0819l7aqi1k1f6i772skmwq1zq-libwebsockets-1.3.drv - x86_64-linux /var/log/guix/drvs/8x//0jgw0819l7aqi1k1f6i772skmwq1zq-libwebsockets-1.3.drv.bz2
Cloning into '/gnu/store/1k4c1a6nckhm2hjpprvdrlkxx6yifclw-libwebsockets-1.3'...
C-c C-c

$ sudo cat /var/guix/substitute-binary/cache/1k4c1a6nckhm2hjpprvdrlkxx6yifclw
(narinfo (version 1) (cache-uri "http://hydra.gnu.org") (date 1413466633) (value "StorePath:...
--8<---------------cut here---------------end--------------->8---

Here, “The following file will be downloaded” means that a substitute is
found and will be used, as seen also in the cache.  But then we find
ourselves performing the derivation locally regardless.

Ludo’.

bug#18747: Fixed-output derivations are not substituted

[Ludovic Courtès]

I just realized that this is fixed by just removing #:local-build? #t
from (guix download).

Initially #:local-build? (aka. ‘preferLocalBuild’) meant “avoid
offloading this derivation”, but Nix commit 55586527 (June 2013) changed
it to “avoid offloading *or substituting* this derivation”, which I
somehow overlooked for all this time.

I think we to distinguish between whether to offload and whether to
substitute.  Typically, we don’t want to offload downloads, because
that’s inefficient.  However, we may want to substitute downloads,
because then if the upstream download URL has become invalid, we can
still get the substitute from hydra.gnu.org.

Until then, I think I’ll just comment out #:local-build? #t in (guix
download) and friends.

Thoughts?

Thanks,
Ludo’.

bug#18747: Fixed-output derivations are not substituted

[Ludovic Courtès]

ludo@gnu.org (Ludovic Courtès) skribis:

> Initially #:local-build? (aka. ‘preferLocalBuild’) meant “avoid
> offloading this derivation”, but Nix commit 55586527 (June 2013) changed
> it to “avoid offloading *or substituting* this derivation”, which I
> somehow overlooked for all this time.

The relevant code is in libstore/build.cc:

--8<---------------cut here---------------start------------->8---
    /* We are first going to try to create the invalid output paths
       through substitutes.  If that doesn't work, we'll build
       them. */
    if (settings.useSubstitutes && !willBuildLocally(drv))
        foreach (PathSet::iterator, i, invalidOutputs)
            addWaitee(worker.makeSubstitutionGoal(*i, buildMode == bmRepair));
--8<---------------cut here---------------end--------------->8---

Ludo’.

bug#18747: [PATCH] Distinguish between “offloadability” and “substitutability”

[Ludovic Courtès]

[-- Attachment #1: Type: text/plain, Size: 574 bytes --]

Hi again,

Nix commit 55586527 (June 2013) changed the semantics of
‘preferLocalBuild’ from “avoid offloading this derivation” to “avoid
offloading *or substituting* this derivation” (see
<http://bugs.gnu.org/18747>.)

This patch introduces a new special key, ‘substitution’, to specify
whether a derivation should be substituted.  ‘preferLocalBuild’ is kept,
but its initial semantics is restored.

As a consequence, existing uses of ‘preferLocalBuild’ must be audited to
check what exactly is intended.

WDYT?

Thanks,
Ludo’.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 1636 bytes --]

diff --git a/nix/libstore/misc.cc b/nix/libstore/misc.cc
index 6ecf878..bdb7a6d 100644
--- a/nix/libstore/misc.cc
+++ b/nix/libstore/misc.cc
@@ -67,6 +67,15 @@ Path findOutput(const Derivation & drv, string id)
 }
 
 
+/* Return true if we are allowed to substitute DRV.  This is the case, unless
+   DRV specifies 'substitution = "0"' in its environment.  */
+static bool substitutionAllowed(const Derivation & drv)
+{
+    auto env = drv.env;
+    auto i = env.find("substitution");
+    return i == env.end() || i->second != "0";
+}
+
 void queryMissing(StoreAPI & store, const PathSet & targets,
     PathSet & willBuild, PathSet & willSubstitute, PathSet & unknown,
     unsigned long long & downloadSize, unsigned long long & narSize)
@@ -120,7 +129,7 @@ void queryMissing(StoreAPI & store, const PathSet & targets,
                 if (invalid.empty()) continue;
 
                 todoDrv.insert(*i);
-                if (settings.useSubstitutes && !willBuildLocally(drv))
+                if (settings.useSubstitutes && substitutionAllowed(drv))
                     query.insert(invalid.begin(), invalid.end());
             }
 
@@ -144,7 +153,7 @@ void queryMissing(StoreAPI & store, const PathSet & targets,
 
             PathSet outputs;
             bool mustBuild = false;
-            if (settings.useSubstitutes && !willBuildLocally(drv)) {
+            if (settings.useSubstitutes && substitutionAllowed(drv)) {
                 foreach (DerivationOutputs::iterator, j, drv.outputs) {
                     if (!wantOutput(j->first, i2.second)) continue;
                     if (!store.isValidPath(j->second.path)) {

bug#18747: [PATCH] Distinguish between “offloadability” and “substitutability”

[Eelco Dolstra]

Hi,

On 01/07/15 17:21, Ludovic Courtès wrote:

> Nix commit 55586527 (June 2013) changed the semantics of
> ‘preferLocalBuild’ from “avoid offloading this derivation” to “avoid
> offloading *or substituting* this derivation” (see
> <http://bugs.gnu.org/18747>.)
> 
> This patch introduces a new special key, ‘substitution’, to specify
> whether a derivation should be substituted.  ‘preferLocalBuild’ is kept,
> but its initial semantics is restored.

Have you seen
https://github.com/NixOS/nix/commit/b64988bb3585478676585a0f0aecbcf4e11d4432,
which essentially does the same thing?

-- 
Eelco Dolstra | LogicBlox, Inc. | http://nixos.org/~eelco/

bug#18747: [PATCH] Distinguish between “offloadability” and “substitutability”

[Ludovic Courtès]

Eelco Dolstra <eelco.dolstra@logicblox.com> skribis:

> On 01/07/15 17:21, Ludovic Courtès wrote:
>
>> Nix commit 55586527 (June 2013) changed the semantics of
>> ‘preferLocalBuild’ from “avoid offloading this derivation” to “avoid
>> offloading *or substituting* this derivation” (see
>> <http://bugs.gnu.org/18747>.)
>> 
>> This patch introduces a new special key, ‘substitution’, to specify
>> whether a derivation should be substituted.  ‘preferLocalBuild’ is kept,
>> but its initial semantics is restored.
>
> Have you seen
> https://github.com/NixOS/nix/commit/b64988bb3585478676585a0f0aecbcf4e11d4432,
> which essentially does the same thing?

I hadn’t, but that’s perfect–we’re on the same page.  ;-)

Thanks!

Ludo’.

bug#18747: Fixed-output derivations are not substituted

[Ludovic Courtès]

Fixed in 4a6aeb6.

Note that there are still a few cases, in guix/*download.scm, where we
haven’t yet reintroduced #:local-build? #t.  We’ll do that when daemons
with the fix are more widely deployed.

Ludo’.