Skip to content

build: fix RELEASE check (URGENT)#1421

Closed
rvagg wants to merge 1 commit into
nodejs:v1.xfrom
rvagg:fix-hell-sed
Closed

build: fix RELEASE check (URGENT)#1421
rvagg wants to merge 1 commit into
nodejs:v1.xfrom
rvagg:fix-hell-sed

Conversation

@rvagg

@rvagg rvagg commented Apr 14, 2015

Copy link
Copy Markdown
Member

Ref: #1405

please review @jbergstroem || @bnoordhuis asap or I'm just going to merge this and release a 1.7.1, 1.7.0 is a write-off because this is broken unfortunately.

@rvagg

rvagg commented Apr 14, 2015

Copy link
Copy Markdown
Member Author

for reference, the error on a release build is:

#NODE_VERSION_IS_RELEASE is set to (hell sed -ne 's/.
Did you remember to update src/node_version.h?

I was weary about this change and even went to the effort of testing the suggested sed change on a few platforms, pity I missed the syntax error in the process.

@rvagg

rvagg commented Apr 14, 2015

Copy link
Copy Markdown
Member Author

meh, merging without review

rvagg added a commit that referenced this pull request Apr 14, 2015
fixes broken 1.7.0 build, unreviewed quick patch

Ref: #1405
PR-URL: #1421
@rvagg

rvagg commented Apr 14, 2015

Copy link
Copy Markdown
Member Author

landed @ fa91b18

@rvagg rvagg closed this Apr 14, 2015
@jbergstroem

Copy link
Copy Markdown
Member

For what its worth: post-merge LGTM. Sorry about the slip-up.

@rvagg rvagg mentioned this pull request Apr 14, 2015
@rvagg

rvagg commented Apr 14, 2015

Copy link
Copy Markdown
Member Author

still no good, on OSX:

Makefile:200: *** unterminated call to function `shell': missing `)'.  Stop.

I don't even ...

Makefiles :shakesfist: and cross-platform sed :shakesfist:

rvagg added a commit to rvagg/io.js that referenced this pull request Apr 14, 2015
Notable changes:

* build: A syntax error in the Makefile for release builds caused
  1.7.0 to be DOA and unreleased. (Rod Vagg) nodejs#1421.
@jbergstroem

Copy link
Copy Markdown
Member

I can't reproduce. Tried on gnu make 3.81:

# cat foo 
all:
    RELEASE=$(shell sed -ne 's/#define NODE_VERSION_IS_RELEASE \([01]\)/\1/p' src/node_version.h)

# make -f foo
RELEASE=0

rvagg added a commit that referenced this pull request Apr 14, 2015
fixes broken 1.7.0 build, unreviewed quick patch

Ref: #1405
PR-URL: #1421
@rvagg

rvagg commented Apr 14, 2015

Copy link
Copy Markdown
Member Author

force-pushed another fix in this from

RELEASE=$(shell sed -ne 's/#define NODE_VERSION_IS_RELEASE \([01]\)/\1/p' src/node_version.h)

to

RELEASE=$(shell sed -ne 's/\#define NODE_VERSION_IS_RELEASE \([01]\)/\1/p' src/node_version.h)

it's now aee86a2

thanks @jbergstroem for spotting that as the problem

rvagg added a commit that referenced this pull request Apr 14, 2015
Notable changes:

* build: A syntax error in the Makefile for release builds caused
  1.7.0 to be DOA and unreleased. (Rod Vagg) #1421
jasnell added a commit to nodejs/dev-policy that referenced this pull request Apr 15, 2015
Add allowance for release time commits and error corrections.
Per: #48 (comment)
Example: nodejs/node#1421 and nodejs/node@fd90b33...50e9fc1.
dreamhigh0525 pushed a commit to dreamhigh0525/dev-policy that referenced this pull request Mar 9, 2023
Add allowance for release time commits and error corrections.
Per: nodejs/dev-policy#48 (comment)
Example: nodejs/node#1421 and nodejs/node@fd90b33...50e9fc1.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants