Closed Bug 1392312 Opened 7 years ago Closed 7 years ago

Fix generated sources FILE remapping on Windows

Categories

(Firefox Build System :: General, enhancement)

Product:
Firefox Build System
Component:
General
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

(firefox57 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla57
Tracking Flags:
Tracking Status
firefox57 --- fixed

People

(Reporter: ted, Assigned: ted)

References

Details

Attachments

(1 file)

bug 1392312 - fix filename mapping in symbolstore.py on Windows.
(deleted), text/x-review-board-request
chmanchester
: review+
Details
bug 1259832 made a bunch of changes, one of which was to make symbolstore.py rewrite FILE entries for generated source files so that they point to the s3 bucket where we're uploading those sources. Unfortunately this doesn't seem to be working for Windows builds. My best guess is that we're doing literal pathname matching and the backslashes are wrong on Windows. I'll test this out on a local Windows build and see. Also it would be great to have this covered by the system test in unit-symbolstore.py, but I couldn't manage that previously because it only dumps symbols from the firefox binary, which doesn't pull in any generated source files that wind up in the debug info.
The problem here seems to be primarily that the filenames I'm storing in the `generated_files` dict in symbolstore.py have forward slashes on Windows, but the filenames that come out of dump_syms have backslashes. A related problem is that apparently `FixFilenameCase`, which symbolstore.py calls to normalize the case of source filenames (because the compiler frequently outputs them in all lowercase), returns filenames with an uppercase drive letter, and the filenames in generated_files have a lowercase drive letter, so they don't match. I'm also noticing that files in dist/include, which are supposed to be mapped back to the original file in the source directory by way of the install manifest, seem to not be doing that. This works on non-Windows platforms, so I think the change to use `GetFinalPathNameByHandleW` in `FixFilenameCase` in bug 1156300 broke that.
Naturally my new test failed on everything-but-MSVC because nsCOMPtr.h doesn't wind up getting included there. I'll change it to a header that works everywhere.
This test is still failing on the TC Windows builds, but it looks like it's legitimately failing, so that's good to know! Downloading the symbols from one of the builds shows (in firefox.sym): FILE 61 hg:hg.mozilla.org/try:obj-firefox/dist/include/mozilla/Assertions.h:4f7a163e1276 ...so the remapping of header files via install manifest is still not working. Additionally the generated files remapping is also not working here: FILE 378652 hg:hg.mozilla.org/try:obj-firefox/toolkit/library/i686-pc-windows-msvc/release/build/gleam-b165e2a9f360a23c/out/gles_bindings.rs:4f7a163e1276
I figured it out: `FixFilenameCase` is calling `GetFinalPathNameByHandleW`, which as it turns out resolves symlinks. The Taskcluster Windows builds build inside a path containing a symlink (which is a thing *I* implemented) so that sccache has consistent paths to get cache hits. The code wasn't running all paths through `FixFilenameCase`, though, so path comparisons were failing. I've got a revised patch that merges `FixFilenameCase` into `normpath` and makes sure to call it everywhere we handle paths that get compared against source file paths. I tested it out with symlinks locally and it seems to do the right thing now.
I can't push to try at the moment because treestatus is broken, but the tests pass on my Windows machine and on my Linux machine, and I sanity checked that dumping symbols from a PDB file from a taskcluster build on my local machine with matching symlinks setup results in a symbol file with correctly-formatted file names. I'll push Windows builds on try as a final test when that's unbroken.
Comment on attachment 8900352 [details] bug 1392312 - fix filename mapping in symbolstore.py on Windows. https://reviewboard.mozilla.org/r/171710/#review177766
Attachment #8900352 - Flags: review?(cmanchester) → review+
Pushed by tmielczarek@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/7bd4ff9f3108 fix filename mapping in symbolstore.py on Windows. r=chmanchester
https://hg.mozilla.org/mozilla-central/rev/7bd4ff9f3108
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox57: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Depends on: 1405713
Product: Core → Firefox Build System
Assignee: nobody → ted
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: