Merge lp:~dooferlad/linaro-android-build-tools/add_ext_tarball into lp:linaro-android-build-tools

[James Tunnicliffe]

A list of external tarballs specified by the $EXTERNAL_TARBALL environment variable are unpacked into $BUILD_SCRIPT_ROOT/../../build/external_tarballs (/mnt/jenkins/workspace/<build name>/build/external_tarballs).

[Paul Sokolovsky (pfalcon)]

This looks nice. Some comments (not worth fixing as such): I try to avoid using pushd in scripts as produces (apparently not suppressible) output which looks strange in bigger context. Also, there is get_url_basename() function, might make sense to use it.

One bigger point which I raised right when request for this feature was made - how would you use contents of such tarball in a build? Its full path would be different for each build, so you couldn't have nice static path to specify in build config for example. The BP should be recording my proposal of making a symlink /tmp/<tarball_basename> -> /mnt/jenkins/workspace/<build name>/build/external_tarballs/<tarball_basename> to facilitate this.

review: Needs Information

[James Tunnicliffe (dooferlad)]

On 30 January 2012 10:01, Paul Sokolovsky <email address hidden> wrote:
> Review: Needs Information
>
> This looks nice. Some comments (not worth fixing as such): I try to avoid
> using pushd in scripts as produces (apparently not suppressible) output
> which looks strange in bigger context. Also, there is get_url_basename()
> function, might make sense to use it.

Good point - I will use get_url_basename(). Not sure if it is worth
replacing pushd / popd since the script produces a lot of output
anyway and they are useful.

> One bigger point which I raised right when request for this feature was
> made - how would you use contents of such tarball in a build? Its full
> path would be different for each build, so you couldn't have nice static
> path to specify in build config for example. The BP should be recording my
> proposal of making a symlink /tmp/<tarball_basename> ->
> /mnt/jenkins/workspace/<build name>/build/external_tarballs/<tarball_basename>
> to facilitate this.

ext_tarball_dir=$BUILD_SCRIPT_ROOT/../../build/external_tarballs is
where I unpack to. I could export this variable (but capitalised) for
the build scripts to use. Using an environment variable seems cleaner
than a fixed path.

What do you think?

--
James Tunnicliffe

[Paul Sokolovsky (pfalcon)]

On Tue, 31 Jan 2012 12:07:26 -0000
James Tunnicliffe <email address hidden> wrote:

> On 30 January 2012 10:01, Paul Sokolovsky
> <email address hidden> wrote:
> > Review: Needs Information
> >
> > This looks nice. Some comments (not worth fixing as such): I try to
> > avoid using pushd in scripts as produces (apparently not
> > suppressible) output which looks strange in bigger context. Also,
> > there is get_url_basename() function, might make sense to use it.
>
> Good point - I will use get_url_basename(). Not sure if it is worth
> replacing pushd / popd since the script produces a lot of output
> anyway and they are useful.
>
> > One bigger point which I raised right when request for this feature
> > was made - how would you use contents of such tarball in a build?
> > Its full path would be different for each build, so you couldn't
> > have nice static path to specify in build config for example. The
> > BP should be recording my proposal of making a
> > symlink /tmp/<tarball_basename> -> /mnt/jenkins/workspace/<build
> > name>/build/external_tarballs/<tarball_basename> to facilitate this.
>
> ext_tarball_dir=$BUILD_SCRIPT_ROOT/../../build/external_tarballs is
> where I unpack to. I could export this variable (but capitalised) for
> the build scripts to use. Using an environment variable seems cleaner
> than a fixed path.
>
> What do you think?
>

Maybe cleaner, but more complicated (as in "more complicated than
something not complicated at all"). I'm not sure defining var in a
build script would be transparent, we want to have explicit config in
a build config, so it all was easily reproducible w/o android-build
(e.g., locally). Original usecase from Android team was to using
a separate toolchain for a specific component. My solution: they define
a make var (FOO_COMPONENT_GCC) with sensible default, then in build
config may override it with
FOO_COMPONENT_GCC=/tmp/some-toolchain/bin/gcc . Your solution?

Anyway, I don't pledge, fairly speaking, they requested ability to
expand tarball into build, requested it some time ago and didn't bump
priority, so this patch can go as is, and if they have additional
reqs, that can be done later.

--
Best Regards,
Paul

Linaro.org | Open source software for ARM SoCs
Follow Linaro: http://www.facebook.com/pages/Linaro
http://twitter.com/#!/linaroorg - http://www.linaro.org/linaro-blog

[Paul Sokolovsky (pfalcon)]

review: Approve

=== modified file 'build-scripts/build-android'
--- build-scripts/build-android	2012-01-06 11:21:23 +0000
+++ build-scripts/build-android	2012-01-26 16:51:30 +0000
@@ -80,6 +80,10 @@
     unpack_overlays "$SOURCE_OVERLAY"
 fi
 
+if [ -n "$EXTERNAL_TARBALL" ]; then
+    unpack_external_tarball "$EXTERNAL_TARBALL"
+fi
+
 if [ -x "${TARGET_TOOLS_PREFIX}gcc" ]; then
     ${TARGET_TOOLS_PREFIX}gcc -v
 fi

=== modified file 'build-scripts/helpers'
--- build-scripts/helpers	2012-01-16 21:02:08 +0000
+++ build-scripts/helpers	2012-01-26 16:51:30 +0000
@@ -83,3 +83,38 @@
     done
     IFS="$ifs_save"
 }
+
+unpack_external_tarball () {
+    ext_tarball_dir=$BUILD_SCRIPT_ROOT/../../build/external_tarballs
+
+    # Always start clean
+    if [[ -d $ext_tarball_dir ]]; then
+        rm -rf $ext_tarball_dir
+    fi
+
+    mkdir -p $ext_tarball_dir
+    pushd $ext_tarball_dir
+
+    local ifs_save="$IFS"
+    IFS=";"
+    for p in $1; do
+        regex_is_http="^https?://"
+        regex_file=".*/(.*?)$"
+
+        if [[ $p =~ $regex_is_http ]]; then
+            # File is on a remote web server, wget it
+            wget --quiet $p
+        fi
+
+        if [[ $p =~ $regex_file ]]; then
+            filename=${BASH_REMATCH[1]}
+            tar -x -a -f "$filename" -C $ext_tarball_dir
+        else
+            echo "Fatal Error: $p - file name can not be determined"
+            exit 1
+        fi
+    done
+    IFS="$ifs_save"
+    popd
+}
+