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

win,build: add creation of zip and 7z package #5995

Closed

Conversation

joaocgreis
Copy link
Member

Pull Request check-list

  • Does make -j8 test (UNIX) or vcbuild test nosign (Windows) pass with
    this change (including linting)?
  • Is the commit message formatted according to [CONTRIBUTING.md][0]?

Affected core subsystem(s)

Windows, build.

Description of change

Add a step in vcbuild.bat to create a minimal package including node and npm that can be used as an alternative to the MSI, compress the node.pdb file as zip and 7z and upload all files as part of build-release.

This requires 7zip to be installed on the release machines. If there is no objection, I plan to add it and test this with a nightly. Files are included in the upload stage, do we need to do anything else to add them to releases @nodejs/build @nodejs/release ?

Fixes: nodejs/build#299
Fixes: #5696

cc @nodejs/platform-windows

@joaocgreis joaocgreis added windows Issues and PRs related to the Windows platform. build Issues and PRs related to build files or the CI. labels Apr 1, 2016
@richardlau
Copy link
Member

Should the zip also contain Node's license file?

@bzoz bzoz force-pushed the Bartosz-MakeWindowsZipPackage branch from 27b681c to 5731fb0 Compare April 5, 2016 17:30
@bzoz
Copy link
Contributor

bzoz commented Apr 5, 2016

Updated missing files, PTAL

@joaocgreis
Copy link
Member Author

@nodejs/build added 7zip to the release machines.

Here is a test build: https://nodejs.org/download/test/v6.0.0-test20160411758655d70e88446a24d8dbbb07f1c1defd317293/

@rvagg
Copy link
Member

rvagg commented Apr 11, 2016

I think the bundle files should go in the parent directory, leaving win-x* for individual files (including the zipped pdb)

@benjamingr
Copy link
Member

Silly question perhaps, but are we completely OK here in terms of licensing? I remember 7z is an open LGPL format and IIRC there are open clients for opening it on all supported platforms but I want to make sure someone "from the loop" is aware.

@bzoz
Copy link
Contributor

bzoz commented Apr 12, 2016

Moved the archives to parent directory.

@joaocgreis
Copy link
Member Author

@benjamingr I'm not completely sure about anything related to licences. We are only using 7 Zip the executable, not linking against it or the dll. Is it an issue?

@benjamingr
Copy link
Member

I'm 99% sure that this is not an issue and I'm probably a lot more clueless than you about it - but since I haven't seen 7z files in the project before I think it would be a good idea to make sure with someone who is sure.

@orangemocha
Copy link
Contributor

@mikeal can you maybe help answer the licensing concern?

@joaocgreis
Copy link
Member Author

joaocgreis commented May 5, 2016

From http://www.7-zip.org/license.txt :

Note:
You can use 7-Zip on any computer, including a computer in a commercial
organization. You don't need to register or pay for 7-Zip.

Hence, we can move ahead if there are no more specific concerns.

@bzoz bzoz force-pushed the Bartosz-MakeWindowsZipPackage branch from 2cdd6fd to ef2e444 Compare May 5, 2016 11:43
@joaocgreis
Copy link
Member Author

@joaocgreis
Copy link
Member Author

@nodejs/build @nodejs/platform-windows this is still in need of a LGTM, can you take a look?

@joaocgreis
Copy link
Member Author

@rvagg After moving the zip and 7z files out of the win-x* directory, they are left in the staging server and do not appear online. Can you point us to the correct place to change to make this work? Thanks!

bzoz and others added 6 commits May 13, 2016 12:44
Add a step in vcbuild.bat to create a minimal package including node
and npm that can be used as an alternative to the MSI.

Compress the node.pdb file as zip and 7z.

All files are uploaded as part of build-release.

Fixes: nodejs/build#299
Fixes: nodejs#5696
@joaocgreis joaocgreis force-pushed the Bartosz-MakeWindowsZipPackage branch from ef2e444 to 36b525a Compare May 13, 2016 11:47
@rvagg
Copy link
Member

rvagg commented May 15, 2016

@joaocgreis I just ran another test @ https://nodejs.org/download/test/v7.0.0-test2016051536b525a781/ and managed to catch this state in staging before the *.done files were cleaned up—which is done every 5 minutes after the matching files are moved to dist.

-rw-r--r--  1 staging staging  6559346 May 15 09:52 node-v7.0.0-test2016051536b525a781-win-x64.7z
-rw-r--r--  1 staging staging 12007980 May 15 09:52 node-v7.0.0-test2016051536b525a781-win-x64.zip
-rw-rw-r--  1 staging staging        0 May 15 09:52 node-v7.0.0-test2016051536b525a781-x64.7z.done
-rw-rw-r--  1 staging staging        0 May 15 09:52 node-v7.0.0-test2016051536b525a781-x64.zip.done

