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

xwayland-run: init at 0.0.2 #273453

Merged
merged 1 commit into from
Dec 19, 2023
Merged

xwayland-run: init at 0.0.2 #273453

merged 1 commit into from
Dec 19, 2023

Conversation

arthsmn
Copy link
Member

@arthsmn arthsmn commented Dec 11, 2023

Description of changes

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/3071

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-already-reviewed/2617/1309

@pbsds
Copy link
Contributor

pbsds commented Dec 14, 2023

It seems the entrypoints need wrapping to properly enter the python environment

$ results/xwayland-run/bin/xwfb-run
Traceback (most recent call last):
  File "/dev/shm/nixpkgs-review/pr-273453/results/xwayland-run/bin/xwfb-run", line 30, in <module>
    from wlheadless.wlheadless_common import WlheadlessCommon
ModuleNotFoundError: No module named 'wlheadless'

Consider buildPythonApplication with pyproject=false instead of stdenv.mkDerivation

@arthsmn
Copy link
Member Author

arthsmn commented Dec 15, 2023

It seems the entrypoints need wrapping to properly enter the python environment

$ results/xwayland-run/bin/xwfb-run
Traceback (most recent call last):
  File "/dev/shm/nixpkgs-review/pr-273453/results/xwayland-run/bin/xwfb-run", line 30, in <module>
    from wlheadless.wlheadless_common import WlheadlessCommon
ModuleNotFoundError: No module named 'wlheadless'

Consider buildPythonApplication with pyproject=false instead of stdenv.mkDerivation

Sorry! Done.

@arthsmn arthsmn requested a review from pbsds December 15, 2023 00:45
Copy link
Contributor

@pbsds pbsds left a comment

Choose a reason for hiding this comment

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

On a headless host i get no output and no side-effects when running a program:

$ /nix/store/798xf6snhdbyi7z7c8s3ism06mxq8fv2-xwayland-run-0.0.2/bin/xwfb-run touch foobar
$ test -f foobar || echo ":("
:(

In strace i can see it handling some error:

read(4, "# subprocess - Subprocesses with"..., 8192) = 8192
read(4, "PollSelector'):\n        _PopenSe"..., 8192) = 8192
read(4, "elf.returncode = returncode\n    "..., 8192) = 8192
read(4, ".\n\n    Prefer an implementation "..., 8192) = 8192
read(4, "      raise ValueError(\"The 'ext"..., 8192) = 8192
read(4, "subprocess exit status creates a"..., 8192) = 8192
read(4, "f _get_handles(self, stdin, stdo"..., 8192) = 8192
read(4, "ot executable:\n                 "..., 8192) = 8192

On a wayland gnome host i get no side-effects and the following error:

Failed to connect to the compositor!

When i run it in cage i get the following output, not sure if relevant:

$ cage results/xwayland-run/bin/xwfb-run touch asd
00:00:00.012 [EGL] command: eglQueryDmaBufModifiersEXT, error: EGL_BAD_PARAMETER (0x300c), message: "EGL_BAD_PARAMETER error: In eglQueryDmaBufModifiersEXT: Invalid format
"
00:00:00.012 [render/egl.c:661] Failed to query dmabuf number of modifiers
00:00:00.012 [EGL] command: eglQueryDmaBufModifiersEXT, error: EGL_BAD_PARAMETER (0x300c), message: "EGL_BAD_PARAMETER error: In eglQueryDmaBufModifiersEXT: Invalid format
"
00:00:00.012 [render/egl.c:661] Failed to query dmabuf number of modifiers
Failed to connect to the compositor!

pkgs/by-name/xw/xwayland-run/package.nix Show resolved Hide resolved
@arthsmn
Copy link
Member Author

arthsmn commented Dec 15, 2023

@pbsds everything should be fine now.

@pbsds
Copy link
Contributor

pbsds commented Dec 17, 2023

Seems to work, but putting both weston, cage and mutter in the closure is quite excessive.
In share/wlheadless/wlheadless.conf it seems weston is the default compositor, and cage and mutter are simply configurable options. Lets remove those

Up for discussion is, should we set cage as the default compositor?

Instead of propagating the runtime dependencies, please wrap the bin/ entrypoints instead.

@arthsmn
Copy link
Member Author

arthsmn commented Dec 17, 2023

Seems to work, but putting both weston, cage and mutter in the closure is quite excessive. In share/wlheadless/wlheadless.conf it seems weston is the default compositor, and cage and mutter are simply configurable options. Lets remove those

Yeah agreed.

Up for discussion is, should we set cage as the default compositor?

I think it's better to keep upstream's defaults.

Instead of propagating the runtime dependencies, please wrap the bin/ entrypoints instead.

I'll take a look tomorrow. Thanks for the suggestion.

@arthsmn
Copy link
Member Author

arthsmn commented Dec 17, 2023

@pbsds wrapped the binaries.

pkgs/by-name/xw/xwayland-run/package.nix Show resolved Hide resolved
pkgs/by-name/xw/xwayland-run/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/xw/xwayland-run/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/xw/xwayland-run/package.nix Outdated Show resolved Hide resolved
@pbsds
Copy link
Contributor

pbsds commented Dec 18, 2023

It seems xwfb-run hides this error, going by the strace, but i managed to invoke it such that we can see it:

[nix-shell:/dev/shm/nixpkgs-review/pr-273453-2]$ results/xwayland-run/bin/wlheadless-run -- results/xwayland-run/bin/xwayland-run -- env
...
Traceback (most recent call last):
  File "/nix/store/78l48rcbzmjnwiv29mj2pz3s6rbgyk73-xwayland-run-0.0.2/bin/..xwayland-run-wrapped-wrapped", line 70, in <module>
    if xwayland.create_xauth_entry() != 0:
       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/nix/store/78l48rcbzmjnwiv29mj2pz3s6rbgyk73-xwayland-run-0.0.2/lib/python3.11/site-packages/wlheadless/xwayland.py", line 108, in create_xauth_entry
    with subprocess.Popen(xauth_command_line, stdin = subprocess.PIPE) as proc:
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/nix/store/5k91mg4qjylxbfvrv748smfh51ppjq0g-python3-3.11.6/lib/python3.11/subprocess.py", line 1026, in __init__
    self._execute_child(args, executable, preexec_fn, close_fds,
  File "/nix/store/5k91mg4qjylxbfvrv748smfh51ppjq0g-python3-3.11.6/lib/python3.11/subprocess.py", line 1950, in _execute_child
    raise child_exception_type(errno_num, err_msg, err_filename)
FileNotFoundError: [Errno 2] No such file or directory: 'xauth'

It might need to be wrapped with xorg.xauth

@pbsds
Copy link
Contributor

pbsds commented Dec 18, 2023

This should work headlessly, which can be emulated in a --pure nix shell ;)

@arthsmn
Copy link
Member Author

arthsmn commented Dec 18, 2023

Fixed.

Copy link
Contributor

@pbsds pbsds left a comment

Choose a reason for hiding this comment

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

Result of nixpkgs-review pr 273453 run on x86_64-linux 1

2 packages built:
  • xwayland-run
  • xwayland-run.man
  • package path fits guidelines
  • package name fits guidelines
  • package version fits guidelines
  • package builds on x86_64-linux
  • executables tested on x86_64-linux
  • meta.description is set and fits guidelines
  • meta.license fits upstream license
  • meta.platforms is set
  • meta.maintainers is set
  • build time only dependencies are declared in nativeBuildInputs
  • source is fetched using the appropriate function
  • the list of phases is not overridden
  • when a phase (like installPhase) is overridden it starts with runHook preInstall and ends with runHook postInstall.
  • patches have a comment describing either the upstream URL or a reason why the patch wasn't upstreamed
  • patches that are remotely available are fetched rather than vendored

@pbsds pbsds merged commit 3204173 into NixOS:master Dec 19, 2023
22 checks passed
@arthsmn
Copy link
Member Author

arthsmn commented Dec 19, 2023

@pbsds thanks for all your help, learned a lot here!

@arthsmn arthsmn deleted the xwayland-run branch December 26, 2023 17:17
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

6 participants