[PATCH] lib/oe/path: try hardlinking instead of guessing when it might fail

[PATCH] lib/oe/path: try hardlinking instead of guessing when it might fail

[Henning Schild]

From: Henning Schild <henning.schild@siemens.com>

The comparison of the stat st_dev is not enough to judge whether
hardlinking will work. One example would be where you try and hardlink
across two bind-mounts of a directory. The st_dev will be the same and
the operation will still fail.

Instead of implementing a check to try and figure out hardlink support
just try hardlinking and fall back to a copy when running into an
exception.

Signed-off-by: Henning Schild <henning.schild@siemens.com>
---
 meta/lib/oe/path.py | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/meta/lib/oe/path.py b/meta/lib/oe/path.py
index fa209b9795..082972457b 100644
--- a/meta/lib/oe/path.py
+++ b/meta/lib/oe/path.py
@@ -99,7 +99,22 @@ def copyhardlinktree(src, dst):
     if os.path.isdir(src) and not len(os.listdir(src)):
         return
 
-    if (os.stat(src).st_dev ==  os.stat(dst).st_dev):
+    canhard = False
+    testfile = None
+    for root, dirs, files in os.walk(src):
+        if len(files):
+            testfile = os.path.join(root, files[0])
+            break
+
+    if testfile is not None:
+        try:
+            os.link(testfile, os.path.join(dst, 'testfile'))
+            os.unlink(os.path.join(dst, 'testfile'))
+            canhard = True
+        except Exception as e:
+            bb.debug(2, "Hardlink test failed with " + str(e))
+
+    if (canhard):
         # Need to copy directories only with tar first since cp will error if two 
         # writers try and create a directory at the same time
         cmd = "cd %s; find . -type d -print | tar --xattrs --xattrs-include='*' -cf - -S -C %s -p --no-recursion --files-from - | tar --xattrs --xattrs-include='*' -xhf - -C %s" % (src, src, dst)
@@ -121,12 +136,9 @@ def copyhardlinktree(src, dst):
 def copyhardlink(src, dst):
     """Make a hard link when possible, otherwise copy."""
 
-    # We need to stat the destination directory as the destination file probably
-    # doesn't exist yet.
-    dstdir = os.path.dirname(dst)
-    if os.stat(src).st_dev == os.stat(dstdir).st_dev:
+    try:
         os.link(src, dst)
-    else:
+    except OSError:
         shutil.copy(src, dst)
 
 def remove(path, recurse=True):
-- 
2.24.1

Re: [PATCH] lib/oe/path: try hardlinking instead of guessing when it might fail

[Henning Schild]

This is related to fixing an issue with "--exclude-path" in wic. On the
Isar side i have everything in place and might send it. But we need
that upstream fix ... or some workaround if that does not get merged.

Henning

On Fri, 24 Jan 2020 14:48:47 +0100
Henning Schild <henning.schild@siemens.com> wrote:

> From: Henning Schild <henning.schild@siemens.com>
> 
> The comparison of the stat st_dev is not enough to judge whether
> hardlinking will work. One example would be where you try and hardlink
> across two bind-mounts of a directory. The st_dev will be the same and
> the operation will still fail.
> 
> Instead of implementing a check to try and figure out hardlink support
> just try hardlinking and fall back to a copy when running into an
> exception.
> 
> Signed-off-by: Henning Schild <henning.schild@siemens.com>
> ---
>  meta/lib/oe/path.py | 24 ++++++++++++++++++------
>  1 file changed, 18 insertions(+), 6 deletions(-)
> 
> diff --git a/meta/lib/oe/path.py b/meta/lib/oe/path.py
> index fa209b9795..082972457b 100644
> --- a/meta/lib/oe/path.py
> +++ b/meta/lib/oe/path.py
> @@ -99,7 +99,22 @@ def copyhardlinktree(src, dst):
>      if os.path.isdir(src) and not len(os.listdir(src)):
>          return
>  
> -    if (os.stat(src).st_dev ==  os.stat(dst).st_dev):
> +    canhard = False
> +    testfile = None
> +    for root, dirs, files in os.walk(src):
> +        if len(files):
> +            testfile = os.path.join(root, files[0])
> +            break
> +
> +    if testfile is not None:
> +        try:
> +            os.link(testfile, os.path.join(dst, 'testfile'))
> +            os.unlink(os.path.join(dst, 'testfile'))
> +            canhard = True
> +        except Exception as e:
> +            bb.debug(2, "Hardlink test failed with " + str(e))
> +
> +    if (canhard):
>          # Need to copy directories only with tar first since cp will
> error if two # writers try and create a directory at the same time
>          cmd = "cd %s; find . -type d -print | tar --xattrs
> --xattrs-include='*' -cf - -S -C %s -p --no-recursion --files-from -
> | tar --xattrs --xattrs-include='*' -xhf - -C %s" % (src, src, dst)
> @@ -121,12 +136,9 @@ def copyhardlinktree(src, dst): def
> copyhardlink(src, dst): """Make a hard link when possible, otherwise
> copy.""" 
> -    # We need to stat the destination directory as the destination
> file probably
> -    # doesn't exist yet.
> -    dstdir = os.path.dirname(dst)
> -    if os.stat(src).st_dev == os.stat(dstdir).st_dev:
> +    try:
>          os.link(src, dst)
> -    else:
> +    except OSError:
>          shutil.copy(src, dst)
>  
>  def remove(path, recurse=True):

