fix(project): Route Theme-Library Resources in BuildReader - #1611
Merged
Merged
Conversation
A theme library contributes resources under a path that can collide with another project's namespace: themelib_sap_horizon has no namespace and serves /resources/sap/ui/core/themes/sap_horizon/library.css. BuildReader walks that path, matches the sap/ui/core namespace of the sap.ui.core library (which does not own the resource), reads null, then falls back to a reader for every project. That fallback (re)builds unrelated stale projects such as sap.m. Add an integration-style harness that records which projects a single byPath request would build, and three tests: the colliding theme resource, the base theme (owned by the library itself), and a non-colliding theme resource. The two theme-library routing tests fail against the current fallback-to-all behavior. The base-theme test already passes.
RandomByte
marked this pull request as ready for review
September 24, 2026 12:57
RandomByte
force-pushed
the
fix/build-reader-getReaderForResource
branch
from
September 24, 2026 13:12
989760a to
755539a
Compare
Theme libraries have no namespace and serve their resources under a path owned by another project, e.g. themelib_sap_horizon serves /resources/sap/ui/core/themes/sap_horizon/library.css. The previous routing matched such a path to the sap/ui/core namespace of the sap.ui.core library, read null there, then requested a reader for every project. That request (re)builds every non-fresh project, including ones unrelated to the theme. Replace the single best-guess reader with an ordered list of candidate reader factories evaluated lazily by byPath, returning the first resource found: 1. cached readers of already-built projects (free, identifies the owner) 2. namespace matches, most specific first 3. theme libraries, when the path has a themes/ segment 4. the application root project, for non-/resources paths 5. all projects (last resort) A reader is requested from the build server only when the preceding candidates did not yield the resource, so the minimal set of projects is built. The base theme, owned by sap.ui.core itself, still resolves at the namespace step without building any theme library.
RandomByte
force-pushed
the
fix/build-reader-getReaderForResource
branch
from
September 24, 2026 13:19
755539a to
6c2cea6
Compare
matz3
approved these changes
Sep 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Theme libraries have no namespace and serve their resources under a path owned by another project, for example
themelib_sap_horizonserves/resources/sap/ui/core/themes/sap_horizon/library.css.BuildReaderrouted such a request by walking the path and matching thesap/ui/corenamespace of thesap.ui.corelibrary, which does not own the resource. The read returned null, so the reader fell back to requesting a reader for every project. That fallback (re)builds every non-fresh project, including libraries such assap.mthat have nothing to do with the theme.Problem
BuildReader.byPathused a single best-guess reader from_getReaderForResource, then a single fallback to a reader for all projects. Namespace matching is path-based, so a theme resource served under another project's namespace segment resolves to the wrong project, reads null, and drops straight to the all-projects fallback. Requesting a reader for all projects is the expensive path: it (re)builds every stale project in the graph, so a single theme request could trigger unrelated builds.Fix
Replace the single best-guess reader with an ordered list of candidate reader factories, evaluated lazily by
byPath, returning the first resource found. A reader is requested from the build server only when the preceding candidates did not yield the resource, so the minimal set of projects is built.The candidate order, most specific and cheapest first:
themes/segment. Theme libraries are tracked separately at construction time because they have no namespace to match on./resources/and/test-resources/.The base theme, owned by
sap.ui.coreitself, still resolves at the namespace step (2) and builds no theme library.Naming
_getReaderForResourcereturned a single reader; the new method returns the ordered candidate list and is named_getReaderCandidates.Testing
An integration-style harness records which projects a single
byPathrequest would build, standing in for the build theBuildServertriggers for a non-fresh project. Three tests cover the routing:themelib_sap_horizonunder thesap.ui.corenamespace) resolves from the theme library, and unrelated projects (sap.m, the application) are not built.The two theme-library routing tests were added first and failed against the previous fallback-to-all behavior. The base-theme test passed before the fix and still passes.