1901592 - Cannot build clang-plugin when home directory has a space in the name

Status: RESOLVED FIXED

(Developer Infrastructure :: Source Code Analysis, defect)

Description

[Ray Kraesig [:rkraesig]]

As of bug 1849075, building the clang-plugin under Windows (with MozillaBuild 4.1) fails.

The specific failure occurs here: the first two flags are a broken reference to a directory with a space in it, and should be a single flag.

The offending variable HOST_CXXFLAGS

• is derived from COMPUTED_HOST_CXXFLAGS, which
• is derived from HOST_COMPILE_FLAGS["HOST_CXXFLAGS"], which
• is derived from CONFIG["LLVM_CXXFLAGS"], which
• is derived from invoking llvm-config.

Testing shows that, with clang 17.0.6, llvm-config used the path embedded in $0 directly, whereas with clang 18.1.5, it performs some sort of normalization on the path. Since we use the short name (8.3) of the home directory to invoke llvm-config in the first place, that means it used to spit out a (spaceless) short path, but no longer does.

This is properly an upstream bug (https://github.com/llvm/llvm-project/issues/28117); but since nobody's touched that in the last eight years, we'll probably want a local workaround.

Comment 1

[Ray Kraesig [:rkraesig]]

Attachment: Bug 1901592 - [1/1] patch m4 file to fix up broken flagsets on Windows r?glandium!

llvm-config doesn't quote paths with spaces -- and unfortunately, as of
the current version used by mozilla-central, expands spaceless short-names
to potentially space-bearing long-names.

Since we install clang into the user's home directory, as a workaround,
string-replace the long name of the directory with the short name for
flagsets obtained from llvm-config on Windows.

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1849075

Comment 3

[Mike Hommey [:glandium]]

I think I'd rather kill clang-plugin for local builds altogether rather than try to fix this. I guess your use case is to reproduce errors it catches on CI, but mach static-analysis should give you that already. If it doesn't, please file bugs.

Comment 4

[Ray Kraesig [:rkraesig]]

I guess your use case is to reproduce errors it catches on CI,

No — my use case is to catch errors before they are raised in CI. The try server has a very long round-trip time, even when it's functioning perfectly, and I don't enjoy spending hours waiting on it to learn about and fix something that could and should have been caught locally on the first compilation attempt.

It's also necessary to be able to test changes to the plugin locally; those don't happen often, but they do happen (e.g. bug 1791682).

but mach static-analysis should give you that already.

mach static-analysis check takes far longer than compiling and using the plugin [0], generates oodles of spam which drown out any real errors, and can't be interrupted with Ctrl+C once it starts analyzing files (!!!). I expect the amount of work and time that would be necessary to get it to an equivalently-usable state would be better spent on fixing llvm-config to escape and/or quote paths on Windows.

It's also not done as part of compilation, which means it's easily forgotten about; conversely, the clang-plugin is build and used as part of mach build, making it difficult to forget once it's in one's mozconfig.

---

[0] >30m just for mach static-analysis check widget/windows, compared to ~17m full mach build from a post-clobber state.

Comment 5

[Mike Hommey [:glandium]]

Attachment: Bug 1901592 - Revert a LLVM change that affects how llvm-config outputs paths.

Before trying to address the issue in depth (possibly backporting an
upcoming llvm-config change), let's just revert to something that is
known to work for the moment.

Comment 6

[Mike Hommey [:glandium]]

(In reply to Ray Kraesig [:rkraesig] from comment #4)

but mach static-analysis should give you that already.

mach static-analysis check takes far longer than compiling and using the plugin [0], generates oodles of spam which drown out any real errors, and can't be interrupted with Ctrl+C once it starts analyzing files (!!!). I expect the amount of work and time that would be necessary to get it to an equivalently-usable state would be better spent on fixing llvm-config to escape and/or quote paths on Windows.

It's also not done as part of compilation, which means it's easily forgotten about; conversely, the clang-plugin is build and used as part of mach build, making it difficult to forget once it's in one's mozconfig.

---

[0] >30m just for mach static-analysis check widget/windows, compared to ~17m full mach build from a post-clobber state.

Please file bugs.

Comment 7

[Ray Kraesig [:rkraesig]]

Done: bug 1902441, bug 1902442, bug 1902443.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9b89f27988a5
Revert a LLVM change that affects how llvm-config outputs paths. r=firefox-build-system-reviewers,sergesanspaille

Comment 9

[Aron Cseh]

https://hg.mozilla.org/mozilla-central/rev/9b89f27988a5

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:rkraesig, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox128 to wontfix.

For more information, please visit BugBot documentation.