Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update ripgrep arguments for file search #12608

Merged
merged 2 commits into from
Jun 12, 2023

Conversation

msujew
Copy link
Member

@msujew msujew commented Jun 12, 2023

What it does

Closes #12607

Updates our used arguments and child process spawn options to be aligned with vscode. In particular, the issue exhibited in #12607 was the result of the added stdio options property.

Removing the stdio field from the options fixed #12607 locally. Also updates the private methods to be protected.

How to test

  1. Build the applications for different OS for Browser and Electron.
  2. Perform file search, it should work as expected.

Review checklist

Reminder for reviewers

@msujew msujew added the file search issues related to the file search label Jun 12, 2023
Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

  • confirmed that searching with various options works as intended (ex: hidden files)
  • CI successfully passes
  • the options are aligned with vscode

@msujew msujew merged commit 6d8affb into master Jun 12, 2023
@msujew msujew deleted the msujew/update-ripgrep-file-search branch June 12, 2023 14:04
@github-actions github-actions bot added this to the 1.39.0 milestone Jun 12, 2023
@giraffesyo
Copy link

@msujew @vince-fugnitto
We noticed a performance regression after these ripgrep changes were introduced, and had to downgrade to the version just before this. This is on linux, inside a container. (Redhat UBI specifically). What we saw was many rg processes in the process list, which led us to try and figure out where they were coming from. We tracked it down to theia and then to this change. For now we're just sticking to v1.38.0 which doesn't have this issue.

@msujew
Copy link
Member Author

msujew commented Apr 15, 2024

@giraffesyo You can try to override the service in question in your adopter application and try to see which arguments/process spawn options lead to the issue. Since I'm unaware of anyone else having that issue, it might be a very specific set of environment+arguments+process spawn options that lead to this issue.

@giraffesyo
Copy link

@giraffesyo You can try to override the service in question in your adopter application and try to see which arguments/process spawn options lead to the issue. Since I'm unaware of anyone else having that issue, it might be a very specific set of environment+arguments+process spawn options that lead to this issue.

Awesome thanks for the very quick reply. That's a great idea, we will try these suggestions out!

@giraffesyo
Copy link

@msujew Our original assumptions of it being ripgrep were incorrect. There was also the @theia/git extension added, which depends on this package: https://github.com/Axosoft/find-git-repositories. That package seems to iterate through all directories in the workspace, to an infinite depth.

We have a mounted sftp using rclone, and the directory structure can be very large on the other side. When putting rclone into verbose mode it showed folders even 10 levels deep being hit before the entire container was OOM killed. After uninstalling the @theia/git extension all is well.

Worth noting, disabling the watchers with the setting

 {
"files.watcherExclude": {
    "**/**": true,
  }
}

also does the trick, but this has the unwanted side-effect that files created don't show up without refreshing the explorer pane.

@msujew
Copy link
Member Author

msujew commented Apr 23, 2024

Hey @giraffesyo, thanks for the update. Maybe it might make sense for you to use the vscode git plugins instead of @theia/git, which is being deprecated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
file search issues related to the file search
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Intermittent file search failure in Electron Forge app
3 participants