Fix AUR dependency resolving (#2169)

Before this fix dependencies for AUR targets were added to the graph
after each addition of a target node. Now dependencies are added only
after all target nodes are added to the graph.

Also added some tests for previously bugged cases.

pkg/dep/dep_graph.go

@@ -270,6 +270,12 @@ func (g *Grapher) GraphFromSrcInfo(ctx context.Context, graph *topo.Graph[string
 	return graph, nil
 }
 
+func (g *Grapher) AddDepsForPkgs(ctx context.Context, pkgs []*aur.Pkg, graph *topo.Graph[string, *InstallInfo]) {
+	for _, pkg := range pkgs {
+		g.addDepNodes(ctx, pkg, graph)
+	}
+}
+
 func (g *Grapher) addDepNodes(ctx context.Context, pkg *aur.Pkg, graph *topo.Graph[string, *InstallInfo]) {
 	if len(pkg.MakeDepends) > 0 {
 		g.addNodes(ctx, graph, pkg.Name, pkg.MakeDepends, MakeDep)
@@ -316,8 +322,6 @@ func (g *Grapher) GraphAURTarget(ctx context.Context,
 		graph = topo.New[string, *InstallInfo]()
 	}
 
-	exists := graph.Exists(pkg.Name)
-
 	graph.AddNode(pkg.Name)
 
 	for i := range pkg.Provides {
@@ -335,10 +339,6 @@ func (g *Grapher) GraphAURTarget(ctx context.Context,
 		Value:      instalInfo,
 	})
 
-	if !exists {
-		g.addDepNodes(ctx, pkg, graph)
-	}
-
 	return graph
 }
 
@@ -366,6 +366,8 @@ func (g *Grapher) GraphFromAUR(ctx context.Context,
 		}
 	}
 
+	aurPkgsAdded := []*aurc.Pkg{}
+
 	for _, target := range targets {
 		if cachedProvidePkg, ok := g.providerCache[target]; ok {
 			aurPkgs = cachedProvidePkg
@@ -405,8 +407,11 @@ func (g *Grapher) GraphFromAUR(ctx context.Context,
 			Source:  AUR,
 			Version: aurPkg.Version,
 		})
+		aurPkgsAdded = append(aurPkgsAdded, aurPkg)
 	}
 
+	g.AddDepsForPkgs(ctx, aurPkgsAdded, graph)
+
 	return graph, nil
 }
 

pkg/dep/dep_graph_test.go

@@ -322,3 +322,348 @@ func TestGrapher_GraphProvides_androidsdk(t *testing.T) {
 		})
 	}
 }
