Skip to content

anbox: unbreak#253146

Merged
rnhmjoj merged 14 commits intoNixOS:masterfrom
rnhmjoj:pr-anbox
Sep 5, 2023
Merged

anbox: unbreak#253146
rnhmjoj merged 14 commits intoNixOS:masterfrom
rnhmjoj:pr-anbox

Conversation

@rnhmjoj
Copy link
Copy Markdown
Contributor

@rnhmjoj rnhmjoj commented Sep 3, 2023

Description of changes

Continuation of PR #125600, which was abandoned.
This brings anbox into a somewhat working state, though it will be eventually removed.
That is because upstream development is dead and newer kernels removed a module needed by anbox.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested via anbox.tests
  • Tested compilation of all packages that depend on this change using (anbox, anbox-postmarketos-image)
  • Tested basic functionality of all binary files
  • 23.11 Release Notes (or backporting 23.05 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.

@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels Sep 3, 2023
@rnhmjoj rnhmjoj mentioned this pull request Sep 3, 2023
14 tasks
@rnhmjoj
Copy link
Copy Markdown
Contributor Author

rnhmjoj commented Sep 3, 2023

I removed the unnecessary kernel and LXC patches and addressed the remaining nitpicks.
Also note that I had to pick the postmarket OS images from an archive because the website is no longer up.

@ofborg ofborg bot added the 8.has: package (new) This PR adds a new package label Sep 3, 2023
@ofborg ofborg bot requested a review from edwtjo September 3, 2023 16:37
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Sep 3, 2023
@Atemu Atemu removed their request for review September 3, 2023 20:10
@samueldr
Copy link
Copy Markdown
Member

samueldr commented Sep 3, 2023

Maybe of interest to you, when I was working on that (I abandoned all that because it didn't work in any usable manner for my personal use-case/opinion) I had built an anbox image using robotnix

YMMV, not sure it all works still.

Comment on lines 121 to 123
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This means it is still racy though...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think the purpose of this patcheset is to get anbox to a more usable state. I wouldn't spend more energy on doing proper things as upstream is dead.

This patch is used actively by postmarketOS, which in turn *just* uses
Alpine Linux packages for anbox.
This helps on slower devices, at the cost of making the command
"resolve" less quickly.

It replaces commands "failing because things weren'd ready", but things
actually still succeeding by commands *possibly* failing in 10× more
time.
The `anbox-application-manager` helper, which is actually broken as it's
not installed as +x, starts "su" by default, which might not be
desirable, depending on the end-user's wishes.
It now *strictly* is a shortcut to launch the application manager.
This fixes two issues

  - store path is hardcoded in desktop files
  - `.anbox-wrapped` is used in desktop files
SDL defaults to reading the executable from the running app

 - https://github.com/libsdl-org/SDL/blob/64724db0a1ffac7c3332df66a942d3ba27b2dc0b/src/video/x11/SDL_x11video.c#L69-L71

Which in turn gives this from xprop:

```
WM_CLASS(STRING) = ".anbox-wrapped", ".anbox-wrapped"
```

Meaning that any special casing a window manager does will be broken.

This will also fallback correctly on Wayland:

 - https://github.com/libsdl-org/SDL/blob/d956636c85aafc055d37153d52370e9b1c2c5929/src/video/wayland/SDL_waylandvideo.c#L93-L99
Currently the windows don't provide icons. It's rather inconvenient for
many reasons.

This patch forwards the icons from the Android application to the
window.

Patch origin: https://fjordtek.com/git/Fincer/anbox-install/commit/89226287860cf8ced3d41868c80cfa216b5237af

Rebased on top of the current tip of anbox master

See also: anbox/anbox#1514
samueldr and others added 3 commits September 5, 2023 10:09
As noted in NixOS#102341 this is not
actually running as a forked process. It only tells the process that it
is running "as a daemon, so shut the warning up".

See `daemon_` here

 - https://github.com/anbox/anbox/blob/9de4e87cdd05135e1c71e6eadb68bf82719cebdf/src/anbox/cmds/container_manager.cpp#L38-L79

It is **strictly** used to hide that message.

Co-authored-by: Matt Votava <mvnetbiz@gmail.com>
@rnhmjoj rnhmjoj merged commit 704c791 into NixOS:master Sep 5, 2023
@vcunat
Copy link
Copy Markdown
Member

vcunat commented Sep 7, 2023

Doesn't download at least for aarch64-linux, apparently? (just FYI, discovered by accident)
https://hydra.nixos.org/build/234208486

@emilylange
Copy link
Copy Markdown
Member

Yeah, I wanted to mention this as well.
This PR has been merged despite a failing ofborg aarch64-linux ci check.

@SuperSandro2000
Copy link
Copy Markdown
Member

That would have been pretty visible if that failure would be red and not grey which github treats as ignoreable when collapsing the CI checks.

@rnhmjoj
Copy link
Copy Markdown
Contributor Author

rnhmjoj commented Sep 10, 2023

The problem is just that build.anbox.io is dead too, only the x86_64 test works because the image has been saved by cache.nixos.org. There's not much I can do besides replacing them with the postmarket OS version: the ARM images aren't even on the internet archive.

Anyway, both x86 and ARM were broken before this PR, so it's not a big deal.

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

Labels

6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants