add p4charset to config file for both source and destination servers

* also address some linter issues
* also errors start lowercase (hopefully doesn't break anyone's workflow?)

Author: danbrakeley

README.md

@@ -41,12 +41,14 @@ Here's an example `config.toml` file:
 [source]
 p4port = "ssl:perforce.example.com:1667"
 p4user = "user"
+p4charset = "none"
 p4client = "user-UE4-Release-Latest-Minimal" # this needs to exist before running p4harmonize
 
 # destination is the perforce server you want to update so that it matches the source
 [destination]
 p4port = "perforce.local:1666"
 p4user = "localuser"
+p4charset = "auto"
 new_client_name = "localuser-harmonize"   # this will be created by p4harmonize
 new_client_root = "d:/p4/local/harmonize" # this will be created by p4harmonize
 new_client_stream = "//test/engine_epic"  # this needs to already exist
@@ -110,11 +112,12 @@ Before opening a pull request, please run `mage longtest` and make sure it is pa
 
 If your PR includes performance improvements, please include some benchmarks that show the benefits of your changes.
 
-## Special Thanks!
+## Special Thanks
 
 Thanks to [Bohdon Sayre](https://github.com/bohdon) and [Jørgen P. Tjernø](https://github.com/jorgenpt) for contributing time and code to help me fix my bugs and dramatically improve performance!
 
-## TODO:
+## TODO
 
+- add unicode vs non-unicode servers to longtest
 - run longtest via github actions?
 - test on a Mac (maybe with github actions?)

cmd/longtest/logger.go

@@ -11,7 +11,7 @@ func createLongtestLogger(filename string) (close func() error, log frog.Logger,
 	// open file
 	f, err := os.Create(filename)
 	if err != nil {
-		return nil, nil, fmt.Errorf("Error creating log file %s: %w", filename, err)
+		return nil, nil, fmt.Errorf("error creating log file %s: %w", filename, err)
 	}
 
 	// console logger

cmd/longtest/main.go

@@ -16,10 +16,10 @@ import (
 )
 
 var Servers = []Server{
-	{Src, "1661", "super", "UE4", "Release-4.20", "./p4/1"},
-	{Src, "1662", "super", "UE4", "Release-4.20", "./p4/2"},
-	{Dst, "1663", "super", "test", "engine", "./p4/3"},
-	{Dst, "1664", "super", "test", "engine", "./p4/4"},
+	{Src, "1661", "super", "none", "UE4", "Release-4.20", "./p4/1"},
+	{Src, "1662", "super", "none", "UE4", "Release-4.20", "./p4/2"},
+	{Dst, "1663", "super", "none", "test", "engine", "./p4/3"},
+	{Dst, "1664", "super", "none", "test", "engine", "./p4/4"},
 }
 
 func main() {
@@ -124,15 +124,15 @@ func bringUpServers(logParent frog.Logger, Servers []Server) {
 			defer wg.Done()
 			close, sh := createBshWithTransientLogger(log)
 			defer close()
-			pf := p4.New(sh, s.Port(), s.User(), "")
+			pf := p4.New(sh, s.Port(), s.User(), s.Charset(), "")
 			if s.IsSrc() {
 				if err := setupSrc(sh, pf, s); err != nil {
 					log.Error("error in setupSrc", frog.Err(err))
 					return
 				}
 			} else {
 				if err := setupDst(sh, pf, s); err != nil {
-					log.Error("error in setupSrc", frog.Err(err))
+					log.Error("error in setupDst", frog.Err(err))
 					return
 				}
 			}
@@ -172,7 +172,7 @@ func runTwoServers(log frog.Logger, configSuffix int, src, dst Server, cl int64,
 	for i := 0; i < expectedRuns; i++ {
 
 		// dst's client must not already exist
-		if err := p4.New(sh, dst.Port(), dst.User(), "").DeleteClient(dst.Client()); err != nil {
+		if err := p4.New(sh, dst.Port(), dst.User(), dst.Charset(), "").DeleteClient(dst.Client()); err != nil {
 			return fmt.Errorf("error deleting client %s from %s: %w", dst.Client(), dst.Port(), err)
 		}
 		// dst's root folder must be empty
@@ -189,8 +189,8 @@ func runTwoServers(log frog.Logger, configSuffix int, src, dst Server, cl int64,
 			return fmt.Errorf("p4harmonize with config %s: %w", cfgName, err)
 		}
 
-		p4src := p4.New(sh, src.Port(), src.User(), src.Client())
-		p4dst := p4.New(sh, dst.Port(), dst.User(), dst.Client())
+		p4src := p4.New(sh, src.Port(), src.User(), src.Charset(), src.Client())
+		p4dst := p4.New(sh, dst.Port(), dst.User(), dst.Charset(), dst.Client())
 
 		// submit p4harmonize's changes
 		if err := p4dst.SubmitChangelist(cl); err != nil {
@@ -245,12 +245,6 @@ func buildDepotFilesLists(p4src, p4dst *p4.P4) (srcFiles, dstFiles string, err e
 		return sb.String(), nil
 	}
 
-	type result struct {
-		pf   *p4.P4
-		list string
-		err  error
-	}
-
 	var wg sync.WaitGroup
 	wg.Add(2)
 
@@ -269,13 +263,13 @@ func buildDepotFilesLists(p4src, p4dst *p4.P4) (srcFiles, dstFiles string, err e
 	wg.Wait()
 
 	if srcErr != nil && dstErr != nil {
-		return "", "", fmt.Errorf("Errors listing depot files from both %s and %s: %v; %v", p4src.Port, p4dst.Port, srcErr, dstErr)
+		return "", "", fmt.Errorf("errors listing depot files from both %s and %s: %v; %v", p4src.Port, p4dst.Port, srcErr, dstErr)
 	}
 	if srcErr != nil {
-		return "", "", fmt.Errorf("Error listing depot files from %s: %w", p4src.Port, srcErr)
+		return "", "", fmt.Errorf("error listing depot files from %s: %w", p4src.Port, srcErr)
 	}
 	if dstErr != nil {
-		return "", "", fmt.Errorf("Error listing depot files from %s: %w", p4dst.Port, dstErr)
+		return "", "", fmt.Errorf("error listing depot files from %s: %w", p4dst.Port, dstErr)
 	}
 
 	return srcList, dstList, nil

cmd/longtest/server.go

@@ -12,12 +12,13 @@ const (
 )
 
 type Server struct {
-	t      ServerType
-	port   string
-	user   string
-	depot  string
-	stream string
-	root   string
+	t       ServerType
+	port    string
+	user    string
+	charset string
+	depot   string
+	stream  string
+	root    string
 }
 
 func (s *Server) IsSrc() bool {
@@ -32,6 +33,10 @@ func (s *Server) Port() string {
 	return s.port
 }
 
+func (s *Server) Charset() string {
+	return s.charset
+}
+
 func (s *Server) User() string {
 	return s.user
 }

cmd/p4harmonize/main.go

@@ -95,7 +95,7 @@ func mainExit() int {
 	start := time.Now()
 	log, close := MakeLogger(frog.New(frog.Auto, frog.POLevel(false), frog.POFieldsLeftMsgRight, frog.POFieldIndent(10)))
 	defer func() {
-		dur := time.Now().Sub(start)
+		dur := time.Since(start)
 		log.Info("Running Time: %v", dur)
 		close()
 	}()

cmd/p4harmonize/update.go

@@ -49,7 +49,7 @@ func Harmonize(log Logger, cfg config.Config) error {
 
 	logDst := log.Dst()
 	shDst := MakeLoggingBsh(logDst)
-	p4dst := p4.New(shDst, cfg.Dst.P4Port, cfg.Dst.P4User, "")
+	p4dst := p4.New(shDst, cfg.Dst.P4Port, cfg.Dst.P4User, cfg.Dst.P4Charset, "")
 
 	logDst.Info("Retrieving info for server %s", p4dst.DisplayName())
 	info, err := p4dst.Info()
@@ -94,7 +94,7 @@ func Harmonize(log Logger, cfg config.Config) error {
 	// block until sync source sync completes
 	srcRes := <-chSrc
 	chSrc = nil
-	if srcRes.Success != true {
+	if !srcRes.Success {
 		return fmt.Errorf("error syncing from source server")
 	}
 
@@ -273,7 +273,7 @@ func preFlightChecks(log Logger, cfg config.Config) bool {
 
 	logSrc := log.Src()
 	shSrc := MakeLoggingBsh(logSrc)
-	p4src := p4.New(shSrc, cfg.Src.P4Port, cfg.Src.P4User, "")
+	p4src := p4.New(shSrc, cfg.Src.P4Port, cfg.Src.P4User, cfg.Src.P4Charset, "")
 
 	if needsLogin, err := p4src.NeedsLogin(); err != nil {
 		logSrc.Error("Error checking login status on %s: %v", p4src.Port, err)
@@ -285,7 +285,7 @@ func preFlightChecks(log Logger, cfg config.Config) bool {
 
 	logDst := log.Dst()
 	shDst := MakeLoggingBsh(logDst)
-	p4dst := p4.New(shDst, cfg.Dst.P4Port, cfg.Dst.P4User, "")
+	p4dst := p4.New(shDst, cfg.Dst.P4Port, cfg.Dst.P4User, cfg.Dst.P4Charset, "")
 
 	if needsLogin, err := p4dst.NeedsLogin(); err != nil {
 		logDst.Error("Error checking login status on %s: %v", p4dst.Port, err)
@@ -329,7 +329,7 @@ func preFlightChecks(log Logger, cfg config.Config) bool {
 // srcSyncAndList connects to the source perforce server, syncs to head, then
 // requests a list of all file names and types.
 func srcSyncAndList(logSrc Logger, shSrc *bsh.Bsh, cfg config.Config) srcThreadResults {
-	p4src := p4.New(shSrc, cfg.Src.P4Port, cfg.Src.P4User, cfg.Src.P4Client)
+	p4src := p4.New(shSrc, cfg.Src.P4Port, cfg.Src.P4User, cfg.Src.P4Charset, cfg.Src.P4Client)
 
 	spec, err := p4src.GetClientSpec()
 	if err != nil {
@@ -495,7 +495,7 @@ func PerforceFileCopy(src, dst, filetype string) error {
 func verifyAndCopy(srcPath, dstPath string) error {
 	srcInfo, err := os.Stat(srcPath)
 	if err != nil {
-		return fmt.Errorf("Unable to stat '%s': %w", srcPath, err)
+		return fmt.Errorf("unable to stat '%s': %w", srcPath, err)
 	}
 
 	if !srcInfo.Mode().IsRegular() {
@@ -505,26 +505,26 @@ func verifyAndCopy(srcPath, dstPath string) error {
 
 	dstDir := filepath.Dir(dstPath)
 	if err := os.MkdirAll(dstDir, os.ModePerm); err != nil {
-		return fmt.Errorf("Unable to mkdir '%s': %w", dstDir, err)
+		return fmt.Errorf("unable to mkdir '%s': %w", dstDir, err)
 	}
 
 	s, err := os.Open(srcPath)
 	if err != nil {
-		return fmt.Errorf("Unable to open '%s': %w", srcPath, err)
+		return fmt.Errorf("unable to open '%s': %w", srcPath, err)
 	}
 	defer s.Close()
 
 	d, err := os.Create(dstPath)
 	if err != nil {
-		return fmt.Errorf("Unable to create '%s': %w", dstPath, err)
+		return fmt.Errorf("unable to create '%s': %w", dstPath, err)
 	}
 	defer d.Close()
 	n, err := io.Copy(d, s)
 	if err != nil {
-		return fmt.Errorf("Unable to copy '%s' to '%s': %w", srcPath, dstPath, err)
+		return fmt.Errorf("unable to copy '%s' to '%s': %w", srcPath, dstPath, err)
 	}
 	if n != srcSize {
-		return fmt.Errorf("Expected '%s' to copy %d bytes to '%s', but only %d were copied", srcPath, n, dstPath, srcSize)
+		return fmt.Errorf("expected '%s' to copy %d bytes to '%s', but only %d were copied", srcPath, n, dstPath, srcSize)
 	}
 	return nil
 }

internal/config/config.go

@@ -10,14 +10,16 @@ import (
 )
 
 type Source struct {
-	P4Port   string `toml:"p4port"`
-	P4User   string `toml:"p4user"`
-	P4Client string `toml:"p4client"`
+	P4Port    string `toml:"p4port"`
+	P4User    string `toml:"p4user"`
+	P4Charset string `toml:"p4charset"`
+	P4Client  string `toml:"p4client"`
 }
 
 type Destination struct {
 	P4Port       string `toml:"p4port"`
 	P4User       string `toml:"p4user"`
+	P4Charset    string `toml:"p4charset"`
 	ClientName   string `toml:"new_client_name"`
 	ClientRoot   string `toml:"new_client_root"`
 	ClientStream string `toml:"new_client_stream"`
@@ -38,11 +40,11 @@ func (c *Config) Filename() string {
 func (c *Config) WriteToFile(path string) error {
 	f, err := os.Create(path)
 	if err != nil {
-		return fmt.Errorf("Error opening '%s': %w", path, err)
+		return fmt.Errorf("error opening '%s': %w", path, err)
 	}
 
 	if err := toml.NewEncoder(f).Encode(c); err != nil {
-		return fmt.Errorf("Error encoding/writing '%s': %w", path, err)
+		return fmt.Errorf("error encoding/writing '%s': %w", path, err)
 	}
 
 	return nil
@@ -53,7 +55,7 @@ func (c *Config) WriteToFile(path string) error {
 func LoadFromFile(path string) (Config, error) {
 	f, err := os.Open(path)
 	if err != nil {
-		return Config{}, fmt.Errorf("Error opening '%s': %w", path, err)
+		return Config{}, fmt.Errorf("error opening '%s': %w", path, err)
 	}
 	defer f.Close()
 	cfg, err := loadConfig(f)

internal/p4/options.go

@@ -18,8 +18,8 @@ type oChangelist struct {
 	CL int64
 }
 
-func (_ oChangelist) isOption()      {}
-func (_ oChangelist) String() string { return "Changelist" }
+func (oChangelist) isOption()      {}
+func (oChangelist) String() string { return "Changelist" }
 
 // Type holds a filetype as a string
 
@@ -31,32 +31,32 @@ type oType struct {
 	Type string
 }
 
-func (_ oType) isOption()      {}
-func (_ oType) String() string { return "Type" }
+func (oType) isOption()      {}
+func (oType) String() string { return "Type" }
 
 // Keep means to keep local files on disk (don't make local changes, just update the server)
 
 var Keep oKeep
 
 type oKeep struct{}
 
-func (_ oKeep) isOption()      {}
-func (_ oKeep) String() string { return "Keep" }
+func (oKeep) isOption()      {}
+func (oKeep) String() string { return "Keep" }
 
 // Do not perform any ignore checking, i.e. ignore any settings specified by P4IGNORE.
 
 var DoNotIgnore oDoNotIgnore
 
 type oDoNotIgnore struct{}
 
-func (_ oDoNotIgnore) isOption()      {}
-func (_ oDoNotIgnore) String() string { return "DoNotIgnore" }
+func (oDoNotIgnore) isOption()      {}
+func (oDoNotIgnore) String() string { return "DoNotIgnore" }
 
 // Allow wildcards in file names (see [p4 add](https://www.perforce.com/manuals/cmdref/Content/CmdRef/p4_add.html))
 
 var AllowWildcards oAllowWildcards
 
 type oAllowWildcards struct{}
 
-func (_ oAllowWildcards) isOption()      {}
-func (_ oAllowWildcards) String() string { return "AllowWildcards" }
+func (oAllowWildcards) isOption()      {}
+func (oAllowWildcards) String() string { return "AllowWildcards" }