Hi,

Thanks for the patch. This isn't ready to be merged yet though as there are some
issues.

On Mon, 2021-08-23 at 15:18 +0800, [email protected] wrote:
> From 1b0d7b4bb4a5b39f7ae0ce7d7ae5897a33637972 Mon Sep 17 00:00:00 2001
> From: Mingrui Ren <[email protected]>
> Date: Mon, 23 Aug 2021 14:49:03 +0800
> Subject: [PATCH v2] bitbake/fetch2: Add a new variable 'BB_FETCH_ENV' to 
> export Fetcher env
> 
> The environment variables used by Fetcher are hard-coded, and are obtained
> from HOST env instead of bitbake datastore

This isn't true, they are looked at first in bitbake's datastore, then taken
from the original host environment if not in the datastore.

> This patch add a new variable 'BB_FETCH_ENV',and modify the default
> BB_ENV_EXTRAWHITE_OE for backwards compatibility, trying to fix the
> problems above.

Why is this a problem? You need to state what the problem is, not what the code
currently does (or in the above case doesn't do).

> Signed-off-by: Mingrui Ren <[email protected]>
> ---
> changes in v2:
>      a.changes the variable name from 'FETCH_ENV_WHITELIST' to 'BB_FETCH_ENV'.
>      b.add 'BB_FETCH_ENV' in local.conf, rather than export it in host
>        enviroment.
>      c.modify existing BB_ENV_EXTRAWHITE_OE for backwards compatibility.
>      d.Two commits recently modified this variable. The commit ID is:
>        348384135272ae7c62a11eeabcc43eddc957811f and 
> 5dce2f3da20a14c0eb5229696561b0c5f6fce54c,
>        So I adjusted the new variables in the patch.
> 
>  bitbake/lib/bb/fetch2/__init__.py | 34 ++++++++-----------------------
>  bitbake/lib/bb/fetch2/wget.py     |  2 +-
>  meta-poky/conf/local.conf.sample  | 12 +++++++++++
>  scripts/oe-buildenv-internal      |  3 ++-
>  4 files changed, 24 insertions(+), 27 deletions(-)
> 
> diff --git a/bitbake/lib/bb/fetch2/__init__.py 
> b/bitbake/lib/bb/fetch2/__init__.py
> index 914fa5c024..cbbe32d1df 100644
> --- a/bitbake/lib/bb/fetch2/__init__.py
> +++ b/bitbake/lib/bb/fetch2/__init__.py
> @@ -808,28 +808,13 @@ def localpath(url, d):
>      fetcher = bb.fetch2.Fetch([url], d)
>      return fetcher.localpath(url)
>  
> -# Need to export PATH as binary could be in metadata paths
> -# rather than host provided
> -# Also include some other variables.
> -FETCH_EXPORT_VARS = ['HOME', 'PATH',
> -                     'HTTP_PROXY', 'http_proxy',
> -                     'HTTPS_PROXY', 'https_proxy',
> -                     'FTP_PROXY', 'ftp_proxy',
> -                     'FTPS_PROXY', 'ftps_proxy',
> -                     'NO_PROXY', 'no_proxy',
> -                     'ALL_PROXY', 'all_proxy',
> -                     'GIT_PROXY_COMMAND',
> -                     'GIT_SSH',
> -                     'GIT_SSL_CAINFO',
> -                     'GIT_SMART_HTTP',
> -                     'SSH_AUTH_SOCK', 'SSH_AGENT_PID',
> -                     'SOCKS5_USER', 'SOCKS5_PASSWD',
> -                     'DBUS_SESSION_BUS_ADDRESS',
> -                     'P4CONFIG',
> -                     'SSL_CERT_FILE',
> -                     'AWS_ACCESS_KEY_ID',
> -                     'AWS_SECRET_ACCESS_KEY',
> -                     'AWS_DEFAULT_REGION']

Firstly, I'd prefer not to move this list out of the fetcher code. Bitbake can
be in theory used standalone without the OE-Core metadata and this data makes
more sense to be maintained in the fetcher.


> +def getfetchenv(d):
> +    # Need to export PATH as binary could be in metadata paths
> +    # rather than host provided
> +    # Also include some other variables.
> +    vars = ['HOME', 'PATH']
> +    vars.extend((d.getVar("BB_FETCH_ENV") or "").split())
> +    return vars
>  
>  def runfetchcmd(cmd, d, quiet=False, cleanup=None, log=None, workdir=None):
>      """
> @@ -839,7 +824,7 @@ def runfetchcmd(cmd, d, quiet=False, cleanup=None, 
> log=None, workdir=None):
>      Optionally remove the files/directories listed in cleanup upon failure
>      """
>  
> -    exportvars = FETCH_EXPORT_VARS
> +    exportvars = getfetchenv(d)
>  
>      if not cleanup:
>          cleanup = []
> @@ -855,9 +840,8 @@ def runfetchcmd(cmd, d, quiet=False, cleanup=None, 
> log=None, workdir=None):
>          d.setVar("PV", "fetcheravoidrecurse")
>          d.setVar("PR", "fetcheravoidrecurse")
>  
> -    origenv = d.getVar("BB_ORIGENV", False)
>      for var in exportvars:
> -        val = d.getVar(var) or (origenv and origenv.getVar(var))
> +        val = d.getVar(var)
>          if val:
>              cmd = 'export ' + var + '=\"%s\"; %s' % (val, cmd)
> 

Please don't drop the BB_ORIGENV handling. Why is that being removed?

>  
> diff --git a/bitbake/lib/bb/fetch2/wget.py b/bitbake/lib/bb/fetch2/wget.py
> index 29fcfbb3d1..0ce06ddb4f 100644
> --- a/bitbake/lib/bb/fetch2/wget.py
> +++ b/bitbake/lib/bb/fetch2/wget.py
> @@ -306,7 +306,7 @@ class Wget(FetchMethod):
>          # to scope the changes to the build_opener request, which is when the
>          # environment lookups happen.
>          newenv = {}
> -        for name in bb.fetch2.FETCH_EXPORT_VARS:
> +        for name in bb.fetch2.getfetchenv(d):
>              value = d.getVar(name)
>              if not value:
>                  origenv = d.getVar("BB_ORIGENV")
> diff --git a/meta-poky/conf/local.conf.sample 
> b/meta-poky/conf/local.conf.sample
> index f1f6d690fb..4e8a6f0c77 100644
> --- a/meta-poky/conf/local.conf.sample
> +++ b/meta-poky/conf/local.conf.sample
> @@ -267,6 +267,18 @@ PACKAGECONFIG:append:pn-qemu-system-native = " sdl"
>  #
>  #BB_SERVER_TIMEOUT = "60"
>  
> +# Bitbake Fetcher Environment Variables
> +#
> +# Specific which environment variables in bitbake datastore used by fetcher 
> when
> +# executing fetch task.
> +# NOTE: You may need to modify BB_ENV_EXTRAWHITE, in order to add environment
> +# variable into bitbake datastore first.
> +BB_FETCH_ENV ?= "HTTP_PROXY http_proxy HTTPS_PROXY https_proxy \
> +FTP_PROXY ftp_proxy FTPS_PROXY ftps_proxy NO_PROXY no_proxy ALL_PROXY 
> all_proxy \
> +GIT_PROXY_COMMAND GIT_SSH GIT_SSL_CAINFO GIT_SMART_HTTP SSH_AUTH_SOCK 
> SSH_AGENT_PID \
> +SOCKS5_USER SOCKS5_PASSWD DBUS_SESSION_BUS_ADDRESS P4CONFIG SSL_CERT_FILE 
> AWS_ACCESS_KEY_ID\
> +AWS_SECRET_ACCESS_KEY AWS_DEFAULT_REGION"
> +

I'd like to see this preserved in bitbake, not in bitbake.conf in OE-Core.

>  # CONF_VERSION is increased each time build/conf/ changes incompatibly and 
> is used to
>  # track the version of this file when it was generated. This can safely be 
> ignored if
>  # this doesn't mean anything to you.
> diff --git a/scripts/oe-buildenv-internal b/scripts/oe-buildenv-internal
> index e0d920f2fc..29cb694790 100755
> --- a/scripts/oe-buildenv-internal
> +++ b/scripts/oe-buildenv-internal
> @@ -111,7 +111,8 @@ HTTPS_PROXY https_proxy FTP_PROXY ftp_proxy FTPS_PROXY 
> ftps_proxy ALL_PROXY \
>  all_proxy NO_PROXY no_proxy SSH_AGENT_PID SSH_AUTH_SOCK BB_SRCREV_POLICY \
>  SDKMACHINE BB_NUMBER_THREADS BB_NO_NETWORK PARALLEL_MAKE GIT_PROXY_COMMAND \
>  SOCKS5_PASSWD SOCKS5_USER SCREENDIR STAMPS_DIR BBPATH_EXTRA 
> BB_SETSCENE_ENFORCE \
> -BB_LOGCONFIG"
> +BB_LOGCONFIG HOME PATH GIT_SSH GIT_SSL_CAINFO GIT_SMART_HTTP 
> DBUS_SESSION_BUS_ADDRESS \
> +P4CONFIG SSL_CERT_FILE AWS_ACCESS_KEY_ID AWS_SECRET_ACCESS_KEY 
> AWS_DEFAULT_REGION"
>  
>  BB_ENV_EXTRAWHITE="$(echo $BB_ENV_EXTRAWHITE $BB_ENV_EXTRAWHITE_OE | tr ' ' 
> '\n' | LC_ALL=C sort --unique | tr '\n' ' ')"

I think if you leave BB_ORIGENV handling in place, these latter changes
shouldn't be needed?

If you need to be able to pass extra variables into the fetcher, I think we
could/should add API for additions rather than allowing the whole list to be
customised. Without stating which problem you're solving, guessing what you
really need is hard though. A better description of the issue you're seeing
would help a lot.

Cheers,

Richard


-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#54653): https://lists.yoctoproject.org/g/yocto/message/54653
Mute This Topic: https://lists.yoctoproject.org/mt/85080430/21656
Group Owner: [email protected]
Unsubscribe: https://lists.yoctoproject.org/g/yocto/unsub 
[[email protected]]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to