On 2018-01-05 20:00, Lars Schneider wrote: > >> On 01 Jan 2018, at 22:59, tboegi@xxxxxx wrote: >> >> From: Torsten Bögershausen <tboegi@xxxxxx> >> >> 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@xxxxxxxxx> >> Signed-off-by: Torsten Bögershausen <tboegi@xxxxxx> >> --- >> 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? All these flags have a different context, and the right #define must be used in the right context/function call. So there is no intention. You start with 1, then use 2 and so on. The same way as family Schmidt and family Meier both call their child "Hans". There is no connection. > > Thanks, > Lars > > > More context: > https://public-inbox.org/git/96B6CD4C-0A0C-47F5-922D-B8BAFB832FD1@xxxxxxxxx/ > (3) We kind of replicate some flags defined in cache.h: > #define HASH_WRITE_OBJECT 1 > #define HASH_RENORMALIZE 4 > >
