[Buildroot] [autobuild v2 1/3] autobuild-run: prevent send_result from failing when gathering config files

[Buildroot] [autobuild v2 1/3] autobuild-run: prevent send_result from failing when gathering config files

[Samuel Martin]

os.makedirs() can fail if the directory already exists, so only create it
when needed.

Signed-off-by: Samuel Martin <s.martin49@gmail.com>

---
changes v1->v2
- remove useless abspath call
---
 scripts/autobuild-run | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/scripts/autobuild-run b/scripts/autobuild-run
index 0e12080..6e83de9 100755
--- a/scripts/autobuild-run
+++ b/scripts/autobuild-run
@@ -661,7 +661,8 @@ def send_results(result, **kwargs):
 
             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.7

[Buildroot] [autobuild v2 2/3] autobuild-run: also save CMake config log files on package failure

[Samuel Martin]

Signed-off-by: Samuel Martin <s.martin49@gmail.com>

---
changes v1->v2
- none
---
 scripts/autobuild-run | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/scripts/autobuild-run b/scripts/autobuild-run
index 6e83de9..01e3265 100755
--- a/scripts/autobuild-run
+++ b/scripts/autobuild-run
@@ -655,12 +655,14 @@ def send_results(result, **kwargs):
 
         srcroot = os.path.join(outputdir, "build", '-'.join(reason))
         destroot = 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.7

[Buildroot] [autobuild v2 2/3] autobuild-run: also save CMake config log files on package failure

[André Erdmann]

2015/4/30 Samuel Martin <s.martin49@gmail.com>:
> Signed-off-by: Samuel Martin <s.martin49@gmail.com>
> 
> ---
> changes v1->v2
> - none
> ---
>  scripts/autobuild-run | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/scripts/autobuild-run b/scripts/autobuild-run
> index 6e83de9..01e3265 100755
> --- a/scripts/autobuild-run
> +++ b/scripts/autobuild-run
> @@ -655,12 +655,14 @@ def send_results(result, **kwargs):
>  
>          srcroot = os.path.join(outputdir, "build", '-'.join(reason))
>          destroot = os.path.join(resultdir, '-'.join(reason))
> +        config_files = ('config.log', 'CMakeCache.txt', 'CMakeError.log',
> +            'CMakeOutput.log')
>

config_files = set(('config.log', ...))

It's negligible in terms of real time difference (<= 0.0d seconds),
but for efficiency reasons, don't use a tuple or list when doing
lookups ("fname in config_files").

os.walk() might return 100s or 1000s of file names in total,
and for each fname, you have to through the entire list/tuple
(until fname found or end of list, whatever comes first).
Data structures like set/frozenset/dict perform lookups faster (on average).
  
>          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))
> 


 

[Buildroot] [autobuild v2 2/3] autobuild-run: also save CMake config log files on package failure

[Thomas Petazzoni]

Dear Andr? Erdmann,

On Thu, 30 Apr 2015 00:54:54 +0200, Andr? Erdmann wrote:

> config_files = set(('config.log', ...))
> 
> It's negligible in terms of real time difference (<= 0.0d seconds),
> but for efficiency reasons, don't use a tuple or list when doing
> lookups ("fname in config_files").
> 
> os.walk() might return 100s or 1000s of file names in total,
> and for each fname, you have to through the entire list/tuple
> (until fname found or end of list, whatever comes first).
> Data structures like set/frozenset/dict perform lookups faster (on average).

Can you submit a follow-up patch implementing this suggestion?

Thanks a lot!

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

[Buildroot] [autobuild v2 2/3] autobuild-run: also save CMake config log files on package failure

[Thomas Petazzoni]

Dear Samuel Martin,

On Wed, 29 Apr 2015 22:10:16 +0200, Samuel Martin wrote:
> Signed-off-by: Samuel Martin <s.martin49@gmail.com>
> 
> ---
> changes v1->v2
> - none
> ---
>  scripts/autobuild-run | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Applied, thanks.

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

[Buildroot] [autobuild v2 3/3] autobuild-run: allow to change default git uri and branch on the command line

[Samuel Martin]

This is useful when hacking the autobuild-run script on some specific
Buildroot tree, or to run the script with custom tree.

Result upload is automatically disable when one of these setting are not
the default ones.

