Skip to content

Commit

Permalink
Use os.{Lstat,ReadDir} instead of os.Stat and ioutil.ReadDir (#180)
Browse files Browse the repository at this point in the history
Signed-off-by: subham sarkar <[email protected]>
  • Loading branch information
shmsr authored Jun 7, 2022
1 parent 43613ca commit 5248e9e
Show file tree
Hide file tree
Showing 6 changed files with 28 additions and 19 deletions.
23 changes: 16 additions & 7 deletions pkg/fileutils/fileutils.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package fileutils
import (
"fmt"
"io"
"io/ioutil"
"io/fs"
"os"
"path/filepath"
)
Expand Down Expand Up @@ -44,7 +44,7 @@ func CopyFile(src, dst string, changeMode bool) (err error) {
return
}

si, err := os.Stat(src)
si, err := os.Lstat(src)
if err != nil {
return
}
Expand All @@ -63,15 +63,15 @@ func CopyDir(src, dst string, changeMode bool) (err error) {
src = filepath.Clean(src)
dst = filepath.Clean(dst)

si, err := os.Stat(src)
si, err := os.Lstat(src)
if err != nil {
return err
}
if !si.IsDir() {
return fmt.Errorf("source is not a directory")
}

_, err = os.Stat(dst)
_, err = os.Lstat(dst)
if err != nil && !os.IsNotExist(err) {
return
}
Expand All @@ -84,11 +84,19 @@ func CopyDir(src, dst string, changeMode bool) (err error) {
return
}

entries, err := ioutil.ReadDir(src)
// NOTE: ioutil.ReadDir -> os.ReadDir as the latter is better:
// """
// As of Go 1.16, os.ReadDir is a more efficient and correct choice:
// it returns a list of fs.DirEntry instead of fs.FileInfo,
// and it returns partial results in the case of an error
// midway through reading a directory.
// """
entries, err := os.ReadDir(src)
if err != nil {
return
}

var fileInfo fs.FileInfo
for _, entry := range entries {
srcPath := filepath.Join(src, entry.Name())
dstPath := filepath.Join(dst, entry.Name())
Expand All @@ -100,7 +108,8 @@ func CopyDir(src, dst string, changeMode bool) (err error) {
}
} else {
// Skip symlinks.
if entry.Mode()&os.ModeSymlink != 0 {
fileInfo, err = entry.Info()
if err != nil || fileInfo.Mode()&os.ModeSymlink != 0 {
continue
}

Expand All @@ -116,7 +125,7 @@ func CopyDir(src, dst string, changeMode bool) (err error) {

// CheckIfExists checks if file or directory exists in the given path.
func CheckIfExists(path string) (bool, error) {
if _, err := os.Stat(path); err != nil {
if _, err := os.Lstat(path); err != nil {
if os.IsNotExist(err) {
return false, nil
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/secret/secret.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func New(logger lumber.Logger) core.SecretParser {
// GetRepoSecret read repo secrets from given path
func (s *secretParser) GetRepoSecret(path string) (map[string]string, error) {
var secretData map[string]string
if _, err := os.Stat(path); os.IsNotExist(err) {
if _, err := os.Lstat(path); os.IsNotExist(err) {
s.logger.Debugf("failed to find user env secrets in path %s, as path does not exists", path)
return nil, nil
}
Expand All @@ -53,7 +53,7 @@ func (s *secretParser) GetOauthSecret(path string) (*core.Oauth, error) {
o := &core.Oauth{
Type: core.Bearer,
}
if _, err := os.Stat(path); os.IsNotExist(err) {
if _, err := os.Lstat(path); os.IsNotExist(err) {
s.logger.Errorf("failed to find oauth secret in path %s", path)
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/secrets/secrets_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ func TestWriteGitSecrets(t *testing.T) {
if err != nil {
t.Errorf("error while writing secrets: %v", err)
}
if _, errS := os.Stat(expectedFile); errS != nil {
if _, errS := os.Lstat(expectedFile); errS != nil {
t.Errorf("could not find the git config file: %v", errS)
}

Expand Down
14 changes: 7 additions & 7 deletions pkg/service/coverage/coverage.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func New(execManager core.ExecutionManager,
if !cfg.CoverageMode {
return nil, nil
}
if _, err := os.Stat(global.CodeCoverageDir); os.IsNotExist(err) {
if _, err := os.Lstat(global.CodeCoverageDir); os.IsNotExist(err) {
return nil, errors.New("coverage directory not mounted")
}
return &codeCoverageService{
Expand All @@ -71,7 +71,7 @@ func New(execManager core.ExecutionManager,

//mergeCodeCoverageFiles merge all the coverage.json into single entity
func (c *codeCoverageService) mergeCodeCoverageFiles(ctx context.Context, commitDir, coverageManifestPath string, threshold bool) error {
if _, err := os.Stat(commitDir); os.IsNotExist(err) {
if _, err := os.Lstat(commitDir); os.IsNotExist(err) {
c.logger.Errorf("coverage files not found, skipping merge")
return nil
}
Expand Down Expand Up @@ -125,7 +125,7 @@ func (c *codeCoverageService) MergeAndUpload(ctx context.Context, payload *core.
commitDir := filepath.Join(repoDir, commit.Sha)
c.logger.Debugf("commit directory %s", commitDir)

if _, err := os.Stat(commitDir); os.IsNotExist(err) {
if _, err := os.Lstat(commitDir); os.IsNotExist(err) {
c.logger.Errorf("code coverage directory not found commit id %s", commit.Sha)
return err
}
Expand Down Expand Up @@ -202,7 +202,7 @@ func (c *codeCoverageService) uploadFile(ctx context.Context, blobPath, filename

func (c *codeCoverageService) parseManifestFile(filepath string) (core.CoverageManifest, error) {
manifestPayload := core.CoverageManifest{}
if _, err := os.Stat(filepath); os.IsNotExist(err) {
if _, err := os.Lstat(filepath); os.IsNotExist(err) {
c.logger.Errorf("manifest file not found in path %s", filepath)
return manifestPayload, err
}
Expand Down Expand Up @@ -268,7 +268,7 @@ func (c *codeCoverageService) downloadAndDecompressParentCommitDir(ctx context.C
}

func (c *codeCoverageService) copyFromParentCommitDir(parentCommitDir, commitDir string, removedFiles ...string) error {
if _, err := os.Stat(parentCommitDir); os.IsNotExist(err) {
if _, err := os.Lstat(parentCommitDir); os.IsNotExist(err) {
c.logger.Errorf("Parent Commit Directory %s not found", parentCommitDir)
return err
}
Expand All @@ -288,7 +288,7 @@ func (c *codeCoverageService) copyFromParentCommitDir(parentCommitDir, commitDir

//TODO: check if copied dir size is not 0
//if file already exists then don't copy from parent directory
if _, err := os.Stat(testfileDir); os.IsNotExist(err) {
if _, err := os.Lstat(testfileDir); os.IsNotExist(err) {
if err := fileutils.CopyDir(path, testfileDir, false); err != nil {
c.logger.Errorf("failed to copy directory from src %s to dest %s, error %v", path, testfileDir, err)
return err
Expand Down Expand Up @@ -376,7 +376,7 @@ func (c *codeCoverageService) sendCoverageData(payload []coverageData) error {
}

func (c *codeCoverageService) getTotalCoverage(filepath string) (json.RawMessage, error) {
if _, err := os.Stat(filepath); os.IsNotExist(err) {
if _, err := os.Lstat(filepath); os.IsNotExist(err) {
c.logger.Errorf("coverage summary file not found in path %s", filepath)
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func InterfaceToMap(in interface{}) map[string]string {

// CreateDirectory creates directory recursively if does not exists
func CreateDirectory(path string) error {
if _, err := os.Stat(path); os.IsNotExist(err) {
if _, err := os.Lstat(path); os.IsNotExist(err) {
if err := os.MkdirAll(path, global.DirectoryPermissions); err != nil {
return errs.ERR_DIR_CRT(err.Error())
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/utils/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ func TestCreateDirectory(t *testing.T) {
return
}
if tt.args.path == newDir {
if _, err := os.Stat(newDir); err != nil {
if _, err := os.Lstat(newDir); err != nil {
t.Errorf("Directory did not exist, error: %v", err)
return
}
Expand Down

0 comments on commit 5248e9e

Please sign in to comment.