[elbe-devel] [RFC PATCH 6/6] efilesystem: Fix copy_filelist to handle symlinks

[Torben Hohn]

On Tue, Apr 07, 2020 at 03:32:46PM -0400, Olivier Dion wrote:
> The algorithm is divided in 3 parts:
> 
>   1. Construct a set of path to copy.  Make it a list, sort it and
>   reverse it.
> 
>   2. Iterate over all paths, ensure that directories exist.  If a path
>   is a symlink, ensure that the target of the link exists, otherwise
>   defer the copy of the symlink after the construction of the target.
>   NOTE! There's no cycle detection here!
> 
>   3. Copy stat for directories that aren't symlink since copying file
>   to it result in utime being changed.
> 
> Signed-off-by: Olivier Dion <dion at linutronix.de>
> ---
>  elbepack/efilesystem.py | 61 +++++++++++++++++++++++++++++++++--------
>  1 file changed, 49 insertions(+), 12 deletions(-)
> 
> diff --git a/elbepack/efilesystem.py b/elbepack/efilesystem.py
> index 15074db2..3c0f64de 100644
> --- a/elbepack/efilesystem.py
> +++ b/elbepack/efilesystem.py
> @@ -27,25 +27,62 @@ from elbepack.shellhelper import (system,
>                                    get_command_out)
>  
>  
> -def copy_filelist(src, filelist, dst):
> -    for f in filelist:
> -        f = f.rstrip("\n")
> -        if src.isdir(f) and not src.islink(f):
> +def copy_filelist(src, file_lst, dst):
> +
> +    files = set()
> +
> +    for f in file_lst:
> +        tmp = f.rstrip('\n')
> +
> +        # Relative to root
> +        if tmp[0] != os.sep:

use os.path.isabs()
But... i dont understand the point.
What is different, if the files list contains a relative path ?
why would that not make the logic generate the parent directories ?

why not use os.path.join('/', tmp) ?

> +            files.add('/%s' % tmp)
> +            continue
> +
> +        parts = f.split(os.sep)[1:]
> +        parts.reverse()
> +        path = ""
> +        while parts:
> +            path += "/%s" % parts.pop()
> +            files.add(path)

this seems overly complicated to me.

i suggest:

------------------------------------------------------------------------
>>> files = set()
>>> parts = '/df/fg/df/fg'
>>> while parts != os.sep:
...     files.add(parts)
...     parts, _ = os.path.split(parts)
...     
... 
>>> files
{'/df/fg/df/fg', '/df/fg', '/df/fg/df', '/df'}
>>> 
------------------------------------------------------------------------

> +
> +    files  = list(files)
> +    files.sort()
> +    files.reverse()
> +
> +    copied = set()
> +
> +    while files:
> +
> +        f = files.pop()
> +        copied.add(f)
> +
> +        if src.islink(f):
> +            tgt = src.readlink(f)
> +            # Relative?
> +            if tgt[0] != os.path.sep:

os.path.isabs()

> +                # Get the absolute target
> +                tgt = os.path.join(os.sep.join(f.split(os.sep)[:-1]), tgt)

os.path.join(os.path.dirname(f), tgt) 

> +            if not dst.exists(tgt):
> +                # Defer symlink after target

improve comment, this confused me :)
f is the symlink, and tgt is the target, of course.
this would also recursively follow things, right ?

> +                files.append(f)
> +                files.append(tgt)
> +            else:
> +                dst.symlink(tgt, f)
> +
> +        elif src.isdir(f):
>              if not dst.isdir(f):
>                  dst.mkdir(f)
>              st = src.stat(f)
>              dst.chown(f, st.st_uid, st.st_gid)
> +
>          else:
> -            if src.isdir(f) and src.islink(f):
> -                tgt = src.readlink(f)
> -                if not dst.isdir(tgt):
> -                    dst.mkdir(tgt)
> -            system('cp -a --reflink=auto "%s" "%s"' % (src.fname(f),
> -                                                       dst.fname(f)))
> +            system('cp -a --reflink=auto "%s" "%s"' % (src.realpath(f),
> +                                                       dst.realpath(f)))
> +
>      # update utime which will change after a file has been copied into
>      # the directory
> -    for f in filelist:
> -        f = f.rstrip("\n")
> +    for f in copied:
>          if src.isdir(f) and not src.islink(f):
>              shutil.copystat(src.fname(f), dst.fname(f))
>  
> -- 
> 2.26.0
> 
> 
> _______________________________________________
> elbe-devel mailing list
> elbe-devel at linutronix.de
> https://lists.linutronix.de/mailman/listinfo/elbe-devel

-- 
Torben Hohn
Linutronix GmbH | Bahnhofstrasse 3 | D-88690 Uhldingen-Mühlhofen
Phone: +49 7556 25 999 18; Fax.: +49 7556 25 999 99

Hinweise zum Datenschutz finden Sie hier (Informations on data privacy 
can be found here): https://linutronix.de/kontakt/Datenschutz.php

Linutronix GmbH | Firmensitz (Registered Office): Uhldingen-Mühlhofen | 
Registergericht (Registration Court): Amtsgericht Freiburg i.Br., HRB700 
806 | Geschäftsführer (Managing Directors): Heinz Egger, Thomas Gleixner