Unable to delete file when the repository will be empty

Hello,

First, thank you for this incredible package!

I get the following error when I try to remove the latest file in a repository and commit:

cannot create empty commit: clean working tree

I think the issue is due to the index entries being empty when the tree is built here:

go-git/worktree_commit.go

Line 156 in 3f1cfde

if len(idx.Entries) == 0 && (opts == nil || !opts.AllowEmptyCommits) {

Here is a screenshot of my debugging session:

You can see that the index entries are empty, but I have a change staged and ready to be committed.

Here is the code that you can use to reproduce my issue: https://gist.github.com/Erouan50/e2c8ec8d8ad21c7f3761b5f193ab6171#file-main-go

I'll create a pull request if you agree it is a bug.

Thank you!

this is happening on my side too 😢

I did a quick test and this is definitely a bug. The issue is that the option CommitOptions.AllowEmptyCommits is checking the wrong thing. It is checking if the repo will be empty after the commit rather than the commit not making any changes.

Eg if you modify the test TestNothingToCommit and make sure the repo is not empty before running the empty commit tests then the test will fail.

can we use ~~FileSystem.IsClean()~~ Status.IsClean() instead?

@pjbgf

I can't see that function? I can see Status.IsClean() however I wouldn't want to require a full status every time we want to commit, I think we need a WorkTree.IsClean() that does the same as WorkTree.Status() but bails out as soon as it finds the first change.

Although WorkTree.Status() is checking both staging and the worktree, I would think we should only need to check staging?

@pjbgf what are your thoughts on this?

@AriehSchneier

something like this @AriehSchneier ?

func (w *Worktree) IsStagingClean() (bool, error) {
	status, err := w.Status()
	if err != nil {
		return false, err
	}

	for _, s := range status {
		if s.Staging != Unmodified {
			return false, nil
		}
	}

	return true, nil
}

Possibly, I would need to read that code carefully to confirm. However that is doing waaay more work than we would want, and could cause performance issues on large repos (I'm not qualified to know how optimised w.Status() is). But I would think we would want to add to merkletrie something like:

type DiffTreeOptions struct {
	// Should the diff stop after finding the first difference
	OnlyFirstDif bool
}
func DiffTreeContextWithOptions(ctx context.Context, fromTree, toTree noder.Noder,
	hashEqual noder.Equal, options DiffTreeOptions) (Changes, error) {
...
		if options.OnlyFirstDif && len(ret) > 0 {
			return ret, nil
		}
...
}

So that we can just search for any change... As long as there is 1.

Hi,

Any updates on this ? for me it's the opposite, AllowEmptyCommits is a NOP

@Erouan50

@Erouan50 thank you for filing the issue and for providing good steps to reproduce. 🙇

if you agree it is a bug

I agree this is a bug.

The issue is that the option CommitOptions.AllowEmptyCommits is checking the wrong thing. It is checking if the repo will be empty after the commit rather than the commit not making any changes.

You are absolutely right.

I wouldn't want to require a full status every time we want to commit

+1

My suggestion here is that we change the logic to compare the new vs previous Tree hash:

if previousTree == treeHash && (opts == nil || !opts.AllowEmptyCommits) {
	return plumbing.ZeroHash, ErrEmptyCommit
}

@AriehSchneier what do you think?

@Erouan50 are you happy to create a PR with the fix? The logic for getting the Tree hash is already within opts.Amend.

I really don't know the inner working to be able to tell if the tree hash check will be enough.
Although reading the man page for --allow-empty sounds like that is correct:

--allow-empty
Usually recording a commit that has the exact same tree as its sole parent commit is a mistake, and the command prevents you from making such a commit. This option bypasses the safety, and is primarily for use by foreign SCM interface scripts.

Ran into this today. There's a proposed fix above, is there anything holding it back besides motion?

@ThinkChaos

@ThinkChaos if you create a PR and test it, I assume it would get merged pretty quickly.

git: worktree_commit, Modify checking empty commit. Fixes go-git#723

Merge pull request #1050 from onee-only/fix-empty-commit

Hi there 👋

This is still happening today, I downloaded the code referenced in the first post to confirm I'm not doing anything special and I got the same exact result.

I'm using the latest version

go.sum:

github.com/go-git/gcfg v1.5.1-0.20230307220236-3a3c6141e376 h1:+zs/tPmkDkHx3U66DAb0lQFJrpS6731Oaa12ikc+DiI=
github.com/go-git/gcfg v1.5.1-0.20230307220236-3a3c6141e376/go.mod h1:an3vInlBmSxCcxctByoQdvwPiA7DTK7jaaFDBTtu0ic=
github.com/go-git/go-billy/v5 v5.5.0 h1:yEY4yhzCDuMGSv83oGxiBotRzhwhNr8VZyphhiu+mTU=
github.com/go-git/go-billy/v5 v5.5.0/go.mod h1:hmexnoNsr2SJU1Ju67OaNz5ASJY3+sHgFRpCtpDCKow=
github.com/go-git/go-git/v5 v5.12.0 h1:7Md+ndsjrzZxbddRDZjF14qK+NN56sy6wkqaVrjZtys=
github.com/go-git/go-git/v5 v5.12.0/go.mod h1:FTM9VKtnI2m65hNI/TenDDDnUf2Q9FHnXYjuz9i5OEY=

@nour-s

This is still happening today, I downloaded the code referenced in the first post to confirm I'm not doing anything special and I got the same exact result.

I'm using the latest version

go.sum:
github.com/go-git/go-git/v5 v5.12.0 h1:7Md+ndsjrzZxbddRDZjF14qK+NN56sy6wkqaVrjZtys=
github.com/go-git/go-git/v5 v5.12.0/go.mod h1:FTM9VKtnI2m65hNI/TenDDDnUf2Q9FHnXYjuz9i5OEY=

@nour-s The fix is on master but no release has been created yet. Try using the master branch.

@AriehSchneier

@AriehSchneier I see, it is for company production so I would stick with the releases I think. I did a workaround by allowing empty commits.

Thanks for the help

Update module github.com/go-git/go-git/v5 to v5.13.1 (forgejo) (#6495)

Update module github.com/go-git/go-git/v5 to v5.13.1 (v10.0/forgejo) …

Navigation Menu

Repository navigation

Unable to delete file when the repository will be empty #723

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Unable to delete file when the repository will be empty #723

Description

Activity

clavinjune commented on Jun 23, 2023

AriehSchneier commented on Jun 25, 2023

clavinjune commented on Jun 25, 2023

AriehSchneier commented on Jun 25, 2023

clavinjune commented on Jun 26, 2023

AriehSchneier commented on Jun 26, 2023

g13013 commented on Jul 31, 2023

pjbgf commented on Sep 27, 2023

AriehSchneier commented on Sep 28, 2023

ThinkChaos commented on Jan 25, 2024

AriehSchneier commented on Jan 25, 2024

nour-s commented on Sep 14, 2024

AriehSchneier commented on Sep 16, 2024

nour-s commented on Sep 17, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions