All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier-oss@weidmueller.com>
To: Richard Purdie <richard.purdie@linuxfoundation.org>,
	bitbake-devel@lists.openembedded.org
Subject: Re: [bitbake-devel] [RFC PATCH 00/15] Make mirror replacement syntax explicit
Date: Thu, 20 Feb 2025 18:37:34 +0100	[thread overview]
Message-ID: <07c3ebb6-8cdf-416a-bc89-ab930d85e78d@weidmueller.com> (raw)
In-Reply-To: <9ecb0289143f35037902a569de8a7f048262f779.camel@linuxfoundation.org>

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

Am 20.02.2025 um 13:21 schrieb Richard Purdie:
> On Thu, 2025-02-20 at 12:45 +0100, Stefan Herbrechtsmeier via 
> lists.openembedded.org wrote:
>> Am 20.02.2025 um 11:22 schrieb Richard Purdie via lists.openembedded.org:
>>> On Wed, 2025-02-05 at 13:12 +0100, Stefan Herbrechtsmeier wrote:
>>>>   Am 05.02.2025 um 11:34 schrieb Richard Purdie:
>>>>   On Wed, 2025-02-05 at 08:15 +0100, Stefan Herbrechtsmeier via lists.openembedded.org wrote:
>>>> I’m open for suggestions. Even ARCHIVE or TARBALL are hard to
>>>> understand because it is only a relative path on the download mirror.
>>>> Alternative we can mark the lines as upstream or download mirror and
>>>> give the replacement different meanings. The path could be the
>>>> original PATH for an upstream mirror or the relative path of the
>>>> downloaded file for the download mirror.
>>> I've been giving this topic some thought. One idea I wondered about was
>>> to instead markup the mirror urls with how they're expected to work
>>> with a new parameter. For example:
>>> git://.*/.*http://downloads.yoctoproject.org/mirror/sources/?mirrorformat=mirrortarball
>> The ? could be problematic because it is the separator for the query. 
>> It is unlikely that the user really use this query parameter but it 
>> could complicate the code because we have to handle additional query 
>> parameters.
>> What does the "?mirrorformat=mirrortarball" mean? Will it work like a 
>> MIRRORTARBALL replacement?
>
> The mirrorformat parameter would be used by the mirroring code itself 
> to understand how to handle the url.

What is the different to a MIRRORTARBALL replacement? The code will 
replace the word with the content.

> It would be dropped from the modified url so is only therefore our 
> code's use. If there are additional parameters they would be passed 
> through as they are now.
>
>> How does a simple replacement should look like?
>>
>> http://  https://
>>
>> Because of the backward compatible this will replace the basename of 
>> the path.
>
> It would depend how the mirror is laid out. Some mirrors flatten the 
> urls like DL_DIR is laid out, some potentially don't. The standard 
> usage would likely have a mirrorformat=dldir parameter added.

How does the user specify an entry that replace the http scheme with 
https and keeps everything else like it is (upstream mirror)?

>>> The possible options would be something like:
>>> mirrortarball - mirror tarballs taken from DL_DIR
>>> flattened - copy of DL_DIR so DL_DIR layout (maybe call it dldir?)
>>> upstream - layout is the same as the upstream directory structure so a direct url replacement
>> Do you think we have to handle the mirror tarball explicit? The 
>> mirror tarball is required for a scheme change.
>
> If we do that, we can avoid having to guess at too many urls to test 
> to figure out a mirror format so I think it would be an improvement on 
> where we are today.

Do you mean we will test if the URL have a mirrortarball and if not skip 
the entry?

>
>>> If using a mirrortarball mirror url, we'd know to use the values from
>>> urldata.mirrortarballs. We could add parameters to the fetcher to have
>>> two parameters, one will be the DL_DIR path and the other would be the
>>> upstream url path.
>> I don't understand where this is needed, because the mirror tarball 
>> and downloadfilename are used by different fetchers.
>
> Please keep in mind that downloadfilename is pretty much a misfeature. 
> It was added as we couldn't control collisions inside dl_dir but it 
> creates all kind of other problems. I think we do need to handle that 
> problem case but it does then mean we have to indicate whether any 
> given mirror uses "dldir" or "upstream" names and paths.

