Skip to content

chore(tools): break tools out into a separate module #2140

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

Merged
merged 1 commit into from
Nov 19, 2019

Conversation

jsternberg
Copy link
Contributor

@jsternberg jsternberg commented Nov 13, 2019

This change breaks the tools out into a separate module so that we can
continue using go modules for pinning go-based dependencies, but to
avoid having these modules become part of our external dependencies.

The gotool.sh script is added to make it easy to run modules within
the tool directory while ensuring the scripts still get executed in the
current directory rather than inside of the tools directory.

Fixes #2040.

Done checklist

  • docs/SPEC.md updated
  • Test cases written
Siteproxy
@jsternberg jsternberg requested a review from affo November 13, 2019 16:26
# dependencies in the tool module.
#
# This allows us to specify tool dependencies using go modules without
# it getting added to the flux module itself.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, brilliant

Copy link
Contributor

@affo affo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great, thank you @jsternberg for this. But I have one question.

I have nothing against this implementation, but, I wonder, why don't you implement what gotools.sh does directly in a func main in internal/tools/tools.go?

Wouldn't it be more maintainable and easy to understand? I mean, more linear.

Something like:

func main() {
        dir := os.Args[1]
        goModule := os.Args[2]
        args := os.Args[3:]
	cmd := exec.Command("go", "run", goModule, args...)
        cmd.Dir = dir
	err := cmd.Run()
	log.Printf("Command finished with error: %v", err)
}

and gotool.sh:

cd $tools_dir && go run $(pwd) "$@"

Copy link
Contributor

@affo affo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After reading my previous comment, I realized I like more this current solution, never mind 😅

This change breaks the tools out into a separate module so that we can
continue using go modules for pinning go-based dependencies, but to
avoid having these modules become part of our external dependencies.

The `gotool.sh` script is added to make it easy to run modules within
the tool directory while ensuring the scripts still get executed in the
current directory rather than inside of the tools directory.
@jsternberg jsternberg merged commit ac49126 into master Nov 19, 2019
@jsternberg jsternberg deleted the chore/tools-module branch November 19, 2019 15:18
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

Successfully merging this pull request may close these issues.

Revisit the pattern of including tooling dependencies
2 participants