[PATCH]

[PATCH]

[Ludovic Courtès]

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

The patch below allows files such as /nix/store/foo to be protected from
GC if a root refers to them.  This makes it easy for a build hook to use
such files for its temporary working files.

WDYT?

Thanks,
Ludo’.


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

diff --git a/src/libstore/gc.cc b/src/libstore/gc.cc
index 96e891f..299bf5f 100644
--- a/src/libstore/gc.cc
+++ b/src/libstore/gc.cc
@@ -287,10 +287,7 @@ static void foundRoot(StoreAPI & store,
     const Path & path, const Path & target, Roots & roots)
 {
     Path storePath = toStorePath(target);
-    if (store.isValidPath(storePath))
     roots[path] = storePath;
-    else
-        printMsg(lvlInfo, format("skipping invalid root from `%1%' to `%2%'") % path % storePath);
 }

[-- Attachment #3: Type: text/plain, Size: 149 bytes --]

_______________________________________________
nix-dev mailing list
nix-dev@lists.science.uu.nl
http://lists.science.uu.nl/mailman/listinfo/nix-dev

Re: [PATCH]

[Shea Levy]

Hi Ludo’,

On Mon, Apr 14, 2014 at 03:38:01PM +0200, Ludovic Courtès wrote:
> The patch below allows files such as /nix/store/foo to be protected from
> GC if a root refers to them.  This makes it easy for a build hook to use
> such files for its temporary working files.
> 
> WDYT?
> 

It seems a bit odd to me to be using the store as a temporary file
location. What's wrong with $TMPDIR?

> 
> Thanks,
> Ludo’.
> 

> diff --git a/src/libstore/gc.cc b/src/libstore/gc.cc
> index 96e891f..299bf5f 100644
> --- a/src/libstore/gc.cc
> +++ b/src/libstore/gc.cc
> @@ -287,10 +287,7 @@ static void foundRoot(StoreAPI & store,
>      const Path & path, const Path & target, Roots & roots)
>  {
>      Path storePath = toStorePath(target);
> -    if (store.isValidPath(storePath))

Hm, don't we at least want to check that it's in the store at all?

> 
>      roots[path] = storePath;
> -    else
> -        printMsg(lvlInfo, format("skipping invalid root from `%1%' to `%2%'") % path % storePath);
>  }

~Shea

> _______________________________________________
> nix-dev mailing list
> nix-dev@lists.science.uu.nl
> http://lists.science.uu.nl/mailman/listinfo/nix-dev

_______________________________________________
nix-dev mailing list
nix-dev@lists.science.uu.nl
http://lists.science.uu.nl/mailman/listinfo/nix-dev

Re: [PATCH] Allow roots to refer to arbitrary files in the store

[Ludovic Courtès]

(Oops, forgot the Subject line; sorry!)

Shea Levy <shea@shealevy.com> skribis:

> On Mon, Apr 14, 2014 at 03:38:01PM +0200, Ludovic Courtès wrote:
>> The patch below allows files such as /nix/store/foo to be protected from
>> GC if a root refers to them.  This makes it easy for a build hook to use
>> such files for its temporary working files.
>> 
>> WDYT?
>> 
>
> It seems a bit odd to me to be using the store as a temporary file
> location. What's wrong with $TMPDIR?

Well, this is useful for build hooks, because it guarantees that any
garbage they leave there will eventually be reclaimed.

>> diff --git a/src/libstore/gc.cc b/src/libstore/gc.cc
>> index 96e891f..299bf5f 100644
>> --- a/src/libstore/gc.cc
>> +++ b/src/libstore/gc.cc
>> @@ -287,10 +287,7 @@ static void foundRoot(StoreAPI & store,
>>      const Path & path, const Path & target, Roots & roots)
>>  {
>>      Path storePath = toStorePath(target);
>> -    if (store.isValidPath(storePath))
>
> Hm, don't we at least want to check that it's in the store at all?

‘toStorePath’ does that:

    if (!isInStore(path))
        throw Error(format("path `%1%' is not in the Nix store") % path);

Thanks,
Ludo’.
_______________________________________________
nix-dev mailing list
nix-dev@lists.science.uu.nl
http://lists.science.uu.nl/mailman/listinfo/nix-dev

Re: [PATCH] Allow roots to refer to arbitrary files in the store

[Ludovic Courtès]

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

> Shea Levy <shea@shealevy.com> skribis:
>
>> On Mon, Apr 14, 2014 at 03:38:01PM +0200, Ludovic Courtès wrote:
>>> The patch below allows files such as /nix/store/foo to be protected from
>>> GC if a root refers to them.  This makes it easy for a build hook to use
>>> such files for its temporary working files.
>>> 
>>> WDYT?
>>> 
>>
>> It seems a bit odd to me to be using the store as a temporary file
>> location. What's wrong with $TMPDIR?
>
> Well, this is useful for build hooks, because it guarantees that any
> garbage they leave there will eventually be reclaimed.

It also guarantees that this thing lives on the same file system as the
store, which means that rename(2) can be used to move it to its final
location in the store.

Ludo’.
_______________________________________________
nix-dev mailing list
nix-dev@lists.science.uu.nl
http://lists.science.uu.nl/mailman/listinfo/nix-dev

Re: [PATCH] Allow roots to refer to arbitrary files in the store

[Shea Levy]

On Mon, Apr 14, 2014 at 04:59:15PM +0200, Ludovic Courtès wrote:
> ludo@gnu.org (Ludovic Courtès) skribis:
> 
> > Shea Levy <shea@shealevy.com> skribis:
> >
> >> On Mon, Apr 14, 2014 at 03:38:01PM +0200, Ludovic Courtès wrote:
> >>> The patch below allows files such as /nix/store/foo to be protected from
> >>> GC if a root refers to them.  This makes it easy for a build hook to use
> >>> such files for its temporary working files.
> >>> 
> >>> WDYT?
> >>> 
> >>
> >> It seems a bit odd to me to be using the store as a temporary file
> >> location. What's wrong with $TMPDIR?
> >
> > Well, this is useful for build hooks, because it guarantees that any
> > garbage they leave there will eventually be reclaimed.
> 
> It also guarantees that this thing lives on the same file system as the
> store, which means that rename(2) can be used to move it to its final
> location in the store.
> 

Hmm using the store as an auto-collected $TMPDIR seems wrong, but this
is a decent argument. Do you have a specific use case already planned?

> 
> Ludo’.
_______________________________________________
nix-dev mailing list
nix-dev@lists.science.uu.nl
http://lists.science.uu.nl/mailman/listinfo/nix-dev

Re: [PATCH] Allow roots to refer to arbitrary files in the store

[Ludovic Courtès]

Shea Levy <shea@shealevy.com> skribis:

> On Mon, Apr 14, 2014 at 04:59:15PM +0200, Ludovic Courtès wrote:
>> ludo@gnu.org (Ludovic Courtès) skribis:
>> 
>> > Shea Levy <shea@shealevy.com> skribis:
>> >
>> >> On Mon, Apr 14, 2014 at 03:38:01PM +0200, Ludovic Courtès wrote:
>> >>> The patch below allows files such as /nix/store/foo to be protected from
>> >>> GC if a root refers to them.  This makes it easy for a build hook to use
>> >>> such files for its temporary working files.
>> >>> 
>> >>> WDYT?
>> >>> 
>> >>
>> >> It seems a bit odd to me to be using the store as a temporary file
>> >> location. What's wrong with $TMPDIR?
>> >
>> > Well, this is useful for build hooks, because it guarantees that any
>> > garbage they leave there will eventually be reclaimed.
>> 
>> It also guarantees that this thing lives on the same file system as the
>> store, which means that rename(2) can be used to move it to its final
>> location in the store.
>> 
>
> Hmm using the store as an auto-collected $TMPDIR seems wrong, but this
> is a decent argument. Do you have a specific use case already planned?

Yup, the build hook, as I wrote (specifically, for the temporary it uses
when restoring from a Nar.)

The other option is to use the temporary root file (that’s what
build-remote.pl does.)  That’s nice, but it requires exposing a lot of
implementation details (GC lock, GC roots directory, etc.)

Ludo’.
_______________________________________________
nix-dev mailing list
nix-dev@lists.science.uu.nl
http://lists.science.uu.nl/mailman/listinfo/nix-dev

Re: [PATCH] Allow roots to refer to arbitrary files in the store

[Ludovic Courtès]

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

> On 14/04/14 16:59, Ludovic Courtès wrote:
>
>>>>> The patch below allows files such as /nix/store/foo to be protected from
>>>>> GC if a root refers to them.  This makes it easy for a build hook to use
>>>>> such files for its temporary working files.
>>>>>
>>>>> WDYT?
>>>>
>>>> It seems a bit odd to me to be using the store as a temporary file
>>>> location. What's wrong with $TMPDIR?
>>>
>>> Well, this is useful for build hooks, because it guarantees that any
>>> garbage they leave there will eventually be reclaimed.
>> 
>> It also guarantees that this thing lives on the same file system as the
>> store, which means that rename(2) can be used to move it to its final
>> location in the store.
>
> The problem with your patch is that those roots don't get cleaned up
> automatically.  A better solution is to generalize the handling of temporary
> store paths like lock files and chroot directories; check out the calls to
> isActiveTempFile() in tryToDelete() in gc.cc. Those don't get deleted while the
> associated build is still running, but do get deleted afterwards.

Hmm yes, I know.

I wanted to avoid going at lengths to expose all that but OK.  That
raises the barrier for build hook implementors...

Thanks,
Ludo’.
_______________________________________________
nix-dev mailing list
nix-dev@lists.science.uu.nl
http://lists.science.uu.nl/mailman/listinfo/nix-dev