Pre-existing files should be treated as locally modified

[dnnr]

Very happy chezmoi user here, so thanks for this amazing tool, first of all. 🙏

After about half a year of using chezmoi, one thing I'm still struggling a bit with is how to operate chezmoi in a defensive manner. I'd like to propose a small change to address this, which would also make chezmoi's behavior more consistent in general.

In short: When chezmoi apply encounters a target file that it has not written previously, it should prompt in the same way as it already does if the target file has been modified since chezmoi last wrote it.

Currently, chezmoi apply has two mechanisms against clobbering target state:

1. Prompt for every single modification (if --interactive is given)
2. Prompt if a target file has been modified since the last write

The crucial issue here is that these two mechanisms are mutually exclusive: The second kind of prompt is effectively shadowed by "always prompting anyway". The downside of this is that users of --interactive never get to learn which of the changes they are being prompted for are very likely to be safe because the existing target file exactly matches what was previously been written by chezmoi. At the same time, they cannot drop --interactive lest they risk clobbering target state whenever a new file is added to the source directory.

Now this could be tackled from two angles:

• apply --interactive could provide more information about whether it has detected local modifications or an existing file it didn't write itself
• apply could get a more complete definition of "don't clobber stuff" by treating pre-existing files like locally modified ones

I'd consider both approaches to be worthwhile improvements (independently from each other), but I'm focusing on the second one here because it would scratch my particular itch: I don't even want to use --interactive, I just want to be more certain that chezmoi apply doesn't clobber something that I couldn't later "restore" by looking at the history of my source repository. Right now, --interactive is the only way to get this, but for every prompt I have to manually try to remember if this particular diff is simply the change I've just pulled, or if it clobbers some temporary/experimental/forgotten local modification that I just didn't yet integrate into my source repository.

For the sake of completeness, I can think of one argument to be made against my suggestion: It's technically a breaking change, negating user expectations (based on past behavior, not based on intuition) that when freshly deploying chezmoi, the very first apply will just work, no questions asked. I personally think that has never been a good thing anyway, but I'm prepared to hear other opinions. Maybe we could to introduce a new command line flag then?

I could only find one previous discussion related to this, which didn't consider the issue of pre-existing files.

I'm not a seasoned Go developer, but if my proposal resonates with you, I'm happy to give it at shot and contribute the change.

[twpayne]

Thank you for your kind words and for opening this discussion.

A few quick thoughts:

• Any changes should not break existing users' workflows. Specifically, running chezmoi apply with no arguments on a new machine should continue to overwrite dotfiles without any warning. I, and I suspect many other people as well, rely on this behavior, especially when installing your dotfiles in temporary environments like Docker containers or temporary VMs. This means that any changes to chezmoi's behavior here should either use a new command line flag or change chezmoi's behavior when interacting with the user.
• My assumption was that people who cared about the initial contents of a dotfile would use either chezmoi diff or chezmoi apply --interactive to review changes before making them.

Currently chezmoi apply --interactive should only prompt for files that chezmoi would change. Is there a case where it doesn't do this?

[dnnr]

I appreciate your care for not breaking existing workflows. I mistakenly thought that we're still on 0.x.x, so there would be some wiggle room for breaking changes. But clearly, changing the behavior of apply like this should (if at all) only happen in a 3.0.0 release. I'd be totally fine with introducing this new behavior behind a flag instead.

Currently chezmoi apply --interactive should only prompt for files that chezmoi would change. Is there a case where it doesn't do this?

This might be a slight misunderstanding. I haven't seen apply --interactive prompt for files it wouldn't change. My issue is with the fact that it prompts for every single file it is about to write. Looking into the target directory, those files-about-to-be-changed fall into one of these categories:

1. Was written by chezmoi before and still looks exactly like that in-target
2. Was written by chezmoi before but has been modified since then
3. Exists but has never been written by chezmoi
4. Doesn't exist yet

