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

os: invalid tmp dir for GOARCH=wasm, GOOS=js when GOHOSTOS=windows #27306

Closed
thesyncim opened this issue Aug 28, 2018 · 10 comments
Closed

os: invalid tmp dir for GOARCH=wasm, GOOS=js when GOHOSTOS=windows #27306

thesyncim opened this issue Aug 28, 2018 · 10 comments

Comments

@thesyncim
Copy link

@thesyncim thesyncim commented Aug 28, 2018

Please answer these questions before submitting your issue. Thanks!

What did you do?

https://play.golang.org/p/e5sTf5-QdnM
//steps to reproduce (GOHOSTOS= windows)
set GOOS=js
set GOARCH=wasm
set CGO_ENABLED=0
go test -c -o test.wasm
node wasm_exec.js test.wasm

What did you expect to see?

test succeed

What did you see instead?

--- FAIL: TestTmpFileGOARCH_wasm_GOHOSTOS_windows (0.00s)
issue_test.go:27: open /tmp/wasm514931654: No such file or directory

Does this issue reproduce with the latest release (go1.11)?

yes

System details

go version devel +76c45877c9 Tue Aug 28 09:26:45 2018 +0000 windows/amd64
GOARCH="wasm"
GOBIN=""
GOCACHE="C:\Users\thesyncim\AppData\Local\go-build"
GOEXE=".exe"
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="windows"
GOOS="js"
GOPATH="C:\Users\thesyncim\go"
GOPROXY=""
GORACE=""
GOROOT="C:\Go"
GOTMPDIR=""
GOTOOLDIR="C:\Go\pkg\tool\windows_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
GOROOT/bin/go version: go version devel +76c45877c9 Tue Aug 28 09:26:45 2018 +0000 windows/amd64
GOROOT/bin/go tool compile -V: compile version devel +76c45877c9 Tue Aug 28 09:26:45 2018 +0000
gdb --version: GNU gdb (GDB) 7.9.1

possible solution

diff --git a/src/os/file_unix.go b/src/os/file_unix.go
index cb90b70735..4c2a6e3792 100644
--- a/src/os/file_unix.go
+++ b/src/os/file_unix.go
@@ -11,6 +11,7 @@ import (
        "internal/syscall/unix"
        "runtime"
        "syscall"
+       "syscall/js"
 )

 // fixLongPath is a noop on non-Windows platforms.
@@ -330,11 +331,15 @@ func Remove(name string) error {
 func tempDir() string {
        dir := Getenv("TMPDIR")
        if dir == "" {
-               if runtime.GOOS == "android" {
+               switch runtime.GOOS {
+               case "android":
                        dir = "/data/local/tmp"
-               } else {
+               case "js":
+                       dir = js.Global().Get("require").Invoke("os").Call("tmpdir").String()
+               default:
                        dir = "/tmp"
                }
+
        }
        return dir
 }
@thesyncim

This comment has been minimized.

Copy link
Author

@thesyncim thesyncim commented Aug 28, 2018

also the os path separator should reflect GOHOSTOS value

@thesyncim

This comment has been minimized.

Copy link
Author

@thesyncim thesyncim commented Aug 28, 2018

@tklauser tklauser added the Arch-Wasm label Aug 28, 2018
@tklauser tklauser changed the title wasm: invalid tmp dir when GOHOSTOS = windows os: invalid tmp dir for GOARCH=wasm, GOOS=js when GOHOSTOS=windows Aug 28, 2018
@tklauser

This comment has been minimized.

Copy link
Member

@tklauser tklauser commented Aug 28, 2018

/cc @bradfitz

@bradfitz

This comment has been minimized.

Copy link
Contributor

@bradfitz bradfitz commented Aug 28, 2018

Does this only affect running tests?

If so, one answer is that we're fine with the Linux-based builders only.

As long as Windows users can still generate working *.wasm files that run in browsers, we might be fine without tests.

But then again, the fix might also be simple enough. It might not be okay for the os package to depend on syscall/js, though.

@thesyncim

This comment has been minimized.

Copy link
Author

@thesyncim thesyncim commented Aug 28, 2018

no,

Let me give you some context:

I'm currently working on a "bridge" between go http handlers and nodejs request listeners.

This allow us to use regular http Handlers as node requestListener,
it can be useful to extend old js services with new functionalities written 100% in go. (even websockets are supported).
I know that browsers represent 99% of wasm use case, but it would be nice to have nodejs support for writing files using os tmpdir.

yes, this is probably a bad idea but is fun and powerful

@FiloSottile FiloSottile added this to the Go1.12 milestone Aug 30, 2018
@neelance

This comment has been minimized.

Copy link
Member

@neelance neelance commented Oct 21, 2018

@thesyncim What about modifying wasm_exec.js so it sets TMPDIR to require("os").tmpdir() if running with Node.js and TMPDIR is not already set? Would you mind creating a CL for it?

@thesyncim

This comment has been minimized.

Copy link
Author

@thesyncim thesyncim commented Oct 22, 2018

sure! i will try to submit the CL this week.

@neelance

This comment has been minimized.

Copy link
Member

@neelance neelance commented Nov 10, 2018

@thesyncim If you need any help with the setup for sending a CL, feel free to talk to me on the Gophers slack. Alternatively you could use the pull request integration: https://golang.org/doc/contribute.html#sending_a_change_github

@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Nov 20, 2018

Change https://golang.org/cl/150437 mentions this issue: misc/wasm: use temporary directory provided by Node.js

@gopherbot gopherbot closed this in 8679b91 Nov 20, 2018
bradfitz pushed a commit that referenced this issue Nov 21, 2018
os.TempDir() did not return a proper directory on Windows with js/wasm,
because js/wasm only uses the Unix variant of TempDir.

This commit passes the temporary directory provided by Node.js to the
Go runtime by adding it as a default value for the TMPDIR environment
variable. It makes TempDir compatible with all platforms.

Fixes #27306.

Change-Id: I8b17e44cfb2ca41939ab2a4f918698fe330cb8bc
Reviewed-on: https://go-review.googlesource.com/c/150437
Run-TryBot: Richard Musiol <neelance@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@ali2210

This comment has been minimized.

Copy link

@ali2210 ali2210 commented Sep 28, 2019

Capture
When I run env command at my command prompt
It show me GOOS="linux" GOARCH="amd64" How to execute wasm

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

Successfully merging a pull request may close this issue.

None yet
7 participants
You can’t perform that action at this time.