Skip to content

Conversation

@bnoordhuis
Copy link
Member

@bnoordhuis bnoordhuis commented Oct 8, 2016

bnoordhuis and others added 4 commits October 8, 2016 10:30
This issue has already submitted to the upstream in
https://code.google.com/p/gyp/issues/detail?id=477
Use this commit until the upstream is to be fixed.

PR-URL: nodejs#1325
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Gyp defaults to gcc/g++ if CC.host/CXX.host is unset. This is not
suitable for environments that only uses the clang toolchain.

Since we already assume that the user will provide clang/clang++
through CC/CXX, lean against it (then drop to gcc/g++).

Also apply the same logic for link/ar for consistency although
it doesn't affect us.

PR-URL: nodejs#6173
Fixes: nodejs#6152
Reviewed-By: João Reis <reis@janeasystems.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Pulling in https://codereview.chromium.org/2019133002/ in its current
state, as gyp seems to be largely abandoned as a project.

Original commit message:

    Hash intermediate file name to avoid ENAMETOOLONG

    Hash the intermediate Makefile target used for multi-output rules
    so that it still works when the involved file names are very long.

    Since the intermediate file's name is effectively arbitrary, this
    does not come with notable behavioural changes.

    The `import hashlib` boilerplate is taken directly
    from `xcodeproj_file.py`.

Concretely, this makes the V8 inspector build currently fail when long
pathnames are involved, notably when using ecryptfs which has a lower
file name length limit.

Fixes: nodejs#7959
Ref: nodejs#7510
PR-URL: nodejs#7963
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Saúl Ibarra Corretgé <saghul@gmail.com>
@bnoordhuis bnoordhuis added the tools Issues and PRs related to the tools directory. label Oct 8, 2016
@bnoordhuis
Copy link
Member Author

Hm, maybe we need a more unified approach to maintaining gyp. nodejs/node and nodejs/node-gyp carry different patches that sometimes overlap, sometimes don't. Let me think about it for a bit.

@rvagg rvagg force-pushed the master branch 2 times, most recently from c133999 to 83c7a88 Compare October 18, 2016 17:02
@bnoordhuis bnoordhuis closed this Nov 19, 2016
@bnoordhuis bnoordhuis deleted the upgrade-gyp branch November 19, 2016 11:15
@kunalspathak
Copy link
Member

@bnoordhuis , any update on this? I believe we can't get in nodejs/node-gyp#873 without this. Am i right?

@bnoordhuis
Copy link
Member Author

@kunalspathak I'm afraid the answer is 'not really.' It's on my todo list but it's complicated by the fact that upstream gyp is pretty moribund. I think we'll end up forking or rewriting it but I haven't hashed out a real plan yet and I don't think anyone else has either.

@kunalspathak
Copy link
Member

I see. So nodejs/node-gyp#873 can only land once we fork/rewriting upstream gyp?

@bnoordhuis
Copy link
Member Author

I think we can land nodejs/node-gyp#873 for pragmatic reasons while we figure this out. Can you rebase it? I probably won't be able to look at it until later this week though.

@kunalspathak
Copy link
Member

Can you rebase it?

done.

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

Labels

tools Issues and PRs related to the tools directory.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants