[Bug]: Automatic commit issues empty commits
jmschaal opened this issue · 7 comments
Describe the bug
The .git/COMMIT_EDITMSG as well as some .git/objects files are (sometimes) changed when the automatic commit fires, even if there have been no changes in the vault.
The .git/COMMIT_EDITMSG file will read for example
vault backup: 2024-10-01 02:55:25
Affected files:
"git reflog" does not show any activity, git seems to ignore the empty commit.
Relevant errors (if available) from notifications or console (CTRL+SHIFT+I)
The obsidian console prints
plugin:obsidian-git:314 obsidian-git: Committed 0 filesInterestingly, in the empty vault I set up in order to reproduce the error, obsidian-git will output "Committed 0 files" or "No changes to commit" randomly.
In my big vault the automatic commit will always commit zero files.
Steps to reproduce
- Create a new obsidian vault.
- Create a git repository in the vault folder
git init. - Create a commit using
git add .andgit commit -m "initial commit". - Enable Community plugins, install the Git plugin and enable it.
- Set the Auto commit-and-sync interval to a non zero value ( e.g. 1).
- Disable Push and Pull on commit-and-sync.
- Don't change any file in the vault and wait for the automatic commit to occur.
Expected Behavior
If no files are changed in the git repository, no commit should be issued. This seems to work only some times.
Addition context
The error seems to stem from this line
if (changedFiles.length !== 0 || hadConflict) {...
I would try to debug this further, but i have never really used ts, so i dont know how to build the plugin from source. (Maybe add a small part to the README on how to help development?)
This might seem like completely benign behavior, however it unfortunately represents a big problem for me.
My obsidian vault is located in a folder synchronized to a nextcloud server.
Since one of the files changed by the empty commit is a 600MB big .pack file, every time the automatic commit is triggered, 600MB of data are uploaded.
Additionally, since nextcloud saves old versions of files, the servers storage is completely filled within hours.
Operating system
Linux
Installation Method
Other
Plugin version
2.27.0
I figured out how to build the plugin.
The changedFiles object is not empty, and the offending file is .obsidian/plugins/obsidian-git/data.json.
I have added
if (
changedFiles.length === 1 &&
changedFiles[0].path == ".obsidian/plugins/obsidian-git/data.json"
) {
changedFiles = [];
}
in front of the if condition mentioned in the bug report as a quick and dirty fix, which seems to be working so far.
I could not detect any changes in the mentioned file, so there has to be something in the logic which identifies the changed files that is misidentifying the data.json file.
That file shouldn't need any extra behavior. Please run git status in the terminal in your repository. If that file is listed there, please run git add -A and run git status again and share all these outputs here, please.
The file was already tracked by git, and git status does not show a change to the file during the bug.
If I change the settings of the obsidian-git plugin, for example the auto commit interval, git status shows the change and the change is committed correctly by the obsidian-git plugin.
Afterwards the weird behavior returns.
I have traced the origin of the bug to be via
if (this.gitManager instanceof SimpleGit) {
this.mayDeleteConflictFile();
status = await this.updateCachedStatus();
async updateCachedStatus(): Promise<Status> {
this.cachedStatus = await this.gitManager.status();
return this.cachedStatus;
}
simpleGit.ts status() line 123
async status(): Promise<Status> {
this.plugin.setState(PluginState.status);
const status = await this.git.status((err) => this.onError(err));
The status const at this point contains the following data:
status: {"not_added":[],"conflicted":[],"created":[],"deleted":[],"modified":[".obsidian/plugins/obsidian-git/data.json"],"renamed":[],"files":[{"path":".obsidian/plugins/obsidian-git/data.json","index":" ","working_dir":"M"}],"staged":[],"ahead":0,"behind":0,"current":"master","tracking":null,"detached":false}
Again, this is the case even though git status outputs:
nothing to commit, working tree clean
It seems to me that the problem lies with the simple-git package.
Were you able to reproduce the behavior on your end using the steps i provided?
No I cannot reproduce your issue. I don't think simple-git produces an outdated or wrong status result. Can you verify via a git show HEAD, after an empty commit, that the commit is actually empty? Without extra config on your side, git wouldn't commit without any changes. (it needs --allow-empty).
Seems like you are already building the plugin yourself, could you add the following line after getting the status in updateCachedStatus() to see the diff of your changes?
await this.gitManager.getDiffString(".obsidian/plugins/obsidian-git/data.json")TLDR:
In order to save you from reading the entirety of my response, here is the result of my investigation:
Removing the function call to this.plugin.saveSettings(); in private doAutoCommitAndSync() fixes the issue.
private doAutoCommitAndSync(): void {
this.plugin.promiseQueue.addTask(() => {
if (this.plugin.settings.differentIntervalCommitAndPush) {
return this.plugin.commit({ fromAuto: true });
} else {
return this.plugin.commitAndSync(true);
}
});
this.saveLastAuto(new Date(), "backup");
this.plugin.saveSettings();
It is unclear to me why the settings are saved during the commit and sync, but I expect there to be a reason.
Is it possible to remove this call completely or check for the condition in which it is needed and only issue the function call if necessary?
Here is the entire response I wrote in parallel to investigating the bug, in case you want to know how I came to my conclusion:
I'm sorry if I was unclear:
Although an empty commit seems to be issued to git, no commit happens.
This is probably exactly because git ignores empty commits without the --allow-empty.
The only changes I have detected so far are the changes of the "modified" and "changed" timestamps that are stored by the filesystem for some of the .git/objects/ files and the content of the .git/COMMIT_EDITMSG file.
These files are probably regenerated/changed by git internally even if an issued commit is ignored because it is empty.
As you have asked, I have changed updateCachedStatus() to
async updateCachedStatus(): Promise<Status> {
this.cachedStatus = await this.gitManager.status();
const diffSting = await this.gitManager.getDiffString(
".obsidian/plugins/obsidian-git/data.json"
);
this.displayMessage("diffSting: " + JSON.stringify(diffSting));
return this.cachedStatus;
}
This yields:
obsidian-git: diffSting: ""
I have also changed the status() function in simpleGit.ts to:
async status(): Promise<Status> {
this.plugin.setState(PluginState.status);
const status = await this.git.status((err) => this.onError(err));
this.plugin.setState(PluginState.idle);
console.log("status: " + JSON.stringify(status));
const hash = await this.git.hashObject(
".obsidian/plugins/obsidian-git/data.json",
false
);
console.log("hash: " + JSON.stringify(hash));
...
This yields:
status: {"not_added":[],"conflicted":[],"created":[],"deleted":[],"modified":[".obsidian/plugins/obsidian-git/data.json"],"renamed":[],"files":[{"path":".obsidian/plugins/obsidian-git/data.json","index":" ","working_dir":"M"}],"staged":[],"ahead":0,"behind":0,"current":"master","tracking":null,"detached":false}
I added the hashObject() function call as an additional test to see if the file changes in between the empty commits. The output is something like
hash: "bd3bc3b56f12bae0c9b67a57ca2d72556fd02061"
This hash does not change between the empty commit attempts. This means that the content of the data.json file does not change.
If I change the auto commit interval in obsidian, the hash changes as expected, since this setting is stored in the data.json file.
Since the content of the file does not change, simple-git has to use something else to detect if a file was modified.
I suspect it uses the "modified" or "changed" timestamps stored in the filesystem metadata for this.
According to this document referenced in this stackoverflow post, git itself uses these timestamps to determine if a file has changed.
Indeed, stat .obsidian/plugins/obsidian-git/data.json shows that these timestamps change every time the auto commit is initiated (on linux systems the stat command prints the file metadata):
File: data.json
Size: 1621 Blocks: 8 IO Block: 4096 regular file
Device: 0,48 Inode: 8241998 Links: 1
Access: (0664/-rw-rw-r--) Uid: ( 1000/ user) Gid: ( 1000/ user)
Access: 2024-10-12 08:57:57.823999000 +0200
Modify: 2024-10-12 08:57:57.823999000 +0200
Change: 2024-10-12 08:57:57.826218349 +0200
Birth: 2024-10-01 04:12:23.445448450 +0200
The "Access", "Modify" and "Change" timestamps all change when the auto commit is triggered.
Since you cannot reproduce my error, I fear this bug might be down to details in how git interacts with the operating system or filesystem I am using (Debian + btrfs).
Preventing these timestamps from changing might fix this issue anyways.
The timestamps are changed due to this call to this.plugin.saveSettings(); in doAutoCommitAndSync()
private doAutoCommitAndSync(): void {
this.plugin.promiseQueue.addTask(() => {
if (this.plugin.settings.differentIntervalCommitAndPush) {
return this.plugin.commit({ fromAuto: true });
} else {
return this.plugin.commitAndSync(true);
}
});
this.saveLastAuto(new Date(), "backup");
this.plugin.saveSettings();
Indeed when I remove this function call, data.json is no longer reported as modified by the this.git.status() call, fixing the issue entirely, and the commit and sync functionality still functions.
This is still very strange, because the saveSettings call shouldn't change the data.json file and cause empty commits.
Nevertheless, while refactoring some code I noticed that useless saveSettings call as well, so it's now removed in v2.28.0. You may try that new version.
The problem is fixed in version 2.28.2. Thank you very much!