All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [autobuild 1/4] autobuild-run: prevent send_result from failing when gathering config files
@ 2015-04-12 14:34 Samuel Martin
  2015-04-12 14:34 ` [Buildroot] [autobuild 2/4] autobuild-run: also save CMake config log files on package failure Samuel Martin
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Samuel Martin @ 2015-04-12 14:34 UTC (permalink / raw)
  To: buildroot

- Make sure using absolute paths in os.path.relpath();
- os.makedirs() can fail if the directory already exists, so only create it
  when needed.

Signed-off-by: Samuel Martin <s.martin49@gmail.com>
---
 scripts/autobuild-run | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/scripts/autobuild-run b/scripts/autobuild-run
index 0e12080..a19d51b 100755
--- a/scripts/autobuild-run
+++ b/scripts/autobuild-run
@@ -653,15 +653,16 @@ def send_results(result, **kwargs):
         if not reason:
             return
 
-        srcroot = os.path.join(outputdir, "build", '-'.join(reason))
-        destroot = os.path.join(resultdir, '-'.join(reason))
+        srcroot = os.path.abspath(os.path.join(outputdir, "build", '-'.join(reason)))
+        destroot = os.path.abspath(os.path.join(resultdir, '-'.join(reason)))
 
         for root, dirs, files in os.walk(srcroot):
             dest = os.path.join(destroot, os.path.relpath(root, srcroot))
 
             for fname in files:
                 if fname == 'config.log':
-                    os.makedirs(dest)
+                    if not os.path.exists(dest):
+                        os.makedirs(dest)
                     shutil.copy(os.path.join(root, fname), os.path.join(dest, fname))
 
     copy_config_log_files()
-- 
2.3.5

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Buildroot] [autobuild 2/4] autobuild-run: also save CMake config log files on package failure
  2015-04-12 14:34 [Buildroot] [autobuild 1/4] autobuild-run: prevent send_result from failing when gathering config files Samuel Martin
@ 2015-04-12 14:34 ` Samuel Martin
  2015-04-12 14:34 ` [Buildroot] [autobuild 3/4] autobuild-run: sanitize make options Samuel Martin
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 11+ messages in thread
From: Samuel Martin @ 2015-04-12 14:34 UTC (permalink / raw)
  To: buildroot

Signed-off-by: Samuel Martin <s.martin49@gmail.com>
---
 scripts/autobuild-run | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/scripts/autobuild-run b/scripts/autobuild-run
index a19d51b..a7cdc12 100755
--- a/scripts/autobuild-run
+++ b/scripts/autobuild-run
@@ -655,12 +655,14 @@ def send_results(result, **kwargs):
 
         srcroot = os.path.abspath(os.path.join(outputdir, "build", '-'.join(reason)))
         destroot = os.path.abspath(os.path.join(resultdir, '-'.join(reason)))
+        config_files = ('config.log', 'CMakeCache.txt', 'CMakeError.log',
+            'CMakeOutput.log')
 
         for root, dirs, files in os.walk(srcroot):
             dest = os.path.join(destroot, os.path.relpath(root, srcroot))
 
             for fname in files:
-                if fname == 'config.log':
+                if fname in config_files:
                     if not os.path.exists(dest):
                         os.makedirs(dest)
                     shutil.copy(os.path.join(root, fname), os.path.join(dest, fname))
-- 
2.3.5

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Buildroot] [autobuild 3/4] autobuild-run: sanitize make options
  2015-04-12 14:34 [Buildroot] [autobuild 1/4] autobuild-run: prevent send_result from failing when gathering config files Samuel Martin
  2015-04-12 14:34 ` [Buildroot] [autobuild 2/4] autobuild-run: also save CMake config log files on package failure Samuel Martin
@ 2015-04-12 14:34 ` Samuel Martin
  2015-04-12 17:22   ` Thomas Petazzoni
  2015-04-12 14:34 ` [Buildroot] [autobuild 4/4] autobuild-run: allow to change default git uri and branch through env. vars Samuel Martin
  2015-04-12 17:20 ` [Buildroot] [autobuild 1/4] autobuild-run: prevent send_result from failing when gathering config files Thomas Petazzoni
  3 siblings, 1 reply; 11+ messages in thread
From: Samuel Martin @ 2015-04-12 14:34 UTC (permalink / raw)
  To: buildroot

So far, --make-opts allows the user to override any make option or
variable, especially '-C' and 'O=' which should not in the autobuild
context.

So, this change drop '-C' option and 'O=' and 'BR2_JLEVEL=' variables
from the --make-opts arguments.

Signed-off-by: Samuel Martin <s.martin49@gmail.com>
---
 scripts/autobuild-run | 37 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/scripts/autobuild-run b/scripts/autobuild-run
index a7cdc12..dbfc33e 100755
--- a/scripts/autobuild-run
+++ b/scripts/autobuild-run
@@ -773,6 +773,40 @@ def merge(dict_1, dict_2):
     return dict((str(key), dict_1.get(key) or dict_2.get(key))
                 for key in set(dict_2) | set(dict_1))
 
+def sanitize_make_opts(make_opts):
+    """ sanitize make options
+
+    - do not allow to override '-C' option
+    - do not allow to user deifined 'O=' and 'BR2_JLEVEL='
+    - print log when overloading 'BR2_DL_DIR='
+
+    Return the sanitized make options string.
+    """
+    make_opts = make_opts.split(" ")
+    for i, arg in enumerate(make_opts):
+        if arg.startswith("-C"):
+            # remove both '-C<path>' and '-C' arguments
+            warn = "WARN: sanitizing make-opts (removing arguments '%s" % arg
+            make_opts.remove(arg)
+            if arg == '-C':
+                # remove '<path>' in case of '-C <path>'
+                # (no need for incrementing i since make_opts[i] already points
+                # to '<path>' after arg (i.e. '-C') has been removed
+                warn += " %s" % make_opts[i]
+                make_opts.remove(make_opts[i])
+            warn += "')"
+            print(warn)
+        elif "=" in arg:
+            var = arg.split("=", 1)[0]
+            if var in ("BR2_DL_DIR",):
+                print("INFO: using user defined '%s' (%s)" % (var, arg))
+            elif var in ("BR2_JLEVEL", "O",):
+                warn = "WARN: sanitizing make variable (removing arguments '%s')" % arg
+                make_opts.remove(arg)
+                print(warn)
+    return " ".join(make_opts)
+
+
 def main():
 
     # Avoid locale settings of autobuilder machine leaking in, for example
@@ -833,6 +867,7 @@ def main():
         sys.exit(1)
 
     buildpid = multiprocessing.Array('i', int(args['--ninstances']))
+    make_opts = sanitize_make_opts(args['--make-opts'])
     processes = []
     for i in range(0, int(args['--ninstances'])):
         p = multiprocessing.Process(target=run_instance, kwargs=dict(
@@ -842,7 +877,7 @@ def main():
                 http_login = args['--http-login'],
                 http_password = args['--http-password'],
                 submitter = args['--submitter'],
-                make_opts = args['--make-opts'],
+                make_opts = make_opts,
                 upload = upload,
                 buildpid = buildpid
             ))
-- 
2.3.5

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Buildroot] [autobuild 4/4] autobuild-run: allow to change default git uri and branch through env. vars.
  2015-04-12 14:34 [Buildroot] [autobuild 1/4] autobuild-run: prevent send_result from failing when gathering config files Samuel Martin
  2015-04-12 14:34 ` [Buildroot] [autobuild 2/4] autobuild-run: also save CMake config log files on package failure Samuel Martin
  2015-04-12 14:34 ` [Buildroot] [autobuild 3/4] autobuild-run: sanitize make options Samuel Martin
@ 2015-04-12 14:34 ` Samuel Martin
  2015-04-12 17:26   ` Thomas Petazzoni
  2015-04-12 17:20 ` [Buildroot] [autobuild 1/4] autobuild-run: prevent send_result from failing when gathering config files Thomas Petazzoni
  3 siblings, 1 reply; 11+ messages in thread
From: Samuel Martin @ 2015-04-12 14:34 UTC (permalink / raw)
  To: buildroot

This is useful when hacking the autobuild-run script on some specific
Buildroot tree.

Signed-off-by: Samuel Martin <s.martin49@gmail.com>
---
 scripts/autobuild-run | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/scripts/autobuild-run b/scripts/autobuild-run
index dbfc33e..e1c6c5d 100755
--- a/scripts/autobuild-run
+++ b/scripts/autobuild-run
@@ -102,6 +102,16 @@ Format of the configuration file:
 Default values for the arguments are:
 
   %s
+
+Environment variables:
+
+  BUILDROOT_GIT_REPO_URI
+    This environment variable allows to fetch the given Buildroot git repository
+    instead of the official one.
+
+  BUILDROOT_GIT_BRANCH
+    This environment variable allows to change the Buildroot git branch instead
+    of master.
 """ % '\n  '.join(
     ['%s = %s' % (key, val) for (key, val) in defaults.items()])
 
@@ -297,8 +307,10 @@ def prepare_build(**kwargs):
     # Clone Buildroot. This only happens if the source directory
     # didn't exist already.
     srcdir = os.path.join(idir, "buildroot")
+    br_repo = os.environ.get("BUILDROOT_GIT_REPO_URI", "git://git.busybox.net/buildroot")
+    br_branch = os.environ.get("BUILDROOT_GIT_BRANCH", "master")
     if not os.path.exists(srcdir):
-        ret = subprocess.call(["git", "clone", "git://git.busybox.net/buildroot", srcdir],
+        ret = subprocess.call(["git", "clone", "-b", br_branch, br_repo, srcdir],
                               stdout=log, stderr=log)
         if ret != 0:
             log_write(log, "ERROR: could not clone Buildroot sources")
-- 
2.3.5

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Buildroot] [autobuild 1/4] autobuild-run: prevent send_result from failing when gathering config files
  2015-04-12 14:34 [Buildroot] [autobuild 1/4] autobuild-run: prevent send_result from failing when gathering config files Samuel Martin
                   ` (2 preceding siblings ...)
  2015-04-12 14:34 ` [Buildroot] [autobuild 4/4] autobuild-run: allow to change default git uri and branch through env. vars Samuel Martin
@ 2015-04-12 17:20 ` Thomas Petazzoni
  2015-04-12 18:15   ` Samuel Martin
  3 siblings, 1 reply; 11+ messages in thread
From: Thomas Petazzoni @ 2015-04-12 17:20 UTC (permalink / raw)
  To: buildroot

Dear Samuel Martin,

On Sun, 12 Apr 2015 16:34:42 +0200, Samuel Martin wrote:
> - Make sure using absolute paths in os.path.relpath();
> - os.makedirs() can fail if the directory already exists, so only create it
>   when needed.
> 
> Signed-off-by: Samuel Martin <s.martin49@gmail.com>
> ---
>  scripts/autobuild-run | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/scripts/autobuild-run b/scripts/autobuild-run
> index 0e12080..a19d51b 100755
> --- a/scripts/autobuild-run
> +++ b/scripts/autobuild-run
> @@ -653,15 +653,16 @@ def send_results(result, **kwargs):
>          if not reason:
>              return
>  
> -        srcroot = os.path.join(outputdir, "build", '-'.join(reason))
> -        destroot = os.path.join(resultdir, '-'.join(reason))
> +        srcroot = os.path.abspath(os.path.join(outputdir, "build", '-'.join(reason)))
> +        destroot = os.path.abspath(os.path.join(resultdir, '-'.join(reason)))

Can you explain why this is needed?

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Buildroot] [autobuild 3/4] autobuild-run: sanitize make options
  2015-04-12 14:34 ` [Buildroot] [autobuild 3/4] autobuild-run: sanitize make options Samuel Martin
@ 2015-04-12 17:22   ` Thomas Petazzoni
  2015-04-12 18:20     ` Samuel Martin
  0 siblings, 1 reply; 11+ messages in thread
From: Thomas Petazzoni @ 2015-04-12 17:22 UTC (permalink / raw)
  To: buildroot

Dear Samuel Martin,

On Sun, 12 Apr 2015 16:34:44 +0200, Samuel Martin wrote:
> So far, --make-opts allows the user to override any make option or
> variable, especially '-C' and 'O=' which should not in the autobuild
> context.
> 
> So, this change drop '-C' option and 'O=' and 'BR2_JLEVEL=' variables
> from the --make-opts arguments.
> 
> Signed-off-by: Samuel Martin <s.martin49@gmail.com>
> ---
>  scripts/autobuild-run | 37 ++++++++++++++++++++++++++++++++++++-
>  1 file changed, 36 insertions(+), 1 deletion(-)
> 
> diff --git a/scripts/autobuild-run b/scripts/autobuild-run
> index a7cdc12..dbfc33e 100755
> --- a/scripts/autobuild-run
> +++ b/scripts/autobuild-run
> @@ -773,6 +773,40 @@ def merge(dict_1, dict_2):
>      return dict((str(key), dict_1.get(key) or dict_2.get(key))
>                  for key in set(dict_2) | set(dict_1))
>  
> +def sanitize_make_opts(make_opts):
> +    """ sanitize make options
> +
> +    - do not allow to override '-C' option
> +    - do not allow to user deifined 'O=' and 'BR2_JLEVEL='
> +    - print log when overloading 'BR2_DL_DIR='
> +
> +    Return the sanitized make options string.
> +    """
> +    make_opts = make_opts.split(" ")
> +    for i, arg in enumerate(make_opts):
> +        if arg.startswith("-C"):
> +            # remove both '-C<path>' and '-C' arguments
> +            warn = "WARN: sanitizing make-opts (removing arguments '%s" % arg
> +            make_opts.remove(arg)
> +            if arg == '-C':
> +                # remove '<path>' in case of '-C <path>'
> +                # (no need for incrementing i since make_opts[i] already points
> +                # to '<path>' after arg (i.e. '-C') has been removed
> +                warn += " %s" % make_opts[i]
> +                make_opts.remove(make_opts[i])
> +            warn += "')"
> +            print(warn)
> +        elif "=" in arg:
> +            var = arg.split("=", 1)[0]
> +            if var in ("BR2_DL_DIR",):
> +                print("INFO: using user defined '%s' (%s)" % (var, arg))
> +            elif var in ("BR2_JLEVEL", "O",):
> +                warn = "WARN: sanitizing make variable (removing arguments '%s')" % arg
> +                make_opts.remove(arg)
> +                print(warn)
> +    return " ".join(make_opts)

To be honest, this seems a bit overkill to me. From my point of view
--make-opts is an advanced option, so users are supposed to understand
what they are doing. So I'm not very fond of adding a lot of fairly
complex code just to check for the validity of the values passed to
this option.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Buildroot] [autobuild 4/4] autobuild-run: allow to change default git uri and branch through env. vars.
  2015-04-12 14:34 ` [Buildroot] [autobuild 4/4] autobuild-run: allow to change default git uri and branch through env. vars Samuel Martin
@ 2015-04-12 17:26   ` Thomas Petazzoni
  2015-04-12 18:06     ` Samuel Martin
  0 siblings, 1 reply; 11+ messages in thread
From: Thomas Petazzoni @ 2015-04-12 17:26 UTC (permalink / raw)
  To: buildroot

Dear Samuel Martin,

On Sun, 12 Apr 2015 16:34:45 +0200, Samuel Martin wrote:
> This is useful when hacking the autobuild-run script on some specific
> Buildroot tree.
> 
> Signed-off-by: Samuel Martin <s.martin49@gmail.com>
> ---
>  scripts/autobuild-run | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/scripts/autobuild-run b/scripts/autobuild-run
> index dbfc33e..e1c6c5d 100755
> --- a/scripts/autobuild-run
> +++ b/scripts/autobuild-run
> @@ -102,6 +102,16 @@ Format of the configuration file:
>  Default values for the arguments are:
>  
>    %s
> +
> +Environment variables:
> +
> +  BUILDROOT_GIT_REPO_URI
> +    This environment variable allows to fetch the given Buildroot git repository
> +    instead of the official one.
> +
> +  BUILDROOT_GIT_BRANCH
> +    This environment variable allows to change the Buildroot git branch instead
> +    of master.
>  """ % '\n  '.join(
>      ['%s = %s' % (key, val) for (key, val) in defaults.items()])
>  
> @@ -297,8 +307,10 @@ def prepare_build(**kwargs):
>      # Clone Buildroot. This only happens if the source directory
>      # didn't exist already.
>      srcdir = os.path.join(idir, "buildroot")
> +    br_repo = os.environ.get("BUILDROOT_GIT_REPO_URI", "git://git.busybox.net/buildroot")
> +    br_branch = os.environ.get("BUILDROOT_GIT_BRANCH", "master")
>      if not os.path.exists(srcdir):
> -        ret = subprocess.call(["git", "clone", "git://git.busybox.net/buildroot", srcdir],
> +        ret = subprocess.call(["git", "clone", "-b", br_branch, br_repo, srcdir],
>                                stdout=log, stderr=log)
>          if ret != 0:
>              log_write(log, "ERROR: could not clone Buildroot sources")

Why environment variables? http://patchwork.ozlabs.org/patch/394526/
was proposing to use options, which was much better IMO. If you could
take this approach (after refreshing the patch), I would be OK.

Note that in the patch from Matt Weber, I also explicitly asked that
when a non-default repo/branch is used, the script should refuse to
submit the results to autobuild.b.o.

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Buildroot] [autobuild 4/4] autobuild-run: allow to change default git uri and branch through env. vars.
  2015-04-12 17:26   ` Thomas Petazzoni
@ 2015-04-12 18:06     ` Samuel Martin
  2015-04-13 12:34       ` Matthew Weber
  0 siblings, 1 reply; 11+ messages in thread
From: Samuel Martin @ 2015-04-12 18:06 UTC (permalink / raw)
  To: buildroot

