Re: [Openembedded-architecture] Cleanup of WORKDIR by separating do_unpack

2024-04-28 Thread Richard Purdie
One interesting side effect I've noticed with testing the a-c patches
was a failure in meta-virtualization.

The issue is a bbappend to sysvinit which does:

"""
FILESEXTRAPATHS:prepend := "${THISDIR}/files:"

SRC_URI += "file://getty-wrapper"

do_install:append() {
install -d ${D}${base_sbindir}
install -m 0755 ${WORKDIR}/getty-wrapper ${D}${base_sbindir}/getty-wrapper
}
FILES:${PN} += "${base_sbindir}/getty-wrapper"
"""

which breaks after I change S to be != WORKDIR.

The correct fix here is to use UNPACKDIR instead of WORKDIR so it is an
easy fix, just annoying. It does suggest we may need to start with the
patches making that substitution before we can start on the S = WORKDIR
removal.

I'm trying to decide how upset people are going to get if I start
making these changes as I realise that whilst core is effectively done
with scarthgap, other layers will still be working on that.

Cheers,

Richard





-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#1997): 
https://lists.openembedded.org/g/openembedded-architecture/message/1997
Mute This Topic: https://lists.openembedded.org/mt/105739556/21656
Group Owner: openembedded-architecture+ow...@lists.openembedded.org
Unsubscribe: https://lists.openembedded.org/g/openembedded-architecture/unsub 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [Openembedded-architecture] Cleanup of WORKDIR by separating do_unpack

2024-04-28 Thread Richard Purdie
On Sun, 2024-04-28 at 11:10 +0100, Paul Barker wrote:
> On 25/04/2024 22:38, Richard Purdie via lists.openembedded.org wrote:
> > As people probably realise, WORKDIR is a bit of a mess at the moment.
> > do_unpack writes things directly into there and we don't keep track of
> > what it writes there very easily. I know there are source tracer
> > solutions but the whole situation is messy.
> > 
> > I'd like to try and improve things somehow but it isn't anywhere near
> > as easy as people think. The key hidden issues are that:
> > 
> > * release tarballs create the subdir under WORKDIR (BPN-PV)
> > * the git fetcher fetches into a "git" subdir by default
> > * plain file:// referenced files end up in WORKDIR directly.
> > 
> > This means:
> > 
> > * git recipes set S = "${WORKDIR}/git"
> > * tarball release recipes don't usually set S as the default works
> > * do_compile/do_install often reference file from SRC_URI in WORKDIR
> > 
> > Why is any of this a problem? If you have a recipe with SRC_URI =
> > "file://myfile1 file://myfile2", run a build, then remove myfile2 from
> > SRC_URI and rebuild, myfile2 will still be in WORKDIR. There are
> > situations that breaks things.
> > 
> > Put another way, the fetcher can't undo the result of do_unpack, there
> > is no directory it can clean unless S != WORKDIR.
> > 
> > It also results in lots of files in WORKDIR which can be very confusing
> > to people.
> > 
> > I think it is time we tried to start and fix this. It is however going
> > to be a little painful.
> > 
> > My initial proposed steps are:
> > 
> > a) Add UNPACKDIR as a variable, defaulting to WORKDIR and switch
> > do_unpack to unpack there instead of WORKDIR directly.
> > 
> > b) Change recipes that use S = "${WORKDIR}" to
> > 
> > S = "${WORKDIR}/sources"
> > UNPACKDIR = "${S}"
> >  
> > instead. Their do_compile/do_install tasks would need to change
> > ${WORKDIR} -> ${S} if they don't have that already.
> 
> To avoid having to make another change later, we should instead set
> 
> UNPACKDIR = "${WORKDIR}/unpack-sources"
> S = "${UNPACKDIR}"
> 
> Then when the default UNPACKDIR is changed to the above, we can simply
> drop the UNPACKDIR assignment in these recipes.

