Subject: bug#73801: 31.0.50; project-try-vc sometimes set wrong cache project-vc-extra-root-markers

From: Dmitry Gutov <[email-masked]>
Date: Tue, 29 Oct 2024 22:31:04 +0200

On 29/10/2024 15:33, Eli Zaretskii wrote:
>> Date: Tue, 29 Oct 2024 04:44:13 +0200
>> From: Dmitry Gutov <[email-masked]>
>> Cc: Zhengyi Fu <[email-masked]>
>>
>> Since I see some changes added to the release branch still,
>>
>> On 28/10/2024 06:06, Dmitry Gutov wrote:
>>> It would be nice to get either of the patches into Emacs 30, too, but it
>>> might be a little late given where it is in the pretest.
>>
>> Eli, could we install either of the fixes for this bug to emacs-30 too?
>>
>> The one I installed on master is longer but should result in less I/O,
>> while the patch by Zhengyi Fu is a one-liner, which might feel a little
>> safer.
> 
> I don't understand the implications of that one-line (nor, TBH, the
> analysis of the original problem), so I'm not sure these changes are
> safe.

The original problem was due to project-try-vc being invoked recursively 
on a parent directory while a variable that affects its computation is 
bound to nil. The function itself (project-try-vc) memoizes its return 
value. As a result, any subsequent call to it with the same argument 
outside of the said binding could return wrong result.

The first fix (one-liner) made sure that we're calling it with the same 
argument that is passed to the current call, ensuring that the cache 
will be rewritten after it returns.

The second fix (mine) was to extract the value computation into a helper 
function, making the recursive call to not be memoized.

> How do we know that catering to this corner case will not screw
> other corner cases?

Difficult to guarantee that 100%, but this specific case seems important 
enough, while at the same time we can infer that the change won't affect 
the majority scenario because the code is guarded by these conditions:

     (when root
       (when (not backend)
          ...