hack: fix install-tools.sh to be stand-alone utility

[synarete]

Commit 3cb40b85 ("hack: helper script to install auxiliary build-tools" introduced 'install-tools.sh' script which is used by top-level Makefile to install missing build tools. However, developer may wish to use this script directly, and install tools under '${GOPATH}/bin' instead of local '.bin' directory. Fixed the script to facilitate this option as well.

Reported by Anoop C S.

Signed-off-by: Shachar Sharon ssharon@redhat.com

[synarete]

Just to double check my understanding: the idea is that if the script is called from the makefile it'll used $PROJECTDIR/.bin >but if used directly by the developer it'll install the tool(s) directly in the user's GOBIN? Correct

Would something like $(PROJECTDIR)/.bin/$(VERSION) be acceptable?

[phlogistonjohn]

This is pretty speculative and subject to all your inputs, but what I was thinking is something like:

# example
echo  "pkg-$( echo -n 'github.com/coolstuff/projectx@v0.6.13' | sha256sum  | head -c16).toolname"
# more code like
echo  "pkg-$( echo -n "${gopkg}" | sha256sum  | head -c16).${binname}"

It avoids having to parse the version out of the go pkg string as well as by including the pkg path in the hash if there was a new package path used it would generate a different hash. I had versions in my original prototype but dealing with the parsing of the go pkgs quickly became the biggest stumbling block.

I was also thinking that each time the script created this hashed-name binary it would also create a symlink to the standard name, like ln -s "$bindir/$hashname" "$bindir/$stdname"

Any approach along these lines is going to need changes to both the makefile and the script, but I figure I'd mention these ideas before we make additional changes to the script. This is a bit "thinking out loud" so feel free to tell me I'm being silly or put your own spin on these thoughts. :-)

[phlogistonjohn]

I'm sorry if I side tracked the discussion on this one. @synarete are you happy with the behavior here? If so, please resolve conflicts and I'll review it for what it is rather than speculate on future features.

[synarete]

@phlogistonjohn I resolved this PR, but the actual code is already merged into master (commit 7afa2c1365c44077a6475207289d6ba4ca4cf026)

[phlogistonjohn]

The difference between branches looks like

-GOBIN=${GOBIN:-$(pwd)/bin}
+GOBIN=${GOBIN:-${GOPATH}/bin}

When I use the CLI. If you're good with this change we can probably merge that.

[synarete]

The difference between branches looks like

-GOBIN=${GOBIN:-$(pwd)/bin}
+GOBIN=${GOBIN:-${GOPATH}/bin}

When I use the CLI. If you're good with this change we can probably merge that.

My bad, missed this one. ACK with me.

[phlogistonjohn]

@synarete this will need a rebase on the CLI. I'll let you do it, but if you say so I can do it and push into your branch.

[anoopcs9]

/retest centos-ci/sink-clustered/mini-k8s-1.23

[phlogistonjohn]

I'm going to merge this directly, because the test failures is most likely a flake and very very unlikely to be caused by this change.