Skip to content

Commit 6e849be

Browse files
hoe-jocastler
authored andcommitted
[rules_score]: bugfix dependable element sphinx build
1 parent 4c83c57 commit 6e849be

5 files changed

Lines changed: 261 additions & 8 deletions

File tree

bazel/rules/rules_score/private/dependable_element.bzl

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ load(
3535
"DependableElementInfo",
3636
"DependableElementLobsterInfo",
3737
"FeatureRequirementsInfo",
38+
"SphinxIndexFileInfo",
3839
"SphinxModuleInfo",
3940
"SphinxNeedsInfo",
4041
"SphinxSourcesInfo",
@@ -967,6 +968,7 @@ def _dependable_element_index_impl(ctx):
967968

968969
return [
969970
DefaultInfo(files = depset(output_files)),
971+
SphinxIndexFileInfo(index_file = index_rst),
970972
CertifiedScope(transitive_scopes = depset(transitive = collected_certified_scopes)),
971973
DependableElementInfo(
972974
integrity_level = ctx.attr.integrity_level,

bazel/rules/rules_score/private/sphinx_module.bzl

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,21 @@
1616
load("@bazel_skylib//lib:paths.bzl", "paths")
1717
load("@rules_python//sphinxdocs:sphinx_docs_library.bzl", "sphinx_docs_library")
1818
load("@rules_python//sphinxdocs/private:sphinx_docs_library_info.bzl", "SphinxDocsLibraryInfo")
19-
load("//bazel/rules/rules_score:providers.bzl", "FilteredExecpathInfo", "SphinxModuleInfo", "SphinxNeedsInfo")
19+
load("//bazel/rules/rules_score:providers.bzl", "FilteredExecpathInfo", "SphinxIndexFileInfo", "SphinxModuleInfo", "SphinxNeedsInfo")
20+
21+
def _get_index_file(ctx):
22+
"""Extract the index file from the index attribute.
23+
24+
If the target provides SphinxIndexFileInfo, use that. Otherwise expect
25+
exactly one file and use it directly.
26+
"""
27+
target = ctx.attr.index
28+
if SphinxIndexFileInfo in target:
29+
return target[SphinxIndexFileInfo].index_file
30+
files = target.files.to_list()
31+
if len(files) != 1:
32+
fail("'index' target must provide SphinxIndexFileInfo or produce exactly one file, got %d files" % len(files))
33+
return files[0]
2034

2135
def _create_config_py(ctx):
2236
"""Get or generate the conf.py configuration file.
@@ -70,7 +84,7 @@ def _score_needs_impl(ctx):
7084
needs_inputs = ctx.files.srcs + [config_file]
7185
needs_args = [
7286
"--index_file",
73-
ctx.attr.index.files.to_list()[0].path,
87+
_get_index_file(ctx).path,
7488
"--output_dir",
7589
needs_output.dirname,
7690
"--config",
@@ -181,12 +195,12 @@ def _score_html_impl(ctx):
181195
# Sphinx only accepts a single directory to read its doc sources from.
182196
# Because plain files and generated files are in different directories,
183197
# we need to merge the two into a single directory.
184-
for orig_file in ctx.files.srcs:
185-
_relocate(orig_file)
198+
index_source_file = _get_index_file(ctx)
186199
relocated_index_file = ""
187-
for input_file in sphinx_source_files:
188-
if input_file.path.endswith("/index.rst"):
189-
relocated_index_file = input_file.path
200+
for orig_file in ctx.files.srcs:
201+
dest = _relocate(orig_file)
202+
if orig_file.path == index_source_file.path:
203+
relocated_index_file = dest.path
190204

191205
# Build HTML with external needs
192206
html_inputs = sphinx_source_files + ctx.files.needs + filtered_files + [config_file, needs_external_needs_json]

bazel/rules/rules_score/providers.bzl

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,13 @@ DependableElementLobsterInfo = provider(
199199
},
200200
)
201201

202+
SphinxIndexFileInfo = provider(
203+
doc = "Provider carrying the single index.rst file for a Sphinx documentation module.",
204+
fields = {
205+
"index_file": "File – the index.rst file to use as the Sphinx master document.",
206+
},
207+
)
208+
202209
SphinxModuleInfo = provider(
203210
doc = "Provider for Sphinx HTML module documentation",
204211
fields = {

bazel/rules/rules_score/test/BUILD

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,9 +68,13 @@ load(
6868
)
6969
load(
7070
":seooc_test.bzl",
71+
"seooc_all_sources_relocated_test",
7172
"seooc_artifacts_copied_test",
73+
"seooc_index_file_provider_test",
7274
"seooc_index_generation_test",
75+
"seooc_multi_index_files_exist_test",
7376
"seooc_needs_provider_test",
77+
"seooc_sphinx_entry_point_is_root_index_test",
7478
"seooc_sphinx_module_generated_test",
7579
)
7680
load(
@@ -496,6 +500,27 @@ seooc_index_generation_test(
496500
target_under_test = ":seooc_test_lib_index",
497501
)
498502

503+
# Regression tests: correct index.rst resolution when multiple index.rst files exist
504+
seooc_multi_index_files_exist_test(
505+
name = "seooc_tests_multi_index_files_exist",
506+
target_under_test = ":test_dependable_element_index",
507+
)
508+
509+
seooc_sphinx_entry_point_is_root_index_test(
510+
name = "seooc_tests_sphinx_entry_point_is_root_index",
511+
target_under_test = ":test_dependable_element_doc",
512+
)
513+
514+
seooc_all_sources_relocated_test(
515+
name = "seooc_tests_all_sources_relocated",
516+
target_under_test = ":test_dependable_element_doc",
517+
)
518+
519+
seooc_index_file_provider_test(
520+
name = "seooc_tests_index_file_provider",
521+
target_under_test = ":test_dependable_element_index",
522+
)
523+
499524
# ============================================================================
500525
# Test Suites
501526
# ============================================================================
@@ -560,9 +585,13 @@ sphinx_module_providers_test_suite(name = "sphinx_module_providers_tests")
560585
test_suite(
561586
name = "seooc_tests",
562587
tests = [
588+
":seooc_tests_all_sources_relocated",
563589
":seooc_tests_artifacts_copied",
590+
":seooc_tests_index_file_provider",
564591
":seooc_tests_index_generation",
592+
":seooc_tests_multi_index_files_exist",
565593
":seooc_tests_needs_provider",
594+
":seooc_tests_sphinx_entry_point_is_root_index",
566595
":seooc_tests_sphinx_module_generated",
567596
],
568597
)

bazel/rules/rules_score/test/seooc_test.bzl

Lines changed: 202 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ Tests the SEooC (Safety Element out of Context) functionality including:
2121
"""
2222

2323
load("@bazel_skylib//lib:unittest.bzl", "analysistest", "asserts")
24-
load("@score_tooling//bazel/rules/rules_score:providers.bzl", "SphinxModuleInfo", "SphinxNeedsInfo")
24+
load("@score_tooling//bazel/rules/rules_score:providers.bzl", "SphinxIndexFileInfo", "SphinxModuleInfo", "SphinxNeedsInfo")
2525

2626
def _seooc_index_generation_test_impl(ctx):
2727
"""Test that dependable_element generates proper index.rst file."""
@@ -115,3 +115,204 @@ def _seooc_needs_provider_test_impl(ctx):
115115
seooc_needs_provider_test = analysistest.make(
116116
impl = _seooc_needs_provider_test_impl,
117117
)
118+
119+
# ============================================================================
120+
# Regression tests: correct index.rst resolution (multi-index scenario)
121+
#
122+
# These tests guard against a bug where _score_html_impl scanned all relocated
123+
# source files for any path ending with "/index.rst" and used the last match.
124+
# A dependable_element with components generates both:
125+
# <name>/index.rst (root – correct Sphinx entry point)
126+
# <name>/components/index.rst (sub-page – must NOT be the entry point)
127+
# The old code would pick the sub-page as the Sphinx entry point.
128+
# ============================================================================
129+
130+
def _seooc_multi_index_files_exist_test_impl(ctx):
131+
"""
132+
Given a dependable_element with at least one component,
133+
When the _index rule generates its output file set,
134+
Then both a root index.rst and a components/index.rst must be present,
135+
confirming the fixture exercises the multi-index scenario.
136+
"""
137+
env = analysistest.begin(ctx)
138+
target_under_test = analysistest.target_under_test(env)
139+
140+
# When: collect all output paths
141+
files = target_under_test[DefaultInfo].files.to_list()
142+
paths = [f.path for f in files]
143+
144+
# Then: a components/index.rst sub-page exists
145+
components_index_paths = [p for p in paths if p.endswith("components/index.rst")]
146+
asserts.true(
147+
env,
148+
len(components_index_paths) >= 1,
149+
"Expected a components/index.rst to be generated (fixture must have at least one component)",
150+
)
151+
152+
# Then: a root index.rst (not inside components/) also exists
153+
root_index_paths = [
154+
p
155+
for p in paths
156+
if p.endswith("index.rst") and "components/" not in p
157+
]
158+
asserts.true(
159+
env,
160+
len(root_index_paths) >= 1,
161+
"Expected a root index.rst (outside components/) to be generated",
162+
)
163+
164+
return analysistest.end(env)
165+
166+
seooc_multi_index_files_exist_test = analysistest.make(
167+
impl = _seooc_multi_index_files_exist_test_impl,
168+
)
169+
170+
def _seooc_sphinx_entry_point_is_root_index_test_impl(ctx):
171+
"""
172+
Given a dependable_element whose source tree contains both a root index.rst
173+
and a components/index.rst,
174+
When the Sphinx HTML build action is constructed,
175+
Then the --index_file argument passed to Sphinx must point to the root
176+
index.rst and must NOT point to the components/index.rst sub-page.
177+
"""
178+
env = analysistest.begin(ctx)
179+
180+
# When: inspect all actions produced for this target
181+
actions = analysistest.target_actions(env)
182+
183+
# Find the Sphinx HTML action: it is a run() action whose outputs contain
184+
# the "_html" directory (declared as <name>/_html).
185+
sphinx_html_action = None
186+
for action in actions:
187+
for output in action.outputs.to_list():
188+
if output.basename == "_html":
189+
sphinx_html_action = action
190+
break
191+
if sphinx_html_action:
192+
break
193+
194+
asserts.true(
195+
env,
196+
sphinx_html_action != None,
197+
"Expected to find the Sphinx HTML build action (output ending in '_html')",
198+
)
199+
200+
# Then: extract the value that follows --index_file in the argument list
201+
argv = sphinx_html_action.argv
202+
index_path = None
203+
for i, arg in enumerate(argv):
204+
if arg == "--index_file" and i + 1 < len(argv):
205+
index_path = argv[i + 1]
206+
break
207+
208+
asserts.true(
209+
env,
210+
index_path != None,
211+
"Sphinx HTML action must contain a --index_file argument",
212+
)
213+
214+
asserts.true(
215+
env,
216+
index_path.endswith("index.rst"),
217+
"The --index_file value must end with index.rst; got: " + str(index_path),
218+
)
219+
220+
asserts.false(
221+
env,
222+
"components/" in index_path,
223+
"The --index_file value must NOT point to components/index.rst; got: " + str(index_path),
224+
)
225+
226+
return analysistest.end(env)
227+
228+
seooc_sphinx_entry_point_is_root_index_test = analysistest.make(
229+
impl = _seooc_sphinx_entry_point_is_root_index_test_impl,
230+
)
231+
232+
def _seooc_all_sources_relocated_test_impl(ctx):
233+
"""
234+
Given a dependable_element that provides source files via SphinxSourcesInfo,
235+
When the Sphinx HTML build action is constructed,
236+
Then all source files must appear as inputs to the action, confirming that
237+
every file is relocated (symlinked) and none are silently dropped.
238+
"""
239+
env = analysistest.begin(ctx)
240+
target_under_test = analysistest.target_under_test(env)
241+
242+
# When: find the Sphinx HTML action (same selection as the entry-point test)
243+
actions = analysistest.target_actions(env)
244+
sphinx_html_action = None
245+
for action in actions:
246+
for output in action.outputs.to_list():
247+
if output.basename == "_html":
248+
sphinx_html_action = action
249+
break
250+
if sphinx_html_action:
251+
break
252+
253+
asserts.true(
254+
env,
255+
sphinx_html_action != None,
256+
"Expected to find the Sphinx HTML build action (output ending in '_html')",
257+
)
258+
259+
# Then: every .rst / .md / .puml / .json source from the index target's
260+
# output file set must appear as an input to the HTML action.
261+
index_files = target_under_test[DefaultInfo].files.to_list()
262+
source_exts = ("rst", "md", "puml", "plantuml", "json")
263+
expected_sources = [
264+
f
265+
for f in index_files
266+
if f.extension in source_exts
267+
]
268+
269+
action_input_paths = {f.path: True for f in sphinx_html_action.inputs.to_list()}
270+
271+
for src in expected_sources:
272+
asserts.true(
273+
env,
274+
src.path in action_input_paths,
275+
"Source file '{}' was not passed as input to the Sphinx HTML action".format(src.path),
276+
)
277+
278+
return analysistest.end(env)
279+
280+
seooc_all_sources_relocated_test = analysistest.make(
281+
impl = _seooc_all_sources_relocated_test_impl,
282+
)
283+
284+
def _seooc_index_file_provider_test_impl(ctx):
285+
"""
286+
Given a _dependable_element_index target,
287+
When its providers are inspected,
288+
Then SphinxIndexFileInfo must be present and its index_file must be the
289+
root index.rst (not a components/index.rst sub-page).
290+
"""
291+
env = analysistest.begin(ctx)
292+
target_under_test = analysistest.target_under_test(env)
293+
294+
asserts.true(
295+
env,
296+
SphinxIndexFileInfo in target_under_test,
297+
"_dependable_element_index must provide SphinxIndexFileInfo",
298+
)
299+
300+
index_file = target_under_test[SphinxIndexFileInfo].index_file
301+
302+
asserts.true(
303+
env,
304+
index_file.basename == "index.rst",
305+
"SphinxIndexFileInfo.index_file must be named index.rst; got: " + index_file.basename,
306+
)
307+
308+
asserts.false(
309+
env,
310+
"components/" in index_file.path,
311+
"SphinxIndexFileInfo.index_file must NOT point to components/index.rst; got: " + index_file.path,
312+
)
313+
314+
return analysistest.end(env)
315+
316+
seooc_index_file_provider_test = analysistest.make(
317+
impl = _seooc_index_file_provider_test_impl,
318+
)

0 commit comments

Comments
 (0)