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

Surprised by the hard dependency on github.com/karalabe/usb #21773

Closed
mvdan opened this issue Nov 1, 2020 · 11 comments
Closed

Surprised by the hard dependency on github.com/karalabe/usb #21773

mvdan opened this issue Nov 1, 2020 · 11 comments
Assignees

Comments

@mvdan
Copy link

mvdan commented Nov 1, 2020

System information

github.com/ethereum/go-ethereum v1.9.20
Linux

Expected behaviour

No hard dependency on USB libraries to run a basic Ethereum node.

Actual behaviour

$ go mod why github.com/karalabe/usb
# github.com/karalabe/usb
<my package>
github.com/ethereum/go-ethereum/node
github.com/ethereum/go-ethereum/accounts/usbwallet
github.com/karalabe/usb

For example, this will mean failures to build on golang alpine images unless linux-headers is installed:


# github.com/karalabe/usb
In file included from /go/pkg/mod/github.com/karalabe/usb@v0.0.0-20191104083709-911d15fe12a9/libusb/libusb/os/linux_usbfs.c:43,
                 from /go/pkg/mod/github.com/karalabe/usb@v0.0.0-20191104083709-911d15fe12a9/libs.go:46:
/go/pkg/mod/github.com/karalabe/usb@v0.0.0-20191104083709-911d15fe12a9/libusb/libusb/os/linux_usbfs.h:24:10: fatal error: linux/types.h: No such file or directory
   24 | #include <linux/types.h>
      |          ^~~~~~~~~~~~~~~

I get that this probably provides good features to some people, but I'm pretty sure that we don't use any USB devices for our Ethereum nodes. It would be nice if this indirect dependency wasn't mandated by the node package, which we can't really stop using.

This "tricky hard dependency" issue is similar to #20590, which I filed a while ago.

@mvdan
Copy link
Author

mvdan commented Nov 1, 2020

A nice way to fix this problem would be to have the user opt into the USB features by importing the node package as well as some other package like usbwallet. Then adding one or two lines using usbwallet would enable its features.

@karalabe
Copy link
Member

karalabe commented Nov 2, 2020

Hmm, you should be able to disable it via --tags=nocgo during build, but I do get that it would be unwise since you only want USB out, not all the C libraries. Would a --nousb build tag be a suitable solution?

@mvdan
Copy link
Author

mvdan commented Nov 2, 2020

A universal CGO_ENABLED=0 or -tags=nocgo is indeed an option, but not the best, like you say. I imagine running an Ethereum node without cgo isn't particularly well supported or tested. Plus, it enforces the rest of the program to not use cgo either.

A "nousb" build tag would be better than nothing, but I still think opting in via import paths is the best option. The default build (with no build tags) should do the right thing, and the user should be in control of what optional dependencies are pulled in via imports.

@karalabe
Copy link
Member

karalabe commented Nov 2, 2020

Ah, you're talking about the node package and whether we could break the dependency on USB. That would be ideal yes.

@fjl
Copy link
Contributor

fjl commented Nov 19, 2020

This issue should be addressed by restructuring wallet initialization. At this time, wallets are initialized in package node:

// Start a USB hub for Ledger hardware wallets

This is very unfortunate because it means that all code using package node will depend on usbwallet (and scwallet!).
It would be nicer to perform the usbwallet/scwallet initialization outside of package node. While accounts.Manager
should remain in package node, it might be required to add a way to add wallet backends after creating the manager.

@mvdan
Copy link
Author

mvdan commented Nov 19, 2020

Indeed, I agree that would be the best solution. Then the extra package/module dependencies need to be explicitly imported by the user's code.

@youngqqcn
Copy link

I run make in termux on my android phone, karalabe/usb cause errors as belowing:

 github.com/karalabe/usb
# github.com/karalabe/usb
In file included from ../../go/pkg/mod/github.com/karalabe/usb@v0.0.0-20190919080040-51dc0efba356/libs.go:48:
../../go/pkg/mod/github.com/karalabe/usb@v0.0.0-20190919080040-51dc0efba356/hidapi/libusb/hid.c:63:3: error: typedef redefinition with different types ('struct pthread_barrier' vs 'struct pthread_barrier_t')
/data/data/com.termux/files/usr/include/bits/pthread_types.h:54:3: note: previous definition is here
In file included from ../../go/pkg/mod/github.com/karalabe/usb@v0.0.0-20190919080040-51dc0efba356/libs.go:48:
../../go/pkg/mod/github.com/karalabe/usb@v0.0.0-20190919080040-51dc0efba356/hidapi/libusb/hid.c:65:12: error: static declaration of 'pthread_barrier_init' follows non-static declaration
/data/data/com.termux/files/usr/include/pthread.h:329:5: note: previous declaration is here
In file included from ../../go/pkg/mod/github.com/karalabe/usb@v0.0.0-20190919080040-51dc0efba356/libs.go:48:
../../go/pkg/mod/github.com/karalabe/usb@v0.0.0-20190919080040-51dc0efba356/hidapi/libusb/hid.c:72:34: error: no member named 'mutex' in 'pthread_barrier_t'
../../go/pkg/mod/github.com/karalabe/usb@v0.0.0-20190919080040-51dc0efba356/hidapi/libusb/hid.c:75:33: error: no member named 'cond' in 'pthread_barrier_t'
../../go/pkg/mod/github.com/karalabe/usb@v0.0.0-20190919080040-51dc0efba356/hidapi/libusb/hid.c:76:35: error: no member named 'mutex' in 'pthread_barrier_t'
../../go/pkg/mod/github.com/karalabe/usb@v0.0.0-20190919080040-51dc0efba356/hidapi/libusb/hid.c:79:11: error: no member named 'trip_count' in 'pthread_barrier_t'
../../go/pkg/mod/github.com/karalabe/usb@v0.0.0-20190919080040-51dc0efba356/hidapi/libusb/hid.c:80:11: error: no member named 'count' in 'pthread_barrier_t'
../../go/pkg/mod/github.com/karalabe/usb@v0.0.0-20190919080040-51dc0efba356/hidapi/libusb/hid.c:85:12: error: static declaration of 'pthread_barrier_destroy' follows non-static declaration
/data/data/com.termux/files/usr/include/pthread.h:330:5: note: previous declaration is here
In file included from ../../go/pkg/mod/github.com/karalabe/usb@v0.0.0-20190919080040-51dc0efba356/libs.go:48:
../../go/pkg/mod/github.com/karalabe/usb@v0.0.0-20190919080040-51dc0efba356/hidapi/libusb/hid.c:87:33: error: no member named 'cond' in 'pthread_barrier_t'
../../go/pkg/mod/github.com/karalabe/usb@v0.0.0-20190919080040-51dc0efba356/hidapi/libusb/hid.c:88:34: error: no member named 'mutex' in 'pthread_barrier_t'
../../go/pkg/mod/github.com/karalabe/usb@v0.0.0-20190919080040-51dc0efba356/hidapi/libusb/hid.c:92:12: error: static declaration of 'pthread_barrier_wait' follows non-static declaration
/data/data/com.termux/files/usr/include/pthread.h:331:5: note: previous declaration is here
In file included from ../../go/pkg/mod/github.com/karalabe/usb@v0.0.0-20190919080040-51dc0efba356/libs.go:48:
../../go/pkg/mod/github.com/karalabe/usb@v0.0.0-20190919080040-51dc0efba356/hidapi/libusb/hid.c:94:31: error: no member named 'mutex' in 'pthread_barrier_t'
../../go/pkg/mod/github.com/karalabe/usb@v0.0.0-20190919080040-51dc0efba356/hidapi/libusb/hid.c:95:14: error: no member named 'count' in 'pthread_barrier_t'
../../go/pkg/mod/github.com/karalabe/usb@v0.0.0-20190919080040-51dc0efba356/hidapi/libusb/hid.c:96:14: error: no member named 'count' in 'pthread_barrier_t'
../../go/pkg/mod/github.com/karalabe/usb@v0.0.0-20190919080040-51dc0efba356/hidapi/libusb/hid.c:96:32: error: no member named 'trip_count' in 'pthread_barrier_t'
../../go/pkg/mod/github.com/karalabe/usb@v0.0.0-20190919080040-51dc0efba356/hidapi/libusb/hid.c:98:12: error: no member named 'count' in 'pthread_barrier_t'
../../go/pkg/mod/github.com/karalabe/usb@v0.0.0-20190919080040-51dc0efba356/hidapi/libusb/hid.c:99:36: error: no member named 'cond' in 'pthread_barrier_t'
../../go/pkg/mod/github.com/karalabe/usb@v0.0.0-20190919080040-51dc0efba356/hidapi/libusb/hid.c:100:34: error: no member named 'mutex' in 'pthread_barrier_t'
../../go/pkg/mod/github.com/karalabe/usb@v0.0.0-20190919080040-51dc0efba356/hidapi/libusb/hid.c:105:31: error: no member named 'cond' in 'pthread_barrier_t'
fatal error: too many errors emitted, stopping now [-ferror-limit=]
util.go:47: exit status 2
exit status 1
make: *** [Makefile:16: geth] Error 1

@holiman
Copy link
Contributor

holiman commented Nov 3, 2021

didn't we fix this already @fjl ?

@holiman
Copy link
Contributor

holiman commented Nov 3, 2021

Yup, fixed

$ go mod why github.com/karalabe/usb
# github.com/karalabe/usb
github.com/ethereum/go-ethereum/accounts/usbwallet
github.com/karalabe/usb

@simbadMarino
Copy link

@youngqqcn hello mate, did you manage to compile on termux? I'm having a similar issue... I've tried lots of thing but still no luck...

@youngqqcn
Copy link

@youngqqcn hello mate, did you manage to compile on termux? I'm having a similar issue... I've tried lots of thing but still no luck...

@simbadMarino

I had solved this issue by removing usbwallet and other unimportant denpendencies.

All modifies in here : https://github.com/orientwalt/go-ethereum/tree/v1.8.39

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

8 participants
@fjl @ligi @karalabe @holiman @mvdan @simbadMarino @youngqqcn and others