Patchwork [bitbake-devel] bitbake/fetch2: add support for wrapper command

login
register
mail settings
Submitter Matthew McClintock
Date Jan. 12, 2012, 11:09 p.m.
Message ID <1326409741-17390-1-git-send-email-msm@freescale.com>
Download mbox | patch
Permalink /patch/19227/
State New
Headers show

Comments

Matthew McClintock - Jan. 12, 2012, 11:09 p.m.
cc: bitbake-devel@lists.openembedded.org
Signed-off-by: Matthew McClintock <msm@freescale.com>
---
 lib/bb/fetch2/__init__.py |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
Richard Purdie - Jan. 13, 2012, 8:44 a.m.
On Thu, 2012-01-12 at 17:09 -0600, Matthew McClintock wrote:
> cc: bitbake-devel@lists.openembedded.org
> Signed-off-by: Matthew McClintock <msm@freescale.com>
> ---
>  lib/bb/fetch2/__init__.py |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/lib/bb/fetch2/__init__.py b/lib/bb/fetch2/__init__.py
> index 3af56e5..1d7d5c7 100644
> --- a/lib/bb/fetch2/__init__.py
> +++ b/lib/bb/fetch2/__init__.py
> @@ -389,6 +389,8 @@ def runfetchcmd(cmd, d, quiet = False, cleanup = []):
>      Optionally remove the files/directories listed in cleanup upon failure
>      """
>  
> +    cmd = "%s %s" % (bb.data.getVar("BB_FETCH_WRAPPER", d, True), cmd)
> +
>      # Need to export PATH as binary could be in metadata paths
>      # rather than host provided
>      # Also include some other variables.

Er, this is going to break for anyone who doesn't have BB_FETCH_WRAPPER
set. It would be nice to document why you require this in the commit
message (I can guess). Since I'm rejecting this patch, can the next one
document the new variable in the fetcher section of bitbake manual too
please? :)

Cheers,

Richard
McClintock Matthew-B29882 - Jan. 13, 2012, 2:44 p.m.
On Fri, Jan 13, 2012 at 2:44 AM, Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
> On Thu, 2012-01-12 at 17:09 -0600, Matthew McClintock wrote:
>> cc: bitbake-devel@lists.openembedded.org
>> Signed-off-by: Matthew McClintock <msm@freescale.com>
>> ---
>>  lib/bb/fetch2/__init__.py |    2 ++
>>  1 files changed, 2 insertions(+), 0 deletions(-)
>>
>> diff --git a/lib/bb/fetch2/__init__.py b/lib/bb/fetch2/__init__.py
>> index 3af56e5..1d7d5c7 100644
>> --- a/lib/bb/fetch2/__init__.py
>> +++ b/lib/bb/fetch2/__init__.py
>> @@ -389,6 +389,8 @@ def runfetchcmd(cmd, d, quiet = False, cleanup = []):
>>      Optionally remove the files/directories listed in cleanup upon failure
>>      """
>>
>> +    cmd = "%s %s" % (bb.data.getVar("BB_FETCH_WRAPPER", d, True), cmd)
>> +
>>      # Need to export PATH as binary could be in metadata paths
>>      # rather than host provided
>>      # Also include some other variables.
>
> Er, this is going to break for anyone who doesn't have BB_FETCH_WRAPPER
> set. It would be nice to document why you require this in the commit
> message (I can guess). Since I'm rejecting this patch, can the next one
> document the new variable in the fetcher section of bitbake manual too
> please? :)

Sorry, was looking for feedback too - I think I need to do:

cmd = "%s %s" % (bb.data.getVar("BB_FETCH_WRAPPER", d, True) or "", cmd)

I wrote a small blurb about why it was needed on the patch to oe-core,
we are trying to do CVS or a SOCKS5 proxy that has authentication (the
proxy) - I didn't see another way around this...

