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

Jump to bottom

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
Labels
arch-wasm WebAssembly issues FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Go1.12

Comments

@thesyncim
Copy link

thesyncim commented Aug 28, 2018 •
edited

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
Copy link
Author

also the os path separator should reflect GOHOSTOS value

@thesyncim
Copy link
Author

@neelance

@tklauser tklauser added the arch-wasm WebAssembly issues 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
Copy link
Member

/cc @bradfitz

@bradfitz
Copy link
Contributor

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
Copy link
Author

thesyncim commented Aug 28, 2018 •
edited

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 the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Aug 30, 2018
@FiloSottile FiloSottile added this to the Go1.12 milestone Aug 30, 2018
@neelance
Copy link
Member

@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
Copy link
Author

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

@neelance
Copy link
Member

@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
Copy link

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

bradfitz pushed a commit that referenced this issue Nov 21, 2018
@neelance
misc/wasm: use temporary directory provided by Node.js
d7183ca 
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
Copy link

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

@golang golang locked and limited conversation to collaborators Sep 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
arch-wasm WebAssembly issues FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Go1.12
Development

No branches or pull requests
7 participants
@bradfitz @neelance @tklauser @thesyncim @FiloSottile @gopherbot @ali2210