commands,creds,lfs,tools: use error join function

In a previous commit in this PR we replaced the Combine() function
in our custom "errors" package with a Join() function that passes
its arguments through to the Join() function of the Go standard
library's "errors" package.  Both these Join() functions and our
earlier Combine() function serve to concatenate multiple error
values into a single error value, with the individual error messages
separated by newline characters when the error is converted to a
string value for output.

In a number of instances we perform the same concatenation of error
messages using loops and assembling the final error message by
explicitly appending each individual error message along with the
newline separator characters.

We can now simplify all these instances where we need to concatenate
error messages by making use of the Join() function in our "errors"
package.  In almost all of these cases, this means we are able to
eliminate the conditional logic used to check if a newline separator
character is required because more than one error has been encountered.
(The exception is in the Fill() method of the CredentialHelpers
structure of our "creds" package where we previously concatenated
the error messages using the Join() method of the "strings" package
from the Go standard library.)

Note that we could also have used the Combine() function from our
"errors" package to simplify these use cases; however, the Join()
function is more convenient in most instances because it accepts
variadic arguments rather than requiring an array of errors as input.

Author: chrisd8088

commands/command_fetch.go

@@ -2,11 +2,11 @@ package commands
 
 import (
 	"encoding/json"
-	"fmt"
 	"os"
 	"sync"
 	"time"
 
+	"github.com/git-lfs/git-lfs/v3/errors"
 	"github.com/git-lfs/git-lfs/v3/filepathfilter"
 	"github.com/git-lfs/git-lfs/v3/git"
 	"github.com/git-lfs/git-lfs/v3/lfs"
@@ -184,11 +184,7 @@ func pointersToFetchForRef(ref string, filter *filepathfilter.Filter) ([]*lfs.Wr
 	var multiErr error
 	tempgitscanner := lfs.NewGitScanner(cfg, func(p *lfs.WrappedPointer, err error) {
 		if err != nil {
-			if multiErr != nil {
-				multiErr = fmt.Errorf("%v\n%v", multiErr, err)
-			} else {
-				multiErr = err
-			}
+			multiErr = errors.Join(multiErr, err)
 			return
 		}
 
@@ -227,11 +223,7 @@ func pointersToFetchForRefs(refs []string) ([]*lfs.WrappedPointer, error) {
 	var numObjs int64
 	tempgitscanner := lfs.NewGitScanner(cfg, func(p *lfs.WrappedPointer, err error) {
 		if err != nil {
-			if multiErr != nil {
-				multiErr = fmt.Errorf("%v\n%v", multiErr, err)
-			} else {
-				multiErr = err
-			}
+			multiErr = errors.Join(multiErr, err)
 			return
 		}
 
@@ -362,11 +354,7 @@ func scanAll() []*lfs.WrappedPointer {
 	var numObjs int64
 	tempgitscanner := lfs.NewGitScanner(cfg, func(p *lfs.WrappedPointer, err error) {
 		if err != nil {
-			if multiErr != nil {
-				multiErr = fmt.Errorf("%v\n%v", multiErr, err)
-			} else {
-				multiErr = err
-			}
+			multiErr = errors.Join(multiErr, err)
 			return
 		}
 

commands/uploader.go

@@ -1,7 +1,6 @@
 package commands
 
 import (
-	"fmt"
 	"io"
 	"net/url"
 	"os"
@@ -139,11 +138,7 @@ func (c *uploadContext) addScannerError(err error) {
 	c.errMu.Lock()
 	defer c.errMu.Unlock()
 
-	if c.scannerErr != nil {
-		c.scannerErr = fmt.Errorf("%v\n%v", c.scannerErr, err)
-	} else {
-		c.scannerErr = err
-	}
+	c.scannerErr = errors.Join(c.scannerErr, err)
 }
 
 func (c *uploadContext) buildGitScanner() *lfs.GitScanner {

creds/creds.go

@@ -500,7 +500,7 @@ var credHelperNoOp = errors.New("no-op!")
 // helpers are added to the skip list, and never attempted again for the
 // lifetime of the current Git LFS command.
 func (s *CredentialHelpers) Fill(what Creds) (Creds, error) {
-	errs := make([]string, 0, len(s.helpers))
+	var multiErr error
 	for i, h := range s.helpers {
 		if s.skipped(i) {
 			continue
@@ -511,7 +511,7 @@ func (s *CredentialHelpers) Fill(what Creds) (Creds, error) {
 			if err != credHelperNoOp {
 				s.skip(i)
 				tracerx.Printf("credential fill error: %s", err)
-				errs = append(errs, err.Error())
+				multiErr = errors.Join(multiErr, err)
 			}
 			continue
 		}
@@ -521,11 +521,11 @@ func (s *CredentialHelpers) Fill(what Creds) (Creds, error) {
 		}
 	}
 
-	if len(errs) > 0 {
-		return nil, errors.New(tr.Tr.Get("credential fill errors:\n%s", strings.Join(errs, "\n")))
+	if multiErr != nil {
+		multiErr = errors.Join(errors.New(tr.Tr.Get("credential fill errors:")), multiErr)
 	}
 
-	return nil, nil
+	return nil, multiErr
 }
 
 // Reject implements CredentialHelper.Reject and rejects the given Creds "what"

lfs/gitfilter_smudge.go

@@ -123,16 +123,7 @@ func (f *GitFilter) downloadFile(writer io.Writer, ptr *Pointer, workingfile, me
 	q.Wait()
 
 	if errs := q.Errors(); len(errs) > 0 {
-		var multiErr error
-		for _, e := range errs {
-			if multiErr != nil {
-				multiErr = fmt.Errorf("%v\n%v", multiErr, e)
-			} else {
-				multiErr = e
-			}
-		}
-
-		return 0, errors.Wrap(multiErr, tr.Tr.Get("Error downloading %s (%s)", workingfile, ptr.Oid))
+		return 0, errors.Wrap(errors.Join(errs...), tr.Tr.Get("Error downloading %s (%s)", workingfile, ptr.Oid))
 	}
 
 	return f.readLocalFile(writer, ptr, mediafile, workingfile, nil)
@@ -155,15 +146,7 @@ func (f *GitFilter) downloadFileFallBack(writer io.Writer, ptr *Pointer, working
 		q.Wait()
 
 		if errs := q.Errors(); len(errs) > 0 {
-			var multiErr error
-			for _, e := range errs {
-				if multiErr != nil {
-					multiErr = fmt.Errorf("%v\n%v", multiErr, e)
-				} else {
-					multiErr = e
-				}
-			}
-			wrappedError := errors.Wrap(multiErr, tr.Tr.Get("Error downloading %s (%s)", workingfile, ptr.Oid))
+			wrappedError := errors.Wrap(errors.Join(errs...), tr.Tr.Get("Error downloading %s (%s)", workingfile, ptr.Oid))
 			if index >= len(remotes)-1 {
 				return 0, wrappedError
 			} else {

lfs/scanner_git_test.go

@@ -5,12 +5,12 @@ package lfs_test // to avoid import cycles
 // which avoids import cycles with testutils
 
 import (
-	"fmt"
 	"sort"
 	"testing"
 	"time"
 
 	"github.com/git-lfs/git-lfs/v3/config"
+	"github.com/git-lfs/git-lfs/v3/errors"
 	. "github.com/git-lfs/git-lfs/v3/lfs"
 	test "github.com/git-lfs/git-lfs/v3/t/cmd/util"
 	"github.com/stretchr/testify/assert"
@@ -94,11 +94,7 @@ func scanUnpushed(remoteName string) ([]*WrappedPointer, error) {
 
 	gitscanner := NewGitScanner(config.New(), func(p *WrappedPointer, err error) {
 		if err != nil {
-			if multiErr != nil {
-				multiErr = fmt.Errorf("%v\n%v", multiErr, err)
-			} else {
-				multiErr = err
-			}
+			multiErr = errors.Join(multiErr, err)
 			return
 		}
 

tools/channels.go

@@ -1,6 +1,6 @@
 package tools
 
-import "fmt"
+import "github.com/git-lfs/git-lfs/v3/errors"
 
 // Interface for all types of wrapper around a channel of results and an error channel
 // Implementors will expose a type-specific channel for results
@@ -17,18 +17,13 @@ type BaseChannelWrapper struct {
 }
 
 func (w *BaseChannelWrapper) Wait() error {
-	var err error
-	for e := range w.errorChan {
-		if err != nil {
-			// Combine in case multiple errors
-			err = fmt.Errorf("%v\n%v", err, e)
-
-		} else {
-			err = e
-		}
+	var multiErr error
+	for err := range w.errorChan {
+		// Combine in case multiple errors
+		multiErr = errors.Join(multiErr, err)
 	}
 
-	return err
+	return multiErr
 }
 
 func NewBaseChannelWrapper(errChan <-chan error) *BaseChannelWrapper {