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

Stubs for other platforms? #107

Closed
klauspost opened this issue Mar 16, 2023 · 27 comments
Closed

Stubs for other platforms? #107

klauspost opened this issue Mar 16, 2023 · 27 comments

Comments

@klauspost
Copy link

I would like to use this for cross-platform development, and use purego where supported, but keep code compiling on all platforms.

However most functions, like Dlopen(...) aren't available for Windows, which is fair enough, but I would need to guard all code with build tags.

In these cases it is very nice to have stubs available, so code compiles, but just returns errors:

var ErrUnsupported = errors.New("purego not available on this platform")

func Dlopen(path string, mode int) (uintptr, error) {
	return 0, ErrUnsupported
}

func Dlclose(handle uintptr) error {
	return ErrUnsupported 
}

func Dlsym(handle uintptr, name string) (uintptr, error) {
	return 0, ErrUnsupported
}

// etc

For my projects this would mean a lot less code duplication. I don't really see any significant downside to adding stubs.

@TotallyGamerJet
Copy link
Collaborator

Wouldn't you still need build tags since the api isn't available on Windows? I'm not opposed to the idea of having subs.

@klauspost
Copy link
Author

Not really. In my case I must have a "software" fallback even for Linux, where the library may or may not be installed.

For me that would mean that I can keep the fallback codepath operating the same, and the code would be...

	handle, err := purego.Dlopen("libxyz.so", purego.RTLD_NOW|purego.RTLD_GLOBAL)
	if err != nil {
		return err  // error triggers fallback upstream.
	}

So in this case it can all be done without build tags at all.

@TotallyGamerJet
Copy link
Collaborator

TotallyGamerJet commented Mar 16, 2023

That makes sense. I can't think of a reason not to add the stubs. @hajimehoshi can you? I know we do stub NewCallback on Linux but that panics. Perhaps it would use the unsupported error. But perhaps that is a non-backwards compatible change?

@hajimehoshi
Copy link
Member

I still think that build tags should work instead of stubs. Is the Dlopen code path used in Windows?

@hajimehoshi
Copy link
Member

I know we do stub NewCallback on Linux but that panics.

Perhaps we should not have defined this for Linux in the first place...

@TotallyGamerJet
Copy link
Collaborator

The stub was necessary for RegisterFunc since it calls it if the argument is a function. NewCallback can be implemented on Linux. There just hasn't been much reason for Ebitengine. Compared to Darwin where there are tons of OS APIs that need a callback.

@hajimehoshi
Copy link
Member

On second thought, having the stub for NewCallback for POSIX OSes makes sense so that we can integrate the same code for POSIX OSes including macOS and Linux.

I still don't think we need stubs for Dlopen for Windows though...

@TotallyGamerJet
Copy link
Collaborator

I still think that build tags should work instead of stubs. Is the Dlopen code path used in Windows?

What do you mean by used? Windows has LoadLibrary. Dlopen and such is not supported on Windows and consequently not implemented in purego for Windows

@hajimehoshi
Copy link
Member

What do you mean by used? Windows has LoadLibrary. Dlopen and such is not supported on Windows and consequently not implemented in purego for Windows

This issue suggests that we have stubs for Dlopen for Windows, right?

@TotallyGamerJet
Copy link
Collaborator

This issue suggests that we have stubs for Dlopen for Windows, right?

If I understand correctly, yes.

@hajimehoshi
Copy link
Member

As Dlopen for Windows does not make sense, I think build tags makes sense rather than stubs.

@TotallyGamerJet
Copy link
Collaborator

Should stubs be added for other platforms like BSDs and such that do support Dlopen? I'd assume they'd eventually be supported in purego.

@hajimehoshi
Copy link
Member

Dlopen stubs for POSIX machines seem to make sense.

@klauspost
Copy link
Author

I still think that build tags should work instead of stubs. Is the Dlopen code path used in Windows?

I think you missed the point completely and you are just repeating the same thing in every comment. It is not a matter of tags not working. It is a matter of being cumbersome to work with.

My point is the API should be the same for all platforms. On non-compatible platforms the calls should just fail and return an error.

This makes it possible to do implementations with fallback without having do to completely separate implementations, and it would allow to compile check code for all platforms.

@hajimehoshi
Copy link
Member

hajimehoshi commented Mar 17, 2023

I think you missed the point completely and you are just repeating the same thing in every comment. It is not a matter of tags not working. It is a matter of being cumbersome to work with.
My point is the API should be the same for all platforms. On non-compatible platforms the calls should just fail and return an error.
This makes it possible to do implementations with fallback without having do to completely separate implementations, and it would allow to compile check code for all platforms.

OK so I understand build tags works, but this is cumbersome to you.

However, I still don't think having Dlopen for Windows makes sense. This is similar to the fact that syscall or golang.org/x/sys/unix or golang.org/x/sys/windows package has different APIs for each platform.

@klauspost
Copy link
Author

klauspost commented Mar 17, 2023

Yes syscall is also a PITB to use. There is nothing positive in emulating that. Having a different implementation for each platform is cumbersome.

os can nicely abstract features, so you don't have to open files with build tags. Some parameters are ignored and other just return errors when it cannot be implemented on a platform.

Having Dlopen return an error on some platforms doesn't really make it worse. In some cases it just makes it easier to work with, since you can actually compile code that uses it and deal with the error.

I can write a wrapper package for purego, but it doesn't really seem necessary.

@hajimehoshi
Copy link
Member

hajimehoshi commented Mar 17, 2023

Dlopen doesn't aim to be an abstract API for every platform, so this is different from os. If we do, we should have another abstract API like OpenLibrary.

I understand the benefit of having stubs, but I'm worried about the semantic incorrectness.

@TotallyGamerJet
Copy link
Collaborator

I think the deciding factor is that it is trivial to write a wrapper around purego that achieves the fallback behavior you want. Since hajimehoshi has pointed out purego is a lower level api I think sticking with just adding stubs for platforms with Dlopen support is the right way. Perhaps a higher level function like OpenLibrary could be added in the future.

@hajimehoshi
Copy link
Member

hajimehoshi commented Mar 17, 2023

As purego is a low-level lib like syscall, I'm against adding stubs (Dlopen for Windows in this case).

@TotallyGamerJet
Copy link
Collaborator

Are you against adding stubs for BSD as well?

@hajimehoshi
Copy link
Member

I'm fine with them for BSD, but Dl* stubs for BSD would mean that this will be implemented in the future, right?

@TotallyGamerJet
Copy link
Collaborator

If we want to eventually remove Cgo from Ebitengine for all platforms we would need to implement them. Perhaps we should wait until the Darwin port is completed first?

@hajimehoshi
Copy link
Member

If we want to eventually remove Cgo from Ebitengine for all platforms we would need to implement them. Perhaps we should wait until the Darwin port is completed first?

Yeah, purego for BSD is nice to have and the priority is not high. Making Ebitengine purego for Darwin and Linux has much higher priority.

@TotallyGamerJet
Copy link
Collaborator

Agreed. No Windows or BSD stubs for purego.

@hajimehoshi
Copy link
Member

Sure, thanks!

So we have decided not to add Dl* stubs for Windows, like what syscall does. Though this is cumbersome, this is a consistent way.

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

No branches or pull requests

4 participants
@hajimehoshi @klauspost @TotallyGamerJet and others