'git status' is not read-only fs friendly

'git status' is not read-only fs friendly

[Marco Costalba]

In a repository under a mounted Windows directory (ntfs) I get this error:

$ git status
fatal: unable to create '.git/index.lock': Read-only file system
$

Is this correct? there exist a workaround? I just need to know if
current working directory is clean and report back to qgit user, so
read-only access would be ok for me.

All other commands commonly used to browse a repository seems to work
well, without pretending to write stuff.

Thanks
Marco

Re: 'git status' is not read-only fs friendly

[Linus Torvalds]

On Fri, 9 Feb 2007, Marco Costalba wrote:
>
> In a repository under a mounted Windows directory (ntfs) I get this error:
> 
> $ git status
> fatal: unable to create '.git/index.lock': Read-only file system
> $
> 
> Is this correct? there exist a workaround? I just need to know if
> current working directory is clean and report back to qgit user, so
> read-only access would be ok for me.

"git status" is kind of strange. It's really technically the engine behind 
the messages for "git commit": both in a very real technical sense (it's 
_literally_ the same script:"git-commit.sh" is not just "git-commit", but 
also "git-status"), but also in a very real historical "that is what the 
code was written for".

And you wouldn't think that it really needs write access, and you'd be 
largely correct, EXCEPT for the fact that git status actually does a 
refresh of the index, to make sure that we don't claim that something is 
dirty just because somebody has touched the file.

IOW, there's an implicit "git update-index --refresh" as part of 
calculating the status, and that's the thing that wants to lock the index 
file (and thus write to the filesystem).

> All other commands commonly used to browse a repository seems to work
> well, without pretending to write stuff.

"git status" doesn't "pretend" to write stuff. It really does. 

You *can* just use "git-runstatus" instead. That's the command that 
actually does all the heavy lifting. But you can see the difference by 
doing this:

	touch Makefile
	git runstatus

vs

	touch Makefile
	git status

Notice how the "runstatus" one claims that Makefile is "modified:". That's 
exactly because it doesn't do the index refresh.

			Linus

Re: 'git status' is not read-only fs friendly

[Marco Costalba]

On 2/9/07, Linus Torvalds <torvalds@linux-foundation.org> wrote:
>
>

> And you wouldn't think that it really needs write access, and you'd be
> largely correct, EXCEPT for the fact that git status actually does a
> refresh of the index, to make sure that we don't claim that something is
> dirty just because somebody has touched the file.
>
> IOW, there's an implicit "git update-index --refresh" as part of
> calculating the status, and that's the thing that wants to lock the index
> file (and thus write to the filesystem).
>
------ cut ------
>
> You *can* just use "git-runstatus" instead. That's the command that
> actually does all the heavy lifting. But you can see the difference by
> doing this:
>
>         touch Makefile
>         git runstatus
>
> vs
>
>         touch Makefile
>         git status
>
> Notice how the "runstatus" one claims that Makefile is "modified:". That's
> exactly because it doesn't do the index refresh.
>
Sorry, perhaps it is a silly question, but why git index should be
different after just touching a file?

IOW is it not possible that "git update-index --refresh" exists
without modifing the index, just because ther's nothing to modify?

So, finally, could be possible making "git status" taking the lock
only _after_ has checked there's something new to write to the index?
So to avoid write access in most cases ? (expecially with repo mounted
on a read-only fs)

Thanks
Marco

Re: 'git status' is not read-only fs friendly

[Junio C Hamano]

"Marco Costalba" <mcostalba@gmail.com> writes:

> Sorry, perhaps it is a silly question, but why git index should be
> different after just touching a file?

It uses the information from lstat(2) that is stored in the
index and taken from the filesystem -- if they are different
(touch changes it as you know, but so does "vi that-file") they
are reported as "modified".

	git checkout -- Makefile
        git diff-files Makefile
	touch Makefile
	git diff-files Makefile

The 'refresh' operation is to update the lstat(2) information in
the index for paths whose contents actually match what is in the
index (and it does not do anything else -- most notably paths
whose contents are different between the working tree and the
index are left as-is).

Re: 'git status' is not read-only fs friendly

[Junio C Hamano]

Linus Torvalds <torvalds@linux-foundation.org> writes:

> "git status" doesn't "pretend" to write stuff. It really does. 
>
> You *can* just use "git-runstatus" instead. That's the command that 
> actually does all the heavy lifting. But you can see the difference by 
> doing this:
>
> 	touch Makefile
> 	git runstatus
>
> vs
>
> 	touch Makefile
> 	git status
>
> Notice how the "runstatus" one claims that Makefile is "modified:". That's 
> exactly because it doesn't do the index refresh.

Running refresh internally in runstatus without writing the
result out _might_ be an option, but that would largely be
a hack to only help qgit.

Other shapes of "git status", such as "git status <filename>"
and "git status -a", still need to perform the same index
manipulation as "git commit" with the same parameters before
calling git-runstatus, and at that point the extra "internal
refresh" in runstatus is an unwelcome extra cycle.

Re: 'git status' is not read-only fs friendly

[Morten Welinder]

> Running refresh internally in runstatus without writing the
> result out _might_ be an option, but that would largely be
> a hack to only help qgit.

I might be overlooking something, but couldn't that updated index be
saved elsewhere?  And subcommands be pointed at that, of course.

Morten

Re: 'git status' is not read-only fs friendly

[Theodore Tso]

On Fri, Feb 09, 2007 at 03:29:16PM -0500, Morten Welinder wrote:
> >Running refresh internally in runstatus without writing the
> >result out _might_ be an option, but that would largely be
> >a hack to only help qgit.
> 
> I might be overlooking something, but couldn't that updated index be
> saved elsewhere?  And subcommands be pointed at that, of course.

You should be able to set the GIT_INDEX_FILE environtment variable to
point the index somewhere else than $GIT_DIR/index, so it's not
located on a read-only filesystem.  See the git(7) man page.

						- Ted

Re: 'git status' is not read-only fs friendly

[Marco Costalba]

On 2/9/07, Junio C Hamano <junkio@cox.net> wrote:
> Linus Torvalds <torvalds@linux-foundation.org> writes:
>
> > "git status" doesn't "pretend" to write stuff. It really does.
> >
> > You *can* just use "git-runstatus" instead. That's the command that
> > actually does all the heavy lifting. But you can see the difference by
> > doing this:
> >
> >       touch Makefile
> >       git runstatus
> >
> > vs
> >
> >       touch Makefile
> >       git status
> >
> > Notice how the "runstatus" one claims that Makefile is "modified:". That's
> > exactly because it doesn't do the index refresh.
>
> Running refresh internally in runstatus without writing the
> result out _might_ be an option, but that would largely be
> a hack to only help qgit.
>

Yes, I agree.

If I modify qgit in running 'git runstatus' as a fallback in case 'git
status' exits with an error (without checking what kind of error
exactly) could be an acceptable path or could hide subtle
side-effects? I have no the knowledge to answer this by hand.

Thanks
Marco

Re: 'git status' is not read-only fs friendly

[Linus Torvalds]

On Fri, 9 Feb 2007, Marco Costalba wrote:
> 
> If I modify qgit in running 'git runstatus' as a fallback in case 'git
> status' exits with an error (without checking what kind of error
> exactly) could be an acceptable path or could hide subtle
> side-effects? I have no the knowledge to answer this by hand.

It's probably better for you to just

 - run "git update-index --refresh" and don't care about the exit value
 - run "git runstatus" unconditionally

which should basically get you something working.

HOWEVER, it's also quite possible that "git-commit.sh" should just do this 
on its own. If the update-index fails, we really only care if we literally 
use the index later to *write* something, ie the commit case. For just 
"git status", maybe we should just silently ignore the error..

		Linus

Re: 'git status' is not read-only fs friendly

[Junio C Hamano]

Linus Torvalds <torvalds@linux-foundation.org> writes:

> On Fri, 9 Feb 2007, Marco Costalba wrote:
>> 
>> If I modify qgit in running 'git runstatus' as a fallback in case 'git
>> status' exits with an error (without checking what kind of error
>> exactly) could be an acceptable path or could hide subtle
>> side-effects? I have no the knowledge to answer this by hand.
>
> It's probably better for you to just
>
>  - run "git update-index --refresh" and don't care about the exit value
>  - run "git runstatus" unconditionally
>
> which should basically get you something working.

We could do "git runstatus --refresh", which only updates the
index in-core.

The patch does two things.

 - it changes the calling convention of run_diff_files() and
   run_diff_index(); earlier, they read the index on their own,
   but now we expect the caller to have populated the index by
   calling read_cache().

 - it adds --refresh flag to git-runstatus, and before
   git-runstatus calls read_cache() to satisfy the updated
   calling convention of run_diff_files() and run_diff_index(),
   it refreshes the in-core copy of index.

---
diff --git a/builtin-diff-files.c b/builtin-diff-files.c
index 5d4a5c5..3ee2605 100644
--- a/builtin-diff-files.c
+++ b/builtin-diff-files.c
@@ -47,5 +47,9 @@ int cmd_diff_files(int argc, const char **argv, const char *prefix)
 	if (rev.pending.nr ||
 	    rev.min_age != -1 || rev.max_age != -1)
 		usage(diff_files_usage);
+	if (read_cache() < 0) {
+		perror("read_cache");
+		return -1;
+	}
 	return run_diff_files(&rev, silent);
 }
