Skip to content

Commit

Permalink
Sqaushed changes of following 2 commits
Browse files Browse the repository at this point in the history
Change dependecy list to arch specific dependecies
Currently we support eext packages to specify a list of dependecies for
the package. Some packages reqiure the dependecies only for a specific
arch (since dependecies for other archs are available upstream).
Hence we support arch specific dependecies for eext packages, allowed
archs are 'i686', 'x86_64' and 'all'(both)

Added documentation and checks for dependencies
Updated the dependencies docstring with additional details for usage.
Added checks to ensure dependencies and not duplicated across archs,
and only valid arch is provided as argument.
Updated supported test cases to cover all arch dependency cases.

Fixes: BUG908812
  • Loading branch information
manith-arista committed Apr 1, 2024
1 parent 3d01047 commit 86bb1a2
Show file tree
Hide file tree
Showing 8 changed files with 130 additions and 36 deletions.
28 changes: 25 additions & 3 deletions impl/mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import (

"github.com/spf13/viper"

"golang.org/x/exp/slices"

"code.arista.io/eos/tools/eext/dnfconfig"
"code.arista.io/eos/tools/eext/manifest"
"code.arista.io/eos/tools/eext/util"
Expand Down Expand Up @@ -93,7 +95,7 @@ func (bldr *mockBuilder) clean() error {
func (bldr *mockBuilder) setupDeps() error {
bldr.log("starting")

if bldr.buildSpec.Dependencies == nil {
if len(bldr.dependencyList) == 0 {
panic(fmt.Sprintf("%sUnexpected call to setupDeps "+
"(manifest doesn't specify any dependencies)",
bldr.errPrefix))
Expand All @@ -108,7 +110,7 @@ func (bldr *mockBuilder) setupDeps() error {
var missingDeps []string
pathMap := make(map[string]string)
mockDepsDir := getMockDepsDir(bldr.pkg, bldr.arch)
for _, dep := range bldr.buildSpec.Dependencies {
for _, dep := range bldr.dependencyList {
depStatisfied := false
for _, arch := range []string{"noarch", bldr.arch} {
depDirWithArch := filepath.Join(depsDir, arch, dep)
Expand Down Expand Up @@ -287,7 +289,9 @@ func (bldr *mockBuilder) runStages() error {
return err
}

if bldr.buildSpec.Dependencies != nil {
// Checking if the package has any dependencies for the target build arch.
// Using length check of dependency list, since bldr.Build.Dependencies might be set for other arch deps.
if len(bldr.dependencyList) != 0 {
bldr.setupStageErrPrefix("setupDeps")
if err := bldr.setupDeps(); err != nil {
return err
Expand Down Expand Up @@ -321,11 +325,23 @@ func (bldr *mockBuilder) runStages() error {
// Mock calls fedora mock to build the RPMS for the specified target
// from the already built SRPMs and places the results in
// <DestDir>/RPMS/<rpmArch>/<package>/
// 'arch' cannot be empty, needs to be a valid architecture.
func Mock(repo string, pkg string, arch string, extraArgs MockExtraCmdlineArgs) error {
if err := setup(); err != nil {
return err
}

// Check if target arch has been set
allowedArchTypes := []string{"i686", "x86_64", "aarch64"}
if arch == "" {
return fmt.Errorf("Arch is not set, please input a valid build architecture.")
}
// Check if target arch is a valid arch value
if !slices.Contains(allowedArchTypes, arch) {
panic(fmt.Sprintf("'%s' is not a valid build arch, must be one of %s", arch,
strings.Join(allowedArchTypes, ", ")))
}

// Error out early if source is not available.
if err := checkRepo(repo,
"", // pkg
Expand Down Expand Up @@ -370,6 +386,11 @@ func Mock(repo string, pkg string, arch string, extraArgs MockExtraCmdlineArgs)
return err
}

dependencyMap := pkgSpec.Build.Dependencies
// golang allows accessing keys of an empty/nil map, without throwing an error.
// If a key is not present in the map, it returns an empty instance of the value.
dependencyList := append(dependencyMap["all"], dependencyMap[arch]...)

bldr := &mockBuilder{
builderCommon: &builderCommon{
pkg: thisPkgName,
Expand All @@ -381,6 +402,7 @@ func Mock(repo string, pkg string, arch string, extraArgs MockExtraCmdlineArgs)
buildSpec: &pkgSpec.Build,
dnfConfig: dnfConfig,
errPrefix: errPrefix,
dependencyList: dependencyList,
},
onlyCreateCfg: extraArgs.OnlyCreateCfg,
noCheck: extraArgs.NoCheck,
Expand Down
3 changes: 2 additions & 1 deletion impl/mock_cfg.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ type builderCommon struct {
buildSpec *manifest.Build
dnfConfig *dnfconfig.DnfConfig
errPrefix util.ErrPrefix
dependencyList []string
}

type mockCfgBuilder struct {
Expand Down Expand Up @@ -104,7 +105,7 @@ func (cfgBldr *mockCfgBuilder) populateTemplateData() error {
}
}

if cfgBldr.buildSpec.Dependencies != nil {
if len(cfgBldr.dependencyList) != 0 {
localRepo := &dnfconfig.DnfRepoParams{
Name: "local-deps",
BaseURL: "file://" + getMockDepsDir(cfgBldr.pkg, arch),
Expand Down
4 changes: 4 additions & 0 deletions impl/mock_cfg_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,13 @@ func testMockConfig(t *testing.T, chained bool) {
}

var sampleManifestFile string
var dependencyList []string
if chained {
sampleManifestFile = "manifest-with-deps.yaml"
dependencyList = []string{"foo"}
} else {
sampleManifestFile = "manifest.yaml"
dependencyList = []string{}
}

t.Log("Copy testData/manifest to src directory")
Expand Down Expand Up @@ -78,6 +81,7 @@ func testMockConfig(t *testing.T, chained bool) {
eextSignature: "my-signature",
buildSpec: &manifestObj.Package[0].Build,
dnfConfig: dnfConfig,
dependencyList: dependencyList,
},
}

Expand Down
64 changes: 41 additions & 23 deletions impl/setupDeps_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ func TestSetupDeps(t *testing.T) {
destDir := filepath.Join(testWorkingDir, "dest")
depsDir := filepath.Join(testWorkingDir, "deps")

for _, subdir := range []string{srcDir, workDir, destDir, depsDir} {
for _, subdir := range []string{srcDir, workDir, destDir} {
os.Mkdir(subdir, 0775)
}

Expand All @@ -52,6 +52,7 @@ func TestSetupDeps(t *testing.T) {
defer viper.Reset()

depPkg := "foo"
depArch := "all"
depPkgArch := "noarch"
depVersion := "1.0.0"
depRelease := "1"
Expand All @@ -63,29 +64,46 @@ func TestSetupDeps(t *testing.T) {
require.Equal(t, pkg, manifestObj.Package[0].Name)
require.NotNil(t, manifestObj.Package[0].Build)
require.NotNil(t, manifestObj.Package[0].Build.Dependencies)
require.Equal(t, depPkg, manifestObj.Package[0].Build.Dependencies[0])
require.Equal(t, depPkg, manifestObj.Package[0].Build.Dependencies[depArch][0])

testutil.SetupDummyDependency(t, depsDir,
depPkg, depPkgArch, depVersion, depRelease)
for _, targetArch := range []string{"x86_64", "i686", "aarch64"} {
// Creating an anonymous function to ensure 'depsDir' folder is removed with 'defer' for each arch
func() {
os.Mkdir(depsDir, 0755)
defer os.RemoveAll(depsDir)

bldr := mockBuilder{
builderCommon: &builderCommon{
pkg: pkg,
arch: "x86_64",
buildSpec: &manifestObj.Package[0].Build,
},
}
dependencyMap := manifestObj.Package[0].Build.Dependencies
dependencyList := append(dependencyMap["all"], dependencyMap[targetArch]...)

for _, depPkg := range dependencyList {
testutil.SetupDummyDependency(t, depsDir,
depPkg, depPkgArch, depVersion, depRelease)
}

bldr := mockBuilder{
builderCommon: &builderCommon{
pkg: pkg,
arch: targetArch,
buildSpec: &manifestObj.Package[0].Build,
dependencyList: dependencyList,
},
}

t.Log("Running setupDeps")
setupDepsErr := bldr.setupDeps()
require.NoError(t, setupDepsErr)
t.Log("Ran setupDeps, verifying results")

depsWorkDir := filepath.Join(workDir, pkg, "mock-x86_64/mock-deps")
expectedDummyDepDir := filepath.Join(depsWorkDir, depPkgArch, depPkg)
expectedDummyDepFilename := fmt.Sprintf("%s-%s-%s.%s.rpm", depPkg, depVersion, depRelease, depPkgArch)
expectedDummyDepFilepath := filepath.Join(expectedDummyDepDir, expectedDummyDepFilename)
require.FileExists(t, expectedDummyDepFilepath)
require.DirExists(t, filepath.Join(depsWorkDir, "repodata"))
t.Log("Results verified")
t.Log("Running setupDeps")
setupDepsErr := bldr.setupDeps()
require.NoError(t, setupDepsErr)
t.Log("Ran setupDeps, verifying results")

for _, depPkg := range dependencyList {
archDir := "mock-" + targetArch + "/mock-deps"
depsWorkDir := filepath.Join(workDir, pkg, archDir)
expectedDummyDepDir := filepath.Join(depsWorkDir, depPkgArch, depPkg)
expectedDummyDepFilename := fmt.Sprintf("%s-%s-%s.%s.rpm", depPkg, depVersion, depRelease, depPkgArch)
expectedDummyDepFilepath := filepath.Join(expectedDummyDepDir, expectedDummyDepFilename)
require.FileExists(t, expectedDummyDepFilepath)
require.DirExists(t, filepath.Join(depsWorkDir, "repodata"))
}
t.Logf("Results verified for %s", targetArch)
}()
}
}
5 changes: 4 additions & 1 deletion impl/testData/manifest-with-deps.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,7 @@ package:
- name: bundle-boo2
version: latest
dependencies:
- foo
all:
- foo
i686:
- bar
49 changes: 45 additions & 4 deletions manifest/manifest.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,33 @@ type Generator struct {

// Build spec
// mock cfg is generated for each target depending on this
//
// RepoBundle specifies while bundles should the eext tool look into,
// to download required upstream dependencies. (Eg: el9, epel9)
// Defined in config/dnfconfig.yaml.
//
// Dependencies helps eext determine, based on the target arch, which package dependencies are required.
// Archs can be of type ['all', 'i686', 'x86_64', 'aarch64'].
// The map specifies the list of package dependencies eext should build locally, based on the current build arch.
// Use 'all' if the dependencies are common for all archs, else use arch specific dependencies.
// Eg:
//
// dependencies:
// all:
// - pkgDep1
// i686:
// - pkgDep2
//
// In this case, for 'i686' build, eext will need to build both pkgDep1 and pkgDep2.
// Whereas for 'x86_64' build, only pkgDep1 needs to be built.
//
// Generator specifies commands for eext generator
// Refer to Generator struct denifition above.
type Build struct {
Include []string `yaml:"include"`
RepoBundle []RepoBundle `yaml:"repo-bundle"`
Dependencies []string `yaml:"dependencies"`
Generator Generator `yaml:"eextgen"`
Include []string `yaml:"include"`
RepoBundle []RepoBundle `yaml:"repo-bundle"`
Dependencies map[string][]string `yaml:"dependencies"`
Generator Generator `yaml:"eextgen"`
}

// DetachedSignature spec
Expand Down Expand Up @@ -141,6 +163,25 @@ func (m Manifest) sanityCheck() error {
pkgSpec.Name)
}

if pkgSpec.Build.Dependencies != nil {
dependencyMap := pkgSpec.Build.Dependencies
allowedArchs := []string{"all", "i686", "x86_64", "aarch64"}
duplicatePkgCheckList := make(map[string]string)
for arch := range dependencyMap {
if !slices.Contains(allowedArchs, arch) {
return fmt.Errorf("'%v' is not a valid/supported arch, use one of %v", arch, allowedArchs)
}
for _, depPkg := range dependencyMap[arch] {
otherArch, exists := duplicatePkgCheckList[depPkg]
if exists && (arch == "all" || otherArch == "all") {
return fmt.Errorf("Dependency package %v cannot belong to 'all' and '%v', choose any one arch",
depPkg, arch)
}
duplicatePkgCheckList[depPkg] = arch
}
}
}

for _, upStreamSrc := range pkgSpec.UpstreamSrc {
specifiedFullSrcURL := (upStreamSrc.FullURL != "")
specifiedSrcBundle := (upStreamSrc.SourceBundle != SourceBundle{})
Expand Down
9 changes: 7 additions & 2 deletions manifest/testData/sampleManifest1.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,13 @@ package:
version: v1
- name: bar
dependencies:
- libpcap
- glibc
all:
- libpcap
- glibc
x86_64:
- gcc11
i686:
- iptables
eextgen:
cmd-options:
mock:
Expand Down
4 changes: 2 additions & 2 deletions testutil/testutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ Name: {{.Name}}
Version: {{.UpstreamVersion}}
{{ .SpecFileReleaseLine }}
BuildArch: {{.Arch}}
License: foo
License: {{.Name}}
{{ range .Requires }}
Requires: {{.}}
Expand All @@ -77,7 +77,7 @@ BuildRequires: {{.}}
%description
foo
{{.Name}}
%prep
true
Expand Down

0 comments on commit 86bb1a2

Please sign in to comment.