refactor: Consolidate UI5 data directory resolution into resolveUi5DataDir#1456
Open
d3xter666 wants to merge 12 commits into
Open
refactor: Consolidate UI5 data directory resolution into resolveUi5DataDir#1456d3xter666 wants to merge 12 commits into
d3xter666 wants to merge 12 commits into
Conversation
…taDir Introduces packages/project/lib/utils/dataDir.js with a single resolveUi5DataDir() utility that encapsulates the full resolution chain: 1. UI5_DATA_DIR environment variable 2. ui5DataDir from configuration file (~/.ui5rc) 3. Default: ~/.ui5 Removes the duplicate getUi5DataDir() function from packages/cli/lib/framework/utils.js and migrates its callers (createFrameworkResolverInstance, frameworkResolverResolveVersion) to use resolveUi5DataDir() directly. The defensive fallbacks in the public Resolver/Installer APIs (AbstractResolver, Openui5Resolver, Sapui5Resolver, Sapui5MavenSnapshotResolver) are intentionally kept — those are synchronous constructors that cannot call the async utility, and they are publicly exported so external callers may pass undefined.
…ve path resolution Renames the cwd option to projectRootPath to make its purpose explicit: this is the root of the specific project being processed (where package.json lives), not necessarily the shell's current working directory. When omitted, falls back to process.cwd(). Callers in createFrameworkResolverInstance and frameworkResolverResolveVersion now thread the project root through so relative ui5DataDir values in config are resolved against the correct base directory.
…i5DataDir
Two call sites that duplicated the env→config→default resolution logic
are migrated to resolveUi5DataDir({projectRootPath}):
- packages/project/lib/graph/helpers/ui5Framework.js:
enrichProjectGraph resolves ui5DataDir via resolveUi5DataDir using
rootProject.getRootPath() as projectRootPath, then passes it to
the framework Resolver. Configuration and os imports removed.
- packages/project/lib/build/cache/CacheManager.create():
The no-ui5DataDir path now delegates to resolveUi5DataDir instead of
inlining env/config/default logic. The explicit ui5DataDir option
still resolves against cwd for backward compatibility. os and
Configuration imports removed.
Tests updated: resolveUi5DataDir stub added to ui5Framework.js esmocks
so the stub mirrors the real resolution logic (including env and config
stubs), and package-exports count updated.
…i5DataDir
Two call sites that duplicated the env→config→default resolution logic
are migrated to resolveUi5DataDir({projectRootPath}):
- packages/project/lib/graph/helpers/ui5Framework.js:
enrichProjectGraph resolves ui5DataDir via resolveUi5DataDir using
rootProject.getRootPath() as projectRootPath, then passes it to
the framework Resolver. Configuration and os imports removed.
- packages/project/lib/build/cache/CacheManager.create():
The no-ui5DataDir path now delegates to resolveUi5DataDir instead of
inlining env/config/default logic. The explicit ui5DataDir option
still resolves against cwd for backward compatibility. os and
Configuration imports removed.
Tests updated: resolveUi5DataDir stub added to ui5Framework.js esmocks
so the stub mirrors the real resolution logic (including env and config
stubs), and package-exports count updated.
d3xter666
force-pushed
the
refactor/resolve-ui5-data-dir
branch
from
July 17, 2026 11:26
95e6631 to
b449a91
Compare
Replace hardcoded POSIX absolute paths (/my/project, /custom/data/dir, etc.) with path.resolve(os.tmpdir(), ...) so assertions hold on all platforms. On Windows, /foo is not an absolute path — it is relative to the current drive root — causing four tests to fail.
d3xter666
force-pushed
the
refactor/resolve-ui5-data-dir
branch
from
July 17, 2026 11:27
b449a91 to
b091c77
Compare
- Use path.resolve instead of path.join for the default ~/.ui5 fallback so the result is absolute even when os.homedir() returns a relative path (e.g. "./" in some CI/container environments). - Add edge-case test that asserts absoluteness when HOME is relative. - Fix CLI framework/utils tests: stubs resolving undefined reflected old behavior where no ui5DataDir meant undefined; add clarifying comments that these tests validate stub wiring, not real resolution. - Add utils/dataDir to the explicit API parity list in package-exports test so a path/mapping regression would be caught by a specific test.
- cli/framework/utils.js: stub now resolves the real default path (path.join(os.homedir(), ".ui5")) instead of undefined, matching the production contract of resolveUi5DataDir. - graph/helpers/ui5Framework.js: stub and assertions use path.resolve instead of path.join for the default ~/.ui5 path, matching the stricter absolute-path guarantee of the implementation.
The stub for resolveUi5DataDir does not need to compute a real filesystem path — it just needs an opaque value that flows through correctly. Replace path.join(os.homedir(), '.ui5') with the string literal 'my-default-ui5-data-dir' and remove the unused os and path imports.
d3xter666
marked this pull request as ready for review
July 17, 2026 14:58
Member
|
I'm wondering whether we still need the fallbacks in https://github.com/UI5/cli/blob/refactor/resolve-ui5-data-dir/packages/project/lib/ui5Framework/AbstractResolver.js#L44 and https://github.com/UI5/cli/blob/refactor/resolve-ui5-data-dir/packages/project/lib/build/cache/CacheManager.js#L77 since we now always provide a |
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
2 participants
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.
Introduces
packages/project/lib/utils/dataDir.jswith a singleresolveUi5DataDir()utility that encapsulates the full resolution chain:UI5_DATA_DIRenvironment variableui5DataDirfrom configuration file~/.ui5Removes the duplicate
getUi5DataDir()function frompackages/cli/lib/framework/utils.jsand migrates its callers (createFrameworkResolverInstance,frameworkResolverResolveVersion) to useresolveUi5DataDir()directly.Consolidates and enforces the new utility accross packages