diff --git a/builtin-diff-index.c b/builtin-diff-index.c
index 95a3db1..083599d 100644
--- a/builtin-diff-index.c
+++ b/builtin-diff-index.c
@@ -38,5 +38,9 @@ int cmd_diff_index(int argc, const char **argv, const char *prefix)
 	if (rev.pending.nr != 1 ||
 	    rev.max_count != -1 || rev.min_age != -1 || rev.max_age != -1)
 		usage(diff_cache_usage);
+	if (read_cache() < 0) {
+		perror("read_cache");
+		return -1;
+	}
 	return run_diff_index(&rev, cached);
 }
diff --git a/builtin-diff.c b/builtin-diff.c
index a659020..12d11f0 100644
--- a/builtin-diff.c
+++ b/builtin-diff.c
@@ -56,6 +56,10 @@ static int builtin_diff_files(struct rev_info *revs,
 	if (revs->max_count < 0 &&
 	    (revs->diffopt.output_format & DIFF_FORMAT_PATCH))
 		revs->combine_merges = revs->dense_combined_merges = 1;
+	if (read_cache() < 0) {
+		perror("read_cache");
+		return -1;
+	}
 	return run_diff_files(revs, silent);
 }
 
@@ -151,6 +155,10 @@ static int builtin_diff_index(struct rev_info *revs,
 	    revs->max_count != -1 || revs->min_age != -1 ||
 	    revs->max_age != -1)
 		usage(builtin_diff_usage);
+	if (read_cache() < 0) {
+		perror("read_cache");
+		return -1;
+	}
 	return run_diff_index(revs, cached);
 }
 
diff --git a/builtin-runstatus.c b/builtin-runstatus.c
index 4b489b1..df66742 100644
--- a/builtin-runstatus.c
+++ b/builtin-runstatus.c
@@ -4,7 +4,7 @@
 extern int wt_status_use_color;
 
 static const char runstatus_usage[] =
-"git-runstatus [--color|--nocolor] [--amend] [--verbose] [--untracked]";
+"git-runstatus [--color|--nocolor] [--refresh] [--amend] [--verbose] [--untracked]";
 
 int cmd_runstatus(int argc, const char **argv, const char *prefix)
 {
@@ -17,6 +17,8 @@ int cmd_runstatus(int argc, const char **argv, const char *prefix)
 	for (i = 1; i < argc; i++) {
 		if (!strcmp(argv[i], "--color"))
 			wt_status_use_color = 1;
+		else if (!strcmp(argv[i], "--refresh"))
+			s.refresh = 1;
 		else if (!strcmp(argv[i], "--nocolor"))
 			wt_status_use_color = 0;
 		else if (!strcmp(argv[i], "--amend")) {
diff --git a/diff-lib.c b/diff-lib.c
index 91cd877..278ba79 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -20,11 +20,7 @@ int run_diff_files(struct rev_info *revs, int silent_on_removed)
 
 	if (diff_unmerged_stage < 0)
 		diff_unmerged_stage = 2;
-	entries = read_cache();
-	if (entries < 0) {
-		perror("read_cache");
-		return -1;
-	}
+	entries = active_nr;
 	for (i = 0; i < entries; i++) {
 		struct stat st;
 		unsigned int oldmode, newmode;
@@ -354,10 +350,6 @@ int run_diff_index(struct rev_info *revs, int cached)
 	if (!revs->ignore_merges)
 		match_missing = 1;
 
-	if (read_cache() < 0) {
-		perror("read_cache");
-		return -1;
-	}
 	mark_merge_entries();
 
 	ent = revs->pending.objects[0].item;
diff --git a/wt-status.c b/wt-status.c
index 5567868..58186d6 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -47,17 +47,10 @@ void wt_status_prepare(struct wt_status *s)
 	unsigned char sha1[20];
 	const char *head;
 
+	memset(s, 0, sizeof(*s));
 	head = resolve_ref("HEAD", sha1, 0, NULL);
 	s->branch = head ? xstrdup(head) : NULL;
-
 	s->reference = "HEAD";
-	s->amend = 0;
-	s->verbose = 0;
-	s->untracked = 0;
-
-	s->commitable = 0;
-	s->workdir_dirty = 0;
-	s->workdir_untracked = 0;
 }
 
 static void wt_status_print_cached_header(const char *reference)
@@ -198,12 +191,22 @@ static void wt_status_print_changed_cb(struct diff_queue_struct *q,
 		wt_status_print_trailer();
 }
 
+static void wt_read_cache(struct wt_status *s)
+{
+	discard_cache();
+	read_cache();
+	if (s->refresh)
+		refresh_cache(0);
+}
+
 void wt_status_print_initial(struct wt_status *s)
 {
 	int i;
 	char buf[PATH_MAX];
 
-	read_cache();
+	wt_read_cache(s);
+	if (s->refresh)
+		refresh_cache(REFRESH_QUIET);
 	if (active_nr) {
 		s->commitable = 1;
 		wt_status_print_cached_header(NULL);
@@ -227,6 +230,7 @@ static void wt_status_print_updated(struct wt_status *s)
 	rev.diffopt.format_callback = wt_status_print_updated_cb;
 	rev.diffopt.format_callback_data = s;
 	rev.diffopt.detect_rename = 1;
+	wt_read_cache(s);
 	run_diff_index(&rev, 1);
 }
 
@@ -238,6 +242,7 @@ static void wt_status_print_changed(struct wt_status *s)
 	rev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;
 	rev.diffopt.format_callback = wt_status_print_changed_cb;
 	rev.diffopt.format_callback_data = s;
+	wt_read_cache(s);
 	run_diff_files(&rev, 0);
 }
 
@@ -294,6 +299,7 @@ static void wt_status_print_verbose(struct wt_status *s)
 	setup_revisions(0, NULL, &rev, s->reference);
 	rev.diffopt.output_format |= DIFF_FORMAT_PATCH;
 	rev.diffopt.detect_rename = 1;
+	wt_read_cache(s);
 	run_diff_index(&rev, 1);
 }
 
@@ -323,7 +329,6 @@ void wt_status_print(struct wt_status *s)
 	}
 	else {
 		wt_status_print_updated(s);
-		discard_cache();
 	}
 
 	wt_status_print_changed(s);
diff --git a/wt-status.h b/wt-status.h
index cfea4ae..680a0ca 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -15,6 +15,7 @@ struct wt_status {
 	int verbose;
 	int amend;
 	int untracked;
+	int refresh;
 	/* These are computed during processing of the individual sections */
 	int commitable;
 	int workdir_dirty;

Re: 'git status' is not read-only fs friendly

[Junio C Hamano]

Junio C Hamano <junkio@cox.net> writes:

> We could do "git runstatus --refresh", which only updates the
> index in-core.
>
> The patch does two things.

Ah, crap.  Sorry for a wip.

[PATCH 1/2] run_diff_{files,index}(): update calling convention.

[Junio C Hamano]

They used to open and read index themselves, but they now expect
their callers to do so.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 * This is preparatoin for the next one...

 builtin-diff-files.c |    4 ++++
 builtin-diff-index.c |    4 ++++
 builtin-diff.c       |    8 ++++++++
 diff-lib.c           |   10 +---------
 wt-status.c          |   12 ++++++++++--
 5 files changed, 27 insertions(+), 11 deletions(-)

diff --git a/builtin-diff-files.c b/builtin-diff-files.c
index 5d4a5c5..3ee2605 100644
--- a/builtin-diff-files.c
+++ b/builtin-diff-files.c
@@ -47,5 +47,9 @@ int cmd_diff_files(int argc, const char **argv, const char *prefix)
 	if (rev.pending.nr ||
 	    rev.min_age != -1 || rev.max_age != -1)
 		usage(diff_files_usage);
+	if (read_cache() < 0) {
+		perror("read_cache");
+		return -1;
+	}
 	return run_diff_files(&rev, silent);
 }
diff --git a/builtin-diff-index.c b/builtin-diff-index.c
index 95a3db1..083599d 100644
--- a/builtin-diff-index.c
+++ b/builtin-diff-index.c
@@ -38,5 +38,9 @@ int cmd_diff_index(int argc, const char **argv, const char *prefix)
 	if (rev.pending.nr != 1 ||
 	    rev.max_count != -1 || rev.min_age != -1 || rev.max_age != -1)
 		usage(diff_cache_usage);
+	if (read_cache() < 0) {
+		perror("read_cache");
+		return -1;
+	}
 	return run_diff_index(&rev, cached);
 }
diff --git a/builtin-diff.c b/builtin-diff.c
index a659020..12d11f0 100644
--- a/builtin-diff.c
+++ b/builtin-diff.c
@@ -56,6 +56,10 @@ static int builtin_diff_files(struct rev_info *revs,
 	if (revs->max_count < 0 &&
 	    (revs->diffopt.output_format & DIFF_FORMAT_PATCH))
 		revs->combine_merges = revs->dense_combined_merges = 1;
+	if (read_cache() < 0) {
+		perror("read_cache");
+		return -1;
+	}
 	return run_diff_files(revs, silent);
 }
 
@@ -151,6 +155,10 @@ static int builtin_diff_index(struct rev_info *revs,
 	    revs->max_count != -1 || revs->min_age != -1 ||
 	    revs->max_age != -1)
 		usage(builtin_diff_usage);
+	if (read_cache() < 0) {
+		perror("read_cache");
+		return -1;
+	}
 	return run_diff_index(revs, cached);
 }
 
diff --git a/diff-lib.c b/diff-lib.c
index 91cd877..278ba79 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -20,11 +20,7 @@ int run_diff_files(struct rev_info *revs, int silent_on_removed)
 
 	if (diff_unmerged_stage < 0)
 		diff_unmerged_stage = 2;
