Re: [PATCH v1] wic: Add --exclude-path option to rootfs source plugin.

[Ed Bartosh <ed.bartosh@linux.intel.com>]

On Fri, Nov 25, 2016 at 01:07:34PM +0100, Kristian Amlie wrote:
> On 25/11/16 11:33, Patrick Ohly wrote:
> > On Fri, 2016-11-25 at 11:15 +0100, Kristian Amlie wrote:
> >> +            if os.stat(real_rootfs_dir).st_dev ==
> >> os.stat(cr_workdir).st_dev:
> >> +                # Optimization if both directories are on the same
> >> file system:
> >> +                # copy using hardlinks.
> >> +                cp_args = "-al"
> >> +            else:
> >> +                cp_args = "-a"
> >> +            exec_cmd("cp %s %s %s" % (cp_args, real_rootfs_dir,
> >> new_rootfs))
> > 
> > Not a full review (I'll leave that to Ed), just one thing which caught
> > my eye: when the rootfs contains xattrs, they get lost here.
> > 
> > Use oe.path.copyhardlinktree() instead, it also does the hardlinking
> > trick.
> 
> Thanks, that's a good tip! I'll include that in the next patchset.
> 

Thank you for so fast implementation!

Sorry for not answering on original e-mail. I've lost it somehow.

My comments so far:

What's the reason of insisting that path must be absolute?
May be it's just me, but I find it a bit scaring to use absolute path in .wks
The patch is relative to the rootfs directory from my point of view.

It also looks quite strange in the code to insist on absolute path
+                if not os.path.isabs(path):
+                    msger.error("Must be absolute: --exclude-path=%s" %

and then immediately making it relative:
+
+                while os.path.isabs(path):
+                    path = path[1:]


I know, this is just a matter of taste, but I'd not use brackets around
head, tail here. They're redundant and the code looks better without
them from my point of view.
+                    (head, tail) = os.path.split(remaining)

This causes rmtree to throw NotADirectoryError on files:
+                    for entry in os.listdir(full_path):
+                        shutil.rmtree(os.path.join(full_path, entry))

--
Regards,
Ed