Re: [PATCH v10 33/40] environment: add set_index_file()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Aug 10, 2016 at 7:34 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Christian Couder <christian.couder@xxxxxxxxx> 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
hold_lock_file_for_update().
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
> series).

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.

Thanks,
Christian.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]