launchpad-reviewers team mailing list archive

["Jeroen T. Vermeulen" <jtv@xxxxxxxxxxxxx>]

Jeroen T. Vermeulen has proposed merging lp:~jtv/launchpad/rocketfuel-dogfood into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jtv/launchpad/rocketfuel-dogfood/+merge/71816

= Summary =

Running rocketfuel-get on dogfood breaks because its lp-sourcedeps is in among the branches.  The division of responsibilities may make it difficult to shuffle directories around on that server, but fixing the script turns out to be easy.


== Proposed fix ==

Filter lp-sourcedeps out of the list of branches that rocketfuel-get discovers in order to re-link their sourcecode directories.


== Pre-implementation notes ==

Brought this up on IRC but got little response.  Then again, getting traction for improving the dogfood setup can be even harder so a code change seems the most lucrative approach.


== Implementation details ==

Along the way I also split a particularly complex command line into more easily understood functions.

Functions are a POSIX shell feature and this is the portable syntax; I use them with dash regularly.  From the portability point of view my biggest worry is that the -z option to grep is specific to GNU grep.

Then again, that's in processing the output of a "find" command line that is specific to GNU find, before feeding it into an xargs that also looks like it depends on GNU features.  So I don't think it'll matter!


== Tests ==

Good question, that.  I'm running an EC2 test.


== Demo and Q/A ==

Run it both on dogfood and a newer development system.  Both should work.


= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  utilities/rocketfuel-get
-- 
https://code.launchpad.net/~jtv/launchpad/rocketfuel-dogfood/+merge/71816
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/launchpad/rocketfuel-dogfood into lp:launchpad.

=== modified file 'utilities/rocketfuel-get'
--- utilities/rocketfuel-get	2011-06-30 23:12:06 +0000
+++ utilities/rocketfuel-get	2011-08-17 06:19:24 +0000
@@ -66,12 +66,35 @@
 
 # Update the current trees in the repo.
 echo "Updating sourcecode dependencies in current local branches:"
-find "$LP_PROJECT_PATH" -mindepth 2 -maxdepth 2 \
-    -type d -name sourcecode -printf '%h\0' | \
+
+# Find directories among local branches containing "sourcecode" directories.
+# Prints each as a null-terminated record (since Unix filenames may contain
+# newlines).
+find_branches_to_relink() {
+    find "$LP_PROJECT_PATH" \
+        -mindepth 2 -maxdepth 2 -type d -name sourcecode -printf '%h\0'
+}
+
+# Some old setups (notably dogfood) may have lp-sourcedeps mixed in with the
+# local branches.  Echo stdin to stdout, with these filenames filtered out.
+# Filenames must be null-terminated on input, and remain null-terminated on
+# output.
+filter_branches_to_relink() {
+    grep -vz '/lp-sourcedeps$'
+}
+
+# Re-link the sourcecode directories for local branches.  Takes the branch
+# paths from stdin, as null-terminated records.
+relink_branches() {
     run-child xargs --no-run-if-empty \
         --max-procs="${CPU_COUNT}" --max-args=1 --null \
         "$LP_TRUNK_PATH/utilities/link-external-sourcecode" \
             --parent "$LP_PROJECT_ROOT/$LP_SOURCEDEPS_DIR" --target
+}
+
+# Actually do it:
+find_branches_to_relink | filter_branches_to_relink | relink_branches
+
 
 # Build launchpad if there were changes.
 if [ $FINAL_REV != $INITIAL_REV ];