zellij-org / zellij

A terminal workspace with batteries included
https://zellij.dev
MIT License
21.09k stars 643 forks source link

Feature: Support termux #1021

Closed dead10ck closed 1 year ago

dead10ck commented 2 years ago

It would be awesome if you could run Zellij in Termux on Android. Currently it fails to build because wasmer doesn't support Android. There's an upstream issue at https://github.com/wasmerio/wasmer/issues/2099 but I thought it would be good to track it here too.

I also wonder if it would be feasible to add a Cargo feature to make the whole plugin system optional. I'm not super familiar with zellij yet, so I'm not sure how much critical functionality comes from the plugins.

jaeheonji commented 2 years ago

I'm not super familiar with zellij yet, so I'm not sure how much critical functionality comes from the plugins.

Currently, plugins only provide optional functions. Most are only related to UI, so they are not involved in the overall function.

I also wonder if it would be feasible to add a Cargo feature to make the whole plugin system optional.

It is theoretically possible. However, this is not the main goal at this point, and it seems that this work will require a lot of verification and modification.

Hope it helps 😄

xJonathanLEI commented 1 year ago

Unfortunately it looks like the upstream issue has been set to low priority. Would love to daily drive with Zellij, but this forces me to use tmux locally and only use Zellij on the remote. The context switch is a bit painful tbh.

xJonathanLEI commented 1 year ago

~Wait maybe this is worth looking into? Looks like the Termux guys actually made it work:~

~https://github.com/termux/termux-packages/pull/7982~

Nvm I overlooked the part where it says ARM builds are not supported yet. Sorry for spamming.

xJonathanLEI commented 1 year ago

Sorry for spamming again. It looks like wasmer 2.3.0 works on Termux. I just tried building it and it works:

$ rustc- vV
rustc 1.64.0
binary: rustc
commit-hash: unknown
commit-date: unknown
host: aarch64-linux-android
release: 1.64.0
LLVM version: 15.0.1

In fact, wasmer v2.3.0 is in the Termux package registry with no patch needed.

Zellij uses wasmer v1 though. Maybe we can migrate to v2 and then it will automatically work on Termux?

Edit:

Obviously migrating to wasmer v2 itself is gonna be a huge task. Other than wasmer, the termwiz crate is also having issues thanks to the nix dependency not supporting Termux.

xJonathanLEI commented 1 year ago

Actually, I just upgraded wasmer to v2.3.0 in #1833.

xJonathanLEI commented 1 year ago

As mentioned in https://github.com/zellij-org/zellij/pull/1833#issuecomment-1292919450, 2 dependencies are broken for building on Android:

After tackling wasmer in #1833 (though it's not merging anytime soon), I just submitted https://github.com/wez/wezterm/pull/2694 for fixing the termwiz error too. After this we should have resolved all the build errors, and can focus on resolving runtime errors instead :)

xJonathanLEI commented 1 year ago

Guys I made it.

Screenshot_20221102_224004_Termux

Turns out it just works after resolving dependencies and a temp dir issue inside the codebase.

@imsnif Can we merge Android support (after I clean things up of course) behind a feature flag that's disabled by default now that I got it working? Sort of like an experimental feature style.

imsnif commented 1 year ago

Exciting! Nice job, @xJonathanLEI !! I'm a little hesitant about merging unstable stuff into main. We've done this in the recent past and refactoring past it was a bit of a pain. What does this change entail? A wasmer upgrade is unfortunately a deal-breaker for me at the moment.

xJonathanLEI commented 1 year ago

Just submitted #1896 for the termwiz fix.

What does this change entail?

To make this work I've done the following:

https://github.com/zellij-org/zellij/blob/4ab04c59a246e5ddea9fe8cb36379073c07c219f/zellij-utils/src/consts.rs#L59

We should be able to safely merge items 2 and 3, so it's really down to wasmer.

I know I might sound annoying at this point, but please hear me out lol.

I understand the general concern over the refactoring burden imposed by unstable code, even on gated features. However, looking at #1833, the only thing in the actual Rust code that would need a cfg treatment is the removal of a single line of #[typetag::serde]. We can just make this attribute use a cfg_attr and that's literally all we need to change on the Rust side.

