Add a vendor make target for updating go module files

[elezar]

This change adds a vendor make target that runs go mod tidy for the root module as well as each of the cmd submodules.

The vendor target is adapted from the containerd makefile.

This also converts cmd/validate to a submodule.

[klihub]

What is the purpose of mod-tidy-% here in $(MOD_TIDY_TARGETS): mod-tidy-% ? AFAICT, it will evaluate to empty.

[elezar]

This is how pattern targets are constructed. The % in the rule matches the last part of the target and allows $(*) to be used in the commands.

[klihub]

Have you tried checking what, for instance, that mod-tidy-% evaluates to in the $(MOD_TIDY_TARGETS): mod-tidy-% rule with something like this:

$(MOD_TIDY_TARGETS): mod-tidy-%:
	echo "*** prerequisites for '$@' => '$^' ***"
	$(Q)echo "Running $@... in $(abspath ./cmd/$(*))"
	$(Q)(cd $(abspath ./cmd/$(*)) && ${GO} mod tidy)

[klihub]

That's not how pattern targets are constructed IMO. In a pattern target there must be a % (exactly one) in the target. Then you can have one or more % in the prerequisites which are then replaced with the same stem. In $(MOD_TIDY_TARGETS) there are 0 %s. That's where my whole confusion is coming from. The way how I originally misread you rule definition definitely does not make any sense. The way how I now more carefully read your rule... I don't understand what it means.

[elezar]

In the rule as written:

$(MOD_TIDY_TARGETS): mod-tidy-%: 

there are no prerequisites defined. This is a static pattern rule of the form:

targets ...: target-pattern: dep-patterns ...
        commands

In this case, dep-patterns is empty.

From the docs:

The target-pattern and dep-patterns say how to compute the dependencies of each target. Each target is matched against the target-pattern to extract a part of the target name, called the stem. This stem is substituted into each of the dep-patterns to make the dependency names (one from each dep-pattern).

If we then expand this we have:

mod-tidy-cdi mod-tidy-validate: mod-tidy-%:

Meaning that this will effectively result in two "rules" with the stem matching cdi and validate respectively. This ensures that the commands associated with the rule have $(*) set to cdi and validate repspectively.

As a note, I use these kinds of rules in our Makefiles to cover cases where we are adding new commands, for example. Here, I generate the top-level targets using wildcards and use replacements and static pattern rules to define generic targets. This means that no make changes are required to add new commands.

If we think that this is excessive for this repo, I can expand these rules instead.

[klihub]

there are no prerequisites defined. This is a static pattern rule of the form:

Thank you ! That is exactly the snippet I was looking for in the same piece of documentation yesterday, but I failed to find it.

[klihub]

What is the purpose of mod-verify-% mod-tidy-% here ? I think they will evaluate to empty. Shouldn't this be instead

$(MOD_VERIFY_TARGETS): $(MOD_TIDY_TARGETS)

[klihub]

That said, I think strictly in the make sense something like mod-verify-%: mod-tidy-% is more correct: "whenever verifying a module, tidy it up first"...

[elezar]

This means that each mod-verify-% target depends on its specific mod-tidy-% target. This means that for each one tidy is run before verify.

I don't think that we need to enforce the global dependencies here i.e: before ANY verify target is run, run ALL tidy targets.

[klihub]

But a simple mod-verify-%: mod-tidy-% would already cover that dependency. There is no need for the $(MOD_VERIFY_TARGETS): there in the beginning.

And I actually misread that whole line before posting my original comment. I thought it said $(MOD_VERIFY_TARGETS): mod-verify-% mod-tidy-%...

But even now with that more careful look, I have to say that I still don't understand what does that extra $(MOD_VERIFY_TARGETS): accomplishes there... Actually and I wasn't even aware you could write a rule like that... or if I ever did I've long forgotten since.

[elezar]

Without the initial part the pattern rules are not matched. Afk at the moment, so let me find a docs link tomorrow.

[elezar]

See static pattern rules

[klihub]

I'm still confused about whether this makes sense and what does the extra variable-target in the beginning means:

$(CMD_MOD_TIDY_TARGETS): mod-tidy-%: mod-tidy-root

But if others (know and) are fine with that then I'm fine with it too.
@bart0sh ?

[bart0sh]

looks good to me overall.

One thing though. Do we need this level of granularity? Would it be enough and simpler to have only 2 targets mod-tidy and mod-verify that would just walk through all modules dirs (something like $(git ls-files '*go.mod' | xargs dirname)) and run either go mod tidy or go mod verify for all of them?

[elezar]

@elezar

Does this guarantee ordering? We would need the commands to be run in the root before the sub-folders. Like I said, I'm > happy to change the way we are doing things here if we feel strongly about it.

It does guarantee it if directories are sorted:

$ git ls-files '*go.mod' | xargs dirname | sort
.
cmd/cdi
cmd/validate

I don't feel strongly about it though. I think this could be simpler and clearer this way, but I'm generally ok with your version.

[elezar]

@bart0sh @klihub anything that we feel still needs to be done here?

I can re-implement this if we feel strongly about it, otherwise I'd like to get this merged.

[klihub]

@bart0sh @klihub anything that we feel still needs to be done here?

I can re-implement this if we feel strongly about it, otherwise I'd like to get this merged.

No too strong feelings. I'm fine with getting it merged.