Re: Fix git-rev-parse breakage
On Wed, 24 Aug 2005, Junio C Hamano wrote: > that is not a right thing so get rid of that assumption" (which > I agree is a good change", and the last sentense says > opposite... Well, the patch makes it an _explicit_ assumption, instead of a very subtly hidden one from the code-flow. It was the non-obvious hidden assumption that caused the bug. > Here is my thinking, requesting for a sanity check. > > * git-whatchanged wants to use it to tell rev-list arguments > from rev-tree arguments. You _do_ want to pick --max-count=10 > or --merge-order in this case, and --revs-only implying > --no-flags would make this impossible. Fair enough. However, there are two kinds of flags: the "revision flags", and the "-p" kind of flags. And the problem was that "git-whatchanged -p" didn't work any more, because we passed "-p" along to "git-rev-list". Gaah. Linus - To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Fix git-rev-parse breakage
Linus Torvalds <[EMAIL PROTECTED]> writes:
> The --flags cleanup caused problems: we used to depend on the fact that
> "revs_only" magically suppressed flags, adn that assumption was broken by
> the recent fixes.
>
> It wasn't a good assumption in the first place, so instead of
> re-introducing it, let's just get rid of it.
>
> This makes "--revs-only" imply "--no-flags".
I was taking a look at this once again after noticing that I
haven't taking any action on it. But now I am a bit confused
reading the above. The first two paragraphs tells me
"--revs-only implied --no-flags and we used to depend on it, but
that is not a right thing so get rid of that assumption" (which
I agree is a good change", and the last sentense says
opposite...
And the code makes --revs-only imply --no-flags.
Here is my thinking, requesting for a sanity check.
* git-whatchanged wants to use it to tell rev-list arguments
from rev-tree arguments. You _do_ want to pick --max-count=10
or --merge-order in this case, and --revs-only implying
--no-flags would make this impossible.
* git-log-script uses it once to make sure it has one commit to
start at, and lacks --no-flags by mistake.
* git-bisect uses it to validate the parameter is a valid ref,
but does not use --verify.
This trivial patch fixes the latter two.
---
diff --git a/git-log-script b/git-log-script
--- a/git-log-script
+++ b/git-log-script
@@ -1,4 +1,4 @@
#!/bin/sh
-revs=$(git-rev-parse --revs-only --default HEAD "$@") || exit
+revs=$(git-rev-parse --revs-only --no-flags --default HEAD "$@") || exit
[ "$revs" ] || die "No HEAD ref"
git-rev-list --pretty $(git-rev-parse --default HEAD "$@") | LESS=-S
${PAGER:-less}
diff --git a/git-bisect-script b/git-bisect-script
--- a/git-bisect-script
+++ b/git-bisect-script
@@ -67,7 +67,7 @@ bisect_good() {
bisect_autostart
case "$#" in
0)revs=$(git-rev-parse --verify HEAD) || exit ;;
- *)revs=$(git-rev-parse --revs-only "$@") || exit ;;
+ *)revs=$(git-rev-parse --revs-only --verify "$@") || exit ;;
esac
for rev in $revs
do
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Fix git-rev-parse breakage
Linus Torvalds <[EMAIL PROTECTED]> writes: > This makes "--revs-only" imply "--no-flags". > > [ Side note: we might want to get rid of these confusing two-way flags, > where some flags say "only print xxx", and others say "don't print yyy". > We'd be better off with just three flags that say "print zzz", where zzz > is one of "flags, revs, norevs" ] I suspect that would not fly too well. Being able to say "give me all flags meant for rev-list", "give me all what are meant for rev-list", and "give me all non-flags that are meant for rev-list" are very handy, so I'd rather want to see --revs-only to mean "meant for rev-list", --no-revs to mean "not meant for rev-list", --flags to mean "only ones that start with a '-'", and --no-flags to mean "only ones that do not start with a '-'". And that would give me (rev/no-rev/lack thereof) x (flag/no-flag/lack thereof) = 9 combinations. - To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Fix git-rev-parse breakage
The --flags cleanup caused problems: we used to depend on the fact that
"revs_only" magically suppressed flags, adn that assumption was broken by
the recent fixes.
It wasn't a good assumption in the first place, so instead of
re-introducing it, let's just get rid of it.
This makes "--revs-only" imply "--no-flags".
[ Side note: we might want to get rid of these confusing two-way flags,
where some flags say "only print xxx", and others say "don't print yyy".
We'd be better off with just three flags that say "print zzz", where zzz
is one of "flags, revs, norevs" ]
Signed-off-by: Linus Torvalds <[EMAIL PROTECTED]>
---
diff --git a/rev-parse.c b/rev-parse.c
--- a/rev-parse.c
+++ b/rev-parse.c
@@ -160,6 +160,7 @@ int main(int argc, char **argv)
}
if (!strcmp(arg, "--revs-only")) {
revs_only = 1;
+ no_flags = 1;
continue;
}
if (!strcmp(arg, "--no-revs")) {
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
