Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure meson's prefix is a valid absolute path #17206

Merged
merged 1 commit into from
Oct 28, 2024

Conversation

shoeffner
Copy link
Contributor

@shoeffner shoeffner commented Oct 23, 2024

The path / is not considered an absolute path on Windows, failing with:

ERROR: prefix value '/' must be an absolute path

os.path.abspath ensures that "/" becomes a valid absolute path on all major platforms, resolving to "/" on Linux and macOS, and to "C:\" (or whichever filesystem is currently active, could be E:\ as well) on Windows.

Closes #17204.

Changelog: Fix: Use a valid prefix path for meson.configure() on Windows, to avoid failures in Python 3.13.
Docs: Omit

  • Refer to the issue that supports this Pull Request.
  • If the issue has missing info, explain the purpose/use case/pain/need that covers this Pull Request.
  • I've read the Contributing guide.
  • I've followed the PEP8 style guides for Python code. -> No, line length is longer than 80 or such
  • I've opened another PR in the Conan docs repo to the develop branch, documenting this one. -> No

@memsharded
Copy link
Member

Thanks for your contribution!

This PR is failing in Windows with:

myhello  undefined
E           
E             User defined options
E               Native files: C:\J\t_v2\9ed6\PY38\tmp_gcx6x3iconans\path with spaces\build-release\conan\conan_meson_native.ini
E               prefix      : C:\
E           
E           Found ninja-1.8.2 at "C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\Common7\IDE\CommonExtensions\Microsoft\CMake\Ninja\ninja.EXE"
E           
E           conanfile.py (demo/0.1): Meson build cmd: meson compile -C "C:\J\t_v2\9ed6\PY38\tmp_gcx6x3iconans\path with spaces\build-release" -j16
E           conanfile.py (demo/0.1): RUN: meson compile -C "C:\J\t_v2\9ed6\PY38\tmp_gcx6x3iconans\path with spaces\build-release" -j16
E           conanvcvars.bat: Activating environment Visual Studio 15 - amd64 - winsdk_version=None - vcvars_ver=14.1
E           [vcvarsall.bat] Environment initialized for: 'x64'
E           [1/2] Compiling C object myhello.exe.p/src_main.c.obj
E           FAILED: myhello.exe.p/src_main.c.obj 
E           "cl" "-Imyhello.exe.p" "-I." "-I.." "-DNDEBUG" "/MD" "/nologo" "/showIncludes" "/utf-8" "/W2" "/O2" "/Gw" "/Fdmyhello.exe.p\src_main.c.pdb" /Fomyhello.exe.p/src_main.c.obj "/c" ../src/main.c
E           .\config.h(8): error C2001: newline in constant
E           ninja: build stopped: subcommand failed.
E           INFO: autodetecting backend as ninja

It is possible that the os.path.abspath result needs to be back to forward slash for Meson to process it?

@shoeffner
Copy link
Contributor Author

