threads / discuss / 6755
Subject: 'git status' is not read-only fs friendly
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
Linus TorvaldsRe: 'git status' is not read-only fs friendly
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
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
Junio C HamanoRe: 'git status' is not read-only fs friendly
"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 MakefileThe '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).
Junio C HamanoRe: 'git status' is not read-only fs friendly
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
> 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
Theodore TsoRe: 'git status' is not read-only fs friendly
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
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
Linus TorvaldsRe: 'git status' is not read-only fs friendly
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
Junio C HamanoRe: 'git status' is not read-only fs friendly
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;
Junio C HamanoRe: 'git status' is not read-only fs friendly
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.
Junio C Hamano[PATCH 1/2] run_diff_{files,index}(): update calling convention.
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.
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
Junio C HamanoRe: [PATCH 1/2] run_diff_{files,index}(): update calling convention.
"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.
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.
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$
Junio C HamanoRe: [PATCH 1/2] run_diff_{files,index}(): update calling convention.
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.
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 ;-)
Junio C HamanoRe: [PATCH 1/2] run_diff_{files,index}(): update calling convention.
"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.
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.
Junio C Hamano[PATCH 2/2] git-runstatus --refresh
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
Johannes SchindelinRe: 'git status' is not read-only fs friendly
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
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
Johannes SchindelinRe: 'git status' is not read-only fs friendly
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
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
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.
Junio C HamanoRe: 'git status' is not read-only fs friendly
"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.
Johannes SchindelinRe: 'git status' is not read-only fs friendly
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
Junio C HamanoRe: 'git status' is not read-only fs friendly
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?
Johannes SchindelinRe: 'git status' is not read-only fs friendly
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
Johannes SchindelinRe: 'git status' is not read-only fs friendly
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
Junio C HamanoRe: 'git status' is not read-only fs friendly
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 ;-).
Johannes SchindelinRe: 'git status' is not read-only fs friendly
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
Johannes SchindelinRe: 'git status' is not read-only fs friendly
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
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
Nicolas PitreRe: 'git status' is not read-only fs friendly
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
Junio C HamanoRe: 'git status' is not read-only fs friendly
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.
Nicolas PitreRe: 'git status' is not read-only fs friendly
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
Junio C HamanoRe: 'git status' is not read-only fs friendly
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.
Nicolas PitreRe: 'git status' is not read-only fs friendly
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
Junio C HamanoRe: 'git status' is not read-only fs friendly
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.
Theodore TsoRe: 'git status' is not read-only fs friendly
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
Nicolas PitreRe: 'git status' is not read-only fs friendly
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
Theodore TsoRe: 'git status' is not read-only fs friendly
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
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.
Linus TorvaldsRe: 'git status' is not read-only fs friendly
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
Nicolas PitreRe: 'git status' is not read-only fs friendly
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
Junio C HamanoRe: 'git status' is not read-only fs friendly
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.
Shawn O. PearceRe: 'git status' is not read-only fs friendly
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.
Johannes SchindelinRe: 'git status' is not read-only fs friendly
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
Junio C HamanoRe: 'git status' is not read-only fs friendly
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
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.