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

Run testbed on fixed Ubuntu version and corresponding system Python version #1813

Merged
merged 10 commits into from
Mar 15, 2023

Conversation

mhsmith
Copy link
Member

@mhsmith mhsmith commented Mar 13, 2023

Similar to beeware/briefcase#1130.

PR Checklist:

  • All new features have been tested
  • All new features have been documented
  • I have read the CONTRIBUTING.md file
  • I will abide by the code of conduct

@mhsmith
Copy link
Member Author

mhsmith commented Mar 13, 2023

@rmartin16 @freakboy3742: Any idea what's going on with this CI failure? I can't reproduce it within Docker on my Linux machine, either on debian:bullseye or ubuntu:jammy.

[testbed] Starting test_suite...
===========================================================================
Install path: /home/runner/work/toga/toga/testbed/build/testbed/ubuntu/jammy/testbed-0.0.1/usr
Pre-initializing Python runtime...
PYTHONPATH:
- /usr/lib/python3.10
- /usr/lib/python3.10/lib-dynload
- /home/runner/work/toga/toga/testbed/build/testbed/ubuntu/jammy/testbed-0.0.1/usr/lib/testbed/app
- /home/runner/work/toga/toga/testbed/build/testbed/ubuntu/jammy/testbed-0.0.1/usr/lib/testbed/app_packages
Configure argc/argv...
Initializing Python runtime...
Running app module: tests.testbed
---------------------------------------------------------------------------
Application quit abnormally (Exit code -6)!
Traceback (most recent call last):
  File "/usr/lib/python3.10/runpy.py", line 196, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "/usr/lib/python3.10/runpy.py", line 86, in _run_code
    exec(code, run_globals)
  File "/home/runner/work/toga/toga/testbed/build/testbed/ubuntu/jammy/testbed-0.0.1/usr/lib/testbed/app/tests/testbed.py", line 3, in <module>
    import tempfile
  File "/usr/lib/python3.10/tempfile.py", line 184, in <module>
    from random import Random as _Random
  File "/usr/lib/python3.10/random.py", line 49, in <module>
    from math import log as _log, exp as _exp, pi as _pi, e as _e, ceil as _ceil
ModuleNotFoundError: No module named 'math'

@freakboy3742
Copy link
Member

@mhsmith Mechanically, it looks like it's not finding the Python standard library - except that it is finding some of the standard library (tempfile). Since it's the math module that is missing, my guess is that it's not finding a binary module; those should be in /usr/bin/python3.10/lib-dynload, AFAIK, but either it's not present on the CI machine (possibly due to no having installed the necessary stdlib package?), or the path is different. Unfortunately, my Linux test machine is back at home, so I can't test that hypothesis easily.

@rmartin16
Copy link
Member

rmartin16 commented Mar 13, 2023

Out of curiosity, I used ldd to show that the loader is finding the Python from actions/setup-python:

[testbed] Building application...
Build bootstrap binary...
make: Entering directory '/home/runner/work/toga/toga/testbed/build/testbed/ubuntu/jammy/bootstrap'
python3.10 -c "import sys; print(f'#define PY_TAG \"{sys.version_info.major}.{sys.version_info.minor}\"')" > pyversion.h
gcc -o testbed main.c pyversion.h -I/usr/include/python3.10 -I/usr/include/python3.10  -Wno-unused-result -Wsign-compare -g      -fstack-protector-strong -Wformat -Werror=format-security  -DNDEBUG -g -fwrapv -O2 -Wall -L/usr/lib/python3.10/config-3.10-x86_64-linux-gnu -L/usr/lib/x86_64-linux-gnu -lpython3.10 -lcrypt -ldl  -lm -lm  -fPIE
mkdir -p ../testbed-0.0.1/usr/bin
cp testbed ../testbed-0.0.1/usr/bin
make: Leaving directory '/home/runner/work/toga/toga/testbed/build/testbed/ubuntu/jammy/bootstrap'
Building bootstrap binary... done

Installing license... done

Installing changelog... done

Installing man page... done

Update file permissions...
Updating file permissions... done

Stripping binary... done


[testbed] Built build/testbed/ubuntu/jammy/testbed-0.0.1/usr/bin/testbed
	linux-vdso.so.1 (0x00007ffd9c59a000)
	libpython3.10.so.1.0 => /opt/hostedtoolcache/Python/3.10.10/x64/lib/libpython3.10.so.1.0 (0x00007fba048e5000)
	libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007fba046b1000)
	libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007fba045ca000)
	/lib64/ld-linux-x86-64.so.2 (0x00007fba04c9f000)

