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

Attempt to workaround venv failure in Actions runner 20240929.1. #950

Merged
merged 6 commits into from
Oct 9, 2024

Conversation

MarkCallow
Copy link
Collaborator

No description provided.

@MarkCallow
Copy link
Collaborator Author

@ShukantPal I need your help. Two updates in a row of the GitHub Actions Windows runner image have broken build of the KTX-Software python interface. I managed to handle the issue last time at considerable time cost. This time I'm stuck. The error, which appeared when using the Windows runner image was updated to 20240929.1, is shown in the following extract:

  Clean up pyktx build artifacts
  Python distributions
  Building Custom Rule D:/a/KTX-Software/KTX-Software/interface/python_binding/CMakeLists.txt
  * Creating venv isolated environment...
  ERROR Failed to create venv. Maybe try installing virtualenv.

The latest change to this PR was to install virtualenv (successfully). However the same error still happens. I've managed to track down the "Clean up ..." and "Python distributions" messages which are in custom commands in the mentioned CMakeLists.txt. I think the " Building ..." message must be printed when everything in that file has finished.

I have been unable to find the "* Creating venv ..." message in any of the files under interface/python_binding so am unable to see the commands being used which is a precursor to understanding what is going wrong.

Unfortunately it is not possible to make GitHub Actions use the previous runner image. We're stuck with the current one.

Please figure out what is going wrong and how to fix it.

@ShukantPal
Copy link
Contributor

Hi @MarkCallow,

It seems like the new images don't have virtualenv pre-installed. You can install it using pip before the build step.

https://virtualenv.pypa.io/en/latest/installation.html

@MarkCallow
Copy link
Collaborator Author

It seems like the new images don't have virtualenv pre-installed. You can install it using pip before the build step.

That is what I thought so the last but one commit installs virtualenv via pipx. The installation was successful but I still get the error. The latest commit tries to invoke activate but it is not working. I think I don't have the right path for Windows. I look further into it tomorrow.

BUT, the change log for the Windows runner image shows no changes whatever to Python components.

@ShukantPal
Copy link
Contributor

Try using pip instead of pipx so that virtualenv is installed as a global Python module. You can see it being referenced using python3 -m virtualenv here:

check_call(['python3', '-m', 'venv', venv_dir])

@MarkCallow
Copy link
Collaborator Author

Try using pip instead of pipx so that virtualenv is installed as a global Python module. You can see it being referenced using python3 -m virtualenv here:

check_call(['python3', '-m', 'venv', venv_dir])

As far as I can see fmt/doc/build.py is only included if the CMake variable FMT_DOC is ON but it appears to be OFF. So I don't think this is the triggering reference.

@MarkCallow
Copy link
Collaborator Author

MarkCallow commented Oct 9, 2024

pip worked. Thanks for the tip @ShukantPal.

These are the commands in the build that are triggering the virtualenv requirement:

${PYTHON_EXECUTABLE} -m build --sdist --outdir ${DIST_DIR}

${PYTHON_EXECUTABLE} -m build --wheel --outdir ${DIST_DIR}

Building the requirements listed in requirements.txt does not require virtualenv.

@MarkCallow MarkCallow merged commit e304057 into main Oct 9, 2024
18 checks passed
@MarkCallow MarkCallow deleted the fix-venv branch October 9, 2024 12:57
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