Update abigen build script #16430
Update abigen build script #16430
Conversation
RayXpub
force-pushed
the
abigen-and-ci-go-version-src-update
branch
from
7d1b99c to
83cd9fc
Compare
tools/bin/build_abigen
Outdated
|
|
||
| NATIVE_ABIGEN_VERSION=v"$( | ||
| "$THIS_DIR/abigen" --version 2> /dev/null | \ | ||
| grep -E -o '([0-9]+\.[0-9]+\.[0-9]+)' | ||
| abigen --version 2>/dev/null | |
There was a problem hiding this comment.
this implies that you are accessing the abigen installed in the $PATH.
Are you sure that there is no usecase with one abigen in the $PATH and another one in the $THIS_DIR?
There was a problem hiding this comment.
I agree, this is a bit dangerous; we don't want the behavior to implicitly depend on user-configured variables like $PATH. Perhaps abigen could be accessed like this:
$ go list -f '{{.Target}}' github.com/ethereum/go-ethereum/cmd/abigen # find path to installed binary
/home/ubuntu/go/bin/abigen
$ $(go list -f '{{.Target}}' github.com/ethereum/go-ethereum/cmd/abigen) # execute it
Fatal: No destination package specified (--pkg)
There was a problem hiding this comment.
I see, in that case I think keeping the original behavior makes more sense. I reverted the change and just added a comment.
There was a problem hiding this comment.
Compared to go run, there might be a slight speed advantage to pulling the abigen binary out of the go-ethereum tree and using a hard-coded path for it.
It may be worth optimizing this script a bit, because it gets invoked a lot during development of an interaction between a go service and an onchain contract, so there's the potential to save people a lot of time with it. (At least, there would be if other people are using it the way I use it.)
At the same time, this script effectively mediates such interactions, via abigen, so changes to the script should be made cautiously.
There was a problem hiding this comment.
I'd suggest we punt optimization to another PR and agree that we should be cautious with changes here.
There was a problem hiding this comment.
If there's a need for flexibility in this regard, perhaps the behavior about which abigen executable to use could be controlled by a specialized env var, like $ABIGEN_EXECUTABLE_PATH. If it's empty, it defaults to the current behavior, otherwise, it uses the specified path.
RayXpub
force-pushed
the
abigen-and-ci-go-version-src-update
branch
from
83cd9fc to
d3dfce0
Compare
pavel-raykov
changed the title
RayXpub
force-pushed
the
abigen-and-ci-go-version-src-update
branch
from
d3dfce0 to
ebab855
Compare
|
The current
abigen.gohas a hardcoded path to the abigen executable. This can be avoided by updating the build script to just use the go install command as instructed by the geth documentation.