Re: [bitbake-devel] [RFC PATCH 08/15] fetch2: use localpath instead of localfile

[Richard Purdie <richard.purdie@linuxfoundation.org>]

On Fri, 2025-02-07 at 09:05 +0100, Stefan Herbrechtsmeier wrote:
>  
> 
>  
>  
> Am 06.02.2025 um 17:21 schrieb Stefan Herbrechtsmeier via
> lists.openembedded.org:
>  
>  
> > Am 06.02.2025 um 16:26 schrieb Richard Purdie: 
> >  
> > > On Wed, 2025-02-05 at 08:15 +0100, Stefan Herbrechtsmeier via
> > > lists.openembedded.org wrote: 
> > >  
> > > > From: Stefan Herbrechtsmeier
> > > > <stefan.herbrechtsmeier@weidmueller.com> 
> > > >  
> > > >  Use localpath variable instead of localfile and DL_DIR
> > > > variable. 
> > > >  
> > > >  Signed-off-by: Stefan Herbrechtsmeier
> > > > <stefan.herbrechtsmeier@weidmueller.com> 
> > > >  --- 
> > > >  
> > > >    lib/bb/fetch2/az.py   | 3 +-- 
> > > >    lib/bb/fetch2/repo.py | 2 +- 
> > > >    lib/bb/fetch2/sftp.py | 3 +-- 
> > > >    lib/bb/fetch2/wget.py | 2 +- 
> > > >    4 files changed, 4 insertions(+), 6 deletions(-) 
> > > >  
> > > >  diff --git a/lib/bb/fetch2/az.py b/lib/bb/fetch2/az.py 
> > > >  index 346124a8b..1f0f1337a 100644 
> > > >  --- a/lib/bb/fetch2/az.py 
> > > >  +++ b/lib/bb/fetch2/az.py 
> > > >  @@ -49,8 +49,7 @@ class Az(Wget): 
> > > >            fetchcmd = self.basecmd + ' --retry-connrefused --
> > > > waitretry=5' 
> > > >              # We need to provide a localpath to avoid wget
> > > > using the SAS 
> > > >  -        # ud.localfile either has the downloadfilename or
> > > > ud.path 
> > > >  -        localpath = os.path.join(d.getVar("DL_DIR"),
> > > > ud.localfile) 
> > > >  +        localpath = ud.localpath 
> > > >            bb.utils.mkdirhier(os.path.dirname(localpath)) 
> > > >            fetchcmd += " -O %s" % shlex.quote(localpath) 
> > > >    diff --git a/lib/bb/fetch2/repo.py b/lib/bb/fetch2/repo.py 
> > > >  index fa4cb8149..5c0edf1f2 100644 
> > > >  --- a/lib/bb/fetch2/repo.py 
> > > >  +++ b/lib/bb/fetch2/repo.py 
> > > >  @@ -46,7 +46,7 @@ class Repo(FetchMethod): 
> > > >        def download(self, ud, d): 
> > > >            """Fetch url""" 
> > > >    -        if os.access(os.path.join(d.getVar("DL_DIR"),
> > > > ud.localfile), os.R_OK): 
> > > >  +        if os.access(ud.localpath, os.R_OK): 
> > > >                logger.debug("%s already exists (or was
> > > > stashed). Skipping repo init / sync.", ud.localpath) 
> > > >                return 
> > > >    diff --git a/lib/bb/fetch2/sftp.py b/lib/bb/fetch2/sftp.py 
> > > >  index 2a2a70a1b..b88dc5a28 100644 
> > > >  --- a/lib/bb/fetch2/sftp.py 
> > > >  +++ b/lib/bb/fetch2/sftp.py 
> > > >  @@ -82,8 +82,7 @@ class SFTP(FetchMethod): 
> > > >                port = '-P %d' % urlo.port 
> > > >                urlo.port = None 
> > > >    -        dldir = d.getVar('DL_DIR') 
> > > >  -        lpath = os.path.join(dldir, ud.localfile) 
> > > >  +        lpath = ud.localpath 
> > > >              user = '' 
> > > >            if urlo.userinfo: 
> > > >  diff --git a/lib/bb/fetch2/wget.py b/lib/bb/fetch2/wget.py 
> > > >  index 161c66bea..1194f0e7a 100644 
> > > >  --- a/lib/bb/fetch2/wget.py 
> > > >  +++ b/lib/bb/fetch2/wget.py 
> > > >  @@ -95,7 +95,7 @@ class Wget(FetchMethod): 
> > > >            fetchcmd = self.basecmd 
> > > >              dldir = os.path.realpath(d.getVar("DL_DIR")) 
> > > >  -        localpath = os.path.join(dldir, ud.localfile) +
> > > > ".tmp" 
> > > >  +        localpath = ud.localpath + ".tmp" 
> > > >            bb.utils.mkdirhier(os.path.dirname(localpath)) 
> > > >            fetchcmd += " -O %s" % shlex.quote(localpath) 
> > > >    
> > > >  
> > >  
> > >  I put the first 8 patches of this series into a test branch and
> > > ran it 
> > >  against the autobuilder. It fails to parse, unable to find files
> > > :(. 
> > >  
> >  
> >  Thanks for the test. 
> >  
> >  
> > > https://autobuilder.yoctoproject.org/valkyrie/#/builders/29/builds/960/steps/13/logs/stdio
> > > https://autobuilder.yoctoproject.org/valkyrie/#/builders/29/builds/960/steps/13/logs/errors
> > >  
> > >  That suggests there is some breaking change in here
> > > unfortunately. 
> > >  
> >  
> >  I assume the @ in the filename is interpreter as username
> > separator . I will look into it and add a test to the selftest.
>  
> We have two solutions to decode an URI with different behavior:
>  class URI - urllib.parse.urlparse(uri)
>  def decodeurl - re.compile(...).match(uri)


decodeurl is older and predates urllib existing. People have tried to
switch things over and use urllib where possible but as you say, the
behaviour is different. I don't remember what the differences are.

>   Additionally the encode of an URI object and the encodeuri function
> have different behavior. The class use the unquoted and the function
> the quoted path. Is this behavior intended or could we use one
> implementation?


I suspect we've been preserving the old behaviour so it was intended if
a bit strange. It is hard to comment on what we could do without an
idea of what it would break and what the differences are.

Cheers,

Richard