git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [RFC v1] Add virtual file system settings and hook proc

From
Ben Peart <peartben@gmail.com>
Date
Nov 5, 2018, 20:00 UTC
Message-ID
<140226a2-a40e-1965-b665-46eec81d907a@gmail.com>
In-Reply-To
<xmqqa7momlx6.fsf@gitster-ct.c.googlers.com>
On 11/4/2018 7:02 PM, Junio C Hamano wrote:
Show 47 quoted lines
> Ben Peart <peartben@gmail.com> writes:
> 
>>>> +	if (*dtype == DT_UNKNOWN)
>>>> +		*dtype = get_dtype(NULL, istate, pathname, pathlen);
>>>
>>> We try to defer paying cost to determine unknown *dtype as late as
>>> possible by having this call in last_exclude_matching_from_list(),
>>> and not here.  If we are doing this, we probably should update the
>>> callpaths that call last_exclude_matching_from_list() to make the
>>> caller responsible for doing get_dtype() and drop the lazy finding
>>> of dtype from the callee.  Alternatively, the new "is excluded from
>>> vfs" helper can learn to do the lazy get_dtype() just like the
>>> existing last_exclude_matching_from_list() does.  I suspect the
>>> latter may be simpler.
>>
>> In is_excluded_from_virtualfilesystem() dtype can't be lazy because it
>> is always needed (which is why I test and die if it isn't known).
> 
> You make a call to that function even when virtual-file-system hook
> is not in use, i.e. instead of the caller saying
> 
> 	if (is_vfs_in_use()) {
> 		*dtype = get_dtype(...);
>                  if (is_excluded_from_vfs(...) > 0)
> 			return 1;
> 	}
> 
> your caller makes an unconditional call to is_excluded_from_vfs().
> Isn't that the only reason why you break the laziness of determining
> dtype?
> 
> You can keep the caller simple by making an unconditional call, but
> maintain the laziness by updating the callig convention to pass
> dtype (not *dtype) to the function, e.g..
> 
> 	if (is_excluded_from_vfs(pathname, pathlen, dtype) > 0)
> 		return 1;
> 
> and then at the beginning of the helper
> 
> 	if (is_vfs_in_use())
> 		return -1; /* undetermined */
> 	*dtype = get_dtype(...);
> 	... whatever logic it has now ...
> 
> no?
> 

Oops! You're right, I docall get_dtype() even if the vfs isn't in use. I'll add an additional test to avoid doing that. Thank you.

I did look into moving the delay load logic into is_excluded_from_vfs() but get_dtype() is static to dir.c and I'd prefer to keep it that way if possible.

Show 9 quoted lines
>> Your comments are all feedback on the code - how it was implemented,
>> style, etc.  Any thoughts on whether this is something we could/should
>> merge into master (after any necessary cleanup)?  Would anyone else
>> find this interesting/helpful?
> 
> I am pretty much neutral.  Not strongly opposed to it, but not all
> that interested until seeing its integration with the "userland" to
> see how the whole thing works ;-)
> 
Previous: Junio C HamanoNext: Duy Nguyen
Message 5 of 21 in “[RFC v1] Add virtual file system settings and hook proc”
  1. Ben PeartOct 30, 2018
  2. Junio C HamanoOct 30, 2018
  3. Ben PeartOct 31, 2018
  4. Junio C HamanoNov 5, 2018
  5. Ben PeartNov 5, 2018
  6. Duy NguyenOct 31, 2018
  7. Ben PeartOct 31, 2018
  8. Duy NguyenNov 4, 2018
  9. brian m. carlsonNov 4, 2018
  10. Duy NguyenNov 5, 2018
  11. Ben PeartNov 5, 2018
  12. Ben PeartNov 5, 2018
  13. Ævar Arnfjörð BjarmasonNov 5, 2018
  14. Duy NguyenNov 5, 2018
  15. Ben PeartNov 5, 2018
  16. Johannes SchindelinNov 5, 2018
  17. teach git to support a virtual (partially populated) work directoryBen Peart, Nov 27, 2018
  18. SZEDER GáborNov 28, 2018
  19. Ben PeartNov 29, 2018
  20. teach git to support a virtual (partially populated) work directoryBen Peart, Dec 13, 2018
  21. Ben PeartJan 28, 2019

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.