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

Fix maven pom parent child relationships #1218

Merged
merged 11 commits into from
Jun 13, 2023

Conversation

csasarak
Copy link
Contributor

@csasarak csasarak commented Jun 9, 2023

Overview

When scanning a directory with a package (A) that declares another package (B) to be its parent, if the parent (B) is also detected in the POM closure code we wouldn't report (A) as a target because it is no longer a root package - it's a child POM of (B).

This is confusing to users because it's possible for the top-level maven package in a directory to not be found as a target if its parent is included in a local subdirectory. This was discovered in a project where the .m2 directory was a subdirectory of the scanned directory. If .m2 weren't present, the top-level would be a target (maven@./) but if it were present it would not be.

This code changes the CLI so what we only establish that parent-child relationship if (A) has (B) as a parent and (B) also declares (A) as a submodule. The consequences of this for existing projects are that we may list more targets for analysis than we did before in the case described above.

Acceptance criteria

We should consider a maven package a target even if its parent also exists in the scanned directory unless the parent declares that package as a submodule.

Testing plan

I have attached a minimal example which demonstrates the problem. The example uses .m2 as the directory, but it could be named anything or located anywhere in the project.

You can also create an example by finding a minimal spring boot project, running mvn compile to make sure it has all deps and then copying ~/.m2 into the project root.

Risks

The only risk I can think of is that some customers may see the number of scanned targets increased, though the conditions that this bug appeared in are probably uncommon.

Metrics

None

References

Checklist

  • I added tests for this PR's change (or explained in the PR description why tests don't make sense).
  • If this PR introduced a user-visible change, I added documentation into docs/.
  • If this change is externally visible, I updated Changelog.md. If this PR did not mark a release, I added my changes into an # Unreleased section at the top.
  • If I made changes to .fossa.yml or fossa-deps.{json.yml}, I updated docs/references/files/*.schema.json. You may also need to update these if you have added/removed new dependency type (e.g. pip) or analysis target type (e.g. poetry).

@csasarak csasarak marked this pull request as ready for review June 12, 2023 18:24
@csasarak csasarak requested a review from a team as a code owner June 12, 2023 18:24
@csasarak csasarak requested review from spatten and zlav June 12, 2023 18:24
Copy link
Contributor

@spatten spatten left a comment

Choose a reason for hiding this comment

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

This looks good to me!

@csasarak csasarak enabled auto-merge (squash) June 13, 2023 14:13
@csasarak csasarak merged commit 0cca671 into master Jun 13, 2023
17 checks passed
@csasarak csasarak deleted the fix-maven-pom-parent-child-relationships branch June 13, 2023 21:07
csasarak added a commit that referenced this pull request Jun 14, 2023
This reverts commit 0cca671.
The changelog has only been appended to.
csasarak added a commit that referenced this pull request Jun 14, 2023
This reverts commit 0cca671.
The changelog has only been appended to.
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.

None yet

2 participants