Skip to content

Conversation

@addaleax
Copy link
Member

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

tools/gyp

Description of change

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: #7959
Ref: #7510

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
@addaleaxaddaleax added build Issues and PRs related to build files or the CI. tools Issues and PRs related to the tools directory. labels Aug 3, 2016
@addaleax
Copy link
MemberAuthor

@thefourtheye
Copy link
Contributor

LGTM


# Hash the target name to avoid generating overlong filenames.
cmddigest=_new_sha1(commandifcommandelseself.target).hexdigest()
intermediate="%s.intermediate"% (cmddigest)
Copy link
Member

Choose a reason for hiding this comment

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

Nitpick but the parens aren't necessary.

@bnoordhuis
Copy link
Member

LGTM. Just curious, why did you name the variable _new_sha1 instead of e.g. sha1?

@addaleax
Copy link
MemberAuthor

That was copied from xcodeproj_file.py, too; I’m changing it, just sha1 sounds good to me too.

@thefourtheye
Copy link
Contributor

thefourtheye commented Aug 3, 2016

Actually, leading _ is correct, according to PEP-8. Quoting it,

Even with __all__ set appropriately, internal interfaces (packages, modules, classes, functions, attributes or other names) should still be prefixed with a single leading underscore.

@addaleax
Copy link
MemberAuthor

Sounds fine to me, I’ve left it and just dropped the extra parentheses.

@jasnell
Copy link
Member

LGTM

# available, avoiding a deprecation warning under 2.6. Import sha otherwise,
# preserving 2.4 compatibility.
try:
importhashlib
Copy link
Member

Choose a reason for hiding this comment

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

since GYP is kinda abandoned and you're floating the patch, you can remove all that and just import hashlib. Python 2.4 predates YouTube, keeping compatibility with that is insane these days.

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

@saghul Done! :)

@saghul
Copy link
Member

LGTM! 👍

@jasnell
Copy link
Member

There was red in the previous CI run that looks like a build bot failure. Trying again just to be safe... CI: https://ci.nodejs.org/job/node-test-pull-request/3543/

@addaleax
Copy link
MemberAuthor

@addaleax
Copy link
MemberAuthor

addaleax commented Aug 8, 2016

Landed in 00dc41d0a033f5ea38953c73cf2fe23f4a8ce5775111e78 … I can finally build master without flags again! 🎉 😄

@addaleaxaddaleax closed this Aug 8, 2016
@addaleaxaddaleax deleted the gyp-filename-float branch August 8, 2016 20:48
addaleax added a commit that referenced this pull request Aug 8, 2016
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: #7959 Ref: #7510 PR-URL: #7963 Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Saúl Ibarra Corretgé <[email protected]>
@cjihrigcjihrig mentioned this pull request Aug 10, 2016
cjihrig pushed a commit that referenced this pull request Aug 10, 2016
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: #7959 Ref: #7510 PR-URL: #7963 Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Saúl Ibarra Corretgé <[email protected]>
@cjihrigcjihrig mentioned this pull request Aug 11, 2016
MylesBorins pushed a commit that referenced this pull request Sep 30, 2016
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: #7959 Ref: #7510 PR-URL: #7963 Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Saúl Ibarra Corretgé <[email protected]>
@MylesBorins
Copy link
Contributor

@addaleax I've backported this to v4.x

please let me know if it shouldn't have been

@bnoordhuis
Copy link
Member

@addaleax I just noticed, you never assigned a reviewer in https://codereview.chromium.org/2019133002, that's probably why it's still open. I'd try one of bradnelson@, mark@ or thakis@.

bnoordhuis pushed a commit to bnoordhuis/io.js that referenced this pull request Oct 8, 2016
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 <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Saúl Ibarra Corretgé <[email protected]>
rvagg pushed a commit that referenced this pull request Oct 18, 2016
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: #7959 Ref: #7510 PR-URL: #7963 Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Saúl Ibarra Corretgé <[email protected]>
MylesBorins pushed a commit that referenced this pull request Oct 26, 2016
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: #7959 Ref: #7510 PR-URL: #7963 Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Saúl Ibarra Corretgé <[email protected]>
@MylesBorinsMylesBorins mentioned this pull request Oct 26, 2016
rvagg pushed a commit to nodejs/node-gyp that referenced this pull request Aug 9, 2018
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/node#7959 Ref: nodejs/node#7510 PR-URL: nodejs/node#7963 Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Saúl Ibarra Corretgé <[email protected]>
rvagg pushed a commit to nodejs/node-gyp that referenced this pull request Aug 9, 2018
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/node#7959 Ref: nodejs/node#7510 PR-URL: nodejs/node#7963 Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Saúl Ibarra Corretgé <[email protected]>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

buildIssues and PRs related to build files or the CI.toolsIssues and PRs related to the tools directory.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@addaleax@thefourtheye@bnoordhuis@jasnell@saghul@MylesBorins