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

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

On Mon, Nov 28, 2016 at 12:01:09PM +0100, Kristian Amlie wrote:
> On 28/11/16 11:52, Ed Bartosh wrote:
> > On Mon, Nov 28, 2016 at 08:15:26AM +0100, Kristian Amlie wrote:
> >>>
> >>> 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:]
> >>
> >> Not really any strong reason. I just thought it was a logical thing to
> >> do from a user perspective: When you're making an image you're thinking
> >> about paths in the final image, and the path after "part" is absolute,
> >> so I thought this one should be too.
> >>
> >> The fact that it's made relative in the code is just an implementation
> >> detail to make join() work correctly.
> >>
> >> I'm fine either way, so just let me know which you prefer.
> >>
> > I'd prefer relative path as I think about --exclude-path as a path in
> > the source rootfs directory that we want to exclude from copying to
> > the target partition. However, I agree that that it can be also treated
> > as a path to the target directory.
> 
> Ok!
> 
> I will get a new patchset up soon.
> 

Great!

I'd appreciate if you also do the following before sending v2:
 - check that your code doesn't regress pylint checks
 - add test case for your code to meta/lib/oeqa/selftest/wic.py
 - check that your code doesn't break oe-selftest --coverage -r wic and
   doesn't regress test coverage

--
Regards,
Ed