On 06/06, Duy Nguyen wrote: > On Wed, Jun 6, 2018 at 6:50 PM, Brandon Williams <bmwill@xxxxxxxxxx> wrote: > > On 06/06, Nguyễn Thái Ngọc Duy wrote: > >> Make the attr API take an index_state instead of assuming the_index in > >> attr code. All call sites are converted blindly to keep the patch > >> simple and retain current behavior. Individual call sites may receive > >> further updates to use the right index instead of the_index. > >> > >> There is one ugly temporary workaround added in attr.c that needs some > >> more explanation. > >> > >> Commit c24f3abace (apply: file commited * with CRLF should roundtrip > >> diff and apply - 2017-08-19) forces one convert_to_git() call to NOT > >> read the index at all. But what do you know, we read it anyway by > >> falling back to the_index. When "istate" from convert_to_git is now > >> propagated down to read_attr_from_array() we will hit segfault > >> somewhere inside read_blob_data_from_index. > >> > >> The right way of dealing with this is to kill "use_index" variable and > >> only follow "istate" but at this stage we are not ready for that: > >> while most git_attr_set_direction() calls just passes the_index to be > >> assigned to use_index, unpack-trees passes a different one which is > >> used by entry.c code, which has no way to know what index to use if we > >> delete use_index. So this has to be done later. > > > > Ugh I remember this when I was doing some refactoring to the attr > > subsystem a year or so ago. I really wanted to get rid of the whole > > "direction" thing because if I remember correctly its one of the only > > remaining globals that affects the outcome of an attr check (everything > > else was converted to use the attr check struct. I always got way to > > far into the weeds when trying to do that too. I'm not expecting that > > from this series (since its way out of scope) but maybe it'll be easier > > afterwards. > > It's not that much easier. This direction thing is still global by > design. It would be great if we have something like Scheme's parameter > (aka. dynamic scoping iirc) then we can still scope this nicely. Git > does not live in such worlds. Yeah i realized this after sending. Either way thanks for simplifying the global state by getting ride of the index global. > -- > Duy -- Brandon Williams
