Skip to content

Conversation

@refack
Copy link
Contributor

@refackrefack commented Mar 12, 2019

Replace the abandoned GYP with GYP3, which has the following benefits:

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

Fixes: #28555

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-botnodejs-github-bot added build Issues and PRs related to build files or the CI. meta Issues and PRs related to the general management of the project. tools Issues and PRs related to the tools directory. labels Mar 12, 2019
@refackrefack self-assigned this Mar 12, 2019
@refackrefack added wip Issues and PRs that are still a work in progress. gyp Issues and PRs related to the GYP tool and .gyp build files labels Mar 12, 2019
@refackrefack added this to the 12.0.0 milestone Mar 12, 2019
@refack

This comment has been minimized.

@Trott
Copy link
Member

(If possible to change now that it's opened, maybe make this a draft pull request?)

@refack
Copy link
ContributorAuthor

refack commented Mar 12, 2019

(If possible to change now that it's opened, maybe make this a draft pull request?)

I was looking for how to do that... Only found it after reading the docs.

Anyway, this should be working now, and it's ready for Rubber Stamp reviews.
https://ci.nodejs.org/job/node-test-pull-request/21487/
https://ci.nodejs.org/job/node-test-commit/26674/
(I might patch GYP3 a bit more)

@refackrefack changed the title [WIP]tools: replace GYP with GYP3tools: replace GYP with GYP3Mar 12, 2019
@refackrefack removed the wip Issues and PRs that are still a work in progress. label Mar 12, 2019
@refackrefackforce-pushed the bump-gyp3-86ab71caad branch from e918e3f to 2b6fd3fCompareMarch 12, 2019 23:57
@richardlau
Copy link
Member

* Lives in GitHub - https://github.com/refack/GYP. 

Whatever happened to the plan to transfer into this org? nodejs/admin#247

@refack
Copy link
ContributorAuthor

Whatever happened to the plan to transfer into this org? nodejs/admin#247

That has been shelved for various $REASONS.

@targos
Copy link
Member

There are unrelated changes in the LICENSE file

@refack
Copy link
ContributorAuthor

refack commented Mar 14, 2019

There are unrelated changes in the LICENSE file

That's what I got from re-generating 🤷‍♂️

BTW: for easy review, first commit is pure GYP vendoring. Second (ATM 2b6fd3f) is just changes in node file.

@refackrefack added the python PRs and issues that require attention from people who are familiar with Python. label Mar 15, 2019
Copy link
Contributor

@ryzokukenryzokuken left a comment

Choose a reason for hiding this comment

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

Major +1.

@targos
Copy link
Member

That's what I got from re-generating 🤷‍♂️

I can't reproduce. If I run ./tools/license-builder.sh on master, LICENSE is unchanged.

@refackrefackforce-pushed the bump-gyp3-86ab71caad branch from 2b6fd3f to 49a1c5fCompareMarch 16, 2019 19:30
@refack
Copy link
ContributorAuthor

That's what I got from re-generating 🤷‍♂️

I can't reproduce. If I run ./tools/license-builder.sh on master, LICENSE is unchanged.

Issue resolved. LICENSE regenerated.

@refack

This comment has been minimized.

Copy link
Member

@targostargos left a comment

Choose a reason for hiding this comment

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

Rubber stamp

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

@gengjiawen
Copy link
Member

@refack Need rebase.

@refack
Copy link
ContributorAuthor

Anyone want to help me figure out what I broke WRT cross-compilation? refack/GYP3@bd11dd1...d1f1343

@refackrefackforce-pushed the bump-gyp3-86ab71caad branch from 73fd43c to a669e35CompareApril 16, 2019 16:07
@nodejs-github-bot

This comment has been minimized.

@richard-townsend-arm
Copy link
Contributor

AIX builds seem to have the following error in their logs (e.g. https://ci.nodejs.org/job/node-test-commit-aix/22631/nodes=aix61-ppc64/console)
17:13:57 ccache: error: Could not find compiler "cc" in PATH

@richard-townsend-arm
Copy link
Contributor

ARMv7 builds seem to fail with this error:

../deps/v8/src/arm/assembler-arm.cc:177:2: error: #error "CAN_USE_ARMV7_INSTRUCTIONS should match CAN_USE_VFP3_INSTRUCTIONS" #error "CAN_USE_ARMV7_INSTRUCTIONS should match CAN_USE_VFP3_INSTRUCTIONS
(https://ci.nodejs.org/job/node-cross-compile/23120/nodes=cross-compiler-armv7-gcc-4.9.4/console)

@refackrefackforce-pushed the bump-gyp3-86ab71caad branch from a669e35 to aec2150CompareMay 30, 2019 23:15
@nodejs-github-bot
Copy link
Collaborator

@Croydon
Copy link

What is the status of this?

@sam-github
Copy link
Contributor

FWIW, sam-github@c70af5c is another vendoring attempt I made that duplicates this, except its done against current master.

@cclauss
Copy link
Contributor

While I was a huge fan of this PR back in the day, I now believe that we can close it. Is there anything that we need to carry forward?

@sam-github
Copy link
Contributor

Closing because it isn't active, but not because it isn't worthwhile if anyone wants to take it up again.

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

Labels

blockedPRs that are blocked by other issues or PRs.buildIssues and PRs related to build files or the CI.gypIssues and PRs related to the GYP tool and .gyp build filesmetaIssues and PRs related to the general management of the project.pythonPRs and issues that require attention from people who are familiar with Python.toolsIssues and PRs related to the tools directory.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Re-vendor node/node-gyp --> tools/gyp/

14 participants

@refack@nodejs-github-bot@Trott@richardlau@targos@bzoz@cclauss@BridgeAR@gengjiawen@richard-townsend-arm@Croydon@sam-github@thefourtheye@ryzokuken