Allow running targets without using bzr

Allow running targets without using bzr

Added the flag NO_BZR which lets you run make as `NO_BZR=1 make [target]` in order to prevent make from running bzr commands, which can be slow due to network access, providing sensible defaults instead.

https://code.launchpad.net/~makyo/juju-gui/bzr-make-1086794/+merge/139518

(do not edit description out of merge proposal)

[matthew.scott, 2012-12-12 17:18:43]

Please take a look.

[gary.poster, 2012-12-12 22:20:07]

Land with changes

Looks good!  Thank you.  This will be great to have.

https://codereview.appspot.com/6873071/diff/1/Makefile
File Makefile (right):

https://codereview.appspot.com/6873071/diff/1/Makefile#newcode13
Makefile:13: JSFILES=$(shell find . -wholename './node_modules*' -prune \
Before this, I think we need a comment explaining the two use cases for not
using bzr commands: we want to make releases without bzr access, so working with
a branch is fast; and we want to allow devs to work with a release (per Kapil; I
really don't like this.  devs should work with a branch.)

https://codereview.appspot.com/6873071/diff/1/Makefile#newcode19
Makefile:19: -o -name '*generateTemplates' \
This could simply be 'generateTemplates', without the wildcard.

https://codereview.appspot.com/6873071/diff/1/Makefile#newcode40
Makefile:40: ifdef NO_BZR
Good solution, given our use cases and the other problems you encountered.

https://codereview.appspot.com/6873071/diff/1/Makefile#newcode102
Makefile:102: TEMPLATE_TARGETS=$(shell find app/templates -type f -print)
Please exclude standard editor effluvia.

https://codereview.appspot.com/6873071/diff/1/Makefile#newcode104
Makefile:104: SPRITE_SOURCE_FILES=$(shell find app/assets/images -type f -print)
Please exclude standard editor effluvia.

https://codereview.appspot.com/6873071/diff/1/Makefile#newcode393
Makefile:393: dist: $(RELEASE_FILE) $(RELEASE_SIGNATURE) upload_release.py
I would like the Makefile to refuse to run the upload_release.py file if NO_BZR
is defined.  We should then have a phony target that only creates the release
file (e.g. "distfile: $(RELEASE_FILE)" that the charm can use (that is, it will
run fine if NO_BZR is true).  See "ifdef BRANCH_IS_GOOD" above for an example of
one way to do this kind of thing.

https://codereview.appspot.com/6873071/diff/1/docs/process.rst
File docs/process.rst (right):

https://codereview.appspot.com/6873071/diff/1/docs/process.rst#newcode214
docs/process.rst:214: all targets.
...except dist, which will refuse to complete.

[benjamin.saller, 2012-12-13 04:02:59]

LGTM

I don't like that our make files keep getting more complex, we have many 
  
   ENV=foo make target

options where ENV's _n_ is too high. I make some suggestions about improving
this in the future, but I'm ok with landing this now.

If you don't agree with the suggestions and don't want to make the card I'd
appreciate making one for a Friday discussion.

https://codereview.appspot.com/6873071/diff/1/Makefile
File Makefile (right):

https://codereview.appspot.com/6873071/diff/1/Makefile#newcode13
Makefile:13: JSFILES=$(shell find . -wholename './node_modules*' -prune \
Looking at how this is used (and I realize its pre-existing) I think it might be
better to make a clearer separation of runtime vs devel files. I'd also suggest
we clearly move 
3rd party libs away from our libraries, 
app/assets/javascripts/3rd-party.


This could lead to simpler rules. 

find app -name \*.js | grep -e app/assets/javascipts/third-party/

might yield the runtime files.

I don't think we need to do this now, but I think we should make a card.

https://codereview.appspot.com/6873071/diff/1/Makefile#newcode102
Makefile:102: TEMPLATE_TARGETS=$(shell find app/templates -type f -print)
This and the var below it are clear wins.

[matthew.scott, 2012-12-13 17:34:04]

Thanks Ben and Gary!

As to the card about ENV=1 flags to make, I understand, and I'll make a card to
discuss this on Friday.

https://codereview.appspot.com/6873071/diff/1/Makefile
File Makefile (right):

https://codereview.appspot.com/6873071/diff/1/Makefile#newcode13
Makefile:13: JSFILES=$(shell find . -wholename './node_modules*' -prune \
I agree that it would be nice to have this be more positive, but I also agree
that it probably needs to be in a card of its own that moves some files around. 
It'd be clearer to just specify valid directories, but now things are kind of
scattered.  I'll make a card.

On 2012/12/13 04:02:59, benjamin.saller wrote:
> Looking at how this is used (and I realize its pre-existing) I think it might
be
> better to make a clearer separation of runtime vs devel files. I'd also
suggest
> we clearly move 
> 3rd party libs away from our libraries, 
> app/assets/javascripts/3rd-party.
> 
> 
> This could lead to simpler rules. 
> 
> find app -name \*.js | grep -e app/assets/javascipts/third-party/
> 
> might yield the runtime files.
> 
> I don't think we need to do this now, but I think we should make a card.

https://codereview.appspot.com/6873071/diff/1/Makefile#newcode13
Makefile:13: JSFILES=$(shell find . -wholename './node_modules*' -prune \
On 2012/12/12 22:20:07, gary.poster wrote:
> Before this, I think we need a comment explaining the two use cases for not
> using bzr commands: we want to make releases without bzr access, so working
with
> a branch is fast; and we want to allow devs to work with a release (per Kapil;
I
> really don't like this.  devs should work with a branch.)

Done.

https://codereview.appspot.com/6873071/diff/1/Makefile#newcode19
Makefile:19: -o -name '*generateTemplates' \
On 2012/12/12 22:20:07, gary.poster wrote:
> This could simply be 'generateTemplates', without the wildcard.

Done.

https://codereview.appspot.com/6873071/diff/1/Makefile#newcode102
Makefile:102: TEMPLATE_TARGETS=$(shell find app/templates -type f -print)
On 2012/12/12 22:20:07, gary.poster wrote:
> Please exclude standard editor effluvia.

Done.

https://codereview.appspot.com/6873071/diff/1/Makefile#newcode104
Makefile:104: SPRITE_SOURCE_FILES=$(shell find app/assets/images -type f -print)
On 2012/12/12 22:20:07, gary.poster wrote:
> Please exclude standard editor effluvia.

Done.

[matthew.scott, 2012-12-13 17:50:43]

*** Submitted:

Allow running targets without using bzr

Added the flag NO_BZR which lets you run make as `NO_BZR=1 make [target]` in
order to prevent make from running bzr commands, which can be slow due to
network access, providing sensible defaults instead.

R=gary.poster, benjamin.saller
CC=
https://codereview.appspot.com/6873071

[benji, 2012-12-13 18:00:30]

This branch looks good.  I just had one comment about an available
simplification.

https://codereview.appspot.com/6873071/diff/6001/Makefile
File Makefile (right):

https://codereview.appspot.com/6873071/diff/6001/Makefile#newcode108
Makefile:108: SPRITE_SOURCE_FILES=$(shell find app/assets/images -type f ! -name
'.*' ! -name '*.swp' ! -name '*~' ! -name '\#*' -print)
It would seem more advisable to guess at the file types we are currently using
as images (not much of a guess) than to guess at all the files that might be in
that directory that are not images.

In other words, this should be

SPRITE_SOURCE_FILES=$(shell find app/assets/images -name '*.png')

Note that -print is the default, so doesn't have to be provided.