Re: [PATCH v5 4/7] repo-settings: prepare_repo_settings only in git repos

[Lessley Dennington <lessleydennington@gmail.com>]

On 12/6/21 8:43 PM, Ævar Arnfjörð Bjarmason wrote:
> 
> On Fri, Dec 03 2021, Lessley Dennington via GitGitGadget wrote:
> 
>> From: Lessley Dennington <lessleydennington@gmail.com>
>>
>> Check whether git directory exists before adding any repo settings. If it
>> does not exist, BUG with the message that one cannot add settings for an
>> uninitialized repository. If it does exist, proceed with adding repo
>> settings.
>>
>> Signed-off-by: Lessley Dennington <lessleydennington@gmail.com>
>> ---
>>   repo-settings.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/repo-settings.c b/repo-settings.c
>> index b93e91a212e..00ca5571a1a 100644
>> --- a/repo-settings.c
>> +++ b/repo-settings.c
>> @@ -17,6 +17,9 @@ void prepare_repo_settings(struct repository *r)
>>   	char *strval;
>>   	int manyfiles;
>>   
>> +	if (!r->gitdir)
>> +		BUG("Cannot add settings for uninitialized repository");
> 
> nit: start BUG(), error() etc. messages with lower-case.
> 
Thanks for catching this. Instead of re-sending the whole series for a
one-letter change, I've included a patch with the fix at the end of this
message.
>> +
>>   	if (r->settings.initialized++)
>>   		return;
> 
> Our config doesn't require us to have a repo, and most of what
> prepare_repo_settings() is doing is reading global config.
> 
> I think that *currently* this won't break things, but e.g. if we ever
> want to have "feature.experimental" or whatever change the behavior of a
> a command that doesn't require a repository we'd need to untangle this
> (currently everything it changes requires a repo AFAICT).
> 
> Perhaps this is fine, and if we ever need such a "global config" point
> we should stick it closer to common-main.c...
> 
Agreed, let's keep the change for now and address updates in the future if
the need arises.
-----------
 From 2764ca684320f488006093505deb074217ac7b31 Mon Sep 17 00:00:00 2001
From: Lessley Dennington <lessleydennington@gmail.com>
Date: Wed, 8 Dec 2021 09:29:56 -0600
Subject: [PATCH] fixup! repo-settings: prepare_repo_settings only in git repos

Update the BUG() message in prepare_repo_settings to begin with a
lower-case letter.

Reported-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Lessley Dennington <lessleydennington@gmail.com>
---
  repo-settings.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/repo-settings.c b/repo-settings.c
index 00ca5571a1..2644ac3353 100644
--- a/repo-settings.c
+++ b/repo-settings.c
@@ -18,7 +18,7 @@ void prepare_repo_settings(struct repository *r)
         int manyfiles;

         if (!r->gitdir)
-               BUG("Cannot add settings for uninitialized repository");
+               BUG("cannot add settings for uninitialized repository");

         if (r->settings.initialized++)
                 return;
--
2.34.0.vfs.0.0