-
Notifications
You must be signed in to change notification settings - Fork 4
Fix CDS extractor for cds-indexer private package dependency installation
#359
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.
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
base: main
Are you sure you want to change the base?
Changes from 5 commits
76340e5
6143cb5
c2d3cc7
c6b632c
2001913
6921c2b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,7 @@ import { delimiter, join } from 'path'; | |
|
|
||
| import { addCdsIndexerDiagnostic } from '../diagnostics'; | ||
| import { cdsExtractorLog } from '../logging'; | ||
| import { projectInstallDependencies } from '../packageManager'; | ||
| import type { CdsDependencyGraph, CdsProject } from './parser/types'; | ||
|
|
||
| /** Maximum time (ms) allowed for a single cds-indexer invocation. */ | ||
|
|
@@ -114,6 +115,7 @@ export function runCdsIndexer( | |
| env, | ||
| stdio: 'pipe', | ||
| timeout: CDS_INDEXER_TIMEOUT_MS, | ||
| shell: true, | ||
|
||
| }); | ||
|
|
||
| result.durationMs = Date.now() - startTime; | ||
|
|
@@ -199,6 +201,19 @@ export function orchestrateCdsIndexer( | |
|
|
||
| if (result.success) { | ||
| summary.successfulRuns++; | ||
|
|
||
| // Install the project's full dependencies so the CDS compiler can | ||
| // 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); | ||
| if (!installResult.success) { | ||
| cdsExtractorLog( | ||
| 'warn', | ||
| `Full dependency installation failed for project '${projectDir}' after successful cds-indexer run: ${installResult.error ?? 'unknown error'}`, | ||
| ); | ||
| } | ||
| } else { | ||
| summary.failedRuns++; | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -497,6 +497,7 @@ function installDependenciesInCache( | |||||||||||||||||||
| execFileSync('npm', ['install', '--quiet', '--no-audit', '--no-fund'], { | ||||||||||||||||||||
| cwd: cacheDir, | ||||||||||||||||||||
| stdio: 'inherit', | ||||||||||||||||||||
| shell: true, | ||||||||||||||||||||
|
||||||||||||||||||||
| execFileSync('npm', ['install', '--quiet', '--no-audit', '--no-fund'], { | |
| cwd: cacheDir, | |
| stdio: 'inherit', | |
| shell: true, | |
| const npmExecutable = process.platform === 'win32' ? 'npm.cmd' : 'npm'; | |
| execFileSync(npmExecutable, ['install', '--quiet', '--no-audit', '--no-fund'], { | |
| cwd: cacheDir, | |
| stdio: 'inherit', |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -75,6 +75,7 @@ export function projectInstallDependencies( | |
| cwd: projectPath, | ||
| stdio: 'inherit', | ||
| timeout: 120000, // 2-minute timeout | ||
| shell: true, | ||
|
||
| }); | ||
|
|
||
| result.success = true; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shellwas switched fromfalsetotruefor all CDS compilation spawns. This changes argument handling semantics (especially for paths with spaces) and increases exposure to shell-quoting edge cases. Prefer keepingshell: falseand using platform-specific executables (npx.cmd/cds.cmd) or only enablingshellon Windows when needed.See below for a potential fix: