* Re: [SCNR] Re: [FYI PATCH] git wrapper: DWIM mistyped commands
From: Johannes Schindelin @ 2008-07-22 20:19 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: git
In-Reply-To: <20080722201648.GB11831@artemis.madism.org>
Hi,
On Tue, 22 Jul 2008, Pierre Habouzit wrote:
> <SCNR>
> Or use a decent shell:
I tried that:
git reab<tab><tab><TAB><TTAAABBB!>
> Despite that, I really like your idea. **hint hint**
I said that _I_ did not mean it for inclusion. **hint hint**
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] Build configuration to skip ctime for modification test
From: Johannes Schindelin @ 2008-07-22 20:17 UTC (permalink / raw)
To: Alex Riesen; +Cc: Junio C Hamano, Johannes Sixt, git
In-Reply-To: <20080722193901.GA5113@blimp.local>
Hi,
On Tue, 22 Jul 2008, Alex Riesen wrote:
> +#ifndef NO_TRUSTABLE_FILEMODE
> if (ce->ce_ctime != (unsigned int) st->st_ctime)
> changed |= CTIME_CHANGED;
> +#endif
Surely you meant trust_executable_bit instead, right?
Otherwise, if you really want to tell at compile time,I think for clarity
you have to introduce another #define, since NO_TRUSTABLE_FILEMODE
definitely says something different than CTIME_IS_USELESS.
Ciao,
Dscho
^ permalink raw reply
* [SCNR] Re: [FYI PATCH] git wrapper: DWIM mistyped commands
From: Pierre Habouzit @ 2008-07-22 20:16 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <alpine.DEB.1.00.0807222100150.8986@racer>
[-- Attachment #1: Type: text/plain, Size: 1627 bytes --]
On Tue, Jul 22, 2008 at 08:01:29PM +0000, Johannes Schindelin wrote:
>
> This patch introduces a modified Damerau-Levenshtein algorithm into
> Git's code base, and uses it with the following penalties to show some
> similar commands when an unknown command was encountered:
>
> swap = 0, insertion = 1, substitution = 2, deletion = 4
>
> A typical output would now look like this:
>
> $ git sm
> git: 'sm' is not a git-command. See 'git --help'.
>
> Did you mean one of these?
> am
> rm
>
> The cut-off is at similarity rating 6, which was empirically determined
> to give sensible results.
>
> As a convenience, if there is only one candidate, Git continues under
> the assumption that the user mistyped it. Example:
>
> $ git reabse
> WARNING: You called a Git program named 'reabse', which does
> not exist.
> Continuing under the assumption that you meant 'rebase'
> [...]
<SCNR>
Or use a decent shell:
When typing e.g.: git tsa<tab>, it yields:
$ git status
---- corrections (errors 1)
status -- show working-tree's status
tag -- create tag object signed with GPG
tar-tree -- create tar archive of the files in the named tree
---- original
tsa
and it even works for non git commands ;)
</SCNR>
Despite that, I really like your idea. **hint hint** One could even hook that
for long options into parse-options.
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: [PATCH] parse-options: fix parsing of "--foobar=" with no value
From: Jeff King @ 2008-07-22 20:09 UTC (permalink / raw)
To: Olivier Marin; +Cc: Pierre Habouzit, Junio C Hamano, git
In-Reply-To: <48863436.50309@free.fr>
On Tue, Jul 22, 2008 at 09:25:42PM +0200, Olivier Marin wrote:
> I found the "random bug" while migrating "git init" to parse-options. I
> think you can reproduce it with:
>
> $ git clone --template= <repo>
> error: ignoring template /var/run/synaptic.socket
> fatal: cannot opendir /var/run/sudo
>
> But now, it appears the problem is not in parse-options, sorry.
Yes, the problem is that copy_templates in builtin-init-db.c is totally
broken for an empty template name. It writes past the beginning of the
string, and then starts copying at "/". Oops.
Maybe something like this is better? It should define --template= to
mean "don't copy any templates" (and I haven't tested it at all).
diff --git a/builtin-init-db.c b/builtin-init-db.c
index 38b4fcb..baf0d09 100644
--- a/builtin-init-db.c
+++ b/builtin-init-db.c
@@ -117,6 +117,8 @@ static void copy_templates(const char *template_dir)
template_dir = getenv(TEMPLATE_DIR_ENVIRONMENT);
if (!template_dir)
template_dir = system_path(DEFAULT_GIT_TEMPLATE_DIR);
+ if (!template_dir[0])
+ return;
strcpy(template_path, template_dir);
template_len = strlen(template_path);
if (template_path[template_len-1] != '/') {
^ permalink raw reply related
* Re: [PATCH] parse-options: fix parsing of "--foobar=" with no value
From: Johannes Schindelin @ 2008-07-22 20:05 UTC (permalink / raw)
To: Olivier Marin; +Cc: Pierre Habouzit, Junio C Hamano, git
In-Reply-To: <48863436.50309@free.fr>
Hi,
On Tue, 22 Jul 2008, Olivier Marin wrote:
> I would have find it convenient for things like --foobar=$var where foobar
> fallback to default when $var is empty.
--foobar=${var:-default} will expand to --foobar=default when $var is
empty.
Hth,
Dscho
^ permalink raw reply
* [FYI PATCH] git wrapper: DWIM mistyped commands
From: Johannes Schindelin @ 2008-07-22 20:01 UTC (permalink / raw)
To: git
This patch introduces a modified Damerau-Levenshtein algorithm into
Git's code base, and uses it with the following penalties to show some
similar commands when an unknown command was encountered:
swap = 0, insertion = 1, substitution = 2, deletion = 4
A typical output would now look like this:
$ git sm
git: 'sm' is not a git-command. See 'git --help'.
Did you mean one of these?
am
rm
The cut-off is at similarity rating 6, which was empirically determined
to give sensible results.
As a convenience, if there is only one candidate, Git continues under
the assumption that the user mistyped it. Example:
$ git reabse
WARNING: You called a Git program named 'reabse', which does
not exist.
Continuing under the assumption that you meant 'rebase'
[...]
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
So I mistyped 'reabse' for the hundred trillionth time, but I
will never have to correct my mistakes again.
Note: this patch is _not_ meant for inclusion.
Makefile | 2 +
builtin.h | 2 +-
git.c | 4 ++-
help.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
levenshtein.c | 47 +++++++++++++++++++++++++++++++++++++++++++
levenshtein.h | 8 +++++++
6 files changed, 121 insertions(+), 3 deletions(-)
create mode 100644 levenshtein.c
create mode 100644 levenshtein.h
diff --git a/Makefile b/Makefile
index 19bdd03..7e114e0 100644
--- a/Makefile
+++ b/Makefile
@@ -347,6 +347,7 @@ LIB_H += git-compat-util.h
LIB_H += graph.h
LIB_H += grep.h
LIB_H += hash.h
+LIB_H += levenshtein.h
LIB_H += list-objects.h
LIB_H += ll-merge.h
LIB_H += log-tree.h
@@ -421,6 +422,7 @@ LIB_OBJS += hash.o
LIB_OBJS += help.o
LIB_OBJS += ident.o
LIB_OBJS += interpolate.o
+LIB_OBJS += levenshtein.o
LIB_OBJS += list-objects.o
LIB_OBJS += ll-merge.o
LIB_OBJS += lockfile.o
diff --git a/builtin.h b/builtin.h
index 0e605d4..fc5f108 100644
--- a/builtin.h
+++ b/builtin.h
@@ -11,7 +11,7 @@ extern const char git_usage_string[];
extern const char git_more_info_string[];
extern void list_common_cmds_help(void);
-extern void help_unknown_cmd(const char *cmd);
+extern const char *help_unknown_cmd(const char *cmd);
extern void prune_packed_objects(int);
extern int read_line_with_nul(char *buf, int size, FILE *file);
extern int fmt_merge_msg(int merge_summary, struct strbuf *in,
diff --git a/git.c b/git.c
index 1bfd271..d7510ef 100644
--- a/git.c
+++ b/git.c
@@ -500,7 +500,9 @@ int main(int argc, const char **argv)
cmd, argv[0]);
exit(1);
}
- help_unknown_cmd(cmd);
+ argv[0] = help_unknown_cmd(cmd);
+ handle_internal_command(argc, argv);
+ execv_dashed_external(argv);
}
fprintf(stderr, "Failed to run command '%s': %s\n",
diff --git a/help.c b/help.c
index bfc84ae..480befe 100644
--- a/help.c
+++ b/help.c
@@ -9,6 +9,7 @@
#include "common-cmds.h"
#include "parse-options.h"
#include "run-command.h"
+#include "levenshtein.h"
static struct man_viewer_list {
struct man_viewer_list *next;
@@ -666,9 +667,67 @@ static void show_html_page(const char *git_cmd)
open_html(page_path.buf);
}
-void help_unknown_cmd(const char *cmd)
+static const char *levenshtein_cmd;
+static int similarity(const char *cmd) {
+ return levenshtein(levenshtein_cmd, cmd, 0, 2, 1, 4);
+}
+
+static int levenshtein_compare(const void *p1, const void *p2)
+{
+ const struct cmdname *const *c1 = p1, *const *c2 = p2;
+ const char *s1 = (*c1)->name, *s2 = (*c2)->name;
+ int l1 = similarity(s1);
+ int l2 = similarity(s2);
+ return l1 != l2 ? l1 - l2 : strcmp(s1, s2);
+}
+
+const char *help_unknown_cmd(const char *cmd)
{
+ int i, best_similarity = 0;
+ char cwd[PATH_MAX];
+
+ if (!getcwd(cwd, sizeof(cwd))) {
+ error("Could not get current working directory");
+ cwd[0] = '\0';
+ }
+
+ load_command_list();
+ ALLOC_GROW(main_cmds.names, main_cmds.cnt + other_cmds.cnt,
+ main_cmds.alloc);
+ memcpy(main_cmds.names + main_cmds.cnt, other_cmds.names,
+ other_cmds.cnt * sizeof(other_cmds.names[0]));
+ main_cmds.cnt += other_cmds.cnt;
+
+ levenshtein_cmd = cmd;
+ qsort(main_cmds.names, main_cmds.cnt,
+ sizeof(*main_cmds.names), levenshtein_compare);
+
+ if (!main_cmds.cnt)
+ die ("Uh oh. Your system reports no Git commands at all.");
+ best_similarity = similarity(main_cmds.names[0]->name);
+ if (main_cmds.cnt < 2 || best_similarity <
+ similarity(main_cmds.names[1]->name)) {
+ if (!*cwd)
+ exit(1);
+ if (chdir(cwd))
+ die ("Could not change directory back to '%s'", cwd);
+ fprintf(stderr, "WARNING: You called a Git program named '%s', "
+ "which does not exist.\n"
+ "Continuing under the assumption that you meant '%s'\n",
+ cmd, main_cmds.names[0]->name);
+ return main_cmds.names[0]->name;
+ }
+
fprintf(stderr, "git: '%s' is not a git-command. See 'git --help'.\n", cmd);
+
+ if (best_similarity < 6) {
+ fprintf(stderr, "\nDid you mean one of these?\n");
+
+ for (i = 0; i < main_cmds.cnt && best_similarity ==
+ similarity(main_cmds.names[i]->name); i++)
+ fprintf(stderr, "\t%s\n", main_cmds.names[i]->name);
+ }
+
exit(1);
}
diff --git a/levenshtein.c b/levenshtein.c
new file mode 100644
index 0000000..db52f2c
--- /dev/null
+++ b/levenshtein.c
@@ -0,0 +1,47 @@
+#include "cache.h"
+#include "levenshtein.h"
+
+int levenshtein(const char *string1, const char *string2,
+ int w, int s, int a, int d)
+{
+ int len1 = strlen(string1), len2 = strlen(string2);
+ int *row0 = xmalloc(sizeof(int) * (len2 + 1));
+ int *row1 = xmalloc(sizeof(int) * (len2 + 1));
+ int *row2 = xmalloc(sizeof(int) * (len2 + 1));
+ int i, j;
+
+ for (j = 0; j <= len2; j++)
+ row1[j] = j * a;
+ for (i = 0; i < len1; i++) {
+ int *dummy;
+
+ row2[0] = (i + 1) * d;
+ for (j = 0; j < len2; j++) {
+ /* substitution */
+ row2[j + 1] = row1[j] + s * (string1[i] != string2[j]);
+ /* swap */
+ if (i > 0 && j > 0 && string1[i - 1] == string2[j] &&
+ string1[i] == string2[j - 1] &&
+ row2[j + 1] > row0[j - 1] + w)
+ row2[j + 1] = row0[j - 1] + w;
+ /* deletion */
+ if (j + 1 < len2 && row2[j + 1] > row1[j + 1] + d)
+ row2[j + 1] = row1[j + 1] + d;
+ /* insertion */
+ if (row2[j + 1] > row2[j] + a)
+ row2[j + 1] = row2[j] + a;
+ }
+
+ dummy = row0;
+ row0 = row1;
+ row1 = row2;
+ row2 = dummy;
+ }
+
+ i = row1[len2];
+ free(row0);
+ free(row1);
+ free(row2);
+
+ return i;
+}
diff --git a/levenshtein.h b/levenshtein.h
new file mode 100644
index 0000000..0173abe
--- /dev/null
+++ b/levenshtein.h
@@ -0,0 +1,8 @@
+#ifndef LEVENSHTEIN_H
+#define LEVENSHTEIN_H
+
+int levenshtein(const char *string1, const char *string2,
+ int swap_penalty, int substition_penalty,
+ int insertion_penalty, int deletion_penalty);
+
+#endif
--
1.6.0.rc0.21.g91175
^ permalink raw reply related
* Re: exit status 141 from git-svn
From: Jan Hudec @ 2008-07-22 17:52 UTC (permalink / raw)
To: Frederik Hohlfeld; +Cc: git
In-Reply-To: <loom.20080623T145909-992@post.gmane.org>
On Mon, Jun 23, 2008 at 15:00:41 +0000, Frederik Hohlfeld wrote:
> Hi
Sorry for late reply. I just noticed your message after returning from
holiday *and* getting time to try to catch up with the list. Hope a reply
might still be useful.
> I have init'ed a git repository with git svn init.
>
> Now I'm using git svn fetch, but it stops after just a few files/revisions. The
> exit status is 141.
>
> What does this 141 want to tell me?
Unix trivia: When process dies from a signal, it's return status (as returned
by wait/waitpid(2) is signal_number * 256. However when bourne/POSIX shell
sees this, it will convert it to 128 + signal_number.
Now 141 - 128 = 13 and signal 13 is SIGPIPE. Process gets a SIGPIPE when it's
writing to a pipe (or socket) and it gets closed on the reading end, so it
sounds like a bad redirection somewhere. Aren't you by chance redirecting the
output to less and than quitting that without first scrolling to the bottom?
(That would be the most situation where process gets SIGPIPE -- and the very
reason why SIGPIPE exists, so that simple commands (like cat) are simply
stopped when nobody is interested in their output anymore).
--
Jan 'Bulb' Hudec <bulb@ucw.cz>
^ permalink raw reply
* [PATCH] Build configuration to skip ctime for modification test
From: Alex Riesen @ 2008-07-22 19:39 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Sixt, git
In-Reply-To: <7vy73tltf5.fsf@gitster.siamese.dyndns.org>
On Windows, the only file attribute we need (executable) cannot be
used, so ctime can be ignored as well. Change time is updated when
file attributes were changed (or it is written to, but in this case,
mtime is updated as well).
Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
---
Junio C Hamano, Tue, Jul 22, 2008 19:28:46 +0200:
> Johannes Sixt <j.sixt@viscovery.net> writes:
>
> >> + if ((changed & DATA_CHANGED) && (ce->ce_size != 0 || S_ISGITLINK(ce->ce_mode)))
> >
> > Does this mean that ce->ce_size is non-zero for gitlinks, at least on
> > Unix? Is this value useful in anyway? I don't think so. Then it shouldn't
> > be a random value that lstat() happens to return.
>
> These ce_xxx fields are the values we read from lstat(2) when the user
> told us to stage that working tree entity, be it a regular file, a
> symlink, or a directory that is a submodule. The only thing required for
> them is that they are stable (i.e. if you haven't touched the working tree
> entity, the value stays the same), and changes across modification. The
> value itself does not have to "mean" anything.
This reminds me... We can't use the only file attribute we care about
on Windows, so we can as well skip check for ctime. Besides, Google
Desktop Search keeps changing ctime when crawling files (ok, GDS is a
major usability nuance anyway, but the point is - we don't use the
file attribute).
read-cache.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/read-cache.c b/read-cache.c
index a50a851..c4f2718 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -181,8 +181,10 @@ static int ce_match_stat_basic(struct cache_entry *ce, struct stat *st)
}
if (ce->ce_mtime != (unsigned int) st->st_mtime)
changed |= MTIME_CHANGED;
+#ifndef NO_TRUSTABLE_FILEMODE
if (ce->ce_ctime != (unsigned int) st->st_ctime)
changed |= CTIME_CHANGED;
+#endif
if (ce->ce_uid != (unsigned int) st->st_uid ||
ce->ce_gid != (unsigned int) st->st_gid)
--
1.6.0.rc0.41.g70446
^ permalink raw reply related
* Re: [PATCH 0/9] Make gitexecdir relative to $(bindir) on Windows
From: Johannes Sixt @ 2008-07-22 19:31 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Junio C Hamano
In-Reply-To: <alpine.DEB.1.00.0807220140170.3407@eeepc-johanness>
On Dienstag, 22. Juli 2008, Johannes Schindelin wrote:
> Hi,
>
> On Mon, 21 Jul 2008, Johannes Sixt wrote:
> > The problem was that argv[0] does not have a path in certain cases.
>
> Note that the same holds true for Linux when calling a program that is in
> the PATH:
Oh, boy!
> I imagine that the proper solution would be to rip out lookup_prog() and
> use it for non-Windows Git, too. Unless you want to limit the usefulness
> of your patch series to Windows, that is.
This certainly goes beyond what I am prepared to do. It is not my itch. The
series is already much longer than I wanted, when there is a much simpler
solution that solves *my* problem: to set bindir = $(gitexecdir).
-- Hannes
^ permalink raw reply
* Re: [PATCH] parse-options: fix parsing of "--foobar=" with no value
From: Olivier Marin @ 2008-07-22 19:25 UTC (permalink / raw)
To: Pierre Habouzit, Olivier Marin, Junio C Hamano, git
In-Reply-To: <20080722185427.GA10453@artemis.madism.org>
Pierre Habouzit a écrit :
> On Tue, Jul 22, 2008 at 06:44:27PM +0000, Olivier Marin wrote:
>
> Wrong, --foobar= is the option "foobar" with the argument "" (empty
> string). as soon as you use the --foobar=... form, that is the "stuck
> form" for long option, there *is* a value.
Ah, OK.
I would have find it convenient for things like --foobar=$var where foobar
fallback to default when $var is empty. But I don't care that much.
> IOW --foobar= is not the same as --foobar at all. If like you claim,
> --foobar= pass a "random" value to the option then *this* is a bug, it
> should pass a pointer to an empty string (IOW a pointer that points to a
> NUL byte), but I see nothing in the code that would explain what you
> claim.
I found the "random bug" while migrating "git init" to parse-options. I
think you can reproduce it with:
$ git clone --template= <repo>
error: ignoring template /var/run/synaptic.socket
fatal: cannot opendir /var/run/sudo
But now, it appears the problem is not in parse-options, sorry.
--
Olivier.
^ permalink raw reply
* Re: [PATCH] parse-options: fix parsing of "--foobar=" with no value
From: Pierre Habouzit @ 2008-07-22 18:54 UTC (permalink / raw)
To: Olivier Marin; +Cc: Junio C Hamano, git
In-Reply-To: <1216752267-12138-1-git-send-email-dkr+ml.git@free.fr>
[-- Attachment #1: Type: text/plain, Size: 1031 bytes --]
On Tue, Jul 22, 2008 at 06:44:27PM +0000, Olivier Marin wrote:
> From: Olivier Marin <dkr@freesurf.fr>
>
> Before this patch, running a git command with a "--foobar=" argument
> will set the "foobar" option with a random value and continue.
> We should instead, exit with an error if a value is required, or use
> the default one if the value is optional.
Wrong, --foobar= is the option "foobar" with the argument "" (empty
string). as soon as you use the --foobar=... form, that is the "stuck
form" for long option, there *is* a value.
IOW --foobar= is not the same as --foobar at all. If like you claim,
--foobar= pass a "random" value to the option then *this* is a bug, it
should pass a pointer to an empty string (IOW a pointer that points to a
NUL byte), but I see nothing in the code that would explain what you
claim.
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: [PATCH] parse-options: fix parsing of "--foobar=" with no value
From: Sverre Rabbelier @ 2008-07-22 18:53 UTC (permalink / raw)
To: Olivier Marin; +Cc: Pierre Habouzit, Junio C Hamano, git
In-Reply-To: <1216752267-12138-1-git-send-email-dkr+ml.git@free.fr>
On Tue, Jul 22, 2008 at 8:44 PM, Olivier Marin <dkr+ml.git@free.fr> wrote:
> We should instead, exit with an error if a value is required, or use
> the default one if the value is optional.
This makes no sense, when I run "git foo --bar=" I either mean "set
bar to empty" or I typo-ed. Why would I specify "--bar=" if I want the
default value?
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* [PATCH] parse-options: fix parsing of "--foobar=" with no value
From: Olivier Marin @ 2008-07-22 18:44 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: Junio C Hamano, git
From: Olivier Marin <dkr@freesurf.fr>
Before this patch, running a git command with a "--foobar=" argument
will set the "foobar" option with a random value and continue.
We should instead, exit with an error if a value is required, or use
the default one if the value is optional.
This patch fix the above issue and add some test cases.
Signed-off-by: Olivier Marin <dkr@freesurf.fr>
---
parse-options.c | 8 ++++----
t/t0040-parse-options.sh | 25 +++++++++++++++++++++++++
test-parse-options.c | 3 +++
3 files changed, 32 insertions(+), 4 deletions(-)
diff --git a/parse-options.c b/parse-options.c
index 71a7acf..67be197 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -17,7 +17,7 @@ static int opterror(const struct option *opt, const char *reason, int flags)
static int get_arg(struct parse_opt_ctx_t *p, const struct option *opt,
int flags, const char **arg)
{
- if (p->opt) {
+ if (p->opt && *p->opt) {
*arg = p->opt;
p->opt = NULL;
} else if (p->argc == 1 && (opt->flags & PARSE_OPT_LASTARG_DEFAULT)) {
@@ -80,7 +80,7 @@ static int get_value(struct parse_opt_ctx_t *p,
case OPTION_STRING:
if (unset)
*(const char **)opt->value = NULL;
- else if (opt->flags & PARSE_OPT_OPTARG && !p->opt)
+ else if (opt->flags & PARSE_OPT_OPTARG && (!p->opt || !*p->opt))
*(const char **)opt->value = (const char *)opt->defval;
else
return get_arg(p, opt, flags, (const char **)opt->value);
@@ -91,7 +91,7 @@ static int get_value(struct parse_opt_ctx_t *p,
return (*opt->callback)(opt, NULL, 1) ? (-1) : 0;
if (opt->flags & PARSE_OPT_NOARG)
return (*opt->callback)(opt, NULL, 0) ? (-1) : 0;
- if (opt->flags & PARSE_OPT_OPTARG && !p->opt)
+ if (opt->flags & PARSE_OPT_OPTARG && (!p->opt || !*p->opt))
return (*opt->callback)(opt, NULL, 0) ? (-1) : 0;
if (get_arg(p, opt, flags, &arg))
return -1;
@@ -102,7 +102,7 @@ static int get_value(struct parse_opt_ctx_t *p,
*(int *)opt->value = 0;
return 0;
}
- if (opt->flags & PARSE_OPT_OPTARG && !p->opt) {
+ if (opt->flags & PARSE_OPT_OPTARG && (!p->opt || !*p->opt)) {
*(int *)opt->value = opt->defval;
return 0;
}
diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh
index 03dbe00..7ce2e86 100755
--- a/t/t0040-parse-options.sh
+++ b/t/t0040-parse-options.sh
@@ -26,6 +26,8 @@ String options
--st <st> get another string (pervert ordering)
-o <str> get another string
--default-string set string to default
+ --optional-string[=<string>]
+ set string to optional if none given
Magic arguments
--quux means --quux
@@ -85,6 +87,29 @@ test_expect_success 'missing required value' '
test $? = 129
'
+test_expect_success 'missing required value with "--foobar="' '
+ test-parse-options --length=;
+ test $? = 129 &&
+ test-parse-options --len=;
+ test $? = 129
+'
+
+cat > expect << EOF
+boolean: 0
+integer: 0
+string: optional
+abbrev: 7
+verbose: 0
+quiet: no
+dry run: no
+EOF
+
+test_expect_success 'optional value with "--foobar="' '
+ test-parse-options --optional-string= > output 2> output.err &&
+ test ! -s output.err &&
+ test_cmp expect output
+'
+
cat > expect << EOF
boolean: 1
integer: 13
diff --git a/test-parse-options.c b/test-parse-options.c
index 2a79e72..76db37e 100644
--- a/test-parse-options.c
+++ b/test-parse-options.c
@@ -42,6 +42,9 @@ int main(int argc, const char **argv)
OPT_STRING('o', NULL, &string, "str", "get another string"),
OPT_SET_PTR(0, "default-string", &string,
"set string to default", (unsigned long)"default"),
+ { OPTION_STRING, 0, "optional-string", &string,
+ "string", "set string to optional if none given",
+ PARSE_OPT_OPTARG, NULL, (unsigned long)"optional" },
OPT_GROUP("Magic arguments"),
OPT_ARGUMENT("quux", "means --quux"),
OPT_GROUP("Standard options"),
--
1.6.0.rc0.16.gb169.dirty
^ permalink raw reply related
* Re: Computing the number of patches in linux-next tree
From: Harvey Harrison @ 2008-07-22 18:23 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, Tony Luck, Git Mailing List
In-Reply-To: <7vabg9n93f.fsf@gitster.siamese.dyndns.org>
On Tue, 2008-07-22 at 10:04 -0700, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > On Tue, 22 Jul 2008, Tony Luck wrote:
> >
> >> git tag | grep next- | sort | while read tag
> >
> > This should not be necessary... AFAICT "git tag" sorts its output already.
> >
> >> What does the "git-where-did-this-tag-branch-from-linus" command look like?
> >
> > git merge-base --all <branch1> <branch2>
> >
> > Be warned: there might be multiple merge bases.
>
> I do not think that approach applies to linux-next, which is constantly
> rewound to the then-tip-of-linus and merge remaining bits. The question
> is "where does this branch begin", which does not have an answer in git.
I thought that was what Stephen has the next/stable branch for:
git log --pretty=oneline --no-merges next/stable..next/master | wc -l
At least for each day you can find the number of patches....I don't
think he tags the stable points, so historically that may be a problem.
Harvey
^ permalink raw reply
* Re: [PATCH 1/2] Introduce leaky().
From: Jan Hudec @ 2008-07-22 18:09 UTC (permalink / raw)
To: Pierre Habouzit, Junio C Hamano, git
In-Reply-To: <20080626213304.GA22111@artemis.madism.org>
On Thu, Jun 26, 2008 at 23:33:04 +0200, Pierre Habouzit wrote:
> On Thu, Jun 26, 2008 at 06:46:43PM +0000, Junio C Hamano wrote:
> > The user would also need to worry about not freeing the original
> > allocation pointer when something is realloc(3)ed, which means either
> > finding the last realloc(3) of the object (that is logically the same, but
> > just being extended) and mark the pointer as leaky() after that realloc,
> > or unregister the original pointer from leaks before calling realloc and
> > register what comes back. It will easily get messy.
>
> Hmm indeed, maybe it isn't such a good idea then.
I don't think it's necessary either. Valgrind (unlike other leak detectors
the poor windows folks are stuck with) is quite smart and will only report
memory as leaked when there is no pointer to it. So it should not report any
memory pointed to by index or any other static structure.
Additionally since valgrind allows one to write rules to exclude
uninteresting reports. So a good approach would be to just keep such
exclusion file around in git with the known uninteresting cases.
> > By the way, the series queued in your repository still has "s/pring/print/"
> > typo in 4/7 and "argv not NULL terminated" comment in 6/7.
>
> I'll fix that and pushed again, without the leaky() series dependency
> (I've put in a comment that I'm aware that it's a leak), and with the
> two fixes you mention done.
>
> --
> ·O· Pierre Habouzit
> ··O madcoder@debian.org
> OOO http://www.madism.org
--
Jan 'Bulb' Hudec <bulb@ucw.cz>
^ permalink raw reply
* Re: [PATCH] Fix update-index --refresh for submodules if stat(2) returns st_size 0
From: Junio C Hamano @ 2008-07-22 17:28 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Alex Riesen, git
In-Reply-To: <4885897C.8010401@viscovery.net>
Johannes Sixt <j.sixt@viscovery.net> writes:
>> + if ((changed & DATA_CHANGED) && (ce->ce_size != 0 || S_ISGITLINK(ce->ce_mode)))
>
> Does this mean that ce->ce_size is non-zero for gitlinks, at least on
> Unix? Is this value useful in anyway? I don't think so. Then it shouldn't
> be a random value that lstat() happens to return.
These ce_xxx fields are the values we read from lstat(2) when the user
told us to stage that working tree entity, be it a regular file, a
symlink, or a directory that is a submodule. The only thing required for
them is that they are stable (i.e. if you haven't touched the working tree
entity, the value stays the same), and changes across modification. The
value itself does not have to "mean" anything.
When trying to see if the user has changes in the working tree entity
since the last such staging of the path, we compare that value with what
comes back from lstat(2), before actually comparing the contents. If the
filesize changed, they cannot be the same and the code says you have
modified it without having to look at the contents.
Side note. This is why you need to be careful after modifying
autocrlf related configuration and attributes. If you had CRLF
contents in the working tree that was incorrectly staged as-is,
then switch autocrlf-on, and "git add" to fix the staged copy to
be LF-terminated, we say "it's unchanged and we do not bother
rehashing" by comparing the ce_xxx fields without looking at the
contents (this is an absolutely necessary optimization to make
"git add ." usable), because ce_size records the size of CRLF
version you have in the working tree, and you haven't changed the
working tree file in this sequence above.
Removing the file and checking things out would be the most
straightforward solution in such a case.
We used to include ce_dev (taken from struct stat.st_dev) in the list of
fields to cache and compare to detect changes, but that is now excluded
because it is not stable (see comments in read-cache.c). If the directory
size is unstable, perhaps it would be better to force it to some fixed
magic value so that it is not used by this "quick change detection" check.
If you network-mount the same directory from POSIX and windows, the former
may give "storage size of the directory" while the latter may give 0.
This would mean that you would need a "update-index --refresh" when you
switch between such machines.
^ permalink raw reply
* Re: Computing the number of patches in linux-next tree
From: Johannes Schindelin @ 2008-07-22 17:24 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Tony Luck, Git Mailing List
In-Reply-To: <7vabg9n93f.fsf@gitster.siamese.dyndns.org>
Hi,
On Tue, 22 Jul 2008, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > On Tue, 22 Jul 2008, Tony Luck wrote:
> >
> >> git tag | grep next- | sort | while read tag
> >
> > This should not be necessary... AFAICT "git tag" sorts its output already.
> >
> >> What does the "git-where-did-this-tag-branch-from-linus" command look like?
> >
> > git merge-base --all <branch1> <branch2>
> >
> > Be warned: there might be multiple merge bases.
>
> I do not think that approach applies to linux-next, which is constantly
> rewound to the then-tip-of-linus and merge remaining bits. The question
> is "where does this branch begin", which does not have an answer in git.
Oh.
Well, there is one thing that _could_ work most of the time, namely
looking at the committer info of the first parent.
Dunno,
Dscho
^ permalink raw reply
* RE: Computing the number of patches in linux-next tree
From: Luck, Tony @ 2008-07-22 17:13 UTC (permalink / raw)
To: Junio C Hamano, Johannes Schindelin; +Cc: Git Mailing List
In-Reply-To: <7vabg9n93f.fsf@gitster.siamese.dyndns.org>
>> git merge-base --all <branch1> <branch2>
>>
>> Be warned: there might be multiple merge bases.
>
> I do not think that approach applies to linux-next, which is constantly
> rewound to the then-tip-of-linus and merge remaining bits. The question
> is "where does this branch begin", which does not have an answer in git.
Using git merge-base on the next-20080701 tag and current Linus tree I get
76 possible merge bases. None of them appear to be the "right" one (if
I check out this tag and look at Next/merge.log the right answer appears
to be 1702b52 if I'm reading the log correctly).
Perhaps my best hope is to
$ git checkout $tag Next/merge.log
... parse merge.log to figure out $base ...
-Tony
^ permalink raw reply
* Re: [PATCH] bring description of git diff --cc up to date
From: Junio C Hamano @ 2008-07-22 17:09 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, David Greaves
In-Reply-To: <20080722111947.BIW29914@m4500-01.uchicago.edu>
Jonathan Nieder <jrnieder@uchicago.edu> writes:
> Just to make sure I understand, here is what I think --cc does:
>
> - In a two-parent merge, it is exactly as Linus has been
> ...
> - In a many-parent merge, the criterion is more stringent.
> ...
>
> Is that correct?
The logic in the code does not have separate criteria for two-parent and
Octopus cases. Actually Linus talks about "when you have two versions to
choose from, and if the result matches one of them, then it is not
interesting". In a two-parent merge, you cannot have three or more
possible versions to choose from by definition, can you?
^ permalink raw reply
* [PATCH] t7601: extend the 'merge picks up the best result' test
From: Miklos Vajna @ 2008-07-22 17:05 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v7ibenx75.fsf@gitster.siamese.dyndns.org>
The test only checked if the best result picking code works if there are
multiple strategies set in the config. Add a similar one that tests if
the same true if the -s option of git merge was used multiple times.
Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---
On Tue, Jul 22, 2008 at 01:24:14AM -0700, Junio C Hamano <gitster@pobox.com> wrote:
> Don't. pull.* has always been defined as "list of strategies", and -s
> has
> always been defined to take "a" strategy.
OK. Here is a testcase for the later. As far as I see the behaviour of
multiple -s was not checked till now.
t/t7601-merge-pull-config.sh | 15 +++++++++++++++
1 files changed, 15 insertions(+), 0 deletions(-)
diff --git a/t/t7601-merge-pull-config.sh b/t/t7601-merge-pull-config.sh
index 6b9f638..55aa6b5 100755
--- a/t/t7601-merge-pull-config.sh
+++ b/t/t7601-merge-pull-config.sh
@@ -112,6 +112,21 @@ test_expect_success 'setup conflicted merge' '
# recusive is choosen.
test_expect_success 'merge picks up the best result' '
+ git config --unset-all pull.twohead &&
+ git reset --hard c5 &&
+ git merge -s resolve c6
+ resolve_count=$(conflict_count) &&
+ git reset --hard c5 &&
+ git merge -s recursive c6
+ recursive_count=$(conflict_count) &&
+ git reset --hard c5 &&
+ git merge -s recursive -s resolve c6
+ auto_count=$(conflict_count) &&
+ test $auto_count = $recursive_count &&
+ test $auto_count != $resolve_count
+'
+
+test_expect_success 'merge picks up the best result (from config)' '
git config pull.twohead "recursive resolve" &&
git reset --hard c5 &&
git merge -s resolve c6
--
1.5.6.4.433.g09651.dirty
^ permalink raw reply related
* Re: Computing the number of patches in linux-next tree
From: Junio C Hamano @ 2008-07-22 17:04 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Tony Luck, Git Mailing List
In-Reply-To: <alpine.DEB.1.00.0807221727210.8986@racer>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> On Tue, 22 Jul 2008, Tony Luck wrote:
>
>> git tag | grep next- | sort | while read tag
>
> This should not be necessary... AFAICT "git tag" sorts its output already.
>
>> What does the "git-where-did-this-tag-branch-from-linus" command look like?
>
> git merge-base --all <branch1> <branch2>
>
> Be warned: there might be multiple merge bases.
I do not think that approach applies to linux-next, which is constantly
rewound to the then-tip-of-linus and merge remaining bits. The question
is "where does this branch begin", which does not have an answer in git.
^ permalink raw reply
* Re: [PATCH] Fix update-index --refresh for submodules if stat(2) returns st_size 0
From: Johannes Sixt @ 2008-07-22 16:59 UTC (permalink / raw)
To: Alex Riesen; +Cc: Junio C Hamano, git
In-Reply-To: <20080722164604.GA3766@blimp.local>
Alex Riesen schrieb:
> Johannes Sixt, Tue, Jul 22, 2008 09:17:16 +0200:
>> Alex Riesen schrieb:
>>> + if ((changed & DATA_CHANGED) && (ce->ce_size != 0 || S_ISGITLINK(ce->ce_mode)))
>> Does this mean that ce->ce_size is non-zero for gitlinks, at least on
>> Unix?
>
> It is non-zero for directories (which is what gitlinks are in working
> directories) on UNIX operating systems I met.
>
>> Is this value useful in anyway?
>
> Sometimes it is (the size a directory takes on storage)
Sure; but is ce->ce_size of gitlinks useful?
-- Hannes
^ permalink raw reply
* Re: problem using jgit
From: Shawn O. Pearce @ 2008-07-22 16:58 UTC (permalink / raw)
To: Marek Zawirski; +Cc: Stephen Bannasch, git, Robin Rosenberg
In-Reply-To: <488482A2.4000601@gmail.com>
Marek Zawirski <marek.zawirski@gmail.com> wrote:
> Marek Zawirski wrote:
>> Stephen Bannasch wrote:
>>> I've setup a simple test class that integrates jgit to clone a git
>>> repository. However I'm getting a NullPointerError when
>>> RevWalk.parseAny ends up producing a null object id.
...
> It's caused by 14a630c3: Cached modification times for symbolic refs too
> Changes introduced by this patch made Repository#getAllRefs() including
> Ref objects with null ObjectId in case of unresolvable (invalid?) HEAD
> symbolic ref, and null Ref for HEAD when it doesn't exist. Previous
> behavior was just not including such refs in result.
My intention here was that if a ref cannot be resolved, it should
not be reported. So Ref.getObjectId should never return null, and
it should also never return an ObjectId for which the object does
not exist in the Repository's object database(s). (Though that can
happen in the face of repository corruption, but lets not go there
just yet).
So IMHO the RefDatabase code is _wrong_ for returning HEAD with a
null objectId.
Now this case can happen if HEAD points at a stillborn branch. This
is easily reproduced in any repository, e.g. just do:
git symbolic-ref HEAD refs/heads/`date`
You'll wind up on a branch which doesn't exist. In this case HEAD
shouldn't be reported back from RefDatabase, it doesn't exist, as
branch `date` does not exist either.
--
Shawn.
^ permalink raw reply
* Re: [PATCH] Fix update-index --refresh for submodules if stat(2) returns st_size 0
From: Alex Riesen @ 2008-07-22 16:46 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Junio C Hamano, git
In-Reply-To: <4885897C.8010401@viscovery.net>
Johannes Sixt, Tue, Jul 22, 2008 09:17:16 +0200:
> Alex Riesen schrieb:
> > Can MSys folks please try it? I noticed it when the test
> > t2103-update-index-ignore-missing.sh (the 5th case) started failing.
>
> I tested it. mingw.git does suffer from the problem, and this fixes it.
>
Yes, I did too (at work).
> > + if ((changed & DATA_CHANGED) && (ce->ce_size != 0 || S_ISGITLINK(ce->ce_mode)))
>
> Does this mean that ce->ce_size is non-zero for gitlinks, at least on
> Unix?
It is non-zero for directories (which is what gitlinks are in working
directories) on UNIX operating systems I met.
> Is this value useful in anyway?
Sometimes it is (the size a directory takes on storage)
> I don't think so. Then it shouldn't be a random value that lstat()
> happens to return.
The problem is: it is not random. I even suspect that Windows is the
ONLY system which has st_size 0 for directories.
^ permalink raw reply
* Re: [PATCH] Fix update-index --refresh for submodules if stat(2) returns st_size 0
From: Alex Riesen @ 2008-07-22 16:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, git
In-Reply-To: <7vbq0qnxyi.fsf@gitster.siamese.dyndns.org>
Junio C Hamano, Tue, Jul 22, 2008 10:07:49 +0200:
> I however have to wonder if you also need to touch the end of
> ce_match_stat_basic() that checks for zero sized cache entry.
I frankly don't know.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox