536524 - pymake build fails with overlong paths

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

My pymake build failed, apparently unable to find crash_generation_client.cc in my source dir. It was trying to stat "c:/home/roc/objdirs/mozilla-central/ff-debug/toolkit/crashreporter/google-breakpad/src/client/windows/crash_generation/../../../../../../../../../../mozilla-central/toolkit/crashreporter/google-breakpad/src/client/windows/crash_generation/crash_generation_client.cc". That path happens to be 266 characters long, so naturally I suspected an overflow of a 256-byte limit. The attached patch to pymake replaces almost every call to os.path.join with a call to util.normaljoin, which tries calling os.path.normpath if the result of the join is over 255 characters. This patch fixes my build.

This is my first ever Python patch!

Comment 1

[(not currently active) Ted Mielczarek]

Just two comments:
1) The canonical Pymake repo is here:
http://hg.mozilla.org/users/bsmedberg_mozilla.com/pymake/
2) It'd be nice to have a unit test for this, you can see how they're written here:
http://hg.mozilla.org/users/bsmedberg_mozilla.com/pymake/file/a69cde204ca3/tests

Comment 2

[Benjamin Smedberg]

Comment on attachment 418980 [details] [diff] [review]
fix

I don't know what to think about this. The maxpathlen on Windows is 1024, not 256, so something along the way must be using a smaller limit. I've tried to think of ways this could break, and I can't think of any offhand... we don't hit this function during make string processing, only during filesystem operations, so it shouldn't affect pattern rules or anything like that.

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Can you commit the patch to the pymake repo? I assume I can't.

Comment 4

[(not currently active) Ted Mielczarek]

There are no additional access controls on user repos. If you can push to any hg repo, you can push to that repo.

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: simpler testcase

Here's a test. It actually fails with gmake, is that OK?

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I checked the patch into the pymake repo with changeset 789e323581cd.

Comment 8

[Benjamin Smedberg]

If it's pymake-only it should have #T gmake skip at the top, like http://mxr.mozilla.org/mozilla-central/source/build/pymake/tests/includedeps-norebuild.mk#1

Comment 9

[Benjamin Smedberg]

Comment on attachment 429888 [details] [diff] [review]
simpler testcase

r=me with the extra line to disable the test for gmake

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Pushed test to pymake repo as changeset 223ecd39ae28, with that line.

Checked fix into mozilla-central as
http://hg.mozilla.org/mozilla-central/rev/c80eb5209859