feat: add herbstluftwm

[tlambert03]

this closes #24 ...
opening for testing, but we still need to discuss whether it's ok or not

[tlambert03]

Can confirm that this branch fixes the test timeout that prompted this issue: https://github.com/pyapp-kit/superqt/actions/runs/12485746657/job/34844970778

[larsoner]

I have never used it. Maybe we should add e new option wm: false | herblustwm with default false for backward compat? But if it's expected just to work better in all cases then I think it's okay to have the default be herblustwm.

[tlambert03]

Yep, having an option makes total sense. I think it should be generally invisible (and perhaps true could be default), but i should check this for pyvista and antthing else you'd want me to test before we do so

[akaszynski]

Maybe we should add e new option wm: false | herblustwm with default false for backward compat?

IMO it's always best to add options whenever possible for backwards compatibility.

Testing this in pyvista/pyvistaqt#669

[akaszynski]

Testing this in pyvista/pyvistaqt#669

PR had no issues. Still would like to have the option to disable this, but we can leave it on by default.

[tlambert03]

Thanks for testing, I'll definitely add an option (but it will likely be a few days, so this can sit for a bit).

[tlambert03]

ok, i updated it to use a wm option, with options false | herblustwm as suggested in #25 (comment) ... however I did set the default to herblustwm, is that ok? (I know this is a relatively frequently used window manager on github actions: https://github.com/search?q=herbstluftwm+path%3Aworkflows

[larsoner]

If it seemed risky enough I could release it as a v4 of the action but it's probably safe enough / bugfix-ey enough just to cut as a v3.2 / v3

[user27182]

I think this PR is the only change from v3.1 to v3.2, and may be the cause of #33

I think this went unnoticed in the pyvista repo due to delays in upgrade from v2 to v3 caused by VTK 9.4 compatibility issues which were only resolved recently.

What's the suggested fix to disable this option? Do we have to set an env var to false?

[tlambert03]

See readme for options https://github.com/pyvista/setup-headless-display-action?tab=readme-ov-file#options

set wm to false if you want to disable it

[user27182]

Thanks, disabling wm worked.

Is having this option enabled by default still desirable if it causes PyVista tests to fail and seg fault? Should we at least add a warning so downstream users are aware that this option may cause this?

[tlambert03]

Is having this option enabled by default still desirable if it causes PyVista tests to fail and seg fault?

I can’t say for sure of course , but my suspicion is that the segfault indicates an actual problem with either the test or the widget chain that having a window manager just reveals. Have you tried running that test on a real Ubuntu computer that actually has a window manager (as opposed to the artificially minimal ci runners?).

the difficulty here is that not having a window manager can also cause tests to fail via timeout (see #24 for the original motivation).

Ultimately, I’m happy to do whatever @larsoner and others want to do. So if you all want to change the default, that’s fine… but my suspicion is that the error you’re seeing is likely actually revealing something, and not just a problem with the window manager itself

[larsoner]

I think I had to set that arg to avoid a problem on a repo or two as well (probably ones that used pyvista / pyvistaqt), might be worth reverting it and cutting a 3.3 and 4.1

[tlambert03]

by reverting it, you mean changing the default right? That's fine with me, but I would greatly appreciate having the option (it's mandatory for many interaction tests)

[larsoner]

Yeah I just mean changing the default!

[tlambert03]

Fine by me 👍