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

feat: tool, a substitute for makefiles #2359

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

thehowl
Copy link
Member

@thehowl thehowl commented Jun 14, 2024

I'm creating this issue to kick-start a discussion. #2318 goes into the direction of applying more magic to our makefiles; by hacking around the make system to have it do what we need. Naturally, the very principle of using make on a Go project is beyond its original scope of being used to compile C/C++ projects; but I guess we're well beyond that.

@ajnavarro recently came out saying that our makefiles are too complex; and I tend to agree. They are starting to use more and more of Makefile's obscure features, while still requiring a lot of duplication across our codebase due to the way that makefiles work. This PR is an attempt to try to provide an alternative system using a simple "scripting system" I wrote a while ago called tool. At the core, it is a simple bash case $1 to have different sub-commands. But it also has support for writing documentation out-of-the-box (something make cannot really do, without using more magic).

The idea of tool is to use it for when its existence can, indeed, improve user flows:

  • It is clunky to hand-install gno and gnodev if you want to put the git repository as the default GNOROOT.
  • It is clunky to use linting and formatting tools, if you don't have them installed on your system.
  • It is clunky to know what paths to build gno, gnokey and gnodev if it's your first time to use the repository.

However, I intentionally left out scripts like build and test, because really, go build and go test can and will do the job just fine. Having them in makefiles is redundant, and we should look to make them work out of the box rather than building complicated scripting on top of them.

tool parses its arguments as flag and arguments, rather than a list of scripts to run. This is in opposition to what happens with Makefiles; but honestly 99% of the time time I just use a single script on make , and I've had to pass environment variables when using make scripts before, so there's that.

Another small feature that tool has is the ability to install a companion script in $PATH. This makes it possible to run tool directly (without referring to the top directory all the time) from any sub-directory of the project.

asciicast

I'm curious to hear if we're open to the idea of switching over to tool (or something similar, really) or not.

Keep in mind, that being a bash script, on macOS/UNIX this will not require additional dependencies other than those on the system.

I believe that having a well-defined script, we can add the remaining functionality we have in the Makefiles, while centralising all important repo tooling in one place.

@thehowl thehowl requested a review from a team as a code owner June 14, 2024 16:02
@thehowl thehowl requested review from deelawn and zivkovicmilos and removed request for a team June 14, 2024 16:02
'./contribs/gnomd'
'.'
)
PROGRAMS_gnokeykc=(
Copy link
Member

@moul moul Jun 17, 2024

Choose a reason for hiding this comment

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

The "magic" we're trying to avoid was initially introduced to circumvent the need to declare all binaries. This strategy, in my opinion, is quite suitable in the context of contribs/. The contribs/ directory is designed to handle exceptions and to prevent contributors from having to modify .github/workflows and contribs/Makefile every time they want to add a binary.

We can eliminate the "magic" in the Makefile by adding only inline rules.

Comment on lines +181 to +204
tidy)
local flags=""
local executed=0
for directory in "$@"; do
if [[ $directory == -* ]]; then
# TODO: not perfect; does not support flag arguments, unless using -flag= syntax.
flags="$flags $directory"
continue
fi
executed=1
(
# prints command as it executes them
set -o xtrace
env -C $directory go mod tidy $flags
)
done
if [ $executed -eq 0 ]; then
(
# prints command as it executes
set -o xtrace
find . -name "go.mod" -execdir go mod tidy $flags \;
)
fi
;;
Copy link
Member

Choose a reason for hiding this comment

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

In my opinion, the previous approach was more straightforward and user-friendly. It allowed for easy copy-pasting to manually run a part of the "suggested flow", which is a practice I frequently do.

@moul
Copy link
Member

moul commented Jun 17, 2024

Something we can do is to keep Makefiles, but replace the magic by simply calling go run zxq.co/rosa/tool COMMAND.

So, the end result is:

  • No more Makefile magic
  • Makefiles ensure the installation of a potentially missing tool binary lazily
  • Something that works the same way everywhere (Linux, Mac, etc.)
  • No need for people to learn tool, but they can
  • Retain the "suggested flows" in the Makefile
  • Advanced commands available from tool for advanced users (we can reference tool commands for them to run)

Copy link

This PR is stale because it has been open 3 months with no activity. Remove stale label or comment or this will be closed in 3 months.

@github-actions github-actions bot added the Stale label Nov 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: No status
Status: No status
Status: In Review
Development

Successfully merging this pull request may close these issues.

2 participants