On Sun, Apr 12, 2015 at 7:26 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Dear Samuel Martin,
>
> On Sun, 12 Apr 2015 16:34:45 +0200, Samuel Martin wrote:
>> This is useful when hacking the autobuild-run script on some specific
>> Buildroot tree.
>>
>> Signed-off-by: Samuel Martin <s.martin49@gmail.com>
>> ---
>>  scripts/autobuild-run | 14 +++++++++++++-
>>  1 file changed, 13 insertions(+), 1 deletion(-)
>>
>> diff --git a/scripts/autobuild-run b/scripts/autobuild-run
>> index dbfc33e..e1c6c5d 100755
>> --- a/scripts/autobuild-run
>> +++ b/scripts/autobuild-run
>> @@ -102,6 +102,16 @@ Format of the configuration file:
>>  Default values for the arguments are:
>>
>>    %s
>> +
>> +Environment variables:
>> +
>> +  BUILDROOT_GIT_REPO_URI
>> +    This environment variable allows to fetch the given Buildroot git repository
>> +    instead of the official one.
>> +
>> +  BUILDROOT_GIT_BRANCH
>> +    This environment variable allows to change the Buildroot git branch instead
>> +    of master.
>>  """ % '\n  '.join(
>>      ['%s = %s' % (key, val) for (key, val) in defaults.items()])
>>
>> @@ -297,8 +307,10 @@ def prepare_build(**kwargs):
>>      # Clone Buildroot. This only happens if the source directory
>>      # didn't exist already.
>>      srcdir = os.path.join(idir, "buildroot")
>> +    br_repo = os.environ.get("BUILDROOT_GIT_REPO_URI", "git://git.busybox.net/buildroot")
>> +    br_branch = os.environ.get("BUILDROOT_GIT_BRANCH", "master")
>>      if not os.path.exists(srcdir):
>> -        ret = subprocess.call(["git", "clone", "git://git.busybox.net/buildroot", srcdir],
>> +        ret = subprocess.call(["git", "clone", "-b", br_branch, br_repo, srcdir],
>>                                stdout=log, stderr=log)
>>          if ret != 0:
>>              log_write(log, "ERROR: could not clone Buildroot sources")
>
> Why environment variables?
Nothing else than offering a way to use a custom (buggy) tree to test
the autobuild-run script.

> http://patchwork.ozlabs.org/patch/394526/
> was proposing to use options, which was much better IMO. If you could
> take this approach (after refreshing the patch), I would be OK.
Fair enough. In fact I wrote the code before Yann mentioned it on
IRC... so, the patch flies away with the others ;)
I'll take this in my stack.

>
> Note that in the patch from Matt Weber, I also explicitly asked that
> when a non-default repo/branch is used, the script should refuse to
> submit the results to autobuild.b.o.
Make totally sense.

>
> Thanks,
>
> Thomas
> --
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com

Regards,

-- 
Samuel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Buildroot] [autobuild 1/4] autobuild-run: prevent send_result from failing when gathering config files
  2015-04-12 17:20 ` [Buildroot] [autobuild 1/4] autobuild-run: prevent send_result from failing when gathering config files Thomas Petazzoni
@ 2015-04-12 18:15   ` Samuel Martin
  0 siblings, 0 replies; 11+ messages in thread
From: Samuel Martin @ 2015-04-12 18:15 UTC (permalink / raw)
  To: buildroot

On Sun, Apr 12, 2015 at 7:20 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Dear Samuel Martin,
>
> On Sun, 12 Apr 2015 16:34:42 +0200, Samuel Martin wrote:
>> - Make sure using absolute paths in os.path.relpath();
>> - os.makedirs() can fail if the directory already exists, so only create it
>>   when needed.
>>
>> Signed-off-by: Samuel Martin <s.martin49@gmail.com>
>> ---
>>  scripts/autobuild-run | 7 ++++---
>>  1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/scripts/autobuild-run b/scripts/autobuild-run
>> index 0e12080..a19d51b 100755
>> --- a/scripts/autobuild-run
>> +++ b/scripts/autobuild-run
>> @@ -653,15 +653,16 @@ def send_results(result, **kwargs):
>>          if not reason:
>>              return
>>
>> -        srcroot = os.path.join(outputdir, "build", '-'.join(reason))
>> -        destroot = os.path.join(resultdir, '-'.join(reason))
>> +        srcroot = os.path.abspath(os.path.join(outputdir, "build", '-'.join(reason)))
>> +        destroot = os.path.abspath(os.path.join(resultdir, '-'.join(reason)))
>
> Can you explain why this is needed?

During my tests, I ran into some weird errors because it was trying to
mkdir some directories in the package builddir itself instead of in
the result location.
I certainly messed up something, I'll check again.
And... indeed after a close look at code, it should not be necessary...

Reagrds,

>
> Thanks,
>
> Thomas
> --
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com



-- 
Samuel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Buildroot] [autobuild 3/4] autobuild-run: sanitize make options
  2015-04-12 17:22   ` Thomas Petazzoni
@ 2015-04-12 18:20     ` Samuel Martin
  0 siblings, 0 replies; 11+ messages in thread
From: Samuel Martin @ 2015-04-12 18:20 UTC (permalink / raw)
  To: buildroot

On Sun, Apr 12, 2015 at 7:22 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Dear Samuel Martin,
>
> On Sun, 12 Apr 2015 16:34:44 +0200, Samuel Martin wrote:
>> So far, --make-opts allows the user to override any make option or
>> variable, especially '-C' and 'O=' which should not in the autobuild
>> context.
>>
>> So, this change drop '-C' option and 'O=' and 'BR2_JLEVEL=' variables
>> from the --make-opts arguments.
>>
>> Signed-off-by: Samuel Martin <s.martin49@gmail.com>
>> ---
>>  scripts/autobuild-run | 37 ++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 36 insertions(+), 1 deletion(-)
>>
>> diff --git a/scripts/autobuild-run b/scripts/autobuild-run
>> index a7cdc12..dbfc33e 100755
>> --- a/scripts/autobuild-run
>> +++ b/scripts/autobuild-run
>> @@ -773,6 +773,40 @@ def merge(dict_1, dict_2):
>>      return dict((str(key), dict_1.get(key) or dict_2.get(key))
>>                  for key in set(dict_2) | set(dict_1))
>>
>> +def sanitize_make_opts(make_opts):
>> +    """ sanitize make options
>> +
>> +    - do not allow to override '-C' option
>> +    - do not allow to user deifined 'O=' and 'BR2_JLEVEL='
>> +    - print log when overloading 'BR2_DL_DIR='
>> +
>> +    Return the sanitized make options string.
>> +    """
>> +    make_opts = make_opts.split(" ")
>> +    for i, arg in enumerate(make_opts):
>> +        if arg.startswith("-C"):
>> +            # remove both '-C<path>' and '-C' arguments
>> +            warn = "WARN: sanitizing make-opts (removing arguments '%s" % arg
>> +            make_opts.remove(arg)
>> +            if arg == '-C':
>> +                # remove '<path>' in case of '-C <path>'
>> +                # (no need for incrementing i since make_opts[i] already points
>> +                # to '<path>' after arg (i.e. '-C') has been removed
>> +                warn += " %s" % make_opts[i]
>> +                make_opts.remove(make_opts[i])
>> +            warn += "')"
>> +            print(warn)
>> +        elif "=" in arg:
>> +            var = arg.split("=", 1)[0]
>> +            if var in ("BR2_DL_DIR",):
>> +                print("INFO: using user defined '%s' (%s)" % (var, arg))
>> +            elif var in ("BR2_JLEVEL", "O",):
>> +                warn = "WARN: sanitizing make variable (removing arguments '%s')" % arg
>> +                make_opts.remove(arg)
>> +                print(warn)
>> +    return " ".join(make_opts)
>
> To be honest, this seems a bit overkill to me. From my point of view
> --make-opts is an advanced option, so users are supposed to understand
> what they are doing. So I'm not very fond of adding a lot of fairly
> complex code just to check for the validity of the values passed to
> this option.

Fair enough!
I'm happy to not make my life harder when hacking this script! ;-)

Regards,

>
> Best regards,
>
> Thomas
> --
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com


-- 
Samuel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Buildroot] [autobuild 4/4] autobuild-run: allow to change default git uri and branch through env. vars.
  2015-04-12 18:06     ` Samuel Martin
@ 2015-04-13 12:34       ` Matthew Weber
  0 siblings, 0 replies; 11+ messages in thread
From: Matthew Weber @ 2015-04-13 12:34 UTC (permalink / raw)
  To: buildroot

Sam,

On Sun, Apr 12, 2015 at 1:06 PM, Samuel Martin <s.martin49@gmail.com> wrote:
> On Sun, Apr 12, 2015 at 7:26 PM, Thomas Petazzoni
> <thomas.petazzoni@free-electrons.com> wrote:
>> Dear Samuel Martin,
>>
>> On Sun, 12 Apr 2015 16:34:45 +0200, Samuel Martin wrote:
>>> This is useful when hacking the autobuild-run script on some specific
>>> Buildroot tree.
>>>
>>> Signed-off-by: Samuel Martin <s.martin49@gmail.com>
>>> ---
>>>  scripts/autobuild-run | 14 +++++++++++++-
>>>  1 file changed, 13 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/scripts/autobuild-run b/scripts/autobuild-run
>>> index dbfc33e..e1c6c5d 100755
>>> --- a/scripts/autobuild-run
>>> +++ b/scripts/autobuild-run
>>> @@ -102,6 +102,16 @@ Format of the configuration file:
>>>  Default values for the arguments are:
>>>
>>>    %s
>>> +
>>> +Environment variables:
>>> +
>>> +  BUILDROOT_GIT_REPO_URI
>>> +    This environment variable allows to fetch the given Buildroot git repository
>>> +    instead of the official one.
>>> +
>>> +  BUILDROOT_GIT_BRANCH
>>> +    This environment variable allows to change the Buildroot git branch instead
>>> +    of master.
>>>  """ % '\n  '.join(
>>>      ['%s = %s' % (key, val) for (key, val) in defaults.items()])
>>>
>>> @@ -297,8 +307,10 @@ def prepare_build(**kwargs):
>>>      # Clone Buildroot. This only happens if the source directory
>>>      # didn't exist already.
>>>      srcdir = os.path.join(idir, "buildroot")
>>> +    br_repo = os.environ.get("BUILDROOT_GIT_REPO_URI", "git://git.busybox.net/buildroot")
>>> +    br_branch = os.environ.get("BUILDROOT_GIT_BRANCH", "master")
>>>      if not os.path.exists(srcdir):
>>> -        ret = subprocess.call(["git", "clone", "git://git.busybox.net/buildroot", srcdir],
>>> +        ret = subprocess.call(["git", "clone", "-b", br_branch, br_repo, srcdir],
>>>                                stdout=log, stderr=log)
>>>          if ret != 0:
>>>              log_write(log, "ERROR: could not clone Buildroot sources")
>>
>> Why environment variables?
> Nothing else than offering a way to use a custom (buggy) tree to test
> the autobuild-run script.
>
>> http://patchwork.ozlabs.org/patch/394526/
>> was proposing to use options, which was much better IMO. If you could
>> take this approach (after refreshing the patch), I would be OK.
> Fair enough. In fact I wrote the code before Yann mentioned it on
> IRC... so, the patch flies away with the others ;)
> I'll take this in my stack.