-	entries = read_cache();
-	if (entries < 0) {
-		perror("read_cache");
-		return -1;
-	}
+	entries = active_nr;
 	for (i = 0; i < entries; i++) {
 		struct stat st;
 		unsigned int oldmode, newmode;
@@ -354,10 +350,6 @@ int run_diff_index(struct rev_info *revs, int cached)
 	if (!revs->ignore_merges)
 		match_missing = 1;
 
-	if (read_cache() < 0) {
-		perror("read_cache");
-		return -1;
-	}
 	mark_merge_entries();
 
 	ent = revs->pending.objects[0].item;
diff --git a/wt-status.c b/wt-status.c
index 2879c3d..e346511 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -191,12 +191,18 @@ static void wt_status_print_changed_cb(struct diff_queue_struct *q,
 		wt_status_print_trailer();
 }
 
+static void wt_read_cache(struct wt_status *s)
+{
+	discard_cache();
+	read_cache();
+}
+
 void wt_status_print_initial(struct wt_status *s)
 {
 	int i;
 	char buf[PATH_MAX];
 
-	read_cache();
+	wt_read_cache(s);
 	if (active_nr) {
 		s->commitable = 1;
 		wt_status_print_cached_header(NULL);
@@ -220,6 +226,7 @@ static void wt_status_print_updated(struct wt_status *s)
 	rev.diffopt.format_callback = wt_status_print_updated_cb;
 	rev.diffopt.format_callback_data = s;
 	rev.diffopt.detect_rename = 1;
+	wt_read_cache(s);
 	run_diff_index(&rev, 1);
 }
 
@@ -231,6 +238,7 @@ static void wt_status_print_changed(struct wt_status *s)
 	rev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;
 	rev.diffopt.format_callback = wt_status_print_changed_cb;
 	rev.diffopt.format_callback_data = s;
+	wt_read_cache(s);
 	run_diff_files(&rev, 0);
 }
 
@@ -287,6 +295,7 @@ static void wt_status_print_verbose(struct wt_status *s)
 	setup_revisions(0, NULL, &rev, s->reference);
 	rev.diffopt.output_format |= DIFF_FORMAT_PATCH;
 	rev.diffopt.detect_rename = 1;
+	wt_read_cache(s);
 	run_diff_index(&rev, 1);
 }
 
@@ -316,7 +325,6 @@ void wt_status_print(struct wt_status *s)
 	}
 	else {
 		wt_status_print_updated(s);
-		discard_cache();
 	}
 
 	wt_status_print_changed(s);
-- 
1.5.0.rc4.26.gcc46

Re: [PATCH 1/2] run_diff_{files,index}(): update calling convention.

[Marco Costalba]

On 2/10/07, Junio C Hamano <junkio@cox.net> wrote:
> They used to open and read index themselves, but they now expect
> their callers to do so.
>
> Signed-off-by: Junio C Hamano <junkio@cox.net>
> ---

Thanks this works for me, while the other workaround seems to have issues:

bash-3.1$ git status
fatal: unable to create '.git/index.lock': Read-only file system
bash-3.1$ git update-index --refresh
fatal: unable to create '.git/index.lock': Read-only file system
bash-3.1$ git runstatus
# On branch master
# Changed but not updated:
#   (use "git add <file>..." to update what will be committed)
#
#       modified:   .gitignore
#       modified:   README
#       modified:   exception_manager.txt

  ------ cut -------

#       src/object_script.qgit.Release
#       src/object_script.qgit_bin.Debug
#       src/object_script.qgit_bin.Release
#       start_qgit.bat
no changes added to commit (use "git add" and/or "git commit -a")


bash-3.1$  git-runstatus --refresh
# On branch master
# Untracked files:
#   (use "git add <file>..." to include in what will be committed)
#
#       AAA_convert.xls.lnk
#       Cygwin.lnk
#       Qt 4.2.2 Command Prompt.lnk
#       bld.bat
#       cmd.txt
#       explore2fs.exe.lnk
#       qgit.lnk
#       src/object_script.qgit.Debug
#       src/object_script.qgit.Release
#       src/object_script.qgit_bin.Debug
#       src/object_script.qgit_bin.Release
#       start_qgit.bat
nothing added to commit but untracked files present (use "git add" to track)
bash-3.1$

Running 'git runstatus' alone shows _all_ the repo files, although are
not modified and not touched. With 'git runstatus' --refresh'
everything seems ok.

Please could you apply your patch before 1.5 so that I can update qgit
and change prerequisite git version to 1.5.

Thanks
Marco

Re: [PATCH 1/2] run_diff_{files,index}(): update calling convention.

[Junio C Hamano]

"Marco Costalba" <mcostalba@gmail.com> writes:

> Running 'git runstatus' alone shows _all_ the repo files, although are
> not modified and not touched.

That does not sound right with or without my patch.  Are you
talking about the 'git runstatus' with my patch is showing
paths that are cache clean?  If that is the case that means my
patch is introudcing regression.

Re: [PATCH 1/2] run_diff_{files,index}(): update calling convention.

[Marco Costalba]

On 2/10/07, Junio C Hamano <junkio@cox.net> wrote:
> "Marco Costalba" <mcostalba@gmail.com> writes:
>
> > Running 'git runstatus' alone shows _all_ the repo files, although are
> > not modified and not touched.
>
> That does not sound right with or without my patch.  Are you
> talking about the 'git runstatus' with my patch is showing
> paths that are cache clean?  If that is the case that means my
> patch is introudcing regression.
>
>
'git runstatus'  shows all the files also _before_ your patch has been
applied (I have tested again now resetting HEAD so to remove your two
patches).

What is strange is that running 'git runstatus' on Linux on a repo
under a ntfs directory shows all the files, while running 'git status'
under windows under the same repo (so now we have write access) shows
things correctly.

It seems that perhaps lstat(2) info of a mounted ntfs directory is not
correct?????? very very  strange!

Re: [PATCH 1/2] run_diff_{files,index}(): update calling convention.

[Marco Costalba]

On 2/10/07, Marco Costalba <mcostalba@gmail.com> wrote:
> >
> 'git runstatus'  shows all the files also _before_ your patch has been
> applied (I have tested again now resetting HEAD so to remove your two
> patches).
>

This is the complete log under Windows (git with Cygwin distribution,
but run _outside_ cygwin shell, directly in Windows cmd.exe shell).

And _after_ under Linus, current git tree _without_ your last patches applied.

***************  UNDER WINDOWS ***********************

C:\varie\c\qgit4>git status
# Untracked files:
#   (use "git add" to add to commit)
#
#	AAA_convert.xls.lnk
#	Cygwin.lnk
#	Qt 4.2.2 Command Prompt.lnk
#	bld.bat
#	cmd.txt
#	explore2fs.exe.lnk
#	log.txt
#	qgit.lnk
#	src/object_script.qgit.Debug
#	src/object_script.qgit.Release
#	src/object_script.qgit_bin.Debug
#	src/object_script.qgit_bin.Release
#	start_qgit.bat
nothing to commit

C:\varie\c\qgit4>git runstatus
# Untracked files:
#   (use "git add" to add to commit)
#
#	AAA_convert.xls.lnk
#	Cygwin.lnk
#	Qt 4.2.2 Command Prompt.lnk
#	bld.bat
#	cmd.txt
#	explore2fs.exe.lnk
#	log.txt
#	qgit.lnk
#	src/object_script.qgit.Debug
#	src/object_script.qgit.Release
#	src/object_script.qgit_bin.Debug
#	src/object_script.qgit_bin.Release
#	start_qgit.bat
nothing to commit


***************  UNDER LINUX **************************

bash-3.1$ git status
fatal: unable to create '.git/index.lock': Read-only file system
bash-3.1$ git runstatus
# On branch master
# Changed but not updated:
#   (use "git add <file>..." to update what will be committed)
#
#       modified:   .gitignore
#       modified:   README
#       modified:   exception_manager.txt
#       modified:   qgit.pro
#       modified:   src/annotate.cpp
#       modified:   src/annotate.h
#       modified:   src/cache.cpp
#       modified:   src/cache.h
#       modified:   src/commit.ui
#       modified:   src/commitimpl.cpp

-------------  cut (all remaining files) -----------------------

#       modified:   src/resources/source_h.png
#       modified:   src/resources/source_java.png
#       modified:   src/resources/source_pl.png
#       modified:   src/resources/source_py.png
#       modified:   src/resources/tab_remove.png
#       modified:   src/resources/tar.png
#       modified:   src/resources/txt.png
#       modified:   src/resources/view_choose.png
#       modified:   src/resources/view_top_bottom.png
#       modified:   src/resources/view_tree.png
#       modified:   src/resources/wizard.png
#       modified:   src/revdesc.cpp
#       modified:   src/revdesc.h
#       modified:   src/revsview.cpp
#       modified:   src/revsview.h
#       modified:   src/revsview.ui
#       modified:   src/settings.ui
#       modified:   src/settingsimpl.cpp
#       modified:   src/settingsimpl.h
#       modified:   src/src.pro
#       modified:   src/todo.txt
#       modified:   src/treeview.cpp
#       modified:   src/treeview.h
#
# Untracked files:
#   (use "git add <file>..." to include in what will be committed)
#
#       AAA_convert.xls.lnk
#       Cygwin.lnk
#       Qt 4.2.2 Command Prompt.lnk
#       bld.bat
#       cmd.txt
#       explore2fs.exe.lnk
#       log.txt
#       qgit.lnk
#       src/object_script.qgit.Debug
#       src/object_script.qgit.Release
#       src/object_script.qgit_bin.Debug
#       src/object_script.qgit_bin.Release
no changes added to commit (use "git add" and/or "git commit -a")
bash-3.1$

Re: [PATCH 1/2] run_diff_{files,index}(): update calling convention.

[Junio C Hamano]

I suspect that you are dual booting and browsing a git
repository that is on a read-only mounted NTFS filesystem from
the Linux side, and the index was created on Cygwin git?

I (perhaps luckily) am fairly ignorant on the way things done in
Windows environment.  For one thing, I do not know if NTFS has
notion of i-number, file owner uid, and other information that
are used in the index (not that I want to know).

If NTFS does not support the information returned by lstat(2)
fully on disk, I would imagine Cygwin and NTFS filesystem driver
in the Linux kernel need to fake some fields that NTFS does not
natively store, and if the value faked by Cygwin and NTFS driver
in the Linux kernel disagree, then it is not at all surprising
to see if an unmodified path shows up as cache-dirty.

Re: [PATCH 1/2] run_diff_{files,index}(): update calling convention.

[Marco Costalba]

On 2/10/07, Junio C Hamano <junkio@cox.net> wrote:
> I suspect that you are dual booting and browsing a git
> repository that is on a read-only mounted NTFS filesystem from
> the Linux side, and the index was created on Cygwin git?
>

Yes. That's it.

> I (perhaps luckily) am fairly ignorant on the way things done in
> Windows environment.  For one thing, I do not know if NTFS has
> notion of i-number, file owner uid, and other information that
> are used in the index (not that I want to know).
>
> If NTFS does not support the information returned by lstat(2)
> fully on disk, I would imagine Cygwin and NTFS filesystem driver
> in the Linux kernel need to fake some fields that NTFS does not
> natively store, and if the value faked by Cygwin and NTFS driver
> in the Linux kernel disagree, then it is not at all surprising
> to see if an unmodified path shows up as cache-dirty.
>

So in this case your patch that introduce '--refresh' option in 'git
runstatus' is not just a shortcut for 'git update-index' + 'git
runstatus' but adds some real value.

One more reason for asking you to add it before 1.5 release ;-)

Re: [PATCH 1/2] run_diff_{files,index}(): update calling convention.

[Junio C Hamano]

"Marco Costalba" <mcostalba@gmail.com> writes:

> So in this case your patch that introduce '--refresh' option in 'git
> runstatus' is not just a shortcut for 'git update-index' + 'git
> runstatus' but adds some real value.
>
> One more reason for asking you to add it before 1.5 release ;-)

The thing is, it touches central part of the system by changing
the calling convention of two rather important functions.  You
might have already fully tested that there is no regression for
git-runstatus, but it affects other callers as well.  I tried to
be careful when I did the conversion but I am not 100% sure
there is no "unintended side effects".

Re: [PATCH 1/2] run_diff_{files,index}(): update calling convention.

[Marco Costalba]

On 2/10/07, Junio C Hamano <junkio@cox.net> wrote:
> "Marco Costalba" <mcostalba@gmail.com> writes:
>
> > So in this case your patch that introduce '--refresh' option in 'git
> > runstatus' is not just a shortcut for 'git update-index' + 'git
> > runstatus' but adds some real value.
> >
> > One more reason for asking you to add it before 1.5 release ;-)
>
> The thing is, it touches central part of the system by changing
> the calling convention of two rather important functions.  You
> might have already fully tested that there is no regression for
> git-runstatus, but it affects other callers as well.  I tried to
> be careful when I did the conversion but I am not 100% sure
> there is no "unintended side effects".
>

I understand this.

[PATCH 2/2] git-runstatus --refresh

[Junio C Hamano]

This teaches git-runstatus a new option --refresh to refresh the
index in-core.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 * If a cache has 20k+ paths, this could be rather expensive
   because git-status already have refreshed the entries for us,
   so I am reluctant to make this the default, but obviously
   then qgit needs to know if it has runstatus that knows about
   this option.

 builtin-runstatus.c |    4 +++-
 wt-status.c         |    2 ++
 wt-status.h         |    1 +
 3 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/builtin-runstatus.c b/builtin-runstatus.c
index 4b489b1..df66742 100644
--- a/builtin-runstatus.c
+++ b/builtin-runstatus.c
@@ -4,7 +4,7 @@
 extern int wt_status_use_color;
 
 static const char runstatus_usage[] =
