Closed Bug 1901592 Opened 1 month ago Closed 1 month ago

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

Categories

(Developer Infrastructure :: Source Code Analysis, defect)

Unspecified
Windows
defect

Tracking

(firefox-esr115 unaffected, firefox127 unaffected, firefox128 wontfix, firefox129 fixed)

RESOLVED FIXED
129 Branch
Tracking Status
firefox-esr115 --- unaffected
firefox127 --- unaffected
firefox128 --- wontfix
firefox129 --- fixed

People

(Reporter: rkraesig, Assigned: glandium)

References

(Regression)

Details

(Keywords: regression)

Attachments

(1 file, 1 obsolete file)

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

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.

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.

Assignee: nobody → rkraesig
Status: NEW → ASSIGNED
Attachment #9406472 - Attachment is obsolete: true

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

Assignee: rkraesig → nobody
Status: ASSIGNED → NEW
Flags: needinfo?(mh+mozilla)
Assignee: nobody → rkraesig
Attachment #9406472 - Attachment is obsolete: false
Status: NEW → ASSIGNED

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.

Flags: needinfo?(mh+mozilla)

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.

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.

(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.

Attachment #9406472 - Attachment is obsolete: true
Pushed by mh@glandium.org:
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
Status: ASSIGNED → RESOLVED
Closed: 1 month ago
Resolution: --- → FIXED
Target Milestone: --- → 129 Branch

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.

Flags: needinfo?(rkraesig)
Assignee: rkraesig → mh+mozilla
Flags: needinfo?(rkraesig) → needinfo?(mh+mozilla)
Flags: needinfo?(mh+mozilla)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: