cody icon indicating copy to clipboard operation
cody copied to clipboard

[WIP] Use custom findFiles implementation that skips symlinks

Open DanTup opened this issue 1 year ago • 6 comments

[WIP - just an idea, related to https://github.com/sourcegraph/cody/pull/3033)

VS Code's findFiles() walks symlinks which can result in very slow locating of workspace ignore files (see https://github.com/sourcegraph/cody/pull/3033).

This reuses the agent/shim version of findFiles (which - perhaps accidentally - does not follow links) and also records the skipped symlinks as automatic ignores.

DanTup avatar Feb 09 '24 13:02 DanTup

While reviewing the latest VS Code release notes I saw there's a proposed API for findFiles that has a followSymlink bool:

https://github.com/microsoft/vscode/blob/4e2aa982224429f161423cbdded9587e39766782/src/vscode-dts/vscode.proposed.findFiles2.d.ts#L60

Also has fuzzy (IIRC, currently the context file search basically fetches all files and then does a local fuzzy match). Might be worth trying out locally (as a proposed API) to see whether it solves the issues @chwarwick was seeing.

DanTup avatar Feb 28 '24 19:02 DanTup

While reviewing the latest VS Code release notes I saw there's a proposed API for findFiles that has a followSymlink bool:

https://github.com/microsoft/vscode/blob/4e2aa982224429f161423cbdded9587e39766782/src/vscode-dts/vscode.proposed.findFiles2.d.ts#L60

Also has fuzzy (IIRC, currently the context file search basically fetches all files and then does a local fuzzy match). Might be worth trying out locally (as a proposed API) to see whether it solves the issues @chwarwick was seeing.

I also saw the new update but it mentioned that it'd still follow symlinks which was the main issue that Chris was seeing. But I agree it's worth waiting and test it out first

abeatrix avatar Feb 29 '24 17:02 abeatrix

I also saw the new update but it mentioned that it'd still follow symlinks

Yeah I thought the wording in the release notes was a bit confusing, but the definition linked above is clearer:

		/**
		 * Whether symlinks should be followed while searching.
		 * Defaults to the value for `search.followSymlinks` in settings.
		 * For more info, see the setting listed above.
		 */
		followSymlinks?: boolean;

Basically there's a flag that lets you choose the behaviour, and it defaults to the value of search.followSymlinks if not defined. So if you explictly pass false, it should skip symlinks regardless of the search.followSymlinks setting. (I haven't tried it out though).

DanTup avatar Mar 01 '24 00:03 DanTup

@DanTup Hey Danny is this still WIP or ready to review? Sorry for the late response since i was on PTO, but happy to pick up from here 😄

abeatrix avatar Mar 18 '24 15:03 abeatrix

Hey Danny is this still WIP or ready to review?

It was initally just an experiment to see whether it solved the issue and whether performance was acceptable. Based on the comments above, I'm not sure the performance is good enough that you'd want to use this instead of the current behaviour (although, slow and working may be better than fast and broken - but I don't know how many people are affected by the symlink issue).

The ideal solution would be for VS Code to finalize the APIs noted above and switch to them (should be fast, can exclude symlinks, and may let you push fuzzy search to VS Code to do less work here). It's hard to tell if that'll turn up next release, or stay provisional for the next 2yrs though.

If you do want to go ahead with this in the short-term, it probably still needs some testing both for correctness and for performance to ensure it's good enough.

Another possible option could be if the search takes more than some amount of time, prompt the user that the workspace search is slow (possibly because of symlinks) and would they like to disable nested ignore files for this workspace (which could allow skipping the search) 🤔

DanTup avatar Mar 18 '24 19:03 DanTup

This PR is marked as stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed automatically in 5 days.

github-actions[bot] avatar May 18 '24 01:05 github-actions[bot]