Cc: Matt Weber <Matthew.Weber@rockwellcollins.com>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Signed-off-by: Samuel Martin <s.martin49@gmail.com>

---
changes v1->v2:
- use cmdline options instead of env. vars.
---
 scripts/autobuild-run | 41 +++++++++++++++++++++++++++++++++--------
 1 file changed, 33 insertions(+), 8 deletions(-)

diff --git a/scripts/autobuild-run b/scripts/autobuild-run
index 01e3265..427d333 100755
--- a/scripts/autobuild-run
+++ b/scripts/autobuild-run
@@ -57,6 +57,9 @@
 
 from __future__ import print_function
 
+BUILDROOT_GIT_REPO_URI = "git://git.busybox.net/buildroot"
+BUILDROOT_GIT_BRANCH = "master"
+
 # Don't tell docopt about the defaults, as it would not allow the following
 # priority hierarchy for arguments: command-line > config file > defaults
 defaults = {
@@ -64,6 +67,8 @@ defaults = {
     '--njobs': '1',
     '--submitter': 'N/A',
     '--make-opts': '',
+    '--br-git-uri': BUILDROOT_GIT_REPO_URI,
+    '--br-git-branch': BUILDROOT_GIT_BRANCH,
 }
 
 doc = """autobuild-run - run Buildroot autobuilder
@@ -84,6 +89,14 @@ Options:
   --make-opts OPTSTRING          string of extra options to pass to Buildroot
                                  make, such as specific command wrappers
                                  [default: ]
+  --br-git-uri GIT_REPO_URI      Buildroot git repository uri used from the instances.
+                                 When an alternative git repo uri is set, result uplaod
+                                 is disabled.
+                                 [default: the official Buildroot repository]
+  --br-git-branch GIT_BRANCH     Buildroot git branch used for the instance
+                                 When an alternative git branch is set, result uplaod
+                                 is disabled.
+                                 [default: master]
   -c, --config CONFIG            path to configuration file
 
 Format of the configuration file:
@@ -98,6 +111,8 @@ Format of the configuration file:
    http-login = <value>
    http-password = <value>
    submitter = <value>
+   br-git-uri = <value>
+   br-git-branch = <value>
 
 Default values for the arguments are:
 
@@ -296,17 +311,17 @@ def prepare_build(**kwargs):
 
     # Clone Buildroot. This only happens if the source directory
     # didn't exist already.
-    srcdir = os.path.join(idir, "buildroot")
+    srcdir = os.path.abspath(os.path.join(idir, "buildroot"))
     if not os.path.exists(srcdir):
-        ret = subprocess.call(["git", "clone", "git://git.busybox.net/buildroot", srcdir],
+        ret = subprocess.call(["git", "clone", "-b", kwargs['br_git_branch'],
+                              kwargs['br_git_repo_uri'], srcdir],
                               stdout=log, stderr=log)
         if ret != 0:
             log_write(log, "ERROR: could not clone Buildroot sources")
             return -1
 
     # Update the Buildroot sources.
-    abssrcdir = os.path.abspath(srcdir)
-    ret = subprocess.call(["git", "pull"], cwd=abssrcdir, stdout=log, stderr=log)
+    ret = subprocess.call(["git", "pull"], cwd=srcdir, stdout=log, stderr=log)
     if ret != 0:
         log_write(log, "ERROR: could not pull Buildroot sources")
         return -1
@@ -794,14 +809,22 @@ def main():
 
     # http_login/password could theoretically be allowed as empty, so check
     # explicitly on None.
-    upload = (args['--http-login'] is not None) \
-             and (args['--http-password'] is not None)
+    upload = True
+    if args['--http-login'] is None or args['--http-password'] is None:
+        print("WARN: due to the lack of http login/password details, results will not be submitted")
+        upload = False
+    # disable result upload when using alternative git repository and/or branch
+    if args['--br-git-uri'] != BUILDROOT_GIT_REPO_URI \
+            or args['--br-git-branch'] != BUILDROOT_GIT_BRANCH:
+        print("WARN: using alternative git repository and/or branch, results will not be submitted")
+        upload = False
+
     if upload:
         sysinfo.needed_progs.append("curl")
     else:
-        print("WARN: due to the lack of http login/password details, results will not be submitted")
         print("WARN: tarballs of results will be kept locally only")
 
+
     if not sysinfo.check_requirements():
         sys.exit(1)
 
@@ -844,7 +867,9 @@ def main():
                 submitter = args['--submitter'],
                 make_opts = args['--make-opts'],
                 upload = upload,
-                buildpid = buildpid
+                buildpid = buildpid,
+                br_git_repo_uri = args['--br-git-uri'],
+                br_git_branch = args['--br-git-branch']
             ))
         p.start()
         processes.append(p)
-- 
2.3.7

[Buildroot] [autobuild v2 3/3] autobuild-run: allow to change default git uri and branch on the command line

[André Erdmann]

2015/4/29 Samuel Martin <s.martin49@gmail.com>:
> This is useful when hacking the autobuild-run script on some specific
> Buildroot tree, or to run the script with custom tree.
> 
> Result upload is automatically disable when one of these setting are not
> the default ones.
> 
> Cc: Matt Weber <Matthew.Weber@rockwellcollins.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Signed-off-by: Samuel Martin <s.martin49@gmail.com>
> 
> ---
> changes v1->v2:
> - use cmdline options instead of env. vars.
> ---
>  scripts/autobuild-run | 41 +++++++++++++++++++++++++++++++++--------
>  1 file changed, 33 insertions(+), 8 deletions(-)
> 
> diff --git a/scripts/autobuild-run b/scripts/autobuild-run
> index 01e3265..427d333 100755
> --- a/scripts/autobuild-run
> +++ b/scripts/autobuild-run
> @@ -57,6 +57,9 @@
>  
>  from __future__ import print_function
>  
> +BUILDROOT_GIT_REPO_URI = "git://git.busybox.net/buildroot"
> +BUILDROOT_GIT_BRANCH = "master"
> +
>  # Don't tell docopt about the defaults, as it would not allow the following
>  # priority hierarchy for arguments: command-line > config file > defaults
>  defaults = {
> @@ -64,6 +67,8 @@ defaults = {
>      '--njobs': '1',
>      '--submitter': 'N/A',
>      '--make-opts': '',
> +    '--br-git-uri': BUILDROOT_GIT_REPO_URI,
> +    '--br-git-branch': BUILDROOT_GIT_BRANCH,
>  }
>  
>  doc = """autobuild-run - run Buildroot autobuilder
> @@ -84,6 +89,14 @@ Options:
>    --make-opts OPTSTRING          string of extra options to pass to Buildroot
>                                   make, such as specific command wrappers
>                                   [default: ]
> +  --br-git-uri GIT_REPO_URI      Buildroot git repository uri used from the instances.
> +                                 When an alternative git repo uri is set, result uplaod
> +                                 is disabled.
> +                                 [default: the official Buildroot repository]

Don't add "[default: <sth>]" here - <sth> will end up in the config ("args" variable),
and has higher priority than any other configuration method (config file, defaults).

The result is that if you run autobuild-run without "--br-git-uri ...", it will try
to fetch from "the official Buildroot repository" and not "git://..." (BUILDROOT_GIT_REPO_URI).

> +  --br-git-branch GIT_BRANCH     Buildroot git branch used for the instance
> +                                 When an alternative git branch is set, result uplaod
> +                                 is disabled.
> +                                 [default: master]

The same applies here.

>    -c, --config CONFIG            path to configuration file
>  
>  Format of the configuration file:
> @@ -98,6 +111,8 @@ Format of the configuration file:
>     http-login = <value>
>     http-password = <value>
>     submitter = <value>
> +   br-git-uri = <value>
> +   br-git-branch = <value>
>  
>  Default values for the arguments are:
>  
> @@ -296,17 +311,17 @@ def prepare_build(**kwargs):
>  
>      # Clone Buildroot. This only happens if the source directory
>      # didn't exist already.
> -    srcdir = os.path.join(idir, "buildroot")
> +    srcdir = os.path.abspath(os.path.join(idir, "buildroot"))
>      if not os.path.exists(srcdir):
> -        ret = subprocess.call(["git", "clone", "git://git.busybox.net/buildroot", srcdir],
> +        ret = subprocess.call(["git", "clone", "-b", kwargs['br_git_branch'],
> +                              kwargs['br_git_repo_uri'], srcdir],
>                                stdout=log, stderr=log)
>          if ret != 0:
>              log_write(log, "ERROR: could not clone Buildroot sources")
>              return -1
>  
>      # Update the Buildroot sources.
> -    abssrcdir = os.path.abspath(srcdir)
> -    ret = subprocess.call(["git", "pull"], cwd=abssrcdir, stdout=log, stderr=log)
> +    ret = subprocess.call(["git", "pull"], cwd=srcdir, stdout=log, stderr=log)

--br-git-{uri,branch} are ignored if srcdir already exists,
which should be documented or changed code-wise.

I'd add a note to the help message for now
("applies to new instance directories" or so).


>      if ret != 0:
>          log_write(log, "ERROR: could not pull Buildroot sources")
>          return -1
> @@ -794,14 +809,22 @@ def main():
>  
>      # http_login/password could theoretically be allowed as empty, so check
>      # explicitly on None.
> -    upload = (args['--http-login'] is not None) \
> -             and (args['--http-password'] is not None)
> +    upload = True
> +    if args['--http-login'] is None or args['--http-password'] is None:
> +        print("WARN: due to the lack of http login/password details, results will not be submitted")
> +        upload = False
> +    # disable result upload when using alternative git repository and/or branch
> +    if args['--br-git-uri'] != BUILDROOT_GIT_REPO_URI \
> +            or args['--br-git-branch'] != BUILDROOT_GIT_BRANCH:
> +        print("WARN: using alternative git repository and/or branch, results will not be submitted")
> +        upload = False
> +
>      if upload:
>          sysinfo.needed_progs.append("curl")
>      else:
> -        print("WARN: due to the lack of http login/password details, results will not be submitted")
>          print("WARN: tarballs of results will be kept locally only")
>  
> +
>      if not sysinfo.check_requirements():
>          sys.exit(1)
>  
> @@ -844,7 +867,9 @@ def main():
>                  submitter = args['--submitter'],
>                  make_opts = args['--make-opts'],
>                  upload = upload,
> -                buildpid = buildpid
> +                buildpid = buildpid,
> +                br_git_repo_uri = args['--br-git-uri'],
> +                br_git_branch = args['--br-git-branch']
>              ))
>          p.start()
>          processes.append(p)
> 

