launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/simplify-buildout-bin-shell into lp:launchpad.

Commit message:
Move shell scripts out of buildout-templates/bin/.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/simplify-buildout-bin-shell/+merge/314973

lint is mainly used by "make lint", so it's simplest to just move that to utilities/.

update-download-cache is (as far as I know) unreferenced and probably rarely used, so again it's simplest to just move that to utilities/.

with-xvfb is likely to be referenced from test wrappers (it certainly is by mine), so I rewrote it in Python and added an entry point for it in setup.py.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/simplify-buildout-bin-shell into lp:launchpad.

=== modified file 'Makefile'
--- Makefile	2016-09-21 02:51:58 +0000
+++ Makefile	2017-01-17 23:39:04 +0000
@@ -52,10 +52,10 @@
     bin/fl-record bin/fl-run-bench bin/fl-run-test bin/googletestservice \
     bin/i18ncompile bin/i18nextract bin/i18nmergeall bin/i18nstats \
     bin/harness bin/iharness bin/ipy bin/jsbuild bin/lpjsmin\
-    bin/killservice bin/kill-test-services bin/lint.sh bin/retest \
+    bin/killservice bin/kill-test-services bin/retest \
     bin/run bin/run-testapp bin/sprite-util bin/start_librarian \
-    bin/tags bin/test bin/tracereport bin/twistd bin/update-download-cache \
-    bin/watch_jsbuild
+    bin/tags bin/test bin/tracereport bin/twistd \
+    bin/watch_jsbuild bin/with-xvfb
 
 BUILDOUT_TEMPLATES = buildout-templates/_pythonpath.py.in
 
@@ -109,10 +109,10 @@
 		lp.services.mailman.tests
 
 lint: ${PY}
-	@bash ./bin/lint.sh
+	@bash ./utilities/lint
 
 lint-verbose: ${PY}
-	@bash ./bin/lint.sh -v
+	@bash ./utilities/lint -v
 
 logs:
 	mkdir logs

=== renamed file 'buildout-templates/bin/with-xvfb.in' => 'lib/lp/scripts/utilities/withxvfb.py'
--- buildout-templates/bin/with-xvfb.in	2011-09-09 15:05:54 +0000
+++ lib/lp/scripts/utilities/withxvfb.py	2017-01-17 23:39:04 +0000
@@ -1,46 +1,42 @@
-#!/bin/bash
-#
-# Copyright 2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2011-2017 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
-#
-# Wrapper that provides a default Xvfb environment for the given
-# command.
-#
-# If the command is not found it is searched for in the same directory
-# as this script. This lets you do `bin/with-xvfb iharness` for
-# example.
-#
-# Follows sinzui's advice to the launchpad-dev list:
-#   https://lists.launchpad.net/launchpad-dev/msg07879.html
-#
-
-set -eu
-
-# Look for $1 - i.e. the command to run - in this script's directory
-# if it's not found along the default PATH.
-if [ $# -ge 1 ] && ! type -P "$1" > /dev/null
-then
-    if command="$(PATH="$(dirname "$0")" type -P "$1")"
-    then
-        # Shift $1 off and set new positional arguments.
-        shift && set -- "$${command}" "$@"
-    fi
-# If no command has been given and SHELL is set, spawn a shell.
-elif [ $# -eq 0 -a -n "$${SHELL:-}" ]
-then
-    set -- "$${SHELL}"
-fi
-
-#
-# --auto-servernum
-#   Try to get a free server number, starting at 99. See xvfb-run(1).
-#
-# --server-args=
-#   -ac disables host-based access control mechanisms. See Xserver(1).
-#   -screen forces a screen configuration. At the time of writing
-#      there is some disagreement between xvfb-run(1) and Xvfb(1)
-#      about what the default is.
-#
-exec xvfb-run \
-    --server-args="-ac -screen 0 1024x768x24" \
-    --auto-servernum -- "$@"
+
+"""Run a command with a default Xvfb environment
+
+If the command is not found it is searched for in the same directory as this
+script. This lets you do `bin/with-xvfb iharness` for example.
+
+Follows sinzui's advice to the launchpad-dev list:
+  https://lists.launchpad.net/launchpad-dev/msg07879.html
+"""
+
+from __future__ import absolute_import, print_function, unicode_literals
+
+import os
+import sys
+
+from lp.services.osutils import find_on_path
+
+
+def main():
+    # Look for the command to run in this script's directory if it's not
+    # found along the default PATH.
+    args = sys.argv[1:]
+    if args and not find_on_path(args[0]):
+        nearby = os.path.join(os.path.dirname(sys.argv[0]), args[0])
+        if os.access(nearby, os.X_OK):
+            args[0] = nearby
+    # If no command has been given and SHELL is set, spawn a shell.
+    elif not args and os.environ.get("SHELL"):
+        args = [os.environ["SHELL"]]
+
+    args = [
+        # -ac disables host-based access control mechanisms. See Xserver(1).
+        # -screen forces a screen configuration. At the time of writing
+        #    there is some disagreement between xvfb-run(1) and Xvfb(1)
+        #    about what the default is.
+        "--server-args=-ac -screen 0 1024x768x24",
+        # Try to get a free server number, starting at 99. See xvfb-run(1).
+        "--auto-servernum",
+        ] + args
+    os.execvp("xvfb-run", args)

=== modified file 'setup.py'
--- setup.py	2016-11-03 15:19:01 +0000
+++ setup.py	2017-01-17 23:39:04 +0000
@@ -164,15 +164,14 @@
     entry_points=dict(
         console_scripts=[  # `console_scripts` is a magic name to setuptools
             'apiindex = lp.scripts.utilities.apiindex:main',
+            'harness = lp.scripts.harness:python',
+            'jsbuild = lp.scripts.utilities.js.jsbuild:main',
             'killservice = lp.scripts.utilities.killservice:main',
-            'jsbuild = lp.scripts.utilities.js.jsbuild:main',
             'run = lp.scripts.runlaunchpad:start_launchpad',
-            'run-testapp = '
-                'lp.scripts.runlaunchpad:start_testapp',
-            'harness = lp.scripts.harness:python',
+            'run-testapp = lp.scripts.runlaunchpad:start_testapp',
+            'start_librarian = lp.scripts.runlaunchpad:start_librarian',
             'twistd = twisted.scripts.twistd:run',
-            'start_librarian = '
-                'lp.scripts.runlaunchpad:start_librarian',
+            'with-xvfb = lp.scripts.utilities.withxvfb:main',
         ]
     ),
 )

=== renamed file 'buildout-templates/bin/lint.sh.in' => 'utilities/lint'
--- buildout-templates/bin/lint.sh.in	2012-05-11 05:14:01 +0000
+++ utilities/lint	2017-01-17 23:39:04 +0000
@@ -1,20 +1,14 @@
 #!/bin/bash
 #
-# Copyright 2009-2010 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2017 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 #
 # Runs pocketlint on files changed from parent branch.
 
 
-${shell-relative-path-setup}
-
-utilitiesdir=${buildout:directory/utilities|shell-path}
-[ -z "$utilitiesdir" ] && utilitiesdir=.
-
-
 if [ -z "$1" ]; then
     # No command line argument provided, lint all changed files.
-    files=$($utilitiesdir/find-changed-files.sh)
+    files=$($(dirname "$0")/find-changed-files.sh)
 else
     # Add newlines so grep filters out pyfiles correctly later.
     files=`echo $* | tr " " "\n"`

=== renamed file 'buildout-templates/bin/update-download-cache.in' => 'utilities/update-download-cache'
--- buildout-templates/bin/update-download-cache.in	2010-04-20 19:10:35 +0000
+++ utilities/update-download-cache	2017-01-17 23:39:04 +0000
@@ -1,4 +1,3 @@
-${shell-relative-path-setup}
-
-bzr up ${buildout:directory/buildout/download-cache|shell-path}
-
+#!/bin/sh
+
+bzr up "$(dirname "$0")/../download-cache"