Misterious warning about file system boundaries

Misterious warning about file system boundaries

[Michael J Gruber]

Heya,

now what is going on here? After upgrading to current next I get

warning: working tree spans across filesystems but
GIT_DISCOVERY_ACROSS_FILESYSTEM is not set.

in several repos, such as my local git.git repo. That is certainly on a
single file system only (ext4 over lvm over luks, all on one partition,
Fedora 13). I also get this for another repo, but not for every repo. It
goes away when I set the var and comes back when I don't set it, of course.

Although I haven't bisected this should be due to
52b98a7 (write-index: check and warn when worktree crosses a filesystem
boundary, 2010-04-04).

How does the code detect a file system boundary, and where could it go
wrong?

Michael

Re: Misterious warning about file system boundaries

[Andreas Ericsson]

On 06/09/2010 10:21 PM, Michael J Gruber wrote:
> Heya,
> 
> now what is going on here? After upgrading to current next I get
> 
> warning: working tree spans across filesystems but
> GIT_DISCOVERY_ACROSS_FILESYSTEM is not set.
> 
> in several repos, such as my local git.git repo. That is certainly on a
> single file system only (ext4 over lvm over luks, all on one partition,
> Fedora 13). I also get this for another repo, but not for every repo. It
> goes away when I set the var and comes back when I don't set it, of course.
> 
> Although I haven't bisected this should be due to
> 52b98a7 (write-index: check and warn when worktree crosses a filesystem
> boundary, 2010-04-04).
> 
> How does the code detect a file system boundary, and where could it go
> wrong?
> 

According to the patch, it checks if the device id recorded from stat(2)
is the same for all files and, if not, warns about it.

It seems that your interpretation of "one partition" differs from that
reported by the kernel. Why that is so, I have no idea.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Considering the successes of the wars on alcohol, poverty, drugs and
terror, I think we should give some serious thought to declaring war
on peace.

Re: Misterious warning about file system boundaries [It's a bug, not a mystery.]

[Michael J Gruber]

Andreas Ericsson venit, vidit, dixit 10.06.2010 10:03:
> On 06/09/2010 10:21 PM, Michael J Gruber wrote:
>> Heya,
>>
>> now what is going on here? After upgrading to current next I get
>>
>> warning: working tree spans across filesystems but
>> GIT_DISCOVERY_ACROSS_FILESYSTEM is not set.
>>
>> in several repos, such as my local git.git repo. That is certainly on a
>> single file system only (ext4 over lvm over luks, all on one partition,
>> Fedora 13). I also get this for another repo, but not for every repo. It
>> goes away when I set the var and comes back when I don't set it, of course.
>>
>> Although I haven't bisected this should be due to
>> 52b98a7 (write-index: check and warn when worktree crosses a filesystem
>> boundary, 2010-04-04).
>>
>> How does the code detect a file system boundary, and where could it go
>> wrong?
>>
> 
> According to the patch, it checks if the device id recorded from stat(2)
> is the same for all files and, if not, warns about it.
> 
> It seems that your interpretation of "one partition" differs from that
> reported by the kernel. Why that is so, I have no idea.
> 

I'm sorry, but "my interpretation"? WTF? This is all on
/home/mjg/src/git which has no bind mounts whatsoever.

I actually mixed up my / and /home situation above, /home is even
simpler: single ext3 over luks dm device over single "real" partition.
All of this (except for single ext3 part.) should not matter, of course.

I bisected it just be sure, and it boils down to 9780e62 which is the
commit merging 52b98a7 to next.

git ls-files|xargs stat -c "%d %D" |sort|uniq

gives

64772 fd04

which is, in particular, 1 device only. Now, here comes funny. After
changing write_index() to print the two ce_dev's which differ, i.e.
printf("%d %d\n", ce->ce_dev, cache[first_valid_ent]->ce_dev);
 I have:

./git-status -s|sort|uniq -c
warning: working tree spans across filesystems but
GIT_DISCOVERY_ACROSS_FILESYSTEM is not set.
    150 64770 64772
    662 64771 64772
      1  M read-cache.c

WTF???

git reset --hard doesn't help this.

rm .git/index && git reset does help.

So, it seems that nowadays not even "reset --hard" refreshes the index
completely. After a reboot your device IDs may change, of course, and
files on the same file system will have different ce_dev values in the
index depending on when their index entry was refreshed last.

I really don't know how to fix this (other than by refreshing stat info
in write_index() before warning). Debugging it was quite an exercise
already.