-- 
Andr?
 

[Buildroot] [autobuild v2 3/3] autobuild-run: allow to change default git uri and branch on the command line

[Samuel Martin]

Hi Andre, all,

Thanks a lot for your review.

On Thu, Apr 30, 2015 at 12:25 AM, Andr? Erdmann <dywi@mailerd.de> wrote:
> 2015/4/29 Samuel Martin <s.martin49@gmail.com>:
>> This is useful when hacking the autobuild-run script on some specific
>> Buildroot tree, or to run the script with custom tree.
>>
>> Result upload is automatically disable when one of these setting are not
>> the default ones.
>>
>> Cc: Matt Weber <Matthew.Weber@rockwellcollins.com>
>> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
>> Signed-off-by: Samuel Martin <s.martin49@gmail.com>
>>
>> ---
>> changes v1->v2:
>> - use cmdline options instead of env. vars.
>> ---
>>  scripts/autobuild-run | 41 +++++++++++++++++++++++++++++++++--------
>>  1 file changed, 33 insertions(+), 8 deletions(-)
>>
>> diff --git a/scripts/autobuild-run b/scripts/autobuild-run
>> index 01e3265..427d333 100755
>> --- a/scripts/autobuild-run
>> +++ b/scripts/autobuild-run
>> @@ -57,6 +57,9 @@
>>
>>  from __future__ import print_function
>>
>> +BUILDROOT_GIT_REPO_URI = "git://git.busybox.net/buildroot"
>> +BUILDROOT_GIT_BRANCH = "master"
>> +
>>  # Don't tell docopt about the defaults, as it would not allow the following
>>  # priority hierarchy for arguments: command-line > config file > defaults
>>  defaults = {
>> @@ -64,6 +67,8 @@ defaults = {
>>      '--njobs': '1',
>>      '--submitter': 'N/A',
>>      '--make-opts': '',
>> +    '--br-git-uri': BUILDROOT_GIT_REPO_URI,
>> +    '--br-git-branch': BUILDROOT_GIT_BRANCH,
>>  }
>>
>>  doc = """autobuild-run - run Buildroot autobuilder
>> @@ -84,6 +89,14 @@ Options:
>>    --make-opts OPTSTRING          string of extra options to pass to Buildroot
>>                                   make, such as specific command wrappers
>>                                   [default: ]
>> +  --br-git-uri GIT_REPO_URI      Buildroot git repository uri used from the instances.
>> +                                 When an alternative git repo uri is set, result uplaod
>> +                                 is disabled.
>> +                                 [default: the official Buildroot repository]
>
> Don't add "[default: <sth>]" here - <sth> will end up in the config ("args" variable),
> and has higher priority than any other configuration method (config file, defaults).
>
> The result is that if you run autobuild-run without "--br-git-uri ...", it will try
> to fetch from "the official Buildroot repository" and not "git://..." (BUILDROOT_GIT_REPO_URI).
>

So, we cannot document the defaults in the doc if we also want to make
them available in the config file... :-/

So the options are:
- either we correctly set the default git uri/branch in the doc
string, but we cannot set them via the config file;
- or we remove them from the doc, and we can still set them via the config file.
Inputs on this point from actual users will be helpful.

>> +  --br-git-branch GIT_BRANCH     Buildroot git branch used for the instance
>> +                                 When an alternative git branch is set, result uplaod
>> +                                 is disabled.
>> +                                 [default: master]
>
> The same applies here.
>
>>    -c, --config CONFIG            path to configuration file
>>
>>  Format of the configuration file:
>> @@ -98,6 +111,8 @@ Format of the configuration file:
>>     http-login = <value>
>>     http-password = <value>
>>     submitter = <value>
>> +   br-git-uri = <value>
>> +   br-git-branch = <value>
>>
>>  Default values for the arguments are:
>>
>> @@ -296,17 +311,17 @@ def prepare_build(**kwargs):
>>
>>      # Clone Buildroot. This only happens if the source directory
>>      # didn't exist already.
>> -    srcdir = os.path.join(idir, "buildroot")
>> +    srcdir = os.path.abspath(os.path.join(idir, "buildroot"))
>>      if not os.path.exists(srcdir):
>> -        ret = subprocess.call(["git", "clone", "git://git.busybox.net/buildroot", srcdir],
>> +        ret = subprocess.call(["git", "clone", "-b", kwargs['br_git_branch'],
>> +                              kwargs['br_git_repo_uri'], srcdir],
>>                                stdout=log, stderr=log)
>>          if ret != 0:
>>              log_write(log, "ERROR: could not clone Buildroot sources")
>>              return -1
>>
>>      # Update the Buildroot sources.
>> -    abssrcdir = os.path.abspath(srcdir)
>> -    ret = subprocess.call(["git", "pull"], cwd=abssrcdir, stdout=log, stderr=log)
>> +    ret = subprocess.call(["git", "pull"], cwd=srcdir, stdout=log, stderr=log)
>
> --br-git-{uri,branch} are ignored if srcdir already exists,
> which should be documented or changed code-wise.
>
> I'd add a note to the help message for now
> ("applies to new instance directories" or so).

I figured that too, but forgot to mention it :-/
I also prefer handling this in another patch ;-)

Another solution (not tested, btw) could be emptying the instance
directories at the beginning of the script, how about this?

>
>
>>      if ret != 0:
>>          log_write(log, "ERROR: could not pull Buildroot sources")
>>          return -1
>> @@ -794,14 +809,22 @@ def main():
>>
>>      # http_login/password could theoretically be allowed as empty, so check
>>      # explicitly on None.
>> -    upload = (args['--http-login'] is not None) \
>> -             and (args['--http-password'] is not None)
>> +    upload = True
>> +    if args['--http-login'] is None or args['--http-password'] is None:
>> +        print("WARN: due to the lack of http login/password details, results will not be submitted")
>> +        upload = False
>> +    # disable result upload when using alternative git repository and/or branch
>> +    if args['--br-git-uri'] != BUILDROOT_GIT_REPO_URI \
>> +            or args['--br-git-branch'] != BUILDROOT_GIT_BRANCH:
>> +        print("WARN: using alternative git repository and/or branch, results will not be submitted")
>> +        upload = False
>> +
>>      if upload:
>>          sysinfo.needed_progs.append("curl")
>>      else:
>> -        print("WARN: due to the lack of http login/password details, results will not be submitted")
>>          print("WARN: tarballs of results will be kept locally only")
>>
>> +
>>      if not sysinfo.check_requirements():
>>          sys.exit(1)
>>
>> @@ -844,7 +867,9 @@ def main():
>>                  submitter = args['--submitter'],
>>                  make_opts = args['--make-opts'],
>>                  upload = upload,
>> -                buildpid = buildpid
>> +                buildpid = buildpid,
>> +                br_git_repo_uri = args['--br-git-uri'],
>> +                br_git_branch = args['--br-git-branch']
>>              ))
>>          p.start()
>>          processes.append(p)
>>
>
> --
> Andr?
>

Regards,

-- 
Samuel

[Buildroot] [autobuild v2 3/3] autobuild-run: allow to change default git uri and branch on the command line

[André Erdmann]

Hi,

2015-04-30 7:20 GMT+02:00 Samuel Martin <s.martin49@gmail.com>:
> Hi Andre, all,
>
> Thanks a lot for your review.
>
> On Thu, Apr 30, 2015 at 12:25 AM, Andr? Erdmann <dywi@mailerd.de> wrote:
>> 2015/4/29 Samuel Martin <s.martin49@gmail.com>:
>>> This is useful when hacking the autobuild-run script on some specific
>>> Buildroot tree, or to run the script with custom tree.
>>>
>>> Result upload is automatically disable when one of these setting are not
>>> the default ones.
>>>
>>> Cc: Matt Weber <Matthew.Weber@rockwellcollins.com>
>>> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
>>> Signed-off-by: Samuel Martin <s.martin49@gmail.com>
>>>
>>> ---
>>> changes v1->v2:
>>> - use cmdline options instead of env. vars.
>>> ---
>>>  scripts/autobuild-run | 41 +++++++++++++++++++++++++++++++++--------
>>>  1 file changed, 33 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/scripts/autobuild-run b/scripts/autobuild-run
>>> index 01e3265..427d333 100755
>>> --- a/scripts/autobuild-run
>>> +++ b/scripts/autobuild-run
>>> @@ -57,6 +57,9 @@
>>>
>>>  from __future__ import print_function
>>>
>>> +BUILDROOT_GIT_REPO_URI = "git://git.busybox.net/buildroot"
>>> +BUILDROOT_GIT_BRANCH = "master"
>>> +
>>>  # Don't tell docopt about the defaults, as it would not allow the following
>>>  # priority hierarchy for arguments: command-line > config file > defaults
>>>  defaults = {
>>> @@ -64,6 +67,8 @@ defaults = {
>>>      '--njobs': '1',
>>>      '--submitter': 'N/A',
>>>      '--make-opts': '',
>>> +    '--br-git-uri': BUILDROOT_GIT_REPO_URI,
>>> +    '--br-git-branch': BUILDROOT_GIT_BRANCH,
>>>  }
>>>
>>>  doc = """autobuild-run - run Buildroot autobuilder
>>> @@ -84,6 +89,14 @@ Options:
>>>    --make-opts OPTSTRING          string of extra options to pass to Buildroot
>>>                                   make, such as specific command wrappers
>>>                                   [default: ]
>>> +  --br-git-uri GIT_REPO_URI      Buildroot git repository uri used from the instances.
>>> +                                 When an alternative git repo uri is set, result uplaod
>>> +                                 is disabled.
>>> +                                 [default: the official Buildroot repository]
>>
>> Don't add "[default: <sth>]" here - <sth> will end up in the config ("args" variable),
>> and has higher priority than any other configuration method (config file, defaults).
>>
>> The result is that if you run autobuild-run without "--br-git-uri ...", it will try
>> to fetch from "the official Buildroot repository" and not "git://..." (BUILDROOT_GIT_REPO_URI).
>>
>
> So, we cannot document the defaults in the doc if we also want to make
> them available in the config file... :-/
>

The defaults get automatically added at the end of the help message,
below "Default values for the arguments are:".


> So the options are:
> - either we correctly set the default git uri/branch in the doc
> string, but we cannot set them via the config file;
> - or we remove them from the doc, and we can still set them via the config file.
> Inputs on this point from actual users will be helpful.
>
>>> +  --br-git-branch GIT_BRANCH     Buildroot git branch used for the instance
>>> +                                 When an alternative git branch is set, result uplaod
>>> +                                 is disabled.
>>> +                                 [default: master]
>>
>> The same applies here.
>>
>>>    -c, --config CONFIG            path to configuration file
>>>
>>>  Format of the configuration file:
>>> @@ -98,6 +111,8 @@ Format of the configuration file:
>>>     http-login = <value>
>>>     http-password = <value>
>>>     submitter = <value>
>>> +   br-git-uri = <value>
>>> +   br-git-branch = <value>
>>>
>>>  Default values for the arguments are:
>>>
>>> @@ -296,17 +311,17 @@ def prepare_build(**kwargs):
>>>
>>>      # Clone Buildroot. This only happens if the source directory
>>>      # didn't exist already.
>>> -    srcdir = os.path.join(idir, "buildroot")
>>> +    srcdir = os.path.abspath(os.path.join(idir, "buildroot"))
>>>      if not os.path.exists(srcdir):
>>> -        ret = subprocess.call(["git", "clone", "git://git.busybox.net/buildroot", srcdir],
>>> +        ret = subprocess.call(["git", "clone", "-b", kwargs['br_git_branch'],
>>> +                              kwargs['br_git_repo_uri'], srcdir],
>>>                                stdout=log, stderr=log)
>>>          if ret != 0:
>>>              log_write(log, "ERROR: could not clone Buildroot sources")
>>>              return -1
>>>
>>>      # Update the Buildroot sources.
>>> -    abssrcdir = os.path.abspath(srcdir)
>>> -    ret = subprocess.call(["git", "pull"], cwd=abssrcdir, stdout=log, stderr=log)
>>> +    ret = subprocess.call(["git", "pull"], cwd=srcdir, stdout=log, stderr=log)
>>
>> --br-git-{uri,branch} are ignored if srcdir already exists,
>> which should be documented or changed code-wise.
>>
>> I'd add a note to the help message for now
>> ("applies to new instance directories" or so).
>
> I figured that too, but forgot to mention it :-/
> I also prefer handling this in another patch ;-)
>
> Another solution (not tested, btw) could be emptying the instance
> directories at the beginning of the script, how about this?
>

In my view, reusing files is an essential feature, because it keeps
the bandwidth requirements at a reasonable level.

If you wipe the instance directories, you lose both the git dirs and
src download dirs (*) whenever the script is started (server reboot,
local testing, ...).

Another approach would be to remove the instances' git srcdirs only
(how to detect that an "rm -r" is necessary?), but my preferred
solution would be having a single git mirror directory maintained
(i.e. regularly updated)) by the "master" process and let worker
processes clone/pull from it.

