[Buildroot] [PATCH 1/2] core: add BR2_PRIMARY_SITE_ONLY_EXTENDED_DOMAINS

[Buildroot] [PATCH 1/2] core: add BR2_PRIMARY_SITE_ONLY_EXTENDED_DOMAINS

[Thomas De Schampheleire]

From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

If configured, the primary site typically points to a mirror on the intranet
of an organization. The purpose of BR2_PRIMARY_SITE_ONLY is then to only
download from this mirror.

However, the organization may also have some local Buildroot packages
that download from a version control repository (git, hg, ...). In this case,
the mirror will normally not contain the sources, instead they should be
cloned via the version control tool. So in this case, BR2_PRIMARY_SITE_ONLY
cannot be used.

This means that the organization must resort to other means to make sure no
external downloads are performed.

This patch attempts to solve this situation by adding
BR2_PRIMARY_SITE_ONLY_EXTENDED_DOMAINS. This string option can contain
additional domains from which download is allowed when BR2_PRIMARY_SITE_ONLY
is set.

The organization can thus set:
BR2_PRIMARY_SITE_ONLY=y
BR2_PRIMARY_SITE_ONLY_EXTENDED_DOMAINS="git.example.com hg.example.com"

to disallow any external downloads other than the primary site and the
mentioned version control domains.

Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
---
 Config.in               | 11 +++++++++++
 package/pkg-download.mk |  8 +++++++-
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/Config.in b/Config.in
index 48fd36ff98..0843215bc5 100644
--- a/Config.in
+++ b/Config.in
@@ -226,6 +226,17 @@ config BR2_PRIMARY_SITE_ONLY
 	  the project can be built even if the upstream tarball
 	  locations disappear.
 
+config BR2_PRIMARY_SITE_ONLY_EXTENDED_DOMAINS
+	string "Additional domains to allow downloads from"
+	depends on BR2_PRIMARY_SITE_ONLY
+	help
+	  If BR2_PRIMARY_SITE_ONLY is enabled, version control downloads
+	  (git, hg, ...) on the 'internal' domain would also be disallowed.
+	  With this option, you can specify additional domains from
+	  which downloads will be allowed in BR2_PRIMARY_SITE_ONLY-mode.
+	  Domains should not include a protocol prefix, and multiple
+	  domains can be separated by spaces.
+
 if !BR2_PRIMARY_SITE_ONLY
 
 config BR2_BACKUP_SITE
diff --git a/package/pkg-download.mk b/package/pkg-download.mk
index de619ba90a..f189dba233 100644
--- a/package/pkg-download.mk
+++ b/package/pkg-download.mk
@@ -78,7 +78,13 @@ DOWNLOAD_URIS += \
 	$(call getschemeplusuri,$(call qstrip,$(BR2_PRIMARY_SITE)),urlencode)
 endif
 
-ifeq ($(BR2_PRIMARY_SITE_ONLY),)
+ifeq ($(BR2_PRIMARY_SITE_ONLY),y)
+# Conditionally add site download if it matches the configured extended domains
+DOWNLOAD_URIS += \
+	$(if $(filter $(call qstrip,$(BR2_PRIMARY_SITE_ONLY_EXTENDED_DOMAINS)),$(call domain,$(1))), \
+		$(patsubst %/,%,$(dir $(call qstrip,$(1)))))
+else
+# Unconditionally add site download
 DOWNLOAD_URIS += \
 	$(patsubst %/,%,$(dir $(call qstrip,$(1))))
 ifneq ($(call qstrip,$(BR2_BACKUP_SITE)),)
-- 
2.26.2

[Buildroot] [PATCH 1/2] core: add BR2_PRIMARY_SITE_ONLY_EXTENDED_DOMAINS

[Thomas De Schampheleire]

From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

If configured, the primary site typically points to a mirror on the intranet
of an organization. The purpose of BR2_PRIMARY_SITE_ONLY is then to only
download from this mirror.

However, the organization may also have some local Buildroot packages
that download from a version control repository (git, hg, ...). In this case,
the mirror will normally not contain the sources, instead they should be
cloned via the version control tool. So in this case, BR2_PRIMARY_SITE_ONLY
cannot be used.

This means that the organization must resort to other means to make sure no
external downloads are performed.

This patch attempts to solve this situation by adding
BR2_PRIMARY_SITE_ONLY_EXTENDED_DOMAINS. This string option can contain
additional domains from which download is allowed when BR2_PRIMARY_SITE_ONLY
is set.

The organization can thus set:
BR2_PRIMARY_SITE_ONLY=y
BR2_PRIMARY_SITE_ONLY_EXTENDED_DOMAINS="git.example.com hg.example.com"

to disallow any external downloads other than the primary site and the
mentioned version control domains.

Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
---
 Config.in               | 12 ++++++++++++
 package/pkg-download.mk |  8 +++++++-
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/Config.in b/Config.in
index e35a78fb71..c9206876ff 100644
--- a/Config.in
+++ b/Config.in
@@ -231,6 +231,18 @@ config BR2_PRIMARY_SITE_ONLY
 	  the project can be built even if the upstream tarball
 	  locations disappear.
 
+config BR2_PRIMARY_SITE_ONLY_EXTENDED_DOMAINS
+	string "Additional domains to allow downloads from"
+	depends on BR2_PRIMARY_SITE_ONLY
+	help
+	  If BR2_PRIMARY_SITE_ONLY is enabled, version control downloads
+	  (git, hg, ...) on the 'internal' domain would also be
+	  disallowed.
+	  With this option, you can specify additional domains from
+	  which downloads will be allowed in BR2_PRIMARY_SITE_ONLY-mode.
+	  Domains should not include a protocol prefix, and multiple
+	  domains can be separated by spaces.
+
 if !BR2_PRIMARY_SITE_ONLY
 
 config BR2_BACKUP_SITE
