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

ci: always use $PATH python #172

Merged
merged 2 commits into from
Dec 10, 2024
Merged

ci: always use $PATH python #172

merged 2 commits into from
Dec 10, 2024

Conversation

tych0
Copy link
Owner

@tych0 tych0 commented Dec 10, 2024

Consider the following output form which python3 and which pytest-3 in the CI environment:

which python3
shell: /usr/bin/bash -e {0}
env:
    pythonLocation: /opt/hostedtoolcache/Python/3.10.15/x64
    PKG_CONFIG_PATH: /opt/hostedtoolcache/Python/3.10.15/x64/lib/pkgconfig
    Python_ROOT_DIR: /opt/hostedtoolcache/Python/3.10.15/x64
    Python2_ROOT_DIR: /opt/hostedtoolcache/Python/3.10.15/x64
    Python3_ROOT_DIR: /opt/hostedtoolcache/Python/3.10.15/x64
    LD_LIBRARY_PATH: /opt/hostedtoolcache/Python/3.10.15/x64/lib
/opt/hostedtoolcache/Python/3.10.15/x64/bin/python3

which pytest-3
shell: /usr/bin/bash -e {0}
env:
    pythonLocation: /opt/hostedtoolcache/Python/3.10.15/x64
    PKG_CONFIG_PATH: /opt/hostedtoolcache/Python/3.10.15/x64/lib/pkgconfig
    Python_ROOT_DIR: /opt/hostedtoolcache/Python/3.10.15/x64
    Python2_ROOT_DIR: /opt/hostedtoolcache/Python/3.10.15/x64
    Python3_ROOT_DIR: /opt/hostedtoolcache/Python/3.10.15/x64
    LD_LIBRARY_PATH: /opt/hostedtoolcache/Python/3.10.15/x64/lib
/usr/bin/pytest-3

Since we were getting e.g. cffi vi apt-get install python3-cffi, we were using the system python's cffi for everything. That worked because we invoked the system pytest-3.

Of course, that means we were not really using the github python action's python in our CI matrix, since we were always using the system python for everything.

Instead, let's set up a venv as a traditional project would, and that way we can explicitly call that venv's python everywhere in the makefile, so we always use that venv's packages and get the right python.

Then, we need only be careful to use the $PATH python, not /usr/bin/python, to set up this venv, and we always get the right version of python to test with.

This likely explains some weirdness that I've seen in the past, should have investigated sooner...

Consider the following output form `which python3` and `which pytest-3` in
the CI environment:

    which python3
    shell: /usr/bin/bash -e {0}
    env:
        pythonLocation: /opt/hostedtoolcache/Python/3.10.15/x64
        PKG_CONFIG_PATH: /opt/hostedtoolcache/Python/3.10.15/x64/lib/pkgconfig
        Python_ROOT_DIR: /opt/hostedtoolcache/Python/3.10.15/x64
        Python2_ROOT_DIR: /opt/hostedtoolcache/Python/3.10.15/x64
        Python3_ROOT_DIR: /opt/hostedtoolcache/Python/3.10.15/x64
        LD_LIBRARY_PATH: /opt/hostedtoolcache/Python/3.10.15/x64/lib
    /opt/hostedtoolcache/Python/3.10.15/x64/bin/python3

    which pytest-3
    shell: /usr/bin/bash -e {0}
    env:
        pythonLocation: /opt/hostedtoolcache/Python/3.10.15/x64
        PKG_CONFIG_PATH: /opt/hostedtoolcache/Python/3.10.15/x64/lib/pkgconfig
        Python_ROOT_DIR: /opt/hostedtoolcache/Python/3.10.15/x64
        Python2_ROOT_DIR: /opt/hostedtoolcache/Python/3.10.15/x64
        Python3_ROOT_DIR: /opt/hostedtoolcache/Python/3.10.15/x64
        LD_LIBRARY_PATH: /opt/hostedtoolcache/Python/3.10.15/x64/lib
    /usr/bin/pytest-3

Since we were getting e.g. cffi vi `apt-get install python3-cffi`, we were
using the *system* python's cffi for everything. That worked because we
invoked the *system* pytest-3.

Of course, that means we were not really using the github python action's
python in our CI matrix, since we were always using the system python for
everything.

Instead, let's set up a venv as a traditional project would, and that way
we can explicitly call that venv's python everywhere in the makefile, so we
always use that venv's packages and get the right python.

Then, we need only be careful to use the $PATH python, not /usr/bin/python,
to set up this venv, and we always get the right version of python to test
with.

This likely explains some weirdness that I've seen in the past, should have
investigated sooner...

Signed-off-by: Tycho Andersen <[email protected]>
@tych0 tych0 mentioned this pull request Dec 10, 2024
we have to flock cabal so it doesn't step on itself, but that does mean we
can run e.g. the venv setup and cabal build in parallel.

Signed-off-by: Tycho Andersen <[email protected]>
@georgeharker
Copy link

This looks good - there's one subtle thing I've run into - which is that when run from xcffib/ import picks up the local module not the env one - so we may want to pushd into those dirs after installs to do checks - but I suspect this will be a big improvement and I can remove the PYTHON env var from my PR and just keep the test venv (which wants to be separate as I trash it with various different flavors of cffi mode as they get tested).

@tych0
Copy link
Owner Author

tych0 commented Dec 10, 2024

xcffib/ import picks up the local module not the env one

I don't think xcffib is currently ever installed to the venv, is it? IIUC we always use the local module (i.e. the one in ./xcffib in the tree) to test. If not, I think we should.

@georgeharker
Copy link

In my latest iteration on the PR I wanted to test that the entire install process works - so I did

check-abi:
	# check abi precompiled mode
	TMPLOC=`mktemp -d`; \
	$(PYTHON) -m venv $${TMPLOC}; \
	source $${TMPLOC}/bin/activate; \
	CC=/bin/false $(PYTHON) -m pip install -v . ; \
	pushd $${TMPLOC}; \
	$(PYTHON) -c "import xcffib; assert xcffib.cffi_mode == 'abi_precompiled'" ; \
	popd; \
	pytest-3 -v --durations=3 -n auto

Which installs with abi mode, ensures we find that by cd-ing away (not the local source copy), checks the cffi mode and does the tests on the installed copy

@tych0
Copy link
Owner Author

tych0 commented Dec 10, 2024

Ah, I see. And this is probably needed just to make sure the cffi setup.py bits work correctly, so that make sense to me.

In any case, I'll land this one since hopefully it fixes the confusion about which python is really running.

@tych0 tych0 merged commit 02d2aec into master Dec 10, 2024
30 checks passed
georgeharker added a commit to georgeharker/xcffib that referenced this pull request Dec 10, 2024
remove python env var post tych0#172 merge
ensure pytest runs even when installed as pytest
@georgeharker
Copy link

Have modified the PR and rebased off latest. Should be cleaner and a good way to test the cffi modes.

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