-M
Richard Purdie - Jan. 13, 2012, 3:16 p.m.
On Fri, 2012-01-13 at 14:44 +0000, McClintock Matthew-B29882 wrote:
> On Fri, Jan 13, 2012 at 2:44 AM, Richard Purdie
> <richard.purdie@linuxfoundation.org> wrote:
> > On Thu, 2012-01-12 at 17:09 -0600, Matthew McClintock wrote:
> >> cc: bitbake-devel@lists.openembedded.org
> >> Signed-off-by: Matthew McClintock <msm@freescale.com>
> >> ---
> >>  lib/bb/fetch2/__init__.py |    2 ++
> >>  1 files changed, 2 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/lib/bb/fetch2/__init__.py b/lib/bb/fetch2/__init__.py
> >> index 3af56e5..1d7d5c7 100644
> >> --- a/lib/bb/fetch2/__init__.py
> >> +++ b/lib/bb/fetch2/__init__.py
> >> @@ -389,6 +389,8 @@ def runfetchcmd(cmd, d, quiet = False, cleanup = []):
> >>      Optionally remove the files/directories listed in cleanup upon failure
> >>      """
> >>
> >> +    cmd = "%s %s" % (bb.data.getVar("BB_FETCH_WRAPPER", d, True), cmd)
> >> +
> >>      # Need to export PATH as binary could be in metadata paths
> >>      # rather than host provided
> >>      # Also include some other variables.
> >
> > Er, this is going to break for anyone who doesn't have BB_FETCH_WRAPPER
> > set. It would be nice to document why you require this in the commit
> > message (I can guess). Since I'm rejecting this patch, can the next one
> > document the new variable in the fetcher section of bitbake manual too
> > please? :)
> 
> Sorry, was looking for feedback too - I think I need to do:
> 
> cmd = "%s %s" % (bb.data.getVar("BB_FETCH_WRAPPER", d, True) or "", cmd)

wrapper = d.getVar("BB_FETCH_WRAPPER", True)
if wrapper:
    cmd = wrapper + " " + cmd

(please use d.getVar, not bb.data.getVar)

> I wrote a small blurb about why it was needed on the patch to oe-core,
> we are trying to do CVS or a SOCKS5 proxy that has authentication (the
> proxy) - I didn't see another way around this...

Can't this be done via the environment or the FETCHCMD statements in
bitbake.conf?

What really worries me here is that people keep adding 101 ways to use
different proxy setups. I'd really like one good way than 101 ways which
half work. All these different approaches and variables make the
codebase a nightmare to maintain and make it hard to explain to someone
how to setup things for their proxy.

Cheers,

Richard
McClintock Matthew-B29882 - Jan. 13, 2012, 3:20 p.m.
On Fri, Jan 13, 2012 at 9:16 AM, Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
> wrapper = d.getVar("BB_FETCH_WRAPPER", True)
> if wrapper:
>    cmd = wrapper + " " + cmd
>
> (please use d.getVar, not bb.data.getVar)

Ok.

>
>> I wrote a small blurb about why it was needed on the patch to oe-core,
>> we are trying to do CVS or a SOCKS5 proxy that has authentication (the
>> proxy) - I didn't see another way around this...
>
> Can't this be done via the environment or the FETCHCMD statements in
> bitbake.conf?
>
> What really worries me here is that people keep adding 101 ways to use
> different proxy setups. I'd really like one good way than 101 ways which
> half work. All these different approaches and variables make the
> codebase a nightmare to maintain and make it hard to explain to someone
> how to setup things for their proxy.

Actually, this way might make more sense... I'll test it out.

-M
McClintock Matthew-B29882 - Jan. 13, 2012, 4:03 p.m.
On Fri, Jan 13, 2012 at 9:20 AM, Matthew McClintock <msm@freescale.com> wrote:
>>> I wrote a small blurb about why it was needed on the patch to oe-core,
>>> we are trying to do CVS or a SOCKS5 proxy that has authentication (the
>>> proxy) - I didn't see another way around this...
>>
>> Can't this be done via the environment or the FETCHCMD statements in
>> bitbake.conf?
>>
>> What really worries me here is that people keep adding 101 ways to use
>> different proxy setups. I'd really like one good way than 101 ways which
>> half work. All these different approaches and variables make the
>> codebase a nightmare to maintain and make it hard to explain to someone
>> how to setup things for their proxy.
>
> Actually, this way might make more sense... I'll test it out.

Something like:

FETCHCOMMAND_cvs_prepend = "socksify "

Seems to work quite well. How should we document this? I see:

https://wiki.yoctoproject.org/wiki/Working_Behind_a_Network_Proxy

Or should we just add an example blurb to local.conf.sample? Or add it
to the I don't see much in the user manual. I think it might be
worthwhile to add section to the Poky Reference Manual.

-M

Patch

diff --git a/lib/bb/fetch2/__init__.py b/lib/bb/fetch2/__init__.py
index 3af56e5..1d7d5c7 100644
--- a/lib/bb/fetch2/__init__.py
+++ b/lib/bb/fetch2/__init__.py
@@ -389,6 +389,8 @@  def runfetchcmd(cmd, d, quiet = False, cleanup = []):
     Optionally remove the files/directories listed in cleanup upon failure
     """
 
+    cmd = "%s %s" % (bb.data.getVar("BB_FETCH_WRAPPER", d, True), cmd)
+
     # Need to export PATH as binary could be in metadata paths
     # rather than host provided
     # Also include some other variables.