uwefladrich / scriptengine-tasks-ecearth

ScriptEngine tasks for the EC-Earth model
GNU General Public License v3.0
3 stars 3 forks source link

Building cartopy as dependency fails in PyPI action #88

Closed valentinaschueller closed 1 year ago

valentinaschueller commented 2 years ago

see https://github.com/uwefladrich/scriptengine-tasks-ecearth/actions/runs/3329060350/jobs/5505823280, error is here:

gcc -pthread -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O3 -Wall -fPIC -I/opt/hostedtoolcache/Python/3.10.8/x64/include -I./lib/cartopy -I/tmp/pip-build-env-2o260i0j/overlay/lib/python3.10/site-packages/numpy/core/include -I/opt/hostedtoolcache/Python/3.10.8/x64/include/python3.10 -c lib/cartopy/trace.cpp -o build/temp.linux-x86_64-cpython-310/lib/cartopy/trace.o
      lib/cartopy/trace.cpp:767:10: fatal error: geos_c.h: No such file or directory
        767 | #include "geos_c.h"
            |          ^~~~~~~~~~
      compilation terminated.
      error: command '/usr/bin/gcc' failed with exit code 1
      [end of output]

while trying to build cartopy wheel, version 0.21 (see these lines of output):

Collecting cartopy>=0.20
  Downloading Cartopy-0.21.0.tar.gz (10.9 MB)
uwefladrich commented 2 years ago

It's a bit strange. In the 90-split-pytest-action branch, I have split the action into sub-jobs. The error comes up there as well, in the coverage job, Install package + remaining PyPI dependencies task, see in this run.

The strange thing is now that the same Install package + remaining PyPI dependencies task runs fine for the pytest (3.7/8/9/10) jobs in the same run.

So somehow the building of the cartopy wheel works sometimes, while it fails in other cases.

uwefladrich commented 2 years ago

https://github.com/SciTools/cartopy/issues/1239

uwefladrich commented 2 years ago

Forget my previous comment. It is in the setup of the Github runner environment. What happens, I think, is that the conda environment is created, but not activated. Then cartopy gets installed with pip when pip install -e . is run to install se-t-ece locally in the runner. That doesn't work.

I have a working version of the runner that does linting, (py)testing and running coverage in the test-gh-actions branch. I will backport the changes to 90-split-pytest-action and that should solve #90 as well as this issue here.

uwefladrich commented 2 years ago

See #91, the tests seem to work again!

There were two issues, in the end:

My recommendation is to merge #91, which should solve both #88 and #90.

uwefladrich commented 2 years ago

on a second thought, I am not actually sure about -e in pip install. Maybe it is supposed to work even without?

uwefladrich commented 2 years ago

Tested, and pip install . does indeed the job, no -e needed. So what remains is the issue with using the base environment (and correct activation). #91 has gone through a lot of successful testing now, so I still assume it solves the issue.

valentinaschueller commented 2 years ago

I remember that I originally switched to using base instead of the default environment because I struggled with properly creating a clean test environment in my initial GitHub action testing. But it is good to see that it was never actually necessary. So have I understood correctly that the issue were some clashing dependencies in the base environment which kept cartopy from being installed properly?

uwefladrich commented 2 years ago

[...] So have I understood correctly that the issue were some clashing dependencies in the base environment which kept cartopy from being installed properly?

My understanding was that cartopy (and all other deps) were installed correctly in the base environment, but it ended up not being activated when it came to the task that run pip install . So then it attempted to install cartopy from PyPI, which doesn't work (at least if you do not install tons of other dependencies manually).

I came to the above conclusion by running conda info after conda-incubator/setup-miniconda and it showed "None" as the current environment. When I switched to the default of setting up and using the test environment, it worked.

But to be honest that doesn't fully explain why it was apparently working correct for some of the actions. The install-from-source action, for example, never had a problem from what I've seen. :thinking:

valentinaschueller commented 2 years ago

Hm, okay... Yes, the whole thing was strange, also because the actions worked for PR #80 but stopped working once you did the version bump to 0.6.1 (which did not have any meaningful changes that should have affected cartopy!). But either way, we can close the issue, at least for now, the problem seems to have been fixed by #91. Thanks for looking into this!

valentinaschueller commented 1 year ago

The issue has reappeared in the version bump to 0.6.3: https://github.com/uwefladrich/scriptengine-tasks-ecearth/actions/runs/3488689976/jobs/5837852201

uwefladrich commented 1 year ago

Yes, it is there again.

I have been looking into this a bit more and my understanding is now that the underlying issue is that cartopy from PyPI is distributed as source distribution (sdist), but not as wheel. This means that cartopy has to be build at installation time, which implies some build dependencies. In particular, it needs GEOS, Shapely, and pyshp (probably header files and such for those as well).

So the issue is that our Github action publish-to-pypi.yml triggers

python -m pip install scriptengine-tasks-ecearth

which will imply pip install cartopy>=0.20, which doesn't work because of missing build dependencies.

The idea was that the environment update from conda_environment.yml already installs cartopy from conda-forge, but that somehow does not work as expected.

uwefladrich commented 1 year ago

The strange thing is that the same job runs well, for example, in pytest.yml (job pytest). There is one difference, however:

    defaults:
      run:
        shell: bash -l {0}

is missing in publish-to-pypi.yml, job test-from-pypi. Could that be the problem? I remember it was needed for something, but not exactly why.

valentinaschueller commented 1 year ago

Could that be the problem? I remember it was needed for something, but not exactly why.

From the setup-miniconda readme:

Bash shells do not use ~/.profile or ~/.bashrc so these shells need to be explicitely declared as shell: bash -el {0} on steps that need to be properly activated (or use a default shell). This is because bash shells are executed with bash --noprofile --norc -eo pipefail {0} thus ignoring updated on bash profile files made by conda init bash. See Github Actions Documentation and thread.

Good catch! See also StackOverflow (as always):

-l to insure a login bash, where the environment is correctly set;

Then the error also makes sense: cartopy is already installed during miniconda setup, so it was weird that it was trying to reinstall it later. This indicates that the environment is not properly activated! In contrast: this is not happening in the pytest action

So I think this can fix the issue, indeed.

On a similar note: In the setup-miniconda, they used to recommend -l but they switched to -el because it can cause problems. So I propose to update all instances of bash -l {0} to bash -el {0}.

valentinaschueller commented 1 year ago

(Should be) closed by #94