Skip to content

Commit

Permalink
ctexplain: make cquery parsing more robust.
Browse files Browse the repository at this point in the history
Specifically:

 - Handle labels with spaces
 - Handle targets with no fragment requirements.

These were both inspired by real analyses.

Testing: $ bazel test //tools/ctexplain:all
Work towards bazelbuild#10613
  • Loading branch information
gregestren committed Mar 12, 2021
1 parent 8dbbde0 commit fe42dcf
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 11 deletions.
25 changes: 16 additions & 9 deletions tools/ctexplain/bazel_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ def run_bazel_in_client(args: List[str]) -> Tuple[int, List[str], List[str]]:
Tuple of (return code, stdout, stderr)
"""
result = subprocess.run(
["blaze"] + args,
["bazel"] + args,
cwd=os.getcwd(),
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
Expand Down Expand Up @@ -145,20 +145,27 @@ def _parse_cquery_result_line(line: str) -> ConfiguredTarget:
Returns:
Corresponding ConfiguredTarget if the line matches else None.
"""
tokens = line.split(maxsplit=2)
label = tokens[0]
if tokens[1][0] != "(" or tokens[1][-1] != ")":
raise ValueError(f"{tokens[1]} in {line} not surrounded by parentheses")
config_hash = tokens[1][1:-1]
config_hash_start = line.find("(")
config_hash_end = line.find(")", config_hash_start)
fragments_start = line.find("[", config_hash_end)
fragments_end = line.find("]", fragments_start)

label = line[0:config_hash_start].strip()
if config_hash_start == -1 or config_hash_end < config_hash_start:
raise ValueError(f'"{line}": no "(<config hash>)" suffix')
config_hash = line[config_hash_start + 1:config_hash_end]
if config_hash == "null":
fragments = ()
else:
if tokens[2][0] != "[" or tokens[2][-1] != "]":
raise ValueError(f"{tokens[2]} in {line} not surrounded by [] brackets")
if fragments_start == -1 or fragments_end < fragments_start:
raise ValueError(f'"{line}": no "[<required fragment>, ...]" suffix')
# The fragments list looks like '[Fragment1, Fragment2, ...]'. Split the
# whole line on ' [' to get just this list, then remove the final ']', then
# split again on ', ' to convert it to a structured tuple.
fragments = tuple(line.split(" [")[1][0:-1].split(", "))
if fragments_start == fragments_end - 1:
fragments = tuple()
else:
fragments = tuple(line[fragments_start + 1:fragments_end].split(", "))
return ConfiguredTarget(
label=label,
config=None, # Not yet available: we'll need `bazel config` to get this.
Expand Down
28 changes: 26 additions & 2 deletions tools/ctexplain/bazel_api_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ def testBasicCquery(self):
self.assertEqual(len(cts), 1)
self.assertEqual(cts[0].label, '//testapp:fg')
self.assertIsNone(cts[0].config)
self.assertGreater(len(cts[0].config_hash), 10)
self.assertGreater(len(cts[0].config_hash), 6)
self.assertIn('PlatformConfiguration', cts[0].transitive_fragments)

def testFailedCquery(self):
Expand Down Expand Up @@ -144,14 +144,38 @@ def testConfigWithStarlarkFlags(self):
'string_flag(name = "my_flag", build_setting_default = "nada")',
'filegroup(name = "fg", srcs = ["a.file"])',
])
cquery_args = ['//testapp:fg', '--//testapp:my_flag', 'algo']
cquery_args = ['//testapp:fg', '--//testapp:my_flag=algo']
cts = self._bazel_api.cquery(cquery_args)[2]
config = self._bazel_api.get_config(cts[0].config_hash)
user_defined_options = config.options['user-defined']
self.assertIsNotNone(user_defined_options)
self.assertDictEqual(user_defined_options._dict,
{'//testapp:my_flag': 'algo'})

def testLabelsWithSpaces(self):
self.ScratchFile('testapp/BUILD', [
'filegroup(name = "labels can have spaces", srcs = ["a.file"])',
])
cquery_args = ['//testapp:labels can have spaces']
res = self._bazel_api.cquery(['//testapp:all'])
success = res[0]
cts = res[2]
self.assertTrue(success)
self.assertEqual(len(cts), 1)
self.assertEqual(cts[0].label, '//testapp:labels can have spaces')

def testEmptyFragmentRequirements(self):
self.ScratchFile('testapp/BUILD', [
'alias(name = "alias_to_src", actual = "source.file")',
])
cquery_args = ['//testapp:alias_to_src']
res = self._bazel_api.cquery(['//testapp:all'])
success = res[0]
cts = res[2]
self.assertTrue(success)
self.assertEqual(len(cts), 1)
self.assertEqual(cts[0].label, '//testapp:alias_to_src')
self.assertEqual(len(cts[0].transitive_fragments), 0)

if __name__ == '__main__':
unittest.main()

0 comments on commit fe42dcf

Please sign in to comment.