i.e. it's just a simple mismatch between the .done files and their promotables:

node-v%FULLVERSION%-win-%target_arch%.7z and node-v%FULLVERSION%-win-%target_arch%.zip

but the touch makes:

node-v%FULLVERSION%-%target_arch%.zip.done and node-v%FULLVERSION%-%target_arch%.7z.done

Put the win- in there and it should all work fine and we can run another test.

@bzoz
Copy link
Contributor

bzoz commented May 16, 2016

I've updated the filenames, PTAL

@joaocgreis
Copy link
Member Author

@rvagg Thanks!

Started another build, this time it looks good: https://nodejs.org/download/test/v7.0.0-test201605168b733b76c753513ff87ed841e2bef69b12fe0460/

@rvagg
Copy link
Member

rvagg commented May 17, 2016

yes, lgtm, but let's give @nodejs/ctc a chance to jump in here before you merge since this is actually not a small deal. Once we start shipping .7z and .zip files for Windows we're not going to be able to undo that very easily in the future, so it's a big commitment.

Our dist directories will start to look like this:

../
docs/
win-x64/
win-x86/
SHASUMS256.txt
node-v7.0.0-darwin-x64.tar.gz
node-v7.0.0-darwin-x64.tar.xz
node-v7.0.0-headers.tar.gz
node-v7.0.0-headers.tar.xz
node-v7.0.0-linux-arm64.tar.gz
node-v7.0.0-linux-arm64.tar.xz
node-v7.0.0-linux-armv6l.tar.gz
node-v7.0.0-linux-armv6l.tar.xz
node-v7.0.0-linux-armv7l.tar.gz
node-v7.0.0-linux-armv7l.tar.xz
node-v7.0.0-linux-ppc64le.tar.gz
node-v7.0.0-linux-ppc64le.tar.xz
node-v7.0.0-linux-ppc64.tar.gz
node-v7.0.0-linux-ppc64.tar.xz
node-v7.0.0-linux-x64.tar.gz
node-v7.0.0-linux-x64.tar.xz
node-v7.0.0-linux-x86.tar.gz
node-v7.0.0-linux-x86.tar.xz
node-v7.0.0.pkg
node-v7.0.0-sunos-x64.tar.gz
node-v7.0.0-sunos-x64.tar.xz
node-v7.0.0-sunos-x86.tar.gz
node-v7.0.0-sunos-x86.tar.xz
node-v7.0.0.tar.gz
node-v7.0.0.tar.xz
node-v7.0.0-win-x64.7z
node-v7.0.0-win-x64.zip
node-v7.0.0-win-x86.7z
node-v7.0.0-win-x86.zip
node-v7.0.0-x64.msi
node-v7.0.0-x86.msi

And both win-x64 and win-x86 have two new files, node_pdb.7z ~ 6.2M and node_pdb.zip ~ 10M.

<= v0.12 we ship(ped) with uncompressed node.pdb files ~31M but we canned that from io.js onward due to skepticism that they were even being used, and we have had minimal call to add them back. Here's the one issue we've had filed about it: #5696. .pdb files contain debugging information for Windows applications.

So, comment now before this all gets locked in.

@Fishrock123
Copy link
Contributor

How large is x86 windows usage? Maybe we could just ship x64 zips? (I have no idea)

@Fishrock123
Copy link
Contributor

How long does it take to produce node_pdb? Will that increase release build time significantly for the windows machines?

@joaocgreis
Copy link
Member Author

@Fishrock123 I made a quick experiment, zipping (2 zip + 2 7z) takes 1m21s in our release machines. I don't know exactly how long the upload takes. The test build I ran took 19m, but the Pi1 still takes the longest with 52m.

If the directory size is a concern, we can keep only the MSI in the main dir and move the others into the specific win dirs.

@Fishrock123
Copy link
Contributor

@joaocgreis I'm not worried about the zipping, I was wondering about generating the pdb file.

@joaocgreis
Copy link
Member Author

@Fishrock123 the pdb is always generated when compiling, no added delay there.

@rvagg
Copy link
Member

rvagg commented May 17, 2016

basic stats are at https://nodejs.org/metrics/summaries/os.png & https://nodejs.org/metrics/summaries/arch.png but there's no combination graph, the data is available @ https://nodejs.org/metrics/ if someone wants to look at the win/arch numbers. My gut feel is that win x86 is still very high.

Re directory size (in terms of number of entries) I'm not so concerned these days, we have a great downloads page that I think most people are using so they don't have to navigate a complex list if they are not capable of doing so.

@orangemocha
Copy link
Contributor

LGTM

@joaocgreis
Copy link
Member Author

@rvagg ok to land?

@rvagg
Copy link
Member

rvagg commented May 31, 2016

lgtm, make it so

joaocgreis pushed a commit that referenced this pull request May 31, 2016
Add a step in vcbuild.bat to create a minimal package including node
and npm that can be used as an alternative to the MSI.

Compress the node.pdb file as zip and 7z.

All files are uploaded as part of build-release.

Reviewed-By: Joao Reis <reis@janeasystems.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Alexis Campailla <orangemocha@nodejs.org>
PR-URL: #5995
Fixes: nodejs/build#299
Fixes: #5696
@joaocgreis
Copy link
Member Author

@joaocgreis joaocgreis closed this May 31, 2016
Fishrock123 pushed a commit that referenced this pull request Jun 1, 2016
Add a step in vcbuild.bat to create a minimal package including node
and npm that can be used as an alternative to the MSI.

Compress the node.pdb file as zip and 7z.

All files are uploaded as part of build-release.

Reviewed-By: Joao Reis <reis@janeasystems.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Alexis Campailla <orangemocha@nodejs.org>
PR-URL: #5995
Fixes: nodejs/build#299
Fixes: #5696
rvagg pushed a commit that referenced this pull request Jun 2, 2016
Add a step in vcbuild.bat to create a minimal package including node
and npm that can be used as an alternative to the MSI.

Compress the node.pdb file as zip and 7z.

All files are uploaded as part of build-release.

Reviewed-By: Joao Reis <reis@janeasystems.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Alexis Campailla <orangemocha@nodejs.org>
PR-URL: #5995
Fixes: nodejs/build#299
Fixes: #5696
@rvagg
Copy link
Member

rvagg commented Jun 8, 2016

oh, hey, these all came out in the latest release https://nodejs.org/download/release/latest-v6.x/

joaocgreis pushed a commit to JaneaSystems/node that referenced this pull request Jul 5, 2016
Add a step in vcbuild.bat to create a minimal package including node
and npm that can be used as an alternative to the MSI.

Compress the node.pdb file as zip and 7z.

All files are uploaded as part of build-release.

Reviewed-By: Joao Reis <reis@janeasystems.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Alexis Campailla <orangemocha@nodejs.org>
PR-URL: nodejs#5995
Fixes: nodejs/build#299
Fixes: nodejs#5696
MylesBorins pushed a commit that referenced this pull request Jul 8, 2016
Add a step in vcbuild.bat to create a minimal package including node
and npm that can be used as an alternative to the MSI.

Compress the node.pdb file as zip and 7z.

All files are uploaded as part of build-release.

Reviewed-By: Joao Reis <reis@janeasystems.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Alexis Campailla <orangemocha@nodejs.org>
PR-URL: #5995
Fixes: nodejs/build#299
Fixes: #5696
MylesBorins pushed a commit that referenced this pull request Jul 11, 2016
Add a step in vcbuild.bat to create a minimal package including node
and npm that can be used as an alternative to the MSI.

Compress the node.pdb file as zip and 7z.

All files are uploaded as part of build-release.

Reviewed-By: Joao Reis <reis@janeasystems.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Alexis Campailla <orangemocha@nodejs.org>
PR-URL: #5995
Fixes: nodejs/build#299
Fixes: #5696
MylesBorins pushed a commit that referenced this pull request Jul 12, 2016
Add a step in vcbuild.bat to create a minimal package including node
and npm that can be used as an alternative to the MSI.

Compress the node.pdb file as zip and 7z.

All files are uploaded as part of build-release.

Reviewed-By: Joao Reis <reis@janeasystems.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Alexis Campailla <orangemocha@nodejs.org>
PR-URL: #5995
Fixes: nodejs/build#299
Fixes: #5696
@MylesBorins MylesBorins mentioned this pull request Jul 12, 2016
MylesBorins pushed a commit that referenced this pull request Jul 14, 2016
Add a step in vcbuild.bat to create a minimal package including node
and npm that can be used as an alternative to the MSI.

Compress the node.pdb file as zip and 7z.

All files are uploaded as part of build-release.

Reviewed-By: Joao Reis <reis@janeasystems.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Alexis Campailla <orangemocha@nodejs.org>
PR-URL: #5995
Fixes: nodejs/build#299
Fixes: #5696
MylesBorins pushed a commit that referenced this pull request Jul 14, 2016
Add a step in vcbuild.bat to create a minimal package including node
and npm that can be used as an alternative to the MSI.

Compress the node.pdb file as zip and 7z.

All files are uploaded as part of build-release.

Reviewed-By: Joao Reis <reis@janeasystems.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Alexis Campailla <orangemocha@nodejs.org>
PR-URL: #5995
Fixes: nodejs/build#299
Fixes: #5696
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI. windows Issues and PRs related to the Windows platform.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants