reader.py can exit with an error that a mozconfig does not sit inside an allowed path, but does not report the location of the mozconfig that is deemed to sit outside the allowed path, and what the allowed path was. Ideally when an error occurs due to an assertion failing, the data that was used to check the assertion should be included with the error message, thus making it possible to see why the assertion failed, not only that it failed. In the recent case that impacted the windows taskcluster builds, it was due to the lexical check not considering that c:\..... could be a subdirectory of C:\..... due to the c: vs C:. This has been resolved separately by changing paths used in command arguments and environment variables, but the improved error message would have been useful and saved time. Soft block only on 1244750 as it would be nice to land it at the same time, but not absolutely required.
Created attachment 8761559 [details] [diff] [review] bug1279027_gecko_v1.patch
Assignee: nobody → pmoore
Attachment #8761559 - Flags: review?(mshal)
Note, this can probably be done more nicely. Suggestions welcome! :-)
Comment on attachment 8761559 [details] [diff] [review] bug1279027_gecko_v1.patch This seems to split the error message into two different locations. We already get the path of the offending file in a message like: """ The error occurred while processing the following file: /home/mshal/mozilla-central-git/moz.build The underlying problem is an illegal file access. This is likely due to trying to access a file outside of the top source directory. The path whose access was denied is: /home/mshal/foo.mozbuild """ I think it might make sense to add what the topsrcdir is to this message, since that is I believe what was missing to make the c:\ vs. C:\ error more apparent. Maybe this could be: The underlying problem is an illegal file access. This is likely due to trying to access a file outside of the top source directory (c:\worker\blah)
Attachment #8761559 - Flags: review?(mshal) → feedback+
Thanks Mike - I'll do it like that. Will attach another patch. :-)
OK, I've had a look, but I don't see an easy way to access the top source directory at that point in the code... Error message is here: https://dxr.mozilla.org/mozilla-central/rev/249d57fa78a0522f63a5b1ed1fdda55f383156e0/python/mozbuild/mozbuild/frontend/reader.py#705 Exception is raised here: https://dxr.mozilla.org/mozilla-central/rev/249d57fa78a0522f63a5b1ed1fdda55f383156e0/python/mozbuild/mozbuild/frontend/reader.py#1066 Somehow the topsrcdir needs to be passed through into the exception, but I couldn't get my head around the exception classes.
(In reply to Pete Moore [:pmoore][:pete] from comment #5) > Exception is raised here: > https://dxr.mozilla.org/mozilla-central/rev/ > 249d57fa78a0522f63a5b1ed1fdda55f383156e0/python/mozbuild/mozbuild/frontend/ > reader.py#1066 Well really it is raised here: https://dxr.mozilla.org/mozilla-central/rev/249d57fa78a0522f63a5b1ed1fdda55f383156e0/python/mozbuild/mozbuild/frontend/reader.py#1207 The previous link is where it is caught and passed up the stack.
(In reply to Pete Moore [:pmoore][:pete] from comment #0) > Soft block only on 1244750 as it would be nice to land it at the same time, > but not absolutely required. Removing the soft block, as it makes sense to land without it.
No longer blocks: 1244750
You need to log in before you can comment on or make changes to this bug.