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

Wlr 0.17 #4752

Merged
merged 58 commits into from
May 19, 2024
Merged

Wlr 0.17 #4752

merged 58 commits into from
May 19, 2024

Conversation

jwijenbergh
Copy link
Contributor

@jwijenbergh jwijenbergh commented Apr 6, 2024

This patch adds wlroots 0.17 support

@jwijenbergh jwijenbergh marked this pull request as draft April 6, 2024 15:01
@jwijenbergh jwijenbergh force-pushed the wlr-0.17 branch 3 times, most recently from b1b8dbb to 1f0fb4e Compare April 6, 2024 15:29
@jwijenbergh jwijenbergh force-pushed the wlr-0.17 branch 5 times, most recently from 3803719 to 92d8b99 Compare May 12, 2024 13:41
@elParaguayo
Copy link
Member

@tych0 @ramnes Tagging you both here to say @jwijenbergh is hoping to have this ready to merge by the weekend.

Arch are looking to package pywlroots 0.17 which was just released but are happy to hold off for a bit while we don't support it. So, I'd suggest we do next release once this PR is merged.

@tych0
Copy link
Member

tych0 commented May 13, 2024

Sure, I will have some time to do a release this weekend if we land it by then.

@tych0
Copy link
Member

tych0 commented May 18, 2024

If you think it's the same failure as before and you're otherwise happy with the branch, I'd say go ahead and merge it and let's do a release.

@jwijenbergh
Copy link
Contributor Author

If you think it's the same failure as before and you're otherwise happy with the branch, I'd say go ahead and merge it and let's do a release.

It seems like it's working now after bumping pywayland, let me wait for full CI

@jwijenbergh jwijenbergh marked this pull request as ready for review May 18, 2024 21:38
@jwijenbergh
Copy link
Contributor Author

jwijenbergh commented May 18, 2024

Should be ready now, bit difficult to test everything but I have been daily driving this for a while without any real issues. There are some small TODOs for upcoming releases though, will make issues for those

Note: I have not tested the release workflow changes, but those are the same on pywlroots where they did work. Maybe the release workflow now doesn't work because of pypy?

@jwijenbergh
Copy link
Contributor Author

jwijenbergh commented May 18, 2024

release workflow is failing (cp3.10 build wheels):

image

I don't understand

@jwijenbergh jwijenbergh marked this pull request as draft May 18, 2024 22:27
@jwijenbergh
Copy link
Contributor Author

jwijenbergh commented May 18, 2024

caused by:

Traceback (most recent call last):
  File "/__w/qtile/qtile/builder.py", line 50, in build_wheel
    import wlroots  # noqa: F401
    ^^^^^^^^^^^^^^
  File "/tmp/build-env-uwvjz0ui/lib/python3.12/site-packages/wlroots/__init__.py", line 8, in <module>
    from ._ffi import ffi, lib
ImportError: while loading wlroots._ffi: failed to import ffi, lib from xkbcommon._ffi

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/opt/python/cp312-cp312/lib/python3.12/site-packages/pyproject_hooks/_in_process/_in_process.py", line 373, in <module>
    main()
  File "/opt/python/cp312-cp312/lib/python3.12/site-packages/pyproject_hooks/_in_process/_in_process.py", line 357, in main
    json_out["return_val"] = hook(**hook_input["kwargs"])
                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/python/cp312-cp312/lib/python3.12/site-packages/pyproject_hooks/_in_process/_in_process.py", line 271, in build_wheel
    return _build_backend().build_wheel(
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/__w/qtile/qtile/builder.py", line 52, in build_wheel
    import wlroots
  File "/tmp/build-env-uwvjz0ui/lib/python3.12/site-packages/wlroots/__init__.py", line 8, in <module>
    from ._ffi import ffi, lib
ImportError: while loading wlroots._ffi: failed to import ffi, lib from xkbcommon._ffi

@tych0
Copy link
Member

tych0 commented May 19, 2024

This is the thing where you need to run ffibuild, right? But also, I have not tested the release scripts either. I guess this is from the test pypi upload job?

@jwijenbergh
Copy link
Contributor Author

This is the thing where you need to run ffibuild, right? But also, I have not tested the release scripts either. I guess this is from the test pypi upload job?

It's this step https://github.com/jwijenbergh/qtile/blob/141da0adcab6dfe6328e908f18198fbcb4b57ad9/.github/workflows/release.yml#L202, haven't changed anything there

@tych0
Copy link
Member

tych0 commented May 19, 2024

That almost looks to me like the ffi build for xkbcommon isn't getting run as part of the wheel build... I have never really understood how all that fits together, and a brief look didn't reveal anything. I'll take a look later.

How are you actually running this? it looks like on a successful run, it will really publish the stuff to pypi given that it's not the test job that's failing, but the real one.

@elParaguayo
Copy link
Member

The issue seems to be the 1.0 release of xkbcommon. I changed the release workflow to this and it worked:

python -m pip install cffi build auditwheel cairocffi xcffib pywayland setuptools-scm setuptools wheel
python -m pip install xkbcommon==0.8
python -m pip install --no-binary :all: pywlroots
python -m build -v --config-setting backend=wayland --no-isolation --wheel .

I think the --no-binary is not necessary.

@jwijenbergh
Copy link
Contributor Author

That almost looks to me like the ffi build for xkbcommon isn't getting run as part of the wheel build... I have never really understood how all that fits together, and a brief look didn't reveal anything. I'll take a look later.

How are you actually running this? it looks like on a successful run, it will really publish the stuff to pypi given that it's not the test job that's failing, but the real one.

I am running this by creating a release in my fork. So I expect the upload to fail but not the build

@jwijenbergh
Copy link
Contributor Author

jwijenbergh commented May 19, 2024

Ok. The problem is that the manylinux containers do not have a recent enough xkbcommon: https://almalinux.pkgs.org/8/almalinux-powertools-x86_64/libxkbcommon-x11-devel-0.9.1-1.el8.x86_64.rpm.html (needs 1.0)

Almalinux 8 (quay.io/pypa/manylinux_2_28_x86_64 container) only comes with xkbcommon 0.9 it seems.
Now that python-xkbcommon 1.0 is released (which needs xkbcommon >= 1.0),
let's build xkbcommon from source such that a xkbcommon._ffi import still works.
@jwijenbergh
Copy link
Contributor Author

jwijenbergh commented May 19, 2024

xkbcommon is now built from source & I disabled pypy builds completely. Now all good

image

@jwijenbergh jwijenbergh marked this pull request as ready for review May 19, 2024 18:09
Copy link
Member

@elParaguayo elParaguayo left a comment

Choose a reason for hiding this comment

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

Great work in getting this done!

python-version: [pypy-3.10, '3.10', '3.11', '3.12']
# TODO: Add back pypy here and in release.yml once:
# https://github.com/pypy/pypy/issues/4956 is resolved
python-version: ['3.10', '3.11', '3.12']
Copy link
Member

Choose a reason for hiding this comment

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

Are we all agreed that we're now dropping pypy 3.10 (as opposed to just allowing it to fail)?

No objections from me!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My thought process was that for releases it would be nice that CI doesn't fail. After the release we can re-enable it

Copy link
Member

Choose a reason for hiding this comment

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

Fine with me, though can you rebase and drop the continue-on-error I just added as well?

Copy link
Member

Choose a reason for hiding this comment

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

If we do get some movement on that issue, we can always drop the note from the CHANGELOG and re-add pypy to CI.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, derp. You had it!

@tych0 tych0 merged commit bf5f820 into qtile:master May 19, 2024
17 checks passed
@elParaguayo
Copy link
Member

elParaguayo commented May 19, 2024

OK - this breaks a fair bit of stuff on qtile-extras. I'll need to work through changes to get it working!

Edit: nowhere near as bad as I feared. All sorted!

@tych0
Copy link
Member

tych0 commented May 19, 2024

Edit: nowhere near as bad as I feared. All sorted!

heh, did qtile migrate help you out? :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants