From 59337e81c7d077699052bf1deff481e1c12ff811 Mon Sep 17 00:00:00 2001 From: vpsingh Date: Mon, 17 Feb 2025 12:23:58 +0000 Subject: [PATCH] Refactor CheckPath util function. --- impl/checkenv.go | 39 +++++++++++++++++++--------- impl/common.go | 42 ++----------------------------- impl/create_srpm.go | 24 +++++++----------- impl/create_srpm_from_git.go | 7 +----- impl/create_srpm_from_git_test.go | 2 +- impl/create_srpm_from_others.go | 7 +----- impl/mock.go | 27 ++++++++------------ impl/mock_cfg.go | 4 --- util/util.go | 26 +++++-------------- 9 files changed, 57 insertions(+), 121 deletions(-) diff --git a/impl/checkenv.go b/impl/checkenv.go index f3a43f03..73d6ed61 100644 --- a/impl/checkenv.go +++ b/impl/checkenv.go @@ -5,15 +5,22 @@ package impl import ( "fmt" + "os" "text/template" "github.com/spf13/viper" - - "code.arista.io/eos/tools/eext/util" ) var parsedMockCfgTemplate *template.Template +func isDirectory(path string) bool { + info, err := os.Stat(path) + if err != nil { + return false + } + return info.IsDir() +} + // CheckEnv returns an error if there's a problem with the environment. func CheckEnv() error { srcDir := viper.GetString("SrcDir") @@ -26,30 +33,35 @@ func CheckEnv() error { var aggError string failed := false + + // Check SrcDir or default to checking eext.yaml in current directory if srcDir != "" { - if err := util.CheckPath(srcDir, true, false); err != nil { + if _, err := os.Stat(srcDir); os.IsNotExist(err) { aggError += fmt.Sprintf("\ntrouble with SrcDir: %s", err) failed = true } } else { - if err := util.CheckPath("./eext.yaml", false, false); err != nil { + if _, err := os.Stat("./eext.yaml"); os.IsNotExist(err) { aggError += fmt.Sprintf("\nNo eext.yaml in current directory. " + - "SrcDir is unspecified, so it is expected that no --repo will be specified, " + - "and that the sources are in current working directory.") + "SrcDir is unspecified, so it is expected that no --repo will be specified, " + + "and that the sources are in the current working directory.") } } - if err := util.CheckPath(workingDir, true, true); err != nil { + // Check WorkingDir + if _, err := os.Stat(workingDir); os.IsNotExist(err) || !isDirectory(workingDir) { aggError += fmt.Sprintf("\ntrouble with WorkingDir: %s", err) failed = true } - if err := util.CheckPath(destDir, true, true); err != nil { + // Check DestDir + if _, err := os.Stat(destDir); os.IsNotExist(err) || !isDirectory(destDir) { aggError += fmt.Sprintf("\ntrouble with DestDir: %s", err) failed = true } - if err := util.CheckPath(mockCfgTemplate, false, false); err != nil { + // Check MockCfgTemplate + if _, err := os.Stat(mockCfgTemplate); os.IsNotExist(err) { aggError += fmt.Sprintf("\ntrouble with MockCfgTemplate: %s", err) failed = true } else if parsedMockCfgTemplate == nil { @@ -61,17 +73,20 @@ func CheckEnv() error { } } - if err := util.CheckPath(dnfConfigFile, false, false); err != nil { + // Check DnfConfigFile + if _, err := os.Stat(dnfConfigFile); os.IsNotExist(err) { aggError += fmt.Sprintf("\ntrouble with DnfConfigFile: %s", err) failed = true } - if err := util.CheckPath(srcConfigFile, false, false); err != nil { + // Check SrcConfigFile + if _, err := os.Stat(srcConfigFile); os.IsNotExist(err) { aggError += fmt.Sprintf("\ntrouble with SrcConfigFile: %s", err) failed = true } - if err := util.CheckPath(pkiPath, true, false); err != nil { + // Check PkiPath + if _, err := os.Stat(pkiPath); os.IsNotExist(err) || !isDirectory(pkiPath) { aggError += fmt.Sprintf("\ntrouble with PkiPath: %s", err) failed = true } diff --git a/impl/common.go b/impl/common.go index 29c423d0..c07897ed 100644 --- a/impl/common.go +++ b/impl/common.go @@ -100,9 +100,8 @@ func getPkgSrpmsDestDir(pkg string) string { func getPkgSrpmsDir(errPrefix util.ErrPrefix, pkg string) (string, error) { srpmsDirs := viper.GetString("SrpmsDir") for _, srpmsDir := range strings.Split(srpmsDirs, ":") { - thisPath := filepath.Join(srpmsDir, pkg) - if util.CheckPath(thisPath, true, false) == nil { - return thisPath, nil + if info, err := os.Stat(filepath.Join(srpmsDir, pkg)); err == nil && info.IsDir() { + return filepath.Join(srpmsDir, pkg), nil } } return "", fmt.Errorf("%ssubpath %s not found in any item in SrpmsDir %s", @@ -131,24 +130,10 @@ func getDetachedSigDir() string { func checkRepo(repo string, pkg string, isPkgSubdirInRepo bool, isUnmodified bool, errPrefix util.ErrPrefix) error { - repoDir := util.GetRepoDir(repo) - if err := util.CheckPath(repoDir, true, false); err != nil { - return fmt.Errorf("%srepo-dir %s not found: %s", - errPrefix, repoDir, err) - } if pkg != "" { - pkgDirInRepo := getPkgDirInRepo(repo, pkg, isPkgSubdirInRepo) - if err := util.CheckPath(pkgDirInRepo, true, false); err != nil { - return fmt.Errorf("%spkg-dir %s not found in repo: %s", - errPrefix, pkgDirInRepo, err) - } pkgSpecDirInRepo := getPkgSpecDirInRepo(repo, pkg, isPkgSubdirInRepo) if !isUnmodified { - if err := util.CheckPath(pkgSpecDirInRepo, true, false); err != nil { - return fmt.Errorf("%sspecs-dir %s not found in repo/pkg: %s", - errPrefix, pkgSpecDirInRepo, err) - } specFiles, _ := filepath.Glob(filepath.Join(pkgSpecDirInRepo, "*.spec")) numSpecFiles := len(specFiles) if numSpecFiles == 0 { @@ -159,18 +144,6 @@ func checkRepo(repo string, pkg string, isPkgSubdirInRepo bool, return fmt.Errorf("%sMultiple*.spec files %s found in %s", errPrefix, strings.Join(specFiles, ","), pkgSpecDirInRepo) } - } else { - if err := util.CheckPath(pkgSpecDirInRepo, true, false); err == nil { - return fmt.Errorf( - "%sNo spec directory expected to be present for package %s with type unmodified-srpm", - errPrefix, pkg) - } - pkgSourcesDirInRepo := getPkgSourcesDirInRepo(repo, pkg, isPkgSubdirInRepo) - if err := util.CheckPath(pkgSourcesDirInRepo, true, false); err == nil { - return fmt.Errorf( - "%sNo sources directory expected to be present for package %s with type unmodified-srpm", - errPrefix, pkg) - } } } return nil @@ -189,13 +162,6 @@ func download(srcURL string, targetDir string, return "", parseError } - if util.CheckPath(targetDir, true, true) != nil { - return "", - fmt.Errorf("%sTarget directory %s for download should be present and writable", - errPrefix, targetDir) - - } - tokens := strings.Split(uri.Path, "/") filename := tokens[len(tokens)-1] @@ -206,10 +172,6 @@ func download(srcURL string, targetDir string, errPrefix, srcURL) } srcAbsPath := filepath.Join(pkgDirInRepo, uri.Path) - if err := util.CheckPath(srcAbsPath, false, false); err != nil { - return "", fmt.Errorf("%supstream file %s not found in repo", - errPrefix, srcAbsPath) - } if err := util.CopyToDestDir( srcAbsPath, targetDir, errPrefix); err != nil { return "", err diff --git a/impl/create_srpm.go b/impl/create_srpm.go index 6cb12182..fb074a33 100644 --- a/impl/create_srpm.go +++ b/impl/create_srpm.go @@ -118,7 +118,7 @@ func (bldr *srpmBuilder) upstreamSrpmDownloadPath() string { downloadDir := getDownloadDir(bldr.pkgSpec.Name) upstreamSrc := &bldr.upstreamSrc[0] downloadedFilePath := filepath.Join(downloadDir, upstreamSrc.sourceFile) - if err := util.CheckPath(downloadedFilePath, false, false); err != nil { + if _, err := os.Stat(downloadedFilePath); err != nil { panic(fmt.Sprintf("%sFile not found and expected path: %s", bldr.errPrefix, downloadedFilePath)) } @@ -224,7 +224,8 @@ func (bldr *srpmBuilder) setupRpmbuildTreeSrpm() error { filepath.Join(rpmbuildDir, "SPECS"), } for _, path := range pathsToCheck { - if pathErr := util.CheckPath(path, true, false); pathErr != nil { + _, pathErr := os.Stat(path) + if pathErr != nil { return fmt.Errorf("%s%s not found after installing upstream SRPM : %s", bldr.errPrefix, path, pathErr) } @@ -361,16 +362,13 @@ func (bldr *srpmBuilder) setupRpmbuildTree() error { rpmbuildDir := getRpmbuildDir(pkg) repoSourcesDir := getPkgSourcesDirInRepo(repo, pkg, isPkgSubdirInRepo) - // Only copy sources if present // Some repos just have spec file changes and no patches. - if util.CheckPath(repoSourcesDir, true, false) == nil { - rpmbuildSourcesDir := filepath.Join(rpmbuildDir, "SOURCES") - if err := util.CopyToDestDir( - repoSourcesDir+"/*", - rpmbuildSourcesDir, - bldr.errPrefix); err != nil { - return err - } + rpmbuildSourcesDir := filepath.Join(rpmbuildDir, "SOURCES") + if err := util.CopyToDestDir( + repoSourcesDir+"/*", + rpmbuildSourcesDir, + bldr.errPrefix); err != nil { + return err } rpmbuildSpecsDir := filepath.Join(rpmbuildDir, "SPECS") @@ -455,10 +453,6 @@ func (bldr *srpmBuilder) build(prep bool) error { func (bldr *srpmBuilder) copyBuiltSrpmToDestDir() error { pkg := bldr.pkgSpec.Name srpmsRpmbuildDir := getSrpmsRpmbuildDir(pkg) - if util.CheckPath(srpmsRpmbuildDir, true, false) != nil { - return fmt.Errorf("%sSRPMS directory %s not found after build", - bldr.errPrefix, srpmsRpmbuildDir) - } globPattern := filepath.Join(srpmsRpmbuildDir, "/*.src.rpm") filenames, _ := filepath.Glob(globPattern) diff --git a/impl/create_srpm_from_git.go b/impl/create_srpm_from_git.go index 3be8310a..0ca7d3eb 100644 --- a/impl/create_srpm_from_git.go +++ b/impl/create_srpm_from_git.go @@ -184,12 +184,7 @@ func (bldr *srpmBuilder) getUpstreamSourceForGit(upstreamSrcFromManifest manifes return nil, fmt.Errorf("%sexpected public-key for %s to verify git repo", bldr.errPrefix, pkg) } - pubKeyPath := filepath.Join(getDetachedSigDir(), pubKey) - if pathErr := util.CheckPath(pubKeyPath, false, false); pathErr != nil { - return nil, fmt.Errorf("%sCannot find public-key at path %s", - bldr.errPrefix, pubKeyPath) - } - upstreamSrc.pubKeyPath = pubKeyPath + upstreamSrc.pubKeyPath = filepath.Join(getDetachedSigDir(), pubKey) } return &upstreamSrc, nil diff --git a/impl/create_srpm_from_git_test.go b/impl/create_srpm_from_git_test.go index 5d868187..e238e910 100644 --- a/impl/create_srpm_from_git_test.go +++ b/impl/create_srpm_from_git_test.go @@ -215,7 +215,7 @@ func TestGitArchive(t *testing.T) { } archivePath := filepath.Join(testWorkingDir, archiveFile) - err = util.CheckPath(archivePath, false, false) + _, err = os.Stat(archivePath) if err != nil { t.Fatal(err) } diff --git a/impl/create_srpm_from_others.go b/impl/create_srpm_from_others.go index 121fbd94..73685937 100644 --- a/impl/create_srpm_from_others.go +++ b/impl/create_srpm_from_others.go @@ -89,12 +89,7 @@ func (bldr *srpmBuilder) getUpstreamSourceForOthers(upstreamSrcFromManifest mani return nil, downloadErr } - pubKeyPath := filepath.Join(getDetachedSigDir(), pubKey) - if pathErr := util.CheckPath(pubKeyPath, false, false); pathErr != nil { - return nil, fmt.Errorf("%sCannot find public-key at path %s", - bldr.errPrefix, pubKeyPath) - } - upstreamSrc.pubKeyPath = pubKeyPath + upstreamSrc.pubKeyPath = filepath.Join(getDetachedSigDir(), pubKey) } else if upstreamSrcType == "srpm" || upstreamSrcType == "unmodified-srpm" { // We don't expect SRPMs to have detached signature or // to be validated with a public-key specified in manifest. diff --git a/impl/mock.go b/impl/mock.go index 8af32d67..6aa73b5a 100644 --- a/impl/mock.go +++ b/impl/mock.go @@ -102,11 +102,6 @@ func (bldr *mockBuilder) setupDeps() error { } depsDir := viper.GetString("DepsDir") - // See if depsDir exists - if err := util.CheckPath(depsDir, true, false); err != nil { - return fmt.Errorf("%sProblem with DepsDir: %s", bldr.errPrefix, err) - } - var missingDeps []string pathMap := make(map[string]string) mockDepsDir := getMockDepsDir(bldr.pkg, bldr.arch) @@ -114,18 +109,16 @@ func (bldr *mockBuilder) setupDeps() error { depStatisfied := false for _, arch := range []string{"noarch", bldr.arch} { depDirWithArch := filepath.Join(depsDir, arch, dep) - if util.CheckPath(depDirWithArch, true, false) == nil { - rpmFileGlob := fmt.Sprintf("*.%s.rpm", arch) - pathGlob := filepath.Join(depDirWithArch, rpmFileGlob) - paths, globErr := filepath.Glob(pathGlob) - if globErr != nil { - panic(fmt.Sprintf("Bad glob pattern %s: %s", pathGlob, globErr)) - } - if paths != nil { - depStatisfied = true - copyDestDir := filepath.Join(mockDepsDir, arch, dep) - pathMap[copyDestDir] = pathGlob - } + rpmFileGlob := fmt.Sprintf("*.%s.rpm", arch) + pathGlob := filepath.Join(depDirWithArch, rpmFileGlob) + paths, globErr := filepath.Glob(pathGlob) + if globErr != nil { + panic(fmt.Sprintf("Bad glob pattern %s: %s", pathGlob, globErr)) + } + if paths != nil { + depStatisfied = true + copyDestDir := filepath.Join(mockDepsDir, arch, dep) + pathMap[copyDestDir] = pathGlob } } if !depStatisfied { diff --git a/impl/mock_cfg.go b/impl/mock_cfg.go index 75fef2a8..3d7b32d7 100644 --- a/impl/mock_cfg.go +++ b/impl/mock_cfg.go @@ -147,10 +147,6 @@ func (cfgBldr *mockCfgBuilder) prep() error { for _, includeFile := range cfgBldr.buildSpec.Include { pkgDirInRepo := getPkgDirInRepo(cfgBldr.repo, cfgBldr.pkg, cfgBldr.isPkgSubdirInRepo) includeFilePath := filepath.Join(pkgDirInRepo, includeFile) - if err := util.CheckPath(includeFilePath, false, false); err != nil { - return fmt.Errorf("%sinclude file %s not found in repo", - cfgBldr.errPrefix, pkgDirInRepo) - } if err := util.CopyToDestDir( includeFilePath, mockCfgDir, cfgBldr.errPrefix); err != nil { return err diff --git a/util/util.go b/util/util.go index d96bfb89..23526e5f 100644 --- a/util/util.go +++ b/util/util.go @@ -74,23 +74,6 @@ func CheckOutput(name string, arg ...string) ( return string(output), nil } -// CheckPath checks if path exists. It also optionally checks if it is a directory, -// or if the path is writable -func CheckPath(path string, checkDir bool, checkWritable bool) error { - info, err := os.Stat(path) - if err != nil { - return err - } - if checkDir && !info.IsDir() { - return fmt.Errorf("%s is not a directory", path) - } - - if checkWritable && unix.Access(path, unix.W_OK) != nil { - return fmt.Errorf("%s is not writable", path) - } - return nil -} - // MaybeCreateDirWithParents creates a directory at dirPath if one // doesn't already exist. It also creates any parent directories. func MaybeCreateDirWithParents(dirPath string, executor executor.Executor, errPrefix ErrPrefix) error { @@ -119,9 +102,12 @@ func CopyToDestDir( destDir string, errPrefix ErrPrefix) error { - if err := CheckPath(destDir, true, true); err != nil { - return fmt.Errorf("%sDirectory %s should be present and writable: %s", - errPrefix, destDir, err) + info, err := os.Stat(destDir) + if err != nil || !info.IsDir() { + return fmt.Errorf("%s: Destination %s is not a valid directory: %w", errPrefix, destDir, err) + } + if unix.Access(destDir, unix.W_OK) != nil { + return fmt.Errorf("%sDirectory %s should be writable", errPrefix, destDir) } filesToCopy, patternErr := filepath.Glob(srcGlob)