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

[Amit Pundir]

Prompt for login/access-id when user try to download private manifests.

[Данило Шеган (danilo)]

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"?

review: Needs Fixing

[Amit Pundir (pundiramit)]

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.

[Данило Шеган (danilo)]

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".

[lp:~pundiramit/linaro-android-build-tools/access_private_manifests]

555. By Amit Pundir on 2013-01-23

user build script: check for already exported access ID

[Amit Pundir (pundiramit)]

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.

[Данило Шеган (danilo)]

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 :).

review: Approve

[Amit Pundir (pundiramit)]

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.

[lp:~pundiramit/linaro-android-build-tools/access_private_manifests]

556. By Amit Pundir on 2013-01-24

user build script: make SOURCE_OVERLAY optional

557. By Amit Pundir on 2013-01-24

user build script: invoke apt-get install only for missing packages

[Amit Pundir (pundiramit)]

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.

[Данило Шеган (danilo)]

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?

review: Needs Information

[lp:~pundiramit/linaro-android-build-tools/access_private_manifests]

558. By Amit Pundir on 2013-01-24

user build script: protect source overlay usage with a flag

[Amit Pundir (pundiramit)]

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.

[Данило Шеган (danilo)]

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.

[lp:~pundiramit/linaro-android-build-tools/access_private_manifests]

559. By Amit Pundir on 2013-01-25

build android: make sure no one sets SOURCE_OVERLAY_OPTIONAL in official build configurations

[Amit Pundir (pundiramit)]

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

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

[Данило Шеган (danilo)]

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.

[Данило Шеган (danilo)]

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.

review: Approve

=== 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:33:23 +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

=== modified file 'build-scripts/create-user-build-script'
--- build-scripts/create-user-build-script	2013-01-23 09:48:37 +0000
+++ build-scripts/create-user-build-script	2013-01-25 11:33:23 +0000
@@ -17,7 +17,7 @@
 	USAGE_SUM="'Usage: \$0 -m <manifest.xml> -o <overlay.tar> [ -t -d directory -l login ]'"
 	USAGE_OVERLAY="'\\n -m <manifest>    If -t is not used, then using a browser with cookies you\\n                  must download the pinned manifest from:\\n             $PINNED_MANIFEST_URL\\n -o               The path to the vendor required overlay.\\n                  Can be downloaded from http://snapshots.linaro.org/android/binaries/$SOURCE_OVERLAY\\n'"
 	USAGE_OPTOVERLAY="m:o:"
-	USAGE_OPTHANDLER="o   ) SOURCE_OVERLAY=\$OPTARG;;  m   ) MANIFEST=\$OPTARG;;"
+	USAGE_OPTHANDLER="o   ) SOURCE_OVERLAY=\$OPTARG; SOURCE_OVERLAY_OPTIONAL=0;;  m   ) MANIFEST=\$OPTARG;;"
 else
 	USAGE_SUM="'Usage: \$0 [ -t -d directory -l login ]'"
 fi
@@ -32,7 +32,10 @@
 
 EXACT=1
 DIR=android
-ID=default-bot
+if [ -z "\${LINARO_ANDROID_ACCESS_ID}" ] ; then
+	LINARO_ANDROID_ACCESS_ID=default-bot
+fi
+SOURCE_OVERLAY_OPTIONAL=1
 
 usage()
 {
@@ -44,7 +47,7 @@
 	echo "                   Default: \${DIR}"
 	echo " -l <login-id>     login-id to clone from linaro-private git repositories"
 	echo "                   If in doubt, kindly raise access request to [e-mail]"
-	echo "                   Default: \${ID}"
+	echo "                   Default: \${LINARO_ANDROID_ACCESS_ID}"
 	exit 1
 }
 
@@ -52,7 +55,7 @@
 	case    \$optn   in
 		$USAGE_OPTHANDLER
 		d   ) DIR=\$OPTARG;;
-		l   ) ID=\$OPTARG;;
+		l   ) LINARO_ANDROID_ACCESS_ID=\$OPTARG;;
 		t   ) EXACT=0;;
 		h   ) usage; exit 1;;
         esac
@@ -77,16 +80,14 @@
 fi
 
 echo "Checking and installing missing dependencies if any .. .."
