On Wed, Nov 14, 2018 at 02:52:24PM +0900, Junio C Hamano wrote: > "Johannes Schindelin via GitGitGadget" <gitgitgadget@xxxxxxxxx> > writes: > > > Back when bw/config-h was developed (and backported to Git for Windows), I > > came up with this patch. It seems to not be strictly necessary, but I like > > the safety of falling back to the Git directory when no common directory is > > configured (for whatever reason). > > Shouldn't that be diagnosed as an error with BUG()? That is, if we > know there is the current repository, and if commondir is not set, > isn't it a bug that we want to look into in the start-up sequence? > > The phrase "for whatever reason" makes me wonder if this is truly > "defensive programming", rather than just sweeping the bug under the > rug. > > FWIW, with this toy patch applied on top of this patch, all tests > that I usually run seem to pass. Heh, I independently made the same change after reading the patch and came here to report similar results. I think the key thing here is that git_dir is never meant to be used as a source for config. It is there to let the "includeIf gitdir" directive work. So it would indeed be surprising and a bug if we had one but not the other. -Peff
