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

lib: Supporting building for wasm32-wasip1 #4519

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Earthmark
Copy link

It's really strange not putting a description of changes in this message.

This is my first set of JJ managed changes, I hope this works.

Copy link
Member

@martinvonz martinvonz left a comment

Choose a reason for hiding this comment

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

Thanks. I suppose it would make sense to add this build to CI. Do you want to follow up with that?

Copy link
Member

@martinvonz martinvonz left a comment

Choose a reason for hiding this comment

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

If anyone here knows more about WebAssembly (more than ~nothing, that is), please review this. Otherwise I think we can merge this in a day or so.

@PhilipMetzger
Copy link
Contributor

Can you please adjust the commit message to lib: Supporting building for wasm32-wasip1, see https://github.com/martinvonz/jj/blob/main/docs/contributing.md#code-reviews. And I'd like to know the motivation behind supporting it, if there are any. And maybe it'd be nice to coordinate the changes with #4478, since that PR is a major rework for lib/src/local_working_copy.rs?

@Earthmark Earthmark force-pushed the earthmark/lib-wasi@origin branch from 805ae84 to ed154fa Compare September 22, 2024 18:05
@Earthmark
Copy link
Author

Earthmark commented Sep 22, 2024

Oh something went wrong, all the files in the project got modified. I think they got mangled newline wise (my dev setup is a bit strange)

Copy link
Member

@thoughtpolice thoughtpolice left a comment

Choose a reason for hiding this comment

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

Seems fine, but let's add a build matrix entry too. I suspect we'll probably need to remove git support for that to work? I would be surprised if libgit2 and the needed deps all build cleanly.

If we could actually build the CLI without git support, we would in theory throw away most of the external deps, and be able to write a WASI backend that used the browser filesystem APIs. Then we could host a version of JJ in the browser as an interactive tutorial. :)

lib/src/local_working_copy.rs Show resolved Hide resolved
@Earthmark Earthmark force-pushed the earthmark/lib-wasi@origin branch from ed154fa to b5abb8d Compare September 22, 2024 18:10
@Earthmark Earthmark changed the title Fixed lib building for wasm32-wasip1 lib: Supporting building for wasm32-wasip1 Sep 22, 2024
@Earthmark
Copy link
Author

Thanks. I suppose it would make sense to add this build to CI. Do you want to follow up with that?

Step added, it should verify lib builds for wasi.

@Earthmark
Copy link
Author

This should probably wait until #4478 goes through, I feel that change is likely a higher precedence and it'd be easy to rebuild these changes.

@Earthmark
Copy link
Author

Can you please adjust the commit message to lib: Supporting building for wasm32-wasip1, see https://github.com/martinvonz/jj/blob/main/docs/contributing.md#code-reviews. And I'd like to know the motivation behind supporting it, if there are any. And maybe it'd be nice to coordinate the changes with #4478, since that PR is a major rework for lib/src/local_working_copy.rs?

Commit message adjusted.

The use case for this is building out very trivial vfs-wasi support to https://github.com/RobertBaruch/dergwasm to represent entities in the game world as entries in a filesystem, and running jj as a wasm executable to version objects inside the VR environment, from inside the environment itself.

@thoughtpolice 's use case of a web tool on an example filesystem is a much more realizeable use case.

I said this in another comment, but I feel #4478 should probably go through first, as it's a much more complex change.

@Earthmark Earthmark force-pushed the earthmark/lib-wasi@origin branch from b5abb8d to 13102f1 Compare September 22, 2024 19:17
@Earthmark
Copy link
Author

Turns out the executable bit of every file got set on accident.

My main machine is windows, but I worked on this repo in a linux dev container with the folder mounted into the container. I'm pretty sure it exposes mounted windows files as 755, which got committed to the repo by the dev container. In my initial change I ran jj from the windows env, for the follow up I ran it in the container itself, which is why the issue only came up in the follow up.

@martinvonz
Copy link
Member

Oops :) Sounds like #4478 will be useful for you too then.

This is adapting the FS apis in `lib` for wasi (excluding git support).

The compilation command for building this is:
`cargo +nightly build --no-default-features --target wasm32-wasip1`

* Nightly is needed for `std::os::wasi::fs::symlink_path`, if symlink support isn't needed then this can be non-nightly.
* No default features is needed to not compile gitoxide, as that doesn't yet support wasi.

This does not adapt the CLI, just lib.
@Earthmark Earthmark force-pushed the earthmark/lib-wasi@origin branch from 13102f1 to e5bc813 Compare September 22, 2024 19:38
@PhilipMetzger
Copy link
Contributor

Can you please adjust the commit message to lib: Supporting building for wasm32-wasip1, see https://github.com/martinvonz/jj/blob/main/docs/contributing.md#code-reviews. And I'd like to know the motivation behind supporting it, if there are any. And maybe it'd be nice to coordinate the changes with #4478, since that PR is a major rework for lib/src/local_working_copy.rs?

Commit message adjusted.

Thanks.

The use case for this is building out very trivial vfs-wasi support to https://github.com/RobertBaruch/dergwasm to represent entities in the game world as entries in a filesystem, and running jj as a wasm executable to version objects inside the VR environment, from inside the environment itself.

That sounds like a really interesting idea, although I think that just having the auto-snapshotting mechanism could be enough for you.

Thank you for enabling the support for it.

@Earthmark
Copy link
Author

The ability to restructure changes is very useful as well. I'm happy to talk about it over discord if you're curious!

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

Successfully merging this pull request may close these issues.

4 participants