* [RFC] Silent File Mods Being Committed
@ 2006-03-23 4:04 Jon Loeliger
2006-03-23 5:13 ` Junio C Hamano
0 siblings, 1 reply; 6+ messages in thread
From: Jon Loeliger @ 2006-03-23 4:04 UTC (permalink / raw)
To: git
Folks,
I sort of got blindsided by committing (accidental) mode
changes on a file that was also textually changed. Because
it was textually changed, I happily expected 'git status'
to show it as modified and subsequently 'git commit'-ed it.
Only after I 'git diff'-ed it later did I see that there
was also a mode change on the file! Argh!
Secondarily, I think that the invisible file changes like
this caused an eariler (yesterday-ish) confusion where an
index looked clean, but was dirty due to stat issues and
poor users (me) couldn't quite see why.
Originally, I was going to propose that git-ls-files be modified
such that the mode and stat changes that ce_modified() correctly
identifies as changing are somehow translated into output that
"git status" shows the user per-file _in_addition_ to the normal
"is modified" header. So I did this mod:
diff --git a/ls-files.c b/ls-files.c
index df25c8c..3e7e55d 100644
--- a/ls-files.c
+++ b/ls-files.c
@@ -450,7 +450,7 @@ static void show_killed_files(void)
}
}
-static void show_ce_entry(const char *tag, struct cache_entry *ce)
+static void show_ce_entry(const char *tag, struct cache_entry *ce, int changed)
{
int len = prefix_len;
int offset = prefix_offset;
@@ -482,6 +482,14 @@ static void show_ce_entry(const char *ta
fputs(tag, stdout);
write_name_quoted("", 0, ce->name + offset,
line_terminator, stdout);
+ if (changed & (MTIME_CHANGED | CTIME_CHANGED)) {
+ putchar(' ');
+ putchar('T');
+ }
+ if (changed & MODE_CHANGED) {
+ putchar(' ');
+ putchar('M');
+ }
putchar(line_terminator);
}
else {
@@ -541,7 +549,7 @@ static void show_files(void)
continue;
if (show_unmerged && !ce_stage(ce))
continue;
- show_ce_entry(ce_stage(ce) ? tag_unmerged : tag_cached, ce);
+ show_ce_entry(ce_stage(ce) ? tag_unmerged : tag_cached, ce, 0);
}
}
if (show_deleted | show_modified) {
@@ -553,9 +561,13 @@ static void show_files(void)
continue;
err = lstat(ce->name, &st);
if (show_deleted && err)
- show_ce_entry(tag_removed, ce);
- if (show_modified && ce_modified(ce, &st, 0))
- show_ce_entry(tag_modified, ce);
+ show_ce_entry(tag_removed, ce, 0);
+ if (show_modified) {
+ int changed = ce_modified(ce, &st, 0);
+ if (changed)
+ show_ce_entry(tag_modified,
+ ce, changed);
+ }
}
}
}
And with that, "git ls-file -m" showed a trailing T or M to
indicate a "time" or a "mode" change happened on each file.
However, 'git status' didn't show that output....
And that is because it is driven by the diffcore instead!
So I _think_ diff_resolve_rename_copy() has to be consulted
to get that DIFF_STATUS_MODIFIED indicator. Except that it is
shared with the SHA1 compare too:
else if (memcmp(p->one->sha1, p->two->sha1, 20) ||
p->one->mode != p->two->mode)
p->status = DIFF_STATUS_MODIFIED;
But I haven't tracked it back to see how to propagate that
status back up to show_modified() in diff-files.c yet...
Maybe there is an easier way...?
jdl
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RFC] Silent File Mods Being Committed
2006-03-23 4:04 [RFC] Silent File Mods Being Committed Jon Loeliger
@ 2006-03-23 5:13 ` Junio C Hamano
2006-03-23 21:47 ` Petr Baudis
0 siblings, 1 reply; 6+ messages in thread
From: Junio C Hamano @ 2006-03-23 5:13 UTC (permalink / raw)
To: Jon Loeliger; +Cc: git
Jon Loeliger <jdl@jdl.com> writes:
> However, 'git status' didn't show that output....
>
> And that is because it is driven by the diffcore instead!
> So I _think_ diff_resolve_rename_copy() has to be consulted
> to get that DIFF_STATUS_MODIFIED indicator. Except that it is
> shared with the SHA1 compare too:
>
> else if (memcmp(p->one->sha1, p->two->sha1, 20) ||
> p->one->mode != p->two->mode)
> p->status = DIFF_STATUS_MODIFIED;
>
> But I haven't tracked it back to see how to propagate that
> status back up to show_modified() in diff-files.c yet...
I think the cleanest way would be either
(1) define another output format to diffcore similar to
--name-status, let that format perform most of what
DIFF_FORMAT_NAME_STATUS does, and update
diff.c:diff_flush_raw() to show the mode change as M+ (old
was not executable but new is) or M- (the other way); or
(2) audit all the scripts to make sure they do not get upset if
we add trailing +/- to the status letter, and do that
unconditionally, like the attached patch does.
If you go the latter route, you would get something like this:
$ ./git-diff-files --abbrev
:100644 100755 c0548ee... 0000000... M+ diff.c
$ ./git-diff-files --name-status
M+ diff.c
---
diff --git a/diff.c b/diff.c
index c0548ee..09b8f7e 100644
--- a/diff.c
+++ b/diff.c
@@ -1034,6 +1034,10 @@ static void diff_flush_raw(struct diff_f
status[0] = p->status;
status[1] = 0;
}
+
+ if (p->one->mode && p->two->mode && p->one->mode != p->two->mode)
+ strcat(status, (p->two->mode & 01) ? "+" : "-");
+
switch (p->status) {
case DIFF_STATUS_COPIED:
case DIFF_STATUS_RENAMED:
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RFC] Silent File Mods Being Committed
2006-03-23 5:13 ` Junio C Hamano
@ 2006-03-23 21:47 ` Petr Baudis
2006-03-23 23:32 ` Junio C Hamano
2006-03-23 23:32 ` Junio C Hamano
0 siblings, 2 replies; 6+ messages in thread
From: Petr Baudis @ 2006-03-23 21:47 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jon Loeliger, git
Dear diary, on Thu, Mar 23, 2006 at 06:13:21AM CET, I got a letter
where Junio C Hamano <junkio@cox.net> said that...
> (2) audit all the scripts to make sure they do not get upset if
> we add trailing +/- to the status letter, and do that
> unconditionally, like the attached patch does.
Cogito will get upset since we assume the mode field is one-char in our
regexps, and when we don't, we compare the mode field with strings and
that would obviously fail if you add random stuff to it.
Otherwise, I like this idea, though.
--
Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
Right now I am having amnesia and deja-vu at the same time. I think
I have forgotten this before.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC] Silent File Mods Being Committed
2006-03-23 21:47 ` Petr Baudis
@ 2006-03-23 23:32 ` Junio C Hamano
2006-03-23 23:32 ` Junio C Hamano
1 sibling, 0 replies; 6+ messages in thread
From: Junio C Hamano @ 2006-03-23 23:32 UTC (permalink / raw)
To: Petr Baudis; +Cc: Jon Loeliger, git
Petr Baudis <pasky@suse.cz> writes:
> Dear diary, on Thu, Mar 23, 2006 at 06:13:21AM CET, I got a letter
> where Junio C Hamano <junkio@cox.net> said that...
>> (2) audit all the scripts to make sure they do not get upset if
>> we add trailing +/- to the status letter, and do that
>> unconditionally, like the attached patch does.
>
> Cogito will get upset since we assume the mode field is one-char in our
> regexps, and when we don't, we compare the mode field with strings and
> that would obviously fail if you add random stuff to it.
>
> Otherwise, I like this idea, though.
Likewise. If it was not obvious, I am not going to commit that
myself. If jdl or somebody cares enough, he or she can prepare
a patch to accomodate set of patches to git-core, Cogito and
StGIT (at least these three should be covered) _and_ parrot my
patch back at me. Hint, hint...
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC] Silent File Mods Being Committed
2006-03-23 21:47 ` Petr Baudis
2006-03-23 23:32 ` Junio C Hamano
@ 2006-03-23 23:32 ` Junio C Hamano
2006-03-24 1:12 ` Petr Baudis
1 sibling, 1 reply; 6+ messages in thread
From: Junio C Hamano @ 2006-03-23 23:32 UTC (permalink / raw)
To: Petr Baudis; +Cc: Jon Loeliger, git
Petr Baudis <pasky@suse.cz> writes:
> Dear diary, on Thu, Mar 23, 2006 at 06:13:21AM CET, I got a letter
> where Junio C Hamano <junkio@cox.net> said that...
>> (2) audit all the scripts to make sure they do not get upset if
>> we add trailing +/- to the status letter, and do that
>> unconditionally, like the attached patch does.
>
> Cogito will get upset since we assume the mode field is one-char in our
> regexps, and when we don't, we compare the mode field with strings and
> that would obviously fail if you add random stuff to it.
>
> Otherwise, I like this idea, though.
Likewise. If it was not obvious, I am not going to commit that
myself. If jdl or somebody cares enough, he or she can prepare
a a set of patches to git-core, Cogito and StGIT (at least these
three should be covered) to teach them the trailing +/- letter,
_and_ parrot my patch back at me. Hint, hint...
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC] Silent File Mods Being Committed
2006-03-23 23:32 ` Junio C Hamano
@ 2006-03-24 1:12 ` Petr Baudis
0 siblings, 0 replies; 6+ messages in thread
From: Petr Baudis @ 2006-03-24 1:12 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jon Loeliger, git
Dear diary, on Fri, Mar 24, 2006 at 12:32:58AM CET, I got a letter
where Junio C Hamano <junkio@cox.net> said that...
> Petr Baudis <pasky@suse.cz> writes:
>
> > Dear diary, on Thu, Mar 23, 2006 at 06:13:21AM CET, I got a letter
> > where Junio C Hamano <junkio@cox.net> said that...
> >> (2) audit all the scripts to make sure they do not get upset if
> >> we add trailing +/- to the status letter, and do that
> >> unconditionally, like the attached patch does.
> >
> > Cogito will get upset since we assume the mode field is one-char in our
> > regexps, and when we don't, we compare the mode field with strings and
> > that would obviously fail if you add random stuff to it.
> >
> > Otherwise, I like this idea, though.
>
> Likewise. If it was not obvious, I am not going to commit that
> myself. If jdl or somebody cares enough, he or she can prepare
> a a set of patches to git-core, Cogito and StGIT (at least these
> three should be covered) to teach them the trailing +/- letter,
> _and_ parrot my patch back at me. Hint, hint...
Well, or it can just add the option to Core Git. Just unconditional
implementation of (2) is no-go because it would break backwards
compatibility, which is baaad.
--
Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
Right now I am having amnesia and deja-vu at the same time. I think
I have forgotten this before.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2006-03-24 1:12 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-03-23 4:04 [RFC] Silent File Mods Being Committed Jon Loeliger
2006-03-23 5:13 ` Junio C Hamano
2006-03-23 21:47 ` Petr Baudis
2006-03-23 23:32 ` Junio C Hamano
2006-03-23 23:32 ` Junio C Hamano
2006-03-24 1:12 ` Petr Baudis
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).