So we can add a wasmer2 feature, which when enabled, works on Android, but it's disabled by default, and there's only one line of Rust code affected. This also encourages people other than Android users to test out using wasmer v2 (since it's just a --features wasmer2 away). What do you think?

Apart from these, there is still one minor issue. I haven't figured out how to make the wasm-wasi target work on Android. So I had to compile the plugins on Linux and then copy over for them to be used on Android. aka. cargo make run does not work for now. I think this is a rather minor thing that we can figure out down the line, once we get the core stuff working (even behind a feature gate).

@imsnif FYI

xJonathanLEI commented 1 year ago

Another screenshot showcasing Zellij running on Android:

Screenshot_20221103_013612_Termux

Runs very smoothly both on phone mode and when docked to a monitor.

imsnif commented 1 year ago

@xJonathanLEI - first, you are not annoying at all. Don't sweat it. I'm very happy to see this work being done and would like to support it.

The issue I have as the main maintainer here is that as soon as this is merged into the codebase, I am liable for it and have to support it. All the good-will of contributors won't change that, and while I'm happy to see you consistently making progress on this - I cannot rely on you sticking around to support this in 6 months, a year or two years. Nothing personal, I've just been burnt by this too many times.

I'll tell you what: it seems like you still have some work to stabilize this on your end. How about you continue this work, and meanwhile I'll try to prioritize the wasmer upgrade as much as possible. I still expect it to take upwards of a few weeks or even a couple of months, since I need to eat away at certain pieces of technical debt (namely stabilizing some parts of the layout system).

I think this is a good compromise and will help me gain confidence in this change and your continued support of it. Sounds fair?

xJonathanLEI commented 1 year ago

Absolutely! For the wasmer upgrade I guess I will simply leave it on my own fork. It should be very easy to rebase anyways given the minimal changes. The other two PRs should be safe to merge I guess.

Meanwhile I'll try to work out the rough edges regarding plugin compilation, and daily drive with it both on Linux and Android.

xJonathanLEI commented 1 year ago

For anyone who stumbles across this thread looking for Android support, here's what you can already do today if you don't mind installing from a fork:

cargo install --locked --git https://github.com/xJonathanLEI/zellij --branch android zellij

There you go, a working zellij installation that works out of the box.

Do note that due to how zellij releases its plugin artifacts, you might get a plugin version mismatch error if you've installed zellij before (obviously depending on how you installed it and the exact version). In that case, delete the ${HOME}/.local/share/zellij folder and try again.

In fact, I recommend deleting that folder every time after you do that install command above, not just when you encounter an error. This is because Zellij will simply use the existing plugins sitting in that folder instead of the latest ones it's built with.

This android branch on my fork is just mixing the 3 pending PRs plus a plugin rebuild commit (since cargo install doesn't rebuild them, obviously). That branch will be constantly rebased on the upstream main branch. This is currently done manually, but I will set up a GitHub Actions workflow to automate that soon, making sure the android branch is always up to date.

Update: workflow already set up. Now the android branch is automatically updated once every day.

dead10ck commented 1 year ago

Thanks for your work on this @xJonathanLEI!

xJonathanLEI commented 1 year ago

Update: the Termux package registry PR has been merged: https://github.com/termux/termux-packages/pull/12997

Termux users can now simply do:

$ pkg add zellij

For getting the latest version of Zellij. Obviously this depends on how up-to-date your mirror is. Some mirrors update more frequently than others.

Note that it only works for aarch64 and x86_64 platforms at the moment.

I guess we can close this issue ahead of the wasmer bump given how the package already sits inside the official Termux registry? We probably want to update the installation instructions before closing though.

dead10ck commented 1 year ago

Awesome, thanks so much for putting that together @xJonathanLEI! However, I do think it makes sense to keep this issue open until the patches are upstreamed. I'm glad they've gotten as small as they have, but they are still significant, in my opinion.

imsnif commented 1 year ago

I also want to echo @dead10ck - awesome work @xJonathanLEI ! I'm very happy to see this coming together and think we'd be able to upstream all the changes very soon.

xJonathanLEI commented 1 year ago

With #1934 merged, all patches have been upstreamed and Zellij works well on Termux now. I think it can use a patch on the hard-coded /etc/ path, but that would require a runtime Termux detection (it's only possible to check for android at compile time, not Termux specifically without some hacks) so it's not very idiomatic. Plus it seems to have not caused any issues without the patch anyways. So I guess we're good.

That said, as mentioned above, right now it's still not possible to build the plugins on Android. Android users who want to install from source would be forced to build the plugins on a supported arch first, copy over, and then install. It's quite cumbersome.

So I'm keeping my fork until this resolves. Meanwhile, anyone can install the latest main branch with:

cargo install --locked --git https://github.com/xJonathanLEI/zellij --branch android zellij

The fork is automatically updated once a day, powered by a scheduled GitHub Actions workflow.

I think we're good to close this one now?

imsnif commented 1 year ago

Hey @xJonathanLEI - what can we do to forgo the need for your fork? I think it would be valuable for us to support this "officially". Is it only the matter of the plugins? What would we need to do in this repo to make it happen?

xJonathanLEI commented 1 year ago

The only issue about Android right now is if you want to use the latest main branch, there's gonna be a plugin version mismatch (because the package version is bumped immediately after each release, whereas the actual new plugins will only be commited at the next release). So it's more of a compile time dependency.

But I understand that it might not cut it to call it "officially supported" if you can't build the whole thing (including the plugins) on the platform. So I've actually started work to make that happen as well. The first (and might be the only) step would be to add the wasm-wasi target to Termux, as tracked here: https://github.com/termux/termux-packages/issues/13117

After the target is added, I will check whether further steps are needed (but gut sense says no).

imsnif commented 1 year ago

Hmm... would a precompiled binary not cut it? eg. one we can put in our releases? Coming to think of it, will the generic musl binary not work now after the patches?

xJonathanLEI commented 1 year ago

Oh if you're only talking about making releases, then it definitely works! And plugins work perfectly on Android. It's just that you cannot build the plugins on Android. Otherwise it's all good.

I guess after all you don't need the codebase to build on a platform to announce official support. It just has to work.

xJonathanLEI commented 1 year ago

In short, plugins do work on Android. They just don't build.

imsnif commented 1 year ago

Alright - thanks for the clarification. What's the incentive to have it build on Android? Is this required for the termux support, or to be included in their package manager?

xJonathanLEI commented 1 year ago

Oh not much incentive to be honest, now that I think about it. It's just so that you can develop Zellij plugins on Android (you can already develop Zellij itself, except the plugins).

It's not required to be included in their package manage. In fact I already added it in this PR: https://github.com/termux/termux-packages/pull/12997

imsnif commented 1 year ago

But this works off of your branch, right? Can we somehow make it work off of the latest release?

xJonathanLEI commented 1 year ago

Hmm so my fork only exists for those who wanna install what's on the latest main. Normally if you want to install from a branch (instead of official releases), you will have to do: cargo make install instead of a bare cargo install, the make command roughly does this:

My branch simply offloads the plugin building step to GitHub Actions, so that Termux users who want to install from the latest main branch can skip it.

Note that this only applies to installing from a branch. Installing from release commits are fine the plugins are already up to date at that point. Looking at an example:

$ git log -2 v0.33.0
commit c086e33ed36a299dd6acad4554eed55bacff3c9a (tag: v0.33.0)
Author: Aram Drevekenin <aram@poor.dev>
Date:   Thu Nov 10 09:50:57 2022 +0100

    chore(release): v0.33.0

commit d68db715d10c8b5c5abb0f23c1926d596586ca18
Author: Aram Drevekenin <aram@poor.dev>
Date:   Wed Nov 9 23:03:11 2022 +0100

    chore(ci): remove msrv check (#1923)

Running cargo install (not cargo make install) on c086e33ed36a299dd6acad4554eed55bacff3c9a works because both plugins and the program itself are 0.33.0. Running cargo install on d68db715d10c8b5c5abb0f23c1926d596586ca18 doesn't work because plugins are 0.32.0 and the program is 0.33.0 - version mismatch.

So my branch is really just for people who want to install from non-release commits, like d68db715d10c8b5c5abb0f23c1926d596586ca18 here. It's the thing you do only when you want a latest feature on main and don't wanna wait for the next release.

In fact, most users will simply install from the package manager, which already works:

$ pkg add zellij
imsnif commented 1 year ago

So... pkg add zellij takes the latest binary from our release?

xJonathanLEI commented 1 year ago

Nah, everytime a release is made on Zellij's side, I would bump the version and sha here:

https://github.com/termux/termux-packages/blob/dc1ce7041694190120d0722e7c6dcdae75929dff/packages/zellij/build.sh#L5_L7

Upon which, a GitHub Actions workflow would be run on an x86 machine where it cross-compiles Zellij using this script:

https://github.com/termux/termux-packages/blob/dc1ce7041694190120d0722e7c6dcdae75929dff/packages/zellij/build.sh#L14_L34

and uploads the binary to their package registry.

When a user runs pkg add zellij, they download the pre-built binary from there.

So Zellij doesn't have to release a binary for Android, though it totally can.

imsnif commented 1 year ago

Alright - so I guess this makes you the Termux package maintainer similar to how we do with the various distributions :)

imsnif commented 1 year ago

So to sum everything up: this is now supported thanks to @xJonathanLEI 's efforts.

You can now install Zellij on termux with pkg add zellij - thanks to all involved and especially to @xJonathanLEI !!