Also, having git reset --index do the equivalent of "rm .git/index &&
git reset" might be good to have.

I'm keeping the bad index, btw, so that I can test a possible future fix.

Michael

Re: Misterious warning about file system boundaries [It's a bug, not a mystery.]

[Erik Faye-Lund]

On Thu, Jun 10, 2010 at 11:05 AM, Michael J Gruber
<git@drmicha.warpmail.net> wrote:
> Andreas Ericsson venit, vidit, dixit 10.06.2010 10:03:
>> On 06/09/2010 10:21 PM, Michael J Gruber wrote:
>>> Heya,
>>>
>>> now what is going on here? After upgrading to current next I get
>>>
>>> warning: working tree spans across filesystems but
>>> GIT_DISCOVERY_ACROSS_FILESYSTEM is not set.
>>>
>>> in several repos, such as my local git.git repo. That is certainly on a
>>> single file system only (ext4 over lvm over luks, all on one partition,
>>> Fedora 13). I also get this for another repo, but not for every repo. It
>>> goes away when I set the var and comes back when I don't set it, of course.
>>>
>>> Although I haven't bisected this should be due to
>>> 52b98a7 (write-index: check and warn when worktree crosses a filesystem
>>> boundary, 2010-04-04).
>>>
>>> How does the code detect a file system boundary, and where could it go
>>> wrong?
>>>
>>
>> According to the patch, it checks if the device id recorded from stat(2)
>> is the same for all files and, if not, warns about it.
>>
>> It seems that your interpretation of "one partition" differs from that
>> reported by the kernel. Why that is so, I have no idea.
>>
>
> I'm sorry, but "my interpretation"? WTF? This is all on
> /home/mjg/src/git which has no bind mounts whatsoever.
>
> I actually mixed up my / and /home situation above, /home is even
> simpler: single ext3 over luks dm device over single "real" partition.
> All of this (except for single ext3 part.) should not matter, of course.
>
> I bisected it just be sure, and it boils down to 9780e62 which is the
> commit merging 52b98a7 to next.
>
> git ls-files|xargs stat -c "%d %D" |sort|uniq
>
> gives
>
> 64772 fd04
>
> which is, in particular, 1 device only. Now, here comes funny. After
> changing write_index() to print the two ce_dev's which differ, i.e.
> printf("%d %d\n", ce->ce_dev, cache[first_valid_ent]->ce_dev);
>  I have:
>
> ./git-status -s|sort|uniq -c
> warning: working tree spans across filesystems but
> GIT_DISCOVERY_ACROSS_FILESYSTEM is not set.
>    150 64770 64772
>    662 64771 64772
>      1  M read-cache.c
>
> WTF???
>
> git reset --hard doesn't help this.
>
> rm .git/index && git reset does help.
>
<snip>
>
> Also, having git reset --index do the equivalent of "rm .git/index &&
> git reset" might be good to have.
>

Doesn't "git update-index --refresh" do the trick?

-- 
Erik "kusma" Faye-Lund

Re: Misterious warning about file system boundaries [It's a bug, not a mystery.]

[Michael J Gruber]

Erik Faye-Lund venit, vidit, dixit 10.06.2010 11:39:
> On Thu, Jun 10, 2010 at 11:05 AM, Michael J Gruber
> <git@drmicha.warpmail.net> wrote:
>> Andreas Ericsson venit, vidit, dixit 10.06.2010 10:03:
>>> On 06/09/2010 10:21 PM, Michael J Gruber wrote:
>>>> Heya,
>>>>
>>>> now what is going on here? After upgrading to current next I get
>>>>
>>>> warning: working tree spans across filesystems but
>>>> GIT_DISCOVERY_ACROSS_FILESYSTEM is not set.
>>>>
>>>> in several repos, such as my local git.git repo. That is certainly on a
>>>> single file system only (ext4 over lvm over luks, all on one partition,
>>>> Fedora 13). I also get this for another repo, but not for every repo. It
>>>> goes away when I set the var and comes back when I don't set it, of course.
>>>>
>>>> Although I haven't bisected this should be due to
>>>> 52b98a7 (write-index: check and warn when worktree crosses a filesystem
>>>> boundary, 2010-04-04).
>>>>
>>>> How does the code detect a file system boundary, and where could it go
>>>> wrong?
>>>>
>>>
>>> According to the patch, it checks if the device id recorded from stat(2)
>>> is the same for all files and, if not, warns about it.
>>>
>>> It seems that your interpretation of "one partition" differs from that
>>> reported by the kernel. Why that is so, I have no idea.
>>>
>>
>> I'm sorry, but "my interpretation"? WTF? This is all on
>> /home/mjg/src/git which has no bind mounts whatsoever.
>>
>> I actually mixed up my / and /home situation above, /home is even
>> simpler: single ext3 over luks dm device over single "real" partition.
>> All of this (except for single ext3 part.) should not matter, of course.
>>
>> I bisected it just be sure, and it boils down to 9780e62 which is the
>> commit merging 52b98a7 to next.
>>
>> git ls-files|xargs stat -c "%d %D" |sort|uniq
>>
>> gives
>>
>> 64772 fd04
>>
>> which is, in particular, 1 device only. Now, here comes funny. After
>> changing write_index() to print the two ce_dev's which differ, i.e.
>> printf("%d %d\n", ce->ce_dev, cache[first_valid_ent]->ce_dev);
>>  I have:
>>
>> ./git-status -s|sort|uniq -c
>> warning: working tree spans across filesystems but
>> GIT_DISCOVERY_ACROSS_FILESYSTEM is not set.
>>    150 64770 64772
>>    662 64771 64772
>>      1  M read-cache.c
>>
>> WTF???
>>
>> git reset --hard doesn't help this.
>>
>> rm .git/index && git reset does help.
>>
> <snip>
>>
>> Also, having git reset --index do the equivalent of "rm .git/index &&
>> git reset" might be good to have.
>>
> 
> Doesn't "git update-index --refresh" do the trick?
> 

No.

And neither does --really-refresh.

I guess we need --I-really-mean-it-refresh.

In fact, not even after recompiling with USE_STDEV=y that
--really-refresh helps which stomps me.But what do I know.

Michael

Re: Misterious warning about file system boundaries [It's a bug, not a mystery.]

[Ævar Arnfjörð Bjarmason]

On Thu, Jun 10, 2010 at 10:36, Michael J Gruber
<git@drmicha.warpmail.net> wrote:
> I guess we need --I-really-mean-it-refresh.
>
> In fact, not even after recompiling with USE_STDEV=y that
> --really-refresh helps which stomps me.But what do I know.

The real problem here is the assumption in 52b98a7 that stat's st_dev
will stay the same for a given device, clearly that's not the case on
Linux if you reboot your machine.

Is there some way of working around that?

Re: Misterious warning about file system boundaries [It's a bug, not a mystery.]

[Jeff King]

On Thu, Jun 10, 2010 at 10:52:40AM +0000, Ævar Arnfjörð Bjarmason wrote:

> On Thu, Jun 10, 2010 at 10:36, Michael J Gruber
> <git@drmicha.warpmail.net> wrote:
> > I guess we need --I-really-mean-it-refresh.
> >
> > In fact, not even after recompiling with USE_STDEV=y that
> > --really-refresh helps which stomps me.But what do I know.
> 
> The real problem here is the assumption in 52b98a7 that stat's st_dev
> will stay the same for a given device, clearly that's not the case on
> Linux if you reboot your machine.
> 
> Is there some way of working around that?

We could simply drop the warning. The real point of the original patch
was to stop cross-device traversal in setup_git_directory, which doesn't
care about crossing reboots. I believe the index check and warning are
just there as an early suggestion. When you actually fail to cross the
device boundary looking for .git in the future, it will complain and
mention GIT_DISCOVERY_ACROSS_FILESYSTEM then.

Since the early-warning suggestion is generating false positives, and I
don't think there is a portable and reliable way around it, dropping it
makes sense to me.

-Peff

Re: Misterious warning about file system boundaries [It's a bug, not a mystery.]

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> Since the early-warning suggestion is generating false positives, and I
> don't think there is a portable and reliable way around it, dropping it
> makes sense to me.

Makes sense.  Let's make it so (I won't have time to do that myself until
late this evening, though).

Thanks.

Re: Misterious warning about file system boundaries [It's a bug, not a mystery.]

[Jeff King]

On Thu, Jun 10, 2010 at 07:53:47AM -0700, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > Since the early-warning suggestion is generating false positives, and I
> > don't think there is a portable and reliable way around it, dropping it
> > makes sense to me.
> 
> Makes sense.  Let's make it so (I won't have time to do that myself until
> late this evening, though).
> 
> Thanks.

Here it is with a nice commit message (though since it is only in next,
I guess you can just merge the early part and both 52b98a and my revert
will be dropped). It's on top of ld/discovery-limit-to-fs, of course.

Note that it would be possible to do a warning in some instances. We
just can't trust the on-disk index ce_dev information, so if you
actually refreshed two cross-filesystem entries we could detect that.
That is such a minority case that I doubt it is worth caring about,
though.

-- >8 --
Subject: [PATCH] Revert "write-index: check and warn when worktree crosses a filesystem boundary"

This reverts commit 52b98a7d2f12b5d0dd076221d40f8fa93598e11a.

The goal of that commit was to warn users early when their
worktree crossed filesystem boundaries. It worked by
comparing the st_dev stat information in the index, and
warning when we had more than one device.

However, the stat information may come from multiple runs,
and the st_dev field is not necessarily stable. In
particular, st_dev will change on Linux across reboots.
Index entries from the previous reboot will appear to come
from a different device, triggering a false positive.

Since this message is really only an early warning for
people who will be bit by the new
GIT_DISCOVERY_ACROSS_FILESYSTEM behavior, and because they
will get an actual error later on (when we can't find their
cross-filesystem .git directory), we can just scrap the
early warning.

Signed-off-by: Jeff King <peff@peff.net>
---
 read-cache.c |   14 --------------
 1 files changed, 0 insertions(+), 14 deletions(-)

diff --git a/read-cache.c b/read-cache.c
index e381ea5..f1f789b 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1550,8 +1550,6 @@ int write_index(struct index_state *istate, int newfd)
 	struct cache_entry **cache = istate->cache;
 	int entries = istate->cache_nr;
 	struct stat st;
-	int first_valid_ent = -1;
-	int more_than_one_dev;
 
 	for (i = removed = extended = 0; i < entries; i++) {
 		if (cache[i]->ce_flags & CE_REMOVE)
@@ -1574,7 +1572,6 @@ int write_index(struct index_state *istate, int newfd)
 	if (ce_write(&c, newfd, &hdr, sizeof(hdr)) < 0)
 		return -1;
 
-	more_than_one_dev = 0;
 	for (i = 0; i < entries; i++) {
 		struct cache_entry *ce = cache[i];
 		if (ce->ce_flags & CE_REMOVE)
@@ -1583,19 +1580,8 @@ int write_index(struct index_state *istate, int newfd)
 			ce_smudge_racily_clean_entry(ce);
 		if (ce_write_entry(&c, newfd, ce) < 0)
 			return -1;
-		if (ce_uptodate(ce)) {
-			if (first_valid_ent < 0)
-				first_valid_ent = i;
-			else if (ce->ce_dev != cache[first_valid_ent]->ce_dev)
-				more_than_one_dev = 1;
-		}
 	}
 
-	if (more_than_one_dev &&
-	    !git_env_bool("GIT_DISCOVERY_ACROSS_FILESYSTEM", 0))
-		warning("working tree spans across filesystems but "
-			"GIT_DISCOVERY_ACROSS_FILESYSTEM is not set.");
-
 	/* Write extension data here */
 	if (istate->cache_tree) {
 		struct strbuf sb = STRBUF_INIT;
-- 
1.7.1.516.gd5539.dirty