Re: [PATCH] lib/oe/path: try hardlinking instead of guessing when it might fail

[Henning Schild]

Merged upstream as f5571bda8327f927feb23b167ab4594b7d0c95bc

Henning

On Fri, 24 Jan 2020 14:51:00 +0100
"[ext] Henning Schild" <henning.schild@siemens.com> wrote:

> This is related to fixing an issue with "--exclude-path" in wic. On
> the Isar side i have everything in place and might send it. But we
> need that upstream fix ... or some workaround if that does not get
> merged.
> 
> Henning
> 
> On Fri, 24 Jan 2020 14:48:47 +0100
> Henning Schild <henning.schild@siemens.com> wrote:
> 
> > From: Henning Schild <henning.schild@siemens.com>
> > 
> > The comparison of the stat st_dev is not enough to judge whether
> > hardlinking will work. One example would be where you try and
> > hardlink across two bind-mounts of a directory. The st_dev will be
> > the same and the operation will still fail.
> > 
> > Instead of implementing a check to try and figure out hardlink
> > support just try hardlinking and fall back to a copy when running
> > into an exception.
> > 
> > Signed-off-by: Henning Schild <henning.schild@siemens.com>
> > ---
> >  meta/lib/oe/path.py | 24 ++++++++++++++++++------
> >  1 file changed, 18 insertions(+), 6 deletions(-)
> > 
> > diff --git a/meta/lib/oe/path.py b/meta/lib/oe/path.py
> > index fa209b9795..082972457b 100644
> > --- a/meta/lib/oe/path.py
> > +++ b/meta/lib/oe/path.py
> > @@ -99,7 +99,22 @@ def copyhardlinktree(src, dst):
> >      if os.path.isdir(src) and not len(os.listdir(src)):
> >          return
> >  
> > -    if (os.stat(src).st_dev ==  os.stat(dst).st_dev):
> > +    canhard = False
> > +    testfile = None
> > +    for root, dirs, files in os.walk(src):
> > +        if len(files):
> > +            testfile = os.path.join(root, files[0])
> > +            break
> > +
> > +    if testfile is not None:
> > +        try:
> > +            os.link(testfile, os.path.join(dst, 'testfile'))
> > +            os.unlink(os.path.join(dst, 'testfile'))
> > +            canhard = True
> > +        except Exception as e:
> > +            bb.debug(2, "Hardlink test failed with " + str(e))
> > +
> > +    if (canhard):
> >          # Need to copy directories only with tar first since cp
> > will error if two # writers try and create a directory at the same
> > time cmd = "cd %s; find . -type d -print | tar --xattrs
> > --xattrs-include='*' -cf - -S -C %s -p --no-recursion --files-from -
> > | tar --xattrs --xattrs-include='*' -xhf - -C %s" % (src, src, dst)
> > @@ -121,12 +136,9 @@ def copyhardlinktree(src, dst): def
> > copyhardlink(src, dst): """Make a hard link when possible, otherwise
> > copy.""" 
> > -    # We need to stat the destination directory as the destination
> > file probably
> > -    # doesn't exist yet.
> > -    dstdir = os.path.dirname(dst)
> > -    if os.stat(src).st_dev == os.stat(dstdir).st_dev:
> > +    try:
> >          os.link(src, dst)
> > -    else:
> > +    except OSError:
> >          shutil.copy(src, dst)
> >  
> >  def remove(path, recurse=True):  
> 

Re: [PATCH] lib/oe/path: try hardlinking instead of guessing when it might fail

[Baurzhan Ismagulov]

Hello Henning,

On Fri, Jan 24, 2020 at 02:51:00PM +0100, Henning Schild wrote:
> This is related to fixing an issue with "--exclude-path" in wic. On the
> Isar side i have everything in place and might send it. But we need
> that upstream fix ... or some workaround if that does not get merged.

Do I understand correctly, you'd be providing a lib/oe update for Isar along
with the other necessary changes? A test case for --exclude-path would be nice.

With kind regards,
Baurzhan.

Re: [PATCH] lib/oe/path: try hardlinking instead of guessing when it might fail

[Henning Schild]

On Sun, 5 Apr 2020 18:58:16 +0200
Baurzhan Ismagulov <ibr@radix50.net> wrote:

> Hello Henning,
> 
> On Fri, Jan 24, 2020 at 02:51:00PM +0100, Henning Schild wrote:
> > This is related to fixing an issue with "--exclude-path" in wic. On
> > the Isar side i have everything in place and might send it. But we
> > need that upstream fix ... or some workaround if that does not get
> > merged.  
> 
> Do I understand correctly, you'd be providing a lib/oe update for
> Isar along with the other necessary changes? A test case for
> --exclude-path would be nice.

