Skip to content
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.

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

Get node-test-node-addon-api green on windows #289

Closed
mhdawson opened this issue Jan 4, 2018 · 5 comments
Closed

Get node-test-node-addon-api green on windows #289

mhdawson opened this issue Jan 4, 2018 · 5 comments
Assignees
Milestone

Comments

@mhdawson
Copy link
Member

mhdawson commented Jan 4, 2018

Works ok except on 10 nightly

Info to reproduce steps in the build job

https://nodejs.org/download/nightly/v10.0.0-nightly201801043f2382bd9a/
node -v
  npm -v
  export npm_loglevel=error
  npm set progress=false
  ls

  git clone https://github.com/$GIT_REPO.git node-addon-api
  cd node-addon-api
  if [ $GIT_BRANCH != "master" ]; then
  git fetch origin $GIT_BRANCH:testBranch
  git checkout testBranch
  fi
  npm install

  # This line needs to be updated to use the configuration string to test right version
  # citgm -v verbose --tmpDir $WORKSPACE/tmp https://github.com/$NODEREPORT_REPO/archive/$NODEREPORT_BRANCH.tar.gz

  npm test

@mhdawson mhdawson added this to the Milestone 9 milestone Jan 4, 2018
@digitalinfinity
Copy link
Contributor

This is because node on windows uses version strings with the style "v10.0.0-pre" instead of "v10.0.0-nightly20171202c589561aaf" (which is what other platforms use). This first showed up on nightly/10.0.0-nightly20171203fff4272fa7 (it was fine as of nightly/10.0.0-nightly20171202c589561aaf). Looking at the commits between those two builds, nodejs/node@9fb390a1c6 seems to be the likely change (although it could be a build issue too). @refack can you help please? With the current behavior, it doesn't seem like node-gyp would work against windows nightly builds.

cc @nodejs/build

@rvagg
Copy link
Member

rvagg commented Jan 8, 2018

-call :run-python configure %configure_flags% --dest-cpu=%target_arch% --tag=%TAG% %link_module%
+call :run-python configure %configure_flags%

(in @digitalinfinity's commit link)

./configure --tag foo -> 'defines': [ 'NODE_TAG="<(node_tag)"' ] in node.gypi -> src/node_version.h:

#ifndef NODE_TAG
# if NODE_VERSION_IS_RELEASE
#  define NODE_TAG ""
# else
#  define NODE_TAG "-pre"
# endif
#else

@rvagg
Copy link
Member

rvagg commented Jan 8, 2018

nodejs/node#18031

rvagg added a commit to rvagg/io.js that referenced this issue Jan 9, 2018
--tag needs to be set after `getnodeversion` because TAG is defined in
there when DISTTYPE is not "release", setting it before `getnodeversion`
leads to --tag not being passed down in to `configure` and
src/node_version.h setting it as `-pre` by default. This change restores
the functionality that properly sets the TAG for nightlies, rc builds
and other custom build types.

Ref: nodejs#17299
Ref: nodejs/abi-stable-node#289
@aruneshchandra
Copy link
Contributor

@mhdawson will start CI on this

mhdawson pushed a commit to nodejs/node that referenced this issue Jan 11, 2018
--tag needs to be set after `getnodeversion` because TAG is defined in
there when DISTTYPE is not "release", setting it before `getnodeversion`
leads to --tag not being passed down in to `configure` and
src/node_version.h setting it as `-pre` by default. This change restores
the functionality that properly sets the TAG for nightlies, rc builds
and other custom build types.

Ref: #17299
Ref: nodejs/abi-stable-node#289

PR-URL: #18031
Ref: #17299
Ref: nodejs/abi-stable-node#289
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: JoãReis <reis@janeasystems.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Kyle Farnung <kfarnung@microsoft.com>
kfarnung pushed a commit to kfarnung/node-chakracore that referenced this issue Jan 11, 2018
--tag needs to be set after `getnodeversion` because TAG is defined in
there when DISTTYPE is not "release", setting it before `getnodeversion`
leads to --tag not being passed down in to `configure` and
src/node_version.h setting it as `-pre` by default. This change restores
the functionality that properly sets the TAG for nightlies, rc builds
and other custom build types.

Ref: nodejs/node#17299
Ref: nodejs/abi-stable-node#289

PR-URL: nodejs/node#18031
Ref: nodejs/node#17299
Ref: nodejs/abi-stable-node#289
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: JoãReis <reis@janeasystems.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Kyle Farnung <kfarnung@microsoft.com>
@mhdawson
Copy link
Member Author

CI is now green, looks good.

evanlucas pushed a commit to nodejs/node that referenced this issue Jan 30, 2018
--tag needs to be set after `getnodeversion` because TAG is defined in
there when DISTTYPE is not "release", setting it before `getnodeversion`
leads to --tag not being passed down in to `configure` and
src/node_version.h setting it as `-pre` by default. This change restores
the functionality that properly sets the TAG for nightlies, rc builds
and other custom build types.

Ref: #17299
Ref: nodejs/abi-stable-node#289

PR-URL: #18031
Ref: #17299
Ref: nodejs/abi-stable-node#289
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: JoãReis <reis@janeasystems.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Kyle Farnung <kfarnung@microsoft.com>
MylesBorins pushed a commit to nodejs/node that referenced this issue May 15, 2018
--tag needs to be set after `getnodeversion` because TAG is defined in
there when DISTTYPE is not "release", setting it before `getnodeversion`
leads to --tag not being passed down in to `configure` and
src/node_version.h setting it as `-pre` by default. This change restores
the functionality that properly sets the TAG for nightlies, rc builds
and other custom build types.

Ref: #17299
Ref: nodejs/abi-stable-node#289

PR-URL: #18031
Ref: #17299
Ref: nodejs/abi-stable-node#289
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: JoãReis <reis@janeasystems.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Kyle Farnung <kfarnung@microsoft.com>
MylesBorins pushed a commit to nodejs/node that referenced this issue May 15, 2018
--tag needs to be set after `getnodeversion` because TAG is defined in
there when DISTTYPE is not "release", setting it before `getnodeversion`
leads to --tag not being passed down in to `configure` and
src/node_version.h setting it as `-pre` by default. This change restores
the functionality that properly sets the TAG for nightlies, rc builds
and other custom build types.

Ref: #17299
Ref: nodejs/abi-stable-node#289

PR-URL: #18031
Ref: #17299
Ref: nodejs/abi-stable-node#289
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: JoãReis <reis@janeasystems.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Kyle Farnung <kfarnung@microsoft.com>
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

No branches or pull requests

4 participants