Skip to content

build: fix node_g target #10153

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
danbev wants to merge 2 commits into from
Closed

build: fix node_g target #10153

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
65091d2
build: fix node_g target
danbev Dec 6, 2016
0be927c
use automatic variables in node/node_g targets
danbev Dec 6, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
build: fix node_g target 
Currently when running make node_g the following error is displayed:
if [ ! -r node -o ! -L  ]; then ln -fs out/Debug/node node_g; fi
/bin/sh: line 0: [: argument expected

It looks like there was a typo for the NODE_EXE where node became
lowercase instead of uppercase.
  • Loading branch information
@danbev
danbev committed Dec 6, 2016
commit 65091d2a766a6828c5da5b7ded58e6f883da9199
2 changes: 1 addition & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ $(NODE_EXE): config.gypi out/Makefile

$(NODE_G_EXE): config.gypi out/Makefile
$(MAKE) -C out BUILDTYPE=Debug V=$(V)
if [ ! -r $(NODE_EXE) -o ! -L $(node_EXE) ]; then ln -fs out/Debug/$(NODE_EXE) $@; fi
if [ ! -r $(NODE_EXE) -o ! -L $(NODE_EXE) ]; then ln -fs out/Debug/$(NODE_EXE) $@; fi
Copy link
Copy Markdown
Member

@richardlau richardlau Dec 6, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the $(NODE_EXE) be $(NODE_G_EXE) for consistency? Or in fact could all of the $(NODE_EXE) in this line be $@?

Copy link
Copy Markdown
Member

@sxa sxa Dec 6, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep it should, otherwise we're creating a "node_g" symlink and then checking for the "node" one. And yeah switching to $@ in both debug and release cases sounds like the better option to me too, so:

if [ ! -r $@ -o ! -L $@ ]; then ln -fs out/Debug/$(NODE_EXE) $@; fi

Same for the non-debug case.


out/Makefile: common.gypi deps/uv/uv.gyp deps/http_parser/http_parser.gyp \
deps/zlib/zlib.gyp deps/v8/gypfiles/toolchain.gypi \
Expand Down