Re: [PATCH v3 1/1] convert_to_git(): checksafe becomes int conv_flags

From: Lars Schneider <larsxschneider@gmail.com>
Date: Jan 5, 2018, 19:00 UTC
Message-ID: <B45F57FC-E798-4C8F-9C40-200944D86029@gmail.com>
In-Reply-To: <20180101215959.1460-1-tboegi@web.de>

Show 32 quoted lines

> On 01 Jan 2018, at 22:59, tboegi@web.de wrote:
> 
> From: Torsten Bögershausen <tboegi@web.de>
> 
> When calling convert_to_git(), the checksafe parameter has been used to
> check if commit would give a non-roundtrip conversion of EOL.
> 
> When checksafe was introduced, 3 values had been in use:
> SAFE_CRLF_FALSE: no warning
> SAFE_CRLF_FAIL:  reject the commit if EOL do not roundtrip
> SAFE_CRLF_WARN:  warn the user if EOL do not roundtrip
> 
> Already today the integer value 0 is passed as the parameter checksafe
> instead of the correct enum value SAFE_CRLF_FALSE.
> 
> Turn the whole call chain to use an integer with single bits, which
> can be extended in the next commits:
> - The global configuration variable safe_crlf is now conv_flags_eol.
> - The parameter checksafe is renamed into conv_flags.
> 
> Helped-By: Lars Schneider <larsxschneider@gmail.com>
> Signed-off-by: Torsten Bögershausen <tboegi@web.de>
> ---
> This is my suggestion.
> (1) The flag bits had been renamed.
> (2) The (theoretical ?) mix of WARN/FAIL is still there,
>    I am not sure if this is a real problem.
> 
> (3) There are 2 reasons that CONV_EOL_RENORMALIZE is set.
>    Either in a renormalizing merge, or by running
>    git add --renormalize .
>    Therefor HASH_RENORMALIZE is not the same as CONV_EOL_RENORMALIZE.

Can you elaborate a bit? I am diving into the code but I am still confused.

I also noticed that the "flags" integer is potentially double booked with the following values (see read-cache.c:add_to_index()):

#define ADD_CACHE_VERBOSE 1 #define ADD_CACHE_PRETEND 2 #define ADD_CACHE_IGNORE_ERRORS 4

#define HASH_WRITE_OBJECT 1 #define HASH_FORMAT_CHECK 2 #define HASH_RENORMALIZE 4

Is this intentional?

Thanks, Lars

More context:
  https://public-inbox.org/git/96B6CD4C-0A0C-47F5-922D-B8BAFB832FD1@gmail.com/
  (3) We kind of replicate some flags defined in cache.h:
     #define HASH_WRITE_OBJECT 1
     #define HASH_RENORMALIZE  4