Re: [PATCH] s3.py: Add support for fetching source mirrors/minor cleanup

[Beth 'pidge' Flanagan <pidge@toganlabs.com>]

On Tue, 2017-03-28 at 14:21 +0100, Richard Purdie wrote:
> On Tue, 2017-03-28 at 15:05 +0200, Beth 'pidge' Flanagan wrote:
> > 
> > On Tue, 2017-03-28 at 13:56 +0100, Richard Purdie wrote:
> > > 
> > > 
> > > On Tue, 2017-03-28 at 13:39 +0200, Elizabeth 'pidge' Flanagan
> > > wrote:
> > > > 
> > > > 
> > > > 
> > > > This commits main purpose is to add support for fetching source
> > > > mirrors. In the current incarnation:
> > > > 
> > > > SOURCE_MIRROR_URL ?= "s3://mybucket/downloads"
> > > > 
> > > > will fail for two reasons. First, download doesn't support it,
> > > > but second, without aws included in HOSTTOOLS you'll end up
> > > > with aws not being found by bitbake (for either source mirrors
> > > > or
> > > > sstate mirrors).
> > > > 
> > > > Part of this is fixed with this commit. However, this will
> > > > still
> > > > fail if HOSTTOOLS doesn't include 'aws' in bitbake.conf. I've
> > > > another
> > > > commit or two to fix that as well.
> > > > 
> > > > I've also DRYed up some of the error handling, removed the
> > > > hardcoded
> > > > aws and added some logging.
> > > > 
> > > > Signed-off-by: Elizabeth 'pidge' Flanagan <pidge@toganlabs.com>
> > > > ---
> > > >  lib/bb/fetch2/s3.py | 20 ++++++++++++++------
> > > >  1 file changed, 14 insertions(+), 6 deletions(-)
> > > > 
> > > > diff --git a/lib/bb/fetch2/s3.py b/lib/bb/fetch2/s3.py
> > > > index 27993aa..dbd86f4 100644
> > > > --- a/lib/bb/fetch2/s3.py
> > > > +++ b/lib/bb/fetch2/s3.py
> > > > @@ -34,6 +34,7 @@ import urllib.request, urllib.parse,
> > > > urllib.error
> > > >  from bb.fetch2 import FetchMethod
> > > >  from bb.fetch2 import FetchError
> > > >  from bb.fetch2 import runfetchcmd
> > > > +from bb.fetch2 import logger
> > > >  
> > > >  class S3(FetchMethod):
> > > >      """Class to fetch urls via 'aws s3'"""
> > > > @@ -60,8 +61,13 @@ class S3(FetchMethod):
> > > >          Fetch urls
> > > >          Assumes localpath was called first
> > > >          """
> > > > -
> > > > -        cmd = 'aws s3 cp s3://%s%s %s' % (ud.host, ud.path,
> > > > ud.localpath)
> > > > +        if 'downloadfilename' in ud.parm:
> > > > +            dldir = d.getVar("DL_DIR", True)
> > > > +            bb.utils.mkdirhier(os.path.dirname(dldir + os.sep
> > > > +
> > > > ud.localfile))
> > > > +            cmd = '%s cp s3://%s%s %s%s%s' %
> > > > (d.getVar("FETCHCMD_s3"), ud.host, ud.path, dldir, os.sep,
> > > > ud.localpath)
> > > > +        else:
> > > > +            cmd = '%s cp s3://%s%s %s' %
> > > > (d.getVar("FETCHCMD_s3"),
> > > > ud.host, ud.path, ud.localpath)
> > > > +        logger.debug(2, "Fetching %s using command '%s'" %
> > > > (ud.url,
> > > > cmd))
> > > Do you actually need/use FETCHCMD_s3 ?
> > > 
> > I use it so I can rip out aws-cli and use s3cmd.
> Ok. The way this needs to work then is that in __init__, you set a
> default command and then if d.getVar("FETCHCMD_s3") is set, overwrite
> it. This is so that the fetcher works without having to FETCHCMD_s3
> (in
> common with all the other fetchers. You'd then use ud.cmd instead of
> all the getVars.
> 

Ah, ok. v2 incoming in a bit.

> Cheers,
> Richard