Enforce solver constraints

- Don't sign installed packages during finalizer execution
- Enforce solver constraints: build ALO and AMO rules taking into account
  that the current package might not be selected at all.
- Force uninstalls on upgrade
- Enable option to tell uninstall to ignore conflict with the analized system state,
  as we don't want any conflict with the installed to raise during the upgrade.
  In this way we both force uninstalls and we avoid to check with conflicts
  against the current system state which is pending to deletion.
  This is due to the fact that now the solver enforces the constraints
  and explictly denies two packages of the same version installed.
- Adapt test as now we generate more constraints, which makes the solver more
  noisy on the package that are explictly selected or not

pkg/installer/installer.go

@@ -106,23 +106,39 @@ func (l *LuetInstaller) Upgrade(s *System) error {
 		return err
 	}
 	// First match packages against repositories by priority
-	//	matches := syncedRepos.PackageMatches(p)
 	allRepos := pkg.NewInMemoryDatabase(false)
 	syncedRepos.SyncDatabase(allRepos)
 	// compute a "big" world
 	solv := solver.NewResolver(s.Database, allRepos, pkg.NewInMemoryDatabase(false), l.Options.SolverOptions.Resolver())
-	uninstall, solution, err := solv.Upgrade()
+	uninstall, solution, err := solv.Upgrade(false)
 	if err != nil {
 		return errors.Wrap(err, "Failed solving solution for upgrade")
 	}
 
+	// We don't want any conflict with the installed to raise during the upgrade.
+	// In this way we both force uninstalls and we avoid to check with conflicts
+	// against the current system state which is pending to deletion
+	// E.g. you can't check for conflicts for an upgrade of a new version of A
+	// if the old A results installed in the system. This is due to the fact that
+	// now the solver enforces the constraints and explictly denies two packages
+	// of the same version installed.
+	forced := false
+	if l.Options.Force {
+		forced = true
+	}
+	l.Options.Force = true
+
 	for _, u := range uninstall {
+		Info(":package: Marked for deletion", u.HumanReadableString())
+
 		err := l.Uninstall(u, s)
 		if err != nil && !l.Options.Force {
 			Error("Failed uninstall for ", u.HumanReadableString())
 			return errors.Wrap(err, "uninstalling "+u.HumanReadableString())
 		}
+
 	}
