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

deps: float gyp patch for long filenames #7963

Closed
wants to merge 3 commits into from

Conversation

addaleax
Copy link
Member

@addaleax addaleax commented Aug 3, 2016

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
@addaleax addaleax 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
Member Author

addaleax commented Aug 3, 2016

@thefourtheye
Copy link
Contributor

LGTM


# Hash the target name to avoid generating overlong filenames.
cmddigest = _new_sha1(command if command else self.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
Member Author

addaleax commented Aug 3, 2016

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
Member Author

addaleax commented Aug 3, 2016

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

@jasnell
Copy link
Member

jasnell commented Aug 3, 2016

LGTM

# available, avoiding a deprecation warning under 2.6. Import sha otherwise,
# preserving 2.4 compatibility.
try:
import hashlib
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
Member Author

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

saghul commented Aug 4, 2016

LGTM! 👍

@jasnell
Copy link
Member

jasnell commented Aug 5, 2016

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
Member Author

addaleax commented Aug 8, 2016

@addaleax
Copy link
Member Author

addaleax commented Aug 8, 2016

Landed in 00dc41d0a033f5ea38953c73cf2fe23f4a8ce577 5111e78 … I can finally build master without flags again! 🎉 😄

@addaleax addaleax closed this Aug 8, 2016
@addaleax addaleax 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 <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>
@cjihrig cjihrig 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 <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>
@cjihrig cjihrig 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 <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>
@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 <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>
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 <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>
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 <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>
@MylesBorins MylesBorins 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 <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>
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 <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>
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.

6 participants