Message ID | 20250529141418.1939172-1-robbin.vandamme@renson.be |
---|---|
State | New |
Headers | show |
Series | [v2,YOCTO,#15882] Add question mark to mark the start of the AZ_SAS query parameters. | expand |
On 5/29/25 16:14, robbin van damme via lists.openembedded.org wrote: > Signed-off-by: Robbin Van Damme <robbinvandamme@gmail.com> > --- > lib/bb/fetch2/az.py | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/lib/bb/fetch2/az.py b/lib/bb/fetch2/az.py > index 346124a8b..327273379 100644 > --- a/lib/bb/fetch2/az.py > +++ b/lib/bb/fetch2/az.py > @@ -36,7 +36,7 @@ class Az(Wget): > > az_sas = d.getVar('AZ_SAS') > if az_sas and az_sas not in ud.url: > - ud.url += az_sas > + ud.url += f'?{az_sas}' > > return Wget.checkstatus(self, fetch, ud, d, try_again) > > @@ -62,7 +62,7 @@ class Az(Wget): > az_sas = d.getVar('AZ_SAS') > > if az_sas: > - azuri = '%s%s%s%s' % ('https://', ud.host, ud.path, az_sas) > + azuri = '%s%s%s?%s' % ('https://', ud.host, ud.path, az_sas) > else: > azuri = '%s%s%s' % ('https://', ud.host, ud.path) > I think that this is going to be a documentation problem. Though not mentioned in the commit message unfortunately, I believe I see the issue: the url has the classic "protocol://domain/path?param=value¶m2=value2..." format. The AZ_SAS variable holds the "param=value..." part. The original commit message[1] does contain a (not too strong) reference that it should start with a question mark, however this didn't reach the documentation, and it gives an example without a question mark[2]. I think the documentation should be updated primarily. Of course the code could be a bit smarter too, for example if az_sas doesn't start with '?', then maybe emit a debug message and prepend a '?' automatically. Though I don't know how many people rely on this fetcher, but I don't think the current behavior should be changed with suddenly not expecting a '?' in this variable. [1]: https://git.openembedded.org/bitbake/commit/lib/bb/fetch2/az.py?id=b103b02f2ce2f8f5079f17ec1a854f904c2110a4 [2]: https://docs.yoctoproject.org/bitbake/2.6/bitbake-user-manual/bitbake-user-manual-fetching.html#az-fetcher-az
Hi, Ok thats fine. Lets forget this patch then. I submitted other one with update of the docs on how to use the AZ_SAS. kind regards, Robbin On Thu, May 29, 2025 at 5:34 PM Gyorgy Sarvari <skandigraun@gmail.com> wrote: > On 5/29/25 16:14, robbin van damme via lists.openembedded.org wrote: > > Signed-off-by: Robbin Van Damme <robbinvandamme@gmail.com> > > --- > > lib/bb/fetch2/az.py | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/lib/bb/fetch2/az.py b/lib/bb/fetch2/az.py > > index 346124a8b..327273379 100644 > > --- a/lib/bb/fetch2/az.py > > +++ b/lib/bb/fetch2/az.py > > @@ -36,7 +36,7 @@ class Az(Wget): > > > > az_sas = d.getVar('AZ_SAS') > > if az_sas and az_sas not in ud.url: > > - ud.url += az_sas > > + ud.url += f'?{az_sas}' > > > > return Wget.checkstatus(self, fetch, ud, d, try_again) > > > > @@ -62,7 +62,7 @@ class Az(Wget): > > az_sas = d.getVar('AZ_SAS') > > > > if az_sas: > > - azuri = '%s%s%s%s' % ('https://', ud.host, ud.path, az_sas) > > + azuri = '%s%s%s?%s' % ('https://', ud.host, ud.path, > az_sas) > > else: > > azuri = '%s%s%s' % ('https://', ud.host, ud.path) > > > I think that this is going to be a documentation problem. > Though not mentioned in the commit message unfortunately, I believe I > see the issue: the url has the classic > "protocol://domain/path?param=value¶m2=value2..." format. The AZ_SAS > variable holds the "param=value..." part. The original commit message[1] > does contain a (not too strong) reference that it should start with a > question mark, however this didn't reach the documentation, and it gives > an example without a question mark[2]. > > I think the documentation should be updated primarily. Of course the > code could be a bit smarter too, for example if az_sas doesn't start > with '?', then maybe emit a debug message and prepend a '?' > automatically. Though I don't know how many people rely on this fetcher, > but I don't think the current behavior should be changed with suddenly > not expecting a '?' in this variable. > > [1]: > > https://git.openembedded.org/bitbake/commit/lib/bb/fetch2/az.py?id=b103b02f2ce2f8f5079f17ec1a854f904c2110a4 > [2]: > > https://docs.yoctoproject.org/bitbake/2.6/bitbake-user-manual/bitbake-user-manual-fetching.html#az-fetcher-az > >
On Fri, 2025-05-30 at 09:21 +0200, robbin van damme via lists.openembedded.org wrote: > Hi, > > Ok thats fine. > Lets forget this patch then. > I submitted other one with update of the docs on how to use the > AZ_SAS. What might also help is adding a sanity check to the fetcher to ensure the variable does start with "?". If it does not, we could raise an error so the user knows what is wrong. Cheers, Richard
diff --git a/lib/bb/fetch2/az.py b/lib/bb/fetch2/az.py index 346124a8b..327273379 100644 --- a/lib/bb/fetch2/az.py +++ b/lib/bb/fetch2/az.py @@ -36,7 +36,7 @@ class Az(Wget): az_sas = d.getVar('AZ_SAS') if az_sas and az_sas not in ud.url: - ud.url += az_sas + ud.url += f'?{az_sas}' return Wget.checkstatus(self, fetch, ud, d, try_again) @@ -62,7 +62,7 @@ class Az(Wget): az_sas = d.getVar('AZ_SAS') if az_sas: - azuri = '%s%s%s%s' % ('https://', ud.host, ud.path, az_sas) + azuri = '%s%s%s?%s' % ('https://', ud.host, ud.path, az_sas) else: azuri = '%s%s%s' % ('https://', ud.host, ud.path)
Signed-off-by: Robbin Van Damme <robbinvandamme@gmail.com> --- lib/bb/fetch2/az.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)