I don't understand the problem. The download mirror will use the 
downloadfilename or its default the basename of the localpath. The 
upstream mirror will use the path. The mirrortarball will use the 
mirrortarball. Why the fetcher need two parameters?

>>> One key question I have is how we might need to
>>> shorted the url path for some mirror urls to add/remove a path prefix
>>> in the mirroring.
>> What do you mean by this? The downloadfilename could contain a path 
>> without any problem after my change.
>
> See above, downloadfilename is not something I'm keen to promote and 
> is creating several of the problems we have by badly trying to hack 
> extra functionality onto the fetcher without thinking through all the 
> issues like mirroring.

What is the desired way to avoid name clashes? The package manager 
fetcher need an generic way to override the basename.

> This is about the fact that you could have:
>
> htttps://some.server/some/deep/multi/level/path/
>
> and a mirror of things there at:
>
> htttps://some.server/shortpath/path/
>
> so the mirror urls need to be able to map
>
> htttps://some.server/some/deep/multi/level/path/a/b/c.tgz

This could be solved if we use my proposed code.

> to
>
> htttps://some.server/shortpath/path/a/b/c.tgz

htttps://some.server/some/deep/multi/level/path/ 
htttps://some.server/shortpath/path/

> but also:
>
> htttps://some.server/shortpath/path/c.tgz

htttps://some.server/some/deep/multi/level/path/.* 
htttps://some.server/shortpath/path/DOWNLOADFILENAME

> or possibly
>
> htttps://some.server/shortpath/path.d.tgz

> if downloadfilename is in action.

Because the fallback for the downloadfilename is the basename the same 
entry works.

htttps://some.server/some/deep/multi/level/path/.* 
htttps://some.server/shortpath/DOWNLOADFILENAME

>>> I think that would cover most of our scenarios and make the mirror urls
>>> much more useful/explicit.
>>> It also would give us a migration path as the code can simply error if
>>> it sees a mirror url without a mirrorformat parameter. This may mean we
>>> could have a clean slate for the mirror urls and switch to one clear
>>> format. I think we can make a case for such a breaking architecture
>>> change.
>> Does this means we doesn't need a backward compatibility?
>
> If we can come up with sufficient justification to break things and 
> can detect and error correctly for old usage and get this signed off 
> by the TSC/community, potentially, yes.

Okay

>> Alternative we could make some replacement mandatory if a wildcard is 
>> used to detect obsolete entries.
>
> I don't understand that.

If we have a wildcard .* in the path we need to know how to replace it. 
This could be the PATH, BASENAME, DOWNLOADFILENAME, MIRRORARCHIVE or re 
group. But in case of the re group this could still be an old entry.

Do support all cases we need a fix prefix or delimiter:

r:http https

http#https
http|https

http?://.*/.*|http://downloads.yoctoproject.org/mirror/sources/DOWNLOADARCHIVE
git://.*/.*|http://downloads.yoctoproject.org/mirror/sources/MIRRORARCHIVE

> I do think we have too many problems in the existing mirroring url 
> mapping and we probably need to rework this rather than try and pile 
> more patches into it and complicate it further.
I have already rework it. If I can remove the backward compatibility and 
replace it with an error this would simplify the code. I only need a 
better name for the DOWNLOADFILENAME (DOWNLOADARCHIVE) and add the 
MIRRORARCHIV.