-"git-runstatus [--color|--nocolor] [--amend] [--verbose] [--untracked]";
+"git-runstatus [--color|--nocolor] [--refresh] [--amend] [--verbose] [--untracked]";
 
 int cmd_runstatus(int argc, const char **argv, const char *prefix)
 {
@@ -17,6 +17,8 @@ int cmd_runstatus(int argc, const char **argv, const char *prefix)
 	for (i = 1; i < argc; i++) {
 		if (!strcmp(argv[i], "--color"))
 			wt_status_use_color = 1;
+		else if (!strcmp(argv[i], "--refresh"))
+			s.refresh = 1;
 		else if (!strcmp(argv[i], "--nocolor"))
 			wt_status_use_color = 0;
 		else if (!strcmp(argv[i], "--amend")) {
diff --git a/wt-status.c b/wt-status.c
index e346511..27c228b 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -195,6 +195,8 @@ static void wt_read_cache(struct wt_status *s)
 {
 	discard_cache();
 	read_cache();
+	if (s->refresh)
+		refresh_cache(REFRESH_QUIET);
 }
 
 void wt_status_print_initial(struct wt_status *s)
diff --git a/wt-status.h b/wt-status.h
index cfea4ae..680a0ca 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -15,6 +15,7 @@ struct wt_status {
 	int verbose;
 	int amend;
 	int untracked;
+	int refresh;
 	/* These are computed during processing of the individual sections */
 	int commitable;
 	int workdir_dirty;
-- 
1.5.0.rc4.26.gcc46

Re: 'git status' is not read-only fs friendly

[Johannes Schindelin]

Hi,

On Fri, 9 Feb 2007, Marco Costalba wrote:

> I just need to know if current working directory is clean and report 
> back to qgit user, so read-only access would be ok for me.

So, why don't you just do a

	git diff --name-only HEAD

and check for an empty output???

No need for a patch to Git (so late in the -rc phase), or backwards 
incompatibility...

Ciao,
Dscho

Re: 'git status' is not read-only fs friendly

[Marco Costalba]

On 2/10/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> Hi,
>
> On Fri, 9 Feb 2007, Marco Costalba wrote:
>
> > I just need to know if current working directory is clean and report
> > back to qgit user, so read-only access would be ok for me.
>
> So, why don't you just do a
>
>         git diff --name-only HEAD
>
> and check for an empty output???
>

It seems to have the same issues of 'git runstatus' in case of ntfs
filesystems, so I would prefer, eventually, use 'git runstatus' that
at least gives me index status of the files.

> No need for a patch to Git (so late in the -rc phase), or backwards
> incompatibility...
>

Well, it's a _new_ option so I fail to see backwards incompatibility.
Perhaps you are referring to qgit backward incompatibility, but in any
case a new version of qgit is due to fix a parsing bug that shows
after a modification of 'git rev-list' output occurred in git 1.5
development.

  Marco

Re: 'git status' is not read-only fs friendly

[Johannes Schindelin]

Hi,

On Sat, 10 Feb 2007, Marco Costalba wrote:

> On 2/10/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > On Fri, 9 Feb 2007, Marco Costalba wrote:
> > 
> > > I just need to know if current working directory is clean and report
> > > back to qgit user, so read-only access would be ok for me.
> > 
> > So, why don't you just do a
> > 
> >         git diff --name-only HEAD
> > 
> > and check for an empty output???
> 
> It seems to have the same issues of 'git runstatus' in case of ntfs 
> filesystems, so I would prefer, eventually, use 'git runstatus' that at 
> least gives me index status of the files.

Which issues? That the lstat data are not equal on Cygwin and Linux? The 
patch does not help here. Maybe a patch to Linux' ntfs driver 
would, but I fail to see how Git could possibly help here.

If git-diff is trying to write files, _that_ would be a bug.

As for your use of git-status: I think it is wrong. You said you want to 
check if the working directory is clean. Then just do that, and do not try 
to generate the message meant for editing the commit message.

> > No need for a patch to Git (so late in the -rc phase), or backwards 
> > incompatibility...
> 
> Well, it's a _new_ option so I fail to see backwards incompatibility.

You need a new version of _Git_ if you use that option.

And if you can do without depending on a newer Git, it is _bad_ to do it 
nevertheless.

Ciao,
Dscho

Re: 'git status' is not read-only fs friendly

[Marco Costalba]

On 2/10/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> Hi,
>
> On Sat, 10 Feb 2007, Marco Costalba wrote:
>
> > On 2/10/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > > On Fri, 9 Feb 2007, Marco Costalba wrote:
> > >
> > > > I just need to know if current working directory is clean and report
> > > > back to qgit user, so read-only access would be ok for me.
> > >
> > > So, why don't you just do a
> > >
> > >         git diff --name-only HEAD
> > >
> > > and check for an empty output???
> >
> > It seems to have the same issues of 'git runstatus' in case of ntfs
> > filesystems, so I would prefer, eventually, use 'git runstatus' that at
> > least gives me index status of the files.
>
> Which issues? That the lstat data are not equal on Cygwin and Linux? The
> patch does not help here.

Well, I tested the patch and indeed it helps a lot ;-)

It's correct that checking Linux lstat against cygwin one (stored in
index) gives different results, but it's now where the patch makes the
difference rechecking all the files (in memory) to see if are really
changed and discarding false positives created by lstat issues.

> Maybe a patch to Linux' ntfs driver
> would, but I fail to see how Git could possibly help here.

>
> You need a new version of _Git_ if you use that option.
>

That's a true point. Altough if git 1.5 ships _without_ '--refresh'
option in 'git runstatus' for a porcelain tool point of view it means
*do forget* that option until next major release. There's no point in
adding the feature one day after git 1.5 is out; qgit will not use
that feature anyway for next months.


   Marco

Re: 'git status' is not read-only fs friendly

[Marco Costalba]

On 2/10/07, Marco Costalba <mcostalba@gmail.com> wrote:
> >
> > You need a new version of _Git_ if you use that option.
> >
>
> That's a true point. Altough if git 1.5 ships _without_ '--refresh'
> option in 'git runstatus' for a porcelain tool point of view it means
> *do forget* that option until next major release. There's no point in
> adding the feature one day after git 1.5 is out; qgit will not use
> that feature anyway for next months.
>

I could opt for shipping qgit 1.5.5 _without_ using '--refresh' and
then ship, as example in a month, qgit 1.5.6 that uses the feaure. But
I can do this _only_ if git 1.5 has it.

Re: 'git status' is not read-only fs friendly

[Junio C Hamano]

"Marco Costalba" <mcostalba@gmail.com> writes:

>> That's a true point. Altough if git 1.5 ships _without_ '--refresh'
>> option in 'git runstatus' for a porcelain tool point of view it means
>> *do forget* that option until next major release. There's no point in
>> adding the feature one day after git 1.5 is out; qgit will not use
>> that feature anyway for next months.
>
> I could opt for shipping qgit 1.5.5 _without_ using '--refresh' and
> then ship, as example in a month, qgit 1.5.6 that uses the feaure. But
> I can do this _only_ if git 1.5 has it.

You can run it once when you start up to see if --refresh is
supported with the git the user has, and keep that result
throughout the life of the qgit process (so you have to do the
check only once).

What would you do when working with older git anyway?  You would
need to fall back on some code -- or would you require a certain
version of git to go with this version of qgit?

About "Cygwin and Linux NTFS seem to disagree with lstat(2)"
problem.  Is it really what is happening here?  It might be an
interesting exercise to printf(3) the struct stat members in
both environments and find out where they disagree.  It might
turn out to be something trivial to fix for the filesystem guys.
I am not personally interested in solving that problem myself
(nor I am equipped to -- I do not have ready access to dual
booting Windows setup), but still it is interesting to find out
what the issue is.

Re: 'git status' is not read-only fs friendly

[Johannes Schindelin]

Hi,

On Sat, 10 Feb 2007, Junio C Hamano wrote:

> About "Cygwin and Linux NTFS seem to disagree with lstat(2)"
> problem.  Is it really what is happening here?

Probably. AFAIR Windows lacks some important information, which is filled 
with zeroes by Cygwin.

Note that this problem already arises between Cygwin and MinGW, and it 
cannot be fixed: Cygwin _has_ symlinks, while MinGW has _not_.

Ciao,
Dscho

Re: 'git status' is not read-only fs friendly

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> On Sat, 10 Feb 2007, Junio C Hamano wrote:
>
>> About "Cygwin and Linux NTFS seem to disagree with lstat(2)"
>> problem.  Is it really what is happening here?
>
> Probably. AFAIR Windows lacks some important information, which is filled 
> with zeroes by Cygwin.

If NTFS driver in the Linux kernel is filling that with zeroes
the same way then there won't be differences, right?

Re: 'git status' is not read-only fs friendly

[Johannes Schindelin]

Hi,

On Sun, 11 Feb 2007, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > On Sat, 10 Feb 2007, Junio C Hamano wrote:
> >
> >> About "Cygwin and Linux NTFS seem to disagree with lstat(2)"
> >> problem.  Is it really what is happening here?
> >
> > Probably. AFAIR Windows lacks some important information, which is filled 
> > with zeroes by Cygwin.
> 
> If NTFS driver in the Linux kernel is filling that with zeroes
> the same way then there won't be differences, right?

Maybe. Although I am quite certain that you'd break something by that.

But after all, this is one really obscure corner case you have there, and 
you are not really working on the repository on Linux either, since you 
have it mounted readonly.

I absolutely have no intention to "fix" performance or other issues for 
that case.

Ciao,
Dscho

Re: 'git status' is not read-only fs friendly

[Johannes Schindelin]

On Sun, 11 Feb 2007, Johannes Schindelin wrote:

> On Sun, 11 Feb 2007, Junio C Hamano wrote:
> 
> > If NTFS driver in the Linux kernel is filling that [lstat information 
> > which Windows does not provide] with zeroes the same way then there 
> > won't be differences, right?
> 
> Maybe. Although I am quite certain that you'd break something by that.

Clarification: I am not a filesystem programmer, and as such do not know 
about this issue as much as I would like. But I am quite confident that 
inodes are an important tool to provide performance. Or something else.

Anyway, I think it is rarely advisable to imitate MS Windows, and as I 
made clean, Marco's particular situation does not seem relevant to me.

Ciao,
Dscho 

Re: 'git status' is not read-only fs friendly

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Hi,
>
> On Sun, 11 Feb 2007, Junio C Hamano wrote:
>
>> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> 
>> > On Sat, 10 Feb 2007, Junio C Hamano wrote:
>> >
>> >> About "Cygwin and Linux NTFS seem to disagree with lstat(2)"
>> >> problem.  Is it really what is happening here?
>> >
>> > Probably. AFAIR Windows lacks some important information, which is filled 
>> > with zeroes by Cygwin.
>> 
>> If NTFS driver in the Linux kernel is filling that with zeroes
>> the same way then there won't be differences, right?
>
> Maybe. Although I am quite certain that you'd break something by that.
>
> But after all, this is one really obscure corner case you have there, and 
> you are not really working on the repository on Linux either, since you 
> have it mounted readonly.
>
> I absolutely have no intention to "fix" performance or other issues for 
> that case.

Ah, you misread me.  What I was trying to drive at was if we
find the subtle difference between Cygwin's lstat(2) emulation
and lstat(2) result from the NTFS driver in the Linux kernel, we
could start and fuel flamewar on _other_ lists (namely, kernel
and Cygwin) saying "you guys are inconsistent which inconvenience
applications great deal".

And watching other people flame each other is a lot more fun
than flamewar raging close to home ;-).

Re: 'git status' is not read-only fs friendly

[Johannes Schindelin]

Hi,

On Sun, 11 Feb 2007, Junio C Hamano wrote:

> Ah, you misread me.  What I was trying to drive at was if we find the 
> subtle difference between Cygwin's lstat(2) emulation and lstat(2) 
> result from the NTFS driver in the Linux kernel, we could start and fuel 
> flamewar on _other_ lists (namely, kernel and Cygwin) saying "you guys 
> are inconsistent which inconvenience applications great deal".

I have no access to cygwin right now, so I'll argue using MinGW instead.

As I already said, I do not know what would happen if we touched st_ino. 
We'd likely have to touch it, since it takes 2 bytes on MinGW, and 4 bytes 
on Linux. Also, IIRC Cygwin fakes the inodes; and it depends on the Cygwin 
version, how it does it.

Also, we check st_uid and st_gid explicitely, which is more a problem to 
be solved by the person mounting the filesystem than the person 
maintaining the filesystem driver.

AFAICT we do not use st_dev anyway.

> And watching other people flame each other is a lot more fun than 
> flamewar raging close to home ;-).

Sometimes I find them fun here, too. That is, if it is not such a tiring 
flamewar as the renaming issues which creep up regularly.

Ciao,
Dsho

Re: 'git status' is not read-only fs friendly

[Johannes Schindelin]

Hi,

On Sat, 10 Feb 2007, Marco Costalba wrote:

> On 2/10/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > On Sat, 10 Feb 2007, Marco Costalba wrote:
> > > On 2/10/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > > > On Fri, 9 Feb 2007, Marco Costalba wrote:
> > > >
> > > > > I just need to know if current working directory is clean and report
> > > > > back to qgit user, so read-only access would be ok for me.
>
> [... talking about a patch to introduce --refresh to git-status ...]
>
> Well, I tested the patch and indeed it helps a lot ;-)

Not really. The thing is, git-status does a lot more than what you need. 
And what you need is _only_ what "git diff --name-only HEAD" does already!

It _also_ checks the index, it _also_ only checks the files with different 
stat information, but it does _not_ try to update the index and prepare a 
message to be displayed when committing.

So, what is the big problem about accepting that patching git-status for 
one obscure use is wrong, wrong, wrong, when git-diff already does what is 
needed???

Ciao,
Dscho

Re: 'git status' is not read-only fs friendly

[Marco Costalba]

On 2/10/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> Hi,
>
> Not really. The thing is, git-status does a lot more than what you need.
> And what you need is _only_ what "git diff --name-only HEAD" does already!
>
> It _also_ checks the index, it _also_ only checks the files with different
> stat information, but it does _not_ try to update the index and prepare a
> message to be displayed when committing.
>
> So, what is the big problem about accepting that patching git-status for
> one obscure use is wrong, wrong, wrong, when git-diff already does what is
> needed???
>
Probably I'm doing something wrong, but that's how working dir
detection is currently implemented in qgit:

void Git::getDiffIndex() {

	QString status;
	if (!run("git status", &status)) // git status refreshes the index,
run as first
		return;

	if (!run("git diff-index HEAD", &_wd.diffIndex))
		return;

	// check for files already updated in cache, we will
	// save this information in status third field
	if (!run("git diff-index --cached HEAD", &_wd.diffIndexCached))
		return;

          ...... other stuff .....


The first call to git-status has been there for ages and with the only
goal to refesh the index so to avoid stale data in following 'git
diff-index' calls.

If I have understood correctly you suggest to remove that call because
useless? And rely  'git diff-index' info directly. Of course if there
are no side effects I'will be happy to drop the call, but I'm not sure
it's the safest way to go.

Another option would be to accept a broken working dir detection in
these corner cases. It's a realistic option and probably the best.
Indeed also subsitute 'git status' with 'git runstatus' as long as I
get back _all_ the repo files it seems to me a lesser option, IMHO
it's better failing with empty case than have a big flow of incorrect
data.

Marco

Re: 'git status' is not read-only fs friendly

[Nicolas Pitre]

On Sat, 10 Feb 2007, Johannes Schindelin wrote:

> So, what is the big problem about accepting that patching git-status for 
> one obscure use is wrong, wrong, wrong, when git-diff already does what is 
> needed???

Because git-status itself is conceptually a read-only operation, and 
having it barf on a read-only file system is justifiably a bug.

But I agree that attempting to fix it now is probably just too risky not 
to compromize the 1.5.0 release.  Better leave it as a known issue for 
v1.5.0 and fix it later... especially if there is a possible workaround 
in the mean time.


Nicolas

Re: 'git status' is not read-only fs friendly

[Junio C Hamano]

Nicolas Pitre <nico@cam.org> writes:

> On Sat, 10 Feb 2007, Johannes Schindelin wrote:
>
>> So, what is the big problem about accepting that patching git-status for 
>> one obscure use is wrong, wrong, wrong, when git-diff already does what is 
>> needed???
>
> Because git-status itself is conceptually a read-only operation, and 
> having it barf on a read-only file system is justifiably a bug.

I do not 100% agree that it is conceptually a read-only operation.

Re: 'git status' is not read-only fs friendly

[Nicolas Pitre]

On Sat, 10 Feb 2007, Junio C Hamano wrote:

> Nicolas Pitre <nico@cam.org> writes:
> 
> > On Sat, 10 Feb 2007, Johannes Schindelin wrote:
> >
> >> So, what is the big problem about accepting that patching git-status for 
> >> one obscure use is wrong, wrong, wrong, when git-diff already does what is 
> >> needed???
> >
> > Because git-status itself is conceptually a read-only operation, and 
> > having it barf on a read-only file system is justifiably a bug.
> 
> I do not 100% agree that it is conceptually a read-only operation.

It is.  It's the technical issue that makes it not so.  But when a user 
asks for a status, he's clearly not expecting to _write_ anything, even 
less for the command to fail if the file system is read-only.  This is 
like if a file system driver refused to open a file when the file system 
is mounted read-only just because it cannot update the file's atime on 
disk.

Just like the atime example, we may refresh the index while at it when 
preparing the status results.  This is a valid technical concern.  But 
it for sure should not be mandatory for the command to succeed if the 
file system is read-only.


Nicolas

Re: 'git status' is not read-only fs friendly

[Junio C Hamano]

Nicolas Pitre <nico@cam.org> writes:

> On Sat, 10 Feb 2007, Junio C Hamano wrote:
>
>> Nicolas Pitre <nico@cam.org> writes:
>> ...
>> > Because git-status itself is conceptually a read-only operation, and 
>> > having it barf on a read-only file system is justifiably a bug.
>> 
>> I do not 100% agree that it is conceptually a read-only operation.
>
> It is.  It's the technical issue that makes it not so.

I do not think so.  It is a workflow issue that user indicates
the cache cleanliness information does not matter anymore.

Is it wrong for "git-status" to be losing the cache cleanliness
information?  The intended audience of that program is those who
are about to make a commit in the repository, as they are asking
"what would I be committing?"  Up to that point, they may have
cared about the reminder they get from "git diff" that they
edited a file and then ended up reverting the whole edit they
did to that file (I find that empty diff from "git diff" often
very useful, although I felt "Huh?"  when I was new to git).
But when they ask "git status", they care more about the real
change, and at that point (since they feel they may be ready to
make a commit -- and that is the whole point of running
"git-status") they do want to lose the cache cleanliness
information.  So "git-status" to be read-write application to
discard the cache-cleanliness information is probably a good
thing.

Re: 'git status' is not read-only fs friendly

[Nicolas Pitre]

On Sat, 10 Feb 2007, Junio C Hamano wrote:

> Nicolas Pitre <nico@cam.org> writes:
> 
> > On Sat, 10 Feb 2007, Junio C Hamano wrote:
> >
> >> Nicolas Pitre <nico@cam.org> writes:
> >> ...
> >> > Because git-status itself is conceptually a read-only operation, and 
> >> > having it barf on a read-only file system is justifiably a bug.
> >> 
> >> I do not 100% agree that it is conceptually a read-only operation.
> >
> > It is.  It's the technical issue that makes it not so.
> 
> I do not think so.  It is a workflow issue that user indicates
> the cache cleanliness information does not matter anymore.

You're making assumption about work flows and using that to justify 
command implementation flaws.  This is not exactly "user friendly".

> Is it wrong for "git-status" to be losing the cache cleanliness
> information?  The intended audience of that program is those who
> are about to make a commit in the repository, as they are asking
> "what would I be committing?"  Up to that point, they may have
> cared about the reminder they get from "git diff" that they
> edited a file and then ended up reverting the whole edit they
> did to that file (I find that empty diff from "git diff" often
> very useful, although I felt "Huh?"  when I was new to git).
> But when they ask "git status", they care more about the real
> change, and at that point (since they feel they may be ready to
> make a commit -- and that is the whole point of running
> "git-status") they do want to lose the cache cleanliness
> information.

I don't dispute that.  But git-status should certainly not be restricted 
_only_ to that usage pattern.

> So "git-status" to be read-write application to
> discard the cache-cleanliness information is probably a good
> thing.

It is... when the file system lets you write.  Like I said this is a 
technically good thing to do.

But a command that is called "status" should provide a "status" even if 
the file system is read-only nevertheless.  The index updating business 
that is done behind the scene is and should be an opportunistic 
optimization, but it should not prevent status reporting.

It is pretty expected that a "commit" command would fail if the file 
system is ro, but not a "status" command.  And this is true 
irrespectively of whatever workflow you might be most likely to use the 
"status" command for.


Nicolas

Re: 'git status' is not read-only fs friendly

[Junio C Hamano]

Nicolas Pitre <nico@cam.org> writes:

> You're making assumption about work flows and using that to justify 
> command implementation flaws.  This is not exactly "user friendly".

I do not necessarily agree that making the command to follow a
BCP workflow is "user unfriendly", but that does not pertain
what I will end up saying in this message, which is to agree
with you.

> But a command that is called "status" should provide a "status" even if 
> the file system is read-only nevertheless.  The index updating business 
> that is done behind the scene is and should be an opportunistic 
> optimization, but it should not prevent status reporting.

Fair enough.  That leaves us two options.

 (0) Do nothing.

 (1) We keep the current "git-status [-v] [-a] [[-i|-o] <paths...>]"
     command line and do the necessary index manipulation
     in-core without writing it out (see git-commit.sh for
     details of what it involves).  

 (2) We drop the support for any command line parameter from
     "git-status", apply my two patches for Marco to
     "git-runstatus", and rename "git-runstatus" to
     "git-status".

If I have to pick between the two, I would probably pick (2).
While (1) would essentially mean doing "git-commit" entirely
in-core without writing the index out until we really make the
commit, which is a good thing in itself in the longer term, it
is out of the question this late in the game for 1.5.0.

And now I think what Linus suggests also make sense -- we could
tweak (2) so that git-runstatus actually writes the refreshed
index out when it finds that it _can_ write it (and drop
subsequent internal refresh).

Now, I am heading out.

Re: 'git status' is not read-only fs friendly

[Theodore Tso]

On Sat, Feb 10, 2007 at 10:00:37AM -0800, Junio C Hamano wrote:
>  (1) We keep the current "git-status [-v] [-a] [[-i|-o] <paths...>]"
>      command line and do the necessary index manipulation
>      in-core without writing it out (see git-commit.sh for
>      details of what it involves).  
> 
>  (2) We drop the support for any command line parameter from
>      "git-status", apply my two patches for Marco to
>      "git-runstatus", and rename "git-runstatus" to
>      "git-status".
> 
> If I have to pick between the two, I would probably pick (2).
> While (1) would essentially mean doing "git-commit" entirely
> in-core without writing the index out until we really make the
> commit, which is a good thing in itself in the longer term, it
> is out of the question this late in the game for 1.5.0.

If you end up doing (2), may I suggest making the functionality of
"git-status" today available as "git-commit -n"?  It is something
useful, so we shouldn't lose it, and -n meaning --no-action is a well
accepted convention for Unix commands.

						- Ted

Re: 'git status' is not read-only fs friendly

[Nicolas Pitre]

On Sat, 10 Feb 2007, Junio C Hamano wrote:

>  (0) Do nothing.
> 
>  (1) We keep the current "git-status [-v] [-a] [[-i|-o] <paths...>]"
>      command line and do the necessary index manipulation
>      in-core without writing it out (see git-commit.sh for
>      details of what it involves).  
> 
>  (2) We drop the support for any command line parameter from
>      "git-status", apply my two patches for Marco to
>      "git-runstatus", and rename "git-runstatus" to
>      "git-status".
> 
> If I have to pick between the two, I would probably pick (2).
> While (1) would essentially mean doing "git-commit" entirely
> in-core without writing the index out until we really make the
> commit, which is a good thing in itself in the longer term, it
> is out of the question this late in the game for 1.5.0.

And don't get me wrong.  I think that for 1.5.0 you should really do (0).


Nicolas

Re: 'git status' is not read-only fs friendly

[Theodore Tso]

On Sat, Feb 10, 2007 at 01:53:44PM -0500, Nicolas Pitre wrote:
> On Sat, 10 Feb 2007, Junio C Hamano wrote:
> 
> >  (0) Do nothing.
> > 
> >  (1) We keep the current "git-status [-v] [-a] [[-i|-o] <paths...>]"
> >      command line and do the necessary index manipulation
> >      in-core without writing it out (see git-commit.sh for
> >      details of what it involves).  
> > 
> >  (2) We drop the support for any command line parameter from
> >      "git-status", apply my two patches for Marco to
> >      "git-runstatus", and rename "git-runstatus" to
> >      "git-status".
> > 
> > If I have to pick between the two, I would probably pick (2).
> > While (1) would essentially mean doing "git-commit" entirely
> > in-core without writing the index out until we really make the
> > commit, which is a good thing in itself in the longer term, it
> > is out of the question this late in the game for 1.5.0.
> 
> And don't get me wrong.  I think that for 1.5.0 you should really do (0).

Well, if we're going to change the semantics of git-status, we would
have to do it in 1.5.0 or wait until 1.6.0, wouldn't we?

							- Ted

Re: 'git status' is not read-only fs friendly

[Marco Costalba]

On 2/10/07, Nicolas Pitre <nico@cam.org> wrote:
> On Sat, 10 Feb 2007, Junio C Hamano wrote:
>
> >  (0) Do nothing.
> >
> >  (1) We keep the current "git-status [-v] [-a] [[-i|-o] <paths...>]"
> >      command line and do the necessary index manipulation
> >      in-core without writing it out (see git-commit.sh for
> >      details of what it involves).
> >
> >  (2) We drop the support for any command line parameter from
> >      "git-status", apply my two patches for Marco to
> >      "git-runstatus", and rename "git-runstatus" to
> >      "git-status".
> >
> > If I have to pick between the two, I would probably pick (2).
> > While (1) would essentially mean doing "git-commit" entirely
> > in-core without writing the index out until we really make the
> > commit, which is a good thing in itself in the longer term, it
> > is out of the question this late in the game for 1.5.0.
>
> And don't get me wrong.  I think that for 1.5.0 you should really do (0).
>

I agree on doing (0) for 1.5.0 and the following Linus lines make me
wonder if is better doing (0) also after 1.5.0

> So the fact is, "git status" _needs_ to refresh the index. Because if it
> doesn't, you'll see every file that doesn't match the index as "dirty",
> and that is not just a "technical issue".
>
> And yes, doing an "internal" refresh, like Junio's patch does, hides the
> issue, but it hides it BY MAKING THE OPTIMIZATION POINTLESS!
>
> I suspect Marco is testing some reasonably small git archive. With
> something like git itself, with less than a thousand files (and most of
> them fairly small, so rehashing them all is quick), the optimization may
> _feel_ like just a small technical detail.

If current 'git runstatus' on a NTFS directory, Linux side, show as
dirty _all_ the repo files, then in case of big repos, as Linus
pointed out, a possible future 'git runstatus --refresh' will be
terribly slow because must filter out as false positives _all_ the
repo files. And worst, have to do it *any time* it is run.

So perhaps the two patches of Junio _seems_ to work to me just because
repo is small, is qgit4 indeed, but on a Linux tree would be veeery
slow, so slow that probably is better to avoid completely and report
quickly to user an empty set, being a corner case user will understand
;-)

