On Wed, Aug 10, 2016 at 7:34 PM, Junio C Hamano <gits...@pobox.com> wrote:
> Christian Couder <christian.cou...@gmail.com> writes:
>>> Isn't the mention on NO_THE_INDEX_COMPATIBILITY_MACROS in the added
>>> comments (there are two) pure red-herring?
>> Yeah, true.
>> So do you want me to refactor the code to use
>> hold_lock_file_for_update() instead of hold_locked_index() and to
>> avoid the set_index_file() hack?
> I somehow thought that we already agreed to accept the technical
> debt, taking your "it is too much work" assessment at the face
> value. If you now think it is feasible within the scope of the
> series, of course we'd prefer it done "right" ;-)
Yeah, it is feasible and perhaps even simpler using
hold_lock_file_for_update() than with set_index_file(), so I dropped
the set_index_file() patch and added a new one that uses
Sorry to have understood only recently what you said in some previous
emails and thanks for the explanations.
> Is the problematic hold_locked_index() something you do yourself, or
> buried deep inside the callchain of a helper function you use?
It is something done by the apply code, so we can easily modify that.
> I am
> sensing that it is the latter (otherwise you wouldn't be doing the
> hack or at least will trivially fixing it up in a later "now we did
> a faithful conversion from the previous code using GIT_INDEX_FILE
> enviornment variable in an earlier step. Let's clean it up by being
> in full control of what file we read from and write to" step in the
It was more a misunderstanding of how the index related functions are working.
> Do you also have an issue on the reading side (i.e. you write it out
> to temporary file and then later re-read the in-core cache from that
> temporary file, for example)? Or is a single "write to a temporary
> index" the only thing you need to work around?
It looks like it is the latter.
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html