I did wonder about that. In the end I decided it was probably clearer
leaving things long term as:

"""
S = "${WORKDIR}/sources"
UNPACKDIR = "${S}"
"""

instead of:

"""
S = "${UNPACKDIR}"
"""

since otherwise you get recipes building in UNPACKDIR which I didn't
really want to encourage. This way, if recipes are building under
UNPACKDIR, it at least gets renamed from the default so we have some
kind of marker for that.

To be clear, I worry less about the fact the build happens there but
more how the code in the tasks is written. We want to encourage the
correct usage of UNPACKDIR vs S.

> > c) Make S = "${WORKDIR}" a fatal error
> > 
> > d) Audit do_compile/do_install usage of ${WORKDIR} and change to
> > ${UNPACKDIR}.
> > 
> > e) Add sanity checks to make ${WORKDIR} references in compile/install
> > fatal
> > 
> > f) make do_unpack clean ${UNPACKDIR} before it reruns if it isn't equal
> > to WORKDIR
> > 
> > g) We could consider passing config into the fetcher to change the
> > "git" default to match BPN-PV to simplify things.
> > 
> > 
> > This in theory then sets the stage for us to change UNPACKDIR to be
> > something like ${WORKDIR}/unpack-sources and we could put links in for
> > "${BPN}-${PV}" and 'git' into WORKDIR. We could then always clean
> > UNPACKDIR.
> 
> We could instead change the default for S to "${UNPACKDIR}/${BP}", then
> we shouldn't need links in ${WORKDIR}.

This is going to encourage people to "build" inside UNPACKDIR though
and I'm not convinced that is something we should do. I'd prefer to
keep the directory with a clear purpose.

> At this point, any recipe manually setting UNPACKDIR = "${WORKDIR}"
> should trigger an error.
> 
> > 
> > Every time I think about all this, my head hurts, people complain about
> > having breaking changes and I end up trying to ignore it a bit more. I
> > don't think it is possible to have a completely scripted transition
> > where the end result is actually nice looking, to get better looking
> > layout and recipes, we're going to have to inject a variable like
> > UNPACKDIR.
> > 
> > Thoughts? Any better ideas?
> 
> I'm strongly in favour of making these changes.
> 
> As Mark pointed out in his reply, this may cause some work for layers
> which currently target multiple Yocto releases with one branch - my own
> meta-linux-mainline is an example of this. I think it's justified
> though, it's important that the project can both move forward and
> revisit past decisions which need changing, sometimes that means we have
> to deal with breaking changes.

Agreed. Whilst people have grown used to making layers target multiple
releases, it is not something we've ever said we'd support. We
definitely need to be able to develop and sadly that does mean invasive
changes when necessary.

Re: [Openembedded-architecture] Cleanup of WORKDIR by separating do_unpack

2024-04-28 Thread Paul Barker
On 25/04/2024 22:38, Richard Purdie via lists.openembedded.org wrote:
> As people probably realise, WORKDIR is a bit of a mess at the moment.
> do_unpack writes things directly into there and we don't keep track of
> what it writes there very easily. I know there are source tracer
> solutions but the whole situation is messy.
> 
> I'd like to try and improve things somehow but it isn't anywhere near
> as easy as people think. The key hidden issues are that:
> 
> * release tarballs create the subdir under WORKDIR (BPN-PV)
> * the git fetcher fetches into a "git" subdir by default
> * plain file:// referenced files end up in WORKDIR directly.
> 
> This means:
> 
> * git recipes set S = "${WORKDIR}/git"
> * tarball release recipes don't usually set S as the default works
> * do_compile/do_install often reference file from SRC_URI in WORKDIR
> 
> Why is any of this a problem? If you have a recipe with SRC_URI =
> "file://myfile1 file://myfile2", run a build, then remove myfile2 from
> SRC_URI and rebuild, myfile2 will still be in WORKDIR. There are
> situations that breaks things.
> 
> Put another way, the fetcher can't undo the result of do_unpack, there
> is no directory it can clean unless S != WORKDIR.
> 
> It also results in lots of files in WORKDIR which can be very confusing
> to people.
> 
> I think it is time we tried to start and fix this. It is however going
> to be a little painful.
> 
> My initial proposed steps are:
> 
> a) Add UNPACKDIR as a variable, defaulting to WORKDIR and switch
> do_unpack to unpack there instead of WORKDIR directly.
> 
> b) Change recipes that use S = "${WORKDIR}" to
> 
> S = "${WORKDIR}/sources"
> UNPACKDIR = "${S}"
>  
> instead. Their do_compile/do_install tasks would need to change
> ${WORKDIR} -> ${S} if they don't have that already.