Marco

P.S: I know I'm looking for flames but, if git-status HAVE to write
the index and if 'status', as Nicolas points out, is a word that
suggest a read only function, why don't change the name of the
command.....'git sync-index' as example.

Re: 'git status' is not read-only fs friendly

[Linus Torvalds]

On Sat, 10 Feb 2007, Nicolas Pitre wrote:
> > >
> > > Because git-status itself is conceptually a read-only operation, and 
> > > having it barf on a read-only file system is justifiably a bug.
> > 
> > I do not 100% agree that it is conceptually a read-only operation.
> 
> It is.

It really isn't. 

It's not even a "technical issue". It's a fundamental optimization. Sure, 
you can call optimizations just "technical issues", but the fact is, it's 
one of the things that makes git so _usable_ on large archives. At some 
point, an "optimization" is no longer just about making things slightly 
faster, it's about something much bigger, and has real semantic meaning.

So the fact is, "git status" _needs_ to refresh the index. Because if it 
doesn't, you'll see every file that doesn't match the index as "dirty", 
and that is not just a "technical issue".

And yes, doing an "internal" refresh, like Junio's patch does, hides the 
issue, but it hides it BY MAKING THE OPTIMIZATION POINTLESS!

I suspect Marco is testing some reasonably small git archive. With 
something like git itself, with less than a thousand files (and most of 
them fairly small, so rehashing them all is quick), the optimization may 
_feel_ like just a small technical detail.

Now, try the same thing on the Linux kernel or somethign similar, 
especially with cold caches or not a huge amount of memory.

That "technical issue" is what makes "git status" take less than a second 
for me, and only a bit longer if things aren't cached - because we don't 
actually have to read all the file data. 

Now, it so happens that _if_ things are cached, at least under Linux, 
cached IO is so _incredibly_ fast that you won't even realize how 
expensive an operation you missed. I can SHA1 every file in the kernel 
archive (21432 files right now - 8 million LOC, and 230MB of data) in less 
than a couple of seconds. But that's only because it's all cached for me 
anyway, because I tend to run with lots of RAM, and I do things like "git 
grep so-and-so" which brings it all into cache.

But try the same thing without caches.

Here's something you can do under linux:

	sudo sh -c "echo 3 > /proc/sys/vm/drop_caches"
	git read-tree HEAD
	time git update-index --refresh

and it takes me *40* seconds. That's with quite a fast disk too - it would 
take a whole lot longer on a laptop.

Then, try it _without_ having to actually read all files, because the 
index is already up-to-date:

	sudo sh -c "echo 3 > /proc/sys/vm/drop_caches"
	time git update-index --refresh

and not it took *4* seconds. That's because it didn't actually need to 
read any file data, it could just do the stats.

Then, cached:

	# bring it all in again
	git grep something-or-other

	# invalidate the index cache
	git read-tree HEAD
	time git update-index --refresh

and I can do it under *2* seconds - because Linux is just damn good at 
cached IO, so I can read all those 21-thousand files and 235MB of data 
from the kernel cache in less than a second.

But finally, do it with caches _and_ the index in place:

	time git update-index --refresh

and it now takes 0.06 seconds. It's what allows me to do "git diff" on the 
kernel tree in a tenth of a second.

THIS IS NOT "JUST A TECHNICAL ISSUE". 

When the difference is 40 seconds vs 4 (uncached), or 2 seconds vs 0.06, 
it's not about "just an optimization" any more. At that point, it's about 
"unusable vs usable".

And yeah, waiting 40 seconds for a global "diff" for a big project may be 
something that a person coming from CVS considers to be just par for the 
course. Maybe I'm just unreasonable. But I think it's a _bug_ if I can't 
get a small diff in about a tenth of a second. It needs to be so fast that 
I never even _think_ about it.

And the index is what makes it so. 

And that's why it's important to keep the index up-to-date. If we have 
operations that allow the index to just *stay* non-coherent, like the 
suggested "git runstatus --refresh" that doesn't actually write it back, 
then that's a *bad* thing. 

I think it would be much better if "git status" always wrote the refreshed 
index file. It could then choose to ignore any errors if they happen, 
because if you have a broken setup like the NTFS read-only thing, then 
tough, it's broken, but git can't do anythign about it. But people should 
be aware that yes, "git status" absolutely _needs_ to write the index 
file. 

It is *not* a read-only operation. The index is too important to be 
considered "just a technical issue". 

		Linus

Re: 'git status' is not read-only fs friendly

[Nicolas Pitre]

On Sat, 10 Feb 2007, Linus Torvalds wrote:

> It is *not* a read-only operation. The index is too important to be 
> considered "just a technical issue". 

There is just a semantic issue that you seem to overlook completely.

According to the dictionarry, "status" is a synonym to a "state".  It is 
_not_ an action.

So, from a _user_ perspective, the git-status command should give back a 
"status".  Of _course_ the user will benefit from the index updating 
business, but as important as this update might be (and I do agree that 
it is fundamental for GIT's performance), this is still a by-product of 
the "status" command.

Therefore, the fact that the index isn't writable should not prevent 
git-status from providing the very result for which its name was chosen.  
The index might as well be brought up to date on disk the next time the 
file system is writable.


Nicolas

Re: 'git status' is not read-only fs friendly

[Junio C Hamano]

Linus Torvalds <torvalds@linux-foundation.org> writes:

> On Sat, 10 Feb 2007, Nicolas Pitre wrote:
>> > >
>> > > Because git-status itself is conceptually a read-only operation, and 
>> > > having it barf on a read-only file system is justifiably a bug.
>> > 
>> > I do not 100% agree that it is conceptually a read-only operation.
>> 
>> It is.
>
> It really isn't. 
>
> It's not even a "technical issue". It's a fundamental optimization. Sure, 
> you can call optimizations just "technical issues", but the fact is, it's
> one of the things that makes git so _usable_ on large archives. At some 
> point, an "optimization" is no longer just about making things slightly 
> faster, it's about something much bigger, and has real semantic meaning.
> ...
> THIS IS NOT "JUST A TECHNICAL ISSUE". 
> ...
> And the index is what makes it so. 
>
> And that's why it's important to keep the index up-to-date.

I think a one paragraph summary of your argument is:

 - index is a good thing -- it is what makes the difference
   between usable and unusable.

 - git-status needs to refresh the index in order to do its
   thing efficiently and usably _anyway_, so once it spends
   cycles to do so, it is senseless not to write the refreshed
   index out when it can.

I do not think anybody disputes that in a repository with 20k+
paths, it is sensible to leave the index stat-dirty for all
paths.  But I think your example

	read-tree HEAD

misses the point by stressing the importance of index too much.
Index is important for the usability and I do not think anybody
is disputing it.

The thing is, nobody switches the index that way without running
"update-index --refresh" afterwards.  Normal people would use
git-reset to switch to a different tree object, and the command
does that for you.  If you are a hardcore, you would know to use
"read-tree -m HEAD" at least to avoid making paths unnecessarily
stat-dirty.  Your example, while it is valid and demonstrates
why the index is a good thing very well, is simply not part of
a normal workflow and not very relevant when discussing the
performance ramifications of what state "git-status" should
leave the index in.

When I said "calling 'update-index --refresh' in git-status
loses stat-dirtiness information", I was certainly _NOT_ talking
about losing the information that 20k+ paths used to be
stat-dirty because the user did "read-tree HEAD" earlier.

At least for me, it is very normal to do something like this.

 * start from a clean index.

 * edit cache.h, diff.h, and diff-lib.c.

 * stop, think, and realize that my earlier edit to change one
   function prototype in diff.h was not needed, and revert the
   change to that line still in the editor.

 * fix things up further by editing other files.

And then, I would run "git diff" to see where I am.  I still
remember that I touched diff.h and I also remember that I once
changed a function prototype but then decided the change was not
necessary after all, but I do not remember if I changed anything
else in the file.  It is _very_ assuring to see the emptiness
that follows "git diff --git" header for diff.h in such a case.
Seeing the path to be stat-dirty is a very good thing for me,
because otherwise I might lose a few seconds thinking that what
I thought I touched might have been cache.h and not diff.h.

To me, running "git status" is "wrapping things up" step.  I do
not need that stat-dirty assurance "git diff" gave me at that
point.  Not seeing diff.h in "modified but updated" list is a
good thing.  And in my workflow, after that 'wrapping things up"
step, I do not need that stat-dirty assurance _anymore_.

I think Nico is correct to point out that "not _anymore_" part
of the above reasoning of mine assumes _my_ workflow and
preference, and I think that is a valid point.  Not saving the
refreshed index would make the stat-dirtiness for diff.h to come
back, which would be inconvenient and annoying to me.

But the user might want to keep it stat-dirty after running
"git-status".  People in "not _anymore_" camp like me can throw
the stat-dirtiness away by "update-index --refresh".  I do not
think he (or anybody) is advocating to keep 20k+ paths in
stat-dirty state (arguably, "artificially" due to use of
"read-tree HEAD"), so your example using "read-tree HEAD" only
confuses the discussion.

Having said all that, I do agree with you that git-status should
throw that stat-dirtiness information away by saving the
refreshed index.  Doing otherwise is annoying to me as I already
said, and I do not think of a valid reason for the user to want
to keep stat-dirtiness information after running "git-status",
because to me the whole point of running "git-status" is to
start wrapping things up.

Re: 'git status' is not read-only fs friendly

[Shawn O. Pearce]

Linus Torvalds <torvalds@linux-foundation.org> wrote:
> It's not even a "technical issue". It's a fundamental optimization. Sure, 
> you can call optimizations just "technical issues", but the fact is, it's 
> one of the things that makes git so _usable_ on large archives. At some 
> point, an "optimization" is no longer just about making things slightly 
> faster, it's about something much bigger, and has real semantic meaning.
> 
> So the fact is, "git status" _needs_ to refresh the index. Because if it 
> doesn't, you'll see every file that doesn't match the index as "dirty", 
> and that is not just a "technical issue".

Indeed.  Except that `git-update-index --refresh` is itself not
very fast on Cygwin+NTFS and large projects (about the size of
the kernel).  So git-status is a real slouch there.  Not running
`git update-index --refresh` saves at least a couple of seconds.

This is why git-gui lets you disable the refresh, and is part of
the reason why it computes the status on its own by diff-index,
diff-files and ls-files --others.
 
> THIS IS NOT "JUST A TECHNICAL ISSUE". 
> 
> When the difference is 40 seconds vs 4 (uncached), or 2 seconds vs 0.06, 
> it's not about "just an optimization" any more. At that point, it's about 
> "unusable vs usable".
> 
> And yeah, waiting 40 seconds for a global "diff" for a big project may be 
> something that a person coming from CVS considers to be just par for the 
> course. Maybe I'm just unreasonable. But I think it's a _bug_ if I can't 
> get a small diff in about a tenth of a second. It needs to be so fast that 
> I never even _think_ about it.

Yes.  Which is why if git-gui finds a file that has an empty diff,
but that was reported as modified by diff-files, it tells the user
its about to go waste a few seconds running `update-index --refresh`,
then does so.

In practice I've found it rare that a file is dirty in the index,
but is not actually modified.  The typical culprit appears to
actually be the virus scanner on a Windows system.  For some reason
it feels a need to modify some random XML 'source' files that are
tracked by Git.  Out of 30,000 files it likes to modify about 100.
*sigh* At least I have Git to tell me it didn't change any content.
 
> I think it would be much better if "git status" always wrote the refreshed 
> index file. It could then choose to ignore any errors if they happen, 
> because if you have a broken setup like the NTFS read-only thing, then 
> tough, it's broken, but git can't do anythign about it. But people should 
> be aware that yes, "git status" absolutely _needs_ to write the index 
> file. 

Not only that, but I think we can do much better with git-runstatus
than we do now.  If we scan the working directory (to search for
untracked files), and we walk the index in parallel, we can update
the index with new stat data if necessary.

Of course that doesn't matter much on Linux; its VFS operations
don't take hours.

-- 
Shawn.

Re: 'git status' is not read-only fs friendly

[Johannes Schindelin]

Hi,

On Sat, 10 Feb 2007, Nicolas Pitre wrote:

> On Sat, 10 Feb 2007, Johannes Schindelin wrote:
> 
> > So, what is the big problem about accepting that patching git-status for 
> > one obscure use is wrong, wrong, wrong, when git-diff already does what is 
> > needed???
> 
> Because git-status itself is conceptually a read-only operation, and 
> having it barf on a read-only file system is justifiably a bug.

Just to fuel the fire even more: Does it make _sense_ running git-status 
when you cannot write? I mean, the only reasonable use cases to ask 
git-status (even interpreting it in the "state" sense you are proposing), 
is when you are _working_ on the files. Which you cannot do without write 
access.

BTW I was not aware that "git diff --name-only HEAD" would not check if 
the file is differing or not, but even then, it is arguably the right 
thing for qgit to show what the index' idea of the status is.

Ciao,
Dscho

Re: 'git status' is not read-only fs friendly

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

>> > > > > I just need to know if current working directory is clean and report
>> > > > > back to qgit user, so read-only access would be ok for me.
>>
>> [... talking about a patch to introduce --refresh to git-status ...]
>>
>> Well, I tested the patch and indeed it helps a lot ;-)
>
> Not really. The thing is, git-status does a lot more than what you need. 
> And what you need is _only_ what "git diff --name-only HEAD" does already!
>
> It _also_ checks the index, it _also_ only checks the files with different 
> stat information, but it does _not_ try to update the index and prepare a 
> message to be displayed when committing.
>
> So, what is the big problem about accepting that patching git-status for 
> one obscure use is wrong, wrong, wrong, when git-diff already does what is 
> needed???

It really depends on what Marco means by "if cwd is clean".

If by "clean" Marco means "no differences after discarding cache
cleanliness information", "git-diff" is not quite it, as it
shows the differences including the cleanliness of the cache
entry.

"git-status", as Marco found out in the message that started
this thread, loses the cache cleanliness information when it
runs [*1*].

If he cares about cache cleanliness information, "git-diff" is
the right thing to use, and using "git status" is wrong -- it
not only does more than he needs (as you pointed out), it loses
information, which may be worse, depending on why he wants to
know.

[Footnote]

*1* To achieve that, it has to write into the repository.

Is it wrong for "git-status" to be losing the cache cleanliness
information?  The intended audience of that program is those who
are about to make a commit in the repository, as they are asking
"what would I be committing?"  Up to that point, they may have
cared about the reminder they get from "git diff" that they
edited a file and then ended up reverting the whole edit they
did to that file (I find that empty diff from "git diff" often
very useful, although I felt "Huh?"  when I was new to git).
But when they ask "git status", they care more about the real
change, and at that point (since they feel they may be ready to
make a commit -- and that is the whole point of running
"git-status") they do want to lose the cache cleanliness
information.  So "git-status" to be read-write application to
discard the cache-cleanliness information is probably a good
thing.

Re: 'git status' is not read-only fs friendly

[Marco Costalba]

On 2/10/07, Junio C Hamano <junkio@cox.net> wrote:
>
> But when they ask "git status", they care more about the real
> change, and at that point (since they feel they may be ready to
> make a commit -- and that is the whole point of running
> "git-status") they do want to lose the cache cleanliness
> information.  So "git-status" to be read-write application to
> discard the cache-cleanliness information is probably a good
> thing.
>

The "let's see what I'm going to commit today" it's definitely the way
I and probably most of qgit users look at the status info shown in
main view left bottom pane and also in commit dialog.