Thanks for the logs! I don't think it's the \ vs / since the native files also use backslashes – and I was able to build inih (see our discussion in #17204) with this change.

I will try to get the full test suite up and running on our build machine and see if I can reproduce and fix the issue.

@memsharded
Copy link
Member

I will try to get the full test suite up and running on our build machine and see if I can reproduce and fix the issue.

Probably not worth the effort. The full suite, specially the functional tests, have tons of tools to configure in conftest and conftest_user.py. You are probably good with running just a small subset. The test failing is test_meson_using_prefix_path_in_application, probably running tests in functional/toolchain/meson is more than enough.

The path / is not considered an absolute path on Windows, failing with:

    ERROR: prefix value '/' must be an absolute path

os.path.abspath ensures that "/" becomes a valid absolute path on all major
platforms, resolving to "/" on Linux and macOS, and to "C:/" (or whichever
filesystem is currently active, could be E:/ as well) on Windows.

Closes conan-io#17204.
@shoeffner
Copy link
Contributor Author

tl;dr: C:\ is interpreted by the failing test as a newline in constant:

E           .\config.h(8): error C2001: newline in constant

Switching it to C:/ works, and all other test results remain unchanged.

long read:

Okay, that took longer than I expected. I had to change test\utils\tools.py as our mvsc has version 194 (in contrast to the default test version 191) and it took me a while to figure that out.

Here's the result of the unmodified (except for the version as described above) develop2 branch with pip-installed meson:

C:\src\conan>pytest -n 16 test\functional\toolchains\meson
===================================== test session starts ======================================
platform win32 -- Python 3.10.11, pytest-7.4.4, pluggy-1.5.0
rootdir: C:\src\conan
configfile: pytest.ini
plugins: xdist-3.6.1
16 workers [32 items]
ssssssssssssssss..s..s.....F....                                                          [100%]
=========================================== FAILURES ===========================================
___________________________________ test_meson_lib_template ____________________________________
===================== 1 failed, 13 passed, 18 skipped in 74.57s (0:01:14) ======================

I haven't checked yet why this particular test fails, maybe it's because I changed the compiler version locally and this assumes a specific hash?

This is the (shortened) output with the choco-installed meson:

=================================== short test summary info ====================================
FAILED test/functional/toolchains/meson/test_install.py::MesonInstall::test_install - Exception:
FAILED test/functional/toolchains/meson/test_subproject.py::test_subproject - Exception:
FAILED test/functional/toolchains/meson/test_backend.py::test_cross_x86 - Exception:
FAILED test/functional/toolchains/meson/test_meson_and_gnu_deps_flags.py::TestMesonToolchainAndGnuFlags::test_mesondeps_flags_are_being_appended_and_not_replacing_toolchain_ones - Exception:
FAILED test/functional/toolchains/meson/test_v2_meson_template.py::test_meson_lib_template - Exception:
FAILED test/functional/toolchains/meson/test_cross_compilation.py::test_windows_cross_compiling_x86 - Exception:
FAILED test/functional/toolchains/meson/test_meson.py::test_meson_using_prefix_path_in_application - Exception:
FAILED test/functional/toolchains/meson/test_meson.py::MesonToolchainTest::test_definition_of_global_options - Exception:
FAILED test/functional/toolchains/meson/test_v2_meson_template.py::test_meson_exe_template - Exception:
FAILED test/functional/toolchains/meson/test_meson.py::MesonToolchainTest::test_meson_default_dirs - Exception:
FAILED test/functional/toolchains/meson/test_pkg_config_reuse.py::MesonPkgConfigTest::test_reuse - Exception:
FAILED test/functional/toolchains/meson/test_test.py::MesonTest::test_reuse - Exception:
FAILED test/functional/toolchains/meson/test_preprocessor_definitions.py::MesonPreprocessorDefinitionsTest::test_build - Exception:
========================== 13 failed, 1 passed, 18 skipped in 20.62s ===========================

And the error is exactly what I observed initially:

E           ..\meson.build:2:0: ERROR: prefix value '/' must be an absolute path

With my fix, I can reproduce the error observed in the CI pipeline:

FAILED test/functional/toolchains/meson/test_meson.py::test_meson_using_prefix_path_in_application - Exception:

A related issue seems to be #14213 and the test there checks if the prefix is set correctly. It appears that particular test catches problematic backslashes in the prefix path, so C:\, while valid for meson, might be a broken value for some recipes. Using a unix filesep as described in the issue works, i.e., passing C:/ instead.

I updated the code and added a _prefix property for which I documented the constraints and reasoning why the os.path.abspath and replace are required.

Copy link
Member

@memsharded memsharded left a comment

Choose a reason for hiding this comment

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

Many thanks for your contribution @shoeffner

Checked that without this the test suite fails in Python 3.13.
We might need to add it to our CI, it seems that 3.13 is changing a few things that are breaking.

Lets merge this to include it in next Conan 2.9 asap

@memsharded memsharded added this to the 2.9.0 milestone Oct 28, 2024
@memsharded memsharded self-assigned this Oct 28, 2024
@memsharded memsharded merged commit 48e9c81 into conan-io:develop2 Oct 28, 2024
2 checks passed
@shoeffner shoeffner deleted the meson-prefix branch October 29, 2024 07:52
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.

[bug] src\meson.build:1:0: ERROR: prefix value '/' must be an absolute path
2 participants