Skip to content

refactor: derive bundle runtime paths from source_dir #820

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

Merged
MaximilianSoerenPollak merged 1 commit into from
Sep 15, 2026
+21 −19
Merged

refactor: derive bundle runtime paths from source_dir #820

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 16 additions & 13 deletions bzl/bundle_rules.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,21 +107,22 @@ def _ensure_unique_entries(entries):
"than one bundle path; include every documentation source directory once") % key)
seen[key] = entry

def _bundle_runtime_path(ctx):
def _source_dir_runtime_path(ctx):
"""Return this bundle source directory's Bazel runtime path.

Bazel spells a source in an external repository as ``../<repo>/...`` in
runfiles. Keep that spelling here; ``_bundle_execroot_path`` converts it to
the corresponding ``external/<repo>/...`` form for build actions.

Local sources use ``<package>/<source_dir>``; sources from an external
repository use ``../<repository>/<package>/<source_dir>``. If ``source_dir``
is ``.`` the package path itself is returned.
"""
# All files were globbed from this bundle's one source_dir, so the first
# file is representative for detecting an external-repository prefix.
source_file = ctx.files.source_dir_globbed[0].short_path
external_prefix = ""
if source_file.startswith("../"):
path_parts = source_file.split("/")
external_prefix = path_parts[0] + "/" + path_parts[1] + "/"
return external_prefix + ctx.attr.strip_prefix.rstrip("/")
source_root = join_path(ctx.label.package, ctx.attr.source_dir)
if ctx.label.workspace_name:
return "../" + ctx.label.workspace_name + "/" + source_root
else:
return source_root

def _source_target_path(source_file):
"""Return the path spelling used by the source-target staging action."""
Expand Down Expand Up @@ -291,7 +292,7 @@ def _docs_bundle_impl(ctx):
"targets") % ctx.label)

if ctx.files.source_dir_globbed:
runtime_path = _bundle_runtime_path(ctx)
runtime_path = _source_dir_runtime_path(ctx)
own_source_root = runtime_path
external = runtime_path.startswith("../")
entries.append(struct(
Expand Down Expand Up @@ -421,7 +422,9 @@ _docs_bundle = rule(
"source_dir_globbed": attr.label_list(allow_files = True),
"source_targets": attr.label_list(allow_files = True),
"sourcelinks_json": attr.label(allow_single_file = True),
"strip_prefix": attr.string(default = ""),

@a-zw a-zw Sep 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why remove strip_prefix?

@MaximilianSoerenPollak MaximilianSoerenPollak Sep 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Strip_prefix was a missnamed argument. It never actually stripped a prefix.
That's why now it has been removed and actuall source_dir has been added.

# An empty value is used for explicit-source and data-only bundles;
# those cases do not call _source_dir_runtime_path().
"source_dir": attr.string(default = ""),
"entry_doc": attr.string(default = "index"),
"bundles": attr.label_list(providers = [DocsBundleInfo]),
"bundle_mount_ats": attr.string_list(),
Expand All @@ -437,7 +440,7 @@ def create_bundle(
source_dir_globbed = [],
source_targets = [],
sourcelinks_json = None,
strip_prefix = "",
source_dir = None,
entry_doc = "index",
data = [],
visibility = None,
Expand All @@ -453,7 +456,7 @@ def create_bundle(
source_dir_globbed = source_dir_globbed,
source_targets = source_targets,
sourcelinks_json = sourcelinks_json,
strip_prefix = strip_prefix,
source_dir = source_dir if source_dir != None else "",
Comment on lines 442 to +459

@MaximilianSoerenPollak MaximilianSoerenPollak Sep 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

But since you put source_dir in default above as "" would the source_dir if in 459 not always be true then, caues it's never the default None?

@AlexanderLanin AlexanderLanin Sep 14, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

when a user calls docs or docs_bundle the source_dir defaults to None. This place is exactly where None is transformed to "". Maybe it makes more sense to store None instead of empty strings, but that would be the next refactoring starting from this point.

@MaximilianSoerenPollak MaximilianSoerenPollak Sep 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I mean here then you say the attribute is `""`` per default.

Is this not an issue?
https://github.com/eclipse-score/docs-as-code/pull/820/changes#diff-637f1bfda16cc7049dbae85157101729a0cadc42705e2acce9653e9382e9c096R425-R427

@AlexanderLanin AlexanderLanin Sep 15, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

That default "" is exactly what is being set here

entry_doc = entry_doc,
bundles = [bundle.bundle for bundle in parsed_bundles],
bundle_mount_ats = [bundle.mount_at for bundle in parsed_bundles],
Expand Down
11 changes: 5 additions & 6 deletions docs.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,10 @@ def _declare_docs_bundle(
source_dir: optional directory holding this bundle's own doc sources. It is
globbed like `docs()` (same file kinds) and the contents are stored after
stripping the `source_dir` prefix. Leave it unset for a pure aggregator.
For a source-bearing bundle this is a package-relative directory name;
`"."` means the package root. If `srcs` is supplied instead, those
explicit files determine the Sphinx action's source root. `None` means
that the bundle has no directory-glob source root.
srcs: Explicit documentation source files, including generated files.
Use this for a source-less bundle whose documentation is produced by a
build action. All files must share one parent directory so they can be
Expand Down Expand Up @@ -275,11 +279,6 @@ def _declare_docs_bundle(
code_targets = code_targets,
)

# Keep the bundle source root relative to the workspace. ``join_path``
# normalizes ``source_dir = "."`` to the package path.
pkg = native.package_name()
strip_prefix = join_path(pkg, source_dir) if source_dir != None else ""

# ``needs_json`` is an inventory consumed by score_metamodel, not content
# owned by this bundle. It must remain in the caller's build/runfile inputs
# for the legacy ``docs(data = [...])`` API, but propagating the TreeArtifact
Expand All @@ -298,7 +297,7 @@ def _declare_docs_bundle(
source_dir_globbed = source_dir_globbed,
source_targets = srcs,
sourcelinks_json = sourcelinks_json,
strip_prefix = strip_prefix,
source_dir = source_dir,
entry_doc = entry_doc,
bundles = bundles,
data = bundle_data,
Expand Down
Loading