> The question is whether the proposal fixes the issues it needs to and 
> has enough simplification and benefit to justify making the change.
My patches support folders in the downloadfilename, upstream mirrors and 
renames. I have to rework the MIRROR strings but therefore the commented 
out tests work.

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

  reply	other threads:[~2025-02-20 17:37 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-02-05  7:15 [RFC PATCH 00/15] Make mirror replacement syntax explicit Stefan Herbrechtsmeier
2025-02-05  7:15 ` [RFC PATCH 01/15] fetch2: remove unnecessary expand function calls Stefan Herbrechtsmeier
2025-02-05  7:15 ` [RFC PATCH 02/15] fetch2: local: use path variable Stefan Herbrechtsmeier
2025-02-05  7:15 ` [RFC PATCH 03/15] fetch2: remove unnecessary unquote Stefan Herbrechtsmeier
2025-02-05  7:15 ` [RFC PATCH 04/15] fetch2: ssh: use common localpath handling Stefan Herbrechtsmeier
2025-02-05  7:15 ` [RFC PATCH 05/15] fetch2: clearcase: remove double DL_DIR from localfile Stefan Herbrechtsmeier
2025-02-05  7:15 ` [RFC PATCH 06/15] fetch2: remove basepath from FetchData Stefan Herbrechtsmeier
2025-02-05  7:15 ` [RFC PATCH 07/15] fetch2: remove basename " Stefan Herbrechtsmeier
2025-02-05  7:15 ` [RFC PATCH 08/15] fetch2: use localpath instead of localfile Stefan Herbrechtsmeier
2025-02-06 15:26   ` [bitbake-devel] " Richard Purdie
2025-02-06 16:21     ` Stefan Herbrechtsmeier
     [not found]     ` <1821AAFAC9C0AAD1.752@lists.openembedded.org>
2025-02-07  8:05       ` Stefan Herbrechtsmeier
2025-02-07  8:10         ` Richard Purdie
2025-02-07 12:48           ` Stefan Herbrechtsmeier
2025-02-05  7:15 ` [RFC PATCH 09/15] fetch2: make DOWNLOADFILENAME and PATH explicit in mirrors Stefan Herbrechtsmeier
2025-02-05  7:15 ` [RFC PATCH 10/15] tests: fetch: unify style of replaceuris in MirrorUriTest Stefan Herbrechtsmeier
2025-02-05  7:15 ` [RFC PATCH 11/15] tests: fetch: fix nonsensical " Stefan Herbrechtsmeier
2025-02-05  7:15 ` [RFC PATCH 12/15] tests: fetch: reenable " Stefan Herbrechtsmeier
2025-02-05  7:15 ` [RFC PATCH 13/15] tests: fetch: comment and add " Stefan Herbrechtsmeier
2025-02-05  7:15 ` [RFC PATCH 14/15] tests: fetch: add npm mirrors " Stefan Herbrechtsmeier
2025-02-05  7:15 ` [RFC PATCH 15/15] tests: fetch: fix nonsensical mirror uris in FetcherNetworkTest Stefan Herbrechtsmeier
2025-02-05 10:34 ` [bitbake-devel] [RFC PATCH 00/15] Make mirror replacement syntax explicit Richard Purdie
2025-02-05 12:12   ` Stefan Herbrechtsmeier
2025-02-20 10:22     ` Richard Purdie
2025-02-20 11:45       ` Stefan Herbrechtsmeier
2025-02-20 12:21         ` Richard Purdie
2025-02-20 17:37           ` Stefan Herbrechtsmeier [this message]
2025-02-20 22:00             ` Richard Purdie
2025-02-21 10:51               ` Stefan Herbrechtsmeier
     [not found]   ` <18214EC364EFB1B6.18160@lists.openembedded.org>
2025-02-05 12:30     ` Stefan Herbrechtsmeier

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=07c3ebb6-8cdf-416a-bc89-ab930d85e78d@weidmueller.com \
    --to=stefan.herbrechtsmeier-oss@weidmueller.com \
    --cc=bitbake-devel@lists.openembedded.org \
    --cc=richard.purdie@linuxfoundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.