Re: [bitbake-devel] 'vendor' fetching discussion cont.

[Stefan Herbrechtsmeier <stefan.herbrechtsmeier-oss@weidmueller.com>]

[-- Attachment #1: Type: text/plain, Size: 22636 bytes --]

Am 17.02.2025 um 18:29 schrieb Richard Purdie:
> FWIW I did try and discuss this in the OE TSC meeting today but it is a
> US holiday. There were only two people who showed up.

I answer your points below but does it make sense to create a RFC to 
show how an alternative source URI management in oe-core could look 
like. I could demonstrate my approach and you could perhaps illustrate 
your concerns in the code.

> On Mon, 2025-02-17 at 17:38 +0100, Stefan Herbrechtsmeier wrote:
>>   Am 17.02.2025 um 13:43 schrieb Richard Purdie:
>>   On Mon, 2025-02-17 at 12:00 +0100, Stefan Herbrechtsmeier wrote:
>>>>   Am 13.02.2025 um 11:43 schrieb Richard Purdie via lists.openembedded.org:
>>>>> e) add the ability to add custom hooks in the fetch process to
>>>>> handle
>>>>> the cases of needing to alter the flow for patching the components
>>>>> list
>>>>>   
>>>> I fear this isn't easy possible and we have a design problem. Bitbake
>>>> and oe-core assume that a patch doesn't influence the fetch and that
>>>> the SRC_URI contains all fetched sources. Many code use the Fetch
>>>> class or the SRC_URI direct and doesn't expect recursive implicit
>>>> URIs of gitsm. In reality the SRC_URI describe the given URIs only
>>>> and doesn't contain the additional implicit URIs. The task flow
>>>> applies patches after the unpack and prevent patches to the gitsm. In
>>>> reality a patch could influence the implicit URIs and isn't
>>>> independent.
>>>>
>>>> I would recommend to keep the patching outside of bitbake and instead
>>>> handle the implicit URIs inside oe-core. We could write the implicit
>>>> URIs into files in the work directory and migrate the scattered
>>>> direct users of the SRC_URI / Fetch class to a common oe-core
>>>> function. This functions returns the given URIs and optional any
>>>> additional implicit URIs. This allows the usage of the task
>>>> dependencies to ensure that the implicit URIs are resolved before
>>>> use.
>>>>
>>>> The package manager lock file needs a single unpack and patch step
>>>> whereas the gitsm needs a recursive unpack and patch of every
>>>> successive gitsm. This makes it impossible to use additional tasks.
>>>> We could add the resolve of the implicit URIs to the fetch task to
>>>> support recursive resolve (gitsm).
>>>>   
>>>> 1. download SRC_URIs
>>>> 2. exit if no URI is marked as unresolved
>>>> 3. unpack unresolved URIs
>>>> 4. apply associated patches
>>>> 5. resolve URIs
>>>> 6. goto 2
>>>>   
>>>> This will eliminate the need for the partial implicit URI and gitsm
>>>> support in bitbake and makes the fetcher code simpler. The patching
>>>> could remain in oe-core and patches could be applied to gitsm and
>>>> package manager lock files. Additionally we remove code duplication
>>>> and simplify future changes to the SRC_URI determination.
>>>>
>>>> It will add an indirection for the SRC_URI but the current code
>>>> already shows that this makes problems. Only the archiver class uses
>>>> the expanded_urldata function and all other classes including the
>>>> spdx class ignore the implicit SRC_URIs.
>>> I'm a bit worried we're getting caught up by the terminology. We don't
>>> necessarily have to "patch" the component list so much as allow
>>> something like a function to hook and adjust things. That then removes
>>> the need for it to be a specific task.
>>>
>>   We can use a common patch without a do_patch task. We could even
>> call the patch code from the do_patch without an extra task. I
>> propose to call the code from the do_patch task in the do_fetch task.
>> It doesn't matter if we use a hook or task. We should minimize the
>> work for the user. The common solution to manipulate a package
>> manager lock file is a patch file and we have the code to handle
>> patch files.
> One of the challenges is the "patch" code/module lives in OE-Core. That
> means the fetcher can't call into it.

