Merge pull request #4274 from microsoft/mandeepsplaha/cherry-pick-rpm-resolution-mechanism-fix

Updated toolkit's package resolution to accept installed packages. (#4211)

Author: mandeepsplaha

toolkit/tools/internal/pkggraph/pkggraph.go

@@ -19,6 +19,7 @@ import (
 	"github.com/microsoft/CBL-Mariner/toolkit/tools/internal/file"
 	"github.com/microsoft/CBL-Mariner/toolkit/tools/internal/logger"
 	"github.com/microsoft/CBL-Mariner/toolkit/tools/internal/pkgjson"
+	"github.com/microsoft/CBL-Mariner/toolkit/tools/internal/sliceutils"
 	"github.com/microsoft/CBL-Mariner/toolkit/tools/internal/versioncompare"
 
 	"gonum.org/v1/gonum/graph"
@@ -1417,10 +1418,7 @@ func rpmsProvidedBySRPM(srpmPath string, pkgGraph *PkgGraph, graphMutex *sync.RW
 		rpmsMap[node.RpmPath] = true
 	}
 
-	rpmFiles = make([]string, 0, len(rpmsMap))
-	for rpm := range rpmsMap {
-		rpmFiles = append(rpmFiles, rpm)
-	}
+	rpmFiles = sliceutils.StringsSetToSlice(rpmsMap)
 
 	return
 }

toolkit/tools/internal/rpm/rpm.go

