Fix CDS extractor for cds-indexer private package dependency installation#359
Merged
data-douser merged 6 commits intomainfrom Apr 22, 2026
Merged
Fix CDS extractor for cds-indexer private package dependency installation#359data-douser merged 6 commits intomainfrom
cds-indexer private package dependency installation#359data-douser merged 6 commits intomainfrom
Conversation
Attempt to fix the CDS extractor's support for private "cds-indexer" package installation by ensuring that the associated project's entire node_modules directory is copied instead of just the cachable subset of the CAP projects NodeJS dependencies.
data-douser
added
bug
* CDS extractor windows fixes - 1
* Fix Windows ENOENT: add shell:true to npm/npx spawn calls
On Windows, npm and npx are .cmd files. execFileSync('npm', ...)
and spawnSync('npx', ...) fail with ENOENT because they bypass the
shell and cannot resolve .cmd extensions. Adding shell: true fixes
this on Windows while being a no-op on Linux/macOS.
Affected files:
- cacheInstaller.ts: execFileSync('npm', ['install', ...])
- projectInstaller.ts: execFileSync('npm', ['install', ...])
- indexer.ts: spawnSync('npx', ['--yes', ...])
* Fix Windows: shell:true for npx spawn, posix paths in LGTM_INDEX_FILTERS
- compile.ts: change shell:false to shell:true so npx (a .cmd on
Windows) can be resolved during CDS compilation
- environment.ts: use explicit forward-slash paths ('exclude:**/*.*')
instead of join() which produces backslashes on Windows, causing
'Illegal use of **' errors in the JS extractor
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the CDS extractor workflow to better handle CAP projects that depend on @sap/cds-indexer and additional (including private) project dependencies, with several Windows-specific robustness fixes.
Changes:
- After a successful
cds-indexerrun, trigger a full project dependency installation to improve CDS import resolution during compilation. - Adjust process execution and scripts for Windows compatibility (batch escaping;
shellusage for npm/npx). - Improve Windows path/glob handling (
LGTM_INDEX_FILTERSpatterns andgloboptions).
Show a summary per file
| File | Description |
|---|---|
| extractors/javascript/tools/pre-finalize.cmd | Escapes parentheses in echo statements inside batch blocks. |
| extractors/cds/tools/index-files.cmd | Escapes parentheses in error message echo for batch compatibility. |
| extractors/cds/tools/src/cds/indexer.ts | Runs cds-indexer with updated spawn options and installs full dependencies after successful runs. |
| extractors/cds/tools/src/packageManager/projectInstaller.ts | Updates npm install execution options used for full project installs. |
| extractors/cds/tools/src/packageManager/cacheInstaller.ts | Updates cached dependency installation command execution options. |
| extractors/cds/tools/src/cds/compiler/compile.ts | Changes spawn options for CDS compilation processes. |
| extractors/cds/tools/src/environment.ts | Uses forward-slash LGTM index filter patterns to avoid Windows glob issues. |
| extractors/cds/tools/src/cds/parser/functions.ts | Sets glob options to improve Windows path behavior. |
| extractors/cds/tools/cds-extractor.ts | Sets glob options to improve Windows path behavior. |
| extractors/cds/tools/test/src/cds/indexer.test.ts | Adds mocks/tests covering post-indexer dependency installation behavior. |
| extractors/cds/tools/test/src/packageManager/projectInstaller.test.ts | Updates expectations for the changed npm install spawn options. |
| extractors/cds/tools/dist/cds-extractor.bundle.js | Regenerated bundle reflecting the TypeScript source changes. |
Copilot's findings
Comments suppressed due to low confidence (1)
extractors/cds/tools/src/cds/indexer.ts:210
- This unconditionally triggers a full
npm installfor every successfulcds-indexerrun. That can significantly increase extractor time/network usage and largely defeats the cached-deps approach. Consider gating this (e.g., only when full deps are actually needed / not yet installed), and if installation succeeds, update the project’sretryStatus.fullDependenciesInstalledso the retry pipeline doesn’t reinstall again later.
// resolve all CDS model imports (e.g. `@sap/cds-shim`) during
// compilation. The cache directory only contains `@sap/cds`,
// `@sap/cds-dk`, and `@sap/cds-indexer`, which is not enough for
// projects that reference additional CDS packages.
const installResult = projectInstallDependencies(project, sourceRoot);
- Files reviewed: 11/14 changed files
- Comments generated: 4
data-douser
added a commit
that referenced
this pull request
Apr 18, 2026
Contributor
There was a problem hiding this comment.
Copilot's findings
Comments suppressed due to low confidence (1)
extractors/cds/tools/src/environment.ts:263
- In
configureLgtmIndexFilters, the logic that preserves existingexclude:lines is too broad: usingline.includes('exclude:**/*')will also match and strip more specific exclusions likeexclude:**/*.ts(and most otherexclude:**/...patterns). This can unintentionally drop user-provided excludes. Consider matching the generic patterns by exact equality (after trimming) instead ofincludes, or using a regex that only matches the two generic patterns.
// Use forward slashes explicitly — join() uses backslashes on Windows
// which causes 'Illegal use of **' errors in the JS extractor.
const allowedExcludePatterns = ['exclude:**/*', 'exclude:**/*.*'];
excludeFilters =
'\n' +
process.env.LGTM_INDEX_FILTERS.split('\n')
.filter(
line =>
line.startsWith('exclude') &&
!allowedExcludePatterns.some(pattern => line.includes(pattern)),
)
- Files reviewed: 13/16 changed files
- Comments generated: 2
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.
What This PR Contributes
This pull request adds logic to install full project dependencies after a successful
cds-indexerrun, ensuring that all required CDS model imports can be resolved during compilation. The change is also thoroughly tested, including cases where dependency installation fails or is unnecessary.Outline of Changes
Dependency installation improvements:
cds-indexerrun, the code now callsprojectInstallDependenciesto install all required project dependencies, ensuring the CDS compiler can resolve all imports. If the installation fails, a warning is logged, but the process continues. [1] [2]Testing enhancements:
projectInstallDependenciesand related functions were added to the test suite to simulate dependency installation outcomes. [1] [2]Future Works
N/A