diff --git a/hgitaly/manifest.py b/hgitaly/manifest.py index 3b3c68a691d50720a7aec15fc0aa90d128a97130_aGdpdGFseS9tYW5pZmVzdC5weQ==..a6238b82d2d0ce21d19e3bbc9442e65f5be80d62_aGdpdGFseS9tYW5pZmVzdC5weQ== 100644 --- a/hgitaly/manifest.py +++ b/hgitaly/manifest.py @@ -20,6 +20,8 @@ of the core manifest object it works on. """ import attr +from mercurial.context import filectx +from mercurial.utils.stringutil import binary as is_binary @attr.s @@ -245,6 +247,17 @@ yield file_path + def iter_files_with_content(self, exclude_binary=False): + manifest = self.changeset.manifest() + repo = self.changeset.repo().unfiltered() + for file_path, file_node, flags in manifest.iterentries(): + # filectx has a `isbinary` method, but it would actually + # read the data once more (not cached on self) + content = filectx(repo, file_path, fileid=file_node).data() + if exclude_binary and is_binary(content): + continue + yield file_path, content, flags + def miner(changeset): """Return an appropriate manifest extractor for the given changeset. diff --git a/hgitaly/service/repository.py b/hgitaly/service/repository.py index 3b3c68a691d50720a7aec15fc0aa90d128a97130_aGdpdGFseS9zZXJ2aWNlL3JlcG9zaXRvcnkucHk=..a6238b82d2d0ce21d19e3bbc9442e65f5be80d62_aGdpdGFseS9zZXJ2aWNlL3JlcG9zaXRvcnkucHk= 100644 --- a/hgitaly/service/repository.py +++ b/hgitaly/service/repository.py @@ -6,7 +6,8 @@ # SPDX-License-Identifier: GPL-2.0-or-later import gc from grpc import StatusCode +from io import BytesIO import itertools import logging import os from pathlib import Path @@ -9,7 +10,8 @@ import itertools import logging import os from pathlib import Path +from collections import deque import re import shutil import tempfile @@ -384,7 +386,66 @@ def SearchFilesByContent(self, request: SearchFilesByContentRequest, context) -> SearchFilesByContentResponse: - not_implemented(context, issue=80) # pragma no cover + """Almost straight results from `git grep` + + this part of the protocol is totally undocumented, but here is what + Gitaly does: + + - each match is sent with two lines of context before and after + - in case of overlapping matches (including context), they are sent + as one + - sending a match means cutting it in chunkd of `WRITE_BUFFER_SIZE`. + - after the full sending of each match aa with the end_of_match` + boolean set to true and no data is sent. + + (see also Comparison Tests for validatation of this). + """ + repo = self.load_repo(request.repository, context) + query = request.query + if not query: + context.abort(StatusCode.INVALID_ARGUMENT, "no query given") + + ref = request.ref + changeset = gitlab_revision_changeset(repo, ref) + if changeset is None: + return + + # TODO filtermaxlen? + rx = re.compile(query.encode('utf-8'), flags=re.IGNORECASE) + # TODO chunked_response (not used by Rails app) + match_data = BytesIO() + miner = manifest.miner(changeset) + for path, content, _flags in miner.iter_files_with_content( + exclude_binary=True): + # TODO rx options. Here's Gitaly reference (Golang), arguments + # passed to `git grep`): + # (with const surroundContext = 2) + # + # git.Flag{Name: "--ignore-case"}, + # git.Flag{Name: "-I"}, + # (ignore binary files) + # git.Flag{Name: "--line-number"}, + # git.Flag{Name: "--null"}, + # (use null as delimiter between path, line number and + # matching line in the output, hence avoiding the need + # to escape) + # git.ValueFlag{Name: "--before-context", + # Value: surroundContext}, + # git.ValueFlag{Name: "--after-context", Value: surroundContext}, + # git.Flag{Name: "--perl-regexp"}, + # git.Flag{Name: "-e"}, + # Gitaly does not fill in `matches` field any more + # (surely deprecated) and the Rails client does not read it + # TODO cheaper iteration on a window of lines (note that + # splitlines() takes care of `\r\n` and even `\r` EOLs.) + for matching_lines in grep_file(rx, content): + render_git_grep_matches( + match_data, + ref, path, + matching_lines) + yield from iter_sfbc_resps(match_data) + match_data.truncate(0) + match_data.seek(0) def set_custom_hooks(self, request, context): def load_repo(req, context): @@ -693,3 +754,87 @@ context.set_details("hg_init_repository(%r): %r" % (repo_path, exc)) raise RepositoryCreationError(repository) + + +def render_git_grep_matches(buf, ref, path, enum_lines): + """Render a slice of lines as git grep does. + + :param enum_lines: iterable of pairs `(line_no, line)` + """ + for lineno, line in enum_lines: + buf.write(b'%s:%s\x00%d\x00%s\n' % (ref, path, lineno, line)) + + +SPLITLINES_RX = re.compile(br'(.*?)(\r\n?|\n|\Z)', re.MULTILINE) + + +def grep_file(rx, data, context_width=2): + """Iterator yielding matches the given regular expression with context + + This implementation avoids duplicating the data in memory. + + :param int context_width: number of lines before and after to include + if possible (same as `grep -C`) + :returns: pairs `(line_no, line)` + """ + prev_match_line_no = None + current_window = deque() # current line and up to 2 lines before + match_lines = [] + # According the the best ranked answer for + # https://stackoverflow.com/questions/3054604, + # this regexp-based splitting for lines can be a bit slower than + # `iter(splitlines)` (things may have changes since then, though). + # Yet, our biggest concern is to avoid exhausting HGitaly's RAM budget + # if this happens to run on large files. + # Unfortunately, we already have to load the entire data in RAM because + # it is typically Mercurial file content, we don't want to do it once + # more in the form of lines. In both cases a bytes string + # is allocated for each line, but that is harder to prevent and they + # should be deallocated at each iteration (no cycles). + # Unless there is a bug in the current approach, it is not worth the + # effort to try and further improve memory efficiency: implementing + # in RHGitaly would be the way to go. + for line_idx, line_m in enumerate(SPLITLINES_RX.finditer(data)): + line = line_m.group(1) + if not line: + continue + line_no = line_idx + 1 + current_window.append((line_no, line)) + if rx.search(line): + if ( + prev_match_line_no is None + or line_no - prev_match_line_no > 2 * context_width + ): + match_lines = list(current_window) + else: + match_lines.append((line_no, line)) + prev_match_line_no = line_no + elif ( + prev_match_line_no is not None + and line_no <= prev_match_line_no + context_width + ): + match_lines.append((line_no, line)) + elif match_lines: + yield match_lines + match_lines = [] + + if len(current_window) > context_width: + current_window.popleft() + if match_lines: + yield match_lines + + +def iter_sfbc_resps(match_data: BytesIO): + """Yield SearchFilesByContentResponse messages for the given match_data. + """ + value = match_data.getvalue() + match_len = len(value) + sent = 0 # actually rounded to the upper WRITE_BUFFER_SIZE + while sent < match_len: + yield SearchFilesByContentResponse( + match_data=value[sent:sent + WRITE_BUFFER_SIZE]) + sent += WRITE_BUFFER_SIZE + + match_data.truncate(0) + match_data.seek(0) + yield SearchFilesByContentResponse(end_of_match=True) diff --git a/hgitaly/service/tests/test_repository_service.py b/hgitaly/service/tests/test_repository_service.py index 3b3c68a691d50720a7aec15fc0aa90d128a97130_aGdpdGFseS9zZXJ2aWNlL3Rlc3RzL3Rlc3RfcmVwb3NpdG9yeV9zZXJ2aWNlLnB5..a6238b82d2d0ce21d19e3bbc9442e65f5be80d62_aGdpdGFseS9zZXJ2aWNlL3Rlc3RzL3Rlc3RfcmVwb3NpdG9yeV9zZXJ2aWNlLnB5 100644 --- a/hgitaly/service/tests/test_repository_service.py +++ b/hgitaly/service/tests/test_repository_service.py @@ -29,6 +29,7 @@ from hgitaly.repository import ( GITLAB_PROJECT_FULL_PATH_FILENAME, ) +from hgitaly.stream import WRITE_BUFFER_SIZE from hgitaly.testing import TEST_DATA_DIR from hgitaly.testing.bundle import list_bundle_contents @@ -52,6 +53,8 @@ RepositoryExistsRequest, RestoreCustomHooksRequest, SearchFilesByNameRequest, + SearchFilesByContentRequest, + SearchFilesByContentResponse, SetFullPathRequest, SetCustomHooksRequest, WriteRefRequest, @@ -66,6 +69,8 @@ parametrize = pytest.mark.parametrize +END_OF_MATCH = SearchFilesByContentResponse(end_of_match=True) + @attr.s class RepositoryFixture(ServiceFixture): @@ -130,6 +135,28 @@ )) for path in resp.files] + def search_files_by_content(self, ref=b'branch/default', + aggregate_splitlines=True, + **kwargs): + stream = self.stub.SearchFilesByContent(SearchFilesByContentRequest( + repository=self.grpc_repo, + ref=ref, + **kwargs, + )) + if not aggregate_splitlines: + return list(stream) + + res = [] + match_data = BytesIO() + for resp in stream: + if resp.end_of_match: + res.append(match_data.getvalue().split(b'\n')) + match_data = BytesIO() # deliberately not as in tested code + res.append(resp) + else: + match_data.write(resp.match_data) + return res + def restore_custom_hooks(self, tarball_path, grpc_repo=None, nb_chunks=2, @@ -607,6 +634,114 @@ assert search(ref=b'unknown', query='.') == [] +def test_search_files_by_content(fixture_with_repo): + fixture = fixture_with_repo + wrapper = fixture.repo_wrapper + + wrapper.write_commit('afoo', message="Some foo") + sub = (wrapper.path / 'sub') + sub.mkdir() + (sub / 'bar').write_text('line1\nline2\nbar content\nline4\nline5') + (sub / 'ba2').write_text('line1\nba2 content') + (sub / 'ba3').write_text('ba3 content\nline2') + # this one has Windows and MacOS Classic line endings: + (sub / 'ba4').write_text('l1\nl2\r\nl3\rba4 content\nl5') + (sub / 'ba5').write_text('m1\nm2\nm3\nm4\nm5\nm6\nba5 content\n') + long_content = 'very large content\n' * 3000 + shutil.copy2(TEST_DATA_DIR / 'backup_additional_no_git.tar', + wrapper.path / 'bin') + (wrapper.path / 'large').write_text(long_content) + + wrapper.commit(rel_paths=['bin', 'sub/bar', 'sub/ba2', 'sub/ba3', + 'sub/ba4', 'sub/ba5', 'large'], + message="zebar", add_remove=True) + search = fixture.search_files_by_content + + # cases not matching + assert not search(query='no match for this one') + assert not search(query='PaxHeader') # tarfile is excluded + assert not search(ref=b"no/such/fref", query='foo') + + # simple case: match, with two lines of content before and after + assert search(query="^bar.c") == [ + [b'branch/default:sub/bar\x001\x00line1', + b'branch/default:sub/bar\x002\x00line2', + b'branch/default:sub/bar\x003\x00bar content', + b'branch/default:sub/bar\x004\x00line4', + b'branch/default:sub/bar\x005\x00line5', + b'', + ], + END_OF_MATCH, + ] + + # with only one line if file before and after match + assert search(query="^ba2.c") == [ + [b'branch/default:sub/ba2\x001\x00line1', + b'branch/default:sub/ba2\x002\x00ba2 content', + b'', + ], + END_OF_MATCH, + ] + assert search(query="^ba3.c") == [ + [b'branch/default:sub/ba3\x001\x00ba3 content', + b'branch/default:sub/ba3\x002\x00line2', + b'', + ], + END_OF_MATCH, + ] + + # more than two lines before match + assert search(query="^ba4.c") == [ + [b'branch/default:sub/ba4\x002\x00l2', + b'branch/default:sub/ba4\x003\x00l3', + b'branch/default:sub/ba4\x004\x00ba4 content', + b'branch/default:sub/ba4\x005\x00l5', + b'', + ], + END_OF_MATCH, + ] + + # two matches, with overlapping context or not + assert search(query="^l1|ba4.c") == [ + [b'branch/default:sub/ba4\x001\x00l1', + b'branch/default:sub/ba4\x002\x00l2', + b'branch/default:sub/ba4\x003\x00l3', + b'branch/default:sub/ba4\x004\x00ba4 content', + b'branch/default:sub/ba4\x005\x00l5', + b'', + ], + END_OF_MATCH, + ] + assert search(query="^m1|ba5.c") == [ + [b'branch/default:sub/ba5\x001\x00m1', + b'branch/default:sub/ba5\x002\x00m2', + b'branch/default:sub/ba5\x003\x00m3', + b'', + ], + END_OF_MATCH, + [ + b'branch/default:sub/ba5\x005\x00m5', + b'branch/default:sub/ba5\x006\x00m6', + b'branch/default:sub/ba5\x007\x00ba5 content', + b'' + ], + END_OF_MATCH, + ] + # several matching files, with case insensitivity + assert len(search(query='ConTent')) == 6 * 2 + + # long match requiring several responses + long_match = search(query='^very', aggregate_splitlines=False) + assert len(long_match) == 3 + assert long_match[-1] == END_OF_MATCH + assert len(long_match[0].match_data) == WRITE_BUFFER_SIZE + + # error cases + with pytest.raises(grpc.RpcError) as exc_info: + search() + assert exc_info.value.code() == grpc.StatusCode.INVALID_ARGUMENT + + @parametrize('rpc_name', ['restore', 'set']) def test_set_custom_hooks(fixture_with_repo, tmpdir, rpc_name): fixture = fixture_with_repo diff --git a/tests_with_gitaly/test_repository_service.py b/tests_with_gitaly/test_repository_service.py index 3b3c68a691d50720a7aec15fc0aa90d128a97130_dGVzdHNfd2l0aF9naXRhbHkvdGVzdF9yZXBvc2l0b3J5X3NlcnZpY2UucHk=..a6238b82d2d0ce21d19e3bbc9442e65f5be80d62_dGVzdHNfd2l0aF9naXRhbHkvdGVzdF9yZXBvc2l0b3J5X3NlcnZpY2UucHk= 100644 --- a/tests_with_gitaly/test_repository_service.py +++ b/tests_with_gitaly/test_repository_service.py @@ -23,6 +23,7 @@ FullPathRequest, RemoveAllRequest, RemoveRepositoryRequest, + SearchFilesByContentRequest, SearchFilesByNameRequest, SetFullPathRequest, ) @@ -626,6 +627,70 @@ assert_compare(ref=b'unknown', query='.') +def test_search_files_by_content(gitaly_comparison): + fixture = gitaly_comparison + + wrapper = fixture.hg_repo_wrapper + wrapper.write_commit('afoo', message="Some foo") + sub = (wrapper.path / 'sub') + sub.mkdir() + (sub / 'bar').write_text('line1\nline2\nbar content\nline4\nline5') + (sub / 'ba2').write_text('line1\nba2 content') + (sub / 'ba3').write_text('ba3 content\nline2') + # This one has a Windows endings, and exhibits that `git grep` normalizes + # to `\n`. Also Git does not interpret the MacOS classic line ending + # '\r' and we do. In that case, we can claim our response to be more + # correct and we will not compare it. + (sub / 'ba4').write_text('l1\r\nl2\nl3\nba4 content\nline6') + (sub / 'ba5').write_text('m1\nm2\nm3\nm4\nm5\nm6\nba5 content\n') + (wrapper.path / 'large').write_text('very large content\n' * 3000) + # TODO OS indep for paths (actually TODO make wrapper.commit easier to + # use, e.g., check how to make it accept patterns) + wrapper.commit(rel_paths=['sub/bar', 'sub/ba2', 'sub/ba3', + 'sub/ba4', 'sub/ba5', 'large'], + message="zebar", add_remove=True) + + default_rev = b'branch/default' + + rpc_helper = fixture.rpc_helper( + stub_cls=RepositoryServiceStub, + method_name='SearchFilesByContent', + request_cls=SearchFilesByContentRequest, + request_defaults=dict( + ref=default_rev, + ), + streaming=True, + error_details_normalizer=lambda s, vcs: s.lower(), + ) + assert_compare = rpc_helper.assert_compare + assert_compare_errors = rpc_helper.assert_compare_errors + + # precondition for the test: mirror worked + assert fixture.git_repo.branch_titles() == { + default_rev: b"zebar", + } + assert_compare(query='no match for this one') + assert_compare(query='^bar.c') + assert_compare(query='^ba2.c') # only one line before match + assert_compare(query='^ba3.c') # only one line after match + assert_compare(query='^ba4.c') # more than two lines before match + assert_compare(query='^very') + assert_compare(query='^l1|ba4') # two matches with overlapping context + assert_compare(query='^m1|ba5') # two matches with non-overlapping context + assert_compare(query='ConTent') # several files and case insensity + + # errors if query is missing + assert_compare_errors() + assert_compare_errors(ref=b'topic/default/does-not-exist') + # unresolvable ref + assert_compare(ref=b'topic/default/does-not-exist', query='foo') + # missing repo, ref + assert_compare_errors(ref=b'') + assert_compare_errors(repository=None) + fixture.gitaly_repo.relative_path = 'no/such/repo' + assert_compare_errors(query='foo', same_details=False) + + def test_remove_all(gitaly_comparison, server_repos_root): fixture = gitaly_comparison rpc_helper = fixture.rpc_helper(