That's the reason I would restrict the fetcher to the plain fetch.

> I appreciate this means you'll argue the fetcher code should move to
> OE, or the vendor code should be in OE, however I am pretty decided
> that is a really bad idea. Whether I can convey why, I don't know but I
> have tried.

I argue that the fetcher should focus on the download and unpack of the 
provides URIs. I don't understand why the parsing of a source file and 
the preparation of a cache or vendor folder belongs to the fetcher. The 
fetcher downloads and unpack the provide URIs. The fetcher doesn't care 
if the provided URIs are static defined inside the recipe or implicit 
generated from a source file.

At the moment a lot of code inside oe-core assume that the SRC_URI 
contains all source URIs. This is already wrong because gitsm expand the 
SRC_URI. Only the archiver bitbake class use the SRC_URI in the expanded 
way. This means we have to fix a lot of users anyway. The gitsm approach 
embed the unpack step into the fetcher and avoid a patch step between 
the unpack and parsing of the source. This works for git because the 
destsuffix is known. In case of an archive we have to provide the 
relative directory inside the archive. This means we have to use 
variables inside the fetcher commands, pass more function arguments or 
extent the SRC_URI parameters. The alternative is to introduce implicit 
URIs. The SRC_URI only contains the static defined URIs from the recipe 
and we introduce a new API to collect the dynamic implicit URIs or all 
URIs together. The URIs are saved inside a fixed folder inside the 
WORKDOR. Its like an .inc file but the file is generated on-the-fly. The 
URIs still use the fetcher for download an unpack but we have a separate 
step for the SRC_URI and the implicit URIs. We have different 
possibilities to combine and distribute the steps onto tasks or 
postfuncs. We could add tasks to resolve, fetch, unpack and patch the 
implicit sources after the do_unpack. We could add tasks to fetch, 
unpack and patch the sources from the SRC_URI or only parts of it before 
the do_fetch or we could integrate the steps into the do_fetch. We could 
reuse the output from an previews task or use a fresh start like a plain 
unpack and patch of all sources. We could reuse the code to provide a 
manual update step to create a inc file. We need to improve the usage of 
the SRC_URI in any case because it is used like any other variable and 
it is unclear that an entry inside the SRC_URI could be expanded to 
multiple URIs.

> I also very strongly want to maintain our separation of do_fetch,
> do_unpack and do_patch.

What does this mean? The do_fetch of embedded dependencies requires an 
unpack. Therefore we have to split the tasks or disallow any dynamic. 
The oe-core solution allows a full separation whereas the bitbake 
solution require duplication inside bitbake.

