Skip to content

Conversation

@markl-arista
Copy link
Contributor

The python version detection logic previously searched for "python" in the full path. This failed for paths where a directory name contained "python" (e.g., .../python39/.../libpython3.9.so), causing the parser to misinterpret the version numbers.

Changes:

  • Parsing logic now searches for "python" only in the filename (after the last slash).
  • Extracted parsing logic into getPythonVersionFromFilename for better testability.
  • Improved exception messages to aid debugging.
  • Added tests/python_path_test.cc to verify correct behavior against various path patterns.

Test plan:

mkdir -p build
cd build
cmake -DCMAKE_BUILD_TYPE=RelWithDebInfo -DPYTHON3=ON ..
make -j
make test

The python version detection logic previously searched for "python" in the full
path. This failed for paths where a directory name contained "python" (e.g.,
.../python39/.../libpython3.9.so), causing the parser to misinterpret the version
numbers.

Changes:
 - Parsing logic now searches for "python" only in the filename (after the last
   slash).
 - Extracted parsing logic into getPythonVersionFromFilename for better
   testability.
 - Improved exception messages to aid debugging.
 - Added tests/python_path_test.cc to verify correct behavior against various path
   patterns.
Fix CMake error so that python_path_test.cc test
will be compiled and run with -DPYTHON3=on.

We need to conditionally compile `python_path_test.cc`. Also,
we should not call `add_subdirectory(tests)` at the beginning
because otherwise Python3_Development_FOUND will be false
when the build process enters `tests/CMakeLists.txt`.
<< filename << " (too short)";

char majorChar = filename[index + 6];
char minorChar = filename[index + 8];
Copy link
Owner

Choose a reason for hiding this comment

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

This particular act of laziness on my part (I think) is gonna bite us with py3.14, but that's not your fault or problem to solve.

@peadar peadar merged commit 6f22265 into peadar:master Jan 15, 2026
5 checks passed
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.

2 participants