Re: 01/03: guix: store: Add ensure-path.

Re: 01/03: guix: store: Add ensure-path.

[Ludovic Courtès]

Hi!

guix-commits@gnu.org skribis:

> commit 2a7e1121d83ee173a7a0dbfb978a4ac363c7c4a6
> Author: Mathieu Othacehe <othacehe@gnu.org>
> AuthorDate: Wed Nov 4 14:39:40 2020 +0100
>
>     guix: store: Add ensure-path.
>     
>     * guix/store.scm (ensure-path): New procedure.

[...]

> +(define-operation (ensure-path (store-path path))
> +  "Make PATH a temporary root for the duration of the current session.
> +Return #t."
> +  boolean)

Looking at LocalStore::ensurePath, I don’t think a GC root is made.
You have to call ‘add-temp-root’ if you want that to happen, no?

However ‘ensure-path’ can substitute the store item, which I suppose
means you can substitute .drv files without building them, am I right?
:-)

Ludo’.

Re: 01/03: guix: store: Add ensure-path.

[Christopher Baines]

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


Ludovic Courtès <ludo@gnu.org> writes:

> Hi!
>
> guix-commits@gnu.org skribis:
>
>> commit 2a7e1121d83ee173a7a0dbfb978a4ac363c7c4a6
>> Author: Mathieu Othacehe <othacehe@gnu.org>
>> AuthorDate: Wed Nov 4 14:39:40 2020 +0100
>>
>>     guix: store: Add ensure-path.
>>     
>>     * guix/store.scm (ensure-path): New procedure.
>
> [...]
>
>> +(define-operation (ensure-path (store-path path))
>> +  "Make PATH a temporary root for the duration of the current session.
>> +Return #t."
>> +  boolean)
>
> Looking at LocalStore::ensurePath, I don’t think a GC root is made.
> You have to call ‘add-temp-root’ if you want that to happen, no?
>
> However ‘ensure-path’ can substitute the store item, which I suppose
> means you can substitute .drv files without building them, am I right?
> :-)

That's an exciting prospect! That would help a lot with the Guix Build
Coordinator, being able to substitute derivations without building them.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 987 bytes --]

Re: 01/03: guix: store: Add ensure-path.

[Mathieu Othacehe]

Hey,

> Looking at LocalStore::ensurePath, I don’t think a GC root is made.
> You have to call ‘add-temp-root’ if you want that to happen, no?

Yes, I think we do indeed.

> However ‘ensure-path’ can substitute the store item, which I suppose
> means you can substitute .drv files without building them, am I right?

Yes I'm using it this way:

--8<---------------cut here---------------start------------->8---
(define (add-to-store outputs url)
  (with-store store
    (for-each (lambda (output)
                (add-to-locks-held store output)
                (add-substitute-url store url)
                (ensure-path store output))
              (map derivation-output-path outputs))))
--8<---------------cut here---------------end--------------->8---

to fetch substitutes without building derivations. The painful part is
that I have to use my own custom RPC: AddToLocksHeld, so that I can add
the substitute to the store even if the output lock is held.

At first, I planned to use "restore-file-set" from (guix nar) but it
only handles nar files produced by "guix archive --export".

Thanks,

Mathieu

Re: 01/03: guix: store: Add ensure-path.

[Ludovic Courtès]

Hi,

Mathieu Othacehe <othacehe@gnu.org> skribis:

>> Looking at LocalStore::ensurePath, I don’t think a GC root is made.
>> You have to call ‘add-temp-root’ if you want that to happen, no?
>
> Yes, I think we do indeed.

So perhaps we should adjust the docstring to reflect that?

>> However ‘ensure-path’ can substitute the store item, which I suppose
>> means you can substitute .drv files without building them, am I right?
>
> Yes I'm using it this way:
>
> (define (add-to-store outputs url)
>   (with-store store
>     (for-each (lambda (output)
>                 (add-to-locks-held store output)
>                 (add-substitute-url store url)
>                 (ensure-path store output))
>               (map derivation-output-path outputs))))
>
> to fetch substitutes without building derivations. The painful part is
> that I have to use my own custom RPC: AddToLocksHeld, so that I can add
> the substitute to the store even if the output lock is held.

Yeah, ‘guix offload’ should not connect to the daemon.  It does so
currently for one thing (registering GC roots IIRC), which should be
done in Scheme instead.

> At first, I planned to use "restore-file-set" from (guix nar) but it
> only handles nar files produced by "guix archive --export".

Specifically it handles “nar bundles” (new name :-)).  But probably you
can extract the relevant bits to import single nars, right?

Thanks,
Ludo’.

Re: 01/03: guix: store: Add ensure-path.

[Mathieu Othacehe]

Hey,

> So perhaps we should adjust the docstring to reflect that?

Sure, done with: d9e57db72479812cfa30ed8e30a6351959f9a2b2.

> Yeah, ‘guix offload’ should not connect to the daemon.  It does so
> currently for one thing (registering GC roots IIRC), which should be
> done in Scheme instead.

Why shouldn't it connect to the daemon? In the current implementation of
the nix daemon, there's the following snippet:

--8<---------------cut here---------------start------------->8---
        if (!missing.empty()) {
            Activity act(*logger, lvlTalkative, actUnknown, fmt("copying outputs from '%s'", storeUri));
            for (auto & i : missing)
                store->locksHeld.insert(store->printStorePath(i)); /* FIXME: ugly */
            copyPaths(ref<Store>(sshStore), store, missing, NoRepair, NoCheckSigs, NoSubstitute);
        }
--8<---------------cut here---------------end--------------->8---

in build-remote.cc. So it looks like it connects to the store, add the
outputs to locks held and import them, which is the behaviour I'm
trying to replicate.

> Specifically it handles “nar bundles” (new name :-)).  But probably you
> can extract the relevant bits to import single nars, right?

Yup you're right, it should be possible, but I'd rater let the daemon do
it if possible.

Thanks,

Mathieu

Re: 01/03: guix: store: Add ensure-path.

[Ludovic Courtès]

Hi,

Mathieu Othacehe <othacehe@gnu.org> skribis:

>> Yeah, ‘guix offload’ should not connect to the daemon.  It does so
>> currently for one thing (registering GC roots IIRC), which should be
>> done in Scheme instead.
>
> Why shouldn't it connect to the daemon? In the current implementation of
> the nix daemon, there's the following snippet:
>
>         if (!missing.empty()) {
>             Activity act(*logger, lvlTalkative, actUnknown, fmt("copying outputs from '%s'", storeUri));
>             for (auto & i : missing)
>                 store->locksHeld.insert(store->printStorePath(i)); /* FIXME: ugly */
>             copyPaths(ref<Store>(sshStore), store, missing, NoRepair, NoCheckSigs, NoSubstitute);
>         }
>
> in build-remote.cc. So it looks like it connects to the store, add the
> outputs to locks held and import them, which is the behaviour I'm
> trying to replicate.

Yes but here ‘store’ is an instance of ‘LocalStore’ typically.

I’m referring to this bit in (guix nar):

  (with-store store
    (let loop ((name (temporary-store-file)))
      ;; Add NAME to the current process' roots.  (Opening this connection to
      ;; the daemon allows us to reuse its code that deals with the
      ;; per-process roots file.)
      (add-temp-root store name)

This is (I think) the only place where ‘guix offload’ connects to the
daemon, and it does so just because we don’t have an ‘add-temp-root’
implementation in Scheme.

>> Specifically it handles “nar bundles” (new name :-)).  But probably you
>> can extract the relevant bits to import single nars, right?
>
> Yup you're right, it should be possible, but I'd rater let the daemon do
> it if possible.

Sure, but see ‘restore-one-item’ vs ‘restore-file-set’.  Perhaps
‘restore-one-item’ already does what you want.

HTH,
Ludo’.