otiai10 / copy

Go copy directory recursively
https://pkg.go.dev/github.com/otiai10/copy
MIT License
722 stars 115 forks source link

Make package work with WASM and GopherJS targets #75

Closed flimzy closed 2 years ago

flimzy commented 2 years ago

At present, this package does not work with WASM or GopherJS compile targets. See for example, the output from go test for WASM:

$ GOOS=js GOARCH=wasm go test ./... -exec=$(go env GOROOT)/misc/wasm/go_js_wasm_exec
# github.com/otiai10/copy [github.com/otiai10/copy.test]
./copy_namedpipes.go:16:9: undefined: syscall.Mkfifo
./stat_times.go:17:30: stat.Atim undefined (type *syscall.Stat_t has no field or method Atim)
./stat_times.go:18:30: stat.Ctim undefined (type *syscall.Stat_t has no field or method Ctim)
./test_setup.go:16:2: undefined: syscall.Mkfifo
FAIL    github.com/otiai10/copy [build failed]
FAIL

and GopherJS:

$ gopherjs test ./...
test_setup.go:16:10: Mkfifo not declared by package syscall
stat_times.go:17:31: stat.Atim undefined (type *syscall.Stat_t has no field or method Atim)
stat_times.go:17:53: stat.Atim undefined (type *syscall.Stat_t has no field or method Atim)
stat_times.go:18:31: stat.Ctim undefined (type *syscall.Stat_t has no field or method Ctim)
stat_times.go:18:53: stat.Ctim undefined (type *syscall.Stat_t has no field or method Ctim)
copy_namedpipes.go:16:17: Mkfifo not declared by package syscall

This PR solves this problem, by using the same fallback used for Windows, which doesn't support Mkfifo, and by using the JS-specific struct field names for Atime/Ctime fields in the syscall package.

codecov-commenter commented 2 years ago

Codecov Report

Merging #75 (94e412f) into main (9aae5f7) will not change coverage. The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main      #75   +/-   ##
=======================================
  Coverage   77.40%   77.40%           
=======================================
  Files          13       13           
  Lines         177      177           
=======================================
  Hits          137      137           
  Misses         20       20           
  Partials       20       20           
Impacted Files Coverage Δ
copy_namedpipes.go 33.33% <ø> (ø)
copy_namedpipes_x.go 0.00% <ø> (ø)
stat_times.go 100.00% <ø> (ø)
test_setup_x.go 100.00% <ø> (ø)
test_setup.go 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 9aae5f7...94e412f. Read the comment docs.

flimzy commented 2 years ago

I don't know that there's anything I can do about the failing tests, as they appear to be transient failures. Is there anything I can do to help this PR advance?

otiai10 commented 2 years ago

@flimzy Thank you. Though I'm still investigating the failing tests, I believe this PR can be merged. Just let me take 1 more day to suppress failing cases.

Thank you again for your proposal!

flimzy commented 2 years ago

Thanks!

It just occurred to me that it might be valuable to have tests run against wasm and GopherJS, as well. If you'd like, I'm happy to submit another PR to add those targets to the CI configuration.

otiai10 commented 2 years ago

that would be perfect! no pressure