Skip to content

Fix prepending duplicate sys_root to python build configs#15589

Open
mgorny wants to merge 4 commits intomesonbuild:masterfrom
mgorny:python-cross
Open

Fix prepending duplicate sys_root to python build configs#15589
mgorny wants to merge 4 commits intomesonbuild:masterfrom
mgorny:python-cross

Conversation

@mgorny
Copy link
Contributor

@mgorny mgorny commented Feb 27, 2026

Fix the logic responsible for handling python build configs to check if paths from build-details.json start with sys_root, and do not prepend it if they do. This matches the pkg-config behavior (of PKG_CONFIG_SYSROOT_DIR). It permits using it both with sysroots that are a regular system mounted in a subdirectory, and that are dedicated prefixes (e.g. created via conda).

@mgorny mgorny requested a review from jpakkane as a code owner February 27, 2026 16:47
@mgorny mgorny marked this pull request as draft February 27, 2026 18:20
@mgorny
Copy link
Contributor Author

mgorny commented Feb 27, 2026

Sorry about the Ubuntu test failure. I'll try to reproduce and fix it tomorrow.

@mgorny
Copy link
Contributor Author

mgorny commented Feb 28, 2026

Uh, is Ubuntu's pkg-config broken?

# PKG_CONFIG_SYSROOT_DIR=/usr pkg-config --cflags python3
-I/usr/usr/include/python3.7m -I/usr/usr/include/x86_64-linux-gnu/python3.7m

@thesamesam
Copy link
Member

Are they still using fd.o pkg-config instead of pkgconf?

@mgorny
Copy link
Contributor Author

mgorny commented Feb 28, 2026

Are they still using fd.o pkg-config instead of pkgconf?

Looks like it:

pkg-config/bionic,now 0.29.1-0ubuntu2 amd64 [installed]

@mgorny mgorny marked this pull request as ready for review March 2, 2026 15:53
@mgorny
Copy link
Contributor Author

mgorny commented Mar 2, 2026

The macOS failures don't seem relevant.

@bonzini
Copy link
Contributor

bonzini commented Mar 17, 2026

Looks good, but please replace sysroot + path with destdir_join(sysroot, path) (from scripts import destdir_join)

@bonzini bonzini added this to the 1.11 milestone Mar 17, 2026
@mgorny
Copy link
Contributor Author

mgorny commented Mar 17, 2026

Done and rebased.

@bonzini
Copy link
Contributor

bonzini commented Mar 17, 2026

Thanks, can you also squash the first three commits?

@mgorny
Copy link
Contributor Author

mgorny commented Mar 17, 2026

Sure. Don't really get why these three, but sure :-).

@bonzini
Copy link
Contributor

bonzini commented Mar 17, 2026

Because the destdir_join part is a separate bug fix. Thanks, I will merge when CI finishes, just to be safe.

@eli-schwartz
Copy link
Member

Looks good, but please replace sysroot + path with destdir_join(sysroot, path) (from scripts import destdir_join)

destdir_join(a, b) is equal to a + b on not-Windows, so it is fine to use a + b inside find_libpy_notwindows(). I find your request to be poor nitpicking and also a micro-deoptimization.

@eli-schwartz
Copy link
Member

Sure. Don't really get why these three, but sure :-).

Commit "Fix get_windows_link_args() return value" is a git commit --fixup for a bug introduced in commit "Fix prepending duplicate sys_root to python build configs".

Squashing is always correct for this.

Commit "Skip pkgconfig+sysroot test without pkgconf" can be reasonably argued to be a standalone change, regardless of whether fixing duplicate sysroot was the motivation to discover it. I think it's fine to leave as two commits.

@bonzini
Copy link
Contributor

bonzini commented Mar 17, 2026

it is fine to use a + b inside find_libpy_notwindows

Sure, not in the Windows cases though, and it's easier to write and review if all of them are replaced.

Commit "Skip pkgconfig+sysroot test without pkgconf" can be reasonably argued to be a standalone change,

True, but as a separate commit after the main one it breaks git bisect. I didn't check if it was possible to move it as a separate commit before the main one, but that would also have been nitpicking...

@eli-schwartz
Copy link
Member

True, but as a separate commit after the main one it breaks git bisect. I didn't check if it was possible to move it as a separate commit before the main one, but that would also have been nitpicking...

The worst case scenario would be that it breaks bisecting in a single CI job that exists to test whether meson still works on Python 3.7 / Ubuntu Bionic, and that is only because it is likely too much effort for too little reward to compile pkgconf just for this CI runner, whereas anyone actually relying on the CI-tested scenario would be motivated to build pkgconf there... (it is generally a bad idea to try to do anything cross with fd.o pkgconfig at this point).

mgorny added 4 commits March 18, 2026 16:18
Fix the logic responsible for handling python build configs to check
if paths from `build-details.json` start with sys_root, and do not
prepend it if they do. This matches the pkg-config behavior (of
`PKG_CONFIG_SYSROOT_DIR`). It permits using it both with sysroots that
are a regular system mounted in a subdirectory, and that are dedicated
prefixes (e.g. created via conda).

Signed-off-by: Michał Górny <mgorny@quansight.com>
Signed-off-by: Michał Górny <mgorny@quansight.com>
Signed-off-by: Michał Górny <mgorny@quansight.com>
Signed-off-by: Michał Górny <mgorny@quansight.com>
@mgorny
Copy link
Contributor Author

mgorny commented Mar 18, 2026

Restored the separate "skip" commit, switched to SkipTest, and switched the other continues to SkipTests too. I didn't revert destdir_join() but it's a separate commit, so you or I can remove it.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants