Re: [PATCH v2.1] commit: add --ignore-submodules[=<when>] parameter

From: Jens Lehmann <jens.lehmann@web.de>
Date: Mar 31, 2014, 20:37 UTC
Message-ID: <5339D213.4070303@web.de>
In-Reply-To: <5339BAE4.8020306@web.de>

Am 31.03.2014 20:58, schrieb Jens Lehmann:

Show 13 quoted lines

> Am 31.03.2014 02:07, schrieb Ronald Weiss:
>> The tests immediately revealed, that the patch was not complete. It
>> didn't work with commit message given on command line (-m). To make
>> that work, I had to also patch the index_differs_from function in
>> diff-lib.c.
> 
> Which is exactly the same function I have to tweak to make my
> "status/commit: always show staged submodules regardless of
> ignore config" patch work for "git commit -m" too ;-)
> 
> I was doing that slightly differently but it seems that your way
> of adding the "ignore_submodules_arg" parameter could serve us
> both. Will update my upcoming patch accordingly.

I've been hacking on that some more (I'll post it as soon as I have all new tests up and running) and think we might be able to handle that even easier. Please see below:

Show 20 quoted lines

>> diff --git a/diff-lib.c b/diff-lib.c
>> index 2eddc66..fec5ad4 100644
>> --- a/diff-lib.c
>> +++ b/diff-lib.c
>> @@ -508,7 +508,8 @@ int do_diff_cache(const unsigned char *tree_sha1, struct diff_options *opt)
>>  	return 0;
>>  }
>>  
>> -int index_differs_from(const char *def, int diff_flags)
>> +int index_differs_from(const char *def, int diff_flags,
>> +		       const char *ignore_submodules_arg)
>>  {
>>  	struct rev_info rev;
>>  	struct setup_revision_opt opt;
>> @@ -520,6 +521,10 @@ int index_differs_from(const char *def, int diff_flags)
>>  	DIFF_OPT_SET(&rev.diffopt, QUICK);
>>  	DIFF_OPT_SET(&rev.diffopt, EXIT_WITH_STATUS);
>>  	rev.diffopt.flags |= diff_flags;
>> +	if (ignore_submodules_arg) {
>> +		DIFF_OPT_SET(&rev.diffopt, OVERRIDE_SUBMODULE_CONFIG);

You'll have to do this unconditionally, as this option tells the diff machinery to ignore any submodule configurations, which is what we want in either case. But ...

Show 5 quoted lines

>> +		handle_ignore_submodules_arg(&rev.diffopt, ignore_submodules_arg);
>> +	}
>>  	run_diff_index(&rev, 1);
>>  	if (rev.pending.alloc)
>>  		free(rev.pending.objects);

... maybe the best way is to leave index_differs_from() unchanged and call that function with the correct diff_flags instead:

+ int diff_flags = DIFF_OPT_OVERRIDE_SUBMODULE_CONFIG; + if (ignore_submodule_arg && + !strcmp(ignore_submodule_arg, "all")) + diff_flags |= DIFF_OPT_IGNORE_SUBMODULES; + commitable = index_differs_from(parent, diff_flags);

Not thoroughly tested yet, but that'd also prevent any fallout for the two callsites of index_differs_from() in sequencer.c.