If I unset LD_LIBRARY_PATH, I got a new error:

[testbed] Starting test_suite...
===========================================================================
Install path: /home/runner/work/toga/toga/testbed/build/testbed/ubuntu/jammy/testbed-0.0.1/usr
Pre-initializing Python runtime...
PYTHONPATH:
- /usr/lib/python3.10
- /usr/lib/python3.10/lib-dynload
- /home/runner/work/toga/toga/testbed/build/testbed/ubuntu/jammy/testbed-0.0.1/usr/lib/testbed/app
- /home/runner/work/toga/toga/testbed/build/testbed/ubuntu/jammy/testbed-0.0.1/usr/lib/testbed/app_packages
Configure argc/argv...
Initializing Python runtime...
Running app module: tests.testbed
---------------------------------------------------------------------------
Application quit abnormally (Exit code -6)!
Traceback (most recent call last):
  File "/usr/lib/python3.10/runpy.py", line 196, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "/usr/lib/python3.10/runpy.py", line 86, in _run_code
    exec(code, run_globals)
  File "/home/runner/work/toga/toga/testbed/build/testbed/ubuntu/jammy/testbed-0.0.1/usr/lib/testbed/app/tests/testbed.py", line 9, in <module>
    import coverage
ModuleNotFoundError: No module named 'coverage'

Trying to understand now why the test_requires packages seemingly aren't being installed....

@rmartin16
Copy link
Member

Ohh...user error 🤦🏼‍♂️. I was calling briefcase build linux separately from briefcase run linux --test....so build didn't install the test_requires packages and run just ran what was already built.

Anyway, using unset LD_LIBRARY_PATH before the briefcase run linux --test was enough to get things working.

@rmartin16
Copy link
Member

rmartin16 commented Mar 13, 2023

Also, since you're in CI...I discovered this little snippet to help make CI a little more efficient:

concurrency:
  group: ${{ github.ref }}
  cancel-in-progress: true

If you add that to the top level, it'll automatically cancel any current CI runs for a PR if a new run is triggered. Useful for quick successive pushes to a branch.

@freakboy3742
Copy link
Member

Trying to understand now why the test_requires packages seemingly aren't being installed....

If you ran the app previously in non-test mode, it won't notice that dependencies have been "updated". A -r pass will fix that.

@mhsmith
Copy link
Member Author

mhsmith commented Mar 14, 2023

Anyway, using unset LD_LIBRARY_PATH before the briefcase run linux --test was enough to get things working.

Thanks very much. I don't know why LD_LIBRARY_PATH would be causing a problem with the setup-python-installed Python, but on further thought, we should be using the Ubuntu system Python package anyway, so I've changed the CI to do that instead.

@mhsmith
Copy link
Member Author

mhsmith commented Mar 14, 2023

I've also changed the system package list slightly, and made it identical in both the CI and the tutorial.

I had to add python3-cairo-dev to deal with this error:

      Package py3cairo was not found in the pkg-config search path.
      Perhaps you should add the directory containing `py3cairo.pc'
      to the PKG_CONFIG_PATH environment variable
      No package 'py3cairo' found
      Command '('pkg-config', '--print-errors', '--exists', 'py3cairo >= 1.16.0')' returned non-zero exit status 1.

And I removed the following redundant requirements:

  • python3-gi is required by python3-gi-cairo
  • gir1.2-gtk-3.0 and libwebkit2gtk-4.0-37 are required by gir1.2-webkit2-4.0

Copy link
Member

@freakboy3742 freakboy3742 left a comment

Choose a reason for hiding this comment

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

A couple of notes inline, but mostly makes sense.

The only change that strikes me as weird is the python-cairo-dev dependency - CI is installing pycairo from a wheel, which I thought would mean that we wouldn't need a system package for Cairo, just the runtime one.

runs-on: ubuntu-latest
pre-command: "sudo apt-get update -y && sudo apt-get install -y python3-gi python3-gi-cairo gir1.2-gtk-3.0 python3-dev libgirepository1.0-dev libcairo2-dev pkg-config libfuse2"
runs-on: ubuntu-22.04
# The package list should be the same as in tutorial-0.rst.
Copy link
Member

Choose a reason for hiding this comment

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

This list also exists in Toga README, and the BeeWare tutorial (docs.beeware.org) step 0. AFAICT, all three should be in sync.

Copy link
Member Author

Choose a reason for hiding this comment

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

OK, I've made the README reference the Toga tutorial, and created beeware/beeware#196 for the BeeWare tutorial.

pre-command: "sudo apt-get update -y && sudo apt-get install -y python3-gi python3-gi-cairo gir1.2-gtk-3.0 python3-dev libgirepository1.0-dev libcairo2-dev pkg-config libfuse2"
runs-on: ubuntu-22.04
# The package list should be the same as in tutorial-0.rst.
pre-command: "sudo apt-get update -y && sudo apt-get install -y python3-dev python3-cairo-dev python3-gi-cairo libgirepository1.0-dev libcairo2-dev libpango1.0-dev gir1.2-webkit2-4.0 pkg-config libfuse2"
Copy link
Member

Choose a reason for hiding this comment

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

I'm fairly certain libfuse2 was only needed because we were building AppImages - It should be possible to drop it now that we're testing system packages.

Copy link
Member Author

Choose a reason for hiding this comment

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

OK, done.

@@ -74,7 +77,7 @@ Next, install Toga into your virtual environment:

# Ubuntu 18.04+ / Debian 10+
(venv) $ sudo apt-get update
(venv) $ sudo apt-get install python3-dev python3-gi python3-gi-cairo libgirepository1.0-dev libcairo2-dev libpango1.0-dev libwebkit2gtk-4.0-37 gir1.2-webkit2-4.0
(venv) $ sudo apt-get install python3-dev python3-cairo-dev python3-gi-cairo libgirepository1.0-dev libcairo2-dev libpango1.0-dev gir1.2-webkit2-4.0 pkg-config libfuse2
Copy link
Member

Choose a reason for hiding this comment

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

As above, syncing with other sources, without libfuse2.

@mhsmith
Copy link
Member Author

mhsmith commented Mar 14, 2023

The only change that strikes me as weird is the python-cairo-dev dependency - CI is installing pycairo from a wheel, which I thought would mean that we wouldn't need a system package for Cairo, just the runtime one.

The failure (full log) was actually when building pygobject. I'm not sure why it failed here when it was working on the last merge to the main branch): maybe because we moved from Python 3.11 to 3.10, or from setup-python to the system Python package. But in this case, I actually could reproduce the error locally, so it looks like it's a genuine requirement.

@rmartin16
Copy link
Member

rmartin16 commented Mar 14, 2023

The only change that strikes me as weird is the python-cairo-dev dependency - CI is installing pycairo from a wheel, which I thought would mean that we wouldn't need a system package for Cairo, just the runtime one.

The failure (full log) was actually when building pygobject. I'm not sure why it failed here when it was working on the last merge to the main branch): maybe because we moved from Python 3.11 to 3.10, or from setup-python to the system Python package. But in this case, I actually could reproduce the error locally, so it looks like it's a genuine requirement.

FWIW, I tracked this down. The pygobject package pokes around to find pycairo during installation. Since pycairo is listed as a build requirement (as well as a runtime requirement), pip puts it the isolated build environment before trying to build pygobject. Normally, this is where pygobject will find pycairo.

However, when you're using the System Python, then /usr/lib/python3/dist-packages comes in to play. Since we're installing python3-gi-cairo, the use of importlib.utils.find_spec() returns /usr/lib/python3/dist-packages/cairo. However, this is a minimal install and doesn't include the header file. When python3-gi-cairo isn't installed, the pycairo that was installed in to the pip build environment for pygobject is found and everything successfully builds.

Furthermore, I was able to run the testbed CI as well as run the testbed app locally without installing python3-gi-cairo (or python3-cairo-dev).

Copy link
Member

@freakboy3742 freakboy3742 left a comment

Choose a reason for hiding this comment

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

I've made a small tweak to the README markup adding a link, rather than a raw path; but otherwise 👍

@freakboy3742 freakboy3742 merged commit 73ace89 into beeware:main Mar 15, 2023
@rmartin16
Copy link
Member

Anyway, using unset LD_LIBRARY_PATH before the briefcase run linux --test was enough to get things working.

Thanks very much. I don't know why LD_LIBRARY_PATH would be causing a problem with the setup-python-installed Python, but on further thought, we should be using the Ubuntu system Python package anyway, so I've changed the CI to do that instead.

Closing this loop....when actions/setup-python installs Python, it sets several env vars including LD_LIBRARY_PATH. So, when the stub runs, it pulls the Python library from setup-python instead of the /usr/lib libraries it was built against. I'm not sure of all the expectations in this situation but avoiding it did resolve the issue for me.

The actions/setup-python action supports the update-environment boolean input to disable taking over an environment like this.....although, avoiding it altogether makes more sense here.

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.

3 participants