Skip to content

Commit

Permalink
checkout $tree: do not throw away unchanged index entries
Browse files Browse the repository at this point in the history
When we "git checkout $tree", we pull paths from $tree into
the index, and then check the resulting entries out to the
worktree. Our method for the first step is rather
heavy-handed, though; it clobbers the entire existing index
entry, even if the content is the same. This means we lose
our stat information, leading checkout_entry to later
rewrite the entire file with identical content.

Instead, let's see if we have the identical entry already in
the index, in which case we leave it in place. That lets
checkout_entry do the right thing. Our tests cover two
interesting cases:

  1. We make sure that a file which has no changes is not
     rewritten.

  2. We make sure that we do update a file that is unchanged
     in the index (versus $tree), but has working tree
     changes. We keep the old index entry, and
     checkout_entry is able to realize that our stat
     information is out of date.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
  • Loading branch information
peff authored and gitster committed Nov 13, 2014
1 parent eeff891 commit c5326bd
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 0 deletions.
18 changes: 18 additions & 0 deletions builtin/checkout.c
Expand Up @@ -67,6 +67,7 @@ static int update_some(const unsigned char *sha1, const char *base, int baselen,
{
int len;
struct cache_entry *ce;
int pos;

if (S_ISDIR(mode))
return READ_TREE_RECURSIVE;
Expand All @@ -79,6 +80,23 @@ static int update_some(const unsigned char *sha1, const char *base, int baselen,
ce->ce_flags = create_ce_flags(0) | CE_UPDATE;
ce->ce_namelen = len;
ce->ce_mode = create_ce_mode(mode);

/*
* If the entry is the same as the current index, we can leave the old
* entry in place. Whether it is UPTODATE or not, checkout_entry will
* do the right thing.
*/
pos = cache_name_pos(ce->name, ce->ce_namelen);
if (pos >= 0) {
struct cache_entry *old = active_cache[pos];
if (ce->ce_mode == old->ce_mode &&
!hashcmp(ce->sha1, old->sha1)) {
old->ce_flags |= CE_UPDATE;
free(ce);
return 0;
}
}

add_cache_entry(ce, ADD_CACHE_OK_TO_ADD | ADD_CACHE_OK_TO_REPLACE);
return 0;
}
Expand Down
17 changes: 17 additions & 0 deletions t/t2022-checkout-paths.sh
Expand Up @@ -61,4 +61,21 @@ test_expect_success 'do not touch unmerged entries matching $path but not in $tr
test_cmp expect.next0 actual.next0
'

test_expect_success 'do not touch files that are already up-to-date' '
git reset --hard &&
echo one >file1 &&
echo two >file2 &&
git add file1 file2 &&
git commit -m base &&
echo modified >file1 &&
test-chmtime =1000000000 file2 &&
git update-index -q --refresh &&
git checkout HEAD -- file1 file2 &&
echo one >expect &&
test_cmp expect file1 &&
echo "1000000000 file2" >expect &&
test-chmtime -v +0 file2 >actual &&
test_cmp expect actual
'

test_done

7 comments on commit c5326bd

@mkress
Copy link

@mkress mkress commented on c5326bd Feb 24, 2016

Choose a reason for hiding this comment

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

that is not working if you do a git checkout beetwen two branches (and one branch is merged into another), so all files do get a new timestamp

@ZwergNaseXXL
Copy link

@ZwergNaseXXL ZwergNaseXXL commented on c5326bd Apr 29, 2016

Choose a reason for hiding this comment

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

@peff Confirmed. I'm new to git and I ran into the same problem quickly.
Switching back & forth between two branches which are completely in sync touches a lot of files. Not all of them, but a lot. Which means I have to wait a couple of minutes for pointless rebuilds in Visual Studio, even though nothing has changed. Might be some weird interaction between git and VS2013 though, who knows...

@ZwergNaseXXL
Copy link

Choose a reason for hiding this comment

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

@peff @gitster Having just re-read your commit comment for this issue, I'm pretty sure it doesn't solve the issue it was meant to solve ('do not touch files that are already up-to-date').

Our company policy requires me to create new feature branches for every issue I work on. So naturally, I switch between branches several times per day. And each time I do so, hundreds of files are touched without any actual changes! A full rebuild of our solution takes about 12 minutes. Everytime I switch branches, I have to wait for about 5 to 7 minutes for the forced rebuild which changes nothing. This is defeating the whole purpose of being able to "quickly" switch branches in the same working tree. It's beginning to take its toll on me and I'm considering to create separate working trees for every feature branch I create. Any interest in fixing or at least investigating this?

@peff
Copy link
Member Author

@peff peff commented on c5326bd May 13, 2016

Choose a reason for hiding this comment

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

@ZwergNaseXXL Do you mind taking this to the git mailing list? That's where development is discussed. Info is at https://git-scm.com/community

@gitster
Copy link
Member

Choose a reason for hiding this comment

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

Isn't the description in the commit message wrong? I think update_some() called here is about "git checkout $tree $path1 $path2 ..." (i.e. grabbing some paths out of tree-ish), not "git checkout $commit" (i.e. branch switching). I think that is what makes mkress and ZwergNaseXXL confused.

I think branch switching already has "preserve timestamps etc." logic for paths that do not change.

$ ls -l COPYING
-rw-r----- 1 jch eng 18765 May 11 14:35 COPYING
$ git checkout next
Switched to branch 'next'
$ ls -l COPYING
-rw-r----- 1 jch eng 18765 May 11 14:35 COPYING
$ git checkout master
Switched to branch 'master'
$ ls -l COPYING
-rw-r----- 1 jch eng 18765 May 11 14:35 COPYING
$ date
Fri May 13 15:52:13 PDT 2016

As we can see, with two branches that do not change a path,
switching between them do not change the timestamp of the
path (for obvious reasons).

@peff
Copy link
Member Author

@peff peff commented on c5326bd May 13, 2016

Choose a reason for hiding this comment

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

Yeah, the commit message said $tree to indicate that, but it really should have been checkout $tree . or similar.

I agree that branch-switching is not related, and also that it already handles this properly.

@mkress
Copy link

@mkress mkress commented on c5326bd May 14, 2016

Choose a reason for hiding this comment

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

as a workaround i use another index-file for every branch - so the checkout is also correct, when switching between branches.

`#!/bin/sh

worktree=path/to/checkout/directory
branch=$(echo "$1" | cut -d/ -f3);

checkout files to worktree

GIT_INDEX_FILE=/path/to/git/direcotry/index-$branch git --work-tree="$worktree" checkout $branch -f
`

Please sign in to comment.