Skip to content

WIP: respect index during checkout - #3839

Open
ethomson wants to merge 8 commits into
mainfrom
ethomson/checkout_conflict_respect_index
Open

WIP: respect index during checkout#3839
ethomson wants to merge 8 commits into
mainfrom
ethomson/checkout_conflict_respect_index

Conversation

@ethomson

Copy link
Copy Markdown
Member

Our current checkout implementation looks to see if the working directory is dirty to determine whether it's safe to checkout a given file. It does not look at the index, however.

This is fine if we're checking out the repository's index itself, but if we're checking out a git_tree then we could get into a place where the repository's index has modifications, but the workdir itself has been reverted. We would obliterate those staged changes.

This adds checking for changes in the index before checking out (raising conflicts as appropriate otherwise) when a working directory file exists. This does not yet cover other cases (for example, when an entry exists in the index but not in the working directory.)

I think that we can simply change the with_wd case to a with_wd_or_index case and use the existing no_wd case to be no_wd_or_index.

Edward Thomson added 8 commits June 25, 2016 18:05
Test that we can successfully force checkout a target when the workdir
was updated (to match the target) but the index differs.
When performing a forced checkout, treat files as modified when the
workdir is identical to the target, but the index differs.  This
ensures that we update the index to match, instead of assuming that
since the working directory does not differ, there's nothing to do.
Otherwise we would keep the index as-is and the staged change would
propagate past the force checkout.
Test that we can successfully force checkout a target when the file
contents are identical, but the mode has changed.
When performing a forced checkout, treat files as modified when the
workdir or the index is identical except for the mode.  This ensures
that force checkout will update the mode to the target.  (Apply this
check for regular files only, if one of the items was a file and the
other was another type of item then this would be a typechange and
handled independently.)
When the target of a checkout has a deletion of an item, a modified
index entry should cause a conflict even when the workdir has been
reverted to the baseline.
When the target of a checkout has a modification of an item, a modified
index entry should cause a conflict even when the workdir has been
reverted to the baseline.
Introduce a new branch that changes the file `README` into a folder.
When the target of a checkout has a typechange of an item, a modified
index entry should cause a conflict even when the workdir has been
reverted to the baseline.
Comment thread tests/checkout/tree.c

cl_git_pass(git_status_list_new(&status, g_repo, NULL));
cl_assert_equal_i(0, git_status_list_entrycount(status));
git_status_list_free(status);

@pks-t pks-t Jun 27, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's the third test now that I see this pattern, checking that our repository is actually in a clean state (that is, asserting the branch and then asserting that no modifications are present). Maybe put this into a function/macro? I bet it would be useful to other tests.

@ethomson ethomson mentioned this pull request Feb 17, 2017
@cjhoward92

cjhoward92 commented Oct 4, 2017

Copy link
Copy Markdown
Contributor

Are there any plans to continue this work? It seems like libgit2 does not properly manage file mode only changes. If I try to force checkout a file with only file mode changes nothing happens, as I believe libgit2 is only checking content, which has not changed. It looks like you fixed that in this WIP.

@ethomson

ethomson commented Oct 5, 2017

Copy link
Copy Markdown
Member Author

Thanks @cjhoward92 for the ping. I'll bring forward those fixes in a separate PR.

@cjhoward92

Copy link
Copy Markdown
Contributor

You rock!

@pks-t

pks-t commented Mar 26, 2018

Copy link
Copy Markdown
Member

@ethomson: can this be closed? I feel like you've already committed most changes in here to master

Base automatically changed from master to main January 7, 2021 10:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants