Remove some unused stuff from the devtools makefile

[SpicyLemon]

Description

This PR removes a bunch of unused stuff from contrib/devtools/Makefile, and also moves some stuff from there into the primary Makefile. There's a little more cleanup done in this too.

---

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

• Targeted PR against correct branch (see CONTRIBUTING.md).
• Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
• Wrote unit and integration tests
• Updated relevant documentation (docs/) or specification (x/<module>/spec/).
• Added relevant godoc comments.
• Added relevant changelog entries under .changelog/unreleased (see Adding Changes).
• Re-reviewed Files changed in the Github PR explorer.
• Review Codecov Report in the comment section below once CI passes.

Summary by CodeRabbit

• Improvements
  • Streamlined the build process by removing obsolete configurations from Makefiles, enhancing maintainability and clarity.
  • Improved logic for detecting the Go executable and validating the Go version, leading to a more robust build experience.
  • Simplified developer tools Makefile by eliminating unnecessary environment detection and setup logic.
  • Updated error handling for missing Go executable to provide clearer user feedback.

These changes contribute to a more efficient and user-friendly development environment.

[coderabbitai]

Walkthrough

The changes involve a comprehensive cleanup of makefiles associated with the project, focusing on removing obsolete configurations, improving logic for determining the Go executable and version, and simplifying the Makefile for developer tools. The modifications enhance clarity, streamline the build process, and eliminate unnecessary environment detection mechanisms.

Changes

File	Change Summary
.changelog/unreleased/improvements/2136-makefile-cleanup.md	Cleanup of unused elements in makefiles for better maintainability; no changes to public entity declarations.
Makefile	Enhanced logic for locating the Go executable and validating Go version, updated versioning logic, and minor formatting changes; includes no changes to public entity declarations.
contrib/devtools/Makefile	Removed environment detection and setup for Go, simplifying the Makefile; no changes to public entity declarations.
sims.mk	Removal of comments and variable assignments related to Go setup, indicating a streamlined approach; includes no changes to public entity declarations.

Sequence Diagram(s)

sequenceDiagram
    participant Developer
    participant Makefile
    participant Go

    Developer->>Makefile: Initiates build process
    Makefile->>Go: Check Go executable
    Go-->>Makefile: Returns Go version
    Makefile->>Developer: Outputs build status

Loading

🐇 In fields so bright and green,
We’ve tidied up, a lovely scene!
With makefiles fresh and clean,
The build will run like a dream!
So hop along, let’s code and play,
With clearer paths, we’ll find our way! 🍃✨

---

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

• X
• Mastodon
• Reddit
• LinkedIn

Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>.
  • Generate unit testing code for this file.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit testing code for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai generate interesting stats about this repository and render them as a table.
  • @coderabbitai show all the console.log statements in this repository.
  • @coderabbitai read src/utils.ts and generate unit testing code.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
  • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai full review to do a full review from scratch and review all the files again.
• @coderabbitai summary to regenerate the summary of the PR.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
• @coderabbitai help to get help.

Other keywords and placeholders

• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• Please see the configuration documentation for more information.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[coderabbitai]

Actionable comments posted: 0

Outside diff range, codebase verification and nitpick comments (1)

Makefile (1)

4-11: Consider adding a comment for clarity.

The logic to find the Go executable is well-implemented. Adding a comment to explain why this check is necessary could improve readability.

 # Find the go executable if it wasn't pre-set (e.g. via env var).
+ # This ensures compatibility across different platforms.

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between c702324 and da47ef7.

Files selected for processing (4)

• .changelog/unreleased/improvements/2136-makefile-cleanup.md (1 hunks)
• Makefile (8 hunks)
• contrib/devtools/Makefile (1 hunks)
• sims.mk (3 hunks)

Files skipped from review due to trivial changes (3)

• .changelog/unreleased/improvements/2136-makefile-cleanup.md
• contrib/devtools/Makefile
• sims.mk

Additional comments not posted (4)

Makefile (4)

12-15: LGTM!

The error handling for the absence of a Go executable is clear and effective.

---

16-18: LGTM!

The logic for setting the GOPATH is straightforward and effective.

---

79-80: LGTM!

The export of CMTVERSION using the Go executable is appropriate and effective.

---

322-330: LGTM!

The Go version validation logic is clear and effective, providing a useful error message if the version is unsupported.

[coderabbitai]

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between da47ef7 and d66fdfd.

Files selected for processing (1)

• Makefile (8 hunks)

Files skipped from review as they are similar to previous changes (1)

• Makefile

[SpicyLemon]

When working on this I found out some interesting things about makefile syntax that I'd like to share.

When you assign a value using =, the RHS is NOT executed. The variable contains all the references still. So if you do FOO = $(shell echo foofy >&2 && echo foo), that shell command is not executed (yet). It'll be executed any/every time you're trying to use the value of the variable though. So if you then do BAR := $(FOO) $(FOO) then you'll see "foofy" printed to stderr twice.

The := assignment operator does evaluate/execute the RHS before setting the variable though. So, building on the above example, if you then do BAZ := $(BAR) $(BAR), BAZ will have the value foo foo and nothing new is printed to stderr since BAR's value had already been resolved too.

Another example:

1. FOO = $(shell echo foofy >&2 && echo foo)
2. BAR2 = $(FOO) $(FOO)
3. BAZ2 := $(BAR2) $(BAR2)

Nothing is printed to stderr when setting BAR2 now, since it uses = there. But from line three, you'll end up with four foofys in stderr because each $(BAR2) is evaluated to $(FOO) $(FOO) which then causes the shell command to be run twice; two $(BAR2)s = four $(FOO)s = four foofys in stderr and BAZ2 = foo foo foo foo.

That means that, if we were to put a line between 2 and 3 that changes FOO to something else, e.g. FOO = $(shell echo foofun >&2 && echo fOO), then when setting BAZ2, it'll end up as fOO fOO fOO fOO and foofun will be printed to stderr four times. This happens because on line 2, we used =, so the RHS wasn't evaluated, and BAR2 just contained references.

You can force a variable to resolve itself by doing FOO := $(FOO), but I find that a little confusing, so have tried to avoid it.

---

Now, to add to all this, we have the ?= assignment operator.

From https://www.gnu.org/software/make/manual/html_node/Setting.html

These two settings of the variable ‘FOO’ are identical

FOO ?= bar

and

ifeq ($(origin FOO), undefined)
FOO = bar
endif

The first critical piece in there is that it uses the = operator, and not :=. So if you do FOO ?= $(shell echo foofy >&2 && echo foo), then if FOO was undefined, it now contains a reference to that shell function, so any uses of $(FOO) will cause foofy to be printed to stderr. So it's probably best to not use ?= if you want the RHS to be a shell execution (or something else that might be expensive).

The second critical piece is that makefile distinguishes between a variable being undefined and being empty. If my environment has export FOO='', then when make gets to FOO ?= whatever, it sees that the env var FOO is defined, so nothing happens, i.e. $(FOO) will resolve to an empty string.

---

Lastly, wrapping quotes are not stripped from variable values. E.g. all three of these are different: FOO := 'foo', FOO := "foo", and FOO := foo. If you did FOO := 'foo', then the test ifeq ($(FOO),foo) is false, but ifeq ($(FOO),'foo') is true.