+
+func TestGrapher_GraphFromAUR_Deps_ceph_bin(t *testing.T) {
+	mockDB := &mock.DBExecutor{
+		SyncPackageFn:       func(string) mock.IPackage { return nil },
+		PackagesFromGroupFn: func(string) []mock.IPackage { return []mock.IPackage{} },
+		SyncSatisfierFn: func(s string) mock.IPackage {
+			switch s {
+			case "ceph-bin", "ceph-libs-bin":
+				return nil
+			case "ceph", "ceph-libs", "ceph-libs=17.2.6-2":
+				return nil
+			}
+
+			panic("implement me " + s)
+		},
+
+		LocalSatisfierExistsFn: func(s string) bool {
+			switch s {
+			case "ceph-libs", "ceph-libs=17.2.6-2":
+				return false
+			case "dep1", "dep2", "dep3", "makedep1", "makedep2", "checkdep1":
+				return true
+			}
+
+			panic("implement me " + s)
+		},
+	}
+
+	mockAUR := &mockaur.MockAUR{GetFn: func(ctx context.Context, query *aurc.Query) ([]aur.Pkg, error) {
+		mockPkgs := map[string]aur.Pkg{
+			"ceph-bin": {
+				Name:        "ceph-bin",
+				PackageBase: "ceph-bin",
+				Version:     "17.2.6-2",
+				Depends:     []string{"ceph-libs=17.2.6-2", "dep1"},
+				Provides:    []string{"ceph=17.2.6-2"},
+			},
+			"ceph-libs-bin": {
+				Name:        "ceph-libs-bin",
+				PackageBase: "ceph-bin",
+				Version:     "17.2.6-2",
+				Depends:     []string{"dep1", "dep2"},
+				Provides:    []string{"ceph-libs=17.2.6-2"},
+			},
+			"ceph": {
+				Name:         "ceph",
+				PackageBase:  "ceph",
+				Version:      "17.2.6-2",
+				Depends:      []string{"ceph-libs=17.2.6-2", "dep1"},
+				MakeDepends:  []string{"makedep1"},
+				CheckDepends: []string{"checkdep1"},
+				Provides:     []string{"ceph=17.2.6-2"},
+			},
+			"ceph-libs": {
+				Name:         "ceph-libs",
+				PackageBase:  "ceph",
+				Version:      "17.2.6-2",
+				Depends:      []string{"dep1", "dep2", "dep3"},
+				MakeDepends:  []string{"makedep1", "makedep2"},
+				CheckDepends: []string{"checkdep1"},
+				Provides:     []string{"ceph-libs=17.2.6-2"},
+			},
+		}
+
+		pkgs := []aur.Pkg{}
+		for _, needle := range query.Needles {
+			if pkg, ok := mockPkgs[needle]; ok {
+				pkgs = append(pkgs, pkg)
+			} else {
+				panic(fmt.Sprintf("implement me %v", needle))
+			}
+		}
+
+		return pkgs, nil
+	}}
+
+	installInfos := map[string]*InstallInfo{
+		"ceph-bin exp": {
+			Source:  AUR,
+			Reason:  Explicit,
+			Version: "17.2.6-2",
+			AURBase: ptrString("ceph-bin"),
+		},
+		"ceph-libs-bin exp": {
+			Source:  AUR,
+			Reason:  Explicit,
+			Version: "17.2.6-2",
+			AURBase: ptrString("ceph-bin"),
+		},
+		"ceph exp": {
+			Source:  AUR,
+			Reason:  Explicit,
+			Version: "17.2.6-2",
+			AURBase: ptrString("ceph"),
+		},
+		"ceph-libs exp": {
+			Source:  AUR,
+			Reason:  Explicit,
+			Version: "17.2.6-2",
+			AURBase: ptrString("ceph"),
+		},
+		"ceph-libs dep": {
+			Source:  AUR,
+			Reason:  Dep,
+			Version: "17.2.6-2",
+			AURBase: ptrString("ceph"),
+		},
+	}
+
+	tests := []struct {
+		name       string
+		targets    []string
+		wantLayers []map[string]*InstallInfo
+		wantErr    bool
+	}{
+		{
+			name:    "ceph-bin ceph-libs-bin",
+			targets: []string{"ceph-bin", "ceph-libs-bin"},
+			wantLayers: []map[string]*InstallInfo{
+				{"ceph-bin": installInfos["ceph-bin exp"]},
+				{"ceph-libs-bin": installInfos["ceph-libs-bin exp"]},
+			},
+			wantErr: false,
+		},
+		{
+			name:    "ceph-libs-bin ceph-bin (reversed order)",
+			targets: []string{"ceph-libs-bin", "ceph-bin"},
+			wantLayers: []map[string]*InstallInfo{
+				{"ceph-bin": installInfos["ceph-bin exp"]},
+				{"ceph-libs-bin": installInfos["ceph-libs-bin exp"]},
+			},
+			wantErr: false,
+		},
+		{
+			name:    "ceph",
+			targets: []string{"ceph"},
+			wantLayers: []map[string]*InstallInfo{
+				{"ceph": installInfos["ceph exp"]},
+				{"ceph-libs": installInfos["ceph-libs dep"]},
+			},
+			wantErr: false,
+		},
+		{
+			name:    "ceph-bin",
+			targets: []string{"ceph-bin"},
+			wantLayers: []map[string]*InstallInfo{
+				{"ceph-bin": installInfos["ceph-bin exp"]},
+				{"ceph-libs": installInfos["ceph-libs dep"]},
+			},
+			wantErr: false,
+		},
+		{
+			name:    "ceph-bin ceph-libs",
+			targets: []string{"ceph-bin", "ceph-libs"},
+			wantLayers: []map[string]*InstallInfo{
+				{"ceph-bin": installInfos["ceph-bin exp"]},
+				{"ceph-libs": installInfos["ceph-libs exp"]},
+			},
+			wantErr: false,
+		},
+		{
+			name:    "ceph-libs ceph-bin (reversed order)",
+			targets: []string{"ceph-libs", "ceph-bin"},
+			wantLayers: []map[string]*InstallInfo{
+				{"ceph-bin": installInfos["ceph-bin exp"]},
+				{"ceph-libs": installInfos["ceph-libs exp"]},
+			},
+			wantErr: false,
+		},
+		{
+			name:    "ceph ceph-libs-bin",
+			targets: []string{"ceph", "ceph-libs-bin"},
+			wantLayers: []map[string]*InstallInfo{
+				{"ceph": installInfos["ceph exp"]},
+				{"ceph-libs-bin": installInfos["ceph-libs-bin exp"]},
+			},
+			wantErr: false,
+		},
+		{
+			name:    "ceph-libs-bin ceph (reversed order)",
+			targets: []string{"ceph-libs-bin", "ceph"},
+			wantLayers: []map[string]*InstallInfo{
+				{"ceph": installInfos["ceph exp"]},
+				{"ceph-libs-bin": installInfos["ceph-libs-bin exp"]},
+			},
+			wantErr: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			g := NewGrapher(mockDB, mockAUR,
+				false, true, false, false, false,
+				text.NewLogger(io.Discard, io.Discard, &os.File{}, true, "test"))
+			got, err := g.GraphFromTargets(context.Background(), nil, tt.targets)
+			require.NoError(t, err)
+			layers := got.TopoSortedLayerMap(nil)
+			require.EqualValues(t, tt.wantLayers, layers, layers)
+		})
+	}
+}
+
+func TestGrapher_GraphFromAUR_Deps_gourou(t *testing.T) {
+	mockDB := &mock.DBExecutor{
+		SyncPackageFn:       func(string) mock.IPackage { return nil },
+		PackagesFromGroupFn: func(string) []mock.IPackage { return []mock.IPackage{} },
+		SyncSatisfierFn: func(s string) mock.IPackage {
+			switch s {
+			case "gourou", "libzip-git":
+				return nil
+			case "libzip":
+				return &mock.Package{
+					PName:    "libzip",
+					PVersion: "1.9.2-1",
+					PDB:      mock.NewDB("extra"),
+				}
+			}
+
+			panic("implement me " + s)
+		},
+
+		LocalSatisfierExistsFn: func(s string) bool {
+			switch s {
+			case "gourou", "libzip", "libzip-git":
+				return false
+			case "dep1", "dep2":
+				return true
+			}
+
+			panic("implement me " + s)
+		},
+	}
+
+	mockAUR := &mockaur.MockAUR{GetFn: func(ctx context.Context, query *aurc.Query) ([]aur.Pkg, error) {
+		mockPkgs := map[string]aur.Pkg{
+			"gourou": {
+				Name:        "gourou",
+				PackageBase: "gourou",
+				Version:     "0.8.1",
+				Depends:     []string{"libzip"},
+			},
+			"libzip-git": {
+				Name:        "libzip-git",
+				PackageBase: "libzip-git",
+				Version:     "1.9.2.r159.gb3ac716c-1",
+				Depends:     []string{"dep1", "dep2"},
+				Provides:    []string{"libzip=1.9.2.r159.gb3ac716c"},
+			},
+		}
+
+		pkgs := []aur.Pkg{}
+		for _, needle := range query.Needles {
+			if pkg, ok := mockPkgs[needle]; ok {
+				pkgs = append(pkgs, pkg)
+			} else {
+				panic(fmt.Sprintf("implement me %v", needle))
+			}
+		}
+
+		return pkgs, nil
+	}}
+
+	installInfos := map[string]*InstallInfo{
+		"gourou exp": {
+			Source:  AUR,
+			Reason:  Explicit,
+			Version: "0.8.1",
+			AURBase: ptrString("gourou"),
+		},
+		"libzip dep": {
+			Source:     Sync,
+			Reason:     Dep,
+			Version:    "1.9.2-1",
+			SyncDBName: ptrString("extra"),
+		},
+		"libzip exp": {
+			Source:     Sync,
+			Reason:     Explicit,
+			Version:    "1.9.2-1",
+			SyncDBName: ptrString("extra"),
+		},
+		"libzip-git exp": {
+			Source:  AUR,
+			Reason:  Explicit,
+			Version: "1.9.2.r159.gb3ac716c-1",
+			AURBase: ptrString("libzip-git"),
+		},
+	}
+
+	tests := []struct {
+		name       string
+		targets    []string
+		wantLayers []map[string]*InstallInfo
+		wantErr    bool
+	}{
+		{
+			name:    "gourou",
+			targets: []string{"gourou"},
+			wantLayers: []map[string]*InstallInfo{
+				{"gourou": installInfos["gourou exp"]},
+				{"libzip": installInfos["libzip dep"]},
+			},
+			wantErr: false,
+		},
+		{
+			name:    "gourou libzip",
+			targets: []string{"gourou", "libzip"},
+			wantLayers: []map[string]*InstallInfo{
+				{"gourou": installInfos["gourou exp"]},
+				{"libzip": installInfos["libzip exp"]},
+			},
+			wantErr: false,
+		},
+		{
+			name:    "gourou libzip-git",
+			targets: []string{"gourou", "libzip-git"},
+			wantLayers: []map[string]*InstallInfo{
+				{"gourou": installInfos["gourou exp"]},
+				{"libzip-git": installInfos["libzip-git exp"]},
+			},
+			wantErr: false,
+		},
+		{
+			name:    "libzip-git gourou (reversed order)",
+			targets: []string{"libzip-git", "gourou"},
+			wantLayers: []map[string]*InstallInfo{
+				{"gourou": installInfos["gourou exp"]},
+				{"libzip-git": installInfos["libzip-git exp"]},
+			},
+			wantErr: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			g := NewGrapher(mockDB, mockAUR,
+				false, true, false, false, false,
+				text.NewLogger(io.Discard, io.Discard, &os.File{}, true, "test"))
+			got, err := g.GraphFromTargets(context.Background(), nil, tt.targets)
+			require.NoError(t, err)
+			layers := got.TopoSortedLayerMap(nil)
+			require.EqualValues(t, tt.wantLayers, layers, layers)
+		})
+	}
+}

pkg/upgrade/service.go

@@ -96,6 +96,8 @@ func (u *UpgradeService) upGraph(ctx context.Context, graph *topo.Graph[string,
 		}
 	}
 
+	aurPkgsAdded := []*aur.Pkg{}
+
 	names := mapset.NewThreadUnsafeSet[string]()
 	for i := range develUp.Up {
 		up := &develUp.Up[i]
@@ -120,6 +122,7 @@ func (u *UpgradeService) upGraph(ctx context.Context, graph *topo.Graph[string,
 			Version:      up.RemoteVersion,
 		})
 		names.Add(up.Name)
+		aurPkgsAdded = append(aurPkgsAdded, aurPkg)
 	}
 
 	for i := range aurUp.Up {
@@ -148,8 +151,11 @@ func (u *UpgradeService) upGraph(ctx context.Context, graph *topo.Graph[string,
 			Version:      up.RemoteVersion,
 			LocalVersion: up.LocalVersion,
 		})
+		aurPkgsAdded = append(aurPkgsAdded, aurPkg)
 	}
 
+	u.grapher.AddDepsForPkgs(ctx, aurPkgsAdded, graph)
+
 	if u.cfg.Mode.AtLeastRepo() {
 		u.log.OperationInfoln(gotext.Get("Searching databases for updates..."))