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.

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

Jump to bottom

build,win: distribute the install-tools scripts in the zip #24233

Closed
wants to merge 2 commits into from
Closed

build,win: distribute the install-tools scripts in the zip #24233

wants to merge 2 commits into from

Conversation

refack
Copy link
Contributor

@refack refack commented Nov 7, 2018

  • pack install-tools.* into the zip
  • reroute MSI files sources
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@refack refack added windows Issues and PRs related to the Windows platform. install Issues and PRs related to the installers. build Issues and PRs related to build files or the CI. labels Nov 7, 2018
@nodejs-github-bot
Copy link
Collaborator

@refack build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/1530/pipeline

@nodejs-github-bot nodejs-github-bot added the tools Issues and PRs related to the tools directory. label Nov 7, 2018
@refack refack added semver-minor PRs that contain new features and should be released in the next minor version. and removed tools Issues and PRs related to the tools directory. labels Nov 7, 2018
@refack refack mentioned this pull request Nov 7, 2018
Open
joaocgreis
joaocgreis suggested changes Nov 10, 2018
View reviewed changes
Copy link
Member

@joaocgreis joaocgreis left a comment

Choose a reason for hiding this comment

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

@refack the changes make sense, thanks! If you could test your changes locally before pushing, that would probably save us review cycles.

vcbuild.bat Outdated Show resolved Hide resolved
tools/msvs/msi/product.wxs Outdated Show resolved Hide resolved
@refack
Copy link
Contributor Author

refack commented Nov 11, 2018

If you could test your changes locally before pushing, that would probably save us review cycles.

Sorry for the broken code, but I couldn't test this locally. IMHO we should cover this code in the test CI, since we have had some regressions, and maybe some more are hiding here ¯_(ツ)_/¯

@Trott
Copy link
Member

Trott commented Nov 20, 2018

@refack Still working on this (or planning to in any event)? Making sure we shouldn't apply the stalled or blocked labels (or help wanted) or anything like that...

@refack refack force-pushed the distribute-windows-install-tools-script branch from f786fad to e815709 Compare November 25, 2018 00:44
@refack
Copy link
Contributor Author

refack commented Nov 25, 2018

Build test release - https://ci-release.nodejs.org/job/iojs+release/3961/

@refack refack added the wip Issues and PRs that are still a work in progress. label Nov 25, 2018
@refack refack force-pushed the distribute-windows-install-tools-script branch from e815709 to 0c802c5 Compare November 25, 2018 22:57
@refack refack removed the wip Issues and PRs that are still a work in progress. label Nov 25, 2018
@refack
Copy link
Contributor Author

refack commented Nov 25, 2018 •
edited
Loading

Tested manually and in https://ci-release.nodejs.org/job/iojs+release/3963/ ✔️
@joaocgreis PTAL

@Trott
Copy link
Member

Trott commented Nov 30, 2018

/ping @joaocgreis LGTY?

joaocgreis
joaocgreis approved these changes Dec 4, 2018
View reviewed changes
Copy link
Member

@joaocgreis joaocgreis left a comment

Choose a reason for hiding this comment

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

Looks great now, thanks!

@Trott
Copy link
Member

Trott commented Dec 5, 2018

Since this touches vcbuild.bat, a generic CI seems in order:
https://ci.nodejs.org/job/node-test-pull-request/19235/

@Trott Trott added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Dec 5, 2018
@Trott
Copy link
Member

Trott commented Dec 5, 2018

Resume Build CI: https://ci.nodejs.org/job/node-test-pull-request/19246/

@Trott
Copy link
Member

Trott commented Dec 6, 2018

Landed in 2fab5d5

@Trott Trott closed this Dec 6, 2018
Trott pushed a commit to Trott/io.js that referenced this pull request Dec 6, 2018
@refack @Trott
build,win: pack the install-tools scripts for dist
2fab5d5 
PR-URL: nodejs#24233
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: João Reis <reis@janeasystems.com>
BridgeAR pushed a commit that referenced this pull request Dec 6, 2018
@refack @BridgeAR
build,win: pack the install-tools scripts for dist
66602ad 
PR-URL: #24233
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: João Reis <reis@janeasystems.com>
BridgeAR added a commit that referenced this pull request Dec 6, 2018
@BridgeAR
2018-12-06, Version 11.4.0 (Current)
758cb07 
Notable Changes:

* console,util:
  * `console` functions now handle symbols as defined in the spec.
    #23708
  * The inspection `depth` default is now back at 2.
    #24326
* dgram,net:
  * Added ipv6Only option for `net` and `dgram`.
    #23798
* http:
  * Chosing between the http parser is now possible per runtime flag.
    #24739
* readline:
  * The `readline` module now supports async iterators.
    #23916
* repl:
  * The multiline history feature is removed.
    #24804
* tls:
  * Added min/max protocol version options.
    #24405
  * The X.509 public key info now includes the RSA bit size and the
    elliptic curve. #24358
* url:
  * `pathToFileURL()` now supports LF, CR and TAB.
    #23720
* Windows:
  * Tools are not installed using Boxstarter anymore.
    #24677
  * The install-tools scripts or now included in the dist.
    #24233
