Skip to content

Conversation

@danbev
Copy link
Contributor

Checklist
  • make -j8 test (UNIX), or vcbuild test nosign (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

build

Description of change

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.

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.
@nodejs-github-botnodejs-github-bot added the build Issues and PRs related to build files or the CI. label Dec 6, 2016
Copy link
Member

@addaleaxaddaleax left a comment

Choose a reason for hiding this comment

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

LGTM, let’s fast-track this

CI: https://ci.nodejs.org/job/node-test-commit/6485/

Makefile Outdated
$(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
Member

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
Member

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.

@richardlau
Copy link
Member

Presumably this slipped through before because the CI doesn't build the debug build?

Copy link
Member

@addaleaxaddaleax left a comment

Choose a reason for hiding this comment

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

Thanks, still LGTM!

@addaleax
Copy link
Member

Landed in 22e2c47

@addaleaxaddaleax closed this Dec 7, 2016
addaleax pushed a commit that referenced this pull request Dec 7, 2016
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. Ref: #9827 PR-URL: #10153 Reviewed-By: Jeremiah Senkpiel <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Italo A. Casas <[email protected]>
addaleax pushed a commit that referenced this pull request Dec 7, 2016
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. Ref: #9827 PR-URL: #10153 Reviewed-By: Jeremiah Senkpiel <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Italo A. Casas <[email protected]>
@italoacasasitaloacasas mentioned this pull request Dec 15, 2016
italoacasas pushed a commit that referenced this pull request Dec 15, 2016
Notable changes: - buffer: - fix single-character string filling (Anna Henningsen) #9837 - handle UCS2.fill() properly on BE (Anna Henningsen) #9837 - url: - including base argument in originFor (joyeecheung) #10021 - improve URLSearchParams spec compliance (Timothy Gu) #9484 - http: - remove stale timeout listeners (Karl Böhlmark) #9440 - build: - fix node_g target (Daniel Bevenius) #10153 PR-URL: #10277
italoacasas pushed a commit that referenced this pull request Dec 15, 2016
Notable changes SEMVER-MINOR - url: - add inspect function to TupleOrigin (Safia Abdalla) #10039 - crypto: - allow adding extra certs to well-known CAs (Sam Roberts) #9139 SEMVER-PATCH - buffer: - fix single-character string filling (Anna Henningsen) #9837 - handle UCS2 .fill() properly on BE (Anna Henningsen) #9837 - url: - including base argument in originFor (joyeecheung) #10021 - improve URLSearchParams spec compliance (Timothy Gu) #9484 - http: - remove stale timeout listeners (Karl Böhlmark) #9440 - build: - fix node_g target (Daniel Bevenius) #10153 - fs: - remove unused argument from copyObject() (Ethan Arrowood) #10041 - timers: - fix handling of cleared immediates (hveldstra) #9759 PR-URL: #10277
italoacasas pushed a commit that referenced this pull request Dec 17, 2016
Notable changes: * **crypto**: - Allow adding extra certificates to well-known CAs. (Sam Roberts) [#9139](#9139) * **buffer**: - Fix single-character string filling. (Anna Henningsen) [#9837](#9837) - Handle UCS2 `.fill()` properly on BE. (Anna Henningsen) [#9837](#9837) * **url**: - Add inspect function to TupleOrigin. (Safia Abdalla) [#10039](#10039) - Including base argument in originFor. (joyeecheung) [#10021](#10021) - Improve URLSearchParams spec compliance. (Timothy Gu) [#9484](#9484) * **http**: - Remove stale timeout listeners. (Karl Böhlmark) [#9440](#9440) * **build**: - Fix node_g target. (Daniel Bevenius) [#10153](#10153) * **fs**: - Remove unused argument from copyObject(). (EthanArrowood) [#10041](#10041) * **timers**: - Fix handling of cleared immediates. (hveldstra) [#9759](#9759) * **src**: - Add wrapper for process.emitWarning(). (SamRoberts) [#9139](#9139) - Fix string format mistake for 32 bit node.(Alex Newman) [#10082](#10082)
@danbevdanbev deleted the fix-node_g-target branch January 17, 2017 07:16
MylesBorins pushed a commit that referenced this pull request Mar 7, 2017
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. Ref: #9827 PR-URL: #10153 Reviewed-By: Jeremiah Senkpiel <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Italo A. Casas <[email protected]>
MylesBorins pushed a commit that referenced this pull request Mar 7, 2017
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. Ref: #9827 PR-URL: #10153 Reviewed-By: Jeremiah Senkpiel <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Italo A. Casas <[email protected]>
MylesBorins pushed a commit that referenced this pull request Mar 9, 2017
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. Ref: #9827 PR-URL: #10153 Reviewed-By: Jeremiah Senkpiel <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Italo A. Casas <[email protected]>
@MylesBorinsMylesBorins mentioned this pull request Mar 9, 2017
MylesBorins pushed a commit that referenced this pull request Mar 9, 2017
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. Ref: #9827 PR-URL: #10153 Reviewed-By: Jeremiah Senkpiel <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Italo A. Casas <[email protected]>
@MylesBorinsMylesBorins mentioned this pull request Mar 9, 2017
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

buildIssues and PRs related to build files or the CI.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants

@danbev@richardlau@addaleax@jasnell@Fishrock123@cjihrig@italoacasas@sxa@MylesBorins@nodejs-github-bot