I'll gladly review/test when you have a v2 of this patch.  You beat me
to rebasing and sending in a refresh of my old patchset :-)

>
>>
>> Note that in the patch from Matt Weber, I also explicitly asked that
>> when a non-default repo/branch is used, the script should refuse to
>> submit the results to autobuild.b.o.
> Make totally sense.
>
>>
>> Thanks,
>>
>> Thomas
>> --
>> Thomas Petazzoni, CTO, Free Electrons
>> Embedded Linux, Kernel and Android engineering
>> http://free-electrons.com
>
> Regards,
>
> --
> Samuel
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot



-- 
Matthew L Weber / Pr Software Engineer
Airborne Information Systems / Security Systems and Software / Secure Platforms
MS 131-100, C Ave NE, Cedar Rapids, IA, 52498, USA
www.rockwellcollins.com

Note: Any Export License Required Information and License Restricted
Third Party Intellectual Property (TPIP) content must be encrypted and
sent to matthew.weber at corp.rockwellcollins.com.

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2015-04-13 12:34 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-12 14:34 [Buildroot] [autobuild 1/4] autobuild-run: prevent send_result from failing when gathering config files Samuel Martin
2015-04-12 14:34 ` [Buildroot] [autobuild 2/4] autobuild-run: also save CMake config log files on package failure Samuel Martin
2015-04-12 14:34 ` [Buildroot] [autobuild 3/4] autobuild-run: sanitize make options Samuel Martin
2015-04-12 17:22   ` Thomas Petazzoni
2015-04-12 18:20     ` Samuel Martin
2015-04-12 14:34 ` [Buildroot] [autobuild 4/4] autobuild-run: allow to change default git uri and branch through env. vars Samuel Martin
2015-04-12 17:26   ` Thomas Petazzoni
2015-04-12 18:06     ` Samuel Martin
2015-04-13 12:34       ` Matthew Weber
2015-04-12 17:20 ` [Buildroot] [autobuild 1/4] autobuild-run: prevent send_result from failing when gathering config files Thomas Petazzoni
2015-04-12 18:15   ` Samuel Martin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.