diff --git a/pkg/installer/installer.go b/pkg/installer/installer.go index f7f25b9f..b465b78d 100644 --- a/pkg/installer/installer.go +++ b/pkg/installer/installer.go @@ -700,7 +700,7 @@ func (l *LuetInstaller) Uninstall(p pkg.Package, s *System) error { return errors.Wrap(err, "Could not solve the uninstall constraints. Tip: try with --solver-type qlearning or with --force, or by removing packages excluding their dependencies with --nodeps") } } else { - solution, err = solv.Uninstall(p, checkConflicts, full) + solution, err = solv.Uninstall(checkConflicts, full, p) if err != nil && !l.Options.Force { return errors.Wrap(err, "Could not solve the uninstall constraints. Tip: try with --solver-type qlearning or with --force, or by removing packages excluding their dependencies with --nodeps") } diff --git a/pkg/solver/parallel.go b/pkg/solver/parallel.go index f62aebf4..6e566932 100644 --- a/pkg/solver/parallel.go +++ b/pkg/solver/parallel.go @@ -615,23 +615,22 @@ func (s *Parallel) Upgrade(checkconflicts, full bool) (pkg.Packages, PackagesAss } // Then try to uninstall the versions in the system, and store that tree - for _, p := range toUninstall { - r, err := s.Uninstall(p, checkconflicts, false) + r, err := s.Uninstall(checkconflicts, false, toUninstall...) + if err != nil { + return nil, nil, errors.Wrap(err, "Could not compute upgrade - couldn't uninstall candidates ") + } + for _, z := range r { + err = installedcopy.RemovePackage(z) if err != nil { - return nil, nil, errors.Wrap(err, "Could not compute upgrade - couldn't uninstall selected candidate "+p.GetFingerPrint()) - } - for _, z := range r { - err = installedcopy.RemovePackage(z) - if err != nil { - return nil, nil, errors.Wrap(err, "Could not compute upgrade - couldn't remove copy of package targetted for removal") - } + return nil, nil, errors.Wrap(err, "Could not compute upgrade - couldn't remove copy of package targetted for removal") } } + if len(toInstall) == 0 { return toUninstall, PackagesAssertions{}, nil } - r, e := s2.Install(toInstall) - return toUninstall, r, e + assertions, e := s2.Install(toInstall) + return toUninstall, assertions, e // To that tree, ask to install the versions that should be upgraded, and try to solve // Return the solution @@ -639,21 +638,30 @@ func (s *Parallel) Upgrade(checkconflicts, full bool) (pkg.Packages, PackagesAss // Uninstall takes a candidate package and return a list of packages that would be removed // in order to purge the candidate. Returns error if unsat. -func (s *Parallel) Uninstall(c pkg.Package, checkconflicts, full bool) (pkg.Packages, error) { +func (s *Parallel) Uninstall(checkconflicts, full bool, packs ...pkg.Package) (pkg.Packages, error) { + if len(packs) == 0 { + return pkg.Packages{}, nil + } var res pkg.Packages - candidate, err := s.InstalledDatabase.FindPackage(c) - if err != nil { + toRemove := pkg.Packages{} - // return nil, errors.Wrap(err, "Couldn't find required package in db definition") - packages, err := c.Expand(s.InstalledDatabase) - // Info("Expanded", packages, err) - if err != nil || len(packages) == 0 { - candidate = c - } else { - candidate = packages.Best(nil) + for _, c := range packs { + candidate, err := s.InstalledDatabase.FindPackage(c) + if err != nil { + + // return nil, errors.Wrap(err, "Couldn't find required package in db definition") + packages, err := c.Expand(s.InstalledDatabase) + // Info("Expanded", packages, err) + if err != nil || len(packages) == 0 { + candidate = c + } else { + candidate = packages.Best(nil) + } + //Relax search, otherwise we cannot compute solutions for packages not in definitions + // return nil, errors.Wrap(err, "Package not found between installed") } - //Relax search, otherwise we cannot compute solutions for packages not in definitions - // return nil, errors.Wrap(err, "Package not found between installed") + + toRemove = append(toRemove, candidate) } // Build a fake "Installed" - Candidate and its requires tree var InstalledMinusCandidate pkg.Packages @@ -661,30 +669,38 @@ func (s *Parallel) Uninstall(c pkg.Package, checkconflicts, full bool) (pkg.Pack // We are asked to not perform a full uninstall (checking all the possible requires that could // be removed). Let's only check if we can remove the selected package if !full && checkconflicts { - if conflicts, err := s.Conflicts(candidate, s.Installed()); conflicts { - return nil, err - } else { - return pkg.Packages{candidate}, nil + for _, candidate := range toRemove { + if conflicts, err := s.Conflicts(candidate, s.Installed()); conflicts { + return nil, err + } } + return toRemove, nil } // TODO: Can be optimized for _, i := range s.Installed() { - if !i.Matches(candidate) { - contains, err := candidate.RequiresContains(s.ParallelDatabase, i) - if err != nil { - return nil, errors.Wrap(err, "Failed getting installed list") - } - if !contains { - InstalledMinusCandidate = append(InstalledMinusCandidate, i) + matched := false + for _, candidate := range toRemove { + if !i.Matches(candidate) { + contains, err := candidate.RequiresContains(s.ParallelDatabase, i) + if err != nil { + return nil, errors.Wrap(err, "Failed getting installed list") + } + if !contains { + matched = true + } + } } + if matched { + InstalledMinusCandidate = append(InstalledMinusCandidate, i) + } } s2 := &Parallel{Concurrency: s.Concurrency, InstalledDatabase: pkg.NewInMemoryDatabase(false), DefinitionDatabase: s.DefinitionDatabase, ParallelDatabase: pkg.NewInMemoryDatabase(false)} s2.SetResolver(s.Resolver) // Get the requirements to install the candidate - asserts, err := s2.Install(pkg.Packages{candidate}) + asserts, err := s2.Install(toRemove) if err != nil { return nil, err } diff --git a/pkg/solver/parallel_test.go b/pkg/solver/parallel_test.go index 5d4cf0d2..f157e822 100644 --- a/pkg/solver/parallel_test.go +++ b/pkg/solver/parallel_test.go @@ -593,7 +593,7 @@ var _ = Describe("Parallel", func() { } s = &Parallel{InstalledDatabase: dbInstalled, Concurrency: 4, DefinitionDatabase: dbDefinitions, ParallelDatabase: db} - solution, err := s.Uninstall(A, true, true) + solution, err := s.Uninstall(true, true, A) Expect(err).ToNot(HaveOccurred()) Expect(solution).To(ContainElement(A)) @@ -619,7 +619,7 @@ var _ = Describe("Parallel", func() { } s = &Parallel{InstalledDatabase: dbInstalled, Concurrency: 4, DefinitionDatabase: dbDefinitions, ParallelDatabase: db} - solution, err := s.Uninstall(&pkg.DefaultPackage{Name: "A", Version: ">1.0"}, true, true) + solution, err := s.Uninstall(true, true, &pkg.DefaultPackage{Name: "A", Version: ">1.0"}) Expect(err).ToNot(HaveOccurred()) Expect(solution).To(ContainElement(A)) @@ -643,7 +643,7 @@ var _ = Describe("Parallel", func() { _, err := dbInstalled.CreatePackage(p) Expect(err).ToNot(HaveOccurred()) } - solution, err := s.Uninstall(A, true, true) + solution, err := s.Uninstall(true, true, A) Expect(err).ToNot(HaveOccurred()) Expect(solution).To(ContainElement(A)) @@ -667,7 +667,7 @@ var _ = Describe("Parallel", func() { _, err := dbInstalled.CreatePackage(p) Expect(err).ToNot(HaveOccurred()) } - solution, err := s.Uninstall(A, true, true) + solution, err := s.Uninstall(true, true, A) Expect(err).ToNot(HaveOccurred()) Expect(solution).To(ContainElement(A)) @@ -690,7 +690,7 @@ var _ = Describe("Parallel", func() { _, err := dbInstalled.CreatePackage(p) Expect(err).ToNot(HaveOccurred()) } - solution, err := s.Uninstall(A, true, true) + solution, err := s.Uninstall(true, true, A) Expect(err).ToNot(HaveOccurred()) Expect(solution).To(ContainElement(A)) @@ -699,6 +699,31 @@ var _ = Describe("Parallel", func() { Expect(len(solution)).To(Equal(1)) }) + It("Uninstalls multiple complex packages correctly, even if shared deps are required by system packages", func() { + D := pkg.NewPackage("D", "", []*pkg.DefaultPackage{}, []*pkg.DefaultPackage{}) + B := pkg.NewPackage("B", "", []*pkg.DefaultPackage{}, []*pkg.DefaultPackage{}) + A := pkg.NewPackage("A", "", []*pkg.DefaultPackage{B}, []*pkg.DefaultPackage{}) + C := pkg.NewPackage("C", "", []*pkg.DefaultPackage{B}, []*pkg.DefaultPackage{}) + + for _, p := range []pkg.Package{A, B, C, D} { + _, err := dbDefinitions.CreatePackage(p) + Expect(err).ToNot(HaveOccurred()) + } + + for _, p := range []pkg.Package{A, B, C, D} { + _, err := dbInstalled.CreatePackage(p) + Expect(err).ToNot(HaveOccurred()) + } + solution, err := s.Uninstall(true, true, A, C) + Expect(err).ToNot(HaveOccurred()) + Expect(solution).To(ContainElement(C)) + + Expect(solution).To(ContainElement(A)) + Expect(solution).ToNot(ContainElement(B)) + + Expect(len(solution)).To(Equal(2)) + }) + It("Uninstalls complex packages in world correctly", func() { C := pkg.NewPackage("C", "", []*pkg.DefaultPackage{}, []*pkg.DefaultPackage{}) D := pkg.NewPackage("D", "", []*pkg.DefaultPackage{}, []*pkg.DefaultPackage{}) @@ -715,7 +740,7 @@ var _ = Describe("Parallel", func() { Expect(err).ToNot(HaveOccurred()) } - solution, err := s.Uninstall(A, true, true) + solution, err := s.Uninstall(true, true, A) Expect(err).ToNot(HaveOccurred()) Expect(solution).To(ContainElement(A)) @@ -741,7 +766,7 @@ var _ = Describe("Parallel", func() { Expect(err).ToNot(HaveOccurred()) } - solution, err := s.Uninstall(A, true, true) + solution, err := s.Uninstall(true, true, A) Expect(err).ToNot(HaveOccurred()) Expect(solution).To(ContainElement(A)) @@ -1070,7 +1095,7 @@ var _ = Describe("Parallel", func() { } val, err := s.Conflicts(D, dbInstalled.World()) - Expect(err.Error()).To(Equal("\n/A-\n/B-")) + Expect(err.Error()).To(Or(Equal("\n/A-\n/B-"), Equal("\n/B-\n/A-"))) Expect(val).To(BeTrue()) }) diff --git a/pkg/solver/solver.go b/pkg/solver/solver.go index f9bd6ad1..fc4479b0 100644 --- a/pkg/solver/solver.go +++ b/pkg/solver/solver.go @@ -37,7 +37,7 @@ const ( type PackageSolver interface { SetDefinitionDatabase(pkg.PackageDatabase) Install(p pkg.Packages) (PackagesAssertions, error) - Uninstall(candidate pkg.Package, checkconflicts, full bool) (pkg.Packages, error) + Uninstall(checkconflicts, full bool, candidate ...pkg.Package) (pkg.Packages, error) ConflictsWithInstalled(p pkg.Package) (bool, error) ConflictsWith(p pkg.Package, ls pkg.Packages) (bool, error) Conflicts(pack pkg.Package, lsp pkg.Packages) (bool, error) @@ -518,23 +518,22 @@ func (s *Solver) Upgrade(checkconflicts, full bool) (pkg.Packages, PackagesAsser } } // Then try to uninstall the versions in the system, and store that tree - for _, p := range toUninstall { - r, err := s.Uninstall(p, checkconflicts, false) + r, err := s.Uninstall(checkconflicts, false, toUninstall...) + if err != nil { + return nil, nil, errors.Wrap(err, "Could not compute upgrade - couldn't uninstall candidates ") + } + for _, z := range r { + err = installedcopy.RemovePackage(z) if err != nil { - return nil, nil, errors.Wrap(err, "Could not compute upgrade - couldn't uninstall selected candidate "+p.GetFingerPrint()) - } - for _, z := range r { - err = installedcopy.RemovePackage(z) - if err != nil { - return nil, nil, errors.Wrap(err, "Could not compute upgrade - couldn't remove copy of package targetted for removal") - } + return nil, nil, errors.Wrap(err, "Could not compute upgrade - couldn't remove copy of package targetted for removal") } } + if len(toInstall) == 0 { return toUninstall, PackagesAssertions{}, nil } - r, e := s2.Install(toInstall) - return toUninstall, r, e + assertions, err := s2.Install(toInstall) + return toUninstall, assertions, err // To that tree, ask to install the versions that should be upgraded, and try to solve // Return the solution @@ -542,52 +541,71 @@ func (s *Solver) Upgrade(checkconflicts, full bool) (pkg.Packages, PackagesAsser // Uninstall takes a candidate package and return a list of packages that would be removed // in order to purge the candidate. Returns error if unsat. -func (s *Solver) Uninstall(c pkg.Package, checkconflicts, full bool) (pkg.Packages, error) { - var res pkg.Packages - candidate, err := s.InstalledDatabase.FindPackage(c) - if err != nil { - - // return nil, errors.Wrap(err, "Couldn't find required package in db definition") - packages, err := c.Expand(s.InstalledDatabase) - // Info("Expanded", packages, err) - if err != nil || len(packages) == 0 { - candidate = c - } else { - candidate = packages.Best(nil) - } - //Relax search, otherwise we cannot compute solutions for packages not in definitions - // return nil, errors.Wrap(err, "Package not found between installed") +func (s *Solver) Uninstall(checkconflicts, full bool, packs ...pkg.Package) (pkg.Packages, error) { + if len(packs) == 0 { + return pkg.Packages{}, nil } + var res pkg.Packages + + toRemove := pkg.Packages{} + + for _, c := range packs { + candidate, err := s.InstalledDatabase.FindPackage(c) + if err != nil { + + // return nil, errors.Wrap(err, "Couldn't find required package in db definition") + packages, err := c.Expand(s.InstalledDatabase) + // Info("Expanded", packages, err) + if err != nil || len(packages) == 0 { + candidate = c + } else { + candidate = packages.Best(nil) + } + //Relax search, otherwise we cannot compute solutions for packages not in definitions + // return nil, errors.Wrap(err, "Package not found between installed") + } + + toRemove = append(toRemove, candidate) + } + // Build a fake "Installed" - Candidate and its requires tree var InstalledMinusCandidate pkg.Packages // We are asked to not perform a full uninstall (checking all the possible requires that could // be removed). Let's only check if we can remove the selected package if !full && checkconflicts { - if conflicts, err := s.Conflicts(candidate, s.Installed()); conflicts { - return nil, err - } else { - return pkg.Packages{candidate}, nil + for _, candidate := range toRemove { + if conflicts, err := s.Conflicts(candidate, s.Installed()); conflicts { + return nil, err + } } + return toRemove, nil } // TODO: Can be optimized for _, i := range s.Installed() { - if !i.Matches(candidate) { - contains, err := candidate.RequiresContains(s.SolverDatabase, i) - if err != nil { - return nil, errors.Wrap(err, "Failed getting installed list") - } - if !contains { - InstalledMinusCandidate = append(InstalledMinusCandidate, i) + matched := false + for _, candidate := range toRemove { + if !i.Matches(candidate) { + contains, err := candidate.RequiresContains(s.SolverDatabase, i) + if err != nil { + return nil, errors.Wrap(err, "Failed getting installed list") + } + if !contains { + matched = true + } + } } + if matched { + InstalledMinusCandidate = append(InstalledMinusCandidate, i) + } } s2 := NewSolver(Options{Type: SingleCoreSimple}, pkg.NewInMemoryDatabase(false), s.DefinitionDatabase, pkg.NewInMemoryDatabase(false)) s2.SetResolver(s.Resolver) // Get the requirements to install the candidate - asserts, err := s2.Install(pkg.Packages{candidate}) + asserts, err := s2.Install(toRemove) if err != nil { return nil, err } diff --git a/pkg/solver/solver_test.go b/pkg/solver/solver_test.go index 3eeee792..97442be9 100644 --- a/pkg/solver/solver_test.go +++ b/pkg/solver/solver_test.go @@ -593,7 +593,7 @@ var _ = Describe("Solver", func() { } s = NewSolver(Options{Type: SingleCoreSimple}, dbInstalled, dbDefinitions, db) - solution, err := s.Uninstall(A, true, true) + solution, err := s.Uninstall(true, true, A) Expect(err).ToNot(HaveOccurred()) Expect(solution).To(ContainElement(A)) @@ -619,7 +619,7 @@ var _ = Describe("Solver", func() { } s = NewSolver(Options{Type: SingleCoreSimple}, dbInstalled, dbDefinitions, db) - solution, err := s.Uninstall(&pkg.DefaultPackage{Name: "A", Version: ">1.0"}, true, true) + solution, err := s.Uninstall(true, true, &pkg.DefaultPackage{Name: "A", Version: ">1.0"}) Expect(err).ToNot(HaveOccurred()) Expect(solution).To(ContainElement(A)) @@ -643,7 +643,7 @@ var _ = Describe("Solver", func() { _, err := dbInstalled.CreatePackage(p) Expect(err).ToNot(HaveOccurred()) } - solution, err := s.Uninstall(A, true, true) + solution, err := s.Uninstall(true, true, A) Expect(err).ToNot(HaveOccurred()) Expect(solution).To(ContainElement(A)) @@ -667,7 +667,7 @@ var _ = Describe("Solver", func() { _, err := dbInstalled.CreatePackage(p) Expect(err).ToNot(HaveOccurred()) } - solution, err := s.Uninstall(A, true, true) + solution, err := s.Uninstall(true, true, A) Expect(err).ToNot(HaveOccurred()) Expect(solution).To(ContainElement(A)) @@ -690,7 +690,7 @@ var _ = Describe("Solver", func() { _, err := dbInstalled.CreatePackage(p) Expect(err).ToNot(HaveOccurred()) } - solution, err := s.Uninstall(A, true, true) + solution, err := s.Uninstall(true, true, A) Expect(err).ToNot(HaveOccurred()) Expect(solution).To(ContainElement(A)) @@ -715,7 +715,7 @@ var _ = Describe("Solver", func() { Expect(err).ToNot(HaveOccurred()) } - solution, err := s.Uninstall(A, true, true) + solution, err := s.Uninstall(true, true, A) Expect(err).ToNot(HaveOccurred()) Expect(solution).To(ContainElement(A)) @@ -741,7 +741,7 @@ var _ = Describe("Solver", func() { Expect(err).ToNot(HaveOccurred()) } - solution, err := s.Uninstall(A, true, true) + solution, err := s.Uninstall(true, true, A) Expect(err).ToNot(HaveOccurred()) Expect(solution).To(ContainElement(A)) @@ -1070,7 +1070,7 @@ var _ = Describe("Solver", func() { } val, err := s.Conflicts(D, dbInstalled.World()) - Expect(err.Error()).To(Equal("\n/A-\n/B-")) + Expect(err.Error()).To(Or(Equal("\n/A-\n/B-"), Equal("\n/B-\n/A-"))) Expect(val).To(BeTrue()) })