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.

Sign up for GitHub

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

Jump to bottom

Support repo code search without setting up an indexer #29998

Merged
merged 13 commits into from Mar 24, 2024
Merged

Support repo code search without setting up an indexer #29998

merged 13 commits into from Mar 24, 2024
+253 −65

Conversation

wxiaoguang
Copy link
Contributor

@wxiaoguang wxiaoguang commented Mar 22, 2024 •
edited

By using git's ability, end users (especially small instance users) do not need to enable the indexer, they could also benefit from the code searching feature.

Fix #29996

image

image

@wxiaoguang wxiaoguang added the type/enhancement An improvement of existing functionality label Mar 22, 2024
@wxiaoguang wxiaoguang added this to the 1.22.0 milestone Mar 22, 2024
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 22, 2024
@pull-request-size pull-request-size bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Mar 22, 2024
@wxiaoguang wxiaoguang mentioned this pull request Mar 22, 2024
Closed
@github-actions github-actions bot added modifies/go Pull requests that update Go code modifies/templates This PR modifies the template files labels Mar 22, 2024
@silverwind
Copy link
Member

What do do about the Fuzzy/Match box? Does this support both modes?

@wxiaoguang
Copy link
Contributor Author

wxiaoguang commented Mar 22, 2024 •
edited

What do do about the Fuzzy/Match box? Does this support both modes?

Yes

image

@silverwind
Copy link
Member

How's the performance? I assume on big repos it will be slow, but not really an issue I suppose.

@wxiaoguang
Copy link
Contributor Author

How's the performance? I assume on big repos it will be slow, but not really an issue I suppose.

Quite fast on my side. And it only reads the first 50 results to avoid consuming too much server resources.

@silverwind
Copy link
Member

silverwind commented Mar 22, 2024 •
edited

How's the performance? I assume on big repos it will be slow, but not really an issue I suppose.

Quite fast on my side. And it only reads the first 50 results to avoid consuming too much server resources.

Maybe test on torvalds/linux to be sure 😆.

@wxiaoguang
Copy link
Contributor Author

wxiaoguang commented Mar 22, 2024 •
edited

How's the performance? I assume on big repos it will be slow, but not really an issue I suppose.

Quite fast on my side. And it only reads the first 50 results to avoid consuming too much server resources.

Maybe test on torvalds/linux to be sure 😆.

Then it is not this PR's scope .... for example: Slow merge with large repos #9642 , and there are many problems when working with large repositories.

silverwind
silverwind reviewed Mar 22, 2024
View reviewed changes
modules/git/grep.go Outdated Show resolved Hide resolved
silverwind
silverwind reviewed Mar 22, 2024
View reviewed changes
modules/git/grep.go Outdated Show resolved Hide resolved
@lunny
Copy link
Member

lunny commented Mar 23, 2024

Please also update the documentations.

@wxiaoguang
Copy link
Contributor Author

Please also update the documentations.

Please elaborate which document needs to be updated.

@lunny
Copy link
Member

lunny commented Mar 23, 2024

Please also update the documentations.

Please elaborate which document needs to be updated.

I think the documentation about repository indexer can be updated https://docs.gitea.com/administration/repo-indexer

@wxiaoguang
Copy link
Contributor Author

wxiaoguang commented Mar 23, 2024 •
edited

Please also update the documentations.

Please elaborate which document needs to be updated.

I think the documentation about repository indexer can be updated https://docs.gitea.com/administration/repo-indexer

edfd40d

