* Fix git-rev-parse breakage
@ 2005-08-24 2:17 Linus Torvalds
2005-08-24 3:22 ` Junio C Hamano
2005-08-24 18:52 ` Junio C Hamano
0 siblings, 2 replies; 6+ messages in thread
From: Linus Torvalds @ 2005-08-24 2:17 UTC (permalink / raw
To: Junio C Hamano; +Cc: Git Mailing List
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 <torvalds@osdl.org>
---
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")) {
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Fix git-rev-parse breakage
2005-08-24 2:17 Fix git-rev-parse breakage Linus Torvalds
@ 2005-08-24 3:22 ` Junio C Hamano
2005-08-24 18:52 ` Junio C Hamano
1 sibling, 0 replies; 6+ messages in thread
From: Junio C Hamano @ 2005-08-24 3:22 UTC (permalink / raw
To: Linus Torvalds; +Cc: Git Mailing List
Linus Torvalds <torvalds@osdl.org> 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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Fix git-rev-parse breakage
2005-08-24 2:17 Fix git-rev-parse breakage Linus Torvalds
2005-08-24 3:22 ` Junio C Hamano
@ 2005-08-24 18:52 ` Junio C Hamano
2005-08-24 19:03 ` Linus Torvalds
1 sibling, 1 reply; 6+ messages in thread
From: Junio C Hamano @ 2005-08-24 18:52 UTC (permalink / raw
To: Linus Torvalds; +Cc: git
Linus Torvalds <torvalds@osdl.org> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Fix git-rev-parse breakage
2005-08-24 18:52 ` Junio C Hamano
@ 2005-08-24 19:03 ` Linus Torvalds
2005-08-24 21:34 ` [PATCH] Audit rev-parse users again Junio C Hamano
2005-08-24 21:40 ` [PATCH] Rationalize output selection in rev-parse Junio C Hamano
0 siblings, 2 replies; 6+ messages in thread
From: Linus Torvalds @ 2005-08-24 19:03 UTC (permalink / raw
To: Junio C Hamano; +Cc: git
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] Audit rev-parse users again.
2005-08-24 19:03 ` Linus Torvalds
@ 2005-08-24 21:34 ` Junio C Hamano
2005-08-24 21:40 ` [PATCH] Rationalize output selection in rev-parse Junio C Hamano
1 sibling, 0 replies; 6+ messages in thread
From: Junio C Hamano @ 2005-08-24 21:34 UTC (permalink / raw
To: git
Some callers to rev-parse were using the output selection flags
inconsistently.
Signed-off-by: Junio C Hamano <junkio@cox.net>
---
git-bisect-script | 4 ++--
git-branch-script | 2 +-
git-log-script | 2 +-
git-request-pull-script | 4 ++--
git-revert-script | 2 +-
5 files changed, 7 insertions(+), 7 deletions(-)
ff84d327dfb8a9aa0634b0aaaca1c018cdc5117a
diff --git a/git-bisect-script b/git-bisect-script
--- a/git-bisect-script
+++ b/git-bisect-script
@@ -58,7 +58,7 @@ bisect_start() {
bisect_bad() {
bisect_autostart
case "$#" in 0 | 1) ;; *) usage ;; esac
- rev=$(git-rev-parse --revs-only --verify --default HEAD "$@") || exit
+ rev=$(git-rev-parse --verify --default HEAD "$@") || exit
echo "$rev" > "$GIT_DIR/refs/bisect/bad"
bisect_auto_next
}
@@ -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 --no-flags "$@") || exit ;;
esac
for rev in $revs
do
diff --git a/git-branch-script b/git-branch-script
--- a/git-branch-script
+++ b/git-branch-script
@@ -25,7 +25,7 @@ case "$#" in
head="$2^0" ;;
esac
branchname="$1"
-rev=$(git-rev-parse --revs-only --verify "$head") || exit
+rev=$(git-rev-parse --verify "$head") || exit
[ -e "$GIT_DIR/refs/heads/$branchname" ] && die "$branchname already exists"
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-request-pull-script b/git-request-pull-script
--- a/git-request-pull-script
+++ b/git-request-pull-script
@@ -19,8 +19,8 @@ head=${3-HEAD}
[ "$revision" ] || usage
[ "$url" ] || usage
-baserev=`git-rev-parse --verify $revision^0` &&
-headrev=`git-rev-parse --verify $head^0` || exit
+baserev=`git-rev-parse --verify "$revision"^0` &&
+headrev=`git-rev-parse --verify "$head"^0` || exit
echo "The following changes since commit $baserev:"
git log --max-count=1 --pretty=short "$baserev" |
diff --git a/git-revert-script b/git-revert-script
--- a/git-revert-script
+++ b/git-revert-script
@@ -10,7 +10,7 @@ case "$status" in
die "Your working tree is dirty; cannot revert a previous patch." ;;
esac
-rev=$(git-rev-parse --no-flags --verify --revs-only "$@") &&
+rev=$(git-rev-parse --verify "$@") &&
commit=$(git-rev-parse --verify "$rev^0") || exit
if git-diff-tree -R -M -p $commit | git-apply --index &&
msg=$(git-rev-list --pretty=oneline --max-count=1 $commit)
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] Rationalize output selection in rev-parse.
2005-08-24 19:03 ` Linus Torvalds
2005-08-24 21:34 ` [PATCH] Audit rev-parse users again Junio C Hamano
@ 2005-08-24 21:40 ` Junio C Hamano
1 sibling, 0 replies; 6+ messages in thread
From: Junio C Hamano @ 2005-08-24 21:40 UTC (permalink / raw
To: git; +Cc: Linus Torvalds
Earlier rounds broke 'whatchanged -p'. In attempting to fix this,
make two axis of output selection in rev-parse orthogonal:
--revs-only tells it not to output things that are not revisions nor
flags that rev-list would take.
--no-revs tells it not to output things that are revisions or
flags that rev-list would take.
--flags tells it not to output parameters that do not start with
a '-'.
--no-flags tells it not to output parameters that starts with a '-'.
So for example 'rev-parse --no-revs -p arch/i386' would yield '-p arch/i386',
while 'rev-parse --no-revs --flags -p arch/i386' would give just '-p'.
Also the meaning of --verify has been made stronger. It now rejects
anything but a single valid rev argument. Earlier it passed some flags
through without complaining.
Signed-off-by: Junio C Hamano <junkio@cox.net>
---
Linus Torvalds <torvalds@osdl.org> writes:
> 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.
Thanks for noticing.
rev-parse.c | 121 ++++++++++++++++++++++++++---------------------------------
1 files changed, 54 insertions(+), 67 deletions(-)
4866ccf0f434db118c4dcdeeab840eb4844d50a4
diff --git a/rev-parse.c b/rev-parse.c
--- a/rev-parse.c
+++ b/rev-parse.c
@@ -7,20 +7,21 @@
#include "commit.h"
#include "refs.h"
+#define DO_REVS 1
+#define DO_NOREV 2
+#define DO_FLAGS 4
+#define DO_NONFLAGS 8
+static int filter = ~0;
+
static char *def = NULL;
-static int no_revs = 0;
-static int single_rev = 0;
-static int revs_only = 0;
-static int do_rev_argument = 1;
-static int output_revs = 0;
-static int flags_only = 0;
-static int no_flags = 0;
-static int output_sq = 0;
-static int symbolic = 0;
#define NORMAL 0
#define REVERSED 1
static int show_type = NORMAL;
+static int symbolic = 0;
+static int output_sq = 0;
+
+static int revs_count = 0;
/*
* Some arguments are relevant "revision" arguments,
@@ -30,13 +31,19 @@ static int show_type = NORMAL;
static int is_rev_argument(const char *arg)
{
static const char *rev_args[] = {
- "--max-count=",
+ "--bisect",
+ "--header",
"--max-age=",
- "--min-age=",
+ "--max-count=",
"--merge-order",
- "--topo-order",
- "--bisect",
+ "--min-age=",
"--no-merges",
+ "--objects",
+ "--parents",
+ "--pretty",
+ "--show-breaks",
+ "--topo-order",
+ "--unpacked",
NULL
};
const char **p = rev_args;
@@ -47,11 +54,13 @@ static int is_rev_argument(const char *a
if (!str)
return 0;
len = strlen(str);
- if (!strncmp(arg, str, len))
+ if (!strcmp(arg, str) ||
+ (str[len-1] == '=' && !strncmp(arg, str, len)))
return 1;
}
}
+/* Output argument as a string, either SQ or normal */
static void show(const char *arg)
{
if (output_sq) {
@@ -70,11 +79,13 @@ static void show(const char *arg)
puts(arg);
}
+/* Output a revision, only if filter allows it */
static void show_rev(int type, const unsigned char *sha1, const char *name)
{
- if (no_revs)
+ if (!(filter & DO_REVS))
return;
- output_revs++;
+ def = NULL;
+ revs_count++;
if (type != show_type)
putchar('^');
@@ -84,29 +95,12 @@ static void show_rev(int type, const uns
show(sha1_to_hex(sha1));
}
-static void show_rev_arg(char *rev)
+/* Output a flag, only if filter allows it. */
+static void show_flag(char *arg)
{
- if (no_revs)
+ if (!(filter & DO_FLAGS))
return;
- show(rev);
-}
-
-static void show_norev(char *norev)
-{
- if (flags_only)
- return;
- if (revs_only)
- return;
- show(norev);
-}
-
-static void show_arg(char *arg)
-{
- if (no_flags)
- return;
- if (do_rev_argument && is_rev_argument(arg))
- show_rev_arg(arg);
- else
+ if (filter & (is_rev_argument(arg) ? DO_REVS : DO_NOREV))
show(arg);
}
@@ -122,7 +116,6 @@ static void show_default(void)
show_rev(NORMAL, sha1, s);
return;
}
- show_norev(s);
}
}
@@ -134,7 +127,7 @@ static int show_reference(const char *re
int main(int argc, char **argv)
{
- int i, as_is = 0;
+ int i, as_is = 0, verify = 0;
unsigned char sha1[20];
const char *prefix = setup_git_directory();
@@ -143,15 +136,13 @@ int main(int argc, char **argv)
char *dotdot;
if (as_is) {
- show_norev(arg);
+ show(arg);
continue;
}
if (*arg == '-') {
if (!strcmp(arg, "--")) {
- show_default();
- if (revs_only || flags_only)
- break;
as_is = 1;
+ continue;
}
if (!strcmp(arg, "--default")) {
def = argv[i+1];
@@ -159,25 +150,24 @@ int main(int argc, char **argv)
continue;
}
if (!strcmp(arg, "--revs-only")) {
- revs_only = 1;
+ filter &= ~DO_NOREV;
continue;
}
if (!strcmp(arg, "--no-revs")) {
- no_revs = 1;
+ filter &= ~DO_REVS;
continue;
}
if (!strcmp(arg, "--flags")) {
- flags_only = 1;
+ filter &= ~DO_NONFLAGS;
continue;
}
if (!strcmp(arg, "--no-flags")) {
- no_flags = 1;
+ filter &= ~DO_FLAGS;
continue;
}
if (!strcmp(arg, "--verify")) {
- revs_only = 1;
- do_rev_argument = 0;
- single_rev = 1;
+ filter &= ~(DO_FLAGS|DO_NOREV);
+ verify = 1;
continue;
}
if (!strcmp(arg, "--sq")) {
@@ -197,12 +187,17 @@ int main(int argc, char **argv)
continue;
}
if (!strcmp(arg, "--show-prefix")) {
- puts(prefix);
+ if (prefix)
+ puts(prefix);
continue;
}
- show_arg(arg);
+ if (verify)
+ die("Needed a single revision");
+ show_flag(arg);
continue;
}
+
+ /* Not a flag argument */
dotdot = strstr(arg, "..");
if (dotdot) {
unsigned char end[20];
@@ -212,9 +207,6 @@ int main(int argc, char **argv)
if (!*n)
n = "HEAD";
if (!get_sha1(n, end)) {
- if (no_revs)
- continue;
- def = NULL;
show_rev(NORMAL, end, n);
show_rev(REVERSED, sha1, arg);
continue;
@@ -223,26 +215,21 @@ int main(int argc, char **argv)
*dotdot = '.';
}
if (!get_sha1(arg, sha1)) {
- if (no_revs)
- continue;
- def = NULL;
show_rev(NORMAL, sha1, arg);
continue;
}
if (*arg == '^' && !get_sha1(arg+1, sha1)) {
- if (no_revs)
- continue;
- def = NULL;
show_rev(REVERSED, sha1, arg+1);
continue;
}
- show_default();
- show_norev(arg);
+ if (verify)
+ die("Needed a single revision");
+ if ((filter & (DO_NONFLAGS|DO_NOREV)) ==
+ (DO_NONFLAGS|DO_NOREV))
+ show(arg);
}
show_default();
- if (single_rev && output_revs != 1) {
- fprintf(stderr, "Needed a single revision\n");
- exit(1);
- }
+ if (verify && revs_count != 1)
+ die("Needed a single revision");
return 0;
}
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2005-08-24 21:40 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-08-24 2:17 Fix git-rev-parse breakage Linus Torvalds
2005-08-24 3:22 ` Junio C Hamano
2005-08-24 18:52 ` Junio C Hamano
2005-08-24 19:03 ` Linus Torvalds
2005-08-24 21:34 ` [PATCH] Audit rev-parse users again Junio C Hamano
2005-08-24 21:40 ` [PATCH] Rationalize output selection in rev-parse Junio C Hamano
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
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 read-only IMAP folder(s) and NNTP newsgroup(s).