Merge lp:~jtv/pyjuju/makefile-fixups into lp:pyjuju

[Jeroen T. Vermeulen]

Get "make" targets check, review etc. to work, and warn about missing pep8/pyflakes.

Makefile improvements and fixes.

Whenever I tried to run "make check" or "make review" in a juju branch, I would get output like:

bzr: ERROR: Not a branch: "/".
    bzr: ERROR: Not a branch: "/".
    bzr: ERROR: Not a branch: "/".
    bzr: ERROR: Not a branch: "/".
    make: *** [check] Error 1

The errors seem to happen (and I'll admit I don't know how or why) because
there are several lines in the Makefile each defining $(modified) as a shell
command involving bzr. I get the same result on a cloud machine as on my development system.
Giving each of those variables a separate name fixed the errors.

There were also other problems: the linters weren't actually being run, there
was no helpful indication of pep8 and/or pyflakes being missing, newly added
files weren't being checked, greps for ".py" suffixes sometimes neglectedto
escape the dot (so it stood for "any character followed by py"). I fixed all of those.

Finally there were some cosmetic nuisances. Spacing of the command lines was
irregular, and some use of Perl-style regexes in grep became unnecessary.

One of the checks in the "review" target was commented out. I don't know why,
so I left that as-is.

[Jeroen T. Vermeulen (jtv)]

Oh, and I documented some targets, since I was figuring out what they were meant to do anyway.

[Ian Booth (wallyworld)]

Looks like nice cleanup.

review: Approve

[Kapil Thangavelu (hazmat)]

looks good, thanks.

review: Approve

[Kapil Thangavelu (hazmat)]

*** Submitted:

Makefile improvements and fixes.

Whenever I tried to run "make check" or "make review" in a juju branch, I would get output like:

bzr: ERROR: Not a branch: "/".
    bzr: ERROR: Not a branch: "/".
    bzr: ERROR: Not a branch: "/".
    bzr: ERROR: Not a branch: "/".
    make: *** [check] Error 1

The errors seem to happen (and I'll admit I don't know how or why) because
there are several lines in the Makefile each defining $(modified) as a shell
command involving bzr. I get the same result on a cloud machine as on my development system.
Giving each of those variables a separate name fixed the errors.

There were also other problems: the linters weren't actually being run, there
was no helpful indication of pep8 and/or pyflakes being missing, newly added
files weren't being checked, greps for ".py" suffixes sometimes neglectedto
escape the dot (so it stood for "any character followed by py"). I fixed all of those.

Finally there were some cosmetic nuisances. Spacing of the command lines was
irregular, and some use of Perl-style regexes in grep became unnecessary.

One of the checks in the "review" target was commented out. I don't know why,
so I left that as-is.

=== modified file 'Makefile'
--- Makefile	2012-09-27 01:43:32 +0000
+++ Makefile	2013-02-21 18:42:36 +0000
@@ -21,24 +21,34 @@
 etags:
 	@ctags -e --python-kinds=-iv -R juju
 
-modified=$(shell bzr status -S  |grep -P '^\s*M' | awk '{print $$2;}'| grep -P ".py$$")
-check:
-	@test -n "$(modified)" && echo $(modified) | xargs $(PEP8) --repeat
-	@test -n "$(modified)" && echo $(modified) | xargs pyflakes
-
-
-modified=$(shell bzr status -S -r ancestor:$(JUJU_TRUNK) |grep -P '^\s*M' | awk '{print $$2;}'| grep -P ".py$$")
-review:
-	#@test -n "$(modified)" && echo $(modified) | xargs $(PEP8) --repeat
-	@test -n "$(modified)" && echo $(modified) | xargs pyflakes
-
-
-modified=$(shell bzr status -S -r branch::prev  |grep -P '^\s*\+?[MN]' | awk '{print $$2;}'| grep -P "test_.*\.py$$")
+
+present_pep8=$(shell which $(PEP8))
+present_pyflakes=$(shell which pyflakes)
+warn_missing_linters:
+	@test -n "$(present_pep8)" || echo "WARNING: $(PEP8) not installed."
+	@test -n "$(present_pyflakes)" || echo "WARNING: pyflakes not installed."
+
+
+# "check": Check uncommitted changes for lint.
+check_changes=$(shell bzr status -S | grep '^[ +]*[MN]' | awk '{print $$2;}' | grep "\\.py$$")
+check: warn_missing_linters
+	@test -z $(present_pep8) || (echo $(check_changes) | xargs -r $(PEP8) --repeat)
+	@test -z $(present_pyflakes) || (echo $(check_changes) | xargs -r pyflakes)
+
+
+# "review": Check all changes compared to trunk for lint.
+review_changes=$(shell bzr status -S -r ancestor:$(JUJU_TRUNK) | grep '^[ +]*[MN]' | awk '{print $$2;}' | grep "\\.py$$")
+review: warn_missing_linters
+	#@test -z $(present_pep8) || (echo $(review_changes) | xargs -r $(PEP8) --repeat)
+	@test -z $(present_pyflakes) || (echo $(review_changes) | xargs -r pyflakes)
+
+
+ptests_changes=$(shell bzr status -S -r branch::prev | grep -P '^[ +]*[MN]' | awk '{print $$2;}'| grep "test_.*\\.py$$")
 ptests:
-	@test -n "$(modified)" && echo $(modified) | xargs ./test
+	@echo $(ptests_changes) | xargs -r ./test
 
-modified=$(shell  bzr status -S -r ancestor:$(JUJU_TRUNK)/|grep -P 'test.*\.py' |awk '{print $$2;}')
+btests_changes=$(shell bzr status -S -r ancestor:$(JUJU_TRUNK)/ | grep "test.*\\.py$$" | awk '{print $$2;}')
 btests:
-	@./test $(modified)
+	@./test $(btests_changes)
 
-.PHONY: tags check review
+.PHONY: tags check review warn_missing_linters