✨ Add --load-build-tags flag

[joelanford]

Fixes #1158

An alternative to #1160

[joelanford]

Taking my self-approved label off to get another approver's approval (say that 10 times fast! 😄)

[tmshort]

Any update on this @joelanford ?

[sbueringer]

loader.LoadRootsWithConfig

	// put our build flags first so that callers can override them
	l.cfg.BuildFlags = append([]string{"-tags", "ignore_autogenerated"}, l.cfg.BuildFlags...)

Is it correct that by adding -tags here we overwrite -tags", "ignore_autogenerated"?

What about using ignore_autogenerated as the default value for our flag?

I think this probably (?) would result in:

• the same behavior as before if the flag is not set by users
• allows to add additional tags
• allows to disable ignore_autogenerated

(not entirely sure, would be good to double check)

[tmshort]

I would require one to remember to add ignore_autogenerated but it's the most flexible.
Unless we think that ignore_autogenerated is required?

[joelanford]

Good catch. Yes, it appears as if ignore_autogenerated will not be used if tags are provided on the command line.

I implemented a small test project to check behavior. The last -tags flag wins, and all previous tags flags are ignored.

So the behavior of what is in the PR now is:

• If no tags are passed in the flag, then the last tags flag is "ignore_autogenerated", and that's what is used.
• If any tags are passed in the flag, "-tags=ignore_autogenerated" is ignored. Only explicit user-defined tags are used.

So I think putting ignore_autogenerated in the default flag value actually has the same outcome, but it is also more explicit, and it makes it easier for our future selves to avoid the question if what happens when -tags is used multiple times.

I can't think of a good reason to force ignore_autogenerated. I think it is better to make that the default but then let a user override that and pass their desired set of tags, which may or may not include ignore_autogenerated.

I'll update the PR to account for this convo.

[joelanford]

Oh, there's one problem moving ignore_autogenerated to the flag default. Users of loader.LoadRoots and loader.LoadRootsWithConfig (and any other functions up the call stack that call those functions) will be broken because those functions would suddenly stop including that build tag.

[joelanford]

So perhaps we put ignore_autogenerated in the flag default (for visibility/clarity), but then also keep a separate (and overridable) -tags=ignore_autogenerated build flag hardcoded in LoadRootsWithConfig with some heavy comments explaining the situation.

[sbueringer]

Makes sense to me

[sbueringer]

Thx!

/lgtm
/approve

[k8s-ci-robot]

LGTM label has been added.

Git tree hash: ebdca06b18b31d24463329f042454cc8f49cc3ef

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: joelanford, sbueringer

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [joelanford,sbueringer]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[sbueringer]

/cherry-pick release-0.17

[k8s-infra-cherrypick-robot]

@sbueringer: #1161 failed to apply on top of branch "release-0.17":

Applying: Add `--build-tags` flag
Using index info to reconstruct a base tree...
M	cmd/controller-gen/main.go
Falling back to patching base and 3-way merge...
Auto-merging cmd/controller-gen/main.go
CONFLICT (content): Merge conflict in cmd/controller-gen/main.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config advice.mergeConflict false"
Patch failed at 0001 Add `--build-tags` flag

In response to this:

/cherry-pick release-0.17

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[sbueringer]

@joelanford @tmshort I created the release-0.17 branch from the commit right before we bumped to the next k8s.io/* minor dependencies. We don't always create release branches in controlller-tools. We just keep releasing patch versions from main until the next minor. But in this case we shouldn't because then we would include the 0.33 k8s.io/* depedencies.

Can you plesae open a "manual" cherry-pick PR against release-0.17?

[joelanford]

@sbueringer #1181