Skip to content
This repository was archived by the owner on Jan 9, 2024. It is now read-only.

fix: handle long classpaths #38

Merged
merged 1 commit into from
Nov 19, 2020
Merged

fix: handle long classpaths #38

merged 1 commit into from
Nov 19, 2020

Conversation

muscar
Copy link
Contributor

@muscar muscar commented Nov 18, 2020

  • Tests written and linted ℹ︎
  • Documentation written ℹ︎
  • Commit history is tidy ℹ︎

What this does

This change set updates the maven wrapper to use only plugins for determining the classpath we pass to the call graph builder. It uses dependency:build-classpath to get the classpath for the dependencies. Sadly, that doesn't include the output directory (I tried to set the scope to compile, but I couldn't get it to add the output directory to the classpath it gives back). To work around this, it uses help:evaluate to get project.build.outputDirectory, and append it to the classpath manually.

This has a few advantages:

  • it's not limited by the maximum length of the command line on any given platform;
  • it doesn't have to parse maven output;

and a few shortcomings:

  • it needs to use temporary files because I couldn't find a way to make maven not produce logs;
  • it needs an extra plugin maven-help, but that seems to be available without having to add it to the POM file.

It's been tested on macOS and Windows, and it gives the same results as the current approach.

For context: https://snyk.slack.com/archives/C07N668DA/p1605690001356600

Notes for the reviewer

I've opted to keep the current method as a fallback. We'll remove it when we can confirm this works in the wild.

More information

@muscar muscar requested a review from a team as a code owner November 18, 2020 13:38
@SarahU
Copy link
Contributor

SarahU commented Nov 18, 2020

it needs an extra plugin maven-help, but that seems to be available without having to add it to the POM file - I double checked this locally so can concur. Seems to require an installation but no POM changes

@muscar muscar force-pushed the fix/handle-long-classpaths branch from fbc2b32 to 44ff95a Compare November 18, 2020 13:48
@muscar muscar force-pushed the fix/handle-long-classpaths branch from 44ff95a to b0743e9 Compare November 18, 2020 14:42
@muscar muscar merged commit 86e4701 into master Nov 19, 2020
@muscar muscar deleted the fix/handle-long-classpaths branch November 19, 2020 08:33
@snyksec
Copy link

snyksec commented Nov 19, 2020

🎉 This PR is included in version 1.16.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

4 participants