On Sun, Dec 19, 2021 at 5:51 PM <rsbecker@xxxxxxxxxxxxx> wrote: > On December 19, 2021 5:23 PM, Sean Allred wrote: > > > what about the comparison code where is_bare_repository_cfg is > > > compared with 1 (it is a boolean and sometimes set to -1). This would > > > not generally pass a code review. > > > > I'm sorry, I'm afraid I don't completely follow. Wouldn't the most > > straightforward change be to simply follow the documented > > recommendation when we create the worktree config in `git sparse- > > checkout init`? Specifically, > > > > + if (is_bare_repository()) > > + git_config_set_in_file_gently(config_path, "core.bare", "false"); > > + > > > > Are we saying the comparison within is_bare_repository() may not be > > appropriate in this case? > > I'm suggesting that: > > worktree->is_bare = (is_bare_repository_cfg == 1) || > is_bare_repository(); > > the == 1 comparison should not be done for boolean-style variables. It is an int, but initialized to -1. Unless -1 and 1 mean different things, but that is not really documented. `is_bare_repository_cfg` is not exactly a boolean; it's a tristate, with -1 meaning "not yet determined". I didn't, at the time, closely follow the discussion[1] of the particular bit of code you're questioning, but the `== 1` was mentioned at least a couple times, once in review by Junio[2], and then in the extra patch commentary by "jtan" when he submitted v2[3]. Anyhow, if I'm following the original discussion correctly, then the usage, `== 1` (or the equivalent `> 0`) is probably correct, and that treating it as a simple boolean (where -1 is true, too) would be undesirable. (Of course, I haven't traced through the init code at all, so I don't even know if it can ever be -1 at this point.) Five existing consumers of this global variable use `== 1`, and only two use `> 0`, so this usage is at least reasonably consistent with other parts of the project. [1]: https://lore.kernel.org/git/20190419172128.130170-1-jonathantanmy@xxxxxxxxxx/T/ [2]: https://lore.kernel.org/git/xmqqo954gira.fsf@xxxxxxxxxxxxxxxxxxxxxxxxx/ [3]: https://lore.kernel.org/git/20190419172128.130170-1-jonathantanmy@xxxxxxxxxx/
