diff mbox series

[RFC,08/15] fetch2: use localpath instead of localfile

Message ID 20250205071538.2681-9-stefan.herbrechtsmeier-oss@weidmueller.com
State New
Headers show
Series Make mirror replacement syntax explicit | expand

Commit Message

Stefan Herbrechtsmeier Feb. 5, 2025, 7:15 a.m. UTC
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(-)

Comments

Richard Purdie Feb. 6, 2025, 3:26 p.m. UTC | #1
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 :(.

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.

Cheers,

Richard
Stefan Herbrechtsmeier Feb. 6, 2025, 4:21 p.m. UTC | #2
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.
Stefan Herbrechtsmeier Feb. 7, 2025, 8:05 a.m. UTC | #3
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)

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?
Richard Purdie Feb. 7, 2025, 8:10 a.m. UTC | #4
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
Stefan Herbrechtsmeier Feb. 7, 2025, 12:48 p.m. UTC | #5
Am 07.02.2025 um 09:10 schrieb Richard Purdie:
> 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.

I have add a patch to fix the problem and an optimization to remove 
duplicated code.

Is it possible to test the changes on the build server?

Regards
   Stefan
diff mbox series

Patch

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)