The current build was producing names like llvm-undname.exe.pdb, which looks unusual to me at least. This switches them to the more common llvm-undname.pdb style.
Details
- Reviewers
thakis - Commits
- rG1137fce9e9e5: gn build: Windows: use a more standard format for PDB filenames
rL353099: gn build: Windows: use a more standard format for PDB filenames
rG1469ff417bba: gn build: Windows: use a more standard format for PDB filenames
rL353094: gn build: Windows: use a more standard format for PDB filenames
Diff Detail
- Repository
- rL LLVM
Event Timeline
llvm/utils/gn/build/toolchain/BUILD.gn | ||
---|---|---|
249 ↗ | (On Diff #184815) | Does pdbfile need to be included in any of the above output(s) lines? (I don't know what they do.) |
Do you care much about this? The naming here might be somewhat unconventional, but it's more flexible (consider the completely hypothetical case of a build having both "chrome.exe" and "chorme.dll" – if you call the pdb for both "chrome.dll" they'll get in each other's way, but with this scheme it works fine) and ever so slightly less code. So I weakly prefer what we currently have, but I almost never do debug builds so I don't care all that much. I'll accept this so that you can land it if you want, but I'd weakly prefer if you didn't :-)
llvm/utils/gn/build/toolchain/BUILD.gn | ||
---|---|---|
249 ↗ | (On Diff #184815) | see llvm/utils/gn/gn.py help tool, section "outputs" :-P Ninja knows that the link rule produces the outputs listed here if depend_output isn't set. Since depend_output _is_ set, I'm not sure what's in outputs matters all that much. If depend_output _wasn't_ set and pdbfile was in this list, then a build step that lists the pdb file as an input without depending on the executable() or shared_library() target that produces it would still correctly run the link to produce the pdb before running that step. But such a step should have a dep on the executable() / shared_library() for cleanliness anyhow, and the pdb is only written in debug builds, which we don't easily know if that's the case here, so I just omitted it. |
I care about it mildly (a hair more than weakly :-P).
For better or worse this is the convention on Windows, including LLVM's cmake build and Microsoft's own published symbols, so this file doesn't seem like the right place to push for a change. If the cmake build changes its tune, I'm fine with you reverting this. :-)