diff --git a/package/pkg-download.mk b/package/pkg-download.mk
index 951d2fb554..d23838a329 100644
--- a/package/pkg-download.mk
+++ b/package/pkg-download.mk
@@ -78,7 +78,13 @@ DOWNLOAD_URIS += \
 	$(call getschemeplusuri,$(call qstrip,$(BR2_PRIMARY_SITE)),urlencode)
 endif
 
-ifeq ($(BR2_PRIMARY_SITE_ONLY),)
+ifeq ($(BR2_PRIMARY_SITE_ONLY),y)
+# Conditionally add site download if it matches the configured extended domains
+DOWNLOAD_URIS += \
+	$(if $(filter $(call qstrip,$(BR2_PRIMARY_SITE_ONLY_EXTENDED_DOMAINS)),$(call domain,$(1))), \
+		$(patsubst %/,%,$(dir $(call qstrip,$(1)))))
+else
+# Unconditionally add site download
 DOWNLOAD_URIS += \
 	$(patsubst %/,%,$(dir $(call qstrip,$(1))))
 ifneq ($(call qstrip,$(BR2_BACKUP_SITE)),)
-- 
2.26.2

[Buildroot] [PATCH 2/2] utils/source-check: new script

[Thomas De Schampheleire]

From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

This source-check script is a replacement for 'make source-check' that
existed in earlier versions of Buildroot.

It takes as input a list of defconfigs, and then efficiently determines
whether all files needed can be downloaded, without actually downloading
them.

The settings of BR2_PRIMARY_SITE, BR2_PRIMARY_SITE_ONLY and
BR2_PRIMARY_SITE_ONLY_EXTENDED_DOMAINS will be used as specified in the
respective defconfigs.

Note: scp, hg, file, and http(s) protocols are currently covered. Others,
like git, bzr, svn currently are not. I don't really use these and am not
sure if it is possible to check remotely if something is valid or not,
without downloading the entire repository.

Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
---
 utils/source-check | 220 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 220 insertions(+)
 create mode 100755 utils/source-check

diff --git a/utils/source-check b/utils/source-check
new file mode 100755
index 0000000000..16566b9e81
--- /dev/null
+++ b/utils/source-check
@@ -0,0 +1,220 @@
+#!/usr/bin/env python3
+"""
+source-check: check that all packages needed for the specified defconfigs can be downloaded
+
+Given a list of defconfigs, determine which URLs are needed to build it, and
+check the accessibility of the packages represented by them. Typically this
+does not actually involve a real download, so this scripts works very fast.
+"""
+
+import json
+import multiprocessing
+import os
+import shutil
+import subprocess
+import sys
+
+# example output of 'make show-info'
+# 'rsync': {'dependencies': ['host-ccache',
+#                            'host-skeleton',
+#                            'host-tar',
+#                            'popt',
+#                            'skeleton',
+#                            'toolchain',
+#                            'zlib'],
+#           'dl_dir': 'rsync',
+#           'downloads': [{'source': 'rsync-3.1.3.tar.gz',
+#                          'uris': ['scp|urlencode+scp://xxx at mirror.example.com/rsync',
+#                                   'scp|urlencode+scp://xxx at mirror.example.com',
+#                                   'http+http://rsync.samba.org/ftp/rsync/src',
+#                                   'http|urlencode+http://sources.buildroot.net/rsync',
+#                                   'http|urlencode+http://sources.buildroot.net']}],
+#           'install_images': False,
+#           'install_staging': False,
+#           'install_target': True,
+#           'licenses': 'GPL-3.0+',
+#           'reverse_dependencies': [],
+#           'type': 'target',
+#           'version': '3.1.3',
+#           'virtual': False},
+
+
+def get_files_to_check_one_defconfig(defconfig):
+    outputdir = 'sourcecheck_%s' % defconfig
+    subprocess.check_call([
+        'make', '--no-print-directory', '-s', 'O=%s' % outputdir,
+        defconfig
+    ])
+    # Note: set suitable-host-package to empty to pretend no suitable tools are
+    # present on the host, and thus force all potentially-needed sources in the
+    # list (e.g. cmake, gzip, ...)
+    output = subprocess.check_output([
+        'make', '--no-print-directory', '-s', 'O=%s' % outputdir,
+        'show-info', 'suitable-host-package='
+    ])
+    info = json.loads(output)
+
+    files_to_check = set()
+
+    for pkg in info:
+        if 'downloads' not in info[pkg]:
+            sys.stderr.write("Warning: %s: no downloads for package '%s'\n" % (defconfig, pkg))
+            continue
+        if not info[pkg]['downloads']:
+            sys.stderr.write("Warning: %s: empty downloads for package '%s'\n" % (defconfig, pkg))
+            continue
+        for download in info[pkg]['downloads']:
+            if 'source' not in download:
+                sys.stderr.write("Warning: %s: no source filename found for package '%s'\n" % (defconfig, pkg))
+                continue
+            if 'uris' not in download:
+                sys.stderr.write("Warning: %s: no uri's found for package '%s'\n" % (defconfig, pkg))
+                continue
+
+            # tuple: (pkg, version, filename, uris)
+            # Note: host packages have the same sources as for target, so strip
+            # the 'host-' prefix. Because we are using a set, this will remove
+            # duplicate entries.
+            pkgname = pkg[5:] if pkg.startswith('host-') else pkg
+            files_to_check.add((
+                pkgname,
+                info[pkg]['version'],
+                download['source'],
+                tuple([uri for uri in download['uris']]),
+            ))
+
+    shutil.rmtree(outputdir)
+    return files_to_check
+
+
+def get_files_to_check(defconfigs):
+    total_files_to_check = set()
+
+    num_processes = multiprocessing.cpu_count() * 2
+    print('Dispatching over %s processes' % num_processes)
+    with multiprocessing.Pool(processes=num_processes) as pool:
+        result_objs = [
+            pool.apply_async(get_files_to_check_one_defconfig, (defconfig,))
+            for defconfig in defconfigs
+        ]
+        results = [p.get() for p in result_objs]
+
+    for result in results:
+        total_files_to_check |= result
+
+    return total_files_to_check
+
+
+def sourcecheck_one_uri(pkg, version, filename, uri):
+
+    def sourcecheck_scp(pkg, version, filename, uri):
+        real_uri = uri.split('+', 1)[1] + '/' + filename
+        if real_uri.startswith('scp://'):
+            real_uri = real_uri[6:]
+        domain, path = real_uri.split(':', 1)
+        with open(os.devnull, 'w') as devnull:
+            ret = subprocess.call(
+                ['ssh', domain, 'test', '-f', path],
+                stderr=devnull
+            )
+        return ret == 0
+
+    def sourcecheck_hg(pkg, version, filename, uri):
+        real_uri = uri.split('+', 1)[1]
+        with open(os.devnull, 'w') as devnull:
+            ret = subprocess.call(
+                ['hg', 'identify', '--rev', version, real_uri],
+                stdout=devnull, stderr=devnull
+            )
+        return ret == 0
+
+    def sourcecheck_file(pkg, version, filename, uri):
+        real_uri = uri.split('+', 1)[1] + '/' + filename
+        if real_uri.startswith('file://'):
+            real_uri = real_uri[7:]
+        return os.path.exists(real_uri)
+
+    def sourcecheck_http(pkg, version, filename, uri):
+        real_uri = uri.split('+', 1)[1] + '/' + filename
+        with open(os.devnull, 'w') as devnull:
+            ret = subprocess.call(
+                ['wget', '--spider', real_uri],
+                stderr=devnull
+            )
+        return ret == 0
+
+    if uri.startswith('scp'):
+        handler = sourcecheck_scp
+    elif uri.startswith('hg'):
+        handler = sourcecheck_hg
+    elif uri.startswith('file'):
+        handler = sourcecheck_file
+    elif uri.startswith('http'):
+        handler = sourcecheck_http
+    else:
+        raise Exception("Cannot handle unknown URI type: '%s' for package '%s'" % (uri, pkg))
+
+    return handler(pkg, version, filename, uri)
+
+
+def sourcecheck_one_file(pkg, version, filename, uris):
+    result = any(
+        sourcecheck_one_uri(pkg, version, filename, uri)
+        for uri in uris
+    )
+    return pkg, version, filename, result
+
+
+def sourcecheck(files_to_check):
+
+    def process_result(result):
+        pkg, version, filename, success = result
+        if success:
+            print(' OK: pkg %s, filename %s' % (pkg, filename))
+        else:
+            sys.stderr.write('NOK: pkg %s, filename %s -- ERROR!\n' % (pkg, filename))
+
+    num_processes = multiprocessing.cpu_count() * 2
+    print('Dispatching over %s processes' % num_processes)
+    with multiprocessing.Pool(processes=num_processes) as pool:
+        result_objs = [
+            pool.apply_async(sourcecheck_one_file, entry, callback=process_result)
+            for entry in files_to_check
+        ]
+        results = [p.get() for p in result_objs]
+
+    succeeded = [
+        (pkg, version, filename, success)
+        for (pkg, version, filename, success) in results
+        if success
+    ]
+    failed = [
+        (pkg, version, filename, success)
+        for (pkg, version, filename, success) in results
+        if not success
+    ]
+
+    print('\nSummary: %s OK, %s NOK, %s total' % (len(succeeded), len(failed), len(results)))
+
+    if len(failed):
+        print('\nFAILED FILES')
+        for pkg, version, filename, success in sorted(failed):
+            print('pkg: %s, version: %s, file: %s/%s' % (pkg, version, pkg, filename))
+
+    return len(failed) == 0
+
+
+def main():
+    defconfigs = sys.argv[1:]
+    if not defconfigs:
+        sys.stderr.write('Error: pass list of defconfigs as arguments\n')
+        sys.exit(1)
+
+    total_files_to_check = get_files_to_check(defconfigs)
+    return sourcecheck(total_files_to_check)
+
+
+if __name__ == '__main__':
+    ret = main()
+    if not ret:
+        sys.exit(1)
-- 
2.26.2

[Buildroot] [PATCH 1/2] core: add BR2_PRIMARY_SITE_ONLY_EXTENDED_DOMAINS

[Yann E. MORIN]

Thomas, All,

On 2020-12-04 13:33 +0100, Thomas De Schampheleire spake thusly:
> From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> 
> If configured, the primary site typically points to a mirror on the intranet
> of an organization. The purpose of BR2_PRIMARY_SITE_ONLY is then to only
> download from this mirror.
> 
> However, the organization may also have some local Buildroot packages
> that download from a version control repository (git, hg, ...). In this case,
> the mirror will normally not contain the sources, instead they should be
> cloned via the version control tool. So in this case, BR2_PRIMARY_SITE_ONLY
> cannot be used.
> 
> This means that the organization must resort to other means to make sure no
> external downloads are performed.
> 
> This patch attempts to solve this situation by adding
> BR2_PRIMARY_SITE_ONLY_EXTENDED_DOMAINS. This string option can contain
> additional domains from which download is allowed when BR2_PRIMARY_SITE_ONLY
> is set.
> 
> The organization can thus set:
> BR2_PRIMARY_SITE_ONLY=y
> BR2_PRIMARY_SITE_ONLY_EXTENDED_DOMAINS="git.example.com hg.example.com"