>>> I'm also not sure that drawing gitsm into this is a great idea when it
>>> is effectively already working  quite well.
>> To my knowledge it isn't possible to patch a sub module revision. Is
>> it intended that the SBOM doesn't contain the recursive implicit git
>> repositories?
> A patch was merged to allow the data about implicit git repos (amongst
> other things) to be available:
>
> https://git.yoctoproject.org/poky/commit/bitbake/?id=ef3e46afd910d4b7727d42c4c18b501525c65695
>
> That tracer code was added after a lot of discussion that took up a ton
> of my time, then people moved on and it hasn't seem much
> development/use :(.
>
> So there were mechanisms added to make the data available.

Why was this needed? We already have a function for the fetchers to 
provide the expand URLs.

>>>   I appreciate the desire to
>>> have neat abstractions applying accross everything equally but I'm not
>>> sure we can achieve that with the level of usability we need and I'd
>>> rather improve usability at the cost of the inclusion of gitsm, which
>>> is the most functional fetcher we currently have in this group.
>> I include gitsm because it is similar to the package manager and have
>> some drawbacks which I solve with my approach. It is the only fetcher
>> that parse a file to determine additional URIs. Its integration into
>> oe-core is sub-optimal.
> I believe there was a way added to solve the drawback you mention. The
> same mechanism could be used for other fetchers too yet we're talking
> about much more radical re-designs.

Do you have a hint for that way so that I can look into it.

>>> I will repeat again that I do strongly feel that a strong API in
>>> bitbake is going to lead to an overall better design that something
>>> bolted onto the fetcher in OE-Core.
> The "fetcher" is an API exposed by Bitbake with the intent of
> standardising the fetching of source code. It's aim is to support
> objectives such as allowing resilience through source mirrors and
> support build reproducibility. There is a document in bitbake which
> tries to at least summarise some of that after it was requested it be
> written down.

What is the fetching of the source code and why doesn't the oe-core 
solution doesn't fit? It uses the fetcher to fetch the sources. The only 
different is that it pass the URIs explicit and doesn't fetch the 
SRC_URI only.

>> What is a fetcher? We have fetchers for protocols (wget, git),
> These were the original fetchers using the original APIs.
>
>> wrapper around other fetchers (crate, gomod and gomodgit)
> These are newer and some work better than others are people have
> attempted to git the newer languages into the older framework.

But this is possible without a fetcher. The crate, gomod and my npm 
fetcher mainly override the urldata_init and unpack function. The same 
functionality can be provided by a python function to define the SRC_URI 
(replace urldata_init function) and a task or postfunc for unpack 
(replace unpack function). The advantage is a plain URL in the SRC_URI 
and the cache or vendor folder handling isn't scattered over a fetcher 
and bbclass. If we use a oe library for the important code we could 
simplify the tests because we could test individual steps. Even the 
debugging could be simpler if we use a separate task to populate the 
cache or vendor folder from the downloaded artifacts. The appropriate 
code is already in my oe-core series.

>>   and wrapper around other fetchers which embedded file parser (gitsm,
>> npmsw).
> Of these, gitsm is now relatively successfully integrated and
> functional.

Doesn't Bruce have problems with it and replace it by plain git URIs?

I think the main part of the successful integration is that you doesn't 
need any manual step to fetch the embedded dependencies. The main 
difference to the package managers is that it is uncommon to use patches 
for gitsm. Every sub module defines the version of its sub module 
whereas a package only defines it compatibility. The package versions 
are defined inside the main source.

>>   Thereby the gitsm fetcher fetch a source and parse the embedded
>> file. The npmsw fetcher parse a file from the meta layer. The gitsm
>> fetcher recursive combines the download and unpack function call in
>> mostly every class function.
> Sadly there did not seem to be any other way to handle that. I'd
> imagine that some of the other fetchers are going to have to work
> similarly, unfortunately.
The problem of the bitbake fetcher is the unclear caching and the 
missing dependency between the steps. This is the advantage of oe-core. 
We can simple arrange everything which need this information after the 
resolve task.

>> The wrappers has the disadvantage that they use uncommon URIs which
>> are useless for the SBOM. We support PyPI without the need for an
>> extra fetcher.
> We support pypi in so much as we have a recipe per dependency. That was
> not deemed possible for npm or crates.
That's no reason for or against a pypi fetcher or a crate_src_uri function.

> I would disagree that uncommon
> URIS are useless for SBOMs. They can be problematic but having the data
> there means it is at least listed.
But it reduce the usability of the SBOM.

>   There are also ways they can be
> extended (e.g. the intercept code I link to above).
>
>> Is PyPI or crate the desired way to go?
> Compared to the complexity in some of the patches proposed, I'd
> actually say yes.
What complexity do you mean? Some of the complexity of my patches comes 
from the limitation and fixed use cases of the existing code.

The gomod fetcher contain a feature to extract the .mod from a .zip 
file. This isn't needed because the go.sum file contains entries for the 
.mod and .zip file. The compatible solution would be do download the 
.mod and .zip file.

The crate fetcher implements its own unpack code. It behavior depends on 
the BP variable. The unpack function writes a meta file which is need to 
use the source inside the vendor folder. This makes it impossible to 
reuse the code for plain URIs. Alternative the URI could be marked as 
crate vendor source and an extra step could create the meta file for all 
marked URIs.

>> nstead of expand the fetcher I would focus the fetcher on the
>> download of provides URIs and keep the file parsing and post unpack
>> processing outside of it.
> The challenge is that once you process it, you need to fetch more
> things. Our ecosystem is built on the idea that fetching happens in
> do_fetch. If we change that most mirroring scripts break for example.
> It is like compiling happening during do_configure or do_install - it
> is not what the user expects and that is problematic. I am against
> changing behaviour in ways users do not expect.

I don't change the exception. The exceptions are the reason for the 
state of the series.

The missing point is that the expectation is related to the end of a 
dependency chain and not to a task itself. In any case the real 
expectation isn't clear. What happens if the configuration need a 
compiled source and it isn't feasible to create a native package. My 
series satisfy the following expectations:

* All sources are downloaded after the do_fetch
* All sources are unpacked after the do_unpack
* All sources are patched after the do_patch

The series contains one possible solutions. There are multiple other 
solutions to reach the same inside oe-core by distributing the following 
steps over task:

run_early_fetch - Download the source with the lock files from SRC_URI
run_early_unpack - Unpack the source with the lock files from SRC_URI 
(after run_early_fetch)
run_early_patch - Apply patch which manipulate the lock files from 
SRC_URI (after run_early_patch)
run_resolve - Parse the lock files to resolve the dependency sources 
(after run_early_patch)
run_vendor_fetch - Download the dependency sources (after run_resolve)
run_vendor_unpack - Unpack the dependency sources (after run_vendor_fetch)
run_vendor_patch - Patch the dependency sources (after run_vendor_unpack)
run_fetch - Download the additional source from SRC_URI
run_unpack - Unpack the additional source from SRC_URI (after run_fetch)
run_patch - Apply the additional source from SRC_URI (after run_unpack)

Some of the steps could be combined, repeat or integrated into other 
task. For example the run_early_fetch and run_fetch are a simple fetch 
of the SRC_URI list whereas the run_early_fetch alone is a fetch of a 
filtered SRC_URI list.

Series:

do_early_fetch : run_early_fetch
do_early_unpack : run_early_unpack
do_early_patch : run_early_patch
do_resolve : run_resolve
do_fetch : run_fetch, run_early_fetch, run_vendor_fetch
do_unpack: run_unpack, run_early_unpack, run_vendor_unpack
do_patch: run_patch, run_early_patch, run_vendor_patch

WIP:

do_fetch : run_fetch, 
run_early_fetch, run_early_unpack, run_early_patch, run_resolve, 
run_vendor_fetch
do_unpack: run_unpack, run_early_unpack, run_vendor_fetch
do_patch: run_patch, run_early_patch, run_vendor_patch

The bitbake approach looks like the WIP but makes the run_early_patch 
complicated or impossible.

I prefer something like the first approach because it is similar to the 
.inc file approach and the separate tasks are easier to debug. The 
do_fetch, do_unpack and do_patch task works like before. The only 
difference is that the implicit URIs are managed outside of the fetcher. 
At the moment this only influence a single user outside of the fetch, 
unpack and patch tasks but we could use the opportunity to fix other 
users as well (ex. SBOM).

>> The package manager lock file is a simple file which references to
>> package sources. Thereby a main source could contain multiple package
>> manager lock files. At the moment we have no fetcher which reference
>> an other source as base or combines multiple fetchers.
> This is the case for good reason. If the lock file references all the
> sources, perhaps we should just include it alongside the recipe and be
> done with it?

This is possible but the user have manual copy the lock file from the 
source to the meta layer, manual apply patches to it and ensure that the 
source and lock file are in sync.

Why is the bitbake fetcher the correct place to parse a lock file?

Why is it a bad idea to generate and manage the URIs outside of bitbake?

>> Based on my two proposed solutions and the work the last month, I
>> have the following feasible requirements:
>>   * Support patches for the package manager lock file to simply
>       updates, back ports and upsteaming
>>   * Support multiple package manager lock files per main source
>>   * Include the real https or git URI in the SBOM
>>   * Include the dependency name and version in the SBOM
>>   * No manual bitbake command call after a SRC_URI, PV or SRCREV
>> change
> I think we may have to relax some of these requirements as we can't do
> everything.
>> The following code is mostly used in oe-core and ignore the implicit
>> URIs:
>>
>>      src_uri = (d.getVar('SRC_URI') or "").split()
>>      fetcher = bb.fetch2.Fetch(src_uri, d)
>>      for url in fetcher.urls:
>>
>> The following code is used in the archiver class only and includes
>> the implicit URIs:
>>
>>      src_uri = get_src_uris(d)
>>      fetcher = bb.fetch2.Fetch(src_uri, d)
>>      for ud in fetcher.expanded_urldata():
>   
> Ironically, I think this an example of the kind of "strong API" I'm
> referring to, where a standard API is exposed by bitbake. In some ways
> this was already attempted with the tracer API above where the
> expand_urldata call would actually be a call into the tracer API to
> obtain the actual urls used.

Sorry but I think the tracer API isn't needed because we have already 
all information and possible functions inside the fetcher.

This "strong API" make it complicated to patch the source. Either the 
API depends on bitbake variables or it depends on the provides hooks. 
This means if somebody miss a hook or a bitbake variable he will get an 
other output.

The primary function of the fetcher is to download and unpack of the 
sources. Why do we use it for management of the source URIs? Why don't 
we manage the source URIs outside of bitbake? We can save the source 
URIs inside the WORKDIR and provide an additional common api to it. At 
the moment every user create a new instance of the Fetch object and in 
many cases manual pass the SRC_URI to it:

     for url in oe.fetcher.get_source_uris(d):

We could add a new task to create the files and allows the user to 
inspect it. I could provide a RFC if it helps the discussion.

>> My last series enable oe-core to generate implicit URIs and therefore
>> replace the usage of the plain SRC_URI with a function:
>>
>>       fetcher = bb.fetch.Fetch(get_src_uris(d), d)
>>      for url in fetcher.urls:
>>
>>   The SRC_URI contains the given URIs and the get_src_uri function
>> contains the given and implicit URIs. The concept is taken from other
>> classes like the spdx classes which use files to exchange data
>> between tasks.
> I believe the get_src_uris() functionality should happen within the
> fetcher module and not outside of it as a bolt on interface.

It isn't bolt on the interface. It provides an interface on top of the 
fetcher. The fetcher download and unpack the source but the sources are 
managed inside oe-core. At the moment the management doesn't really 
exist because the sources are only consists of the SRC_URI variable.

> I do not know where we go from here. You disagree with me on several
> key things

It's not about consent, it's about trying to understand the reasons. 
I've been working on this topic for years and have tried out different 
options. I think I have a solution that is very easy to use. In order to 
improve the solution, I try to understand the reasons for the other 
decisions. I don't understand why it is desired to add code to the 
fetcher which works independent of network access and why it is desired 
to create a fetcher object to receive the list of source URIs.

>   such as whether the code should be in bitbake or OE-Core,
I have the fear that the bitbake solution limit the possibilities 
without an advantage. The complexity depends on the feature not on the 
point of implementation.

> whether gitsm should be changed,

I use gitsm as a benchmark to evaluate concepts and approaches. It is 
better known as a lock file and a complex use case because of the 
recursive sources. It doesn't matter if we change it but the selected 
approach should support a replacement without limitations or clearly 
state why it isn't supported.

>   whether we need to have some kind of
> checksum to verify the "lockfile" is correct as originally intended in
> the recipe and probably more.
I want to prevent us from having the wrong motivation due to a lack of 
understanding of the lock file. Why don't we add a look file across all 
expanded URIs, regardless of the source of the URI, because we could 
have the same problems in the other fetchers.

> I'm in the position of trying to mediate things (i.e. see the different
> viewpoints and bring people together, trying to find common ground) and
> yet at the same time, express views as someone who as spent a lot of
> years trying to maintain and improve this code.
I appreciate your work.

But I sometimes have the feeling that the code is a patchwork quilt and 
every attempt to replace several patches with something common is met 
with rejection.

> I have tried to seek help from the people who effectively oversee me as
> a maintainer (i.e. the TSC) with limited success. I'm not sure what to
> do from here.

I really like to upstream my work because I think it would simplify the 
package manager support, but there doesn't seem to be much interest in 
the community for it.

[-- Attachment #2: Type: text/html, Size: 31262 bytes --]