Add 'quick-check' option to install-build-deps.sh

build/install-build-deps.sh

Index: build/install-build-deps.sh
diff --git a/build/install-build-deps.sh b/build/install-build-deps.sh
index cda72a66978c7b365c8ef2bb6f61f1c36e56bbdd..13bdb631d9e538b6e6c326e96d0b6dfbdbdb2d32 100755
--- a/build/install-build-deps.sh
+++ b/build/install-build-deps.sh
@@ -17,6 +17,10 @@ usage() {
   echo "--[no-]chromeos-fonts: enable or disable installation of Chrome OS"\
        "fonts"
   echo "--no-prompt: silently select standard options/defaults"
+  echo "--quick-check: quickly try to determine if dependencies are installed"
+  echo "               (this avoids interactive prompts and sudo commands,"
+  echo "               so might not be 100% accurate)"
+  echo "--unsupported: attempt installation even on unsupported systems"
   echo "Script will prompt interactively if options not given."
   exit 1
 }
@@ -41,6 +45,7 @@ do
   --no-prompt)              do_default=1
                             do_quietly="-qq --assume-yes"
     ;;
+  --quick-check)            do_quick_check=1;;
   --unsupported)            do_unsupported=1;;
   *) usage;;
   esac
@@ -55,7 +60,7 @@ ubuntu_issue="Ubuntu ($ubuntu_versions|$ubuntu_codenames)"
 # they're doing.
 gcel_issue="^GCEL"
 
-if [ 0 -eq "${do_unsupported-0}" ] ; then
+if [ 0 -eq "${do_unsupported-0}" ] && [ 0 -eq "${do_quick_check-0}" ] ; then

[Markus (顧孟勤), 2013/10/25 00:40:50]

I generally prefer "${...:-...}" over "${...-...}", but it probably doesn't
matter much.

[Michael Moss, 2013/10/25 02:45:31]

Yeah, I was just staying consistent with other uses nearby, though the file as a
whole isn't terribly consistent about much :/

   if ! egrep -q "($ubuntu_issue|$gcel_issue)" /etc/issue; then
     echo "ERROR: Only Ubuntu 12.04 (precise) through 13.04 (raring) are"\
         "currently supported" >&2
@@ -68,14 +73,14 @@ if [ 0 -eq "${do_unsupported-0}" ] ; then
   fi
 fi
 
-if [ "x$(id -u)" != x0 ]; then
+if [ "x$(id -u)" != x0 ] && [ 0 -eq "${do_quick_check-0}" ]; then
   echo "Running as non-root user."
   echo "You might have to enter your password one or more times for 'sudo'."
   echo
 fi
 
 # Packages needed for chromeos only
-chromeos_dev_list="libbluetooth-dev libbrlapi-dev"
+chromeos_dev_list="libbluetooth-dev"

[Markus (顧孟勤), 2013/10/25 00:40:50]

This sounds as if it should be in an unrelated CL. But maybe, I am missing
something non-obvious

[Michael Moss, 2013/10/25 02:45:31]

I was just cleaning up dupes (that package is also in dev_list, so it's not
chromeos-only).

 
 # Packages need for development
 dev_list="apache2.2-bin bison curl elfutils fakeroot flex g++ gperf
@@ -100,7 +105,7 @@ if [ "$(uname -m)" = "x86_64" ]; then
 fi
 
 # Run-time libraries required by chromeos only
-chromeos_lib_list="libpulse0 libbz2-1.0 libcurl4-gnutls-dev"
+chromeos_lib_list="libpulse0 libbz2-1.0"

[Markus (顧孟勤), 2013/10/25 00:40:50]

Same here

[Michael Moss, 2013/10/25 02:45:31]

Another dev_list dupe.

 
 # Full list of required run-time libraries
 lib_list="libatk1.0-0 libc6 libasound2 libcairo2 libcups2 libexpat1
@@ -203,7 +208,7 @@ yes_no() {
   done
 }
 
-if test "$do_inst_syms" = ""
+if test "$do_inst_syms" = "" && test 0 -eq ${do_quick_check-0}

[Markus (顧孟勤), 2013/10/25 00:40:50]

We should really clean up the code and either use "test" everywhere or use "[ ]"
everywhere. Oh well, probably not something that you want to do right now.

They are all sorts of other style issues with this script anyway.

[Michael Moss, 2013/10/25 02:45:31]

Agreed, but probably better as a dedicated clean up CL.

 then
   echo "This script installs all tools and libraries needed to build Chromium."
   echo ""
@@ -216,9 +221,9 @@ then
   fi
 fi
 if test "$do_inst_syms" = "1"; then
-  echo "Installing debugging symbols."
+  echo "Including debugging symbols."
 else
-  echo "Skipping installation of debugging symbols."
+  echo "Skipping debugging symbols."
   dbg_list=
 fi
 
@@ -229,19 +234,60 @@ if [ "$(uname -m)" = "x86_64" ]; then
   arm_list="$arm_list g++-multilib"
 fi
 
-if test "$do_inst_arm" = "1"; then
+if test "$do_inst_arm" = "1" ; then
   . /etc/lsb-release
-  if test "$DISTRIB_CODENAME" != "precise"; then
+  if test "$DISTRIB_CODENAME" != "precise" && \
+      test 0 -eq "${do_unsupported-0}"; then

[Markus (顧孟勤), 2013/10/25 00:40:50]

You do realize that you can write "-o" and "-a", do you? Just checking. Either
option should be OK in general.

For some reason, I find the double-negation hard to read. Would it be more
readable to change the logic:

  if ! [ "${DISTRIB_CODENAME}" = "precise" -o 1 -eq "${do_unsupported-0}" ];
then ...

It's really your call. I am probably just too dense to read the existing code
without having the scratch my head.

[Michael Moss, 2013/10/25 02:45:31]

Done.

     echo "ERROR: Installing the ARM cross toolchain is only available on" \
          "Ubuntu precise." >&2
     exit 1
   fi
-  echo "Installing ARM cross toolchain."
+  echo "Including ARM cross toolchain."
 else
-  echo "Skipping installation of ARM cross toolchain."
+  echo "Skipping ARM cross toolchain."
   arm_list=
 fi
 
+packages="$(echo "${dev_list} ${lib_list} ${dbg_list} ${arm_list}" | \
+  tr " " "\n" | sort -u | tr "\n" " ")"

[Markus (顧孟勤), 2013/10/25 00:40:50]

Minor nit-pick. You can avoid using "tr", if you use
${parameter//pattern/string}. This is generally a good idea, as it is faster
than spawning an external process. But it makes the code less readable. So, it's
probably a premature optimization.

[Michael Moss, 2013/10/25 02:45:31]

Yeah, since I still have to spawn for sort, I think keeping the 'tr's in the
pipeline is a bit more clear.

+
+if [ 1 -eq "${do_quick_check-0}" ] ; then
+  failed_check="$(dpkg-query -W -f '${PackageSpec}:${Status}\n' \
+    ${packages} 2>&1 | grep -v "ok installed" || /bin/true)"

[Markus (顧孟勤), 2013/10/25 00:40:50]

You should use ":" instead of "/bin/true". That's the more common pattern for
this type of thing, and it is more efficient than spawning an external process.

Think of ":" as being roughly equivalent to "pass" in Python.

[Michael Moss, 2013/10/25 02:45:31]

'pass' is exactly what I was looking for :)
Done.

+  if [ "$failed_check" ]; then

[Markus (顧孟勤), 2013/10/25 00:40:50]

Shouldn't that read "[ -n "${failed_check}" ]". You don't want to confuse "[" by
whatever happens to be the contents of "${failed_check}". The heuristics that
"[" needs to apply when parsing arguments are crazy enough as is; let's try to
not make things more fragile.

[Michael Moss, 2013/10/25 02:45:31]

Done.

+    nomatch="$(echo "$failed_check" | \
+      grep "No packages found matching" | sed -e "s/^.* \(.*\).$/\1/")"

[Markus (顧孟勤), 2013/10/25 00:40:50]

You can probably combine "grep" and "sed" into a single command.

This is untested, but I think it should work:

sed -e '/No packages found matching/s/^.* \(.*\)[.]$/\1/;t;d'

For the "missing" case, it would then be:

sed -e '/No packages found matching/d;s/^\([^:]*\):.*$/\1/'

More importantly though, I can't trigger any dpkg-query that would match the
"missing" pattern. Can you give an example of what you would expect to see here,
and how you tested this? My machine is probably just not set up the way you
expect it (i.e. everything is managed by apt-get).

[Michael Moss, 2013/10/25 02:45:31]

Ugh, yeah. I actually had these in different areas initially, then combined them
but didn't rewrite it.
If I do something like:

dpkg-query -W -f '${PackageSpec}:${Status}\n' libbrlapi-dev libc6-armel-cross

I get:

libbrlapi-dev:unknown ok not-installed
No packages found matching libc6-armel-cross.

'missing' matches the first and 'nomatch' matches the second.

+    missing="$(echo "$failed_check" | \
+      grep -v "No packages found matching" | sed -e "s/^\(.*\):.*$/\1/")"
+    if [ "$nomatch" ]; then
+      # Distinguish between packages that actually aren't available to the
+      # system (i.e. not in any repo) and packages that just aren't known to
+      # dpkg (i.e. managed by apt).
+      unknown=""
+      for p in $nomatch; do
+        if apt-cache show ${p} > /dev/null 2>&1; then

[Markus (顧孟勤), 2013/10/25 00:40:50]

This is a bash script, so you could write ">&/dev/null", if you prefer the more
concise form.

[Michael Moss, 2013/10/25 02:45:31]

2>&1 is used elsewhere, so I'll leave it consistent for now. That can be a clean
up CL change.

+          missing="${p}\n${missing}"
+        else
+          unknown="${p}\n${unknown}"
+        fi
+      done
+      if [ "$unknown" ]; then
+        echo
+        echo "WARNING: The following packages are unknown to your system"
+        echo "(maybe missing a repo or need to 'sudo apt-get update'):"
+        echo -e "$unknown" | sed -e "s/^\(.*\)$/  \1/"
+      fi
+    fi
+    if [ "$missing" ]; then
+      echo
+      echo "WARNING: The following packages are not installed:"
+      echo -e "$missing" | sed -e "s/^\(.*\)$/  \1/"
+    fi
+    exit 1
+  fi
+  exit 0
+fi
+
 sudo apt-get update
 
 # We initially run "apt-get" with the --reinstall option and parse its output.
@@ -249,7 +295,6 @@ sudo apt-get update
 # without accidentally promoting any packages from "auto" to "manual".
 # We then re-run "apt-get" with just the list of missing packages.
 echo "Finding missing packages..."
-packages="${dev_list} ${lib_list} ${dbg_list} ${arm_list}"
 # Intentionally leaving $packages unquoted so it's more readable.
 echo "Packages required: " $packages
 echo