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

Generating bom for a mixed project includes devDependencies when using --required-only #1598

Open
Torbjorn-Svensson opened this issue Jan 24, 2025 · 5 comments

Comments

@Torbjorn-Svensson
Copy link

In some of our deliverables, we use a combination of package.json and Cargo.toml.
The reason for the mix is that we want to use some tools to format and check files in the delivery (among other releng things).
Note: We are not using the package.json for the delivery, only as a means to ensure we have the right tools.

Example package.json:

{
    "name": "foo",
    "version": "1.2.3",
    "license": "Our corporate license",
    "scripts": {
      "build": "cargo build",
      "format:check": "yarn prettier --check .",
    },
    "devDependencies": {
      "prettier": "^3.4.2"
   }
}

To ensure reproducibility, we also have the corresponding yarn.lock file in our repository.

When running cdxgen on this type of setup, everything in the yarn.lock file is consider a required dependency. Is this expected?
From what I can tell, the reason why prettier, from my example above, is considered a required dependency is that there is no import or exports detected (since there is no sources for this package).

cdxgen is invoked with --deep --fail-on-error --required-only.

From my POV, I do not expect prettier to be listed in the bom as it's a dev-dependency, but maybe I'm wrong here?

@prabhu
Copy link
Collaborator

prabhu commented Jan 26, 2025

@Torbjorn-Svensson This is a good bug. It appears like the parseYarnLock method isn't attempting to set the scope attribute.

pkgList.push({

In such cases, if the complete source code were present, then the babel-based usage analysis might have helped. Are you running cdxgen with just the lock file without the source code? In such cases, the logic could be improved to use the package.json to mark optional packages.

@Torbjorn-Svensson
Copy link
Author

I've tried generating the bom after running yarn install --production and yarn install, but I do not see any difference in the resulting json file.
I do not know if it makes any difference, but I'm launching cdxgen using npx.

@prabhu
Copy link
Collaborator

prabhu commented Jan 26, 2025

Needs some triaging. Have to print the values here and see what is going on.

allImports = retData.allImports;

Have you tried simple things like passing an absolute path to the current directory to cdxgen? Add $(pwd) to the end of the command?

@Torbjorn-Svensson
Copy link
Author

I did some debugging last Friday before opening this issue and from what I can tell, allImports will only contain something if there is actually javascript code in the tree. In the case I mentioned in this issue, there is none (only rust code).

Supplying a relative or an absolute path does not matter.

I'm trying to work out how to run cdxgen to get the right content for a few different projects.

  1. I can only get the right bom content by running yarn install in the rust example that I gave in this issue. yarn install --production will include prettier (dev dep).
  2. I can only get the right bom content by running yarn install --production for other parts that do contain javascript code. yarn install will include a lot of dev dep. that should not be listed.

So, as you can see, the bom content depends on what extra options you called yarn install with. To me, this looks buggy and cdxgen would have to consider what the package.json file has declared the dependency as (currently is kinda consider everything to be mandatory dep. and then eliminate all modules that does not appear to be imported).

Note, in all my trails, I've used the same command line options to cdxgen (only thing changed is the way to install the deps and relative/absolute/no path given to cdxgen).

@prabhu
Copy link
Collaborator

prabhu commented Jan 26, 2025

Sorry, didn't read your original comment entirely. This confirms my theory. Yes, we need an enhancement to fall back to using package.json. Will keep this issue open.

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

No branches or pull requests

2 participants