Skip to content

Commit 68ad0ca

Browse files
authored
Use --query_file to avoid to long args (#272)
`bazel_query` passed the query expression as a single `argv` element. PR with a huge diff caused `OSError: [Errno 7] Argument list too long`. Instead use `--query_file` now.
1 parent 969765d commit 68ad0ca

2 files changed

Lines changed: 54 additions & 16 deletions

File tree

‎tools/lint/changed_targets.py‎

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import os
1313
import subprocess
1414
import sys
15+
import tempfile
1516
from pathlib import Path
1617
from typing import Callable, Sequence
1718

@@ -106,23 +107,28 @@ def query_labels(result: subprocess.CompletedProcess) -> list[str]:
106107

107108
def bazel_query(workspace: Path) -> RunQuery:
108109
def run(expression: str) -> list[str]:
109-
return query_labels(
110-
subprocess.run(
111-
[
112-
"bazel",
113-
"query",
114-
"--keep_going",
115-
"--output=label",
116-
"--noshow_progress",
117-
"--ui_event_filters=-info",
118-
expression,
119-
],
120-
cwd=workspace,
121-
check=False,
122-
capture_output=True,
123-
text=True,
110+
with tempfile.NamedTemporaryFile(
111+
"w", suffix=".query", encoding="utf-8"
112+
) as query_file:
113+
query_file.write(expression)
114+
query_file.flush()
115+
return query_labels(
116+
subprocess.run(
117+
[
118+
"bazel",
119+
"query",
120+
"--keep_going",
121+
"--output=label",
122+
"--noshow_progress",
123+
"--ui_event_filters=-info",
124+
f"--query_file={query_file.name}",
125+
],
126+
cwd=workspace,
127+
check=False,
128+
capture_output=True,
129+
text=True,
130+
)
124131
)
125-
)
126132

127133
return run
128134

‎tools/lint/test_changed_targets.py‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,15 @@
88
import io
99
import subprocess
1010
import unittest
11+
from pathlib import Path
1112
from unittest import mock
1213

1314
import tools.lint.changed_targets as changed_targets
1415
from tools.lint.changed_targets import (
1516
QUERY_PARTIAL_EXIT_CODE,
1617
SOURCE_EXTENSIONS,
1718
QueryError,
19+
bazel_query,
1820
main,
1921
owning_targets,
2022
parse_args,
@@ -148,6 +150,36 @@ def test_other_failure_raises(self):
148150
self.assertEqual(raised.exception.returncode, 1)
149151

150152

153+
class TestBazelQuery(unittest.TestCase):
154+
def run_query(self, expression: str) -> subprocess.CompletedProcess:
155+
recorded = {}
156+
157+
def fake_run(command, **kwargs):
158+
recorded["command"] = command
159+
query_file = command[-1].removeprefix("--query_file=")
160+
recorded["expression"] = Path(query_file).read_text()
161+
return completed(0, "//a:a\n")
162+
163+
with mock.patch.object(subprocess, "run", side_effect=fake_run):
164+
bazel_query(Path("/workspace"))(expression)
165+
return recorded
166+
167+
def test_expression_goes_to_a_file_not_argv(self):
168+
"""A large pull request exceeds MAX_ARG_STRLEN if the expression is an argument."""
169+
expression = "same_pkg_direct_rdeps(set(%s))" % " ".join(
170+
f"pkg/file{index}.cc" for index in range(20000)
171+
)
172+
recorded = self.run_query(expression)
173+
self.assertEqual(recorded["expression"], expression)
174+
self.assertNotIn(expression, recorded["command"])
175+
self.assertTrue(recorded["command"][-1].startswith("--query_file="))
176+
177+
def test_query_file_is_removed(self):
178+
recorded = self.run_query("//a:a")
179+
query_file = recorded["command"][-1].removeprefix("--query_file=")
180+
self.assertFalse(Path(query_file).exists())
181+
182+
151183
class TestMain(unittest.TestCase):
152184
def run_main(self, run_query):
153185
with (

0 commit comments

Comments
 (0)