-sudo apt-get install \${PKGS}
-
-MISSING=\`dpkg-query -W -f='\${Status}\n' \${PKGS} 2>&1 | grep 'No packages found matching' | cut -d' ' -f5\`
+MISSING=\`dpkg-query -W -f='\${Status}\n' \${PKGS} 2>&1 | grep -i 'No packages found matching' | cut -d' ' -f6\`
 if [ -n "\$MISSING" ] ; then
-	echo "Missing required packages:"
+	echo -n "Missing required packages: "
 	for m in \$MISSING ; do
 		echo -n "\${m%?} "
 	done
 	echo
-	exit 1
+	sudo apt-get install \${MISSING}
 fi
 
 EOF
@@ -96,9 +97,11 @@
 	echo "ERROR: no pinned manifest provided. Please download from $PINNED_MANIFEST_URL. This must be done from a browser that accepts cookies."
 	exit 1
 fi
-if [ "a\$SOURCE_OVERLAY" == "a" ]; then
-	echo "ERROR: no source overlay provided. Please download from http://snapshots.linaro.org/android/binaries/$SOURCE_OVERLAY. This must be done from a browser that accepts cookies."
-	exit 1
+if [ \$SOURCE_OVERLAY_OPTIONAL -ne 1 ]; then
+	if [ "a\$SOURCE_OVERLAY" == "a" ]; then
+		echo "ERROR: no source overlay provided. Please download from http://snapshots.linaro.org/android/binaries/$SOURCE_OVERLAY. This must be done from a browser that accepts cookies."
+		exit 1
+	fi
 fi
 EOF
 	fi
@@ -118,7 +121,24 @@
 jenkins_configs_method()
 {
 	cat <<EOF
-export MANIFEST_REPO=${MANIFEST_REPO}
+# check for linaro private manifests
+PM=\`echo ${MANIFEST_REPO} | grep -i "linaro-private" | wc -l\`
+if [ \${PM} -gt 0 ] ; then
+	if [ "\${LINARO_ANDROID_ACCESS_ID}" == "default-bot" ] ; then
+		echo "You must specify valid login/access-id to clone from linaro-private manifest repositories."
+		echo "Press "y" to continue (which may result in incomplete build or failure), OR"
+		echo "Press "n" to enter login details, OR"
+		echo "Press "h" for help."
+		read NEXT
+		if [ \${NEXT} == n ] ; then
+			echo "Enter login/access-id:"
+			read LINARO_ANDROID_ACCESS_ID
+		elif [ \${NEXT} == h ] ; then
+			usage
+		fi
+	fi
+fi
+export MANIFEST_REPO=`echo ${MANIFEST_REPO} | sed 's/\/\/.*-bot@/\/\/'"\${LINARO_ANDROID_ACCESS_ID}"'@/'`
 export MANIFEST_BRANCH=${MANIFEST_BRANCH}
 export MANIFEST_FILENAME=${MANIFEST_FILENAME}
 export TARGET_PRODUCT=${TARGET_PRODUCT}
@@ -182,7 +202,7 @@
 # check for linaro private git repositories
 PRI=\`grep -i "linaro-private" .repo/manifests/\${MANIFEST_FILENAME} | wc -l\`
 if [ \${PRI} -gt 0 ] ; then
-	if [ "\${ID}" == "default-bot" ] ; then
+	if [ "\${LINARO_ANDROID_ACCESS_ID}" == "default-bot" ] ; then
 		echo "You must specify valid login/access-id to clone from linaro-private git repositories."
 		echo "Press "y" to continue (which may result in incomplete build), OR"
 		echo "Press "n" to enter login details, OR"
@@ -190,12 +210,12 @@
 		read NEXT
 		if [ \${NEXT} == n ] ; then
 			echo "Enter login/access-id:"
-			read ID
+			read LINARO_ANDROID_ACCESS_ID
 		elif [ \${NEXT} == h ] ; then
 			usage
 		fi
 	fi
-	sed -i 's/\/\/.*-bot@/\/\/'"\${ID}"'@/' .repo/manifests/\${MANIFEST_FILENAME}
+	sed -i 's/\/\/.*-bot@/\/\/'"\${LINARO_ANDROID_ACCESS_ID}"'@/' .repo/manifests/\${MANIFEST_FILENAME}
 fi
 ./repo sync
 
@@ -213,8 +233,10 @@
 	if [ -n "$SOURCE_OVERLAY" ]; then
 		cat <<EOF
 
-# extract the vendor's source overlay
-tar -x -a -f "\$SOURCE_OVERLAY" -C .
+if [ \$SOURCE_OVERLAY_OPTIONAL -ne 1 ]; then
+	# extract the vendor's source overlay
+	tar -x -a -f "\$SOURCE_OVERLAY" -C .
+fi
 EOF
 	fi
 	echo