All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [pull request] buildroot-test: misc enhancenments for br-reproduce-build
@ 2015-02-14 10:52 Yann E. MORIN
  2015-02-14 10:52 ` [Buildroot] [PATCH 1/4] br-reproduce-build: accept BASE_GIT and OUTPUT_DIR from the env Yann E. MORIN
                   ` (3 more replies)
  0 siblings, 4 replies; 15+ messages in thread
From: Yann E. MORIN @ 2015-02-14 10:52 UTC (permalink / raw)
  To: buildroot

Hello All!

This series contains a few improvements to the br-reproduce-build script
of the buildroot-test infrastructure:

  - allow specifying the 'configuration' in the environment, rather
    than modifying the script;

  - fix URL where to get files from; there is a redirect that is
    badly handled server-side. This is a quick fix to avoid having
    to twidle with the server;

  - preventively quote variables (in case of weird characters in paths)

Regards,
Yann E. MORIN.


The following changes since commit df8198a69a36e35979e130b383ac6e50a548a32f:

  autobuild-run: Do not build strace with outdated MIPS uClibc toolchains (2015-02-02 12:30:55 +0100)

are available in the git repository at:

  git://git.busybox.net/~ymorin/git/buildroot-test yem/reproduce

for you to fetch changes up to 0b43bc60a4856f45924e3814010dc49d0240b1a4:

  br-reproduce-build: quote all variables (2015-02-14 11:32:56 +0100)

----------------------------------------------------------------
Yann E. MORIN (4):
      br-reproduce-build: accept BASE_GIT and OUTPUT_DIR from the env
      br-reproduce-build: use current directory as the default for outputs
      br-reproduce-build: fix URL of gitid
      br-reproduce-build: quote all variables

 utils/br-reproduce-build | 30 ++++++++++++++++--------------
 1 file changed, 16 insertions(+), 14 deletions(-)

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

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

* [Buildroot] [PATCH 1/4] br-reproduce-build: accept BASE_GIT and OUTPUT_DIR from the env
  2015-02-14 10:52 [Buildroot] [pull request] buildroot-test: misc enhancenments for br-reproduce-build Yann E. MORIN
@ 2015-02-14 10:52 ` Yann E. MORIN
  2015-02-14 21:08   ` Thomas Petazzoni
  2015-02-14 10:52 ` [Buildroot] [PATCH 2/4] br-reproduce-build: use current directory as the default for outputs Yann E. MORIN
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 15+ messages in thread
From: Yann E. MORIN @ 2015-02-14 10:52 UTC (permalink / raw)
  To: buildroot

Currently, when we want to set those variables, one has to edit the
script, so would get a conflict on the next git-pull.

Instead, allow the user to specify those variables in the environment.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
---
 utils/br-reproduce-build | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/utils/br-reproduce-build b/utils/br-reproduce-build
index 7b4961c..a84bd5a 100755
--- a/utils/br-reproduce-build
+++ b/utils/br-reproduce-build
@@ -3,12 +3,12 @@
 # A directory that contains an existing Git repository of
 # Buildroot. The script will git clone from it instead of git cloning
 # from the official repo, in order to reduce the clone time.
-BASE_GIT=/home/test/buildroot/
+BASE_GIT="${BASE_GIT:-/home/test/buildroot/}"
 
 # Location where the output directories will be created. One
 # subdirectory, named after the build ID will be created for each
 # build.
-OUTPUT_DIR=/home/test/outputs/
+OUTPUT_DIR="${OUTPUT_DIR:-/home/test/outputs/}"
 
 if [ $# -ne 1 ] ; then
     echo "Usage: $0 buildid" ;
-- 
1.9.1

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

* [Buildroot] [PATCH 2/4] br-reproduce-build: use current directory as the default for outputs
  2015-02-14 10:52 [Buildroot] [pull request] buildroot-test: misc enhancenments for br-reproduce-build Yann E. MORIN
  2015-02-14 10:52 ` [Buildroot] [PATCH 1/4] br-reproduce-build: accept BASE_GIT and OUTPUT_DIR from the env Yann E. MORIN
