Procedure for removing "unused" flags

[lberki]

Bazel currently has more than a thousand flags, which is way too many.

A large part of these flags is unused or mostly-unused (for our Google-internal version, telemetry shows that about a third of the flags have not been used in our monorepo in the last 90 days). This suggests that a significant cleanup is possible.

What should the sufficient conditions be for removing a flag? Given that we have no visibility at all into what features of Bazel are used and how often, the best I can think of is to search code on GitHub (or maybe on the wider web), and if there is only a "small enough" number of hits, go ahead with the deletion.

Any better ideas?

[ulrfa]

Searching GitHub (or wider web) might not reflect internal usage in companies.

Would it make sense to list candidates for removal, and then have a poll where people can vote for those flags they think are important to preserve?

[keith]

If you wanted a safer way than a poll I suppose you could make options that were candidates for removing start to warn and leave that for a release before removing them, but that would likely extend the timeline

[lberki]

Yeah, the official was is to add a --incompatible_* flag, but adding a flag to remove another sounds silly.

[keith]

I think it would be really interesting if you could share a list of flags that are unused at google since I imagine it would lead to some obvious candidates for removal.

[lberki]

See below as a reply to @aiuto 's comment.

[aiuto]

@lberki Do you think we can open that list up?

[lberki]

Here is the list of 251 Bazel flags that were unused in the 90 days up to around a week ago according to Google-internal telemetry (we only have logs about the use within our monorepo),:

https://gist.github.com/lberki/a07ffdd3799a69583512c626cb7b8997

A quick glance at the code tells that there are at least two classes:

• Flags that do the same thing at Google and in Bazel and which seem to be genuinely unused (e.g. --line_terminator_null)
• Flags whose function is not used in the Google monorepo but is very useful for Bazel so one can't derive a signal about Bazel from the use at Google (e.g. --experimental_repository_remote_exec)

[aiuto]

I didn't realize lberki started this thread in the first place.

[lberki]

The question above is still relevant regardless of who started the thread :)

[sluongng]

What should the sufficient conditions be for removing a flag?

Mark it for deprecation at least 1 stable release should be adequate for removal I think.
Deprecation should come with (if not already) some verbose WARN log to communicate to the end user regarding the expected removal time.

Experimental flags need to be treated with more care though.
There are plenty of experimental flags that people depend on day-to-day so there should be a discussion regarding stabilizing them or removing them after 1 stable release.

[lberki]

I'm ambivalent about experimental flags; the contract there is very explicitly that we are allowed to do anything with them at any time, and if we don't exercise that power, the distinction between experimental flags and non-experimental ones will disappear (cue Hyrum's law)

Marking flags as deprecated for a full stable release would mean that one could remove a flag only after a year in the worst case (if the decision is made right after cutting a stable release) ; for widely-used but harmful ones, I think that's completely reasonable, but not for ones that are very niche or unused altogether. But then again, how does one determine into which category a particular flag falls?

[sluongng]

I do see the argument from your point of view. However, there has not been a strong merit to drive the stabilization of experimental flags. This led to many flags left there as experimental forever(?). That's not a good state to be in, especially if you think about new Bazel adopters who look for best practices and were pointed to flags that are experimental as the only solution.

Here are a very few that I have in mind:

--experimental_remote_downloader
--experimental_downloader_config
--experimental_inmemory_jdeps_files

So with that reality, my argument is we either need to have a process to stabilize/remove the experimental flags after a fixed period/set of releases Or accept it for what it is: not all are experimental, and be more careful when we attempt to remove them.

[lberki]

We do have a problem with making up our collective minds about --experimental_* flags indeed. With that in mind, how about this plan:

1. I'll endeavor to scan GitHub for these ~200 flags for any uses. If I determine that a given flag has significant presence, the flag will be removed from the list of candidates to remove.
2. After that is done, I'll share the list of candidates here and on bazel-discuss for visibility in some way that gives people a way to object to the removal of any given flag. Objections will be considered, but the presence of an objection will not be an absolute contraindication for the removal of the flag.
3. The remaining candidates will be removed.

This is quite a flawed process, but I have no better idea :(

[sluongng]

I don't have an objection here. I think others have mentioned it here as well, but some flags such as experimental_downloader_config are used exclusively in private enterprise settings. So as long as there is sufficient time given to the objection process in (2), we should be Gucci 🤗

[lberki]

Update: it looks like in the absence of other opinions, we'll go with the above proposal: we compile a list of flags we want to remove, publish it, then wait for feedback.

[lberki]

We have identified a set of 69 flags candidates for removal:

https://gist.github.com/lberki/e187abc04a59b6d3d1eb0e42e515fb60

Please take a look and chime in if you think you need a flag from this list!

There may be some more once we get further with the Google-internal pass, but this is already a good start.

[lberki]

@tjgq that sounds reasonable, but as a separate effort. I don't want to conflate functional changes with code cleanups. I'd totally review the change if you sent it to me, though!

[sluongng]

experimental_repository_hash_file

This probably could be removed once BzlMod landed (post 7.0). Then the question is: do we still need bazel sync?

experimental_multi_threaded_digest

This is not used outside Google because the multi-threaded hash algorithm has not been open-sourced.
Is there any plan to open source that part of the code base?

incompatible_disable_third_party_license_checking

Sounds important... But seems like a Google-only flag

incompatible_use_platforms_repo_for_constraints
toolchain_resolution_override

These have already been removed?

legacy_external_runfiles
show_warnings

Not sure if this should be flipped before removal.

[benjaminp]

experimental_multi_threaded_digest

This is not used outside Google because the multi-threaded hash algorithm has not been open-sourced. Is there any plan to open source that part of the code base?

That's not what this flag was for. In fact, it's already a no-op, but #1210 is the history.

[sluongng]

That's not what this flag was for. In fact, it's already a no-op, but #1210 is the history.

Thanks for pointing that out!

I was confusing it with what was mentioned in bazelbuild/remote-apis#235 (comment) 👀

[lberki]

--incompatible_disable_third_party_license_checking is IIRC a remnant of a flag flip internal to Google. It's also a no-op so there isn't an argument for it to stay.

--experimental_multi_threaded_digest is, as @benjaminp says, a no-op.

For the rest of the flags that still exist, I'll start separate threads below.

[lberki]

--experimental_repository_hash_file: @meteorcloudy indicated that there isn't a good use case for this, and therefore should be removed (cc @sluongng ) re: bazel sync, I don't want to take sides, at least here; let's keep the discussion focused on flags.

[sluongng]

I think the context was properly documented here https://blog.bazel.build/2018/07/09/bazel-sync-and-resolved-file.html

Since that blog post came out, there have been a few organizations that adopted the flag as a way to help validate reproducible builds. So, at minimum, it's worth putting out some comm regarding (a) what is being removed and (b) how folks should migrate from using it.

In this case, I believe the replacement for bazel sync and the resolved file is BzlMod(?).

[meteorcloudy]

OK, the exact functionality this flag provides is to check the hash of the external repository directory, which isn't considered in the Bzlmod design. @Wyverald is implementing a true shared repo cache, maybe this is relevant.

[Wyverald]

This can be subsumed by the module lockfile so I think we can remove it.

[lberki]

--show_warnings: this is not a flag to be flipped, but a knob on bazel canonicalize-flags, so as long as we don't have a use for it I say, let's remove it.

[lberki]

--legacy_external_runfiles: this should at least be renamed to --incompatible_disable_legacy_external_runfiles, but given that it's part of the long road towards flipping --experimental_sibling_repository_layout, on second thought, it should stay. WDYT @meteorcloudy ?

[meteorcloudy]

Yeah, we'll also have to evaluate how much downstream damage it causes.

[aiuto]

Yes. --incompatible_disable_third_party_license_checking is a remnant. I deleted the code and moved it to the graveyard a year ago. I just created a PR to remove it.

[gregestren]

I think we should be somewhat aggressive. If a flag looks like it could potentially be theoretically useful but there's no evidence anyone's using it, let's delete it. If there's really a problem for someone we can put it back in a patch release.

i.e. we should push back against responses like "oh I could totally see this being useful". We want evidence of someone actually practically using it, or having upcoming concrete plans to use it, as a threshold for preserving. Theoretical utility is just brainstorming, which isn't a strong enough argument for production code.

I'm particularly thinking of

bazel/src/main/java/com/google/devtools/build/lib/analysis/AnalysisOptions.java

Line 54 in 735e59d

name = "max_config_changes_to_show",

as I write this.

[gregestren]

Any flag that was introduced years ago and got no traction is a strong candidate for this, whatever its original intention.

[lberki]

I agree, although the benefit of removal has to be weighed against the potential disruption. I don't mind leaving in a tiny knob in that takes five lines of code to implement, but I do mind having large swathes of code around that no one or only vanishingly few people use.

[lberki]

On second thought (while aggressively removing obsolete options), let's keep this option alive; to the best of my understanding, it's the only way to show all config differences, which sounds like useful functionality.

(@gregestren if you feel strongly that it should be gone, I'll nevertheless remove it)

[gregestren]

I think the principles we talk about are good. But if we're agonizing over a specific small-impact flag then no I don't have strong feelings about removing it.

[martis42]

Thanks for taking the initiative on this cleanup 👍

If you don't mind I would like to share a "outside of Google" user perspective on this issue.

Generally speaking I believe it is worthwhile to cleanup old flags. Even if they only relate to 5 small lines in Bazel code. All those flags needlessly enlarge the Bazel documentation which is read quite often by many people. Removing superfluous parts from the already large and hard to grasp documentation is imho a worthwhile effort.

The situation regarding experimental flags is annoying from the perspective of one not having insight into Google. It seems there are several flags which are now experimental for several major releases but don't change. As user one does not know if the feature was given up and forgotten or if there are major concerns keeping it in experimental state.
From user perspective it would be great if experimental flags reliable would perform one of the following changes after being available for 2 or more major releases without changes to the feature protected by the flag

• Remove the flag and the feature as it did not play out as expected
• Make it a normal feature
• Link a tracking GitHub issue in the documentation which outlines why the flag stays experimental, but it is still desired to make it stable eventually

[lberki]

The final tally is in!

As per the above discussion, the list of flags to be removed is here:

https://gist.github.com/lberki/1f9c599185214ecf5c4b03b03fa9f674

It's very similar to the above candidate list, the only difference being --legacy_external_runfiles. I haven't separately removed the flags that don't exist anymore so that the two lists can be easily compared and because there isn't a lot of room for error there: one can't do much harm when working on a non-existent flag.

[lberki]

@martis42 I understand your concerns; I am also very frustrated about the menagerie of "eternally experimental" flags we have. But I think it's best treated as an effort that's separate from removing unused flags.

My game plan is to pick the "low-hanging fruit" here, i.e. the definitely unused flags, then work my way up to flags that are less and less obviously unnecessary.

You are not the first person to recommend such a "tracking system" for experimental flags (IIRC @joeleba wanted to have one, too)., My take on this is that processes are easy to add and difficult to remove, so I'd rather only do that if the benefit is obvious and if I am convinced that we can actually keep ourselves to the process. I don't have an opinion as to whether these two requirements would hold; I wanted to tackle this after figuring out what to do with the flags that already exist and are not tracked that way, but only after we have made some dent in removing unused or barely-used flags.