Right now, chezmoi either treats all these cases the same (with apply --interactive which prompts for every write) or it clumps together 1+3+4 (with apply, which only prompts for 2). I would like to be prompted only for 2+3, which are the only cases where something actually gets clobbered that I could probably only restore from backup. (That's assuming that I only run apply on a clean, non-dirty source repository, so I have a record of past source states.)

My assumption was that people who cared about the initial contents of a dotfile would use either chezmoi diff or chezmoi apply --interactive to review changes before making them.

True, and that's what I do, but this workflow suffers from the same issue as later apply runs: You get bothered with prompts for entirely non-destructive actions (category 4) along with prompts that are actually worth reviewing (category 3).

As a comparison, my proposal matches how many system package managers deal with files in /etc during updates: If they see that a file has not been modified since the package was installed, they silently replace it with the new version. If there's a different file, they ask the user what to do. In other words, they prompt for 2+3 only. Prompting for 1 or 4 would result in magnitudes more prompts that nobody wants. Not prompting at all would be downright catastrophic.

[twpayne]

Thank you, what you say makes a lot of sense.

Would you be willing to write, either in Go or any other language (or psuedocode) the logic that you would like chezmoi to follow so that you're only prompted for 2+3?

The exact code that implements the current behavior and would need to be changed is:

chezmoi/internal/cmd/config.go

Lines 1056 to 1158 in 57705ba

	// defaultPreApplyFunc is the default pre-apply function. If the target entry
	// has changed since chezmoi last wrote it then it prompts the user for the
	// action to take.
	func (c *Config) defaultPreApplyFunc(
	targetRelPath chezmoi.RelPath,
	targetEntryState, lastWrittenEntryState, actualEntryState *chezmoi.EntryState,
	) error {
	c.logger.Info("defaultPreApplyFunc",
	chezmoilog.Stringer("targetRelPath", targetRelPath),
	slog.Any("targetEntryState", targetEntryState),
	slog.Any("lastWrittenEntryState", lastWrittenEntryState),
	slog.Any("actualEntryState", actualEntryState),
	)
	switch {
	case c.force:
	return nil
	case targetEntryState.Equivalent(actualEntryState):
	return nil
	}
	if c.Interactive {
	prompt := fmt.Sprintf("Apply %s", targetRelPath)
	var choices []string
	actualContents := actualEntryState.Contents()
	targetContents := targetEntryState.Contents()
	if actualContents != nil || targetContents != nil {
	choices = append(choices, "diff")
	}
	choices = append(choices, choicesYesNoAllQuit...)
	for {
	switch choice, err := c.promptChoice(prompt, choices); {
	case err != nil:
	return err
	case choice == "diff":
	err := c.diffFile(
	targetRelPath,
	c.DestDirAbsPath.Join(targetRelPath), actualContents, actualEntryState.Mode,
	chezmoi.EmptyAbsPath, targetContents, targetEntryState.Mode,
	)
	if err != nil {
	return err
	}
	case choice == "yes":
	return nil
	case choice == "no":
	return fs.SkipDir
	case choice == "all":
	c.Interactive = false
	return nil
	case choice == "quit":
	return chezmoi.ExitCodeError(1)
	default:
	panic(choice + ": unexpected choice")
	}
	}
	}
	switch {
	case targetEntryState.Overwrite():
	return nil
	case targetEntryState.Type == chezmoi.EntryStateTypeScript:
	return nil
	case lastWrittenEntryState == nil:
	return nil
	case lastWrittenEntryState.Equivalent(actualEntryState):
	return nil
	}
	prompt := fmt.Sprintf("%s has changed since chezmoi last wrote it", targetRelPath)
	var choices []string
	actualContents := actualEntryState.Contents()
	targetContents := targetEntryState.Contents()
	if actualContents != nil || targetContents != nil {
	choices = append(choices, "diff")
	}
	choices = append(choices, "overwrite", "all-overwrite", "skip", "quit")
	for {
	switch choice, err := c.promptChoice(prompt, choices); {
	case err != nil:
	return err
	case choice == "diff":
	if err := c.diffFile(
	targetRelPath,
	c.DestDirAbsPath.Join(targetRelPath), actualContents, actualEntryState.Mode,
	chezmoi.EmptyAbsPath, targetContents, targetEntryState.Mode,
	); err != nil {
	return err
	}
	case choice == "overwrite":
	return nil
	case choice == "all-overwrite":
	c.force = true
	return nil
	case choice == "skip":
	return fs.SkipDir
	case choice == "quit":
	return chezmoi.ExitCodeError(0)
	default:
	panic(choice + ": unexpected choice")
	}
	}
	}

[dnnr]

Thanks for the starting point, I'll gladly give it a go!

[dnnr]

I've opened #4678. The name of the flag is of course a suggestion at best. Maybe something like --interactive-conflicts is a better fit?