@wxiaoguang wxiaoguang changed the title Support repo code search without external indexer Support repo code search without setting up a indexer Mar 23, 2024
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Mar 23, 2024
@wxiaoguang wxiaoguang changed the title Support repo code search without setting up a indexer Support repo code search without setting up an indexer Mar 24, 2024
6543
6543 reviewed Mar 24, 2024
View reviewed changes
routers/web/repo/search.go
var total int
var searchResults []*code_indexer.Result
var searchResultLanguages []*code_indexer.SearchResultLanguages
if setting.Indexer.RepoIndexerEnabled {
Copy link
Member

Choose a reason for hiding this comment

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

I would suggest to intigrate the git grep search as its own indexer and set it as default.

This way it is transparent for webUI or API what to do.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No idea how to do that clearly, and I am not a fan of adding a lot of "options".

If you have better ideas, free free to edit this PR directly or have some following PRs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a prompt like this, maybe it could make it clearer. What do you think?

image

Copy link
Member

Choose a reason for hiding this comment

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

that will help but is unrelated to the architecture idea of mine.

I try to create a pull request to your branch that would move acording to my proposal, so it can be checked out and tested etc ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would expect that there is no new option to be introduced.

@wxiaoguang
Copy link
Contributor Author

wxiaoguang commented Mar 24, 2024 •
edited

To reviewers: since there are some gt- refactoring PRs, this PR has conflicts for 2 times and I have resolved the conflicts 2 times. So if there is no fundamental problem in this PR, I hope this PR could be merged before more conflicts come. Improvements could be done in following PRs.

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Mar 24, 2024
@silverwind
Copy link
Member

To reviewers: since there are some gt- refactoring PRs, this PR has conflicts for 2 times and I have resolved the conflicts 2 times. So if there is no fundamental problem in this PR, I hope this PR could be merged before more conflicts come. Improvements could be done in following PRs.

#30043 is the last of these "merge conflict" PRs.

@silverwind silverwind merged commit 4734d43 into go-gitea:main Mar 24, 2024
26 checks passed
@silverwind silverwind added type/feature Completely new functionality. Can only be merged if feature freeze is not active. and removed type/feature Completely new functionality. Can only be merged if feature freeze is not active. labels Mar 24, 2024
@6543
Copy link
Member

6543 commented Mar 24, 2024

Didn't we where in feature freze?!?

@silverwind
Copy link
Member

Not sure it qualifies as a feature in the strict sense. It's more like an addition to an existing feature.

@wxiaoguang wxiaoguang deleted the git-code-search branch March 24, 2024 17:14
@ligaard
Copy link

ligaard commented Mar 24, 2024 •
edited

In my gitea installation this does not work if the repository is in an organisation. It works fine for non-organisation repositories. For repo in an organisation I receive a 500 error page.
UPDATE: It have a non-organisation repository where is also fails with a 500 error page. So I cannot say what causes it, but my initial suggestion that it was related to organisation seems not to be valid.

Log line states: "2024-03-24 19:55:12 2024/03/24 19:55:12 ...s/web/repo/search.go:69:Search() [E] GrepSearch: unable to run git grep: signal: killed, stderr:"

I am running with the latest gitea docker image from docker hub.

@wxiaoguang
Copy link
Contributor Author

wxiaoguang commented Mar 25, 2024 •
edited

UPDATE: It have a non-organisation repository where is also fails with a 500 error page. So I cannot say what causes it, but my initial suggestion that it was related to organisation seems not to be valid.

Hmm ... no idea why it happens. On my side, "org repo" just works:

image

signal: killed seems related to the sub-process, do you have enough memory? Or is the repo large?

@wxiaoguang
Copy link
Contributor Author

@ligaard thank you very much for your report and thanks a lot to @KN4CK3R for finding the root problem.

-> Fix git grep search limit, add test #30071

@lunny lunny added type/feature Completely new functionality. Can only be merged if feature freeze is not active. and removed type/enhancement An improvement of existing functionality labels Apr 5, 2024
@lng2020 lng2020 mentioned this pull request Apr 14, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@6543 6543 6543 left review comments

@lunny lunny lunny approved these changes

@silverwind silverwind silverwind approved these changes

Assignees
No one assigned
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/docs modifies/go Pull requests that update Go code modifies/templates This PR modifies the template files modifies/translation size/L Denotes a PR that changes 100-499 lines, ignoring generated files. type/feature Completely new functionality. Can only be merged if feature freeze is not active.
Projects
None yet
Milestone
1.22.0
Development

Successfully merging this pull request may close these issues.

Always support repo code search even if the indexer is not enabled
6 participants
@wxiaoguang @silverwind @lunny @6543 @ligaard @GiteaBot