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

tools: bump gyp to eb296f6 #12450

Closed
wants to merge 0 commits into from
Closed

tools: bump gyp to eb296f6 #12450

wants to merge 0 commits into from

Conversation

refack
Copy link
Contributor

@refack refack commented Apr 16, 2017

Bump GYP to eb296f6
Re float all patches since last bump - 1d65b99

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

tools,build,gyp

@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. tools Issues and PRs related to the tools directory. labels Apr 16, 2017
@refack
Copy link
Contributor Author

refack commented Apr 16, 2017

@addaleax
Copy link
Member

deps: float gyp patch for long filenames 592f89a

It should be safe to drop this now, the patch has been applied upstream

build: enable cctest to use generated objects 6ed488a

This commit seems noticeably different from the original? It might be a hint for solving @gibfahn’s #12448, though…

@refack
Copy link
Contributor Author

refack commented Apr 16, 2017

/cc @nodejs/build @addaleax @shigeki @jbergstroem @danbev
Do you know of any older than 1d65b99 patches that might need to be reapplied?

@refack
Copy link
Contributor Author

refack commented Apr 16, 2017

build: enable cctest to use generated objects 6ed488a

This commit seems noticeably different from the original? It might be a hint for solving @gibfahn’s #12448, though…

Was a mixed core + GYP commit (see my #11956 (comment)) I picked just the GYP part

@refack
Copy link
Contributor Author

refack commented Apr 16, 2017

deps: float gyp patch for long filenames 592f89a

It should be safe to drop this now, the patch has been applied upstream

Dropped

@addaleax
Copy link
Member

Was a mixed core + GYP commit (see my #11956 (comment)) I picked just the GYP part

I see that, but including it like this still seems a little misleading… at least the commit message and the PR-URL should probably be different here, to avoid confusion with the original commit.

@refack
Copy link
Contributor Author

refack commented Apr 16, 2017

I see that, but including it like this still seems a little misleading… at least the commit message and the PR-URL should probably be different here, to avoid confusion with the original commit.

gotcha

@refack
Copy link
Contributor Author

refack commented Apr 16, 2017

@addaleax bb49593 better?

@refack
Copy link
Contributor Author

refack commented Apr 16, 2017

As for the PR URL, I think it should stay since it's the actual motivation for the change. I'm just re-applying it in the context of GYP
I guess both should be there.

@shigeki
Copy link
Contributor

shigeki commented Apr 17, 2017

I only know the XCode floating patch that is included in this PR. I'm not sure if other else patches are needed.
As for the XCode patch, @bnoordhuis submitted the patch to the upstream in https://codereview.chromium.org/1213123002/ but it's been stalled.

@joaocgreis
Copy link
Member

tools/gyp/buildbot/aosp_manifest.xml, tools/gyp/pylib/gyp/generator/android.py and tools/gyp/pylib/gyp/generator/compile_commands_json.py are not present upstream, should be deleted in the first commit.

Isn't 3c46bb9 needed as well?

@refack about the partial commit message, it would be good to use a format similar to 5111e78, with the original message indented. Metadata should be at the end, so that core-validate-commit passes, and you probably want to avoid @-mentioning in the commit message because you'll get a notification every time this is rebased or cherry-picked.

@aqrln
Copy link
Contributor

aqrln commented Apr 17, 2017

@refack

As for the PR URL, I think it should stay since it's the actual motivation for the change. I'm just re-applying it in the context of GYP

I believe there's no need in the original metadata since the original commit hash is present in the commit message and it's easy to do git show locally or just click a link on GitHub. Yeah, by the way, that hash is wrong in your last commit.

@refack
Copy link
Contributor Author

refack commented Apr 17, 2017

@joaocgreis 3c46bb9 added. As for the partial commit, I'm with @aqrln, same tree, so just a pointer.

@refack
Copy link
Contributor Author

refack commented Apr 17, 2017

@aqrln
Copy link
Contributor

aqrln commented Apr 17, 2017

@refack still wrong commit hash. 6a09a69 is what you want ;)

@refack
Copy link
Contributor Author

refack commented Apr 17, 2017

@refack still wrong commit hash. 6a09a69 is what you want ;)

GitHub is a mysterious place, where does f5a9b16 exist?

@addaleax
Copy link
Member

where does f5a9b16 exist?

$ git for-each-ref --contains f5a9b16910878e35eb36f551ed6790c2ed8006f1
2d9967c1c6177c6e9273d6a87bd5ea99571afd74 commit	refs/remotes/upstream/pr/11956

#11956, where it came from :)

@refack
Copy link
Contributor Author

refack commented Apr 17, 2017

So GitHub creates pseudo-branches for PRs, independent of originating branch... I suspected so after I accidently patched #12231

@aqrln
Copy link
Contributor

aqrln commented Apr 17, 2017

@refack as Anna has just noted, in the PR, obviously :) But you don't want to link to the commit from the PR branch, you rather need the commit that has been landed to master.

UPD: oops, I think I'm late for a second with my answer.

@refack
Copy link
Contributor Author

refack commented Jun 18, 2017

@refack I think it's been long enough, could you raise a backport PR against v6.x-staging that contains all the relevant commits you mentioned above?

Should I add 444920d (enabling VS2017) as @joaocgreis requested?

@gibfahn
Copy link
Member

gibfahn commented Jun 18, 2017

Should I add 444920d (enabling VS2017) as @joaocgreis requested?

I'm assuming you mean 1c93e8c (the commit you mentioned doesn't have the PR-URL).

Yes why not, that landed a while back, and has been in 8 since 8.0.0. Are there any outstanding bugs/issues with it?

@refack
Copy link
Contributor Author

refack commented Jun 18, 2017

3fe980d (typo)
&
780acc2 (tooling edge case)
Both are minor

@gibfahn
Copy link
Member

gibfahn commented Jun 18, 2017

I meant bugs that haven't been fixed (and can't be backported as part of the same PR) 😁 .

Go ahead and open it, if people have issues they can comment on the PR.

refack pushed a commit to refack/node that referenced this pull request Jul 18, 2017
this is a re-base of the gyp part of
3c46bb9
after bumping GYP version to
https://chromium.googlesource.com/external/gyp/+/eb296f67da078ec01f5e3a9ea9cdc6d26d680161

Original-Review-By: James M Snell <jasnell@gmail.com>
Ref: nodejs#7986
PR-URL: nodejs#12450
Reviewed-By: João Reis <reis@janeasystems.com>
refack pushed a commit to refack/node that referenced this pull request Jul 18, 2017
this is a re-base of the gyp part of
6a09a69
after bumping GYP version to
https://chromium.googlesource.com/external/gyp/+/eb296f67da078ec01f5e3a9ea9cdc6d26d680161

Original-PR-URL: nodejs#11956
Original-Ref: nodejs#9163
Original-Reviewed-By: James M Snell <jasnell@gmail.com>

PR-URL: nodejs#12450
Reviewed-By: João Reis <reis@janeasystems.com>
refack pushed a commit to refack/node that referenced this pull request Aug 23, 2017
this is a re-base of the gyp part of
3c46bb9
after bumping GYP version to
https://chromium.googlesource.com/external/gyp/+/eb296f67da078ec01f5e3a9ea9cdc6d26d680161

Original-Review-By: James M Snell <jasnell@gmail.com>
Ref: nodejs#7986
PR-URL: nodejs#12450
Reviewed-By: João Reis <reis@janeasystems.com>
refack pushed a commit to refack/node that referenced this pull request Aug 23, 2017
this is a re-base of the gyp part of
6a09a69
after bumping GYP version to
https://chromium.googlesource.com/external/gyp/+/eb296f67da078ec01f5e3a9ea9cdc6d26d680161

Original-PR-URL: nodejs#11956
Original-Ref: nodejs#9163
Original-Reviewed-By: James M Snell <jasnell@gmail.com>

PR-URL: nodejs#12450
Reviewed-By: João Reis <reis@janeasystems.com>
addaleax pushed a commit to addaleax/ayo that referenced this pull request Aug 25, 2017
this is a re-base of the gyp part of
3c46bb9
after bumping GYP version to
https://chromium.googlesource.com/external/gyp/+/eb296f67da078ec01f5e3a9ea9cdc6d26d680161

Original-Review-By: James M Snell <jasnell@gmail.com>
Ref: nodejs/node#7986
PR-URL: nodejs/node#12450
Reviewed-By: João Reis <reis@janeasystems.com>
addaleax pushed a commit to addaleax/ayo that referenced this pull request Aug 25, 2017
this is a re-base of the gyp part of
6a09a69
after bumping GYP version to
https://chromium.googlesource.com/external/gyp/+/eb296f67da078ec01f5e3a9ea9cdc6d26d680161

Original-PR-URL: nodejs/node#11956
Original-Ref: nodejs/node#9163
Original-Reviewed-By: James M Snell <jasnell@gmail.com>

PR-URL: nodejs/node#12450
Reviewed-By: João Reis <reis@janeasystems.com>
addaleax pushed a commit to ayojs/ayo that referenced this pull request Aug 28, 2017
this is a re-base of the gyp part of
3c46bb9
after bumping GYP version to
https://chromium.googlesource.com/external/gyp/+/eb296f67da078ec01f5e3a9ea9cdc6d26d680161

Original-Review-By: James M Snell <jasnell@gmail.com>
Ref: nodejs/node#7986
PR-URL: nodejs/node#12450
Reviewed-By: João Reis <reis@janeasystems.com>
addaleax pushed a commit to ayojs/ayo that referenced this pull request Aug 28, 2017
this is a re-base of the gyp part of
6a09a69
after bumping GYP version to
https://chromium.googlesource.com/external/gyp/+/eb296f67da078ec01f5e3a9ea9cdc6d26d680161

Original-PR-URL: nodejs/node#11956
Original-Ref: nodejs/node#9163
Original-Reviewed-By: James M Snell <jasnell@gmail.com>

PR-URL: nodejs/node#12450
Reviewed-By: João Reis <reis@janeasystems.com>
MylesBorins pushed a commit that referenced this pull request Sep 10, 2017
this is a re-base of the gyp part of
3c46bb9
after bumping GYP version to
https://chromium.googlesource.com/external/gyp/+/eb296f67da078ec01f5e3a9ea9cdc6d26d680161

Original-Review-By: James M Snell <jasnell@gmail.com>
Ref: #7986
PR-URL: #12450
Reviewed-By: João Reis <reis@janeasystems.com>
MylesBorins pushed a commit that referenced this pull request Sep 10, 2017
this is a re-base of the gyp part of
6a09a69
after bumping GYP version to
https://chromium.googlesource.com/external/gyp/+/eb296f67da078ec01f5e3a9ea9cdc6d26d680161

Original-PR-URL: #11956
Original-Ref: #9163
Original-Reviewed-By: James M Snell <jasnell@gmail.com>

PR-URL: #12450
Reviewed-By: João Reis <reis@janeasystems.com>
@MylesBorins MylesBorins mentioned this pull request Sep 10, 2017
MylesBorins pushed a commit that referenced this pull request Sep 12, 2017
this is a re-base of the gyp part of
3c46bb9
after bumping GYP version to
https://chromium.googlesource.com/external/gyp/+/eb296f67da078ec01f5e3a9ea9cdc6d26d680161

Original-Review-By: James M Snell <jasnell@gmail.com>
Ref: #7986
PR-URL: #12450
Reviewed-By: João Reis <reis@janeasystems.com>
MylesBorins pushed a commit that referenced this pull request Sep 12, 2017
this is a re-base of the gyp part of
6a09a69
after bumping GYP version to
https://chromium.googlesource.com/external/gyp/+/eb296f67da078ec01f5e3a9ea9cdc6d26d680161

Original-PR-URL: #11956
Original-Ref: #9163
Original-Reviewed-By: James M Snell <jasnell@gmail.com>

PR-URL: #12450
Reviewed-By: João Reis <reis@janeasystems.com>
@MylesBorins
Copy link
Contributor

ping re: v6.x backport

rvagg pushed a commit to nodejs/node-gyp that referenced this pull request Aug 9, 2018
this is a re-base of the gyp part of
3c46bb9931ecea71167342322e09121ee48cde8e
after bumping GYP version to
https://chromium.googlesource.com/external/gyp/+/eb296f67da078ec01f5e3a9ea9cdc6d26d680161

Original-Review-By: James M Snell <jasnell@gmail.com>
Ref: nodejs/node#7986
PR-URL: nodejs/node#12450
Reviewed-By: João Reis <reis@janeasystems.com>
rvagg pushed a commit to nodejs/node-gyp that referenced this pull request Aug 9, 2018
this is a re-base of the gyp part of
6a09a69ec9d36b705e9bde2ac1a193566a702d96
after bumping GYP version to
https://chromium.googlesource.com/external/gyp/+/eb296f67da078ec01f5e3a9ea9cdc6d26d680161

Original-PR-URL: nodejs/node#11956
Original-Ref: nodejs/node#9163
Original-Reviewed-By: James M Snell <jasnell@gmail.com>

PR-URL: nodejs/node#12450
Reviewed-By: João Reis <reis@janeasystems.com>
rvagg pushed a commit to nodejs/node-gyp that referenced this pull request Aug 9, 2018
this is a re-base of the gyp part of
3c46bb9931ecea71167342322e09121ee48cde8e
after bumping GYP version to
https://chromium.googlesource.com/external/gyp/+/eb296f67da078ec01f5e3a9ea9cdc6d26d680161

Original-Review-By: James M Snell <jasnell@gmail.com>
Ref: nodejs/node#7986
PR-URL: nodejs/node#12450
Reviewed-By: João Reis <reis@janeasystems.com>
rvagg pushed a commit to nodejs/node-gyp that referenced this pull request Aug 9, 2018
this is a re-base of the gyp part of
6a09a69ec9d36b705e9bde2ac1a193566a702d96
after bumping GYP version to
https://chromium.googlesource.com/external/gyp/+/eb296f67da078ec01f5e3a9ea9cdc6d26d680161

Original-PR-URL: nodejs/node#11956
Original-Ref: nodejs/node#9163
Original-Reviewed-By: James M Snell <jasnell@gmail.com>

PR-URL: nodejs/node#12450
Reviewed-By: João Reis <reis@janeasystems.com>
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. tools Issues and PRs related to the tools directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants