Diff sometimes(?) broken

VorpalBlade commented 1 year ago

Describe the bug

chezmoi diff/ chezmoi apply -v display incorrect diffs sometimes. Actually applying does do the right thing however.

This has been going on for a little while, but at first I thought it was my modify script doing something wrong. Now I have seen the same issue for plain chezmoi-managed files without scripts or templates as well.

To reproduce

Download the attached archive with common.sh.sys and common.sh.chezmoi: files.tar.gz
The former file goes into your home directory, the latter in your chezmoi source directory (removing the final extension obviously so they match up).
Then run chezmoi diff.

index 556c41f3677cd444e0ebd5574fdf45bd83ad2973..26f22a12a5caa1f201f173e5035a7f833648f226 100644
--- a/.config/local/common.sh
+++ b/.config/local/common.sh
@@ -204,9 +204,11 @@     fi
 fi

 # $1: Environment variable
-# Load host specific values from chezmoi if available (not available when using ssh teleportation)
+    declare -A chezmoi_custom
     add_path 1 "/sbin"
     add_path 1 "/usr/sbin"
 fi

 unset _hostname
+
+[[ -d "$HOME/.cargo/bin" ]] && add_path 1 "$HOME/.cargo/bin"

The first diff is completely wrong, and copies lines from earlier in the file.

Expected behavior

What normal diff produces:

diff -Naur ~/.config/local/common.sh ~ch/private_dot_config/local/common.sh 
--- /home/arvid/.config/local/common.sh 2022-10-28 13:32:46.046484415 +0200
+++ /home/arvid/.local/share/chezmoi/private_dot_config/local/common.sh 2023-01-24 12:51:25.256985416 +0100
@@ -203,9 +203,11 @@
     fi
 fi

-if [[ $_hostname == vorpal ]]; then
+if [[ $_hostname == vorpal || $_hostname == cerberus || $_hostname == hydra ]]; then
     add_path 1 "/sbin"
     add_path 1 "/usr/sbin"
 fi

 unset _hostname
+
+[[ -d "$HOME/.cargo/bin" ]] && add_path 1 "$HOME/.cargo/bin"

Output of command with the `--verbose` flag

$ chezmoi --verbose diff ~/.config/local/common.sh                                                                                                                       36s
diff --git a/.config/local/common.sh b/.config/local/common.sh
index 556c41f3677cd444e0ebd5574fdf45bd83ad2973..26f22a12a5caa1f201f173e5035a7f833648f226 100644
--- a/.config/local/common.sh
+++ b/.config/local/common.sh
@@ -204,9 +204,11 @@     fi
 fi

 # $1: Environment variable
-# Load host specific values from chezmoi if available (not available when using ssh teleportation)
+    declare -A chezmoi_custom
     add_path 1 "/sbin"
     add_path 1 "/usr/sbin"
 fi

 unset _hostname
+
+[[ -d "$HOME/.cargo/bin" ]] && add_path 1 "$HOME/.cargo/bin"

Output of `chezmoi doctor`

```console $ chezmoi doctor RESULT CHECK MESSAGE warning version v2.29.3, built at 2023-01-19T23:30:23Z ok latest-version v2.29.3 ok os-arch linux/amd64 (Arch Linux) ok uname Linux minerva 5.15.89-1-lts #1 SMP Wed, 18 Jan 2023 11:24:46 +0000 x86_64 GNU/Linux ok go-version go1.19.5 (gc) ok executable /usr/bin/chezmoi ok config-file ~/.config/chezmoi/chezmoi.yaml, last modified 2023-01-20T14:23:12+01:00 ok source-dir ~/.local/share/chezmoi is a git working tree (clean) warning suspicious-entries ~/.local/share/chezmoi/.chezmoi_modify_manager.add_hook ok working-tree ~/.local/share/chezmoi is a git working tree (clean) ok dest-dir ~ is a directory ok umask 022 ok cd-command found /bin/zsh ok cd-args /bin/zsh info diff-command not set ok edit-command found /usr/bin/nano ok edit-args /usr/bin/nano ok git-command found /usr/bin/git, version 2.39.1 ok merge-command found /usr/bin/bcompare ok shell-command found /bin/zsh ok shell-args /bin/zsh info age-command age not found in $PATH ok gpg-command found /usr/bin/gpg, version 2.2.40 info pinentry-command not set info 1password-command op not found in $PATH info bitwarden-command bw not found in $PATH info gopass-command gopass not found in $PATH ok keepassxc-command found /usr/bin/keepassxc-cli, version 2.7.4 info keepassxc-db not set info keeper-command keeper not found in $PATH info lastpass-command lpass not found in $PATH info pass-command pass not found in $PATH info passhole-command ph not found in $PATH info vault-command vault not found in $PATH info secret-command not set ```

Additional context

This chezmoi is packaged by Arch Linux. I also tried the binary from github. Exactly the same result.
I have not tried this outside my normal dotfiles repo, but I don't know what else could be in there that could influence things.
I have observed the issue on multiple computers.

michelschinz commented 1 year ago

In case this helps, I have the very same problem (on macOS 13.1, although I doubt it has any impact), and was about to open a new issue when I noticed this one. The problem seems to appear each time a diff is shown, I've also observed it when doing chezmoi re-add --interactive and then hitting d to get the diff.

VorpalBlade commented 1 year ago

Out of curiosity I decided to run chezmoi under valgrind to see if it would report any sort of memory error when printing a diff.

There are lots (invalid reads and writes mostly)! But they also show up for chezmoi doctor, chezmoi --help etc. Most are attributed to runtime or various upstream libraries though. Or perhaps go is incompatible with valgrind?

There should probably be a separate bug opened about that.

bradenhilton commented 1 year ago

Perhaps this is already fixed by https://github.com/twpayne/chezmoi/pull/2707.

I also tried the binary from github.

Is this referring to the release binary? If so, could you possibly test on a local build of the latest commit?

VorpalBlade commented 1 year ago

@bradenhilton It was the release yes.

I'm not set up (or have the knowledge) to build Go code. I use C++/Python/Rust for the most part.

twpayne commented 1 year ago

As @bradenhilton says, #2707 should fix it. I'm pretty sure this is a duplicate of #2706.

You can download a build from the latest master here.

Re valgrind, feel free to open an issue but it sounds like valgrind and Go do not support each other. Go is a memory safe language and doesn't do anything particularly weird (although the BoltDB library chezmoi uses does do some funky memory protection stuff).

VorpalBlade commented 1 year ago

You can download a build from the latest master here.

Yes, that fixes it.

Re valgrind, feel free to open an issue but it sounds like valgrind and Go do not support each other. Go is a memory safe language and doesn't do anything particularly weird (although the BoltDB library chezmoi uses does do some funky memory protection stuff).

It seems to indeed be incompatible based on some googling. That seems quite inconvenient if you are doing FFI with a C library or similar though. All of the errors happens before the first line of output from chezmoi (during static constructors perhaps? If that is a thing in Go).

twpayne commented 1 year ago

Great, thanks for testing. I've just tagged v2.29.4 which includes the fix (rollback to sergi/go-diff 1.1.0). The release with the fix should be available in about 45 minutes.

twpayne / chezmoi