Junio C Hamano wrote: > Phillip Wood <phillip.wood123@xxxxxxxxx> writes: > >> This keeps the current behavior of not refreshing when --quiet is >> given. I wonder how disruptive it would be to take the opportunity to >> get rid of that hack and go back the the original behavior of >> refreshing when --quiet is given. There are a couple of assumptions >> that make me think it might be acceptable >> >> 1 - anyone using a sparse index wont notice as refreshing the index >> should be fast for them >> >> 2 - the large repositories that are affected exist in managed >> environments where an admin who reads the release notes could set >> reset.refresh in a central config so individual users are not >> inconvenienced. > > I would very much prefer to see "--quiet" not making contribution to > the decision to refresh or not in the longer term. Many plumbing > commands expect that the calling scripts refresh the index with an > explicit use of "update-index --refresh" and leave the index not > refreshed, but working on unrefreshed index is a trade-off between > performance and correctness. > > * Turning "--quiet" not to refresh may incur performance regression > for shorter term. It will not hurt correctness. > I tend to agree with you and Phillip on this. I took a more conservative approach with the intention of preserving as much backward compatibility as possible, but having '--quiet' disable refresh (to me) actively hurts its correctness. If backwards-compatibility isn't a huge concern, I'll gladly make that change. > * Introducing "--no-refresh" to mark "reset --quiet" invocations, > where the freshness of the index does not matter for correctness, > would help regain performance without breaking scripts. All > "reset --quiet" invocations in scripts written before this series > are supposed to be safe (as they lived with their "reset --quiet" > that does not refresh), but newly written scripts may start > expecting that "reset --quiet" would refresh for correctness. > > * If we allow reset.refresh to be set to "no", however, that would > affect _all_ uses of "reset --quiet", including the ones in newly > written scripts that expect "reset --quiet" to refresh. They > would be forced to say "reset --quiet --refresh", just in case > the user has such a configuration; otherwise these scripts will > be declared "buggy" for not explicitly saying "--refresh". > I added the option as a "replacement" for 'reset.quiet' (specifically, its ability to summarily disable refresh), but I can definitely see how it would lead to issues in the future. I'm happy to remove it, but should 'reset.quiet' be removed as well? No other commands have a config option for 'quiet', and it presents a similar issue of potentially suppressing a helpful feature (in this case, informational printouts) across all invocations unless otherwise specified. > I do not think reset.refresh is a good idea, but I very much like > the idea to making "reset" (regardless of "--quiet") to refresh by > default. > I was hesitant to go this far because it would force people that were comfortably relying on 'reset.quiet' to need to always use '--no-refresh' to get the same behavior. But, to Phillip's point earlier, there are other options now (like sparse index) that could provide a just-as-substantial (if not greater) performance boost without sacrificing the refresh. > Thanks. > > Since this is already in 'next' (and, in its current state, I don't think it does any more damage than the pre-series state), I'll send a new series on top of this that deprecates 'reset.refresh' and 'reset.quiet', and makes '--refresh' the default for all of 'reset'. Thanks, both!