BR2_PRIMRAY_SITE_ONLY_ALLOWED_DOMAINS would be more meaningful, I
think...

> to disallow any external downloads other than the primary site and the
> mentioned version control domains.
> 
> Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

Although I do understand the issue, and I do agree that this is a sane
policy to not rely on external resources in such a situation, I still
think this is pushing the boundaries a bit on Buildroot...

First, there would be nothing that would prevent a local user to enter
menuconfig and set this to whatever they want, or to simply disable
primary-only. So, an option such as BR2_PRIMRAY_SITE_ONLY_ALLOWED_DOMAINS
would not cater to local users on their development machines.

So, we're left with automated builds, running in a CI. In this case, the
environment is much more controlled, and I think this is where such a
limitation should take place. The CI system should not be able to reach
out to the wider internet, and should be constrained to access local the
network(s).

For example, in the CI at $work, there are two steps:
 1. make source, which has access only to the local network;
 2. make, which does not have access to the network at all.

Also, when considering the upcoming "package managers" (go, cargo,
etc...), the filtering is way too early, and can't be applied to them.
I.e. there is no way to tell (e.g.) cargo that a set of domains are
allowed while others are not. So an option like you propose would only
have a limited use, with un-pluggable holes hidden from sight, ready to
trip users hard-time. A properly set-up CI system would however prevent
those package managers from indeed reaching outside the company's
network(s).

So, no, I am very much not in favour of this new option...

Regards,
Yann E. MORIN.

> ---
>  Config.in               | 12 ++++++++++++
>  package/pkg-download.mk |  8 +++++++-
>  2 files changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/Config.in b/Config.in
> index e35a78fb71..c9206876ff 100644
> --- a/Config.in
> +++ b/Config.in
> @@ -231,6 +231,18 @@ config BR2_PRIMARY_SITE_ONLY
>  	  the project can be built even if the upstream tarball
>  	  locations disappear.
>  
> +config BR2_PRIMARY_SITE_ONLY_EXTENDED_DOMAINS
> +	string "Additional domains to allow downloads from"
> +	depends on BR2_PRIMARY_SITE_ONLY
> +	help
> +	  If BR2_PRIMARY_SITE_ONLY is enabled, version control downloads
> +	  (git, hg, ...) on the 'internal' domain would also be
> +	  disallowed.
> +	  With this option, you can specify additional domains from
> +	  which downloads will be allowed in BR2_PRIMARY_SITE_ONLY-mode.
> +	  Domains should not include a protocol prefix, and multiple
> +	  domains can be separated by spaces.
> +
>  if !BR2_PRIMARY_SITE_ONLY
>  
>  config BR2_BACKUP_SITE
> diff --git a/package/pkg-download.mk b/package/pkg-download.mk
> index 951d2fb554..d23838a329 100644
> --- a/package/pkg-download.mk
> +++ b/package/pkg-download.mk
> @@ -78,7 +78,13 @@ DOWNLOAD_URIS += \
>  	$(call getschemeplusuri,$(call qstrip,$(BR2_PRIMARY_SITE)),urlencode)
>  endif
>  
> -ifeq ($(BR2_PRIMARY_SITE_ONLY),)
> +ifeq ($(BR2_PRIMARY_SITE_ONLY),y)
> +# Conditionally add site download if it matches the configured extended domains
> +DOWNLOAD_URIS += \
> +	$(if $(filter $(call qstrip,$(BR2_PRIMARY_SITE_ONLY_EXTENDED_DOMAINS)),$(call domain,$(1))), \
> +		$(patsubst %/,%,$(dir $(call qstrip,$(1)))))
> +else
> +# Unconditionally add site download
>  DOWNLOAD_URIS += \
>  	$(patsubst %/,%,$(dir $(call qstrip,$(1))))
>  ifneq ($(call qstrip,$(BR2_BACKUP_SITE)),)
> -- 
> 2.26.2
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 2/2] utils/source-check: new script

[Yann E. MORIN]

Thomas, All,

On 2020-12-04 13:33 +0100, Thomas De Schampheleire spake thusly:
> From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> 
> This source-check script is a replacement for 'make source-check' that
> existed in earlier versions of Buildroot.
> 
> It takes as input a list of defconfigs,

Make that work on the current configured directory. I.e. it should
require a .config file to be already present.

Scanning for multiple defconfigs should be left as an exercise to the
interested parties (e.g. a job in a CI system, I believe).

e.g.;

    for cfg in configs/*_defconfig; do
        make ${cfg#*/}
        ./utils/source-check || { printf 'Failed: %s\n' "${cfg#*/}"; break; }
    done

> and then efficiently determines
> whether all files needed can be downloaded, without actually downloading
> them.
> 
> The settings of BR2_PRIMARY_SITE, BR2_PRIMARY_SITE_ONLY and
> BR2_PRIMARY_SITE_ONLY_EXTENDED_DOMAINS will be used as specified in the
> respective defconfigs.
> 
> Note: scp, hg, file, and http(s) protocols are currently covered. Others,
> like git, bzr, svn currently are not. I don't really use these and am not
> sure if it is possible to check remotely if something is valid or not,
> without downloading the entire repository.
> 
> Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> ---
>  utils/source-check | 220 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 220 insertions(+)
>  create mode 100755 utils/source-check
> 
> diff --git a/utils/source-check b/utils/source-check
> new file mode 100755
> index 0000000000..16566b9e81
> --- /dev/null
> +++ b/utils/source-check
> @@ -0,0 +1,220 @@
> +#!/usr/bin/env python3
> +"""
> +source-check: check that all packages needed for the specified defconfigs can be downloaded
> +
> +Given a list of defconfigs, determine which URLs are needed to build it, and
> +check the accessibility of the packages represented by them. Typically this
> +does not actually involve a real download, so this scripts works very fast.
> +"""
> +
> +# example output of 'make show-info'
> +# 'rsync': {'dependencies': ['host-ccache',
> +#                            'host-skeleton',
> +#                            'host-tar',
> +#                            'popt',
> +#                            'skeleton',
> +#                            'toolchain',
> +#                            'zlib'],
> +#           'dl_dir': 'rsync',
> +#           'downloads': [{'source': 'rsync-3.1.3.tar.gz',
> +#                          'uris': ['scp|urlencode+scp://xxx at mirror.example.com/rsync',
> +#                                   'scp|urlencode+scp://xxx at mirror.example.com',
> +#                                   'http+http://rsync.samba.org/ftp/rsync/src',
> +#                                   'http|urlencode+http://sources.buildroot.net/rsync',
> +#                                   'http|urlencode+http://sources.buildroot.net']}],
> +#           'install_images': False,
> +#           'install_staging': False,
> +#           'install_target': True,
> +#           'licenses': 'GPL-3.0+',
> +#           'reverse_dependencies': [],
> +#           'type': 'target',
> +#           'version': '3.1.3',
> +#           'virtual': False},
> +
> +
> +def get_files_to_check_one_defconfig(defconfig):
> +    outputdir = 'sourcecheck_%s' % defconfig
> +    subprocess.check_call([
> +        'make', '--no-print-directory', '-s', 'O=%s' % outputdir,
> +        defconfig
> +    ])
> +    # Note: set suitable-host-package to empty to pretend no suitable tools are
> +    # present on the host, and thus force all potentially-needed sources in the
> +    # list (e.g. cmake, gzip, ...)
> +    output = subprocess.check_output([
> +        'make', '--no-print-directory', '-s', 'O=%s' % outputdir,
> +        'show-info', 'suitable-host-package='
> +    ])
> +    info = json.loads(output)
> +
> +    files_to_check = set()
> +
> +    for pkg in info:
> +        if 'downloads' not in info[pkg]:
> +            sys.stderr.write("Warning: %s: no downloads for package '%s'\n" % (defconfig, pkg))
> +            continue
> +        if not info[pkg]['downloads']:
> +            sys.stderr.write("Warning: %s: empty downloads for package '%s'\n" % (defconfig, pkg))
> +            continue

Does that relayy warrant a warning? virtual packages have no download
key; some system-level packages (skeletons, mkpasswd et al.) have a
download key, but the dictionnary is empty. Having spurious warnings is
the best way for people to simply ignore them...

> +        for download in info[pkg]['downloads']:
> +            if 'source' not in download:
> +                sys.stderr.write("Warning: %s: no source filename found for package '%s'\n" % (defconfig, pkg))
> +                continue
> +            if 'uris' not in download:
> +                sys.stderr.write("Warning: %s: no uri's found for package '%s'\n" % (defconfig, pkg))
> +                continue

A download without a source or without a URI is an error, not a warning.
Either it is a bug in the package, or it is a bug in the show-info infra.
Either way, it must be fixed.

> +            # tuple: (pkg, version, filename, uris)
> +            # Note: host packages have the same sources as for target, so strip
> +            # the 'host-' prefix. Because we are using a set, this will remove
> +            # duplicate entries.
> +            pkgname = pkg[5:] if pkg.startswith('host-') else pkg
> +            files_to_check.add((
> +                pkgname,

No need for the intermediate variable pkgname:

    files_to_check.add((
        pkg[5:] if pkg.startswith('host-') else pkg,
        ...
    ))

> +                info[pkg]['version'],
> +                download['source'],
> +                tuple([uri for uri in download['uris']]),
> +            ))
> +
> +    shutil.rmtree(outputdir)
> +    return files_to_check
> +
> +
> +def get_files_to_check(defconfigs):
> +    total_files_to_check = set()
> +
> +    num_processes = multiprocessing.cpu_count() * 2
> +    print('Dispatching over %s processes' % num_processes)
> +    with multiprocessing.Pool(processes=num_processes) as pool:
> +        result_objs = [
> +            pool.apply_async(get_files_to_check_one_defconfig, (defconfig,))
> +            for defconfig in defconfigs
> +        ]
> +        results = [p.get() for p in result_objs]
> +
> +    for result in results:
> +        total_files_to_check |= result
> +
> +    return total_files_to_check
> +
> +
> +def sourcecheck_one_uri(pkg, version, filename, uri):
> +

flake8 does not whine with or wihtout a leading empty line. I prefer
when there is none, though...

However, let's try something:

    def sourcecheck_one_uri(pkg, version, filename, uri):
        handler = dict()

        def sourcecheck_file(...):
            ...
        handler['file'] = source_check_file

        def sourcecheck_hg(...);
            ...
        handler['hg'] = source_check_file

        try:
            return handler[uri.split('://', 1)[0]](pkg, version, filename, uri)
        except KeyError:
            raise Exeption('Meh unknown URI type "{}"'.format(uri)) from None

Not sure how nicer the code would be. At least, we get an easy one-liner
to demux the URI type (plus the try-except boilerplate).

> +    def sourcecheck_scp(pkg, version, filename, uri):

Please define handler in alphabetical order.

> +        real_uri = uri.split('+', 1)[1] + '/' + filename
> +        if real_uri.startswith('scp://'):
> +            real_uri = real_uri[6:]
> +        domain, path = real_uri.split(':', 1)
> +        with open(os.devnull, 'w') as devnull:
> +            ret = subprocess.call(
> +                ['ssh', domain, 'test', '-f', path],
> +                stderr=devnull
> +            )
> +        return ret == 0
> +
> +    def sourcecheck_hg(pkg, version, filename, uri):
> +        real_uri = uri.split('+', 1)[1]
> +        with open(os.devnull, 'w') as devnull:
> +            ret = subprocess.call(
> +                ['hg', 'identify', '--rev', version, real_uri],
> +                stdout=devnull, stderr=devnull
> +            )
> +        return ret == 0
> +
> +    def sourcecheck_file(pkg, version, filename, uri):
> +        real_uri = uri.split('+', 1)[1] + '/' + filename
> +        if real_uri.startswith('file://'):
> +            real_uri = real_uri[7:]
> +        return os.path.exists(real_uri)
> +
> +    def sourcecheck_http(pkg, version, filename, uri):
> +        real_uri = uri.split('+', 1)[1] + '/' + filename
> +        with open(os.devnull, 'w') as devnull:
> +            ret = subprocess.call(
> +                ['wget', '--spider', real_uri],
> +                stderr=devnull
> +            )
> +        return ret == 0
> +
> +    if uri.startswith('scp'):
> +        handler = sourcecheck_scp
> +    elif uri.startswith('hg'):
> +        handler = sourcecheck_hg
> +    elif uri.startswith('file'):
> +        handler = sourcecheck_file
> +    elif uri.startswith('http'):
> +        handler = sourcecheck_http
> +    else:
> +        raise Exception("Cannot handle unknown URI type: '%s' for package '%s'" % (uri, pkg))
> +
> +    return handler(pkg, version, filename, uri)
> +
> +
> +def sourcecheck_one_file(pkg, version, filename, uris):
> +    result = any(
> +        sourcecheck_one_uri(pkg, version, filename, uri)
> +        for uri in uris
> +    )
> +    return pkg, version, filename, result
> +
> +
> +def sourcecheck(files_to_check):
> +
> +    def process_result(result):
> +        pkg, version, filename, success = result
> +        if success:
> +            print(' OK: pkg %s, filename %s' % (pkg, filename))
> +        else:
> +            sys.stderr.write('NOK: pkg %s, filename %s -- ERROR!\n' % (pkg, filename))
> +
> +    num_processes = multiprocessing.cpu_count() * 2
> +    print('Dispatching over %s processes' % num_processes)

Hmm... I don't much like this auto-parallelism... I think we should not
try to do parallsism at all. But if you really insist, then make that a
command line option (e.g. source-check -jN).

Regards,
Yann E. MORIN.

> +    with multiprocessing.Pool(processes=num_processes) as pool:
> +        result_objs = [
> +            pool.apply_async(sourcecheck_one_file, entry, callback=process_result)
> +            for entry in files_to_check
> +        ]
> +        results = [p.get() for p in result_objs]
> +
> +    succeeded = [
> +        (pkg, version, filename, success)
> +        for (pkg, version, filename, success) in results
> +        if success
> +    ]
> +    failed = [
> +        (pkg, version, filename, success)
> +        for (pkg, version, filename, success) in results
> +        if not success
> +    ]
> +
> +    print('\nSummary: %s OK, %s NOK, %s total' % (len(succeeded), len(failed), len(results)))
> +
> +    if len(failed):
> +        print('\nFAILED FILES')
> +        for pkg, version, filename, success in sorted(failed):
> +            print('pkg: %s, version: %s, file: %s/%s' % (pkg, version, pkg, filename))
> +
> +    return len(failed) == 0
> +
> +
> +def main():
> +    defconfigs = sys.argv[1:]
> +    if not defconfigs:
> +        sys.stderr.write('Error: pass list of defconfigs as arguments\n')
> +        sys.exit(1)
> +
> +    total_files_to_check = get_files_to_check(defconfigs)
> +    return sourcecheck(total_files_to_check)
> +
> +
> +if __name__ == '__main__':
> +    ret = main()
> +    if not ret:
> +        sys.exit(1)
> -- 
> 2.26.2
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 1/2] core: add BR2_PRIMARY_SITE_ONLY_EXTENDED_DOMAINS

[Thomas De Schampheleire]

Hi Yann, all,

Sorry for the delay in answering.

El s?b, 2 ene 2021 a las 23:07, Yann E. MORIN
(<yann.morin.1998@free.fr>) escribi?:
>
> Thomas, All,
>
> On 2020-12-04 13:33 +0100, Thomas De Schampheleire spake thusly:
> > From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> >
> > If configured, the primary site typically points to a mirror on the intranet
> > of an organization. The purpose of BR2_PRIMARY_SITE_ONLY is then to only
> > download from this mirror.
> >
> > However, the organization may also have some local Buildroot packages
> > that download from a version control repository (git, hg, ...). In this case,
> > the mirror will normally not contain the sources, instead they should be
> > cloned via the version control tool. So in this case, BR2_PRIMARY_SITE_ONLY
> > cannot be used.
> >
> > This means that the organization must resort to other means to make sure no
> > external downloads are performed.
> >
> > This patch attempts to solve this situation by adding
> > BR2_PRIMARY_SITE_ONLY_EXTENDED_DOMAINS. This string option can contain
> > additional domains from which download is allowed when BR2_PRIMARY_SITE_ONLY
> > is set.
> >
> > The organization can thus set:
> > BR2_PRIMARY_SITE_ONLY=y
> > BR2_PRIMARY_SITE_ONLY_EXTENDED_DOMAINS="git.example.com hg.example.com"
>
> BR2_PRIMRAY_SITE_ONLY_ALLOWED_DOMAINS would be more meaningful, I
> think...
>
> > to disallow any external downloads other than the primary site and the
> > mentioned version control domains.
> >
> > Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
>
> Although I do understand the issue, and I do agree that this is a sane
> policy to not rely on external resources in such a situation, I still
> think this is pushing the boundaries a bit on Buildroot...
>
> First, there would be nothing that would prevent a local user to enter
> menuconfig and set this to whatever they want, or to simply disable
> primary-only. So, an option such as BR2_PRIMRAY_SITE_ONLY_ALLOWED_DOMAINS
> would not cater to local users on their development machines.
>
> So, we're left with automated builds, running in a CI. In this case, the
> environment is much more controlled, and I think this is where such a
> limitation should take place. The CI system should not be able to reach
> out to the wider internet, and should be constrained to access local the
> network(s).
>
> For example, in the CI at $work, there are two steps:
>  1. make source, which has access only to the local network;
>  2. make, which does not have access to the network at all.
>
> Also, when considering the upcoming "package managers" (go, cargo,
> etc...), the filtering is way too early, and can't be applied to them.
> I.e. there is no way to tell (e.g.) cargo that a set of domains are
> allowed while others are not. So an option like you propose would only
> have a limited use, with un-pluggable holes hidden from sight, ready to
> trip users hard-time. A properly set-up CI system would however prevent
> those package managers from indeed reaching outside the company's
> network(s).
>
> So, no, I am very much not in favour of this new option...

BR2_PRIMARY_SITE_ONLY_ALLOWED_DOMAINS is merely an extension of
BR2_PRIMARY_SITE_ONLY, taking into account the fact that
BR2_PRIMARY_SITE_ONLY does not consider downloads from version control
systems, i.e. only assuming wget or scp from a fixed mirror.
I think your arguments, for example related to external package
managers, also apply to the existing BR2_PRIMARY_SITE_ONLY. So based
on these arguments alone, the conclusion would be to get rid of
BR2_PRIMARY_SITE_ONLY too.

In fact, even if the external network is (intentionally) not
accessible, just enabling BR2_PRIMARY_SITE_ONLY (possibly in
combination with BR2_PRIMARY_SITE_ONLY_ALLOWED_DOMAINS) will help with
making the logs more clear. Indeed, without it, absence of a certain
tarball on the primary mirror, will still result in download attempts
to the external network, which fails. Users that read the log, without
knowing much detail about the buildroot download mechanisms, then
conclude that there is a problem with the external download, instead
of realizing that the real problem is either connection to the mirror,
or absence of the package on that mirror.
(I'm not talking hypothetically here, I am faced with such behavior frequently).

So the option(s) to download only from a mirror have valid reasons to
exist IMHO.
The concerns about other download managers are also valid, but already
exists without these changes. And not all users actually use packages
of these types.

Best regards,
Thomas

[Buildroot] [PATCH 2/2] utils/source-check: new script

[Thomas De Schampheleire]

El s?b, 2 ene 2021 a las 23:56, Yann E. MORIN
(<yann.morin.1998@free.fr>) escribi?:
>
> Thomas, All,
>
> On 2020-12-04 13:33 +0100, Thomas De Schampheleire spake thusly:
> > From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> >
> > This source-check script is a replacement for 'make source-check' that
> > existed in earlier versions of Buildroot.
> >
> > It takes as input a list of defconfigs,
>
> Make that work on the current configured directory. I.e. it should
> require a .config file to be already present.
>
> Scanning for multiple defconfigs should be left as an exercise to the
> interested parties (e.g. a job in a CI system, I believe).
>
> e.g.;
>
>     for cfg in configs/*_defconfig; do
>         make ${cfg#*/}
>         ./utils/source-check || { printf 'Failed: %s\n' "${cfg#*/}"; break; }
>     done
>

The method you propose is much less efficient, as it would check each
common package N times.
In fact, this was roughly what I was doing previously with the 'old'
source-check that I proposed.
The current approach, combining all defconfigs in one go, is way
faster, because it first creates a unique list of all URLs to be
checked, and then checks each of them exactly once. This is
particularly relevent when you have many defconfigs to check,
typically in some corporate environments.

[..]
> > +
> > +    for pkg in info:
> > +        if 'downloads' not in info[pkg]:
> > +            sys.stderr.write("Warning: %s: no downloads for package '%s'\n" % (defconfig, pkg))
> > +            continue
> > +        if not info[pkg]['downloads']:
> > +            sys.stderr.write("Warning: %s: empty downloads for package '%s'\n" % (defconfig, pkg))
> > +            continue
>
> Does that relayy warrant a warning? virtual packages have no download
> key; some system-level packages (skeletons, mkpasswd et al.) have a
> download key, but the dictionnary is empty. Having spurious warnings is
> the best way for people to simply ignore them...

Perhaps it is too much; I added it to help me in finding potential
issues in scanning the packages.

>
> > +        for download in info[pkg]['downloads']:
> > +            if 'source' not in download:
> > +                sys.stderr.write("Warning: %s: no source filename found for package '%s'\n" % (defconfig, pkg))
> > +                continue
> > +            if 'uris' not in download:
> > +                sys.stderr.write("Warning: %s: no uri's found for package '%s'\n" % (defconfig, pkg))
> > +                continue
>
> A download without a source or without a URI is an error, not a warning.
> Either it is a bug in the package, or it is a bug in the show-info infra.
> Either way, it must be fixed.

OK.

>
> > +            # tuple: (pkg, version, filename, uris)
> > +            # Note: host packages have the same sources as for target, so strip
> > +            # the 'host-' prefix. Because we are using a set, this will remove
> > +            # duplicate entries.
> > +            pkgname = pkg[5:] if pkg.startswith('host-') else pkg
> > +            files_to_check.add((
> > +                pkgname,
>
> No need for the intermediate variable pkgname:
>
>     files_to_check.add((
>         pkg[5:] if pkg.startswith('host-') else pkg,
>         ...
>     ))

OK

>
> > +                info[pkg]['version'],
> > +                download['source'],
> > +                tuple([uri for uri in download['uris']]),
> > +            ))
> > +
> > +    shutil.rmtree(outputdir)
> > +    return files_to_check
> > +
> > +
> > +def get_files_to_check(defconfigs):
> > +    total_files_to_check = set()
> > +
> > +    num_processes = multiprocessing.cpu_count() * 2
> > +    print('Dispatching over %s processes' % num_processes)
> > +    with multiprocessing.Pool(processes=num_processes) as pool:
> > +        result_objs = [
> > +            pool.apply_async(get_files_to_check_one_defconfig, (defconfig,))
> > +            for defconfig in defconfigs
> > +        ]
> > +        results = [p.get() for p in result_objs]
> > +
> > +    for result in results:
> > +        total_files_to_check |= result
> > +
> > +    return total_files_to_check
> > +
> > +
> > +def sourcecheck_one_uri(pkg, version, filename, uri):
> > +
>
> flake8 does not whine with or wihtout a leading empty line. I prefer
> when there is none, though...
>
> However, let's try something:
>
>     def sourcecheck_one_uri(pkg, version, filename, uri):
>         handler = dict()
>
>         def sourcecheck_file(...):
>             ...
>         handler['file'] = source_check_file
>
>         def sourcecheck_hg(...);
>             ...
>         handler['hg'] = source_check_file
>
>         try:
>             return handler[uri.split('://', 1)[0]](pkg, version, filename, uri)
>         except KeyError:
>             raise Exeption('Meh unknown URI type "{}"'.format(uri)) from None
>
> Not sure how nicer the code would be. At least, we get an easy one-liner
> to demux the URI type (plus the try-except boilerplate).

Sure, we can do something like that, thanks.

>
> > +    def sourcecheck_scp(pkg, version, filename, uri):
>
> Please define handler in alphabetical order.

OK

>
> > +        real_uri = uri.split('+', 1)[1] + '/' + filename
> > +        if real_uri.startswith('scp://'):
> > +            real_uri = real_uri[6:]
> > +        domain, path = real_uri.split(':', 1)
> > +        with open(os.devnull, 'w') as devnull:
> > +            ret = subprocess.call(
> > +                ['ssh', domain, 'test', '-f', path],
> > +                stderr=devnull
> > +            )
> > +        return ret == 0
> > +
> > +    def sourcecheck_hg(pkg, version, filename, uri):
> > +        real_uri = uri.split('+', 1)[1]
> > +        with open(os.devnull, 'w') as devnull:
> > +            ret = subprocess.call(
> > +                ['hg', 'identify', '--rev', version, real_uri],
> > +                stdout=devnull, stderr=devnull
> > +            )
> > +        return ret == 0
> > +
> > +    def sourcecheck_file(pkg, version, filename, uri):
> > +        real_uri = uri.split('+', 1)[1] + '/' + filename
> > +        if real_uri.startswith('file://'):
> > +            real_uri = real_uri[7:]
> > +        return os.path.exists(real_uri)
> > +
> > +    def sourcecheck_http(pkg, version, filename, uri):
> > +        real_uri = uri.split('+', 1)[1] + '/' + filename
> > +        with open(os.devnull, 'w') as devnull:
> > +            ret = subprocess.call(
> > +                ['wget', '--spider', real_uri],
> > +                stderr=devnull
> > +            )
> > +        return ret == 0
> > +
> > +    if uri.startswith('scp'):
> > +        handler = sourcecheck_scp
> > +    elif uri.startswith('hg'):
> > +        handler = sourcecheck_hg
> > +    elif uri.startswith('file'):
> > +        handler = sourcecheck_file
> > +    elif uri.startswith('http'):
> > +        handler = sourcecheck_http
> > +    else:
> > +        raise Exception("Cannot handle unknown URI type: '%s' for package '%s'" % (uri, pkg))
> > +
> > +    return handler(pkg, version, filename, uri)
> > +
> > +
> > +def sourcecheck_one_file(pkg, version, filename, uris):
> > +    result = any(
> > +        sourcecheck_one_uri(pkg, version, filename, uri)
> > +        for uri in uris
> > +    )
> > +    return pkg, version, filename, result
> > +
> > +
> > +def sourcecheck(files_to_check):
> > +
> > +    def process_result(result):
> > +        pkg, version, filename, success = result
> > +        if success:
> > +            print(' OK: pkg %s, filename %s' % (pkg, filename))
> > +        else:
> > +            sys.stderr.write('NOK: pkg %s, filename %s -- ERROR!\n' % (pkg, filename))
> > +
> > +    num_processes = multiprocessing.cpu_count() * 2
> > +    print('Dispatching over %s processes' % num_processes)
>
> Hmm... I don't much like this auto-parallelism... I think we should not
> try to do parallsism at all. But if you really insist, then make that a
> command line option (e.g. source-check -jN).

Might I ask why you don't like that?

The parallelism gives a great speed boost. Every connection that needs
to be set up takes some time.
When I run the script for just one defconfig with 180 download URIs on
my laptop with 4 cores:
- dispatched to 8 processes: 40 seconds
- dispatched to 1 process: 3 minutes

But we can indeed add an option to determine the amount of parallelism.

Best regards,
Thomas