To avoid having to make another change later, we should instead set

UNPACKDIR = "${WORKDIR}/unpack-sources"
S = "${UNPACKDIR}"

Then when the default UNPACKDIR is changed to the above, we can simply
drop the UNPACKDIR assignment in these recipes.

> 
> c) Make S = "${WORKDIR}" a fatal error
> 
> d) Audit do_compile/do_install usage of ${WORKDIR} and change to
> ${UNPACKDIR}.
> 
> e) Add sanity checks to make ${WORKDIR} references in compile/install
> fatal
> 
> f) make do_unpack clean ${UNPACKDIR} before it reruns if it isn't equal
> to WORKDIR
> 
> g) We could consider passing config into the fetcher to change the
> "git" default to match BPN-PV to simplify things.
> 
> 
> This in theory then sets the stage for us to change UNPACKDIR to be
> something like ${WORKDIR}/unpack-sources and we could put links in for
> "${BPN}-${PV}" and 'git' into WORKDIR. We could then always clean
> UNPACKDIR.

We could instead change the default for S to "${UNPACKDIR}/${BP}", then
we shouldn't need links in ${WORKDIR}.

At this point, any recipe manually setting UNPACKDIR = "${WORKDIR}"
should trigger an error.

> 
> Every time I think about all this, my head hurts, people complain about
> having breaking changes and I end up trying to ignore it a bit more. I
> don't think it is possible to have a completely scripted transition
> where the end result is actually nice looking, to get better looking
> layout and recipes, we're going to have to inject a variable like
> UNPACKDIR.
> 
> Thoughts? Any better ideas?

I'm strongly in favour of making these changes.

As Mark pointed out in his reply, this may cause some work for layers
which currently target multiple Yocto releases with one branch - my own
meta-linux-mainline is an example of this. I think it's justified
though, it's important that the project can both move forward and
revisit past decisions which need changing, sometimes that means we have
to deal with breaking changes.

And to check I understand the consequences of this:

* with do_unpack[cleandirs] = ${UNPACKDIR}, re-running do_unpack would
  delete ${UNPACKDIR} before re-populating it based on the recipe's
  current SRC_URI. So if ${S} is under ${UNPACKDIR}, this will be
  removed and re-populated. For recipes which do in-tree builds or
  otherwise modify ${S} during compilation, this will effectively reset
  ${S} to the state it was in before do_patch/do_configure/do_compile.

* do_install, do_package, etc have cleandirs set, so they will similarly
  have their directories reset when they re-run and will not see files
  from a previous build.

* do_configure & do_compile do not have cleandirs set - so if ${B} !=
  ${S}, contents of ${B} from a previous do_configure/do_compile may
  affect the new build even if we have forced do_unpack to re-run.

This leaves me wondering, if we're taking on a breaking change, do we
also want to address ${B} at the same time? Should ${B} always be placed
under ${UNPACKDIR}? Or should we set do_configure[cleandirs]?

Thanks,

-- 
Paul Barker


OpenPGP_0x27F4B3459F002257.asc
Description: OpenPGP public key


OpenPGP_signature.asc
Description: OpenPGP digital signature

-=-=-=-=-=-=-=-=-=-=-=-
Links: You