Re: [PATCH v2 1/3] devtool modify: Update devtool modify to copy source from work-shared if its already downloaded.

[Paul Eggleton <paul.eggleton@linux.intel.com>]

Thanks for looking into this problem - this is a tricky one. My apologies for the delay in reviewing. Comments below.

On Saturday, 26 January 2019 8:53:00 AM NZDT Sai Hari Chandana Kalluri wrote:
> --- a/scripts/lib/devtool/standard.py
> +++ b/scripts/lib/devtool/standard.py
> @@ -717,6 +717,72 @@ def _check_preserve(config, recipename):
>                      tf.write(line)
>      os.rename(newfile, origfile)
>  
> +# Function links a file from src location to dest location
> +def copy_file(c,dest):
> +    import errno
> +    destdir = os.path.dirname(dest)
> +    if os.path.islink(c):
> +       linkto = os.readlink(c)
> +       if os.path.lexists(dest):
> +            if not os.path.islink(dest):
> +                raise OSError(errno.EEXIST, "Link %s already exists as a file" % dest, dest)
> +            if os.readlink(dest) == linkto:
> +                return dest
> +            raise OSError(errno.EEXIST, "Link %s already exists to a different location? (%s vs %s)" % (dest, os.readlink(dest), linkto), dest)
> +       os.symlink(linkto, dest)
> +    else:
> +       try:
> +           os.link(c, dest)
> +       except OSError as err:
> +           if err.errno == errno.EXDEV:
> +                bb.utils.copyfile(c, dest)
> +           else:
> +                raise
> +
> +# Function creates folders in a given target location
> +def copy_dirs(root,dirs,target):
> +    for d in dirs:
> +        destdir =  os.path.join(target,d)
> +        if os.path.islink(os.path.join(root,d)):
> +             linkto = os.readlink(os.path.join(root,d))
> +             os.symlink(linkto,destdir) 
> +        else:
> +             bb.utils.mkdirhier(target+d)
> +
> +# Function to link src dir to dest dir
> +def copy_src_to_ws(srcdir,srctree):
> +    target = srctree
> +    if os.path.exists(target):
> +        raise DevtoolError('source already in your workspace')
> +
> +    bb.utils.mkdirhier(target)
> +    for root,dirs,files in os.walk(srcdir):
> +        #convert abspath to relpath for root
> +        destdir = root.replace(srcdir,"")
> +        target = srctree+destdir+"/"
> +        copy_dirs(root,dirs,target)  
> +        for f in files:
> +           copy_file(os.path.join(root,f),os.path.join(target,f))

We already have a function to do this - oe.path.copyhardlinktree(). I appreciate that does not symlink if it can't hard link as you're doing here, but given the side-effects if the kernel source goes away I think we'd prefer "hard link or copy" rather than "hard link or symlink".

> @@ -852,10 +990,12 @@ def modify(args, config, basepath, workspace):
>                  f.write('\ndo_patch() {\n'
>                          '    :\n'
>                          '}\n')
> -                f.write('\ndo_configure_append() {\n'
> -                        '    cp ${B}/.config ${S}/.config.baseline\n'
> -                        '    ln -sfT ${B}/.config ${S}/.config.new\n'
> -                        '}\n')
> +
> +            if rd.getVarFlag('do_menuconfig','task'):
> +                    f.write('\ndo_configure_append() {\n'
> +                    '    cp ${B}/.config ${S}/.config.baseline\n'
> +                    '    ln -sfT ${B}/.config ${S}/.config.new\n'
> +                    '}\n')
>              if initial_rev:
>                  f.write('\n# initial_rev: %s\n' % initial_rev)
>                  for commit in commits:

This change does not appear to be directly related to the rest of the commit, can it be split out and properly described?

Thanks,
Paul

-- 

Paul Eggleton
Intel Open Source Technology Centre