Makefile: run 'npm install' whenever 'package.json' is updated. (issue 7681046)

16 views
Skip to first unread message

usrb...@yahoo.com

unread,
Mar 25, 2013, 2:29:25 PM3/25/13
to a...@chromium.org, traceur-comp...@googlegroups.com, re...@codereview-hr.appspotmail.com
Reviewers: arv-chromium,

Message:
NOTE: this shell code deletes the local 'node_modules' Not that anyone
has any business putting non-reinstallable stuff in 'node_modules'.

#--cut--
git checkout 2070a8aeb6406db0 # last merge with branch master

dl_check() {
local f=$(basename $1)
test -e $f || curl -sO $1
openssl sha1 < $f | grep -q "= $2" && return 0
echo sha1 mismatch!
return 1
}

git checkout -b issue7681046-make-npm-install

dl_check https://codereview.appspot.com/download/issue7681046_1.diff \
e3473d9729bbdba6 && git apply issue7681046_1.diff

rm -r node_modules
make
touch package.json
make
rm -r node_modules/source-map
make
#--cut--

There are a few other Makefile fixes I have to get to. Mostly
prerequisite fixes and incomplete 'clean' targets.



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

https://codereview.appspot.com/7681046/diff/1/Makefile#newcode74
Makefile:74: node build/expand-js-template.js $< $@
$< indicates the first prerequisite only. Unfortunately, you have to do
something like $(wordlist 1,n,$^) -- untested -- in order to get the
first n prerequisites.

https://codereview.appspot.com/7681046/diff/1/Makefile#newcode77
Makefile:77: node_modules/source-map/lib/source-map/*.js
Eventually I want to generate this, but it's a low priority for now. The
best place to do the fix would be in expand-js-template.js

Description:
Makefile: run 'npm install' whenever 'package.json' is updated.

- Make 'node_modules' an order-only dependency of 'bin/dep.mk'
- Add the 'source-map' dependencies to SourceMapIntegration.js

BUG=http://code.google.com/p/traceur-compiler/issues/detail?id=224
TEST=None


Please review this at https://codereview.appspot.com/7681046/

Affected files:
M Makefile


Index: Makefile
diff --git a/Makefile b/Makefile
index
201794b3ce1bba8851adc8708125bece339a45ff..dacadbe9db48c7eaf65ae80587818ba485a6bf8e
100644
--- a/Makefile
+++ b/Makefile
@@ -51,7 +51,7 @@ bin/traceur.js force:
# Prerequisites following '|' are rebuilt just like ordinary prerequisites.
# However, they don't cause remakes if they're newer than the target. See:
# http://www.gnu.org/software/make/manual/html_node/Prerequisite-Types.html
-build/dep.mk: | $(GENSRC)
+build/dep.mk: | $(GENSRC) node_modules
node build/makedep.js --depTarget bin/traceur.js $(TFLAGS) $(SRC) > $@

src/syntax/trees/ParseTrees.js: \
@@ -71,7 +71,18 @@ src/codegeneration/ParseTreeTransformer.js: \
node $^ > $@

%.js: %.js-template.js
- node build/expand-js-template.js $^ $@
+ node build/expand-js-template.js $< $@
+
+src/outputgeneration/SourceMapIntegration.js: \
+ node_modules/source-map/lib/source-map/*.js
+
+NPM_INSTALL = npm install --local && touch node_modules
+
+node_modules/%:
+ $(NPM_INSTALL)
+
+node_modules: package.json
+ $(NPM_INSTALL)

bin/traceur.ugly.js: bin/traceur.js
uglifyjs bin/traceur.js --compress -m -o $@


a...@chromium.org

unread,
Mar 25, 2013, 3:02:00 PM3/25/13
to usrb...@yahoo.com, traceur-comp...@googlegroups.com, re...@codereview-hr.appspotmail.com
LGTM

Committed as 636ed7a8fef2c25e11e72f8b0554afa7da89256f

https://codereview.appspot.com/7681046/
Reply all
Reply to author
Forward
0 new messages