Create Python virtual environment

[shenxianpeng]

Not sure if this is correct (maybe needs to have if else conditions to check different paltform because of different commands), please help to review it.

[2bndy5]

Activating a venv already adds the python exe path to the env var PATH. I'm wondering if the venv needs to be activated for each step...

[shenxianpeng]

OK will try

[shenxianpeng]

It works https://github.com/cpp-linter/test-cpp-linter-action/actions/runs/3270648589/jobs/5379521124

Just the code does not look good. Maybe we can remove some activate venv code from the steps.

[2bndy5]

It looks like that did the trick! Now time to polish it up.

1. We probably don't want to pollute the users' workspace with our venv, so we could use a temp dir instead of the /users/runner/workspace. Github actions has a runner.temp variable that might be used to do this.
2. It would be best practice to use the venv on every platform, so no need to use bash if [[ ${{runner.os}} == 'macOS' ]].

[shenxianpeng]

It would be best practice to use the venv on every platform, so no need to use bash if [[ ${{runner.os}} == 'macOS' ]].

Windows will failed if not specified https://github.com/cpp-linter/test-cpp-linter-action/actions/runs/3270593867/jobs/5379399304
Maybe I can create a shell script to deal with all platforms.

[2bndy5]

It would be best practice to use the venv on every platform, so no need to use bash if [[ ${{runner.os}} == 'macOS' ]].

Windows will failed if not specified https://github.com/cpp-linter/test-cpp-linter-action/actions/runs/3270593867/jobs/5379399304 Maybe I can create a shell script to deal with all platforms.

That's because the activate command is located differently on Windows:

./venv/Scripts/activate

where as on Linux & macOS:

source ./venv/bin/activate

It might be better to have the venv creation/activation in a .sh script file, so we could just call it from any steps that use the venv.

[2bndy5]

You can use github.action_path to access venv.sh. Optionally, I would recommend using the env var RUNNER_TEMP in the venv.sh file to keep the venv located outside the users workspace.

[2bndy5]

I see that you're using the ${{ runner.os }} that isn't working in the venv.sh file. There's a RUNNER_OS env var that you can use in venv.sh.

[2bndy5]

Hmm. The venv executable path doesn't seem to be getting used after the venv.sh script completes. I'm looking at a similar approach that was done in arduino/compile-sketches composite action in which they directly invoke the executable located in the venv that they create with a bash script.

[2bndy5]

nope 😠 . It starting to seem easier to just manage the venv in actions.yml as we had before.

[shenxianpeng]

I like it hides details in action-setup.sh, if we rewrite like https://github.com/arduino/compile-sketches/blob/main/action-setup.sh, it may be work

[2bndy5]

That will only work on Linux because they use the deadsnake PPA to install a certain build of python.

[2bndy5]

And remember that the set-outputs command is now deprecated.

[2bndy5]

We can probably combine the "create venv" step with the "install deps" step.

[2bndy5]

Now it seems to be having problems with the Windows path delimiters in a bash prompt on Windows:

D:\a\_actions\cpp-linter\cpp-linter-action\latest\venv\Scripts\activate

is executed in bash like escape chars:

D:a_actionscpp-lintercpp-linter-actionlatestvenvScriptsactivate

[2bndy5]

OMG! Why is this one of those things that is easier said than done?!

Sorry, but I'm going to sleep now. I'll try to pick this up tomorrow (& hopefully try resolve cpp-linter/clang-tools-pip#15 in a more platform independent way).

[shenxianpeng]

Haha.. Devils in the details. have a good night~

[2bndy5]

I've started trying to use actions/setup-python to create a venv that doesn't modify the workspace env, but I think we still need to address the cpp-linter/clang-tools-pip#15 so that cpp-linter can invoke the clang-*-<version> executable (or symlink).

The problem, as I see it, is that the python executable path is not guaranteed to be in the env var PATH. Meaning we have to choose a path that is guaranteed to be in the env var PATH when installing the clang-tools (or symlinks).

[2bndy5]

closing in favor of #113