The test-case is already somewhere in a branch of mine ;). The problem
now is that this upstream fix need to come down into Isar.

But i am afraid that our lib/oe is a fork, miles away from upstream.
Not sure what to do here. A backport would work for sure but i am not
sure i would want to contribute to forking away further ...
On the other hand it is probably not realistic to switch to upstream.

I did not look into it so far, i am afraid of what i would find ...

We use a vanilla bitbake, and wic, but not that other stuff.

Henning

> With kind regards,
> Baurzhan.
> 

Re: [PATCH] lib/oe/path: try hardlinking instead of guessing when it might fail

[Jan Kiszka]

On 06.04.20 09:42, Henning Schild wrote:
> On Sun, 5 Apr 2020 18:58:16 +0200
> Baurzhan Ismagulov <ibr@radix50.net> wrote:
> 
>> Hello Henning,
>>
>> On Fri, Jan 24, 2020 at 02:51:00PM +0100, Henning Schild wrote:
>>> This is related to fixing an issue with "--exclude-path" in wic. On
>>> the Isar side i have everything in place and might send it. But we
>>> need that upstream fix ... or some workaround if that does not get
>>> merged.
>>
>> Do I understand correctly, you'd be providing a lib/oe update for
>> Isar along with the other necessary changes? A test case for
>> --exclude-path would be nice.
> 
> The test-case is already somewhere in a branch of mine ;). The problem
> now is that this upstream fix need to come down into Isar.
> 
> But i am afraid that our lib/oe is a fork, miles away from upstream.

We are fairly close, picking patches from upstream is often possible 
when we aren't. I just did that not long ago 
(9813e9e821cb2c4f0ea66544e35030fa261f1a47). Some bits may have to remain 
different sometimes, though.

Jan

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux

Re: [PATCH] lib/oe/path: try hardlinking instead of guessing when it might fail

[Henning Schild]

On Mon, 6 Apr 2020 09:51:23 +0200
Jan Kiszka <jan.kiszka@siemens.com> wrote:

> On 06.04.20 09:42, Henning Schild wrote:
> > On Sun, 5 Apr 2020 18:58:16 +0200
> > Baurzhan Ismagulov <ibr@radix50.net> wrote:
> >   
> >> Hello Henning,
> >>
> >> On Fri, Jan 24, 2020 at 02:51:00PM +0100, Henning Schild wrote:  
> >>> This is related to fixing an issue with "--exclude-path" in wic.
> >>> On the Isar side i have everything in place and might send it.
> >>> But we need that upstream fix ... or some workaround if that does
> >>> not get merged.  
> >>
> >> Do I understand correctly, you'd be providing a lib/oe update for
> >> Isar along with the other necessary changes? A test case for
> >> --exclude-path would be nice.  
> > 
> > The test-case is already somewhere in a branch of mine ;). The
> > problem now is that this upstream fix need to come down into Isar.
> > 
> > But i am afraid that our lib/oe is a fork, miles away from
> > upstream.  
> 
> We are fairly close, picking patches from upstream is often possible 
> when we aren't. I just did that not long ago 
> (9813e9e821cb2c4f0ea66544e35030fa261f1a47). Some bits may have to
> remain different sometimes, though.

Ok, i will try a backport and send that test-case along with it.

Henning

> Jan
> 

Re: [PATCH] lib/oe/path: try hardlinking instead of guessing when it might fail

[Henning Schild]

On Mon, 6 Apr 2020 12:03:47 +0200
"[ext] Henning Schild" <henning.schild@siemens.com> wrote:

> On Mon, 6 Apr 2020 09:51:23 +0200
> Jan Kiszka <jan.kiszka@siemens.com> wrote:
> 
> > On 06.04.20 09:42, Henning Schild wrote:  
> > > On Sun, 5 Apr 2020 18:58:16 +0200
> > > Baurzhan Ismagulov <ibr@radix50.net> wrote:
> > >     
> > >> Hello Henning,
> > >>
> > >> On Fri, Jan 24, 2020 at 02:51:00PM +0100, Henning Schild wrote:
> > >>   
> > >>> This is related to fixing an issue with "--exclude-path" in wic.
> > >>> On the Isar side i have everything in place and might send it.
> > >>> But we need that upstream fix ... or some workaround if that
> > >>> does not get merged.    
> > >>
> > >> Do I understand correctly, you'd be providing a lib/oe update for
> > >> Isar along with the other necessary changes? A test case for
> > >> --exclude-path would be nice.    
> > > 
> > > The test-case is already somewhere in a branch of mine ;). The
> > > problem now is that this upstream fix need to come down into Isar.
> > > 
> > > But i am afraid that our lib/oe is a fork, miles away from
> > > upstream.    
> > 
> > We are fairly close, picking patches from upstream is often
> > possible when we aren't. I just did that not long ago 
> > (9813e9e821cb2c4f0ea66544e35030fa261f1a47). Some bits may have to
> > remain different sometimes, though.  
> 
> Ok, i will try a backport and send that test-case along with it.

Just sent something that fixes and tests in the future.

Henning

> Henning
> 
> > Jan
> >   
>