launchpad-reviewers team mailing list archive

[Brad Crittenden <bac@xxxxxxxxxxxxx>]

Brad Crittenden has proposed merging lp:~bac/launchpad/cleanish into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1004088 in Launchpad itself: "Apache configuration is necessary for tests but makes it impossible to use "make clean" safely in lxc"
  https://bugs.launchpad.net/launchpad/+bug/1004088

For more details, see:
https://code.launchpad.net/~bac/launchpad/cleanish/+merge/107385

= Summary =

Complete removal of /var/tmp/bazaar.launchpad.dev can cause problems
in environments where the machine goes up and down a lot, thus causing
Apache to bounce.  If that directory does not exist when Apache comes
up it will be recreated by root with the wrong permissions.

== Proposed fix ==

Create a new cleaning target that simply removes the contents of that
directory but not the directory itself.

A more aggressive approach would be to make this change to the
existing 'clean' target but since we don't want to cause any
unforeseen problems we have left the behavior of that target as is.

== Pre-implementation notes ==

Props to Graham and Gary for helping to track down this spawn of
Satan.

== Tests ==

N/A

== Demo and Q/A ==

N/A

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  Makefile

./Makefile
     202: Line exceeds 80 characters.
-- 
https://code.launchpad.net/~bac/launchpad/cleanish/+merge/107385
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~bac/launchpad/cleanish into lp:launchpad.

=== modified file 'Makefile'
--- Makefile	2012-05-16 15:45:33 +0000
+++ Makefile	2012-05-25 13:49:34 +0000
@@ -137,13 +137,15 @@
 pagetests: build
 	env PYTHONPATH=$(PYTHONPATH) bin/test test_pages
 
-inplace: build combobuild logs clean_logs
+codehosting-dir:
 	mkdir -p $(CODEHOSTING_ROOT)/mirrors
 	mkdir -p $(CODEHOSTING_ROOT)/config
 	mkdir -p /var/tmp/bzrsync
 	touch $(CODEHOSTING_ROOT)/rewrite.log
 	chmod 777 $(CODEHOSTING_ROOT)/rewrite.log
 	touch $(CODEHOSTING_ROOT)/config/launchpad-lookup.txt
+
+inplace: build combobuild logs clean_logs codehosting-dir
 	if [ -d /srv/launchpad.dev ]; then \
 		ln -sfn $(WD)/build/js $(CONVOY_ROOT); \
 	fi
@@ -398,7 +400,11 @@
 	$(RM) -r lib/mailman
 endif
 
-clean: clean_js clean_mailman clean_buildout clean_logs
+lxc-clean: clean_js clean_mailman clean_buildout clean_logs
+	# It is important for parallel tests inside LXC that the
+	# $(CODEHOSTING_ROOT) directory not be completely removed.
+	# This target removes its contents but not the directory and
+	# it does everything expected from a clean target.
 	$(MAKE) -C sourcecode/pygettextpo clean
 	# XXX gary 2009-11-16 bug 483782
 	# The pygettextpo Makefile should have this next line in it for its make
@@ -413,7 +419,7 @@
 	    -print0 | xargs -r0 $(RM)
 	$(RM) -r lib/subvertpy/*.so
 	$(RM) -r $(LP_BUILT_JS_ROOT)/*
-	$(RM) -r $(CODEHOSTING_ROOT)
+	$(RM) -r $(CODEHOSTING_ROOT)/*
 	$(RM) -r $(APIDOC_DIR)
 	$(RM) -r $(APIDOC_DIR).tmp
 	$(RM) -r build
@@ -438,6 +444,8 @@
 		$(RM) -r /var/tmp/launchpad_mailqueue; \
 	fi
 
+clean: lxc-clean
+	$(RM) -r $(CODEHOSTING_ROOT)
 
 realclean: clean
 	$(RM) TAGS tags
@@ -470,8 +478,8 @@
 		-e 's,%LISTEN_ADDRESS%,$(LISTEN_ADDRESS),' \
 		configs/development/local-launchpad-apache > \
 		/etc/apache2/sites-available/local-launchpad
-	touch /var/tmp/bazaar.launchpad.dev/rewrite.log
-	chown -R $(SUDO_UID):$(SUDO_GID) /var/tmp/bazaar.launchpad.dev
+	touch $(CODEHOSTING_ROOT)/rewrite.log
+	chown -R $(SUDO_UID):$(SUDO_GID) $(CODEHOSTING_ROOT)
 	if [ ! -d /srv/launchpad.dev ]; then \
 		mkdir /srv/launchpad.dev; \
 		chown $(SUDO_UID):$(SUDO_GID) /srv/launchpad.dev; \