Re: [RFC PATCH v3 8/8] --sparse for porcelains

From

Nguyen Thai Ngoc Duy <pclouds@gmail.com>

Date

Aug 17, 2009, 14:41 UTC

Message-ID

<fcaeb9bf0908170741v210e7f4et9f1c68bc9a81ca65@mail.gmail.com>

In-Reply-To

<alpine.DEB.1.00.0908171524150.4991@intel-tinevez-2-302>

> Hi,
>
> On Mon, 17 Aug 2009, Nguyen Thai Ngoc Duy wrote:
>
>> On Mon, Aug 17, 2009 at 4:08 PM, Johannes
>> Schindelin<Johannes.Schindelin@gmx.de> wrote:
>> > Turns out that somebody on IRC had a problem that requires to have
>> > sparse'd out files which _do_ have working directory copies.
>> >
>> > So just having the assume-changed bit may not be enough.
>> >
>> > The scenario is this: the repository contains a file that users are
>> > supposed to change, but not commit to (only the super-intelligent
>> > inventor of this scenario is allowed to).  As this repository is
>> > originally a subversion one, there is no problem: people just do not
>> > switch branches.
>> >
>> > But this guy uses git-svn, so he does switch branches, and to avoid
>> > committing the file by mistake, he marked it assume-unchanged.
>>
>> Hmm.. never thought of this use before. If he does not want to commit by
>> mistake, should he add to-be-committed changes to index and do "git
>> commit" without "-a" (even better, do "git diff --cached" first)?
>
> You probably agree that this would be a _very_ fragile setup.  Very easy
> to make mistakes.
>
> But we try to get away from that, don't we?  Git had a reputation to be
> easy fsck up for long enough.

Well.. of course I don't want Git to keep that reputation :-)

>> > Only that a branch switch overwrites the local changes.
>>
>> I don't think branch switch overwrites changes in this case. Whenever
>> Git is to touch worktree files, it ignores assumed-unchanged bit and
>> does lstat() to make sure worktree files are up to date.
>
> Well, it does there, thankyouverymuch.
>
> The problem of course is that the other branch has an ancient version of
> that file (which should _not_ overwrite the current, modified version!),
> i.e. "git diff HEAD..other -- file" does not come empty.
>
> As 'file' is assume-unchanged, zinnnng, the file gets "updated".

diff --git a/unpack-trees.c b/unpack-trees.c
index eb47676..7b9ddf6 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -538,7 +538,9 @@ static int verify_uptodate_1(struct cache_entry *ce,
 {
 	struct stat st;

-	if (o->index_only || o->reset || ce_uptodate(ce))
+	if (o->index_only || o->reset ||
+	    /* we are going to update worktree, don't trust ce_uptodate if
it is CE_VALID'd */
+	    (!(ce->ce_flags & CE_VALID) && ce_uptodate(ce)))
 		return 0;

 	if (!lstat(ce->name, &st)) {


>> > I suggested the use of the sparse feature, and mark this file (and
>> > this file alone) as sparse'd-out.
>>
>> Sparse checkout only removes a file if its assume-unchanged bit
>> changes from 0 to 1.
>
> The problem is not removing, but overwriting.
>
> And in this respect, 'assume-unchanged' is a very different beast from
> 'sparse'.  I am growing more and more convinced that you cannot just reuse
> the assume-unchanged bit.

And assume-unchanged bit could get lost during index merging, which
may cause unexpected effect if sparse checkout bases off
assume-unchanged. Let me think more of it tonight.

>> If it's already 1, it does not care whether there is a corresponding
>> file in worktree. So something like this should work:
>>
>> git checkout my-branch
>> git update-index --assume-unchanged that-special-file
>> echo that-special-file > .git/info/sparse
>> # edit that-special-file
>> git commit -a
>> # do whatever you want, git pull/checkout/read-tree... won't touch
>> that-special-file because it's assume-unchanged already
>
> ... except if you changed .git/info/sparse and a formerly sparse'd-out
> file is overwritten by "pull".  Not good.

Again, I think it's a bug.

>> > Is this an intended usage scenario?  Then we cannot reuse the
>> > assume-changed bit [*1*].
>>
>> It'd be great if people tell us all the scenarios they have. My use
>> could be too limited.
>
> The use case I would have is where a collaborator wants to work only on
> one subdirectory and the top-level directory.  All other subdirectories
> are of no interest to him.
>
> Another use case: documentation.  I do not have that use case yet, but I
> know about people who do.

Translators usually checkout one or two files (I am Vietnamese
Translation Coordinator of GNOME, but well... I check them all out. I
suppose "normal" translators would not want to do like I do.)

>  Specifying what you _want_ to have checked out
> is much more straight-forward here than the opposite.

I think it depends on type of projects. For documentation projects,
you may want a few files. For software projects, usually you need
everything _except_ a few big directories. For WebKit, it's a bunch of
test data that I don't care about. Firmware in hardware-related
projects or media files in game projects fall in the same category. I
don't have strong opinion on this. Either include or exclude is fine
to me.

-- 
Duy