(*) side note: as part of the build process, worker processes remove a
few randomly chosen files from the dl dir to ensure that these files
can be re-downloaded.


> [snip]


-- 
Andr?

[Buildroot] [autobuild v2 3/3] autobuild-run: allow to change default git uri and branch on the command line

[Thomas Petazzoni]

Dear Samuel Martin,

On Wed, 29 Apr 2015 22:10:17 +0200, Samuel Martin wrote:
> This is useful when hacking the autobuild-run script on some specific
> Buildroot tree, or to run the script with custom tree.
> 
> Result upload is automatically disable when one of these setting are not
> the default ones.
> 
> Cc: Matt Weber <Matthew.Weber@rockwellcollins.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Signed-off-by: Samuel Martin <s.martin49@gmail.com>

Considering the comments made by Andr?, I've marked this patch as
Changes Requested in patchwork.

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

[Buildroot] [autobuild v2 1/3] autobuild-run: prevent send_result from failing when gathering config files

[Thomas Petazzoni]

Dear Samuel Martin,

On Wed, 29 Apr 2015 22:10:15 +0200, Samuel Martin wrote:
> os.makedirs() can fail if the directory already exists, so only create it
> when needed.
> 
> Signed-off-by: Samuel Martin <s.martin49@gmail.com>
> 
> ---
> changes v1->v2
> - remove useless abspath call
> ---
>  scripts/autobuild-run | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Applied, thanks.

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