* Added new collaborator:
  * [antsmartian](https://github.com/antsmartian) - Anto Aravinth.
    #24655

PR-URL: #24854
@BridgeAR BridgeAR mentioned this pull request Dec 6, 2018
Merged
4 tasks
BridgeAR pushed a commit that referenced this pull request Dec 7, 2018
@refack @BridgeAR
build,win: pack the install-tools scripts for dist
706bc41 
PR-URL: #24233
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: João Reis <reis@janeasystems.com>
BridgeAR pushed a commit that referenced this pull request Dec 7, 2018
@refack @BridgeAR
build,win: pack the install-tools scripts for dist
b80690d 
PR-URL: #24233
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: João Reis <reis@janeasystems.com>
BridgeAR added a commit that referenced this pull request Dec 7, 2018
@BridgeAR
2018-12-07, Version 11.4.0 (Current)
4f1297f 
Notable Changes:

* console,util:
  * `console` functions now handle symbols as defined in the spec.
    #23708
  * The inspection `depth` default is now back at 2.
    #24326
* dgram,net:
  * Added ipv6Only option for `net` and `dgram`.
    #23798
* http:
  * Chosing between the http parser is now possible per runtime flag.
    #24739
* readline:
  * The `readline` module now supports async iterators.
    #23916
* repl:
  * The multiline history feature is removed.
    #24804
* tls:
  * Added min/max protocol version options.
    #24405
  * The X.509 public key info now includes the RSA bit size and the
    elliptic curve. #24358
* url:
  * `pathToFileURL()` now supports LF, CR and TAB.
    #23720
* Windows:
  * Tools are not installed using Boxstarter anymore.
    #24677
  * The install-tools scripts or now included in the dist.
    #24233
* Added new collaborator:
  * [antsmartian](https://github.com/antsmartian) - Anto Aravinth.
    #24655

PR-URL: #24854
BridgeAR added a commit that referenced this pull request Dec 7, 2018
@BridgeAR
2018-12-07, Version 11.4.0 (Current)
bcef949 
Notable Changes:

* console,util:
  * `console` functions now handle symbols as defined in the spec.
    #23708
  * The inspection `depth` default is now back at 2.
    #24326
* dgram,net:
  * Added ipv6Only option for `net` and `dgram`.
    #23798
* http:
  * Chosing between the http parser is now possible per runtime flag.
    #24739
* readline:
  * The `readline` module now supports async iterators.
    #23916
* repl:
  * The multiline history feature is removed.
    #24804
* tls:
  * Added min/max protocol version options.
    #24405
  * The X.509 public key info now includes the RSA bit size and the
    elliptic curve. #24358
* url:
  * `pathToFileURL()` now supports LF, CR and TAB.
    #23720
* Windows:
  * Tools are not installed using Boxstarter anymore.
    #24677
  * The install-tools scripts or now included in the dist.
    #24233
* Added new collaborator:
  * [antsmartian](https://github.com/antsmartian) - Anto Aravinth.
    #24655

PR-URL: #24854
This was referenced Dec 7, 2018
Closed
Closed
Closed
Closed
Closed
@refack refack deleted the distribute-windows-install-tools-script branch December 21, 2018 01:12
refack added a commit to refack/node that referenced this pull request Jan 14, 2019
@refack
build,win: pack the install-tools scripts for dist
87fdadd 
PR-URL: nodejs#24233
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: João Reis <reis@janeasystems.com>
refack pushed a commit to refack/node that referenced this pull request Jan 14, 2019
@BridgeAR @refack
2018-12-07, Version 11.4.0 (Current)
5912f6d 
Notable Changes:

* console,util:
  * `console` functions now handle symbols as defined in the spec.
    nodejs#23708
  * The inspection `depth` default is now back at 2.
    nodejs#24326
* dgram,net:
  * Added ipv6Only option for `net` and `dgram`.
    nodejs#23798
* http:
  * Chosing between the http parser is now possible per runtime flag.
    nodejs#24739
* readline:
  * The `readline` module now supports async iterators.
    nodejs#23916
* repl:
  * The multiline history feature is removed.
    nodejs#24804
* tls:
  * Added min/max protocol version options.
    nodejs#24405
  * The X.509 public key info now includes the RSA bit size and the
    elliptic curve. nodejs#24358
* url:
  * `pathToFileURL()` now supports LF, CR and TAB.
    nodejs#23720
* Windows:
  * Tools are not installed using Boxstarter anymore.
    nodejs#24677
  * The install-tools scripts or now included in the dist.
    nodejs#24233
* Added new collaborator:
  * [antsmartian](https://github.com/antsmartian) - Anto Aravinth.
    nodejs#24655

PR-URL: nodejs#24854
@refack refack restored the distribute-windows-install-tools-script branch January 22, 2019 21:42
@BethGriggs
Copy link
Member

Should this land on v10.x? Please open a backport PR if so

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@richardlau richardlau richardlau approved these changes

@jasnell jasnell jasnell approved these changes

@joaocgreis joaocgreis joaocgreis approved these changes

@bzoz bzoz Awaiting requested review from bzoz

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. build Issues and PRs related to build files or the CI. install Issues and PRs related to the installers. semver-minor PRs that contain new features and should be released in the next minor version. windows Issues and PRs related to the Windows platform.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@refack @nodejs-github-bot @Trott @BethGriggs @joaocgreis @jasnell @richardlau