Skip to content

Commit a977bfd

Browse files
committed
Address PR review feedback
1 parent 7dac699 commit a977bfd

File tree

5 files changed

+83
-14
lines changed

5 files changed

+83
-14
lines changed

extractors/cds/tools/dist/cds-extractor.bundle.js

Lines changed: 4 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

extractors/cds/tools/dist/cds-extractor.bundle.js.map

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

extractors/cds/tools/src/packageManager/cacheInstaller.ts

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -259,12 +259,14 @@ export function cacheInstallDependencies(
259259
);
260260
continue;
261261
}
262+
}
262263

263-
// Copy the project's .npmrc (if any) so npm respects custom registries
264-
const firstProjectDir = Array.from(dependencyGraph.projects.keys())[0];
265-
if (firstProjectDir) {
266-
copyNpmrcToCache(cacheDir, join(sourceRoot, firstProjectDir));
267-
}
264+
// Ensure the cache directory has an .npmrc that reflects the projects' registry configuration
265+
const npmrcProjectDir = Array.from(dependencyGraph.projects.values())
266+
.map(project => project.projectDir)
267+
.find(projectDir => projectDir && existsSync(join(sourceRoot, projectDir, '.npmrc')));
268+
if (npmrcProjectDir) {
269+
copyNpmrcToCache(cacheDir, join(sourceRoot, npmrcProjectDir));
268270
}
269271

270272
// Try to install dependencies in the cache directory

extractors/cds/tools/test/src/packageManager/cacheInstaller.test.ts

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -809,5 +809,72 @@ describe('installer', () => {
809809

810810
expect(fs.copyFileSync).not.toHaveBeenCalled();
811811
});
812+
813+
it('should find .npmrc from the project that has one, not just the first project', () => {
814+
// Only project2 has an .npmrc; project1 does not
815+
(fs.existsSync as jest.Mock).mockImplementation(
816+
(p: string) => p === '/source/project2/.npmrc',
817+
);
818+
819+
const dependencyGraph = createMockDependencyGraph([
820+
{
821+
projectDir: 'project1',
822+
packageJson: {
823+
name: 'project1',
824+
dependencies: { '@sap/cds': '6.1.3' },
825+
devDependencies: { '@sap/cds-dk': '6.0.0' },
826+
},
827+
},
828+
{
829+
projectDir: 'project2',
830+
packageJson: {
831+
name: 'project2',
832+
dependencies: { '@sap/cds': '6.1.3' },
833+
devDependencies: { '@sap/cds-dk': '6.0.0' },
834+
},
835+
},
836+
]);
837+
838+
cacheInstallDependencies(dependencyGraph, '/source', '/codeql');
839+
840+
expect(fs.copyFileSync).toHaveBeenCalledWith(
841+
expect.stringContaining('.npmrc'),
842+
expect.stringContaining('.npmrc'),
843+
);
844+
});
845+
846+
it('should copy .npmrc even when the cache directory already exists', () => {
847+
// Simulate cache directory already existing but .npmrc present
848+
(fs.existsSync as jest.Mock).mockImplementation((p: string) => {
849+
if (p === '/source/project1/.npmrc') return true;
850+
// Cache root exists
851+
if (
852+
p.includes('.cds-extractor-cache') &&
853+
!p.includes('node_modules') &&
854+
!p.includes('.npmrc')
855+
)
856+
return true;
857+
return false;
858+
});
859+
860+
const dependencyGraph = createMockDependencyGraph([
861+
{
862+
projectDir: 'project1',
863+
packageJson: {
864+
name: 'project1',
865+
dependencies: { '@sap/cds': '6.1.3' },
866+
devDependencies: { '@sap/cds-dk': '6.0.0' },
867+
},
868+
},
869+
]);
870+
871+
cacheInstallDependencies(dependencyGraph, '/source', '/codeql');
872+
873+
// .npmrc should still be copied even though cache dir already exists
874+
expect(fs.copyFileSync).toHaveBeenCalledWith(
875+
'/source/project1/.npmrc',
876+
expect.stringContaining('.npmrc'),
877+
);
878+
});
812879
});
813880
});

extractors/javascript/tools/pre-finalize.cmd

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,9 @@ if not defined CODEQL_EXTRACTOR_CDS_SKIP_EXTRACTION (
1212
-- ^
1313
"%CODEQL_EXTRACTOR_JAVASCRIPT_WIP_DATABASE%"
1414

15-
if %ERRORLEVEL% neq 0 (
16-
echo database index-files for CDS (.cds) files failed with exit code %ERRORLEVEL%.
17-
exit /b %ERRORLEVEL%
15+
if errorlevel 1 (
16+
echo database index-files for CDS (.cds) files failed.
17+
exit /b 1
1818
)
1919

2020
echo Finished running database index-files for CDS (.cds) files.

0 commit comments

Comments
 (0)