Web lists-archives.com

RE: [PATCH 1/1] reset: fix reset when using the sparse-checkout feature.




> From: Jacob Keller [mailto:jacob.keller@xxxxxxxxx]
> Sent: Tuesday, September 12, 2017 4:29 PM
> 
> On Tue, Sep 12, 2017 at 1:20 PM, Kevin Willford <kewillf@xxxxxxxxxxxxx> wrote:
> >
> > I think this is where I need to do a better job of explaining so here is a
> > simple example.
> >
> > I have a file "a" that was added in the latest commit.
> > $ git log --oneline
> > c1fa646 (HEAD -> reset, master) add file a
> > 40b342c Initial commit with file i
> >
> > Running the reset without using a sparse-checkout file
> >
> > $ git reset HEAD~1
> > $ git status
> > On branch reset
> > Untracked files:
> >   (use "git add <file>..." to include in what will be committed)
> >
> >         a
> >
> > nothing added to commit but untracked files present (use "git add" to track)
> >
> > Turning on sparse-checkout and running checkout to make my working
> > directory sparse
> >
> > $ git config core.sparsecheckout true
> > $ echo /i > .git/info/sparse-checkout
> > $ git checkout -f
> >
> > Running reset gives me
> > $ git reset HEAD~1
> > $ git status
> > On branch reset
> > nothing to commit, working tree clean
> > $ git ls-files
> > i
> >
> > file a is gone.  Not in the index and not in the working directory.
> > Nothing to let the user know that anything changed.
> >
> > With a modified file no sparse-checkout
> > $ git log --oneline
> > 6fbd34a (HEAD -> reset, modified) modified file m
> > c734d72 Initial commit with file i and m
> > $ git reset HEAD~1
> > Unstaged changes after reset:
> > M       m
> > $ git status
> > On branch reset
> > Changes not staged for commit:
> >   (use "git add <file>..." to update what will be committed)
> >   (use "git checkout -- <file>..." to discard changes in working directory)
> >
> >         modified:   m
> >
> > no changes added to commit (use "git add" and/or "git commit -a")
> >
> > With sparse-checkout
> > $ git reset HEAD~1
> > Unstaged changes after reset:
> > D       m
> > $ git status
> > On branch reset
> > Changes not staged for commit:
> >   (use "git add/rm <file>..." to update what will be committed)
> >   (use "git checkout -- <file>..." to discard changes in working directory)
> >
> >         deleted:    m
> >
> > no changes added to commit (use "git add" and/or "git commit -a")
> >
> 
> Wasn't "m" outside the sparse checkout? Or was it a file in the sparse
> checkout? I mean to say, the file after setting up sparse checkout was
> one of the "interesting" files that sparse checked out?
> 
> Or was it in fact a separate file which wasn't there?

Sorry.  It was not in the sparse-checkout file. 
$ git config core.sparsecheckout true
$ echo /i > .git/info/sparse-checkout
$ git checkout -f
was ran in the modified file case as well and "i" was the only file in the
working directory before reset. 

> 
> I would think that in sparse-checkout world, you should only *ever*
> have the files you list in sparse.
> 
> So files outside sparse world should be ignored, not shown and not
> show up in status, but they should absolutely not show up in the
> working tree either.

Sparse checkout uses the skip-worktree bit which has the following
documentation:
https://git-scm.com/docs/git-update-index 
"Skip-worktree bit can be defined in one (long) sentence: When reading
an entry, if it is marked as skip-worktree, then Git pretends its working
directory version is up to date and read the index version instead.

To elaborate, "reading" means checking for file existence, reading file
attributes or file content. The working directory version may be present
or absent. If present, its content may match against the index version or
not. Writing is not affected by this bit, content safety is still first priority.
Note that Git can update working directory file, that is marked skip-worktree,
if it is safe to do so (i.e. working directory version matches index version)"

I'm not saying that is the right behavior, just pointing out the documentation.
And from my experience git will turn on and off the skip-worktree flag
depending on the command as we see happening with reset.

> 
> You're not "changing" any commits, because the status of the file at
> HEAD~1 is exactly what HEAD~1 says it is, but you just don't have a
> checked out copy of it.

In the case of reset yes and it matches HEAD and if the sparse flag was on,
the user does not know that the file was changed in any way, which if I add
and commit will create a commit with the file content at HEAD~1
that is outside the sparse area without the user even knowing what
those changes were unless after the commit they run a git diff HEAD~1 and
see.  So files will be changed outside of the sparse area without the
user's knowledge.

> 
> I think the key problem is that reset is clearing the sparse flag of a
> file so that it no longer shows up as sparse, which is why status
> subsequently shows the file deleted (since you don't have a local copy
> anymore).

And in the case of an added file there is not a sparse flag to clear or keep
Because the file didn't exist at HEAD~1 and there is no entry in the index
to keep and the file and any knowledge of it is gone.

> 
> Am I understanding the problem correctly? I think your examples above
> are not clear because they don't seem to be each complete individually
> (The sparse checkout examples should start from a clean world and
> explain how they got there rather than relying on imformation in the
> prior non sparse example. We should be clear so everyone knows what
> the problem is).
> 

I thought that the 'git log --oneline' with the commit message make it
clear what the state of the repo was before the reset in either case.
The only difference is that in the sparse case, it is turned on and one
file added to the sparse-checkout file and a checkout is performed
to make the working directory sparse.

> If you're performing a sparse checkout and you modify files outside of
> the sparse area, I think that will cause problems, and we may not be
> making the best efforts to resolve it. I do agree that if you have
> created a file "m" when only "i" is in your path, that we should
> absolutely not delete "m" but leave it as is.
> 

Again documentation says and from my experience, git can update and
delete "m" if it matches the index version.  

> Thanks,
> Jake
>