[bitbake-devel] [PATCH] bb/fetch2: fixes copying of file://dir; subdir=foo, bug 6128 and bug 6129

[Richard Purdie]

On Fri, 2016-02-26 at 16:51 +0000, Richard Purdie wrote:
> On Fri, 2016-02-26 at 11:46 +0000, Richard Purdie wrote:
> > On Fri, 2016-02-26 at 13:30 +0200, Sasha Shashkevich wrote:
> > > > On Feb 26, 2016, at 12:51, Richard Purdie <
> > > > richard.purdie at linuxfoundation.org> wrote:
> > > > 
> > > > On Thu, 2016-02-25 at 18:32 +0200, Alexander Shashkevich wrote:
> > > > > When in SRC_URI appears file://dir;subdir=foo unpacker copies
> > > > > 'dir'
> > > > > to ${WORKDIR}, not
> > > > > ${WORKDIR}/foo as it should be.
> > > > > 
> > > > > These changes are fixing following bugs as well:
> > > > > Bug 6128 - Incorrect wildcard unpack behaviour in fetcher
> > > > > Bug 6129 - Local directories unpack to a different location
> > > > > than
> > > > > local files
> > > > 
> > > > Sadly, this change isn't without side effects. For example:
> > > > 
> > > > https://autobuilder.yoctoproject.org/main/builders/nightly-x86-
> > > > 64
> > > > /b
> > > > uilds/680/steps/Building%20Toolchain%20Images/logs/stdio
> > > > 
> > > > > NOTE: Unpacking /home/pokybuild/yocto-autobuilder/yocto
> > > > > -worker/nightly-x86-64/build/scripts/runqemu to
> > > > > /home/pokybuild/yocto-autobuilder/yocto-worker/nightly-x86
> > > > > -64/build/build/tmp/work/i686-nativesdk-pokysdk
> > > > > -linux/nativesdk
> > > > > -qemu-helper/1.0-r9/
> > > > > cp: `/home/pokybuild/yocto-autobuilder/yocto-worker/nightly
> > > > > -x86
> > > > > -64/build/scripts/runqemu' and `/home/pokybuild/yocto
> > > > > -autobuilder/yocto-worker/nightly-x86
> > > > > -64/build/scripts/runqemu'
> > > > > are the same file
> > > > > DEBUG: Python function base_do_unpack finished
> > > > > DEBUG: Python function do_unpack finished
> > > 
> > > 
> > > Despite all tests were successfully passed, something remains
> > > incorrect. Very sensitive feature. I'll check what is wrong and
> > > supply additional test cases with updated patch. Seems I need to
> > > build several configurations locally before submitting such
> > > patches
> > > –
> > > just tests are not enough.
> > 
> > Yes, sadly our tests aren't entirely complete. I'd much appreciate
> > added test cases if we can figure out what the issue is.
> > 
> > This change does introduce a change in behaviour, admittedly one
> > where
> > we think the current behaviour is incorrect. It is possible recipes
> > are
> > relying on that incorrect behaviour though and they may need
> > adjusting.
> 
> FWIW, a:
> 
> -                if urldata.type == "file":
> +                if urldata.type == "file" and not
> urldata.path.startswith("/"):
> 
> did seem to fix the failing recipe which was doing something valid.
> What 
> effect elsewhere this has I haven't checked.

I'm wrong, the correct fix appears to be:

-                    urlpath = urldata.path.rstrip('/') # Trailing '/' does a copying to wrong place
+                    # Trailing '/' does a copying to wrong place
+                    urlpath = urldata.path.rstrip('/')
+                    # Want files places relative to cwd so no leading '/'
+                    urldata.path.lstrip('/')

and we're missing a test case with absolute file:// urls.

Cheers,

Richard