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

Remove dependency on golang.org/x/sys #270

Closed
1 of 6 tasks
david50407 opened this issue Aug 22, 2024 · 19 comments · Fixed by #278
Closed
1 of 6 tasks

Remove dependency on golang.org/x/sys #270

david50407 opened this issue Aug 22, 2024 · 19 comments · Fixed by #278

Comments

@david50407
Copy link

Operating System

  • Windows
  • macOS
  • Linux
  • FreeBSD
  • Android
  • iOS

What feature would you like to be added?

Remove dependency on golang.org/x/sys. Aim to be zero dependencies.

Why is this needed?

The discussion was started from Discord chat: https://discord.com/channels/842049801528016967/1123106378731487345/1275998637826248744

For now, the only dependency is golang.org/x/sys.

If we can remove the dependency, we can embed purego into more situation like Golang toolchain.

Currently I'm working on a go fork and try to introduce our own C lib into Go toolchain, if purego can have zero dependencies, we can import the C lib without CGO (instead, use purego for sure). But in the toolchain, dependency chain has limitation that golang.org/x/sys is not allowed to be here. :(

hajimehoshi added a commit that referenced this issue Aug 22, 2024
The example still has the dependency on golang.org/x/sys.

Updates #270
hajimehoshi added a commit that referenced this issue Aug 22, 2024
The example still has the dependency on golang.org/x/sys.

Updates #270
@hajimehoshi
Copy link
Member

@david50407 Please try 5905af50730466425f56454a0491ae991d644beb

@texadactyl
Copy link

My partner (@platypusguy) and I are working on a large and complex Go project that is being amended to access JVM library functions. It runs on Linux, Unix, MacOS, and Windows. It would be very helpful if we only had to deal with a single generic purego API set (Dlopen, Dlclose, etc.).

Is a single purego API set one of the goals of this issue?

@hajimehoshi
Copy link
Member

hajimehoshi commented Aug 24, 2024

It would be very helpful if we only had to deal with a single generic purego API set (Dlopen, Dlclose, etc.).

This is a different topic from this issue. The answer is we don't plan to do that, since Windows doesn't have dl* functions.

@TotallyGamerJet
Copy link
Collaborator

Is a single purego API set one of the goals of this issue?

For the explanation why see #107

@texadactyl

This comment was marked as off-topic.

@hajimehoshi

This comment was marked as off-topic.

@hajimehoshi
Copy link
Member

@david50407 ping

@david50407
Copy link
Author

david50407 commented Aug 28, 2024

Sorry for the late. I've tried the version you metioned.

Once I add purego as dependency of golang toolchain 1.22.5 and build the toolchain.
It failes the test of dependency version consistent, it says that requires x/sys v0.15.0 but we are providing v0.23.0 which purego depends on in go.mod. : (

So I think if we want to embed purego into the toolchain, we have to remove the dependency in go.mod directly.
And I know that will be a problem because examples are using x/sys. Do we have any other way to remove this dependency from go.mod and keep examples works well?

edit: I think that's why MS load libraries by calling syscall.NewLazyDLL directly in their go-crypto-winnative project, they need to avoid the dependency to x/sys directly

@hajimehoshi
Copy link
Member

Once I add purego as dependency of golang toolchain 1.22.5 and build the toolchain.
It failes the test of dependency version consistent, it says that requires x/sys v0.15.0 but we are providing v0.23.0 which purego depends on in go.mod. : (

What kind of commands did you execute? I'd like to know the details.

edit: I think that's why MS load libraries by calling syscall.NewLazyDLL directly in their go-crypto-winnative project, they need to avoid the dependency to x/sys directly

NewLazyDLL and NewLazySystemDLL are slightly different in terms of security. Let me think.. Probably I'd have to immitate NewLazySystemDLL's behavior. (LoadLibraryEx + appropriate flags?)

@david50407
Copy link
Author

david50407 commented Aug 28, 2024

What kind of commands did you execute? I'd like to know the details.

Here's the steps I took:

  1. Add purego to my module A with go get github.com/ebitengine/purego@5905af50730466425f56454a0491ae991d644beb
  2. Add some usage into module A, to simpify the process, I copied the example code into the init function, like this:
package module_a

import (
	"github.com/ebitengine/purego"
)

func init() {
	libc, err := purego.Dlopen("/usr/lib/libSystem.B.dylib", purego.RTLD_NOW|purego.RTLD_GLOBAL)
	if err != nil {
		panic(err)
	}
	var puts func(string)
	purego.RegisterLibFunc(&puts, libc, "puts")
	puts("Calling C from Go with purego!")
}
  1. Add module A into golang toolchain 1.22.5 with go get private.pkgs/module_a@commitid, and get the output:
go: downloading private.pkgs/module_a v0.1.0-alpha.3.0.20240828062439-2e1170466358
go: added github.com/ebitengine/purego v0.8.0-alpha.5
go: upgraded private.pkgs/module_a v0.1.0-alpha.3.0.20240815033454-aa6aed5d72d1 => v0.1.0-alpha.3.0.20240828062439-2e1170466358
go: upgraded golang.org/x/sys v0.15.0 => v0.23.0
  1. Add some calls to module A into go/src/crypto/internal
  2. Run go mod vendor, get a wired error:
go: std/crypto/tls imports
	golang.org/x/crypto/chacha20poly1305 imports
	golang.org/x/sys/cpu: missing go.sum entry for module providing package golang.org/x/sys/cpu (imported by golang.org/x/crypto/chacha20poly1305); to add:
	go get golang.org/x/crypto/chacha20poly1305@v0.16.1-0.20231129163542-152cdb1503eb
  1. Run go get golang.org/x/sys/cpu to get missing dependencies, and re-run go mod vendor:
go: downloading golang.org/x/sys v0.24.0
go: upgraded golang.org/x/sys v0.23.0 => v0.24.0
  1. Build the toolchain with bash make.bash
  2. Run the test from toolchain with bash run.bash --no-rebuild, and get the output with this (skipped some output):
...
--- FAIL: TestDependencies (1.46s)
    deps_test.go:765: unexpected dependency: github.com/ebitengine/purego imports [errors fmt github.com/ebitengine/purego/internal/cgo github.com/ebitengine/purego/internal/fakecgo github.com/ebitengine/purego/internal/strings math reflect runtime runtime/cgo sync syscall unsafe]
    deps_test.go:765: unexpected dependency: github.com/ebitengine/purego/internal/cgo imports [C errors unsafe]
    deps_test.go:765: unexpected dependency: github.com/ebitengine/purego/internal/fakecgo imports [syscall unsafe]
    deps_test.go:765: unexpected dependency: github.com/ebitengine/purego/internal/strings imports [unsafe]
    deps_test.go:765: unexpected dependency: private.pkgs/module_a imports [github.com/ebitengine/purego]
FAIL
FAIL	go/build	2.835s
...
--- FAIL: TestDependencyVersionsConsistent (0.00s)
    moddeps_test.go:415: Modules within GOROOT require different versions of golang.org/x/sys.
    moddeps_test.go:427: std	requires v0.24.0
    moddeps_test.go:427: cmd	requires v0.15.0
FAIL
FAIL	cmd/internal/moddeps	4.626s
...

For the TestDependencies, I can solve the problem by add rules for importing purego.
But for the TestDependencyVersionsConsistent, Go complains that golang.org/x/sys must be consistent (to v0.15.0)

@hajimehoshi
Copy link
Member

Hmm, I understood that you have modified the Go toolchain by yourself and the toolchain has some tricky things to resolve dependencies. Thank you for elaborating!

I'll modify the example not to use golang.org/x/sys tonight, preserving the current behavior or adding comments to explain that usually NewLazySystemDLL is preferrable.

@david50407
Copy link
Author

@hajimehoshi thank you so much!

@TotallyGamerJet
Copy link
Collaborator

I'm confused. It looks like the error you are getting isn't related to not being able to import the sys package but that you have differing versions.

I'd like to know why std/crypto imports an outdated version of golang.org/x/sys and why you can't just update its version to match purego's?

@david50407
Copy link
Author

@TotallyGamerJet the version of golang.org/x/sys is not decided by me, it's declared from original golang source, I'm not sure if I upgrades the version won't break something inside Go

https://github.com/golang/go/blob/release-branch.go1.22/src/go.mod#L11

@TotallyGamerJet
Copy link
Collaborator

You're already using your own forked version of Go seems like things could easily break for other reasons.

Update and run the test suite. I'm sure it would find most issues if there was one

@david50407
Copy link
Author

@TotallyGamerJet upgrading x/sys may cause issues.

Althought I can run the test suite to find out the issues and fix them, but it is not a reasonable work while purego is not using x/sys anymore, but I still have to upgrade x/sys and facing potential issues that doesn't bring any benifit.

Updating just for the sake of updating doesn't make sense, right?

@hajimehoshi
Copy link
Member

Even though this is not an ideal solution, I'm fine to remove the dependency on golang.org/x/sys from this repository including the example.

@TotallyGamerJet what do you think?

@TotallyGamerJet
Copy link
Collaborator

I don't like removing it from the examples but I goes it's fine

@hajimehoshi
Copy link
Member

hajimehoshi commented Sep 3, 2024

I don't prefer this solution either. However, I understand that, in very special cases like modifying the Go toolchain with Pure Go, an extra dependency in go.mod could be a problem.

As a bonus, removing all the dependencies from go.mod might be good for marketing. We can assert that Pure Go is zero-dependency. For example, Wazero says so. Technically it is possible to say the core part is zero-dependency even with some dependencies in go.mod, and actually the current Pure Go is. So, this is just a 'marketing' thing.

TotallyGamerJet pushed a commit to TotallyGamerJet/purego that referenced this issue Oct 14, 2024
The example still has the dependency on golang.org/x/sys.

Updates ebitengine#270
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants