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.

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

Initial version of platform-independent CDS extractor #169

Open
wants to merge 27 commits into
base: main
Choose a base branch
from

Conversation

data-douser
Copy link
Collaborator

Adds .cmd script equivalents for autobuild.sh, index-files.sh, and pre-finalize.sh scripts -- with the (unverified) intention being that the CDS extractor should be able to run on Linux, Mac, or Windows.

Migrates most of the index-files script logic from shell script to javascript.

Adds a package.json file to assist with managing dependencies for the new extractors/cds/tools/index-files.js script.

Adds .cmd script equivalents for autobuild.sh, index-files.sh, and
pre-finalize.sh scripts -- with the (unverified) intention being that
the CDS extractor should be able to run on Linux, Mac, or Windows.

Migrates most of the `index-files` script logic from shell script to
javascript.

Adds a `package.json` file to assist with managing dependencies for
the new `extractors/cds/tools/index-files.js` script.
@data-douser data-douser self-assigned this Jan 23, 2025
Copy link
Contributor

@lcartey lcartey left a comment

Choose a reason for hiding this comment

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

Added some initial feedback - looking good overall, just a few specific comments.

Adds comments in index-files.js to better reflect the documented
intentions from the old version of index-files.sh script. Better
explains the script magic, in places.

Attempts to make the index-files.js (JavaScript) script more useful
as a multi-platform solution by normalizing file paths using the
path.join() function.
First attempt at fixing `Indirect uncontrolled command line` code
scanning alerts for the `index-fils.js` script.

Improves error handling and improves the reliability and security of
code that creates child (exec/spawn) processes.

Attempts to improve the passing of env vars to child processes,
especially for the `LGTM_INDEX_FILTERS` env var.

WIP because CDS extractor invocation is still failing to identify
.cds.json files. Possible problem in the way env vars are passed
within the javascript extractor autobuilder shell script (to the JVM
launched by the javascript extractor autobuilder).
Fixes the invocation of the javascript extractor autobuild within the
index-files.js script of the CDS extractor. Ensures that the `cwd`
property/option is set when spawning the process that runs the
javascript extractor autobuild.

Potential working version of initial rewrite for the CDS extractor.
Changes the way the index-files.js script is invoked such that the
original `--source-root` directory is used, where possible, as the
current working directory for any work performed with within the
extractor. Passes the original working directory as a parameter of
the index-files.js script to allow that (child) script to run from
the project/source root while ensuring node package dependencies
are still installed in `extractors/cds/tools/node_modules`.
Forces the `cds` compilier to output JSON to a file (via stdout) instead
of creating an output directory. Accounts for what appears to be a
change in the behavior of the `cds` (CLI) compiler, where the `-o` (or
`--dest`) option now `Writes output to the given folder instead of stdout`.

```
$ npx cds --version

@cap-js/asyncapi: 1.0.2
@cap-js/db-service: 1.16.0
@cap-js/openapi: 1.1.2
@cap-js/sqlite: 1.7.7
@capire/bookshop: 1.0.0
@sap/cds: 8.5.0
@sap/cds-compiler: 5.2.0
@sap/cds-dk: 8.7.1
@sap/cds-fiori: 1.2.7
@sap/cds-foss: 5.0.1
@sap/cds-mtxs: 2.5.1
@sap/eslint-plugin-cds: 3.1.2
Node.js: v20.15.0
```
Avoids changing directory when running the `cds` compiler, which
ensures that paths generated in .cds.json files are relative to the
"source root" directory. This replicates the behavior of the original
index-files.sh (shell) script, which works but is probably not correct.
Adds .cmd script equivalents for autobuild.sh, index-files.sh, and
pre-finalize.sh scripts -- with the (unverified) intention being that
the CDS extractor should be able to run on Linux, Mac, or Windows.

Migrates most of the `index-files` script logic from shell script to
javascript.

Adds a `package.json` file to assist with managing dependencies for
the new `extractors/cds/tools/index-files.js` script.
Adds comments in index-files.js to better reflect the documented
intentions from the old version of index-files.sh script. Better
explains the script magic, in places.

Attempts to make the index-files.js (JavaScript) script more useful
as a multi-platform solution by normalizing file paths using the
path.join() function.
First attempt at fixing `Indirect uncontrolled command line` code
scanning alerts for the `index-fils.js` script.

Improves error handling and improves the reliability and security of
code that creates child (exec/spawn) processes.

Attempts to improve the passing of env vars to child processes,
especially for the `LGTM_INDEX_FILTERS` env var.

WIP because CDS extractor invocation is still failing to identify
.cds.json files. Possible problem in the way env vars are passed
within the javascript extractor autobuilder shell script (to the JVM
launched by the javascript extractor autobuilder).
Fixes the invocation of the javascript extractor autobuild within the
index-files.js script of the CDS extractor. Ensures that the `cwd`
property/option is set when spawning the process that runs the
javascript extractor autobuild.

Potential working version of initial rewrite for the CDS extractor.
Changes the way the index-files.js script is invoked such that the
original `--source-root` directory is used, where possible, as the
current working directory for any work performed with within the
extractor. Passes the original working directory as a parameter of
the index-files.js script to allow that (child) script to run from
the project/source root while ensuring node package dependencies
are still installed in `extractors/cds/tools/node_modules`.
Forces the `cds` compilier to output JSON to a file (via stdout) instead
of creating an output directory. Accounts for what appears to be a
change in the behavior of the `cds` (CLI) compiler, where the `-o` (or
`--dest`) option now `Writes output to the given folder instead of stdout`.

```
$ npx cds --version

@cap-js/asyncapi: 1.0.2
@cap-js/db-service: 1.16.0
@cap-js/openapi: 1.1.2
@cap-js/sqlite: 1.7.7
@capire/bookshop: 1.0.0
@sap/cds: 8.5.0
@sap/cds-compiler: 5.2.0
@sap/cds-dk: 8.7.1
@sap/cds-fiori: 1.2.7
@sap/cds-foss: 5.0.1
@sap/cds-mtxs: 2.5.1
@sap/eslint-plugin-cds: 3.1.2
Node.js: v20.15.0
```
Avoids changing directory when running the `cds` compiler, which
ensures that paths generated in .cds.json files are relative to the
"source root" directory. This replicates the behavior of the original
index-files.sh (shell) script, which works but is probably not correct.
…codeql-sap-js into data-douser/extractor-dev-2
@data-douser data-douser marked this pull request as ready for review February 24, 2025 17:42
@data-douser data-douser requested a review from lcartey February 24, 2025 17:42
@data-douser
Copy link
Collaborator Author

This version does not enforce the use of @sap/[email protected], as was recently changed on main branch.
However, it seems that we don't need to pin to that version in order for checks to pass, so I have not copied that change in this PR. Open to feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants