Merge lp://qastaging/~pundiramit/linaro-android-build-tools/access_private_manifests into lp://qastaging/linaro-android-build-tools

Does it not make sense to read ID from some environment variable as well? (so people can simply set it in their .bashrc or .profile to ensure the same ID is used going forward)

Of course, if you make it an environment variable, I suppose naming it like LINARO_ANDROID_ACCESS_ID would be better than just "ID".

Btw, what are the reasons to replace ".*-bot" with ${ID}, and not simply replace "default-bot"?

I did try to read userID from .bashrc/.profile/.gitconfig/bazaar.conf at first but the thing is that this userID is generated by linaro-private git administration and it may not be consistent with a user's existing usernames. At least in my case, the admin gave me an username "amitpundir" whereas my launchpad/git/gerrit/bzr ID is "pundiramit".

I can certainly rename ID to a much understandable LINARO_ANDROID_ACCESS_ID environmental variable.

I did not understand the last concern.
I'm initializing ID with "default-bot", which is a default unauthorised bot and do not have access to any private gits. I can very well leave ID uninitialized as well.
Once user provide correct access ID, I override "default-bot" with user provided ID and then replace "*-bot" e.g. "linaro-big-little-switcher-bot" with ID. I used "*-bot" because it could be any project specific bot. I'm counting on future default bots to be named on $PROJECT-bot as well.

I think it's fine not to try to guess the user ID. What I meant was that it should be possible for someone to simply do something like:

  export LINARO_ANDROID_ACCESS_ID=my-linaro-username
  ./linaro_android_build_cmds.sh

If you unconditionally set the variable in the script, that will not be possible. You can do that by setting the variable in the script as (hyphen to separate the variable name and default value):

  VARNAME=${VARNAME-default value}

eg.

  LINARO_ANDROID_ACCESS_ID=${LINARO_ANDROID_ACCESS_ID-default-bot}

Then, someone would be able to put "export LINARO_ANDROID_ACCESS_ID=danilo" in their .bashrc and not worry about it when using "./linaro_android_build_cmds.sh".

Script fixed to check for already exported LINARO_ANDROID_ACCESS_ID. An exported LINARO_ANDROID_ACCESS_ID can be override with "./l_a_b_c.sh -l <access-id>" switch though.

This looks good, did you perhaps test it out (a link to a test build would be nice) to confirm escaping works as expected (it's all too easy to make a typo somewhere, and a best test is to get a runnable script out :).

I did test the changes separately on a local script. The best way, indeed, is to get a runnable script out and test it to confirm that escaping work as expected.

Pushed a couple of more/unrelated changes to this branch:
1) Make source_overlay optional.
2) Invoke apt-get, only for missing packages. This fixes an issue on hackbox where "apt-get install" returns exit code 1, when trying to install already installed packages.

The changes for LINARO_ANDROID_ACCESS_ID look good.

However, as I said in the previous MP at

  https://code.launchpad.net/~pundiramit/linaro-android-build-tools/labt/+merge/144239

I don't want us to allow builds on android-build.linaro.org without the overlay. How does this ensure this still remains the case?

Ok. With this new patchset, I changed how I was handling SOURCE_OVERLAY variable. Since SOURCE_OVERLAY variable is something which may or may not be directly used by jenkins for build purpose, I'm not playing around it anymore.

What I did this time is that I introduced a new flag SOURCE_OVERLAY_OPTIONAL which is set to "1" to make overlay optional. This SOURCE_OVERLAY_OPTIONAL flag will be toggled to "0" when we explicitly set overlay using "-o <overlay>" switch. To protect S_OVERLAY portion of the script, I have put relevant conditional checks for S_O_OPTIONAL flag.

Let me know if you have any concerns on this one.

This generally looks good. I think only one thing is missing: what happens when someone puts the SOURCE_OVERLAY_OPTIONAL in the build configuration on android-build?

Since the config file is generated on the build with the script

  http://bazaar.launchpad.net/~linaro-infrastructure/linaro-android-build-tools/trunk/view/head:/node/prepare_build_config.py

based on the data passed in from android-build.linaro.org build config, we should make sure that the SOURCE_OVERLAY_OPTIONAL triggers an error there.

So I figured out that even this script "build-scripts/build-android" can be modified to make sure that no one put SOURCE_OVERLAY_OPTIONAL in the official build configurations. Is my understanding correct?

-------------------------------
$ bzr diff
=== modified file 'build-scripts/build-android'
--- build-scripts/build-android 2012-12-19 10:05:03 +0000
+++ build-scripts/build-android 2013-01-25 11:29:39 +0000
@@ -14,6 +14,12 @@
     exit 1
 fi

+if [ "$SOURCE_OVERLAY_OPTIONAL" == "1" -o -n "$SOURCE_OVERLAY_OPTIONAL" ]; then
+ echo "ERROR: SOURCE_OVERLAY_OPTIONAL should not be set in official build configuration."
+ echo " It is meant to be set only in local build scripts to bypass overlays."
+ exit 1
+fi
+
 source "${BUILD_SCRIPT_ROOT}"/helpers

 trap infrastructure_error ERR

$
-------------------------------

Yeah, this looks good. I am testing your branch now with https://android-build.linaro.org/builds/~danilo/optional-overlay/: let's see if it works as expected. If it does, I'll merge it in.

Ok, I tested a few cases (based on galaxynexus-linaro build):

 1. SOURCE_OVERLAY missing, SOURCE_OVERLAY_OPTIONAL=1
    https://android-build.linaro.org/jenkins/job/danilo_optional-overlay/1/console

    Ok, fails with SOURCE_OVERLAY missing.

 2. SOURCE_OVERLAY present, SOURCE_OVERLAY_OPTIONAL=1
    https://android-build.linaro.org/jenkins/job/danilo_optional-overlay/2/console

    OK, fails with your new error message.

 3. SOURCE_OVERLAY present, no SOURCE_OVERLAY_OPTIONAL (regular builds)
    https://android-build.linaro.org/jenkins/job/danilo_optional-overlay/3/console

    Just starting now. If that one builds ok, I'll merge your branch asap.