Repository: twpayne/chezmoi
Pull request #79 (2018-12-17 14:07:58)

Internal refactor


Patch:
diff --git a/cmd/add.go b/cmd/add.go
index 508d12435..09a8480f3 100644
--- a/cmd/add.go
+++ b/cmd/add.go
@@ -41,7 +41,7 @@ func (c *Config) runAddCommand(fs vfs.FS, command *cobra.Command, args []string)
 	info, err := fs.Stat(c.SourceDir)
 	switch {
 	case err == nil && info.Mode().IsDir():
-		if info.Mode()&os.ModePerm != 0700 {
+		if info.Mode().Perm() != 0700 {
 			if err := mutator.Chmod(c.SourceDir, 0700); err != nil {
 				return err
 			}
diff --git a/cmd/cmd.go b/cmd/cmd.go
index c6db52300..4ac2ed9de 100644
--- a/cmd/cmd.go
+++ b/cmd/cmd.go
@@ -32,7 +32,7 @@ type Config struct {
 	add              addCommandConfig
 	dump             dumpCommandConfig
 	edit             editCommandConfig
-	importC          importCommandConfig
+	_import          importCommandConfig
 	keyring          keyringCommandConfig
 }
 
diff --git a/cmd/import.go b/cmd/import.go
index 21def2ea3..e028ee78a 100644
--- a/cmd/import.go
+++ b/cmd/import.go
@@ -19,7 +19,7 @@ import (
 	vfs "github.com/twpayne/go-vfs"
 )
 
-var importCommand = &cobra.Command{
+var _importommand = &cobra.Command{
 	Use:   "import",
 	Args:  cobra.MaximumNArgs(1),
 	Short: "Import an archive",
@@ -33,12 +33,12 @@ type importCommandConfig struct {
 }
 
 func init() {
-	rootCommand.AddCommand(importCommand)
+	rootCommand.AddCommand(_importommand)
 
-	persistentFlags := importCommand.PersistentFlags()
-	persistentFlags.StringVarP(&config.importC.destination, "destination", "d", "", "destination prefix")
-	persistentFlags.IntVar(&config.importC.stripComponents, "strip-components", 0, "strip components")
-	persistentFlags.BoolVarP(&config.importC.removeDestination, "remove-destination", "r", false, "remove destination before import")
+	persistentFlags := _importommand.PersistentFlags()
+	persistentFlags.StringVarP(&config._import.destination, "destination", "d", "", "destination prefix")
+	persistentFlags.IntVar(&config._import.stripComponents, "strip-components", 0, "strip components")
+	persistentFlags.BoolVarP(&config._import.removeDestination, "remove-destination", "r", false, "remove destination before import")
 }
 
 func (c *Config) runImportCommand(fs vfs.FS, cmd *cobra.Command, args []string) error {
@@ -71,8 +71,8 @@ func (c *Config) runImportCommand(fs vfs.FS, cmd *cobra.Command, args []string)
 		}
 	}
 	mutator := c.getDefaultMutator(fs)
-	if c.importC.removeDestination {
-		entry, err := ts.Get(c.importC.destination)
+	if c._import.removeDestination {
+		entry, err := ts.Get(c._import.destination)
 		switch {
 		case err == nil:
 			if err := mutator.RemoveAll(filepath.Join(c.SourceDir, entry.SourceName())); err != nil {
@@ -83,5 +83,5 @@ func (c *Config) runImportCommand(fs vfs.FS, cmd *cobra.Command, args []string)
 			return err
 		}
 	}
-	return ts.Import(tar.NewReader(r), c.importC.destination, c.importC.stripComponents, mutator)
+	return ts.Import(tar.NewReader(r), c._import.destination, c._import.stripComponents, mutator)
 }
diff --git a/cmd/root.go b/cmd/root.go
index 90ae9cd6d..50adbf6c5 100644
--- a/cmd/root.go
+++ b/cmd/root.go
@@ -88,8 +88,8 @@ func (c *Config) persistentPreRunRootE(fs vfs.FS, command *cobra.Command, args [
 	switch {
 	case err == nil && !info.Mode().IsDir():
 		return fmt.Errorf("%s: not a directory", c.SourceDir)
-	case err == nil && info.Mode()&os.ModePerm != 0700:
-		fmt.Printf("%s: want permissions 0700, got 0%o\n", c.SourceDir, info.Mode()&os.ModePerm)
+	case err == nil && info.Mode().Perm() != 0700:
+		fmt.Printf("%s: want permissions 0700, got 0%o\n", c.SourceDir, info.Mode().Perm())
 	case os.IsNotExist(err):
 	default:
 		return err
diff --git a/lib/chezmoi/chezmoi.go b/lib/chezmoi/chezmoi.go
index 37b74e7d2..704cd6bb9 100644
--- a/lib/chezmoi/chezmoi.go
+++ b/lib/chezmoi/chezmoi.go
@@ -138,13 +138,13 @@ func (psfn ParsedSourceFileName) SourceFileName() string {
 	fileName := ""
 	switch psfn.Mode & os.ModeType {
 	case 0:
-		if psfn.Mode&os.ModePerm&os.FileMode(077) == os.FileMode(0) {
+		if psfn.Mode.Perm()&os.FileMode(077) == os.FileMode(0) {
 			fileName = privatePrefix
 		}
 		if psfn.Empty {
 			fileName += emptyPrefix
 		}
-		if psfn.Mode&os.ModePerm&os.FileMode(0111) != os.FileMode(0) {
+		if psfn.Mode.Perm()&os.FileMode(0111) != os.FileMode(0) {
 			fileName += executablePrefix
 		}
 	case os.ModeSymlink:
diff --git a/lib/chezmoi/dir.go b/lib/chezmoi/dir.go
index b976b2762..ba75fb9d9 100644
--- a/lib/chezmoi/dir.go
+++ b/lib/chezmoi/dir.go
@@ -40,7 +40,7 @@ func (d *Dir) Apply(fs vfs.FS, targetDir string, umask os.FileMode, mutator Muta
 	info, err := fs.Lstat(targetPath)
 	switch {
 	case err == nil && info.Mode().IsDir():
-		if info.Mode()&os.ModePerm != d.Perm&^umask {
+		if info.Mode().Perm() != d.Perm&^umask {
 			if err := mutator.Chmod(targetPath, d.Perm&^umask); err != nil {
 				return err
 			}
@@ -98,7 +98,7 @@ func (d *Dir) Evaluate() error {
 
 // Private returns true if d is private.
 func (d *Dir) Private() bool {
-	return d.Perm&os.ModePerm&077 == 0
+	return d.Perm&077 == 0
 }
 
 // SourceName implements Entry.SourceName.
@@ -116,7 +116,7 @@ func (d *Dir) archive(w *tar.Writer, headerTemplate *tar.Header, umask os.FileMo
 	header := *headerTemplate
 	header.Typeflag = tar.TypeDir
 	header.Name = d.targetName
-	header.Mode = int64(d.Perm & os.ModePerm &^ umask)
+	header.Mode = int64(d.Perm &^ umask)
 	if err := w.WriteHeader(&header); err != nil {
 		return err
 	}
diff --git a/lib/chezmoi/file.go b/lib/chezmoi/file.go
index 28473d646..bd0375bcb 100644
--- a/lib/chezmoi/file.go
+++ b/lib/chezmoi/file.go
@@ -52,7 +52,7 @@ func (f *File) Apply(fs vfs.FS, targetDir string, umask os.FileMode, mutator Mut
 		if !bytes.Equal(currData, contents) {
 			break
 		}
-		if info.Mode()&os.ModePerm != f.Perm&^umask {
+		if info.Mode().Perm() != f.Perm&^umask {
 			if err := mutator.Chmod(targetPath, f.Perm&^umask); err != nil {
 				return err
 			}
@@ -111,7 +111,7 @@ func (f *File) Executable() bool {
 
 // Private returns true if f is private.
 func (f *File) Private() bool {
-	return f.Perm&os.ModePerm&077 == 0
+	return f.Perm&077 == 0
 }
 
 // SourceName implements Entry.SourceName.
diff --git a/lib/chezmoi/symlink.go b/lib/chezmoi/symlink.go
index c5f6f94ce..e12636a6f 100644
--- a/lib/chezmoi/symlink.go
+++ b/lib/chezmoi/symlink.go
@@ -13,9 +13,9 @@ type Symlink struct {
 	sourceName       string
 	targetName       string
 	Template         bool
-	linkName         string
-	linkNameErr      error
-	evaluateLinkName func() (string, error)
+	linkname         string
+	linknameErr      error
+	evaluateLinkname func() (string, error)
 }
 
 type symlinkConcreteValue struct {
@@ -23,12 +23,12 @@ type symlinkConcreteValue struct {
 	SourcePath string `json:"sourcePath" yaml:"sourcePath"`
 	TargetPath string `json:"targetPath" yaml:"targetPath"`
 	Template   bool   `json:"template" yaml:"template"`
-	LinkName   string `json:"linkName" yaml:"linkName"`
+	Linkname   string `json:"linkname" yaml:"linkname"`
 }
 
 // Apply ensures that the state of s's target in fs matches s.
 func (s *Symlink) Apply(fs vfs.FS, targetDir string, umask os.FileMode, mutator Mutator) error {
-	target, err := s.LinkName()
+	target, err := s.Linkname()
 	if err != nil {
 		return err
 	}
@@ -53,7 +53,7 @@ func (s *Symlink) Apply(fs vfs.FS, targetDir string, umask os.FileMode, mutator
 
 // ConcreteValue implements Entry.ConcreteValue.
 func (s *Symlink) ConcreteValue(targetDir, sourceDir string, recursive bool) (interface{}, error) {
-	linkName, err := s.LinkName()
+	linkname, err := s.Linkname()
 	if err != nil {
 		return nil, err
 	}
@@ -62,13 +62,13 @@ func (s *Symlink) ConcreteValue(targetDir, sourceDir string, recursive bool) (in
 		SourcePath: filepath.Join(sourceDir, s.SourceName()),
 		TargetPath: filepath.Join(targetDir, s.TargetName()),
 		Template:   s.Template,
-		LinkName:   linkName,
+		Linkname:   linkname,
 	}, nil
 }
 
 // Evaluate evaluates s's target.
 func (s *Symlink) Evaluate() error {
-	_, err := s.LinkName()
+	_, err := s.Linkname()
 	return err
 }
 
@@ -77,13 +77,13 @@ func (s *Symlink) SourceName() string {
 	return s.sourceName
 }
 
-// LinkName returns s's link name.
-func (s *Symlink) LinkName() (string, error) {
-	if s.evaluateLinkName != nil {
-		s.linkName, s.linkNameErr = s.evaluateLinkName()
-		s.evaluateLinkName = nil
+// Linkname returns s's link name.
+func (s *Symlink) Linkname() (string, error) {
+	if s.evaluateLinkname != nil {
+		s.linkname, s.linknameErr = s.evaluateLinkname()
+		s.evaluateLinkname = nil
 	}
-	return s.linkName, s.linkNameErr
+	return s.linkname, s.linknameErr
 }
 
 // TargetName implements Entry.TargetName.
@@ -93,13 +93,13 @@ func (s *Symlink) TargetName() string {
 
 // archive writes s to w.
 func (s *Symlink) archive(w *tar.Writer, headerTemplate *tar.Header, umask os.FileMode) error {
-	linkName, err := s.LinkName()
+	linkname, err := s.Linkname()
 	if err != nil {
 		return err
 	}
 	header := *headerTemplate
 	header.Name = s.targetName
 	header.Typeflag = tar.TypeSymlink
-	header.Linkname = linkName
+	header.Linkname = linkname
 	return w.WriteHeader(&header)
 }
diff --git a/lib/chezmoi/target_state.go b/lib/chezmoi/target_state.go
index 2fe7e4a8b..fcd1886e4 100644
--- a/lib/chezmoi/target_state.go
+++ b/lib/chezmoi/target_state.go
@@ -80,105 +80,36 @@ func (ts *TargetState) Add(fs vfs.FS, targetPath string, info os.FileInfo, addEm
 		entries = parentDir.Entries
 	}
 
-	name := filepath.Base(targetName)
 	switch {
 	case info.Mode().IsDir():
-		if entry, ok := entries[name]; ok {
-			if _, ok := entry.(*Dir); !ok {
-				return fmt.Errorf("%s: already added and not a directory", targetName)
-			}
-			return nil // entry already exists
-		}
-		sourceName := ParsedSourceDirName{
-			DirName: name,
-			Perm:    info.Mode() & os.ModePerm,
-		}.SourceDirName()
-		if parentDirSourceName != "" {
-			sourceName = filepath.Join(parentDirSourceName, sourceName)
-		}
-		if err := mutator.Mkdir(filepath.Join(ts.SourceDir, sourceName), 0777&^ts.Umask); err != nil {
-			return err
-		}
-		// If the directory is empty, add a .keep file so the directory is
-		// managed by git. Chezmoi will ignore the .keep file as it begins with
-		// a dot.
+		perm := info.Mode().Perm()
 		infos, err := fs.ReadDir(targetPath)
 		if err != nil {
 			return err
 		}
-		if len(infos) == 0 {
-			if err := mutator.WriteFile(filepath.Join(ts.SourceDir, sourceName, ".keep"), nil, 0666&^ts.Umask, nil); err != nil {
-				return err
-			}
-		}
-		entries[name] = newDir(sourceName, targetName, info.Mode()&os.ModePerm)
+		empty := len(infos) == 0
+		return ts.addDir(targetName, entries, parentDirSourceName, perm, empty, mutator)
 	case info.Mode().IsRegular():
-		if entry, ok := entries[name]; ok {
-			if _, ok := entry.(*File); !ok {
-				return fmt.Errorf("%s: already added and not a regular file", targetName)
-			}
-			return nil // entry already exists
-		}
 		if info.Size() == 0 && !addEmpty {
 			return nil
 		}
-		sourceName := ParsedSourceFileName{
-			FileName: name,
-			Mode:     info.Mode() & os.ModePerm,
-			Empty:    info.Size() == 0,
-			Template: addTemplate,
-		}.SourceFileName()
-		if parentDirSourceName != "" {
-			sourceName = filepath.Join(parentDirSourceName, sourceName)
-		}
-		data, err := fs.ReadFile(targetPath)
+		contents, err := fs.ReadFile(targetPath)
 		if err != nil {
 			return err
 		}
 		if addTemplate {
-			data = autoTemplate(data, ts.Data)
-		}
-		if err := mutator.WriteFile(filepath.Join(ts.SourceDir, sourceName), data, 0666&^ts.Umask, nil); err != nil {
-			return err
-		}
-		entries[name] = &File{
-			sourceName: sourceName,
-			targetName: targetName,
-			Empty:      len(data) == 0,
-			Perm:       info.Mode() & os.ModePerm,
-			Template:   addTemplate,
-			contents:   data,
+			contents = autoTemplate(contents, ts.Data)
 		}
+		return ts.addFile(targetName, entries, parentDirSourceName, info, addTemplate, contents, mutator)
 	case info.Mode()&os.ModeType == os.ModeSymlink:
-		if entry, ok := entries[name]; ok {
-			if _, ok := entry.(*Symlink); !ok {
-				return fmt.Errorf("%s: already added and not a symlink", targetName)
-			}
-			return nil // entry already exists
-		}
-		sourceName := ParsedSourceFileName{
-			FileName: name,
-			Mode:     os.ModeSymlink,
-		}.SourceFileName()
-		if parentDirSourceName != "" {
-			sourceName = filepath.Join(parentDirSourceName, sourceName)
-		}
-		data, err := fs.Readlink(targetPath)
+		linkname, err := fs.Readlink(targetPath)
 		if err != nil {
 			return err
 		}
-		if err := mutator.WriteFile(filepath.Join(ts.SourceDir, sourceName), []byte(data), 0666&^ts.Umask, nil); err != nil {
-			return err
-		}
-		entries[name] = &Symlink{
-			sourceName: sourceName,
-			targetName: targetName,
-			linkName:   data,
-		}
+		return ts.addSymlink(targetName, entries, parentDirSourceName, linkname, mutator)
 	default:
-		return fmt.Errorf("%s: not a regular file or directory", targetName)
+		return fmt.Errorf("%s: not a regular file, directory, or symlink", targetName)
 	}
-	return nil
 }
 
 // Archive writes ts to w.
@@ -336,17 +267,17 @@ func (ts *TargetState) Populate(fs vfs.FS) error {
 					sourceName:       relPath,
 					targetName:       targetName,
 					Empty:            psfp.Empty,
-					Perm:             psfp.Mode & os.ModePerm,
+					Perm:             psfp.Mode.Perm(),
 					Template:         psfp.Template,
 					evaluateContents: evaluateContents,
 				}
 			case os.ModeSymlink:
-				evaluateLinkName := func() (string, error) {
+				evaluateLinkname := func() (string, error) {
 					data, err := fs.ReadFile(path)
 					return string(data), err
 				}
 				if psfp.Template {
-					evaluateLinkName = func() (string, error) {
+					evaluateLinkname = func() (string, error) {
 						data, err := ts.executeTemplate(fs, path)
 						return string(data), err
 					}
@@ -355,7 +286,7 @@ func (ts *TargetState) Populate(fs vfs.FS) error {
 					sourceName:       relPath,
 					targetName:       targetName,
 					Template:         psfp.Template,
-					evaluateLinkName: evaluateLinkName,
+					evaluateLinkname: evaluateLinkname,
 				}
 			default:
 				return fmt.Errorf("%v: unsupported mode: %d", path, psfp.Mode&os.ModeType)
@@ -368,6 +299,132 @@ func (ts *TargetState) Populate(fs vfs.FS) error {
 	})
 }
 
+func (ts *TargetState) addDir(targetName string, entries map[string]Entry, parentDirSourceName string, perm os.FileMode, empty bool, mutator Mutator) error {
+	name := filepath.Base(targetName)
+	var existingDir *Dir
+	if entry, ok := entries[name]; ok {
+		existingDir, ok = entry.(*Dir)
+		if !ok {
+			return fmt.Errorf("%s: already added and not a directory", targetName)
+		}
+	}
+	sourceName := ParsedSourceDirName{
+		DirName: name,
+		Perm:    perm,
+	}.SourceDirName()
+	if parentDirSourceName != "" {
+		sourceName = filepath.Join(parentDirSourceName, sourceName)
+	}
+	dir := newDir(sourceName, targetName, perm)
+	if existingDir != nil {
+		if existingDir.sourceName == dir.sourceName {
+			return nil
+		}
+		return mutator.Rename(filepath.Join(ts.SourceDir, existingDir.sourceName), filepath.Join(ts.SourceDir, dir.sourceName))
+	}
+	// If the directory is empty, add a .keep file so the directory is
+	// managed by git. Chezmoi will ignore the .keep file as it begins with
+	// a dot.
+	if empty {
+		if err := mutator.WriteFile(filepath.Join(ts.SourceDir, sourceName, ".keep"), nil, 0666&^ts.Umask, nil); err != nil {
+			return err
+		}
+	}
+	entries[name] = dir
+	return mutator.Mkdir(filepath.Join(ts.SourceDir, sourceName), 0777&^ts.Umask)
+}
+
+func (ts *TargetState) addFile(targetName string, entries map[string]Entry, parentDirSourceName string, info os.FileInfo, template bool, contents []byte, mutator Mutator) error {
+	name := filepath.Base(targetName)
+	var existingFile *File
+	var existingContents []byte
+	if entry, ok := entries[name]; ok {
+		existingFile, ok = entry.(*File)
+		if !ok {
+			return fmt.Errorf("%s: already added and not a regular file", targetName)
+		}
+		var err error
+		existingContents, err = existingFile.Contents()
+		if err != nil {
+			return err
+		}
+	}
+	perm := info.Mode().Perm()
+	empty := info.Size() == 0
+	sourceName := ParsedSourceFileName{
+		FileName: name,
+		Mode:     perm,
+		Empty:    empty,
+		Template: template,
+	}.SourceFileName()
+	if parentDirSourceName != "" {
+		sourceName = filepath.Join(parentDirSourceName, sourceName)
+	}
+	file := &File{
+		sourceName: sourceName,
+		targetName: targetName,
+		Empty:      empty,
+		Perm:       perm,
+		Template:   template,
+		contents:   contents,
+	}
+	if existingFile != nil {
+		if bytes.Equal(existingFile.contents, file.contents) {
+			if existingFile.sourceName == file.sourceName {
+				return nil
+			}
+			return mutator.Rename(filepath.Join(ts.SourceDir, existingFile.sourceName), filepath.Join(ts.SourceDir, file.sourceName))
+		}
+		if err := mutator.RemoveAll(filepath.Join(ts.SourceDir, existingFile.sourceName)); err != nil {
+			return err
+		}
+	}
+	entries[name] = file
+	return mutator.WriteFile(filepath.Join(ts.SourceDir, sourceName), contents, 0666&^ts.Umask, existingContents)
+}
+
+func (ts *TargetState) addSymlink(targetName string, entries map[string]Entry, parentDirSourceName string, linkname string, mutator Mutator) error {
+	name := filepath.Base(targetName)
+	var existingSymlink *Symlink
+	var existingLinkname string
+	if entry, ok := entries[name]; ok {
+		existingSymlink, ok = entry.(*Symlink)
+		if !ok {
+			return fmt.Errorf("%s: already added and not a symlink", targetName)
+		}
+		var err error
+		existingLinkname, err = existingSymlink.Linkname()
+		if err != nil {
+			return err
+		}
+	}
+	sourceName := ParsedSourceFileName{
+		FileName: name,
+		Mode:     os.ModeSymlink,
+	}.SourceFileName()
+	if parentDirSourceName != "" {
+		sourceName = filepath.Join(parentDirSourceName, sourceName)
+	}
+	symlink := &Symlink{
+		sourceName: sourceName,
+		targetName: targetName,
+		linkname:   linkname,
+	}
+	if existingSymlink != nil {
+		if existingSymlink.linkname == symlink.linkname {
+			if existingSymlink.sourceName == symlink.sourceName {
+				return nil
+			}
+			return mutator.Rename(filepath.Join(ts.SourceDir, existingSymlink.sourceName), filepath.Join(ts.SourceDir, symlink.sourceName))
+		}
+		if err := mutator.RemoveAll(filepath.Join(ts.SourceDir, existingSymlink.sourceName)); err != nil {
+			return err
+		}
+	}
+	entries[name] = symlink
+	return mutator.WriteFile(filepath.Join(ts.SourceDir, symlink.sourceName), []byte(symlink.linkname), 0666&^ts.Umask, []byte(existingLinkname))
+}
+
 func (ts *TargetState) executeTemplate(fs vfs.FS, path string) ([]byte, error) {
 	data, err := fs.ReadFile(path)
 	if err != nil {
@@ -448,120 +505,21 @@ func (ts *TargetState) importHeader(r io.Reader, header *tar.Header, destination
 		parentDirSourceName = parentDir.sourceName
 		entries = parentDir.Entries
 	}
-	name := filepath.Base(targetName)
 	switch header.Typeflag {
 	case tar.TypeDir:
-		var existingDir *Dir
-		if entry, ok := entries[name]; ok {
-			existingDir, ok = entry.(*Dir)
-			if !ok {
-				return fmt.Errorf("%s: already added and not a directory", targetName)
-			}
-		}
-		perm := os.FileMode(header.Mode) & os.ModePerm
-		sourceName := ParsedSourceDirName{
-			DirName: name,
-			Perm:    perm,
-		}.SourceDirName()
-		if parentDirSourceName != "" {
-			sourceName = filepath.Join(parentDirSourceName, sourceName)
-		}
-		dir := newDir(sourceName, targetName, perm)
-		if existingDir != nil {
-			if existingDir.sourceName == dir.sourceName {
-				return nil
-			}
-			return mutator.Rename(filepath.Join(ts.SourceDir, existingDir.sourceName), filepath.Join(ts.SourceDir, dir.sourceName))
-		}
-		// FIXME Add a .keep file if the directory is empty
-		entries[name] = dir
-		return mutator.Mkdir(filepath.Join(ts.SourceDir, sourceName), 0777&^ts.Umask)
+		perm := os.FileMode(header.Mode).Perm()
+		empty := false // FIXME don't assume directory is empty
+		return ts.addDir(targetName, entries, parentDirSourceName, perm, empty, mutator)
 	case tar.TypeReg:
-		var existingFile *File
-		var existingContents []byte
-		if entry, ok := entries[name]; ok {
-			existingFile, ok = entry.(*File)
-			if !ok {
-				return fmt.Errorf("%s: already added and not a regular file", targetName)
-			}
-			existingContents, err = existingFile.Contents()
-			if err != nil {
-				return err
-			}
-		}
-		perm := os.FileMode(header.Mode) & os.ModePerm
-		empty := header.Size == 0
-		sourceName := ParsedSourceFileName{
-			FileName: name,
-			Mode:     perm,
-			Empty:    empty,
-		}.SourceFileName()
-		if parentDirSourceName != "" {
-			sourceName = filepath.Join(parentDirSourceName, sourceName)
-		}
+		info := header.FileInfo()
 		contents, err := ioutil.ReadAll(r)
 		if err != nil {
 			return err
 		}
-		file := &File{
-			sourceName: sourceName,
-			targetName: targetName,
-			Empty:      empty,
-			Perm:       perm,
-			Template:   false,
-			contents:   contents,
-		}
-		if existingFile != nil {
-			if bytes.Equal(existingFile.contents, file.contents) {
-				if existingFile.sourceName == file.sourceName {
-					return nil
-				}
-				return mutator.Rename(filepath.Join(ts.SourceDir, existingFile.sourceName), filepath.Join(ts.SourceDir, file.sourceName))
-			}
-			if err := mutator.RemoveAll(filepath.Join(ts.SourceDir, existingFile.sourceName)); err != nil {
-				return err
-			}
-		}
-		entries[name] = file
-		return mutator.WriteFile(filepath.Join(ts.SourceDir, sourceName), contents, 0666&^ts.Umask, existingContents)
+		return ts.addFile(targetName, entries, parentDirSourceName, info, false, contents, mutator)
 	case tar.TypeSymlink:
-		var existingSymlink *Symlink
-		var existingLinkName string
-		if entry, ok := entries[name]; ok {
-			existingSymlink, ok = entry.(*Symlink)
-			if !ok {
-				return fmt.Errorf("%s: already added and not a symlink", targetName)
-			}
-			existingLinkName, err = existingSymlink.LinkName()
-			if err != nil {
-				return err
-			}
-		}
-		sourceName := ParsedSourceFileName{
-			FileName: name,
-			Mode:     os.ModeSymlink,
-		}.SourceFileName()
-		if parentDirSourceName != "" {
-			sourceName = filepath.Join(parentDirSourceName, sourceName)
-		}
-		symlink := &Symlink{
-			sourceName: sourceName,
-			targetName: targetName,
-			linkName:   header.Linkname,
-		}
-		if existingSymlink != nil {
-			if existingSymlink.linkName == symlink.linkName {
-				if existingSymlink.sourceName == symlink.sourceName {
-					return nil
-				}
-				return mutator.Rename(filepath.Join(ts.SourceDir, existingSymlink.sourceName), filepath.Join(ts.SourceDir, symlink.sourceName))
-			}
-			if err := mutator.RemoveAll(filepath.Join(ts.SourceDir, existingSymlink.sourceName)); err != nil {
-				return err
-			}
-		}
-		entries[name] = symlink
-		return mutator.WriteFile(filepath.Join(ts.SourceDir, symlink.sourceName), []byte(symlink.linkName), 0666&^ts.Umask, []byte(existingLinkName))
+		linkname := header.Linkname
+		return ts.addSymlink(targetName, entries, parentDirSourceName, linkname, mutator)
 	default:
 		return fmt.Errorf("%s: unspported typeflag '%c'", header.Name, header.Typeflag)
 	}


Test patch:
diff --git a/cmd/add_test.go b/cmd/add_test.go
index 561d3979b..8cc2ef867 100644
--- a/cmd/add_test.go
+++ b/cmd/add_test.go
@@ -21,8 +21,14 @@ func TestAddCommand(t *testing.T) {
 				"/home/jenkins/.bashrc": "foo",
 			},
 			tests: []vfst.Test{
-				vfst.TestPath("/home/jenkins/.chezmoi", vfst.TestIsDir, vfst.TestModePerm(0700)),
-				vfst.TestPath("/home/jenkins/.chezmoi/dot_bashrc", vfst.TestModeIsRegular, vfst.TestContentsString("foo")),
+				vfst.TestPath("/home/jenkins/.chezmoi",
+					vfst.TestIsDir,
+					vfst.TestModePerm(0700),
+				),
+				vfst.TestPath("/home/jenkins/.chezmoi/dot_bashrc",
+					vfst.TestModeIsRegular,
+					vfst.TestContentsString("foo"),
+				),
 			},
 		},
 		{
@@ -37,7 +43,10 @@ func TestAddCommand(t *testing.T) {
 				"/home/jenkins/.gitconfig": "[user]\n\tname = John Smith\n\temail = john.smith@company.com\n",
 			},
 			tests: []vfst.Test{
-				vfst.TestPath("/home/jenkins/.chezmoi/dot_gitconfig.tmpl", vfst.TestModeIsRegular, vfst.TestContentsString("[user]\n\tname = {{ .name }}\n\temail = {{ .email }}\n")),
+				vfst.TestPath("/home/jenkins/.chezmoi/dot_gitconfig.tmpl",
+					vfst.TestModeIsRegular,
+					vfst.TestContentsString("[user]\n\tname = {{ .name }}\n\temail = {{ .email }}\n"),
+				),
 			},
 		},
 		{
@@ -52,7 +61,10 @@ func TestAddCommand(t *testing.T) {
 				"/home/jenkins/.config/micro/settings.json": "{}",
 			},
 			tests: []vfst.Test{
-				vfst.TestPath("/home/jenkins/.chezmoi/dot_config/micro/settings.json", vfst.TestModeIsRegular, vfst.TestContentsString("{}")),
+				vfst.TestPath("/home/jenkins/.chezmoi/dot_config/micro/settings.json",
+					vfst.TestModeIsRegular,
+					vfst.TestContentsString("{}"),
+				),
 			},
 		},
 		{
@@ -64,7 +76,10 @@ func TestAddCommand(t *testing.T) {
 				"/home/jenkins/.config/micro/settings.json": "{}",
 			},
 			tests: []vfst.Test{
-				vfst.TestPath("/home/jenkins/.chezmoi/dot_config/micro/settings.json", vfst.TestModeIsRegular, vfst.TestContentsString("{}")),
+				vfst.TestPath("/home/jenkins/.chezmoi/dot_config/micro/settings.json",
+					vfst.TestModeIsRegular,
+					vfst.TestContentsString("{}"),
+				),
 			},
 		},
 		{
@@ -79,7 +94,10 @@ func TestAddCommand(t *testing.T) {
 				"/home/jenkins/empty":    "",
 			},
 			tests: []vfst.Test{
-				vfst.TestPath("/home/jenkins/.chezmoi/empty_empty", vfst.TestModeIsRegular, vfst.TestContents(nil)),
+				vfst.TestPath("/home/jenkins/.chezmoi/empty_empty",
+					vfst.TestModeIsRegular,
+					vfst.TestContents(nil),
+				),
 			},
 		},
 		{
@@ -91,7 +109,10 @@ func TestAddCommand(t *testing.T) {
 				"/home/jenkins/foo":      &vfst.Symlink{Target: "bar"},
 			},
 			tests: []vfst.Test{
-				vfst.TestPath("/home/jenkins/.chezmoi/symlink_foo", vfst.TestModeIsRegular, vfst.TestContentsString("bar")),
+				vfst.TestPath("/home/jenkins/.chezmoi/symlink_foo",
+					vfst.TestModeIsRegular,
+					vfst.TestContentsString("bar"),
+				),
 			},
 		},
 		{
@@ -106,8 +127,13 @@ func TestAddCommand(t *testing.T) {
 				"/home/jenkins/foo/bar":  &vfst.Symlink{Target: "baz"},
 			},
 			tests: []vfst.Test{
-				vfst.TestPath("/home/jenkins/.chezmoi/foo", vfst.TestIsDir),
-				vfst.TestPath("/home/jenkins/.chezmoi/foo/symlink_bar", vfst.TestModeIsRegular, vfst.TestContentsString("baz")),
+				vfst.TestPath("/home/jenkins/.chezmoi/foo",
+					vfst.TestIsDir,
+				),
+				vfst.TestPath("/home/jenkins/.chezmoi/foo/symlink_bar",
+					vfst.TestModeIsRegular,
+					vfst.TestContentsString("baz"),
+				),
 			},
 		},
 		{
@@ -119,9 +145,16 @@ func TestAddCommand(t *testing.T) {
 				"/home/jenkins/foo/bar/baz": &vfst.Symlink{Target: "qux"},
 			},
 			tests: []vfst.Test{
-				vfst.TestPath("/home/jenkins/.chezmoi/foo", vfst.TestIsDir),
-				vfst.TestPath("/home/jenkins/.chezmoi/foo/bar", vfst.TestIsDir),
-				vfst.TestPath("/home/jenkins/.chezmoi/foo/bar/symlink_baz", vfst.TestModeIsRegular, vfst.TestContentsString("qux")),
+				vfst.TestPath("/home/jenkins/.chezmoi/foo",
+					vfst.TestIsDir,
+				),
+				vfst.TestPath("/home/jenkins/.chezmoi/foo/bar",
+					vfst.TestIsDir,
+				),
+				vfst.TestPath("/home/jenkins/.chezmoi/foo/bar/symlink_baz",
+					vfst.TestModeIsRegular,
+					vfst.TestContentsString("qux"),
+				),
 			},
 		},
 	} {
@@ -151,3 +184,44 @@ func TestAddCommand(t *testing.T) {
 		})
 	}
 }
+
+func TestAddAfterModification(t *testing.T) {
+	c := &Config{
+		SourceDir: "/home/jenkins/.chezmoi",
+		TargetDir: "/home/jenkins",
+		Umask:     022,
+		DryRun:    false,
+		Verbose:   true,
+	}
+	fs, cleanup, err := vfst.NewTestFS(map[string]interface{}{
+		"/home/jenkins":          &vfst.Dir{Perm: 0755},
+		"/home/jenkins/.chezmoi": &vfst.Dir{Perm: 0700},
+		"/home/jenkins/.bashrc":  "# contents of .bashrc\n",
+	})
+	defer cleanup()
+	if err != nil {
+		t.Fatalf("vfst.NewTestFS(_) == _, _, %v, want _, _, <nil>", err)
+	}
+	args := []string{"/home/jenkins/.bashrc"}
+	if err := c.runAddCommand(fs, nil, args); err != nil {
+		t.Errorf("c.runAddCommand(fs, nil, %+v) == %v, want <nil>", args, err)
+	}
+	vfst.RunTests(t, fs, "",
+		vfst.TestPath("/home/jenkins/.chezmoi/dot_bashrc",
+			vfst.TestModeIsRegular,
+			vfst.TestContentsString("# contents of .bashrc\n"),
+		),
+	)
+	if err := fs.WriteFile("/home/jenkins/.bashrc", []byte("# new contents of .bashrc\n"), 0644); err != nil {
+		t.Errorf("fs.WriteFile(...) == %v, want <nil>", err)
+	}
+	if err := c.runAddCommand(fs, nil, args); err != nil {
+		t.Errorf("c.runAddCommand(fs, nil, %+v) == %v, want <nil>", args, err)
+	}
+	vfst.RunTests(t, fs, "",
+		vfst.TestPath("/home/jenkins/.chezmoi/dot_bashrc",
+			vfst.TestModeIsRegular,
+			vfst.TestContentsString("# new contents of .bashrc\n"),
+		),
+	)
+}
diff --git a/lib/chezmoi/chezmoi_test.go b/lib/chezmoi/chezmoi_test.go
index fee9d46f7..fdd8a7e66 100644
--- a/lib/chezmoi/chezmoi_test.go
+++ b/lib/chezmoi/chezmoi_test.go
@@ -332,7 +332,7 @@ func TestTargetStatePopulate(t *testing.T) {
 					"foo": &Symlink{
 						sourceName: "symlink_foo",
 						targetName: "foo",
-						linkName:   "bar",
+						linkname:   "bar",
 					},
 				},
 			},
@@ -351,7 +351,7 @@ func TestTargetStatePopulate(t *testing.T) {
 					".foo": &Symlink{
 						sourceName: "symlink_dot_foo",
 						targetName: ".foo",
-						linkName:   "bar",
+						linkname:   "bar",
 					},
 				},
 			},
@@ -377,7 +377,7 @@ func TestTargetStatePopulate(t *testing.T) {
 						sourceName: "symlink_foo.tmpl",
 						targetName: "foo",
 						Template:   true,
-						linkName:   "bar-example.com",
+						linkname:   "bar-example.com",
 					},
 				},
 			},