Skip to content

Commit

Permalink
Set the original requirement in patches from suggest (#1117)
Browse files Browse the repository at this point in the history
Previously, `origRequire` is not set in patches from suggest, however we
plan to use this field to differentiate patches from override, this PR
sets `origRequire` for patches from suggest so that `manifest.Write()`
is able to only add dependencies from patches generated by override.
  • Loading branch information
cuixq committed Jul 15, 2024
1 parent df6de20 commit 2600b92
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 23 deletions.
7 changes: 4 additions & 3 deletions internal/remediation/suggest/maven.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,10 @@ func (ms *MavenSuggester) Suggest(ctx context.Context, client resolve.Client, mf
}

changedDeps = append(changedDeps, manifest.DependencyPatch{
Pkg: req.PackageKey,
Type: req.Type,
NewRequire: latest.Version,
Pkg: req.PackageKey,
Type: req.Type,
OrigRequire: req.Version,
NewRequire: latest.Version,
})
}

Expand Down
40 changes: 25 additions & 15 deletions internal/remediation/suggest/maven_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -272,75 +272,85 @@ func TestSuggest(t *testing.T) {
System: resolve.Maven,
Name: "org.dep:plugin-dep",
},
Type: depPlugin,
NewRequire: "2.3.4",
Type: depPlugin,
OrigRequire: "2.3.3",
NewRequire: "2.3.4",
},
{
Pkg: resolve.PackageKey{
System: resolve.Maven,
Name: "org.example:abc",
},
NewRequire: "1.0.2",
OrigRequire: "1.0.1",
NewRequire: "1.0.2",
},
{
Pkg: resolve.PackageKey{
System: resolve.Maven,
Name: "org.example:another-property",
},
NewRequire: "1.1.0",
OrigRequire: "1.0.0",
NewRequire: "1.1.0",
},
{
Pkg: resolve.PackageKey{
System: resolve.Maven,
Name: "org.example:property",
},
NewRequire: "1.0.1",
OrigRequire: "1.0.0",
NewRequire: "1.0.1",
},
{
Pkg: resolve.PackageKey{
System: resolve.Maven,
Name: "org.example:property-no-update",
},
NewRequire: "2.0.0",
OrigRequire: "1.9",
NewRequire: "2.0.0",
},
{
Pkg: resolve.PackageKey{
System: resolve.Maven,
Name: "org.example:same-property",
},
NewRequire: "1.0.1",
OrigRequire: "1.0.0",
NewRequire: "1.0.1",
},
{
Pkg: resolve.PackageKey{
System: resolve.Maven,
Name: "org.example:xyz",
},
Type: depMgmt,
NewRequire: "2.0.1",
Type: depMgmt,
OrigRequire: "2.0.0",
NewRequire: "2.0.1",
},
{
Pkg: resolve.PackageKey{
System: resolve.Maven,
Name: "org.import:xyz",
},
Type: depProfileTwoMgmt,
NewRequire: "6.7.0",
Type: depProfileTwoMgmt,
OrigRequire: "6.6.6",
NewRequire: "6.7.0",
},
{
Pkg: resolve.PackageKey{
System: resolve.Maven,
Name: "org.profile:abc",
},
Type: depProfileOne,
NewRequire: "1.2.4",
Type: depProfileOne,
OrigRequire: "1.2.3",
NewRequire: "1.2.4",
},
{
Pkg: resolve.PackageKey{
System: resolve.Maven,
Name: "org.profile:def",
},
Type: depProfileOne,
NewRequire: "2.3.5",
Type: depProfileOne,
OrigRequire: "2.3.4",
NewRequire: "2.3.5",
},
},
EcosystemSpecific: manifest.MavenManifestSpecific{
Expand Down
14 changes: 9 additions & 5 deletions internal/resolution/manifest/maven.go
Original file line number Diff line number Diff line change
Expand Up @@ -403,12 +403,16 @@ func buildPatches(patches []DependencyPatch, specific MavenManifestSpecific) (Ma
propertyPatches := MavenPropertyPatches{}
for _, patch := range patches {
origDep := originalDependency(patch, specific.OriginalRequirements)
if origDep.Name() == "" {
if origDep.Name() == ":" {
// An empty name indicates the dependency is not found, so the original dependency is not in the base project.
// TODO: enable for guided remeidation
// if err := depPatches.addPatch(patch, false); err != nil {
// return MavenDependencyPatches{}, MavenPropertyPatches{}, err
// }
// If the patch is from suggest (origRequire is set), we ignore this patch.
// If the patch if from override (origRequire is empty), we add this patch.
if patch.OrigRequire == "" {
if err := depPatches.addPatch(patch, false); err != nil {
return MavenDependencyPatches{}, MavenPropertyPatches{}, err
}
}

continue
}

Expand Down
25 changes: 25 additions & 0 deletions internal/resolution/manifest/maven_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -606,6 +606,23 @@ func TestBuildPatches(t *testing.T) {
Type: depParent,
NewRequire: "1.2.0",
},
{
Pkg: resolve.PackageKey{
System: resolve.Maven,
Name: "org.example:suggest",
},
Type: depMgmt,
OrigRequire: "1.0.0",
NewRequire: "2.0.0",
},
{
Pkg: resolve.PackageKey{
System: resolve.Maven,
Name: "org.example:override",
},
Type: depMgmt,
NewRequire: "2.0.0",
},
}
specific := MavenManifestSpecific{
Properties: []PropertyWithOrigin{
Expand Down Expand Up @@ -697,6 +714,14 @@ func TestBuildPatches(t *testing.T) {
},
NewRequire: "2.0.1",
}: true,
{
DependencyKey: maven.DependencyKey{
GroupID: "org.example",
ArtifactID: "override",
Type: "jar",
},
NewRequire: "2.0.0",
}: false,
},
"profile@profile-one": map[MavenPatch]bool{
{
Expand Down

0 comments on commit 2600b92

Please sign in to comment.