On Wed, 2024-05-15 at 17:42 +0000, Peter Kjellerstedt wrote: > > -----Original Message----- > > From: [email protected] > > <[email protected]> On Behalf Of Richard Purdie > > Sent: den 15 maj 2024 13:56 > > To: [email protected] > > Subject: [OE-core] [PATCH 05/10] base: Switch UNPACKDIR to a subdir of > > WORKDIR > > > > Change do_unpack to unpack files to a subdirectory of WORKDIR instead of > > WORKDIR > > itself. There are several good reasons for this but it is mainly about > > being able > > to isolate the output of the unpack task and tell the files apart from > > other things > > which are created in workdir (logs, sysroots, temp dirs and more). > > > > This means that when the do_unpack task reruns, we can clean UNPACKDIR and > > know > > we have a standard point to start builds from. > > > > It also makes code in tools like devtool and recipetool easier. > > > > To reduce the impact to users, if a subdirectory under UNPACKDIR matches > > the first subdirectory under WORKDIR of S, that directory is moved into > > position > > inside WORKDIR. This preserves the behaviour of S = "${WORKDIR}/git", > > S = "${WORKDIR}/${BPN}" and other commonly used source directory setups. > > > > The directory is moved since sadly many autotools based projects can't cope > > with > > symlinks in their paths. > > Would it be an option to create a symbolic link by default, and instead > let the autotools bbclass replace it with a moved directory? If it is > in fact only autotools based projects that have this problem.
Maybe. I've reached the point with this where I really didn't want to try and explore that particular set of problems and I'd rather things were consistent. I'm not going to rule out trying to clean up and improve this somehow in the future but for now, it works, it solves the original major problem (cleanup of obsolete sources) and we have most of the major pieces like devtool working. It also doesn't look as different to users as moving S would from a documentation perspective. I'm a bit torn on what the end result for S should look like. I'm happy enough with the changes so far and I think the best thing to do is merge these, then consider if we do want to go further or not. > > Signed-off-by: Richard Purdie <[email protected]> > > --- > > meta/classes-global/base.bbclass | 21 ++++++++++++++++++--- > > meta/conf/bitbake.conf | 2 +- > > 2 files changed, 19 insertions(+), 4 deletions(-) > > > > diff --git a/meta/classes-global/base.bbclass b/meta/classes- > > global/base.bbclass > > index 066f3848f7c..cdce0538273 100644 > > --- a/meta/classes-global/base.bbclass > > +++ b/meta/classes-global/base.bbclass > > @@ -153,20 +153,35 @@ python base_do_fetch() { > > } > > > > addtask unpack after do_fetch > > -do_unpack[dirs] = "${UNPACKDIR}" > > - > > -do_unpack[cleandirs] = "${@d.getVar('S') if > > os.path.normpath(d.getVar('S')) != os.path.normpath(d.getVar('WORKDIR')) > > else os.path.join('${S}', 'patches')}" > > +do_unpack[cleandirs] = "${UNPACKDIR}" > > > > python base_do_unpack() { > > + import shutil > > + > > src_uri = (d.getVar('SRC_URI') or "").split() > > if not src_uri: > > return > > > > + sourcedir = d.getVar('S') > > + basedir = None > > + workdir = d.getVar('WORKDIR') > > + unpackdir = d.getVar('UNPACKDIR') > > + if sourcedir.startswith(workdir) and not > > sourcedir.startswith(unpackdir): > > + basedir = sourcedir.replace(workdir, '').strip("/").split('/')[0] > > + bb.utils.remove(sourcedir, True) > > This remove() seems wrong and should not be needed. There are two > cases here: > > 1) either ${S} == ${WORKDIR}, in which case the above will remove > ${WORKDIR}, which is sure to lead to problems, or Which is why S = WORKDIR has to become a hard error. We have to drop support for it. > 2) ${S} == ${WORKDIR}/foo[/...], in which case the removal of > workdir + '/' + basedir below will also remove ${S} as > basedir == "foo". You're right. This code did go through several iterations as I found all the weird corner cases this needs to support. Ultimately, I suspect it should probably unconditionally remove ${S}. I'd need yet another test run to try and identify if that causes any new weird issues though. > > > + if basedir: > > + bb.utils.remove(workdir + '/' + basedir, True) > > + > > try: > > fetcher = bb.fetch2.Fetch(src_uri, d) > > fetcher.unpack(d.getVar('UNPACKDIR')) > > except bb.fetch2.BBFetchException as e: > > bb.fatal("Bitbake Fetcher Error: " + repr(e)) > > + > > + if basedir and os.path.exists(unpackdir + '/' + basedir): > > + # Compatibility magic to ensure ${WORKDIR}/git and ${WORKDIR}/${BP} > > + # as often used in S work as expected. > > + shutil.move(unpackdir + '/' + basedir, workdir + '/' + basedir) > > } > > > > SSTATETASKS += "do_deploy_source_date_epoch" > > diff --git a/meta/conf/bitbake.conf b/meta/conf/bitbake.conf > > index b2c500d8739..75c850760f6 100644 > > --- a/meta/conf/bitbake.conf > > +++ b/meta/conf/bitbake.conf > > @@ -405,7 +405,7 @@ STAMP = > > "${STAMPS_DIR}/${MULTIMACH_TARGET_SYS}/${PN}/${PV}" > > STAMPCLEAN = "${STAMPS_DIR}/${MULTIMACH_TARGET_SYS}/${PN}/*-*" > > BASE_WORKDIR ?= "${TMPDIR}/work" > > WORKDIR = "${BASE_WORKDIR}/${MULTIMACH_TARGET_SYS}/${PN}/${PV}" > > -UNPACKDIR ??= "${WORKDIR}" > > +UNPACKDIR ??= "${WORKDIR}/sources-unpack" > > "sources-unpack" does not exactly fall off the tongue. > Would "unpack" or "unpacked" be alternatives? I don't feel they're as clear... Cheers, Richard
-=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#199424): https://lists.openembedded.org/g/openembedded-core/message/199424 Mute This Topic: https://lists.openembedded.org/mt/106112374/21656 Group Owner: [email protected] Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