+	l.Options.Force = forced
 
 	toInstall := []pkg.Package{}
 	for _, assertion := range solution {
@@ -249,29 +265,27 @@ func (l *LuetInstaller) Install(cp []pkg.Package, s *System) error {
 	close(all)
 	wg.Wait()
 
+	for _, c := range toInstall {
+		// Annotate to the system that the package was installed
+		_, err := s.Database.CreatePackage(c.Package)
+		if err != nil && !l.Options.Force {
+			return errors.Wrap(err, "Failed creating package")
+		}
+	}
 	executedFinalizer := map[string]bool{}
 
 	// TODO: Lower those errors as warning
 	for _, w := range p {
 		// Finalizers needs to run in order and in sequence.
 		ordered := solution.Order(allRepos, w.GetFingerPrint())
+	ORDER:
 		for _, ass := range ordered {
 			if ass.Value {
-				// Annotate to the system that the package was installed
+
-				if _, err := s.Database.FindPackage(ass.Package); err == nil {
-					err := s.Database.UpdatePackage(ass.Package)
-					if err != nil && !l.Options.Force {
-						return errors.Wrap(err, "Failed updating package")
-					}
-				} else {
-					_, err := s.Database.CreatePackage(ass.Package)
-					if err != nil && !l.Options.Force {
-						return errors.Wrap(err, "Failed creating package")
-					}
-				}
 				installed, ok := toInstall[ass.Package.GetFingerPrint()]
 				if !ok {
-					return errors.New("Couldn't find ArtifactMatch for " + ass.Package.HumanReadableString())
+					// It was a dep already installed in the system, so we can skip it safely
+					continue ORDER
 				}
 
 				treePackage, err := installed.Repository.GetTree().GetDatabase().FindPackage(ass.Package)
@@ -381,12 +395,17 @@ func (l *LuetInstaller) uninstall(p pkg.Package, s *System) error {
 }
 
 func (l *LuetInstaller) Uninstall(p pkg.Package, s *System) error {
-	// compute uninstall from all world - remove packages in parallel - run uninstall finalizer (in order) - mark the uninstallation in db
+	// compute uninstall from all world - remove packages in parallel - run uninstall finalizer (in order) TODO - mark the uninstallation in db
 	// Get installed definition
 
+	checkConflicts := true
+	if l.Options.Force == true {
+		checkConflicts = false
+	}
+
 	if !l.Options.NoDeps {
 		solv := solver.NewResolver(s.Database, s.Database, pkg.NewInMemoryDatabase(false), l.Options.SolverOptions.Resolver())
-		solution, err := solv.Uninstall(p)
+		solution, err := solv.Uninstall(p, checkConflicts)
 		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")
 		}

pkg/package/package.go

@@ -479,7 +479,7 @@ func (pack *DefaultPackage) BuildFormula(definitiondb PackageDatabase, db Packag
 			}
 			B := bf.Var(encodedB)
 			if !p.Matches(cp) {
-				formulas = append(formulas, bf.Or(A, bf.Or(bf.Not(A), bf.Not(B))))
+				formulas = append(formulas, bf.Or(bf.Not(A), bf.Or(bf.Not(A), bf.Not(B))))
 			}
 		}
 	}
@@ -495,9 +495,7 @@ func (pack *DefaultPackage) BuildFormula(definitiondb PackageDatabase, db Packag
 			if err != nil || len(packages) == 0 {
 				required = requiredDef
 			} else {
-				if len(packages) == 1 {
+
-					required = packages[0]
-				} else {
 				var ALO, priorityConstraints, priorityALO []bf.Formula
 
 				// Try to prio best match
@@ -525,7 +523,7 @@ func (pack *DefaultPackage) BuildFormula(definitiondb PackageDatabase, db Packag
 					C = bf.Var(encodedC)
 					// Or the Candidate is true, or all the others might be not true
 					// This forces the CDCL sat implementation to look first at a solution with C=true
-						formulas = append(formulas, bf.Or(bf.Or(C, bf.Or(priorityConstraints...)), bf.Or(bf.Not(C), bf.Or(priorityALO...))))
+					formulas = append(formulas, bf.Or(bf.Not(A), bf.Or(bf.Or(C, bf.Or(priorityConstraints...)), bf.Or(bf.Not(C), bf.Or(priorityALO...)))))
 				}
 
 				// AMO - At most one
@@ -543,14 +541,14 @@ func (pack *DefaultPackage) BuildFormula(definitiondb PackageDatabase, db Packag
 						}
 						I := bf.Var(encodedI)
 						if !o.Matches(i) {
-								formulas = append(formulas, bf.Or(bf.Not(I), bf.Not(B)))
+							formulas = append(formulas, bf.Or(bf.Not(A), bf.Or(bf.Not(I), bf.Not(B))))
 						}
 					}
 				}
-					formulas = append(formulas, bf.Or(ALO...)) // ALO - At least one
+				formulas = append(formulas, bf.Or(bf.Not(A), bf.Or(ALO...))) // ALO - At least one
 				continue
 			}
-			}
+
 		}
 
 		encodedB, err := required.Encode(db)

pkg/solver/solver.go

@@ -28,11 +28,11 @@ import (
 type PackageSolver interface {
 	SetDefinitionDatabase(pkg.PackageDatabase)
 	Install(p []pkg.Package) (PackagesAssertions, error)
-	Uninstall(candidate pkg.Package) ([]pkg.Package, error)
+	Uninstall(candidate pkg.Package, checkconflicts bool) ([]pkg.Package, error)
 	ConflictsWithInstalled(p pkg.Package) (bool, error)
 	ConflictsWith(p pkg.Package, ls []pkg.Package) (bool, error)
 	World() []pkg.Package
-	Upgrade() ([]pkg.Package, PackagesAssertions, error)
+	Upgrade(checkconflicts bool) ([]pkg.Package, PackagesAssertions, error)
 
 	SetResolver(PackageResolver)
 
@@ -210,7 +210,7 @@ func (s *Solver) ConflictsWithInstalled(p pkg.Package) (bool, error) {
 	return s.ConflictsWith(p, s.Installed())
 }
 
-func (s *Solver) Upgrade() ([]pkg.Package, PackagesAssertions, error) {
+func (s *Solver) Upgrade(checkconflicts bool) ([]pkg.Package, PackagesAssertions, error) {
 
 	// First get candidates that needs to be upgraded..
 
@@ -236,12 +236,11 @@ func (s *Solver) Upgrade() ([]pkg.Package, PackagesAssertions, error) {
 			}
 		}
 	}
-
 	s2 := NewSolver(installedcopy, s.DefinitionDatabase, pkg.NewInMemoryDatabase(false))
 	s2.SetResolver(s.Resolver)
 	// Then try to uninstall the versions in the system, and store that tree
 	for _, p := range toUninstall {
-		r, err := s.Uninstall(p)
+		r, err := s.Uninstall(p, checkconflicts)
 		if err != nil {
 			return nil, nil, errors.Wrap(err, "Could not compute upgrade - couldn't uninstall selected candidate "+p.GetFingerPrint())
 		}
@@ -262,7 +261,7 @@ func (s *Solver) Upgrade() ([]pkg.Package, PackagesAssertions, error) {
 
 // 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) ([]pkg.Package, error) {
+func (s *Solver) Uninstall(c pkg.Package, checkconflicts bool) ([]pkg.Package, error) {
 	var res []pkg.Package
 	candidate, err := s.InstalledDatabase.FindPackage(c)
 	if err != nil {
@@ -294,17 +293,19 @@ func (s *Solver) Uninstall(c pkg.Package) ([]pkg.Package, error) {
 		}
 	}
 
+	s2 := NewSolver(pkg.NewInMemoryDatabase(false), s.DefinitionDatabase, pkg.NewInMemoryDatabase(false))
+	s2.SetResolver(s.Resolver)
 	// Get the requirements to install the candidate
-	saved := s.InstalledDatabase
+	asserts, err := s2.Install([]pkg.Package{candidate})
-	s.InstalledDatabase = pkg.NewInMemoryDatabase(false)
-	asserts, err := s.Install([]pkg.Package{candidate})
 	if err != nil {
 		return nil, err
 	}
-	s.InstalledDatabase = saved
-
 	for _, a := range asserts {
 		if a.Value {
+			if !checkconflicts {
+				res = append(res, a.Package.IsFlagged(false))
+				continue
+			}
 
 			c, err := s.ConflictsWithInstalled(a.Package)
 			if err != nil {

pkg/solver/solver_test.go

@@ -429,7 +429,7 @@ var _ = Describe("Solver", func() {
 			Expect(solution).To(ContainElement(PackageAssert{Package: D1, Value: false}))
 			Expect(solution).ToNot(ContainElement(PackageAssert{Package: E, Value: true}))
 
-			Expect(len(solution)).To(Equal(5))
+			Expect(len(solution)).To(Equal(6))
 			Expect(err).ToNot(HaveOccurred())
 		})
 
@@ -470,7 +470,7 @@ var _ = Describe("Solver", func() {
 			Expect(solution).To(ContainElement(PackageAssert{Package: D1, Value: false}))
 			Expect(solution).ToNot(ContainElement(PackageAssert{Package: E, Value: true}))
 
-			Expect(len(solution)).To(Equal(4))
+			Expect(len(solution)).To(Equal(6))
 			Expect(err).ToNot(HaveOccurred())
 		})
 
@@ -511,7 +511,7 @@ var _ = Describe("Solver", func() {
 			Expect(solution).To(ContainElement(PackageAssert{Package: D1, Value: false}))
 			Expect(solution).ToNot(ContainElement(PackageAssert{Package: E, Value: true}))
 
-			Expect(len(solution)).To(Equal(4))
+			Expect(len(solution)).To(Equal(6))
 			Expect(err).ToNot(HaveOccurred())
 		})
 
@@ -533,7 +533,7 @@ var _ = Describe("Solver", func() {
 			}
 			s = NewSolver(dbInstalled, dbDefinitions, db)
 
-			solution, err := s.Uninstall(A)
+			solution, err := s.Uninstall(A, true)
 			Expect(err).ToNot(HaveOccurred())
 
 			Expect(solution).To(ContainElement(A.IsFlagged(false)))
@@ -559,7 +559,7 @@ var _ = Describe("Solver", func() {
 			}
 			s = NewSolver(dbInstalled, dbDefinitions, db)
 
-			solution, err := s.Uninstall(&pkg.DefaultPackage{Name: "A", Version: ">1.0"})
+			solution, err := s.Uninstall(&pkg.DefaultPackage{Name: "A", Version: ">1.0"}, true)
 			Expect(err).ToNot(HaveOccurred())
 
 			Expect(solution).To(ContainElement(A.IsFlagged(false)))
@@ -674,7 +674,7 @@ var _ = Describe("Solver", func() {
 				_, err := dbInstalled.CreatePackage(p)
 				Expect(err).ToNot(HaveOccurred())
 			}
-			solution, err := s.Uninstall(A)
+			solution, err := s.Uninstall(A, true)
 			Expect(err).ToNot(HaveOccurred())
 
 			Expect(solution).To(ContainElement(A.IsFlagged(false)))
@@ -698,7 +698,7 @@ var _ = Describe("Solver", func() {
 				_, err := dbInstalled.CreatePackage(p)
 				Expect(err).ToNot(HaveOccurred())
 			}
-			solution, err := s.Uninstall(A)
+			solution, err := s.Uninstall(A, true)
 			Expect(err).ToNot(HaveOccurred())
 
 			Expect(solution).To(ContainElement(A.IsFlagged(false)))
@@ -721,7 +721,7 @@ var _ = Describe("Solver", func() {
 				_, err := dbInstalled.CreatePackage(p)
 				Expect(err).ToNot(HaveOccurred())
 			}
-			solution, err := s.Uninstall(A)
+			solution, err := s.Uninstall(A, true)
 			Expect(err).ToNot(HaveOccurred())
 
 			Expect(solution).To(ContainElement(A.IsFlagged(false)))
@@ -746,7 +746,7 @@ var _ = Describe("Solver", func() {
 				Expect(err).ToNot(HaveOccurred())
 			}
 
-			solution, err := s.Uninstall(A)
+			solution, err := s.Uninstall(A, true)
 			Expect(err).ToNot(HaveOccurred())
 
 			Expect(solution).To(ContainElement(A.IsFlagged(false)))
@@ -772,7 +772,7 @@ var _ = Describe("Solver", func() {
 				Expect(err).ToNot(HaveOccurred())
 			}
 
-			solution, err := s.Uninstall(A)
+			solution, err := s.Uninstall(A, true)
 			Expect(err).ToNot(HaveOccurred())
 
 			Expect(solution).To(ContainElement(A.IsFlagged(false)))
@@ -893,7 +893,7 @@ var _ = Describe("Solver", func() {
 				_, err := dbInstalled.CreatePackage(p)
 				Expect(err).ToNot(HaveOccurred())
 			}
-			uninstall, solution, err := s.Upgrade()
+			uninstall, solution, err := s.Upgrade(true)
 			Expect(err).ToNot(HaveOccurred())
 
 			Expect(len(uninstall)).To(Equal(1))

tests/fixtures/upgrade_integration/c/definition.yaml

@@ -1,3 +1,9 @@
 category: "test"
 name: "c"
 version: "1.0"
+# Boom?
+
+requires:
+- category: "test"
+  name: "a"
+  version: ">=0.1"

tests/fixtures/upgrade_integration/cat/a/alatest/build.yaml

@@ -0,0 +1,11 @@
+image: "alpine"
+prelude:
+  - echo foo > /test
+  - echo bar > /test2
+steps:
+  - echo artifact3 > /testlatest
+  - echo artifact4 > /testlatest2
+requires:
+- category: "test"
+  name: "b"
+  version: "1.0"

tests/fixtures/upgrade_integration/cat/a/alatest/definition.yaml

@@ -0,0 +1,3 @@
+category: "test"
+name: "a"
+version: "1.2"

tests/integration/03_qlearning.sh

@@ -12,7 +12,7 @@ oneTimeTearDown() {
 
 testBuild() {
     mkdir $tmpdir/testbuild
-    luet build --all --tree "$ROOT_DIR/tests/fixtures/qlearning" --destination $tmpdir/testbuild --compression gzip
+    luet build --all --concurrency 1 --tree "$ROOT_DIR/tests/fixtures/qlearning" --destination $tmpdir/testbuild --compression gzip
     buildst=$?
     assertEquals 'builds successfully' "$buildst" "0"
     assertTrue 'create package dep B' "[ -e '$tmpdir/testbuild/b-test-1.0.package.tar.gz' ]"

tests/integration/05_upgrade.sh

@@ -33,6 +33,18 @@ testBuild() {
 
     assertTrue 'create package A 1.1' "[ -e '$tmpdir/testbuild/a-test-1.1.package.tar.gz' ]"
 
+    luet build --tree "$ROOT_DIR/tests/fixtures/upgrade_integration" --destination $tmpdir/testbuild --compression gzip test/a-1.2
+    buildst=$?
+    assertEquals 'builds successfully' "$buildst" "0"
+
+    assertTrue 'create package A 1.2' "[ -e '$tmpdir/testbuild/a-test-1.2.package.tar.gz' ]"
+
+
+    luet build --tree "$ROOT_DIR/tests/fixtures/upgrade_integration" --destination $tmpdir/testbuild --compression gzip test/c-1.0
+    buildst=$?
+    assertEquals 'builds successfully' "$buildst" "0"
+    assertTrue 'create package C 1.0' "[ -e '$tmpdir/testbuild/c-test-1.0.package.tar.gz' ]"
+
 }
 
 testRepo() {
@@ -73,27 +85,36 @@ EOF
 
 testInstall() {
     luet install --config $tmpdir/luet.yaml test/b-1.0
-    #luet install --config $tmpdir/luet.yaml test/c-1.0 > /dev/null
     installst=$?
     assertEquals 'install test successfully' "$installst" "0"
     assertTrue 'package installed B' "[ -e '$tmpdir/testrootfs/test5' ]"
 
     luet install --config $tmpdir/luet.yaml test/a-1.0
-    #luet install --config $tmpdir/luet.yaml test/c-1.0 > /dev/null
     assertTrue 'package installed A' "[ -e '$tmpdir/testrootfs/testaa' ]"
-
     installst=$?
+    assertEquals 'install test successfully' "$installst" "0"
+
+    luet install --config $tmpdir/luet.yaml test/a-1.1
+    assertTrue 'package installed A' "[ -e '$tmpdir/testrootfs/testaa' ]"
+    installst=$?
+    assertEquals 'install test successfully' "$installst" "0"
+    assertTrue 'package keeps old A' "[ -e '$tmpdir/testrootfs/testaa' ]"
+    assertTrue 'package new A was not installed' "[ ! -e '$tmpdir/testrootfs/testlatest' ]"
+
+    luet install --config $tmpdir/luet.yaml test/c-1.0
+    installst=$?
+    assertEquals 'install test successfully' "$installst" "0"
+    assertTrue 'package installed C' "[ -e '$tmpdir/testrootfs/c' ]"
 }
 
 testUpgrade() {
-    output=$(luet --config $tmpdir/luet.yaml upgrade)
+    luet --config $tmpdir/luet.yaml upgrade
     installst=$?
-    echo $output
     assertEquals 'install test successfully' "$installst" "0"
     assertTrue 'package uninstalled B' "[ ! -e '$tmpdir/testrootfs/test5' ]"
     assertTrue 'package installed B' "[ -e '$tmpdir/testrootfs/newc' ]"
     assertTrue 'package uninstalled A' "[ ! -e '$tmpdir/testrootfs/testaa' ]"
-    assertTrue 'package installed new A' "[ -e '$tmpdir/testrootfs/test3' ]"
+    assertTrue 'package installed new A' "[ -e '$tmpdir/testrootfs/testlatest' ]"
 }
 
 # Load shUnit2.