Skip to content

Conversation

@amyspark
Copy link
Contributor

@amyspark amyspark commented Sep 1, 2023

👋

This is a PR that fixes #12191 (at least to the extent that my local project stash could check).

It requires #12144 to fix argument detection.

@DFOVIT
Copy link
Contributor

DFOVIT commented Sep 1, 2023

Feel free to ignore as this is only tangentially related to your problem but i'm pretty sure that this line needs the same Path().as_posix() treatment because meson devenv will generate a vscode env file with paths like c:\folder and you will get surprises when your folder name start with a 'n'. I tested with the Path().as_posix() and it generates a correct vscode env file for windows.

@tristan957 tristan957 added this to the 1.3.0 milestone Sep 1, 2023
@amyspark amyspark force-pushed the fix-microsoft-clang-paths branch from 06ca5fd to 66a1728 Compare September 6, 2023 01:26
@amyspark amyspark force-pushed the fix-microsoft-clang-paths branch 2 times, most recently from 533fe7f to bc3f022 Compare September 17, 2023 19:16
@xclaesse
Copy link
Member

I'm a bit worried that scattering millions of Path().as_posix() and replace('\\', '/') in random places of Meson is going to be slow and inconsistent. Most of the time it is to workaround broken tools that should instead get fixed.

Alternatively, we could make a project wide search-and-replace to ban usage of os.path.join() and str(Path()), and ensure all our paths are always posix from the beginning, instead of fixing them in random places.

@amyspark
Copy link
Contributor Author

Most of the time it is to workaround broken tools that should instead get fixed.

I could agree -- in this case, Microsoft Clang not accepting Windows-style paths.

But, if you check the changes that are necessary in the unit tests, this assertion also applies to Meson itself 😃 so the proper fix would be to use one or the other consistently. But there are 300+ places where this is used, so I don't quite know which ones are for internal consumption only and which ones do are passed externally.

@xclaesse xclaesse modified the milestones: 1.3.0, 1.4.0 Oct 17, 2023
@amyspark amyspark force-pushed the fix-microsoft-clang-paths branch from bc3f022 to df0b725 Compare February 25, 2024 16:38
@amyspark amyspark changed the title clang: Always pass POSIX paths to consumers backends: Use POSIX paths for target paths Feb 25, 2024
@amyspark
Copy link
Contributor Author

@eli-schwartz I just found #12564 which solves a lot of the issues I was facing. However, there are a couple spots that still needed fixing when using Microsoft clang. I've rebased and fixed them here, and rephrased the commit to clarify that.

@amyspark amyspark force-pushed the fix-microsoft-clang-paths branch 3 times, most recently from 93c68f0 to e3c9dc2 Compare February 25, 2024 17:31
@nirbheek
Copy link
Member

nirbheek commented Jul 3, 2025

Needs rebasing, branch has conflicts.

Copy link
Member

@nirbheek nirbheek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, since all these are places where we were using os.path.join and now are using as_posix().

For future reference: replacing os.path.join good, and str.replace('\\', '/') BAD BAD BAD.

@nirbheek nirbheek added the OS:windows Winodows OS specific issues label Jul 3, 2025
This commit completes 5de09cb,
ensuring that only POSIX style paths are passed to the compiler
line, and thus fixing UNIX-style tools that treat single
backward slashes as Unicode escaped characters.

Fixes mesonbuild#12191

Completes mesonbuild#12534

Completes mesonbuild#12564
@amyspark amyspark force-pushed the fix-microsoft-clang-paths branch from e3c9dc2 to d139854 Compare July 3, 2025 22:27
@amyspark amyspark requested a review from eli-schwartz July 3, 2025 22:27
@nirbheek nirbheek merged commit 12563f7 into mesonbuild:master Jul 13, 2025
32 checks passed
@nirbheek nirbheek modified the milestones: 1.4.0, 1.9 Jul 20, 2025
@amyspark amyspark deleted the fix-microsoft-clang-paths branch August 3, 2025 17:02
@jpakkane
Copy link
Member

Unfortunately this had to be reverted because it was breaking projects in the wild. :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

OS:windows Winodows OS specific issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OS path syntax usage for all compilers breaks linking and PDB with Microsoft Clang + LINK.EXE

7 participants