Open Bug 1417180 Opened 7 years ago Updated 2 years ago

improve error message for bad path in jar.mn

Categories

(Firefox Build System :: General, defect)

Product:
Firefox Build System
Component:
General
Type:
defect

Tracking

(firefox59 affected)

Tracking Flags:
Tracking Status
firefox59 --- affected

People

(Reporter: dbaron, Unassigned)

References

Details

Attachments

(1 file)

Bug 1417180 - Improve error messages for not-found paths in jar.mn files.
59 bytes, text/x-review-board-request
Details
The error message in bug 1416968 was really lacking in detail about what needed to be fixed. I have a patch to improve the error message so that it gives enough information to fix the problem.
This patch will change the line: > 0:25.08 ValueError: Object directory paths are not allowed from bug 1416968 comment 0 to be: > 0:12.88 Exception: /home/dbaron/builds/mozilla-central/mozilla/browser/branding/official/content/jar.mn: Object directory paths are not allowed: !../mozicon128.png while preserving the traceback prior to it. It uses one of the techniques in http://www.ianbicking.org/blog/2007/09/re-raising-exceptions.html although it's not clear to me whether it's the best one. (It's more robust against string exceptions, but changes the exception type from ValueError to Exception.)
Attachment #8928264 - Flags: review?(mh+mozilla) → review?(core-build-config-reviews)
Attachment #8928264 - Flags: review?(core-build-config-reviews) → review?(gps)
Comment on attachment 8928264 [details] Bug 1417180 - Improve error messages for not-found paths in jar.mn files. https://reviewboard.mozilla.org/r/199482/#review204984 There are a few problems with the Python: 1. bareword `except` is considered bad because it traps `KeyboardInterrupt` and `SystemExit`, which are exceptions raised after SIGINT and process termination, respectively. If you want to trap all non-special exceptions, you use `except Exception`. 2. Re-raising exceptions in Python while preserving the stack is kinda wonky. The blog post you cited is old and doesn't reflect the modern way of doing things. In Python 3, there is a built-in mechanism for doing this. In Python 2, it is... not great. But, we shouldn't need to involve any weird exception handling here. I would copy what is done on line 492 and add the current object's path to the exception message.
Attachment #8928264 - Flags: review?(gps) → review-
(In reply to Gregory Szorc [:gps] from comment #4) > But, we shouldn't need to involve any weird exception handling here. I would > copy what is done on line 492 and add the current object's path to the > exception message. So, from the dupe, the current work on line 492 is not good enough either - it also gives just "jar.mn" and doesn't list the entire path for the target file either. Given that the relative path in the string is not necessarily relative to the jar.mn file due to 'relativesrcdir' directives elsewhere in the jar.mn, IMHO we should print the 'full' path for both files (ie the jar.mn file and the "problematic"/missing/inaccessible file it references.
(In reply to Gregory Szorc [:gps] from comment #4) > But, we shouldn't need to involve any weird exception handling here. I would > copy what is done on line 492 and add the current object's path to the > exception message. I guess I'm not sure how you're suggesting to do this. add to the message how? The exception here is coming from the code in frontend/context.py
Flags: needinfo?(gps)
(Note that the original report here and the dupe were about different exceptions... and I failed to properly fix up the one that the dupe was about because I didn't notice it.)
Er, I didn't mean to re-request review, but I guess mozreview thinks it's supposed to do that if I repost the patch to address the comments that I knew how to address.
Has this landed? I get the following error one one of my PCs where I develop TB patches, and not sure how to address it. File "/NREF-COMM-CENTRAL/comm-central/mozilla/python/mozbuild/mozbuild/frontend/context.py", line 655, in __init__ raise ValueError('Object directory paths are not allowed') Using google, I searched for "ValueError: Object directory paths are not allowed" and found this bugzilla entry. After reading this, I suspect the error has something to do with incorrect path. This started to happen after I updated my source tree in mid-December which I had not done since later summer. So the suspicion that the change in the last two months or so or earlier was the culprit (in bug 1416968 ) rings a bell. But maybe now come to think of it, in this case, the file path in question may be TB-specific TIA PS: I suppose not many people enable official branding in local build.
Probably for the TB build, the strace revealed this: 12326 stat("/NREF-COMM-CENTRAL/comm-central/other-licenses/branding/thunderbird/content/about-background.png", {st_mode=S_IFREG|0644, st_size=149074, ...}) = 0 12326 stat("/NREF-COMM-CENTRAL/comm-central/other-licenses/branding/thunderbird/content/about-logo.png", {st_mode=S_IFREG|0644, st_size=59982, ...}) = 0 12326 stat("/NREF-COMM-CENTRAL/comm-central/other-licenses/branding/thunderbird/content/about-wordmark.png", {st_mode=S_IFREG|0644, st_size=3586, ...}) = 0 12326 stat("/NREF-COMM-CENTRAL/comm-central/other-licenses/branding/thunderbird/content/about.png", {st_mode=S_IFREG|0644, st_size=57687, ...}) = 0 12326 stat("/NREF-COMM-CENTRAL/comm-central/other-licenses/branding/thunderbird/mailicon48.png", 0x7fff67a4d1a0) = -1 ENOENT (No such file or directory) 12326 write(2, "Traceback (most recent call last"..., 35) = 35 12251 <... read resumed> "T", 1) = 1 12326 write(2, " File \"/NREF-COMM-CENTRAL/comm-"..., 76 <unfinished ...> So I suspect that mailcon48.png no longer exists, but it is being sought. That is one step ahead. But looking through strace record, I found something horrendous. Either python interpreter is broken or the way it is invoked under mozilla build scheme is totally hosed. No wonder it seems slow. It seems to read the interpreted source file ONE character at a time? (Oh wait a second. Is it only during the backtrace stage when it needs to scan the source file after an exception occurs, since I presume that it reads the byte-compiled code. ) I notice the following snippet near the error occurred. Maybe it is reading the byte code file. In any case, very inefficient read operation. It is near the error occurrence. ... 12251 read(4, "b", 1) = 1 12251 read(4, "u", 1) = 1 12251 read(4, "i", 1) = 1 12251 read(4, "l", 1) = 1 12251 read(4, "d", 1) = 1 12251 read(4, "/", 1) = 1 12251 read(4, "f", 1) = 1 12251 read(4, "r", 1) = 1 12251 read(4, "o", 1) = 1 12251 read(4, "n", 1) = 1 12251 read(4, "t", 1) = 1 12251 read(4, "e", 1) = 1 12251 read(4, "n", 1) = 1 12251 read(4, "d", 1) = 1 12251 read(4, "/", 1) = 1 12251 read(4, "c", 1) = 1 12251 read(4, "o", 1) = 1 12251 read(4, "n", 1) = 1 12251 read(4, "t", 1) = 1 12251 read(4, "e", 1) = 1 12251 read(4, "x", 1) = 1 12251 read(4, "t", 1) = 1 12251 read(4, ".", 1) = 1 12251 read(4, "p", 1) = 1 12251 read(4, "y", 1) = 1 12251 read(4, "\"", 1) = 1 12251 read(4, ",", 1) = 1 12251 read(4, " ", 1) = 1 12251 read(4, "l", 1) = 1 12251 read(4, "i", 1) = 1 12251 read(4, "n", 1) = 1 12251 read(4, "e", 1) = 1 12251 read(4, " ", 1) = 1 12251 read(4, "6", 1) = 1 12251 read(4, "5", 1) = 1 12251 read(4, "5", 1) = 1 12251 read(4, ",", 1) = 1 12251 read(4, " ", 1) = 1 12251 read(4, "i", 1) = 1 12251 read(4, "n", 1) = 1 12251 read(4, " ", 1) = 1 12251 read(4, "_", 1) = 1 12251 read(4, "_", 1) = 1 12251 read(4, "i", 1) = 1 12251 read(4, "n", 1) = 1 12251 read(4, "i", 1) = 1 12251 read(4, "t", 1) = 1 12251 read(4, "_", 1) = 1 12251 read(4, "_", 1) = 1 12251 read(4, "\n", 1) = 1 12251 read(4, " ", 1) = 1 12251 read(4, " ", 1) = 1 12251 read(4, " ", 1) = 1 12251 read(4, " ", 1) = 1 12251 read(4, "r", 1) = 1 12251 read(4, "a", 1) = 1 12251 read(4, "i", 1) = 1 12251 read(4, "s", 1) = 1 12251 read(4, "e", 1) = 1 12251 read(4, " ", 1) = 1 12251 read(4, "V", 1) = 1 12251 read(4, "a", 1) = 1 12251 read(4, "l", 1) = 1 12251 read(4, "u", 1) = 1 12251 read(4, "e", 1) = 1 12251 read(4, "E", 1) = 1 12251 read(4, "r", 1) = 1 12251 read(4, "r", 1) = 1 12251 read(4, "o", 1) = 1 12251 read(4, "r", 1) = 1 12251 read(4, "(", 1) = 1 12251 read(4, "'", 1) = 1 12251 read(4, "O", 1) = 1 12251 read(4, "b", 1) = 1 12251 read(4, "j", 1) = 1 12251 read(4, "e", 1) = 1 12251 read(4, "c", 1) = 1 12251 read(4, "t", 1) = 1 12251 read(4, " ", 1) = 1 12251 read(4, "d", 1) = 1 12251 read(4, "i", 1) = 1 12251 read(4, "r", 1) = 1 12251 read(4, "e", 1) = 1 12251 read(4, "c", 1) = 1 12251 read(4, "t", 1) = 1 12251 read(4, "o", 1) = 1 12251 read(4, "r", 1) = 1 12251 read(4, "y", 1) = 1 12251 read(4, " ", 1) = 1 12251 read(4, "p", 1) = 1 12251 read(4, "a", 1) = 1 12251 read(4, "t", 1) = 1 12251 read(4, "h", 1) = 1 12251 read(4, "s", 1) = 1 12251 read(4, " ", 1) = 1 12251 read(4, "a", 1) = 1 12251 read(4, "r", 1) = 1 12251 read(4, "e", 1) = 1 12251 read(4, " ", 1) = 1 12251 read(4, "n", 1) = 1 12251 read(4, "o", 1) = 1 12251 read(4, "t", 1) = 1 12252 <... select resumed> ) = 0 (Timeout) 12252 getpid() = 12213 12252 write(1, " File \"/NREF-COMM-CENTRAL/comm-"..., 117) = 117 12252 write(3, "[1516786616.506465, \"build_outpu"..., 168) = 168 12252 select(0, NULL, NULL, NULL, {tv_sec=0, tv_usec=1000} <unfinished ...> 12251 read(4, " ", 1) = 1 12251 read(4, "a", 1) = 1 12251 read(4, "l", 1) = 1 12251 read(4, "l", 1) = 1 12251 read(4, "o", 1) = 1 12251 read(4, "w", 1) = 1 12251 read(4, "e", 1) = 1 12251 read(4, "d", 1) = 1 12251 read(4, "'", 1) = 1 12251 read(4, ")", 1) = 1 12251 read(4, "\n", 1) = 1 12251 read(4, "V", 1) = 1 12251 read(4, "a", 1) = 1 12251 read(4, "l", 1) = 1 12251 read(4, "u", 1) = 1 12251 read(4, "e", 1) = 1 12251 read(4, "E", 1) = 1 12251 read(4, "r", 1) = 1 12251 read(4, "r", 1) = 1 12251 read(4, "o", 1) = 1 12251 read(4, "r", 1) = 1 12251 read(4, ":", 1) = 1 12251 read(4, " ", 1) = 1 12251 read(4, "O", 1) = 1 12251 read(4, "b", 1) = 1 12251 read(4, "j", 1) = 1 12251 read(4, "e", 1) = 1 12251 read(4, "c", 1) = 1 12251 read(4, "t", 1) = 1 12251 read(4, " ", 1) = 1 12251 read(4, "d", 1) = 1 12251 read(4, "i", 1) = 1 12251 read(4, "r", 1) = 1 12251 read(4, "e", 1) = 1 12251 read(4, "c", 1) = 1 12251 read(4, "t", 1) = 1 12251 read(4, "o", 1) = 1 12251 read(4, "r", 1) = 1 12251 read(4, "y", 1) = 1 12251 read(4, " ", 1) = 1 12251 read(4, "p", 1) = 1 12251 read(4, "a", 1) = 1 12251 read(4, "t", 1) = 1 12251 read(4, "h", 1) = 1 12251 read(4, "s", 1) = 1 12251 read(4, " ", 1) = 1 12251 read(4, "a", 1) = 1 12251 read(4, "r", 1) = 1 12251 read(4, "e", 1) = 1 12251 read(4, " ", 1) = 1 12251 read(4, "n", 1) = 1 12251 read(4, "o", 1) = 1 12251 read(4, "t", 1) = 1 12251 read(4, " ", 1) = 1 12251 read(4, "a", 1) = 1 12251 read(4, "l", 1) = 1 12251 read(4, "l", 1) = 1 12251 read(4, "o", 1) = 1 12251 read(4, "w", 1) = 1 12251 read(4, "e", 1) = 1 12251 read(4, "d", 1) = 1 12251 read(4, "\n", 1) = 1 12251 read(4, <unfinished ...> 12252 <... select resumed> ) = 0 (Timeout) 12252 getpid() = 12213 12252 write(1, " raise ValueError('Object dir"..., 63) = 63 12252 write(3, "[1516786616.507944, \"build_outpu"..., 112) = 112 12252 getpid() = 12213 12252 write(1, "ValueError: Object directory pat"..., 51) = 51 12252 write(3, "[1516786616.508138, \"build_outpu"..., 100) = 100 This was simply captured by strace -f -o /tmp/t.out bash my-script-to-invoke-mach TIA
See Also: → 1432903
Product: Core → Firefox Build System
Assignee: dbaron → nobody

Clear a needinfo that is pending on an inactive user.

For more information, please visit auto_nag documentation.

Flags: needinfo?(gps)
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: