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

Support build / run under wasm / gopherjs #15

Merged
merged 1 commit into from Apr 21, 2021

Conversation

paralin
Copy link
Contributor

@paralin paralin commented Apr 19, 2021

image

Uses a in-memory tree when osfs would otherwise be used.

Signed-off-by: Christian Stewart <christian@paral.in>
@@ -0,0 +1,23 @@
package osfs
Copy link
Member

Choose a reason for hiding this comment

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

Since in go-git, you can open and clone a repository declaring the kind of fs, I rather having this implementation as an implementation that returns "Not Implemented" errors for every method or one that simply panics if you call to os.New()

@paralin
Copy link
Contributor Author

paralin commented Apr 20, 2021

The bug / compile failure this is fixing is this:

paralin@forge go-git % GOOS=js GOARCH=wasm go build -v
package github.com/go-git/go-git/v5
        imports github.com/go-git/go-billy/v5/osfs
        imports golang.org/x/sys/unix: build constraints exclude all Go files in /home/paralin/go/pkg/mod/golang.org/x/sys@v0.0.0-20210415045647-66c3f260301c/unix

You can't import x/sys/unix without causing issues in wasm due to the build constraints.

In any case, it's necessary to exclude these from being imported on js, and there's already conditional platform files for all the other platforms, so why not js also?

If you have a example in go-git (which is what I ran above) and it doesn't work the same way in js without adding additional conditional flags, or doesn't compile at all, or unexpectedly throws errors, why is that better than just using a memfs to shim it?

@mcuadros mcuadros force-pushed the master branch 6 times, most recently from 609712c to 3d7f61f Compare April 21, 2021 23:20
@mcuadros
Copy link
Member

Ideally, we should use https://github.com/WebAssembly/wasi-filesystem in the feature but since wasm_exec.js doesn't make use of this, this looks like the best solution.

@mcuadros mcuadros merged commit 8d1f328 into go-git:master Apr 21, 2021
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.

None yet

2 participants