@@ -12,6 +12,7 @@ import (
 	"github.com/microsoft/CBL-Mariner/toolkit/tools/internal/file"
 	"github.com/microsoft/CBL-Mariner/toolkit/tools/internal/logger"
 	"github.com/microsoft/CBL-Mariner/toolkit/tools/internal/shell"
+	"github.com/microsoft/CBL-Mariner/toolkit/tools/internal/sliceutils"
 )
 
 const (
@@ -53,36 +54,38 @@ const (
 )
 
 const (
+	installedRPMRegexRPMIndex = 1
+
 	rpmProgram      = "rpm"
 	rpmSpecProgram  = "rpmspec"
 	rpmBuildProgram = "rpmbuild"
 )
 
-var goArchToRpmArch = map[string]string{
-	"amd64": "x86_64",
-	"arm64": "aarch64",
-}
-
-// GetRpmArch converts the GOARCH arch into an RPM arch
-func GetRpmArch(goArch string) (rpmArch string, err error) {
-	rpmArch, ok := goArchToRpmArch[goArch]
-	if !ok {
-		err = fmt.Errorf("Unknown GOARCH detected (%s)", goArch)
+var (
+	goArchToRpmArch = map[string]string{
+		"amd64": "x86_64",
+		"arm64": "aarch64",
 	}
-	return
-}
 
-var (
 	// Output from 'rpm' prints installed RPMs in a line with the following format:
 	//
 	//	D: ========== +++ [name]-[version]-[release].[distribution] [architecture]-linux [hex_value]
 	//
 	// Example:
 	//
 	//	D: ========== +++ systemd-devel-239-42.cm2 x86_64-linux 0x0
-	installedRPMLineRegex = regexp.MustCompile(`^D: =+ \+{3} (\S+).*$`)
+	installedRPMRegex = regexp.MustCompile(`^D: =+ \+{3} (\S+).*$`)
 )
 
+// GetRpmArch converts the GOARCH arch into an RPM arch
+func GetRpmArch(goArch string) (rpmArch string, err error) {
+	rpmArch, ok := goArchToRpmArch[goArch]
+	if !ok {
+		err = fmt.Errorf("Unknown GOARCH detected (%s)", goArch)
+	}
+	return
+}
+
 // SetMacroDir adds RPM_CONFIGDIR=$(newMacroDir) into the shell's environment for the duration of a program.
 // To restore the environment the caller can use shell.SetEnvironment() with the returned origenv.
 // On an empty string argument return success immediately and do not modify the environment.
@@ -318,13 +321,13 @@ func QueryRPMProvides(rpmFile string) (provides []string, err error) {
 // end up being installed after resolving outdated, obsoleted, or conflicting packages.
 func ResolveCompetingPackages(rootDir string, rpmPaths ...string) (resolvedRPMs []string, err error) {
 	const (
-		queryFormat       = ""
-		installedRPMIndex = 1
-		squashErrors      = true
+		queryFormat  = ""
+		squashErrors = true
 	)
 
 	args := []string{
 		"-Uvvh",
+		"--replacepkgs",
 		"--nodeps",
 		"--root",
 		rootDir,
@@ -340,15 +343,15 @@ func ResolveCompetingPackages(rootDir string, rpmPaths ...string) (resolvedRPMs
 	}
 
 	splitStdout := strings.Split(stderr, "\n")
+	uniqueResolvedRPMs := map[string]bool{}
 	for _, line := range splitStdout {
-		matches := installedRPMLineRegex.FindStringSubmatch(line)
-		if len(matches) == 0 {
-			continue
+		matches := installedRPMRegex.FindStringSubmatch(line)
+		if len(matches) != 0 {
+			uniqueResolvedRPMs[matches[installedRPMRegexRPMIndex]] = true
 		}
-
-		resolvedRPMs = append(resolvedRPMs, matches[installedRPMIndex])
 	}
 
+	resolvedRPMs = sliceutils.StringsSetToSlice(uniqueResolvedRPMs)
 	return
 }
 

toolkit/tools/internal/sliceutils/sliceutils.go

@@ -41,3 +41,17 @@ func FindMatches(slice []string, isMatch func(string) bool) []string {
 func StringMatch(expected, given interface{}) bool {
 	return expected.(string) == given.(string)
 }
+
+func StringsSetToSlice(inputSet map[string]bool) []string {
+	index := 0
+	outputSlice := make([]string, len(inputSet))
+
+	for element, elementInSet := range inputSet {
+		if elementInSet {
+			outputSlice[index] = element
+			index++
+		}
+	}
+
+	return outputSlice[:index]
+}

toolkit/tools/internal/sliceutils/sliceutils_test.go

@@ -0,0 +1,48 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+package sliceutils
+
+import (
+	"os"
+	"testing"
+
+	"github.com/microsoft/CBL-Mariner/toolkit/tools/internal/logger"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestMain(m *testing.M) {
+	logger.InitStderrLog()
+	os.Exit(m.Run())
+}
+
+func TestShouldCreateEmptySliceFromNil(t *testing.T) {
+	outputSlice := StringsSetToSlice(nil)
+
+	assert.NotNil(t, outputSlice)
+	assert.Empty(t, outputSlice)
+}
+
+func TestShouldCreateEmptySliceFromEmptySet(t *testing.T) {
+	outputSlice := StringsSetToSlice(map[string]bool{})
+
+	assert.NotNil(t, outputSlice)
+	assert.Empty(t, outputSlice)
+}
+
+func TestShouldReturnValuesForAllTrueElementsInSet(t *testing.T) {
+	inputSet := map[string]bool{
+		"A": true,
+		"B": true,
+		"X": false,
+		"Y": false,
+	}
+	outputSlice := StringsSetToSlice(inputSet)
+
+	assert.NotNil(t, outputSlice)
+	assert.Len(t, outputSlice, 2)
+	assert.Contains(t, outputSlice, "A")
+	assert.Contains(t, outputSlice, "B")
+	assert.NotContains(t, outputSlice, "X")
+	assert.NotContains(t, outputSlice, "Y")
+}

toolkit/tools/scheduler/schedulerutils/buildworker.go

@@ -181,10 +181,7 @@ func getBuildDependencies(node *pkggraph.PkgNode, pkgGraph *pkggraph.PkgGraph, g
 		return
 	})
 
-	dependencies = make([]string, 0, len(dependencyLookup))
-	for depName := range dependencyLookup {
-		dependencies = append(dependencies, depName)
-	}
+	dependencies = sliceutils.StringsSetToSlice(dependencyLookup)
 
 	return
 }

toolkit/tools/scheduler/schedulerutils/graphbuildstate.go

@@ -9,6 +9,7 @@ import (
 
 	"github.com/microsoft/CBL-Mariner/toolkit/tools/internal/logger"
 	"github.com/microsoft/CBL-Mariner/toolkit/tools/internal/pkggraph"
+	"github.com/microsoft/CBL-Mariner/toolkit/tools/internal/sliceutils"
 )
 
 // nodeState represents the build state of a single node
@@ -90,26 +91,18 @@ func (g *GraphBuildState) BuildFailures() []*BuildResult {
 // ConflictingRPMs will return a list of *.rpm files which should not have been rebuilt.
 // This list is based on the manifest of pre-built toolchain rpms.
 func (g *GraphBuildState) ConflictingRPMs() (rpms []string) {
-	rpms = make([]string, len(g.conflictingRPMs))
-	i := 0
-	for f := range g.conflictingRPMs {
-		rpms[i] = f
-		i++
-	}
+	rpms = sliceutils.StringsSetToSlice(g.conflictingRPMs)
 	sort.Strings(rpms)
+
 	return rpms
 }
 
 // ConflictingSRPMs will return a list of *.src.rpm files which created rpms that should not have been rebuilt.
 // This list is based on the manifest of pre-built toolchain rpms.
 func (g *GraphBuildState) ConflictingSRPMs() (srpms []string) {
-	srpms = make([]string, len(g.conflictingSRPMs))
-	i := 0
-	for f := range g.conflictingSRPMs {
-		srpms[i] = f
-		i++
-	}
+	srpms = sliceutils.StringsSetToSlice(g.conflictingSRPMs)
 	sort.Strings(srpms)
+
 	return srpms
 }