@ 2015-02-14 10:52 ` Yann E. MORIN
  2015-02-14 21:08   ` Thomas Petazzoni
  2015-02-14 10:52 ` [Buildroot] [PATCH 3/4] br-reproduce-build: fix URL of gitid Yann E. MORIN
  2015-02-14 10:52 ` [Buildroot] [PATCH 4/4] br-reproduce-build: quote all variables Yann E. MORIN
  3 siblings, 1 reply; 15+ messages in thread
From: Yann E. MORIN @ 2015-02-14 10:52 UTC (permalink / raw)
  To: buildroot

This allows the user running something like:

    cd /somewhere
    /path/to/br-reproduce-build ID

Or even have br-reproduce-build in his PATH.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
---
 utils/br-reproduce-build | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/utils/br-reproduce-build b/utils/br-reproduce-build
index a84bd5a..c0dc530 100755
--- a/utils/br-reproduce-build
+++ b/utils/br-reproduce-build
@@ -8,7 +8,7 @@ BASE_GIT="${BASE_GIT:-/home/test/buildroot/}"
 # Location where the output directories will be created. One
 # subdirectory, named after the build ID will be created for each
 # build.
-OUTPUT_DIR="${OUTPUT_DIR:-/home/test/outputs/}"
+OUTPUT_DIR="${OUTPUT_DIR:-$(pwd)}"
 
 if [ $# -ne 1 ] ; then
     echo "Usage: $0 buildid" ;
-- 
1.9.1

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

* [Buildroot] [PATCH 3/4] br-reproduce-build: fix URL of gitid
  2015-02-14 10:52 [Buildroot] [pull request] buildroot-test: misc enhancenments for br-reproduce-build Yann E. MORIN
  2015-02-14 10:52 ` [Buildroot] [PATCH 1/4] br-reproduce-build: accept BASE_GIT and OUTPUT_DIR from the env Yann E. MORIN
  2015-02-14 10:52 ` [Buildroot] [PATCH 2/4] br-reproduce-build: use current directory as the default for outputs Yann E. MORIN
@ 2015-02-14 10:52 ` Yann E. MORIN
  2015-02-14 21:10   ` Thomas Petazzoni
  2015-02-14 10:52 ` [Buildroot] [PATCH 4/4] br-reproduce-build: quote all variables Yann E. MORIN
  3 siblings, 1 reply; 15+ messages in thread
From: Yann E. MORIN @ 2015-02-14 10:52 UTC (permalink / raw)
  To: buildroot

Reported-by: Fabio Porcedda <fabio.porcedda@gmail.com>
Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
---
 utils/br-reproduce-build | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/utils/br-reproduce-build b/utils/br-reproduce-build
index c0dc530..9987684 100755
--- a/utils/br-reproduce-build
+++ b/utils/br-reproduce-build
@@ -16,9 +16,11 @@ if [ $# -ne 1 ] ; then
     exit 1 ;
 fi
 
-BUILD_ID=$1
+# BUILD_ID must be in the form xxx/xxxyyyyyyyyyy (xxxyyyyy... being the sha1)
+BUILD_ID="${1#*/}"
+BUILD_ID="${BUILD_ID%${BUILD_ID#???}}/${BUILD_ID}"
 
-BUILD_DIR=${OUTPUT_DIR}/${BUILD_ID}
+BUILD_DIR="${OUTPUT_DIR}/${BUILD_ID#*/}"
 
 mkdir -p ${BUILD_DIR}
 if [ $? -ne 0 ] ; then
-- 
1.9.1

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

* [Buildroot] [PATCH 4/4] br-reproduce-build: quote all variables
  2015-02-14 10:52 [Buildroot] [pull request] buildroot-test: misc enhancenments for br-reproduce-build Yann E. MORIN
                   ` (2 preceding siblings ...)
  2015-02-14 10:52 ` [Buildroot] [PATCH 3/4] br-reproduce-build: fix URL of gitid Yann E. MORIN
@ 2015-02-14 10:52 ` Yann E. MORIN
  2015-02-14 21:10   ` Thomas Petazzoni
  3 siblings, 1 reply; 15+ messages in thread
From: Yann E. MORIN @ 2015-02-14 10:52 UTC (permalink / raw)
  To: buildroot

Even though we do not really support building in a path that contains
spaces (or other weird characters), it's still better to quote
variables.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
---
 utils/br-reproduce-build | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/utils/br-reproduce-build b/utils/br-reproduce-build
index 9987684..0033102 100755
--- a/utils/br-reproduce-build
+++ b/utils/br-reproduce-build
@@ -22,26 +22,26 @@ BUILD_ID="${BUILD_ID%${BUILD_ID#???}}/${BUILD_ID}"
 
 BUILD_DIR="${OUTPUT_DIR}/${BUILD_ID#*/}"
 
-mkdir -p ${BUILD_DIR}
+mkdir -p "${BUILD_DIR}"
 if [ $? -ne 0 ] ; then
     echo "Cannot create output directory"
     exit 1
 fi
 
-wget -O ${BUILD_DIR}/config http://autobuild.buildroot.org/results/${BUILD_ID}/config
+wget -O "${BUILD_DIR}/config http://autobuild.buildroot.org/results/${BUILD_ID}/config"
 if [ $? -ne 0 ] ; then
     echo "Cannot get configuration for build ${BUILD_ID}"
-    rm -f ${BUILD_DIR}
+    rm -f "${BUILD_DIR}"
     exit 1
 fi
 
-wget -O ${BUILD_DIR}/gitid http://autobuild.buildroot.org/results/${BUILD_ID}/gitid
+wget -O "${BUILD_DIR}/gitid http://autobuild.buildroot.org/results/${BUILD_ID}/gitid"
 
-cd ${BUILD_DIR}
-git clone ${BASE_GIT}
+cd "${BUILD_DIR}"
+git clone "${BASE_GIT}"
 if [ $? -ne 0 ] ; then
     echo "Cannot clone Buildroot Git repository"
-    rm -rf ${BUILD_DIR}
+    rm -rf "${BUILD_DIR}"
     exit 1
 fi
 
@@ -52,17 +52,17 @@ git remote set-url origin git://git.busybox.net/buildroot
 git pull
 if [ $? -ne 0 ] ; then
     echo "Cannot pull Buildroot Git repository"
-    rm -rf ${BUILD_DIR}
+    rm -rf "${BUILD_DIR}"
     exit 1
 fi
 
 git checkout $(cat ../gitid)
 if [ $? -ne 0 ] ; then
     echo "Cannot checkout commit " $(cat ../gitid)
-    rm -rf ${BUILD_DIR}
+    rm -rf "${BUILD_DIR}"
     exit 1
 fi
 
 mkdir ../output
-cp ${BUILD_DIR}/config ../output/.config
+cp "${BUILD_DIR}/config" ../output/.config
 make 2>&1 O=../output | tee logfile
-- 
1.9.1

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

* [Buildroot] [PATCH 1/4] br-reproduce-build: accept BASE_GIT and OUTPUT_DIR from the env
  2015-02-14 10:52 ` [Buildroot] [PATCH 1/4] br-reproduce-build: accept BASE_GIT and OUTPUT_DIR from the env Yann E. MORIN
@ 2015-02-14 21:08   ` Thomas Petazzoni
  2015-02-15  0:30     ` Yann E. MORIN
  0 siblings, 1 reply; 15+ messages in thread
From: Thomas Petazzoni @ 2015-02-14 21:08 UTC (permalink / raw)
  To: buildroot

Dear Yann E. MORIN,

On Sat, 14 Feb 2015 11:52:03 +0100, Yann E. MORIN wrote:
> Currently, when we want to set those variables, one has to edit the
> script, so would get a conflict on the next git-pull.
> 
> Instead, allow the user to specify those variables in the environment.
> 
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> ---
>  utils/br-reproduce-build | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Applied, thanks. Though maybe the chosen environment variable names are
a bit too generic to be globally exported in the user's .bashrc file,
for example.

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

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

* [Buildroot] [PATCH 2/4] br-reproduce-build: use current directory as the default for outputs
  2015-02-14 10:52 ` [Buildroot] [PATCH 2/4] br-reproduce-build: use current directory as the default for outputs Yann E. MORIN
@ 2015-02-14 21:08   ` Thomas Petazzoni
  0 siblings, 0 replies; 15+ messages in thread
From: Thomas Petazzoni @ 2015-02-14 21:08 UTC (permalink / raw)
  To: buildroot

Dear Yann E. MORIN,

On Sat, 14 Feb 2015 11:52:04 +0100, Yann E. MORIN wrote:
> This allows the user running something like:
> 
>     cd /somewhere
>     /path/to/br-reproduce-build ID
> 
> Or even have br-reproduce-build in his PATH.
> 
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> ---
>  utils/br-reproduce-build | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied, thanks.

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

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

* [Buildroot] [PATCH 3/4] br-reproduce-build: fix URL of gitid
  2015-02-14 10:52 ` [Buildroot] [PATCH 3/4] br-reproduce-build: fix URL of gitid Yann E. MORIN
@ 2015-02-14 21:10   ` Thomas Petazzoni
  2015-02-14 21:58     ` Yann E. MORIN
  0 siblings, 1 reply; 15+ messages in thread
From: Thomas Petazzoni @ 2015-02-14 21:10 UTC (permalink / raw)
  To: buildroot

Dear Yann E. MORIN,

On Sat, 14 Feb 2015 11:52:05 +0100, Yann E. MORIN wrote:
> Reported-by: Fabio Porcedda <fabio.porcedda@gmail.com>
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> ---
>  utils/br-reproduce-build | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/utils/br-reproduce-build b/utils/br-reproduce-build
> index c0dc530..9987684 100755
> --- a/utils/br-reproduce-build
> +++ b/utils/br-reproduce-build
> @@ -16,9 +16,11 @@ if [ $# -ne 1 ] ; then
>      exit 1 ;
>  fi
>  
> -BUILD_ID=$1
> +# BUILD_ID must be in the form xxx/xxxyyyyyyyyyy (xxxyyyyy... being the sha1)
> +BUILD_ID="${1#*/}"
> +BUILD_ID="${BUILD_ID%${BUILD_ID#???}}/${BUILD_ID}"
>  
> -BUILD_DIR=${OUTPUT_DIR}/${BUILD_ID}
> +BUILD_DIR="${OUTPUT_DIR}/${BUILD_ID#*/}"

This shell stuff is so complicated that I don't even understand what is
the behavior. When you say "BUILD_ID must be in the form
xxx/xxxyyyyyyyyyy", does it mean that the user is supposed to pass as
argument to the script a value in the form "xxx/xxxyyyyyyyyyyy" ? If
so, then it's clearly not the intended behavior: the full hash should
be sufficient.

Maybe a few more comments would be useful to understand the magic.

Thanks!

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

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

* [Buildroot] [PATCH 4/4] br-reproduce-build: quote all variables
  2015-02-14 10:52 ` [Buildroot] [PATCH 4/4] br-reproduce-build: quote all variables Yann E. MORIN
@ 2015-02-14 21:10   ` Thomas Petazzoni
  2015-02-14 22:00     ` Yann E. MORIN
  0 siblings, 1 reply; 15+ messages in thread
From: Thomas Petazzoni @ 2015-02-14 21:10 UTC (permalink / raw)
  To: buildroot

Dear Yann E. MORIN,

On Sat, 14 Feb 2015 11:52:06 +0100, Yann E. MORIN wrote:

> -wget -O ${BUILD_DIR}/config http://autobuild.buildroot.org/results/${BUILD_ID}/config
> +wget -O "${BUILD_DIR}/config http://autobuild.buildroot.org/results/${BUILD_ID}/config"

How can this work? You're putting both the -O argument and the URL
within the same quotes.

> -wget -O ${BUILD_DIR}/gitid http://autobuild.buildroot.org/results/${BUILD_ID}/gitid
> +wget -O "${BUILD_DIR}/gitid http://autobuild.buildroot.org/results/${BUILD_ID}/gitid"

Ditto.

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

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

* [Buildroot] [PATCH 3/4] br-reproduce-build: fix URL of gitid
  2015-02-14 21:10   ` Thomas Petazzoni
@ 2015-02-14 21:58     ` Yann E. MORIN
  2015-02-18 20:12       ` Fabio Porcedda
  0 siblings, 1 reply; 15+ messages in thread
From: Yann E. MORIN @ 2015-02-14 21:58 UTC (permalink / raw)
  To: buildroot

Thomas, All,

On 2015-02-14 22:10 +0100, Thomas Petazzoni spake thusly:
> On Sat, 14 Feb 2015 11:52:05 +0100, Yann E. MORIN wrote:
> > Reported-by: Fabio Porcedda <fabio.porcedda@gmail.com>
> > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> > ---
> >  utils/br-reproduce-build | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/utils/br-reproduce-build b/utils/br-reproduce-build
> > index c0dc530..9987684 100755
> > --- a/utils/br-reproduce-build
> > +++ b/utils/br-reproduce-build
> > @@ -16,9 +16,11 @@ if [ $# -ne 1 ] ; then
> >      exit 1 ;
> >  fi
> >  
> > -BUILD_ID=$1
> > +# BUILD_ID must be in the form xxx/xxxyyyyyyyyyy (xxxyyyyy... being the sha1)
> > +BUILD_ID="${1#*/}"
> > +BUILD_ID="${BUILD_ID%${BUILD_ID#???}}/${BUILD_ID}"
> >  
> > -BUILD_DIR=${OUTPUT_DIR}/${BUILD_ID}
> > +BUILD_DIR="${OUTPUT_DIR}/${BUILD_ID#*/}"
> 
> This shell stuff is so complicated that I don't even understand what is
> the behavior.

Ah, sorry, that's indeed not completely trivial (eveb though I can read
it quite clearly! ;-) )

What this does is:

  - BUILD_ID="${1#*/}"
    get rid of anything before a '/', included

  - ${BUILD_ID#???}
    get rid of the first three chars

  - ${BUILD_ID%foo}
    get rid of the trailing string 'foo', so:

  - ${BUILD_ID%${BUILD_ID#???}}
    get rid of all but the first three chars

Thus, what the code above does is:

  - if the user passes a sha1 xxxyyyyyy transform it to xxx/xxxyyyyy
    (since that the way results are organised on the website

  - if the user already passes xxx/xxxyyyyyyy we simply get rid of the
    leading xxx/ (even though it is the correct form) to keep the
    xxxyyyyyy form, and we are back to the first case, above.

> When you say "BUILD_ID must be in the form
> xxx/xxxyyyyyyyyyy", does it mean that the user is supposed to pass as
> argument to the script a value in the form "xxx/xxxyyyyyyyyyyy" ? If
> so, then it's clearly not the intended behavior: the full hash should
> be sufficient.

Yes, that's exactly the point of all the above: a sha1 should be enough.
However, if the user is smart enough to pass the xxx/xxxyyyyy form, we
still accept it.

> Maybe a few more comments would be useful to understand the magic.

Yes, obviously. Sorry, it seemed clear enough to me (but I'm very well
used to using shell tricks that in retrospect are not so obvious).

Thanks!

Regards,
Yann E. MORIN.

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

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

* [Buildroot] [PATCH 4/4] br-reproduce-build: quote all variables
  2015-02-14 21:10   ` Thomas Petazzoni
@ 2015-02-14 22:00     ` Yann E. MORIN
  0 siblings, 0 replies; 15+ messages in thread
From: Yann E. MORIN @ 2015-02-14 22:00 UTC (permalink / raw)
  To: buildroot

Thomas, All,

On 2015-02-14 22:10 +0100, Thomas Petazzoni spake thusly:
> On Sat, 14 Feb 2015 11:52:06 +0100, Yann E. MORIN wrote:
> 
> > -wget -O ${BUILD_DIR}/config http://autobuild.buildroot.org/results/${BUILD_ID}/config
> > +wget -O "${BUILD_DIR}/config http://autobuild.buildroot.org/results/${BUILD_ID}/config"
> 
> How can this work? You're putting both the -O argument and the URL
> within the same quotes.

Indeed, it can not work. And I noticed that when testing, and I fixed
it, but...

> > -wget -O ${BUILD_DIR}/gitid http://autobuild.buildroot.org/results/${BUILD_ID}/gitid
> > +wget -O "${BUILD_DIR}/gitid http://autobuild.buildroot.org/results/${BUILD_ID}/gitid"
> 
> Ditto.

... obviously, I forgot to commit --amend... :-(

Thanks!

Regards,
Yann E. MORIN.

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

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

* [Buildroot] [PATCH 1/4] br-reproduce-build: accept BASE_GIT and OUTPUT_DIR from the env
  2015-02-14 21:08   ` Thomas Petazzoni
@ 2015-02-15  0:30     ` Yann E. MORIN
  0 siblings, 0 replies; 15+ messages in thread
From: Yann E. MORIN @ 2015-02-15  0:30 UTC (permalink / raw)
  To: buildroot

Thomas, All,

On 2015-02-14 22:08 +0100, Thomas Petazzoni spake thusly:
> Dear Yann E. MORIN,
> 
> On Sat, 14 Feb 2015 11:52:03 +0100, Yann E. MORIN wrote:
> > Currently, when we want to set those variables, one has to edit the
> > script, so would get a conflict on the next git-pull.
> > 
> > Instead, allow the user to specify those variables in the environment.
> > 
> > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> > ---
> >  utils/br-reproduce-build | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> Applied, thanks. Though maybe the chosen environment variable names are
> a bit too generic to be globally exported in the user's .bashrc file,
> for example.

Indeed not. However, I use it like:

    BASE_GIT=~/cache/upstream/buildroot OUTPUT_DIR=$(pwd) \
    ./buildroot-test/utils/br-reproduce-build sha1

Which IMHO is good enough. :-)

Regards,
Yann E. MORIN.

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

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

* [Buildroot] [PATCH 3/4] br-reproduce-build: fix URL of gitid
  2015-02-14 21:58     ` Yann E. MORIN
@ 2015-02-18 20:12       ` Fabio Porcedda
  2015-02-18 20:26         ` Yann E. MORIN
  0 siblings, 1 reply; 15+ messages in thread
From: Fabio Porcedda @ 2015-02-18 20:12 UTC (permalink / raw)
  To: buildroot

On Sat, Feb 14, 2015 at 10:58 PM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> Thomas, All,
>
> On 2015-02-14 22:10 +0100, Thomas Petazzoni spake thusly:
>> On Sat, 14 Feb 2015 11:52:05 +0100, Yann E. MORIN wrote:
>> > Reported-by: Fabio Porcedda <fabio.porcedda@gmail.com>
>> > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
>> > ---
>> >  utils/br-reproduce-build | 6 ++++--
>> >  1 file changed, 4 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/utils/br-reproduce-build b/utils/br-reproduce-build
>> > index c0dc530..9987684 100755
>> > --- a/utils/br-reproduce-build
>> > +++ b/utils/br-reproduce-build
>> > @@ -16,9 +16,11 @@ if [ $# -ne 1 ] ; then
>> >      exit 1 ;
>> >  fi
>> >
>> > -BUILD_ID=$1
>> > +# BUILD_ID must be in the form xxx/xxxyyyyyyyyyy (xxxyyyyy... being the sha1)
>> > +BUILD_ID="${1#*/}"
>> > +BUILD_ID="${BUILD_ID%${BUILD_ID#???}}/${BUILD_ID}"
>> >
>> > -BUILD_DIR=${OUTPUT_DIR}/${BUILD_ID}
>> > +BUILD_DIR="${OUTPUT_DIR}/${BUILD_ID#*/}"
>>
>> This shell stuff is so complicated that I don't even understand what is
>> the behavior.
>
> Ah, sorry, that's indeed not completely trivial (eveb though I can read
> it quite clearly! ;-) )
>
> What this does is:
>
>   - BUILD_ID="${1#*/}"
>     get rid of anything before a '/', included
>
>   - ${BUILD_ID#???}
>     get rid of the first three chars
>
>   - ${BUILD_ID%foo}
>     get rid of the trailing string 'foo', so:
>
>   - ${BUILD_ID%${BUILD_ID#???}}
>     get rid of all but the first three chars
>
> Thus, what the code above does is:
>
>   - if the user passes a sha1 xxxyyyyyy transform it to xxx/xxxyyyyy
>     (since that the way results are organised on the website
>
>   - if the user already passes xxx/xxxyyyyyyy we simply get rid of the
>     leading xxx/ (even though it is the correct form) to keep the
>     xxxyyyyyy form, and we are back to the first case, above.
>
>> When you say "BUILD_ID must be in the form
>> xxx/xxxyyyyyyyyyy", does it mean that the user is supposed to pass as
>> argument to the script a value in the form "xxx/xxxyyyyyyyyyyy" ? If
>> so, then it's clearly not the intended behavior: the full hash should
>> be sufficient.
>
> Yes, that's exactly the point of all the above: a sha1 should be enough.
> However, if the user is smart enough to pass the xxx/xxxyyyyy form, we
> still accept it.
>
>> Maybe a few more comments would be useful to understand the magic.
>
> Yes, obviously. Sorry, it seemed clear enough to me (but I'm very well
> used to using shell tricks that in retrospect are not so obvious).

I think this patch is useful, are you going to send an updated version?

Thanks and BR
-- 
Fabio Porcedda

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

* [Buildroot] [PATCH 3/4] br-reproduce-build: fix URL of gitid
  2015-02-18 20:12       ` Fabio Porcedda
@ 2015-02-18 20:26         ` Yann E. MORIN
  2015-02-18 20:37           ` Fabio Porcedda
  0 siblings, 1 reply; 15+ messages in thread
From: Yann E. MORIN @ 2015-02-18 20:26 UTC (permalink / raw)
  To: buildroot

Fabio, All,

On 2015-02-18 21:12 +0100, Fabio Porcedda spake thusly:
> On Sat, Feb 14, 2015 at 10:58 PM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> > On 2015-02-14 22:10 +0100, Thomas Petazzoni spake thusly:
> >> On Sat, 14 Feb 2015 11:52:05 +0100, Yann E. MORIN wrote:
> >> > Reported-by: Fabio Porcedda <fabio.porcedda@gmail.com>
> >> > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> >> > ---
> >> >  utils/br-reproduce-build | 6 ++++--
> >> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >> >
> >> > diff --git a/utils/br-reproduce-build b/utils/br-reproduce-build
> >> > index c0dc530..9987684 100755
> >> > --- a/utils/br-reproduce-build
> >> > +++ b/utils/br-reproduce-build
> >> > @@ -16,9 +16,11 @@ if [ $# -ne 1 ] ; then
> >> >      exit 1 ;
> >> >  fi
> >> >
> >> > -BUILD_ID=$1
> >> > +# BUILD_ID must be in the form xxx/xxxyyyyyyyyyy (xxxyyyyy... being the sha1)
> >> > +BUILD_ID="${1#*/}"
> >> > +BUILD_ID="${BUILD_ID%${BUILD_ID#???}}/${BUILD_ID}"
> >> >
> >> > -BUILD_DIR=${OUTPUT_DIR}/${BUILD_ID}
> >> > +BUILD_DIR="${OUTPUT_DIR}/${BUILD_ID#*/}"
> >>
> >> This shell stuff is so complicated that I don't even understand what is
> >> the behavior.
> >
> > Ah, sorry, that's indeed not completely trivial (eveb though I can read
> > it quite clearly! ;-) )
> >
> > What this does is:
> >
> >   - BUILD_ID="${1#*/}"
> >     get rid of anything before a '/', included
> >
> >   - ${BUILD_ID#???}
> >     get rid of the first three chars
> >
> >   - ${BUILD_ID%foo}
> >     get rid of the trailing string 'foo', so:
> >
> >   - ${BUILD_ID%${BUILD_ID#???}}
> >     get rid of all but the first three chars
> >
> > Thus, what the code above does is:
> >
> >   - if the user passes a sha1 xxxyyyyyy transform it to xxx/xxxyyyyy
> >     (since that the way results are organised on the website
> >
> >   - if the user already passes xxx/xxxyyyyyyy we simply get rid of the
> >     leading xxx/ (even though it is the correct form) to keep the
> >     xxxyyyyyy form, and we are back to the first case, above.
> >
> >> When you say "BUILD_ID must be in the form
> >> xxx/xxxyyyyyyyyyy", does it mean that the user is supposed to pass as
> >> argument to the script a value in the form "xxx/xxxyyyyyyyyyyy" ? If
> >> so, then it's clearly not the intended behavior: the full hash should
> >> be sufficient.
> >
> > Yes, that's exactly the point of all the above: a sha1 should be enough.
> > However, if the user is smart enough to pass the xxx/xxxyyyyy form, we
> > still accept it.
> >
> >> Maybe a few more comments would be useful to understand the magic.
> >
> > Yes, obviously. Sorry, it seemed clear enough to me (but I'm very well
> > used to using shell tricks that in retrospect are not so obvious).
> 
> I think this patch is useful, are you going to send an updated version?

Yes, I will. Quoting Eric_L on IRC just now: "so many things to play
with, so little time to do..."

Regards,
Yann E. MORIN.

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

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

* [Buildroot] [PATCH 3/4] br-reproduce-build: fix URL of gitid
  2015-02-18 20:26         ` Yann E. MORIN
@ 2015-02-18 20:37           ` Fabio Porcedda
  0 siblings, 0 replies; 15+ messages in thread
From: Fabio Porcedda @ 2015-02-18 20:37 UTC (permalink / raw)
  To: buildroot

On Wed, Feb 18, 2015 at 9:26 PM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> Fabio, All,
>
> On 2015-02-18 21:12 +0100, Fabio Porcedda spake thusly:
>> On Sat, Feb 14, 2015 at 10:58 PM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
>> > On 2015-02-14 22:10 +0100, Thomas Petazzoni spake thusly:
>> >> On Sat, 14 Feb 2015 11:52:05 +0100, Yann E. MORIN wrote:
<snip>
>> >> Maybe a few more comments would be useful to understand the magic.
>> >
>> > Yes, obviously. Sorry, it seemed clear enough to me (but I'm very well
>> > used to using shell tricks that in retrospect are not so obvious).
>>
>> I think this patch is useful, are you going to send an updated version?
>
> Yes, I will. Quoting Eric_L on IRC just now: "so many things to play
> with, so little time to do..."

Great, I will wait patiently :)

Thanks
-- 
Fabio Porcedda

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

end of thread, other threads:[~2015-02-18 20:37 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-02-14 10:52 [Buildroot] [pull request] buildroot-test: misc enhancenments for br-reproduce-build Yann E. MORIN
2015-02-14 10:52 ` [Buildroot] [PATCH 1/4] br-reproduce-build: accept BASE_GIT and OUTPUT_DIR from the env Yann E. MORIN
2015-02-14 21:08   ` Thomas Petazzoni
2015-02-15  0:30     ` Yann E. MORIN
2015-02-14 10:52 ` [Buildroot] [PATCH 2/4] br-reproduce-build: use current directory as the default for outputs Yann E. MORIN
2015-02-14 21:08   ` Thomas Petazzoni
2015-02-14 10:52 ` [Buildroot] [PATCH 3/4] br-reproduce-build: fix URL of gitid Yann E. MORIN
2015-02-14 21:10   ` Thomas Petazzoni
2015-02-14 21:58     ` Yann E. MORIN
2015-02-18 20:12       ` Fabio Porcedda
2015-02-18 20:26         ` Yann E. MORIN
2015-02-18 20:37           ` Fabio Porcedda
2015-02-14 10:52 ` [Buildroot] [PATCH 4/4] br-reproduce-build: quote all variables Yann E. MORIN
2015-02-14 21:10   ` Thomas Petazzoni
2015-02-14 22:00     ` Yann E. MORIN

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.