Skip to content
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.

Sign up for GitHub

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

cds extractor: npm install in each package.json #163

Merged
merged 2 commits into from
Dec 3, 2024
Merged

cds extractor: npm install in each package.json #163

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f45f8a5
npm install in each package.json
lcartey Dec 2, 2024
dd52d5d
Merge branch 'main' into lcartey/npm-install
jeongsoolee09 Dec 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion extractors/cds/tools/index-files.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ then
# directory.
#
# We also ensure we skip node_modules, as we can end up in a recursive loop
find . -type d -name node_modules -prune -false -o -type f \( -iname 'package.json' \) -exec grep -ql '@sap/cds' {} \; -execdir bash -c "grep -q \"^\$(pwd)\(/\|$\)\" \"$response_file\"" \; -execdir bash -c "echo \"Installing @sap/cds-dk into \$(pwd) to enable CDS compilation.\"" \; -execdir npm install --silent @sap/cds-dk \;
find . -type d -name node_modules -prune -false -o -type f \( -iname 'package.json' \) -exec grep -ql '@sap/cds' {} \; -execdir bash -c "grep -q \"^\$(pwd)\(/\|$\)\" \"$response_file\"" \; -execdir bash -c "echo \"Installing @sap/cds-dk into \$(pwd) to enable CDS compilation.\"" \; -execdir npm install --silent @sap/cds-dk \; -execdir npm install --silent \;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A quick thought: would it make sense to install @sap/cds-dk globally? Then we don't have to run this find command.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately not. We have seen cases of monorepos where different versions of @sap/cds are specified in different directories. The validity of the CDS file format can change between major versions, so it's not always possible to just install one version and have it work everywhere. This approach ensures we install a compatible version of the @sap/cds-dk tools for each directory, minimising the chance of failure.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then we'll have to do it project by project then. Thanks!


# Use the npx command to dynamically install the cds development kit (@sap/cds-dk) package if necessary,
# which then provides the cds command line tool in directories which are not covered by the package.json
Expand Down
Loading