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

Please remove eu-strip from tarball #20280

Closed
kapouer opened this issue Apr 25, 2018 · 4 comments
Closed

Please remove eu-strip from tarball #20280

kapouer opened this issue Apr 25, 2018 · 4 comments
Labels
build Issues and PRs related to build files or the CI. good first issue Issues that are suitable for first-time contributors.

Comments

@kapouer
Copy link
Contributor

kapouer commented Apr 25, 2018

nodejs 10.0.0 tarball contains deps/v8/third_party/eu-strip/ which provides a precompiled elf-executable for v8 build bots (i think). It's been added to v8 here:
v8/v8@571076e

This poses some problems:

  • the source is not provided (this is the worst problem from the distributor point of view)
  • it adds 105ko of useless files

Could it be removed ? Thanks !
@bnoordhuis bnoordhuis added the build Issues and PRs related to build files or the CI. label Apr 25, 2018
@bnoordhuis
Copy link
Member

I think we can drop it entirely, not just from the tarball. It's only used in the android and fuchsia GN builds.

@bnoordhuis bnoordhuis added the good first issue Issues that are suitable for first-time contributors. label Apr 25, 2018
@richardlau
Copy link
Member

If we do strip this out we should also update the instructions in https://github.com/nodejs/node/blob/master/doc/guides/maintaining-V8.md and the git node v8 command in https://github.com/nodejs/node-core-utils to stop it coming back the next time V8 is updated.

cc @nodejs/v8-update

@jvelezpo
Copy link
Contributor

Hello,
I will be more than happy to provide a PR for this, here and also on node-core-utils
@richardlau could you give me some advice where should i modify the doc (https://github.com/nodejs/node/blob/master/doc/guides/maintaining-V8.md), i am reading it, but i dont see where :/

@richardlau
Copy link
Member

@jvelezpo you're probably better off asking @targos who's done most of the major V8 updates to Node.js.

@jvelezpo jvelezpo mentioned this issue Apr 25, 2018
Closed
2 tasks
MylesBorins pushed a commit that referenced this issue May 4, 2018
@jvelezpo @MylesBorins
doc: remove eu-strip from tarball
9c11a18 
This is been removed because of this:
the source is not provided
it adds 105ko of useless files
It's only used in the android and fuchsia GN builds

Fixes: #20280

PR-URL: #20304
Fixes: #20280
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Matheus Marchini <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this issue Aug 17, 2018
@jvelezpo @MylesBorins
doc: remove eu-strip from tarball
24a105f 
This is been removed because of this:
the source is not provided
it adds 105ko of useless files
It's only used in the android and fuchsia GN builds

Fixes: #20280

PR-URL: #20304
Fixes: #20280
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Matheus Marchini <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
build Issues and PRs related to build files or the CI. good first issue Issues that are suitable for first-time contributors.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@kapouer @bnoordhuis @jvelezpo @richardlau