* Re: [PATCH] bash completion: Fix the . -> .. revision range completion
From: Andreas Ericsson @ 2008-07-15 8:05 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Linus Torvalds, Junio C Hamano, Petr Baudis, git
In-Reply-To: <20080715042553.GD2432@spearce.org>
Shawn O. Pearce wrote:
> Linus Torvalds <torvalds@linux-foundation.org> wrote:
>> On Mon, 14 Jul 2008, Shawn O. Pearce wrote:
>>> What is $COMP_WORDBREAKS set to in your shell? In mine it
>>> appears to be:
>>>
>>> " \"'@><=;|&(:"
>> Ahhah. Indeed. I don't have the ':'.
> ...
>> Umm, if so, git should just set it in the completion script, no?
>
> OK, so it turns out not having : in COMP_WORDBREAKS is a very common
> case that we should somehow deal with, to aid our users.
>
> I'm concerned about just setting COMP_WORDBREAKS back to the default
> in the git completion script because then we get into an ordering
> game with the profile scripts, don't we? If git completion sources
> before the gvfs script we don't get our COMP_WORDBREAKS setting.
>
> I think we may need to do two things.
>
> If COMP_WORDBREAKS doesn't contain a :, try to reset it to include
> one when the script is sourced. This may "fix" git completion but
> make gvfs completion act differently, resulting in a thread on the
> gvfs lists. ;-)
>
> If COMP_WORDBREAKS doesn't contain : during a completion event than
> we need to do what your original patch asked, which is to include
> "$ref:" in the prefix, so the ref isn't lost.
>
> At least we understand the problem now, finally. I'll try to write
> up a patch for it tomorrow. Unfortunately packing to move has been
> really sucking up my time lately.
>
I beat you to it ;-) This works just fine for me regardless of whether
or not I have a colon in COMP_WORDBREAKS.
--%<--%<--%<--
From: Andreas Ericsson <ae@op5.se>
Subject: git-completion.bash: Handle "rev:path" completion properly
The gvfs package on at least Fedora9 installs its own bash
completion script which removes the colon from COMP_WORDBREAKS,
which acts as a list of characters where bash should consider
as word boundaries. Doing so breaks the git bash completion
script when handling any rev:path style argument.
This patch fixes it by prepending the "rev" part and the colon
(which otherwise gets lost) before adding the "path" part if
COMP_WORDBREAKS doesn't contain the colon we would otherwise
need.
Also fixes a nearby indented-with-spaces issue.
Spotted-by: Linus Torvalds <torvalds@linux-foundation.org>
Investigated-by: Björn Steinbrink <b.steinbrink@gmx.de>
Signed-off-by: Andreas Ericsson <ae@op5.se>
---
contrib/completion/git-completion.bash | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index d268e6f..e138022 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -293,7 +293,11 @@ __git_complete_file ()
*)
ls="$ref"
;;
- esac
+ esac
+ # When completing something like 'rev:path', bash behaves
+ # differently whether or not COMP_WORDBREAKS contains a
+ # colon or not. This lets it handle both cases
+ test "${COMP_WORDBREAKS//:}" = "$COMP_WORDBREAKS" && pfx="$ref:$pfx"
COMPREPLY=($(compgen -P "$pfx" \
-W "$(git --git-dir="$(__gitdir)" ls-tree "$ls" \
| sed '/^100... blob /s,^.* ,,
--
1.5.6.3.315.g10ce0
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply related
* Re: [PATCH 0/5] replacement for the part of js/more-win that is in pu
From: Johannes Sixt @ 2008-07-15 7:59 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Steffen Prohaska, git
In-Reply-To: <1216071689-14823-1-git-send-email-johannes.sixt@telecom.at>
Johannes Sixt schrieb:
> The interdiff to js/more-win is below. It is mostly the changes
> of 1/5.
>
> Johannes Sixt (5):
> Makefile: Normalize $(bindir) and $(gitexecdir) before comparing
> Record the command invocation path early
> Fix relative built-in paths to be relative to the command
> invocation
> Allow the built-in exec path to be relative to the command
> invocation path
> Allow add_path() to add non-existent directories to the path
I retract this series and also the earlier version (the part that is in pu).
If I had done due diligence, I could have found out earlier that it does
not solve the problem it tried to solve. Appologies for the noise. :-(
The series tries to derive the exec-path from argv[0] (if the built-in
path is relative). But if a command is invoked from CMD on Windows,
argv[0] doesn't have a path, there is only the program name, "git.exe". In
the past, we relied on the global variable _pgmptr (only Windows's C
runtime has this), which does contain the full path, and if we set
gitexecdir = $(bindir)
in the Makefile, then we get a working git.exe, but we put back all
commands into $PATH.
-- Hannes
^ permalink raw reply
* Re: [PATCH 1/6] archive: remove args member from struct archiver
From: René Scharfe @ 2008-07-15 7:49 UTC (permalink / raw)
Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <487BA7C1.3000109@lsrfire.ath.cx>
[Resent with fixed line wrap.]
Pass struct archiver and struct archiver_args explicitly to parse_archive_args
and remove the latter from the former. This allows us to get rid of struct
archiver_desc and simplifies the code a bit.
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
archive.h | 5 +---
builtin-archive.c | 51 +++++++++++++++++++---------------------------
builtin-upload-archive.c | 11 +++++----
3 files changed, 28 insertions(+), 39 deletions(-)
diff --git a/archive.h b/archive.h
index ddf004a..1b24ae3 100644
--- a/archive.h
+++ b/archive.h
@@ -21,14 +21,11 @@ typedef void *(*parse_extra_args_fn_t)(int argc, const char **argv);
struct archiver {
const char *name;
- struct archiver_args args;
write_archive_fn_t write_archive;
parse_extra_args_fn_t parse_extra;
};
-extern int parse_archive_args(int argc,
- const char **argv,
- struct archiver *ar);
+extern int parse_archive_args(int argc, const char **argv, const struct archiver **ar, struct archiver_args *args);
extern void parse_treeish_arg(const char **treeish,
struct archiver_args *ar_args,
diff --git a/builtin-archive.c b/builtin-archive.c
index c2e0c1e..6ee3677 100644
--- a/builtin-archive.c
+++ b/builtin-archive.c
@@ -15,12 +15,7 @@
static const char archive_usage[] = \
"git-archive --format=<fmt> [--prefix=<prefix>/] [--verbose] [<extra>] <tree-ish> [path...]";
-static struct archiver_desc
-{
- const char *name;
- write_archive_fn_t write_archive;
- parse_extra_args_fn_t parse_extra;
-} archivers[] = {
+const struct archiver archivers[] = {
{ "tar", write_tar_archive, NULL },
{ "zip", write_zip_archive, parse_extra_zip_args },
};
@@ -79,21 +74,15 @@ static int run_remote_archiver(const char *remote, int argc,
return !!rv;
}
-static int init_archiver(const char *name, struct archiver *ar)
+static const struct archiver *lookup_archiver(const char *name)
{
- int rv = -1, i;
+ int i;
for (i = 0; i < ARRAY_SIZE(archivers); i++) {
- if (!strcmp(name, archivers[i].name)) {
- memset(ar, 0, sizeof(*ar));
- ar->name = archivers[i].name;
- ar->write_archive = archivers[i].write_archive;
- ar->parse_extra = archivers[i].parse_extra;
- rv = 0;
- break;
- }
+ if (!strcmp(name, archivers[i].name))
+ return &archivers[i];
}
- return rv;
+ return NULL;
}
void parse_pathspec_arg(const char **pathspec, struct archiver_args *ar_args)
@@ -145,7 +134,8 @@ void parse_treeish_arg(const char **argv, struct archiver_args *ar_args,
ar_args->time = archive_time;
}
-int parse_archive_args(int argc, const char **argv, struct archiver *ar)
+int parse_archive_args(int argc, const char **argv, const struct archiver **ar,
+ struct archiver_args *args)
{
const char *extra_argv[MAX_EXTRA_ARGS];
int extra_argc = 0;
@@ -190,17 +180,18 @@ int parse_archive_args(int argc, const char **argv, struct archiver *ar)
/* We need at least one parameter -- tree-ish */
if (argc - 1 < i)
usage(archive_usage);
- if (init_archiver(format, ar) < 0)
+ *ar = lookup_archiver(format);
+ if (!*ar)
die("Unknown archive format '%s'", format);
if (extra_argc) {
- if (!ar->parse_extra)
+ if (!(*ar)->parse_extra)
die("'%s' format does not handle %s",
- ar->name, extra_argv[0]);
- ar->args.extra = ar->parse_extra(extra_argc, extra_argv);
+ (*ar)->name, extra_argv[0]);
+ args->extra = (*ar)->parse_extra(extra_argc, extra_argv);
}
- ar->args.verbose = verbose;
- ar->args.base = base;
+ args->verbose = verbose;
+ args->base = base;
return i;
}
@@ -238,7 +229,8 @@ static const char *extract_remote_arg(int *ac, const char **av)
int cmd_archive(int argc, const char **argv, const char *prefix)
{
- struct archiver ar;
+ const struct archiver *ar = NULL;
+ struct archiver_args args;
int tree_idx;
const char *remote = NULL;
@@ -248,14 +240,13 @@ int cmd_archive(int argc, const char **argv, const char *prefix)
setvbuf(stderr, NULL, _IOLBF, BUFSIZ);
- memset(&ar, 0, sizeof(ar));
- tree_idx = parse_archive_args(argc, argv, &ar);
+ tree_idx = parse_archive_args(argc, argv, &ar, &args);
if (prefix == NULL)
prefix = setup_git_directory();
argv += tree_idx;
- parse_treeish_arg(argv, &ar.args, prefix);
- parse_pathspec_arg(argv + 1, &ar.args);
+ parse_treeish_arg(argv, &args, prefix);
+ parse_pathspec_arg(argv + 1, &args);
- return ar.write_archive(&ar.args);
+ return ar->write_archive(&args);
}
diff --git a/builtin-upload-archive.c b/builtin-upload-archive.c
index 371400d..295e24c 100644
--- a/builtin-upload-archive.c
+++ b/builtin-upload-archive.c
@@ -19,7 +19,8 @@ static const char lostchild[] =
static int run_upload_archive(int argc, const char **argv, const char *prefix)
{
- struct archiver ar;
+ const struct archiver *ar;
+ struct archiver_args args;
const char *sent_argv[MAX_ARGS];
const char *arg_cmd = "argument ";
char *p, buf[4096];
@@ -65,12 +66,12 @@ static int run_upload_archive(int argc, const char **argv, const char *prefix)
sent_argv[sent_argc] = NULL;
/* parse all options sent by the client */
- treeish_idx = parse_archive_args(sent_argc, sent_argv, &ar);
+ treeish_idx = parse_archive_args(sent_argc, sent_argv, &ar, &args);
- parse_treeish_arg(sent_argv + treeish_idx, &ar.args, prefix);
- parse_pathspec_arg(sent_argv + treeish_idx + 1, &ar.args);
+ parse_treeish_arg(sent_argv + treeish_idx, &args, prefix);
+ parse_pathspec_arg(sent_argv + treeish_idx + 1, &args);
- return ar.write_archive(&ar.args);
+ return ar->write_archive(&args);
}
static void error_clnt(const char *fmt, ...)
--
1.5.6.2.212.g08b51
^ permalink raw reply related
* Re: [PATCH 3/6] archive: add baselen member to struct archiver_args
From: René Scharfe @ 2008-07-15 7:49 UTC (permalink / raw)
Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <487BA7C9.6090908@lsrfire.ath.cx>
[Resent with fixed line wrap.]
Calculate the length of base and save it in a new member of struct
archiver_args. This way we don't have to compute it in each of the
format backends.
Note: parse_archive_args() guarantees that ->base won't ever be NULL.
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
archive-tar.c | 8 +++-----
archive-zip.c | 8 +++-----
archive.h | 1 +
builtin-archive.c | 1 +
4 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/archive-tar.c b/archive-tar.c
index 6eaf59e..63cc2ec 100644
--- a/archive-tar.c
+++ b/archive-tar.c
@@ -268,19 +268,17 @@ static int write_tar_entry(const unsigned char *sha1, const char *base,
int write_tar_archive(struct archiver_args *args)
{
- int plen = args->base ? strlen(args->base) : 0;
-
git_config(git_tar_config, NULL);
archive_time = args->time;
verbose = args->verbose;
commit = args->commit;
- base_len = args->base ? strlen(args->base) : 0;
+ base_len = args->baselen;
if (args->commit_sha1)
write_global_extended_header(args->commit_sha1);
- if (args->base && plen > 0 && args->base[plen - 1] == '/') {
+ if (args->baselen > 0 && args->base[args->baselen - 1] == '/') {
char *base = xstrdup(args->base);
int baselen = strlen(base);
@@ -290,7 +288,7 @@ int write_tar_archive(struct archiver_args *args)
0, NULL);
free(base);
}
- read_tree_recursive(args->tree, args->base, plen, 0,
+ read_tree_recursive(args->tree, args->base, args->baselen, 0,
args->pathspec, write_tar_entry, NULL);
write_trailer();
diff --git a/archive-zip.c b/archive-zip.c
index 0d24f3f..d18254c 100644
--- a/archive-zip.c
+++ b/archive-zip.c
@@ -316,17 +316,15 @@ static void dos_time(time_t *time, int *dos_date, int *dos_time)
int write_zip_archive(struct archiver_args *args)
{
- int plen = strlen(args->base);
-
dos_time(&args->time, &zip_date, &zip_time);
zip_dir = xmalloc(ZIP_DIRECTORY_MIN_SIZE);
zip_dir_size = ZIP_DIRECTORY_MIN_SIZE;
verbose = args->verbose;
commit = args->commit;
- base_len = args->base ? strlen(args->base) : 0;
+ base_len = args->baselen;
- if (args->base && plen > 0 && args->base[plen - 1] == '/') {
+ if (args->baselen > 0 && args->base[args->baselen - 1] == '/') {
char *base = xstrdup(args->base);
int baselen = strlen(base);
@@ -336,7 +334,7 @@ int write_zip_archive(struct archiver_args *args)
0, NULL);
free(base);
}
- read_tree_recursive(args->tree, args->base, plen, 0,
+ read_tree_recursive(args->tree, args->base, args->baselen, 0,
args->pathspec, write_zip_entry, NULL);
write_zip_trailer(args->commit_sha1);
diff --git a/archive.h b/archive.h
index 1b24ae3..34151f4 100644
--- a/archive.h
+++ b/archive.h
@@ -6,6 +6,7 @@
struct archiver_args {
const char *base;
+ size_t baselen;
struct tree *tree;
const unsigned char *commit_sha1;
const struct commit *commit;
diff --git a/builtin-archive.c b/builtin-archive.c
index 6ee3677..e7f4ec6 100644
--- a/builtin-archive.c
+++ b/builtin-archive.c
@@ -192,6 +192,7 @@ int parse_archive_args(int argc, const char **argv, const struct archiver **ar,
}
args->verbose = verbose;
args->base = base;
+ args->baselen = strlen(base);
return i;
}
--
1.5.6.2.212.g08b51
^ permalink raw reply related
* Re: [PATCH 0/6] archive: refactor and cleanup
From: René Scharfe @ 2008-07-15 7:49 UTC (permalink / raw)
To: Lea Wiemann; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <487BE440.9010006@gmail.com>
Lea Wiemann schrieb:
> René Scharfe wrote:
>> This series is a collection of cleanups for git archive
>
> A few things:
>
> * The note quoted above probably shouldn't be in the commit message but
> after the "---".
Since it's in the zeroth message, it's not intended to be committed.
> * You patch messages seem to be In-Reply-To a message that wasn't posted
> on the list, not to PATCH 0/6.
Hmm, that's strange. Won't happen next time, though.
> * Unless you have a specific reason, I suggest that you don't Cc Junio
> on patches; he reads all messages on this list (more or less) and will
> usually simply apply your patches once they're reviewed. (Watch for his
> periodical "What's cooking in git.git" messages to see if he got them.)
We're both wrong, quoth Documentation/SubmittingPatches:
Note that your maintainer does not necessarily read everything
on the git mailing list. If your patch is for discussion first,
send it "To:" the mailing list, and optionally "cc:" him. If it
is trivially correct or after the list reached a consensus, send
it "To:" the maintainer and optionally "cc:" the list.
Obviously, my patches weren't trivially correct nor a consensus reached.
> * And most importantly, your Thunderbird introduced line-breaks, so it's
> not possible to apply the patches. May I suggest you use git-send-email
> instead?
Well, it is possible to send usable patches with Thunderbird, using the
extension Toggle Word Wrap. Patches 1 and 3 are damaged because I
edited them again to correct a typo -- with word wrap accidentally on.
Trying git-send-email again is a good idea, though.
> Feel free to ping me on IRC (lea_w in #git) if you need help.
I've slacked long enough to become a complete noob again. Oh, well.
Thanks,
René
^ permalink raw reply
* Re: Git describe question
From: Mark Burton @ 2008-07-15 6:40 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jean-Luc Herren, git
In-Reply-To: <7v1w1wf42n.fsf@gitster.siamese.dyndns.org>
On Mon, 14 Jul 2008 16:17:52 -0700
Junio C Hamano <gitster@pobox.com> wrote:
> Jean-Luc Herren <jlh@gmx.ch> writes:
>
> > Mark Burton wrote:
> > ...
> > I don't think people usually say "tag X is on branch Y", excepted
> > maybe if Y has never been merged anywhere. Specifically, nobody
> > would say v1.5.6.3 is *on* branch master. But it's part of its
> > history. v1.5.6.3 is *on* maint, at best.
>
> Actually I am somewhat sympathetic to Mark here. Probably what he wants
> is to describe 10ce020 as v1.5.6-390-10ce020.
>
> While that probably is doable by using the first-parent-only traversal, I
> do not think it is such a good idea. It is not how branches in git are
> designed to work. As Merlyn always says in #git at freenode, a branch is
> an illusion, and it is especially true in the presense of fast-forward
> merge (aka the upstream maintainer asking a subsystem lieutenant to do a
> merge for him).
Hi Guys,
Many thanks for the replies which make sense.
I guess the thing that prompted my original email was the fact that
when you look at output from gitk, the "distance" between 1.5.6.3 and
HEAD is only a few commits and that led me to expect that the output of
describe would show a smaller number than it does. I now understand why
it is how it is.
However, the git describe manual page does mention branches:
>With something like git.git current tree, I get:
>
> [torvalds@g5 git]$ git-describe parent
> v1.0.4-14-g2414721
>i.e. the current head of my "parent" branch is based on v1.0.4,
>but since it has a handful commits on top of that, describe has added
>the number of additional commits ("14") and an abbreviated object name
>for the commit itself ("2414721") at the end.
If you're a git newbie (like me) those words suggest that the commits
are "on" branch parent and you could measure the direct distance
between them by following the branch. Perhaps, the manual page should
contain a sentence similar to this:
Branches have no influence on the git describe long format output
which is derived only from the number of commits in the chain from the
described tag to the committish.
Cheers,
Mark
^ permalink raw reply
* Re: [PATCH v2] bash: Teach the bash completion about 'git send-email'
From: Shawn O. Pearce @ 2008-07-15 6:31 UTC (permalink / raw)
To: Teemu Likonen; +Cc: git, gitster
In-Reply-To: <20080715063034.GA3316@mithlond.arda.local>
Teemu Likonen <tlikonen@iki.fi> wrote:
> Add the following long options to be completed with 'git send-email':
>
> --bcc --cc --cc-cmd --chain-reply-to --compose --dry-run
> --envelope-sender --from --identity --in-reply-to
> --no-chain-reply-to --no-signed-off-by-cc --no-suppress-from
> --no-thread --quiet --signed-off-by-cc --smtp-pass --smtp-server
> --smtp-server-port --smtp-ssl --smtp-user --subject --suppress-cc
> --suppress-from --thread --to
>
> Short ones like --to and --cc are not usable for actual completion
> because of the shortness itself and because there are longer ones which
> start with same letters (--thread, --compose). It's still useful to have
> these shorter options _listed_ when user presses TAB key after typing
> two dashes. It gives user an idea what options are available (and --to
> and --cc are probably the most commonly used).
>
> Signed-off-by: Teemu Likonen <tlikonen@iki.fi>
Acked-by: Shawn O. Pearce <spearce@spearce.org>
Thanks for the quick reply cleaning up the minor details. :)
> Shawn O. Pearce wrote (2008-07-15 04:38 +0000):
>
> > Don't use __git_complete_file here. As far as I remember,
> > git-send-email does not accept "origin/maint:some.patch" as an email
> > to extract from Git prior to sending. It looks for files in the local
> > filesystem. So you want standard bash completion for anything not
> > starting with --.
> >
> > Just use COMPREPLY=() at the end. See _git_am for an example.
>
> Done. And thanks.
>
> > > +complete -o default -o nospace -F _git_send_email git-send-email
>
> > Hmm. With dash form commands gone in 1.6 we should remove these.
> >
> > But I suspect this completion patch could be shipped in the next 1.5.6
> > maint release as its really quite trivial. Junio, any comment on
> > that?
>
> This is a for-1.6 version so the completion for dashed command
> (git-send-email) is dropped. I see Shawn already sent a patch which
> drops all those.
>
>
>
> contrib/completion/git-completion.bash | 19 +++++++++++++++++++
> 1 files changed, 19 insertions(+), 0 deletions(-)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index d268e6f..48ebbf7 100755
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -905,6 +905,24 @@ _git_rebase ()
> __gitcomp "$(__git_refs)"
> }
>
> +_git_send_email ()
> +{
> + local cur="${COMP_WORDS[COMP_CWORD]}"
> + case "$cur" in
> + --*)
> + __gitcomp "--bcc --cc --cc-cmd --chain-reply-to --compose
> + --dry-run --envelope-sender --from --identity
> + --in-reply-to --no-chain-reply-to --no-signed-off-by-cc
> + --no-suppress-from --no-thread --quiet
> + --signed-off-by-cc --smtp-pass --smtp-server
> + --smtp-server-port --smtp-ssl --smtp-user --subject
> + --suppress-cc --suppress-from --thread --to"
> + return
> + ;;
> + esac
> + COMPREPLY=()
> +}
> +
> _git_config ()
> {
> local cur="${COMP_WORDS[COMP_CWORD]}"
> @@ -1376,6 +1394,7 @@ _git ()
> rebase) _git_rebase ;;
> remote) _git_remote ;;
> reset) _git_reset ;;
> + send-email) _git_send_email ;;
> shortlog) _git_shortlog ;;
> show) _git_show ;;
> show-branch) _git_log ;;
> --
> 1.5.6.3.316.g01fc
>
--
Shawn.
^ permalink raw reply
* [PATCH v2] bash: Teach the bash completion about 'git send-email'
From: Teemu Likonen @ 2008-07-15 6:30 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git, gitster
In-Reply-To: <20080715043839.GE2432@spearce.org>
Add the following long options to be completed with 'git send-email':
--bcc --cc --cc-cmd --chain-reply-to --compose --dry-run
--envelope-sender --from --identity --in-reply-to
--no-chain-reply-to --no-signed-off-by-cc --no-suppress-from
--no-thread --quiet --signed-off-by-cc --smtp-pass --smtp-server
--smtp-server-port --smtp-ssl --smtp-user --subject --suppress-cc
--suppress-from --thread --to
Short ones like --to and --cc are not usable for actual completion
because of the shortness itself and because there are longer ones which
start with same letters (--thread, --compose). It's still useful to have
these shorter options _listed_ when user presses TAB key after typing
two dashes. It gives user an idea what options are available (and --to
and --cc are probably the most commonly used).
Signed-off-by: Teemu Likonen <tlikonen@iki.fi>
---
Shawn O. Pearce wrote (2008-07-15 04:38 +0000):
> Don't use __git_complete_file here. As far as I remember,
> git-send-email does not accept "origin/maint:some.patch" as an email
> to extract from Git prior to sending. It looks for files in the local
> filesystem. So you want standard bash completion for anything not
> starting with --.
>
> Just use COMPREPLY=() at the end. See _git_am for an example.
Done. And thanks.
> > +complete -o default -o nospace -F _git_send_email git-send-email
> Hmm. With dash form commands gone in 1.6 we should remove these.
>
> But I suspect this completion patch could be shipped in the next 1.5.6
> maint release as its really quite trivial. Junio, any comment on
> that?
This is a for-1.6 version so the completion for dashed command
(git-send-email) is dropped. I see Shawn already sent a patch which
drops all those.
contrib/completion/git-completion.bash | 19 +++++++++++++++++++
1 files changed, 19 insertions(+), 0 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index d268e6f..48ebbf7 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -905,6 +905,24 @@ _git_rebase ()
__gitcomp "$(__git_refs)"
}
+_git_send_email ()
+{
+ local cur="${COMP_WORDS[COMP_CWORD]}"
+ case "$cur" in
+ --*)
+ __gitcomp "--bcc --cc --cc-cmd --chain-reply-to --compose
+ --dry-run --envelope-sender --from --identity
+ --in-reply-to --no-chain-reply-to --no-signed-off-by-cc
+ --no-suppress-from --no-thread --quiet
+ --signed-off-by-cc --smtp-pass --smtp-server
+ --smtp-server-port --smtp-ssl --smtp-user --subject
+ --suppress-cc --suppress-from --thread --to"
+ return
+ ;;
+ esac
+ COMPREPLY=()
+}
+
_git_config ()
{
local cur="${COMP_WORDS[COMP_CWORD]}"
@@ -1376,6 +1394,7 @@ _git ()
rebase) _git_rebase ;;
remote) _git_remote ;;
reset) _git_reset ;;
+ send-email) _git_send_email ;;
shortlog) _git_shortlog ;;
show) _git_show ;;
show-branch) _git_log ;;
--
1.5.6.3.316.g01fc
^ permalink raw reply related
* Increased maintainer work during Summer of Code
From: Shawn O. Pearce @ 2008-07-15 6:04 UTC (permalink / raw)
To: Junio C Hamano
Cc: Christian Couder, Daniel Barkalow, Johannes Schindelin, J.H.,
Sam Vilain, git
In reading through the submitted mid-term surveys from students I
found this excellent answer from Stephan Beyer:
Q: What advice would you give to future would-be Summer of Code
mentoring organizations?
> I am wondering about one thing in git which is perhaps true for
> a lot of other mentoring organizations: There is one person, the
> maintainer, that is the final authority of deciding what patches
> go into the repositories and what not. (This is ok and good and if
> somebody disagrees a lot with such decisions he or she can make up
> a fork repo without problems.) But the point is that the maintainer
> has to review and add a lot of patches each day and _during GSoC
> this is even a lot more_. I sometimes wonder how the maintainer is
> able to handle that much work ;-) So my "advice" could be to think
> about the "problem" of more contributions during GSoC, and if it
> is useful to have co-maintainers or something. :)
Junio, I know you have been working extra hard lately with the
merge of builtin merge, and now gitweb and the sequencer are also
being looked at in much greater detail.
What can we do to smooth out this workload better? Its awesome
that we were so fortunate to get these great students this year,
and have so much contributed in so little time, but we also do
not want to see maintainer burn-out. We also want to avoid a
huge backlog of patches.
I don't think we really ever talked about how to help Junio work
through these large contributions that are coming his way.
--
Shawn.
^ permalink raw reply
* [PATCH] bash completion: Remove dashed command completion support
From: Shawn O. Pearce @ 2008-07-15 5:58 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Since only 'git' and 'gitk' are in the user's $PATH now we do not
expect users to need completion support for git-fetch, and expect
they will instead rely upon the completion support for 'git fetch'.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
contrib/completion/git-completion.bash | 53 --------------------------------
1 files changed, 0 insertions(+), 53 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 5844804..0e493a4 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1440,64 +1440,11 @@ _gitk ()
complete -o default -o nospace -F _git git
complete -o default -o nospace -F _gitk gitk
-complete -o default -o nospace -F _git_am git-am
-complete -o default -o nospace -F _git_apply git-apply
-complete -o default -o nospace -F _git_bisect git-bisect
-complete -o default -o nospace -F _git_branch git-branch
-complete -o default -o nospace -F _git_bundle git-bundle
-complete -o default -o nospace -F _git_checkout git-checkout
-complete -o default -o nospace -F _git_cherry git-cherry
-complete -o default -o nospace -F _git_cherry_pick git-cherry-pick
-complete -o default -o nospace -F _git_commit git-commit
-complete -o default -o nospace -F _git_describe git-describe
-complete -o default -o nospace -F _git_diff git-diff
-complete -o default -o nospace -F _git_fetch git-fetch
-complete -o default -o nospace -F _git_format_patch git-format-patch
-complete -o default -o nospace -F _git_gc git-gc
-complete -o default -o nospace -F _git_log git-log
-complete -o default -o nospace -F _git_ls_remote git-ls-remote
-complete -o default -o nospace -F _git_ls_tree git-ls-tree
-complete -o default -o nospace -F _git_merge git-merge
-complete -o default -o nospace -F _git_merge_base git-merge-base
-complete -o default -o nospace -F _git_name_rev git-name-rev
-complete -o default -o nospace -F _git_pull git-pull
-complete -o default -o nospace -F _git_push git-push
-complete -o default -o nospace -F _git_rebase git-rebase
-complete -o default -o nospace -F _git_config git-config
-complete -o default -o nospace -F _git_remote git-remote
-complete -o default -o nospace -F _git_reset git-reset
-complete -o default -o nospace -F _git_shortlog git-shortlog
-complete -o default -o nospace -F _git_show git-show
-complete -o default -o nospace -F _git_stash git-stash
-complete -o default -o nospace -F _git_submodule git-submodule
-complete -o default -o nospace -F _git_svn git-svn
-complete -o default -o nospace -F _git_log git-show-branch
-complete -o default -o nospace -F _git_tag git-tag
-complete -o default -o nospace -F _git_log git-whatchanged
# The following are necessary only for Cygwin, and only are needed
# when the user has tab-completed the executable name and consequently
# included the '.exe' suffix.
#
if [ Cygwin = "$(uname -o 2>/dev/null)" ]; then
-complete -o default -o nospace -F _git_add git-add.exe
-complete -o default -o nospace -F _git_apply git-apply.exe
complete -o default -o nospace -F _git git.exe
-complete -o default -o nospace -F _git_branch git-branch.exe
-complete -o default -o nospace -F _git_bundle git-bundle.exe
-complete -o default -o nospace -F _git_cherry git-cherry.exe
-complete -o default -o nospace -F _git_describe git-describe.exe
-complete -o default -o nospace -F _git_diff git-diff.exe
-complete -o default -o nospace -F _git_format_patch git-format-patch.exe
-complete -o default -o nospace -F _git_log git-log.exe
-complete -o default -o nospace -F _git_ls_tree git-ls-tree.exe
-complete -o default -o nospace -F _git_merge_base git-merge-base.exe
-complete -o default -o nospace -F _git_name_rev git-name-rev.exe
-complete -o default -o nospace -F _git_push git-push.exe
-complete -o default -o nospace -F _git_config git-config
-complete -o default -o nospace -F _git_shortlog git-shortlog.exe
-complete -o default -o nospace -F _git_show git-show.exe
-complete -o default -o nospace -F _git_log git-show-branch.exe
-complete -o default -o nospace -F _git_tag git-tag.exe
-complete -o default -o nospace -F _git_log git-whatchanged.exe
fi
--
1.5.6.2.393.g45096
^ permalink raw reply related
* [PATCH] bash completion: Resolve git show ref:path<tab> losing ref: portion
From: Shawn O. Pearce @ 2008-07-15 5:52 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20080715053907.GA3254@spearce.org>
Linus reported that the bash completion for git show often dropped
the ref portion of the argument (stuff before the :) when trying
to complete a file name of a file in another branch or tag.
Björn Steinbrink tracked it down to the gvfs completion script
which comes standard on many Fedora Core based systems. That is
removing : from COMP_WORDBREAKS, making readline treat the entire
argument (including the ref) as the name that must be completed.
When the git completion routines supplied a completion of just the
filename, readline replaced everything.
Since Git users often need to use "ref:path" or "ref:ref" sort of
arguments, and expect completion support on both sides of the :
we really want the : in COMP_WORDBREAKS to provide a good user
experience. This is also the default that ships with bash as it
can be useful in other contexts, such as rcp/scp.
We now try to add : back to COMP_WORDBREAKS if it has been removed
by a script that loaded before us. However if this doesn't work
(as the : is stripped after we load) we fallback in the completion
routines to include "ref:" as part of the prefix for completions,
allowing readine to fully insert the argument the user wanted.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
Second time's the charm? The prior version had a mangled commit
message, due to character encoding stupidity between my keyboard
and my chair.
contrib/completion/git-completion.bash | 27 +++++++++++++++++++++++++--
1 files changed, 25 insertions(+), 2 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index c03230a..5844804 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -45,6 +45,11 @@
# git@vger.kernel.org
#
+case "$COMP_WORDBREAKS" in
+*:*) : great ;;
+*) COMP_WORDBREAKS="$COMP_WORDBREAKS:"
+esac
+
__gitdir ()
{
if [ -z "$1" ]; then
@@ -294,6 +299,12 @@ __git_complete_file ()
ls="$ref"
;;
esac
+
+ case "$COMP_WORDBREAKS" in
+ *:*) : great ;;
+ *) pfx="$ref:$pfx" ;;
+ esac
+
local IFS=$'\n'
COMPREPLY=($(compgen -P "$pfx" \
-W "$(git --git-dir="$(__gitdir)" ls-tree "$ls" \
@@ -700,7 +711,12 @@ _git_fetch ()
*)
case "$cur" in
*:*)
- __gitcomp "$(__git_refs)" "" "${cur#*:}"
+ local pfx=""
+ case "$COMP_WORDBREAKS" in
+ *:*) : great ;;
+ *) pfx="${cur%%:*}:" ;;
+ esac
+ __gitcomp "$(__git_refs)" "$pfx" "${cur#*:}"
;;
*)
local remote
@@ -876,7 +892,14 @@ _git_push ()
git-push) remote="${COMP_WORDS[1]}" ;;
git) remote="${COMP_WORDS[2]}" ;;
esac
- __gitcomp "$(__git_refs "$remote")" "" "${cur#*:}"
+
+ local pfx=""
+ case "$COMP_WORDBREAKS" in
+ *:*) : great ;;
+ *) pfx="${cur%%:*}:" ;;
+ esac
+
+ __gitcomp "$(__git_refs "$remote")" "$pfx" "${cur#*:}"
;;
+*)
__gitcomp "$(__git_refs)" + "${cur#+}"
--
1.5.6.2.393.g45096
^ permalink raw reply related
* [PATCH] bash completion: Resolve git show ref:path<tab> losing ref: portion
From: Shawn O. Pearce @ 2008-07-15 5:39 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Linus Torvalds
Linus reported that the bash completion for git show often dropped
the ref portion of the argument (stuff before the :) when trying
to complete a file name of a file in another branch or tag.
Bj?rn Steinbrink tracked it down to the gvfs completion script
which comes standard on many Fedora Core based systems. That is
removing : from COMP_WORDBREAKS, making readline treat the entire
argument (including the ref) as the name that must be completed.
When the git completion routines supplied a completion of just
the filename, readline replaced everything.
Since Git users often need to use "ref:path" or "ref:ref" sort of
arguments, and expect completion support on both sides of the :
we really want the : in COMP_WORDBREAKS to provide a good user
experience. This is also the default that ships with bash as it
can be useful in other contexts, such as rcp/scp.
We now try to add : back to COMP_WORDBREAKS if it has been removed
by a script that loaded before us. However if this doesn't work
(as the : is stripped after we load) we fallback in the completion
routines to include "ref:" as part of the prefix for completions,
allowing readine to fully insert the argument the user wanted.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
contrib/completion/git-completion.bash | 27 +++++++++++++++++++++++++--
1 files changed, 25 insertions(+), 2 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index c03230a..5844804 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -45,6 +45,11 @@
# git@vger.kernel.org
#
+case "$COMP_WORDBREAKS" in
+*:*) : great ;;
+*) COMP_WORDBREAKS="$COMP_WORDBREAKS:"
+esac
+
__gitdir ()
{
if [ -z "$1" ]; then
@@ -294,6 +299,12 @@ __git_complete_file ()
ls="$ref"
;;
esac
+
+ case "$COMP_WORDBREAKS" in
+ *:*) : great ;;
+ *) pfx="$ref:$pfx" ;;
+ esac
+
local IFS=$'\n'
COMPREPLY=($(compgen -P "$pfx" \
-W "$(git --git-dir="$(__gitdir)" ls-tree "$ls" \
@@ -700,7 +711,12 @@ _git_fetch ()
*)
case "$cur" in
*:*)
- __gitcomp "$(__git_refs)" "" "${cur#*:}"
+ local pfx=""
+ case "$COMP_WORDBREAKS" in
+ *:*) : great ;;
+ *) pfx="${cur%%:*}:" ;;
+ esac
+ __gitcomp "$(__git_refs)" "$pfx" "${cur#*:}"
;;
*)
local remote
@@ -876,7 +892,14 @@ _git_push ()
git-push) remote="${COMP_WORDS[1]}" ;;
git) remote="${COMP_WORDS[2]}" ;;
esac
- __gitcomp "$(__git_refs "$remote")" "" "${cur#*:}"
+
+ local pfx=""
+ case "$COMP_WORDBREAKS" in
+ *:*) : great ;;
+ *) pfx="${cur%%:*}:" ;;
+ esac
+
+ __gitcomp "$(__git_refs "$remote")" "$pfx" "${cur#*:}"
;;
+*)
__gitcomp "$(__git_refs)" + "${cur#+}"
--
1.5.6.2.393.g45096
^ permalink raw reply related
* Re: [PATCH 1/2] restore legacy behavior for read_sha1_file()
From: Nicolas Pitre @ 2008-07-15 5:12 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <alpine.LFD.1.10.0807142134450.12484@xanadu.home>
On Mon, 14 Jul 2008, Nicolas Pitre wrote:
> Since commit 8eca0b47ff1598a6d163df9358c0e0c9bd92d4c8, it is possible
> for read_sha1_file() to return NULL even with existing objects when they
> are corrupted. Previously a corrupted object would have terminated the
> program immediately, effectively making read_sha1_file() return NULL
> only when specified object is not found.
>
> Let's restore this behavior for all users of read_sha1_file() and
> provide a separate function with the ability to not terminate when
> bad objects are encountered.
>
> Signed-off-by: Nicolas Pitre <nico@cam.org>
Grrrr. Forgot to 'git add' one file. So this goes with the same
commit:
diff --git a/cache.h b/cache.h
index 4a8b125..bc52af6 100644
--- a/cache.h
+++ b/cache.h
@@ -540,6 +540,9 @@ extern int write_sha1_file(void *buf, unsigned long len, const char *type, unsig
extern int pretend_sha1_file(void *, unsigned long, enum object_type, unsigned char *);
extern int force_object_loose(const unsigned char *sha1, time_t mtime);
+/* just like read_sha1_file(), but non fatal in presence of bad objects */
+extern void *read_object(const unsigned char *sha1, enum object_type *type, unsigned long *size);
+
extern int check_sha1_signature(const unsigned char *sha1, void *buf, unsigned long size, const char *type);
extern int move_temp_to_file(const char *tmpfile, const char *filename);
^ permalink raw reply related
* unpack_entry (was: [BUG] commit walk machinery is dangerous !)
From: Shawn O. Pearce @ 2008-07-15 5:10 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nicolas Pitre, git
In-Reply-To: <7vod50dote.fsf@gitster.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> wrote:
> This is unrelated to the issue at hand, but I also notice that there are
> few callsites outside sha1_file.c that bypasses cache_or_unpack_entry()
> and call unpack_entry() directly. I wonder if they should be using the
> cached version, making unpack_entry() static...
There are two such callsites that I found:
$ git grep -n --cached unpack_entry
fast-import.c:1201: return unpack_entry(p, oe->offset, &type, sizep);
pack-check.c:114: data = unpack_entry(p, entries[i].offset, &type,
Now the one in fast-import.c could be using the cached version.
This is simply fast-import trying to reuse sha1_file.c for reading
a previously written object to the pack.
Stuffing the item into the cache is perhaps pointless here as gfi
does its own sort of caching and only goes through this code path
when that caching has tossed the least-recently-used data and its
suddenly needed again.
So yea, it could be using the caching form, but it would maybe be
doing more work (and using more memory) than it really needs.
In pack-check.c we are looping through the objects in the order they
appear in the index so we can unpack them and verify each object's
SHA-1 signature. Please note this is perhaps the slowest way to
enumerate through the pack and is why you can clone a repository
over git:// faster than you can run `fsck --full`.
If you really want to verify every single object's SHA-1, run the
damn pack through index-pack and compare the new index to the old
index (hint they should be identical, bit-for-bit).
So again, in this case the caching may only cause us to do more
work (and use more memory) than we need as we are slamming the
delta base cache with all sorts of unrelated base objects. It
probably has a low hit ratio anyway during this loop. :-|
--
Shawn.
^ permalink raw reply
* Re: [PATCH v2] index-pack: Honor core.deltaBaseCacheLimit when resolving deltas
From: Nicolas Pitre @ 2008-07-15 5:05 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Junio C Hamano, git, Stephan Hennig, Andreas Ericsson
In-Reply-To: <20080715044534.GA2794@spearce.org>
On Tue, 15 Jul 2008, Shawn O. Pearce wrote:
> If we are trying to resolve deltas for a long delta chain composed
> of multi-megabyte objects we can easily run into requiring 500M+
> of memory to hold each object in the chain on the call stack while
> we recurse into the dependent objects and resolve them.
>
> We now use a simple delta cache that discards objects near the
> bottom of the call stack first, as they are the most least recently
> used objects in this current delta chain. If we recurse out of a
> chain we may find the base object is no longer available, as it was
> free'd to keep memory under the deltaBaseCacheLimit. In such cases
> we must unpack the base object again, which will require recursing
> back to the root of the top of the delta chain as we released that
> root first.
>
> The astute reader will probably realize that we can still exceed
> the delta base cache limit, but this happens only if the most
> recent base plus the delta plus the inflated dependent sum up to
> more than the base cache limit. Due to the way patch_delta is
> currently implemented we cannot operate in less memory anyway.
You may also set the limit lower than the size of the largest object
which is going to bust it too. Same goes for the cache in sha1_file.c.
There is simply a limit to how tight we can sanely make memory usage.
> Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
ACK
> ---
>
> Version 2 plugs the case Nico noticed, where the patch was causing
> the exact behavior it was trying to prevent while recovering from
> what it did to avoid the excessive memory usage in the first place.
>
> The change was in get_base_data() where we now unpack the delta
> after we have unpacked the base. Nico and I both missed that we
> must also bump base_cache_used when we restore the base, and we
> must also prune the bases in case this base has caused us to go
> over the limit.
>
> :-)
>
> index-pack.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++--
> 1 files changed, 46 insertions(+), 2 deletions(-)
>
> diff --git a/index-pack.c b/index-pack.c
> index 7239e89..b4ec736 100644
> --- a/index-pack.c
> +++ b/index-pack.c
> @@ -52,6 +52,7 @@ struct delta_entry
> static struct object_entry *objects;
> static struct delta_entry *deltas;
> static struct base_data *base_cache;
> +static size_t base_cache_used;
> static int nr_objects;
> static int nr_deltas;
> static int nr_resolved_deltas;
> @@ -220,6 +221,20 @@ static void bad_object(unsigned long offset, const char *format, ...)
> die("pack has bad object at offset %lu: %s", offset, buf);
> }
>
> +static void prune_base_data(struct base_data *retain)
> +{
> + struct base_data *b = base_cache;
> + for (b = base_cache;
> + base_cache_used > delta_base_cache_limit && b;
> + b = b->child) {
> + if (b->data && b != retain) {
> + free(b->data);
> + b->data = NULL;
> + base_cache_used -= b->size;
> + }
> + }
> +}
> +
> static void link_base_data(struct base_data *base, struct base_data *c)
> {
> if (base)
> @@ -229,6 +244,8 @@ static void link_base_data(struct base_data *base, struct base_data *c)
>
> c->base = base;
> c->child = NULL;
> + base_cache_used += c->size;
> + prune_base_data(c);
> }
>
> static void unlink_base_data(struct base_data *c)
> @@ -238,7 +255,10 @@ static void unlink_base_data(struct base_data *c)
> base->child = NULL;
> else
> base_cache = NULL;
> - free(c->data);
> + if (c->data) {
> + free(c->data);
> + base_cache_used -= c->size;
> + }
> }
>
> static void *unpack_entry_data(unsigned long offset, unsigned long size)
> @@ -456,6 +476,30 @@ static void sha1_object(const void *data, unsigned long size,
> }
> }
>
> +static void *get_base_data(struct base_data *c)
> +{
> + if (!c->data) {
> + struct object_entry *obj = c->obj;
> +
> + if (obj->type == OBJ_REF_DELTA || obj->type == OBJ_OFS_DELTA) {
> + void *base = get_base_data(c->base);
> + void *raw = get_data_from_pack(obj);
> + c->data = patch_delta(
> + base, c->base->size,
> + raw, obj->size,
> + &c->size);
> + free(raw);
> + if (!c->data)
> + bad_object(obj->idx.offset, "failed to apply delta");
> + } else
> + c->data = get_data_from_pack(obj);
> +
> + base_cache_used += c->size;
> + prune_base_data(c);
> + }
> + return c->data;
> +}
> +
> static void resolve_delta(struct object_entry *delta_obj,
> struct base_data *base_obj, enum object_type type)
> {
> @@ -468,7 +512,7 @@ static void resolve_delta(struct object_entry *delta_obj,
> delta_obj->real_type = type;
> delta_data = get_data_from_pack(delta_obj);
> delta_size = delta_obj->size;
> - result.data = patch_delta(base_obj->data, base_obj->size,
> + result.data = patch_delta(get_base_data(base_obj), base_obj->size,
> delta_data, delta_size,
> &result.size);
> free(delta_data);
> --
> 1.5.6.2.393.g45096
>
Nicolas
^ permalink raw reply
* Re: [PATCH] bash: Add long option completion for 'git send-email'
From: Shawn O. Pearce @ 2008-07-15 4:58 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Teemu Likonen, git
In-Reply-To: <7vmykjda5i.fsf@gitster.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> wrote:
> "Shawn O. Pearce" <spearce@spearce.org> writes:
>
> > Hmm. With dash form commands gone in 1.6 we should remove these.
> >
> > But I suspect this completion patch could be shipped in the next
> > 1.5.6 maint release as its really quite trivial. Junio, any comment
> > on that?
>
> Yeah, it is trivial but does it deserve "bugfix -- we need to deliver this
> change to the end users, otherwise they will suffer" label? Probably not.
Not really, but we have shipped new completion functionality (missing
--graph option to git-log) in maint releases in recent history.
If it was me maintaining git.git, I'd probably toss this into maint.
But I think I have a more aggressive personality than you, and am
more willing to take a risk. Users frequently don't like such risk.
I defer to your wise judgement. Given prior statements about what
goes where, you are being consistent to say this should go in 1.6.
In which case I suggest Teemu should drop that last hunk as we
should really drop that entire block of code from the script.
--
Shawn.
^ permalink raw reply
* Re: [PATCH] bash: Add long option completion for 'git send-email'
From: Junio C Hamano @ 2008-07-15 4:49 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Teemu Likonen, git
In-Reply-To: <20080715043839.GE2432@spearce.org>
"Shawn O. Pearce" <spearce@spearce.org> writes:
> Hmm. With dash form commands gone in 1.6 we should remove these.
>
> But I suspect this completion patch could be shipped in the next
> 1.5.6 maint release as its really quite trivial. Junio, any comment
> on that?
Yeah, it is trivial but does it deserve "bugfix -- we need to deliver this
change to the end users, otherwise they will suffer" label? Probably not.
^ permalink raw reply
* [PATCH v2] index-pack: Honor core.deltaBaseCacheLimit when resolving deltas
From: Shawn O. Pearce @ 2008-07-15 4:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Nicolas Pitre, Stephan Hennig, Andreas Ericsson
In-Reply-To: <20080715031800.GD1700@spearce.org>
If we are trying to resolve deltas for a long delta chain composed
of multi-megabyte objects we can easily run into requiring 500M+
of memory to hold each object in the chain on the call stack while
we recurse into the dependent objects and resolve them.
We now use a simple delta cache that discards objects near the
bottom of the call stack first, as they are the most least recently
used objects in this current delta chain. If we recurse out of a
chain we may find the base object is no longer available, as it was
free'd to keep memory under the deltaBaseCacheLimit. In such cases
we must unpack the base object again, which will require recursing
back to the root of the top of the delta chain as we released that
root first.
The astute reader will probably realize that we can still exceed
the delta base cache limit, but this happens only if the most
recent base plus the delta plus the inflated dependent sum up to
more than the base cache limit. Due to the way patch_delta is
currently implemented we cannot operate in less memory anyway.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
Version 2 plugs the case Nico noticed, where the patch was causing
the exact behavior it was trying to prevent while recovering from
what it did to avoid the excessive memory usage in the first place.
The change was in get_base_data() where we now unpack the delta
after we have unpacked the base. Nico and I both missed that we
must also bump base_cache_used when we restore the base, and we
must also prune the bases in case this base has caused us to go
over the limit.
:-)
index-pack.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++--
1 files changed, 46 insertions(+), 2 deletions(-)
diff --git a/index-pack.c b/index-pack.c
index 7239e89..b4ec736 100644
--- a/index-pack.c
+++ b/index-pack.c
@@ -52,6 +52,7 @@ struct delta_entry
static struct object_entry *objects;
static struct delta_entry *deltas;
static struct base_data *base_cache;
+static size_t base_cache_used;
static int nr_objects;
static int nr_deltas;
static int nr_resolved_deltas;
@@ -220,6 +221,20 @@ static void bad_object(unsigned long offset, const char *format, ...)
die("pack has bad object at offset %lu: %s", offset, buf);
}
+static void prune_base_data(struct base_data *retain)
+{
+ struct base_data *b = base_cache;
+ for (b = base_cache;
+ base_cache_used > delta_base_cache_limit && b;
+ b = b->child) {
+ if (b->data && b != retain) {
+ free(b->data);
+ b->data = NULL;
+ base_cache_used -= b->size;
+ }
+ }
+}
+
static void link_base_data(struct base_data *base, struct base_data *c)
{
if (base)
@@ -229,6 +244,8 @@ static void link_base_data(struct base_data *base, struct base_data *c)
c->base = base;
c->child = NULL;
+ base_cache_used += c->size;
+ prune_base_data(c);
}
static void unlink_base_data(struct base_data *c)
@@ -238,7 +255,10 @@ static void unlink_base_data(struct base_data *c)
base->child = NULL;
else
base_cache = NULL;
- free(c->data);
+ if (c->data) {
+ free(c->data);
+ base_cache_used -= c->size;
+ }
}
static void *unpack_entry_data(unsigned long offset, unsigned long size)
@@ -456,6 +476,30 @@ static void sha1_object(const void *data, unsigned long size,
}
}
+static void *get_base_data(struct base_data *c)
+{
+ if (!c->data) {
+ struct object_entry *obj = c->obj;
+
+ if (obj->type == OBJ_REF_DELTA || obj->type == OBJ_OFS_DELTA) {
+ void *base = get_base_data(c->base);
+ void *raw = get_data_from_pack(obj);
+ c->data = patch_delta(
+ base, c->base->size,
+ raw, obj->size,
+ &c->size);
+ free(raw);
+ if (!c->data)
+ bad_object(obj->idx.offset, "failed to apply delta");
+ } else
+ c->data = get_data_from_pack(obj);
+
+ base_cache_used += c->size;
+ prune_base_data(c);
+ }
+ return c->data;
+}
+
static void resolve_delta(struct object_entry *delta_obj,
struct base_data *base_obj, enum object_type type)
{
@@ -468,7 +512,7 @@ static void resolve_delta(struct object_entry *delta_obj,
delta_obj->real_type = type;
delta_data = get_data_from_pack(delta_obj);
delta_size = delta_obj->size;
- result.data = patch_delta(base_obj->data, base_obj->size,
+ result.data = patch_delta(get_base_data(base_obj), base_obj->size,
delta_data, delta_size,
&result.size);
free(delta_data);
--
1.5.6.2.393.g45096
^ permalink raw reply related
* Re: [PATCH] bash: Add long option completion for 'git send-email'
From: Shawn O. Pearce @ 2008-07-15 4:38 UTC (permalink / raw)
To: Teemu Likonen; +Cc: git, gitster
In-Reply-To: <1216023662-9109-1-git-send-email-tlikonen@iki.fi>
Teemu Likonen <tlikonen@iki.fi> wrote:
> Add the following long options to be completed with 'git send-email':
...
> Short ones like --to and --cc are not usable for actual completion
I agree, these are worth including.
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index d268e6f..b15f3a9 100755
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -905,6 +905,24 @@ _git_rebase ()
> __gitcomp "$(__git_refs)"
> }
>
> +_git_send_email ()
> +{
> + local cur="${COMP_WORDS[COMP_CWORD]}"
> + case "$cur" in
> + --*)
> + __gitcomp "--bcc --cc --cc-cmd --chain-reply-to --compose
> + --dry-run --envelope-sender --from --identity
> + --in-reply-to --no-chain-reply-to --no-signed-off-by-cc
> + --no-suppress-from --no-thread --quiet
> + --signed-off-by-cc --smtp-pass --smtp-server
> + --smtp-server-port --smtp-ssl --smtp-user --subject
> + --suppress-cc --suppress-from --thread --to"
> + return
> + ;;
> + esac
> + __git_complete_file
Don't use __git_complete_file here. As far as I remember,
git-send-email does not accept "origin/maint:some.patch"
as an email to extract from Git prior to sending. It looks
for files in the local filesystem. So you want standard bash
completion for anything not starting with --.
Just use COMPREPLY=() at the end. See _git_am for an example.
> @@ -1435,6 +1454,7 @@ complete -o default -o nospace -F _git_rebase git-rebase
> complete -o default -o nospace -F _git_config git-config
> complete -o default -o nospace -F _git_remote git-remote
> complete -o default -o nospace -F _git_reset git-reset
> +complete -o default -o nospace -F _git_send_email git-send-email
> complete -o default -o nospace -F _git_shortlog git-shortlog
> complete -o default -o nospace -F _git_show git-show
> complete -o default -o nospace -F _git_stash git-stash
Hmm. With dash form commands gone in 1.6 we should remove these.
But I suspect this completion patch could be shipped in the next
1.5.6 maint release as its really quite trivial. Junio, any comment
on that?
--
Shawn.
^ permalink raw reply
* Re: [PATCH] bash completion: Fix the . -> .. revision range completion
From: Shawn O. Pearce @ 2008-07-15 4:25 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Junio C Hamano, Petr Baudis, git
In-Reply-To: <alpine.LFD.1.10.0807140741580.3305@woody.linux-foundation.org>
Linus Torvalds <torvalds@linux-foundation.org> wrote:
> On Mon, 14 Jul 2008, Shawn O. Pearce wrote:
> >
> > What is $COMP_WORDBREAKS set to in your shell? In mine it
> > appears to be:
> >
> > " \"'@><=;|&(:"
>
> Ahhah. Indeed. I don't have the ':'.
...
> Umm, if so, git should just set it in the completion script, no?
OK, so it turns out not having : in COMP_WORDBREAKS is a very common
case that we should somehow deal with, to aid our users.
I'm concerned about just setting COMP_WORDBREAKS back to the default
in the git completion script because then we get into an ordering
game with the profile scripts, don't we? If git completion sources
before the gvfs script we don't get our COMP_WORDBREAKS setting.
I think we may need to do two things.
If COMP_WORDBREAKS doesn't contain a :, try to reset it to include
one when the script is sourced. This may "fix" git completion but
make gvfs completion act differently, resulting in a thread on the
gvfs lists. ;-)
If COMP_WORDBREAKS doesn't contain : during a completion event than
we need to do what your original patch asked, which is to include
"$ref:" in the prefix, so the ref isn't lost.
At least we understand the problem now, finally. I'll try to write
up a patch for it tomorrow. Unfortunately packing to move has been
really sucking up my time lately.
--
Shawn.
^ permalink raw reply
* Re: [PATCH] git-svn: find-rev and rebase for SVN::Mirror repositories
From: Eric Wong @ 2008-07-15 4:25 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, João Abecasis
In-Reply-To: <7bf6f1d20807140828ve04bc16u96d0245209491b2e@mail.gmail.com>
João Abecasis <joao@abecasis.name> wrote:
> find-rev and rebase error out on svm because git-svn doesn't trace the
> original svn revision numbers back to git commits. The updated test
> case, included in the patch, shows the issue and passes with the rest of
> the patch applied.
>
> This fixes Git::SVN::find_by_url to find branches based on the
> svm:source URL, where useSvmProps is set. Also makes sure cmd_find_rev
> and working_head_info use the information they have to correctly track
> the source repository. This is enough to get find-rev and rebase
> working.
>
> Signed-off-by: João Abecasis <joao@abecasis.name>
Thanks João,
Acked-by: Eric Wong <normalperson@yhbt.net>
> ---
> git-svn.perl | 39 ++++++++++++++++++++++++++++++++++---
> t/t9110-git-svn-use-svm-props.sh | 9 ++++++++
> 2 files changed, 44 insertions(+), 4 deletions(-)
>
> diff --git a/git-svn.perl b/git-svn.perl
> index a366c89..0aa994a 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -537,13 +537,13 @@ sub cmd_find_rev {
> my $head = shift;
> $head ||= 'HEAD';
> my @refs;
> - my (undef, undef, undef, $gs) = working_head_info($head, \@refs);
> + my (undef, undef, $uuid, $gs) = working_head_info($head, \@refs);
> unless ($gs) {
> die "Unable to determine upstream SVN information from ",
> "$head history\n";
> }
> my $desired_revision = substr($revision_or_hash, 1);
> - $result = $gs->rev_map_get($desired_revision);
> + $result = $gs->rev_map_get($desired_revision, $uuid);
> } else {
> my (undef, $rev, undef) = cmt_metadata($revision_or_hash);
> $result = $rev;
> @@ -1162,7 +1162,7 @@ sub working_head_info {
> if (defined $url && defined $rev) {
> next if $max{$url} and $max{$url} < $rev;
> if (my $gs = Git::SVN->find_by_url($url)) {
> - my $c = $gs->rev_map_get($rev);
> + my $c = $gs->rev_map_get($rev, $uuid);
> if ($c && $c eq $hash) {
> close $fh; # break the pipe
> return ($url, $rev, $uuid, $gs);
> @@ -1416,11 +1416,17 @@ sub fetch_all {
>
> sub read_all_remotes {
> my $r = {};
> + my $use_svm_props = eval { command_oneline(qw/config --bool
> + svn.useSvmProps/) };
> + $use_svm_props = $use_svm_props eq 'true' if $use_svm_props;
> foreach (grep { s/^svn-remote\.// } command(qw/config -l/)) {
> if (m!^(.+)\.fetch=\s*(.*)\s*:\s*refs/remotes/(.+)\s*$!) {
> my ($remote, $local_ref, $remote_ref) = ($1, $2, $3);
> $local_ref =~ s{^/}{};
> $r->{$remote}->{fetch}->{$local_ref} = $remote_ref;
> + $r->{$remote}->{svm} = {} if $use_svm_props;
> + } elsif (m!^(.+)\.usesvmprops=\s*(.*)\s*$!) {
> + $r->{$1}->{svm} = {};
> } elsif (m!^(.+)\.url=\s*(.*)\s*$!) {
> $r->{$1}->{url} = $2;
> } elsif (m!^(.+)\.(branches|tags)=
> @@ -1437,6 +1443,23 @@ sub read_all_remotes {
> }
> }
> }
> +
> + map {
> + if (defined $r->{$_}->{svm}) {
> + my $svm;
> + eval {
> + my $section = "svn-remote.$_";
> + $svm = {
> + source => tmp_config('--get',
> + "$section.svm-source"),
> + replace => tmp_config('--get',
> + "$section.svm-replace"),
> + }
> + };
> + $r->{$_}->{svm} = $svm;
> + }
> + } keys %$r;
> +
> $r;
> }
>
> @@ -1563,13 +1586,21 @@ sub find_by_url { # repos_root and, path are optional
> }
> my $p = $path;
> my $rwr = rewrite_root({repo_id => $repo_id});
> + my $svm = $remotes->{$repo_id}->{svm}
> + if defined $remotes->{$repo_id}->{svm};
> unless (defined $p) {
> $p = $full_url;
> my $z = $u;
> + my $prefix = '';
> if ($rwr) {
> $z = $rwr;
> + } elsif (defined $svm) {
> + $z = $svm->{source};
> + $prefix = $svm->{replace};
> + $prefix =~ s#^\Q$u\E(?:/|$)##;
> + $prefix =~ s#/$##;
> }
> - $p =~ s#^\Q$z\E(?:/|$)## or next;
> + $p =~ s#^\Q$z\E(?:/|$)#$prefix# or next;
> }
> foreach my $f (keys %$fetch) {
> next if $f ne $p;
> diff --git a/t/t9110-git-svn-use-svm-props.sh b/t/t9110-git-svn-use-svm-props.sh
> index 047659f..04d2a65 100755
> --- a/t/t9110-git-svn-use-svm-props.sh
> +++ b/t/t9110-git-svn-use-svm-props.sh
> @@ -49,4 +49,13 @@ test_expect_success 'verify metadata for /dir' "
> grep '^git-svn-id: $dir_url@1 $uuid$'
> "
>
> +test_expect_success 'find commit based on SVN revision number' "
> + git-svn find-rev r12 |
> + grep `git rev-parse HEAD`
> + "
> +
> +test_expect_success 'empty rebase' "
> + git-svn rebase
> + "
> +
> test_done
> --
> 1.5.6
^ permalink raw reply
* Re: [PATCH 0/4] Honor core.deltaBaseCacheLimit during index-pack
From: Shawn O. Pearce @ 2008-07-15 4:19 UTC (permalink / raw)
To: Jakub Narebski
Cc: Johannes Schindelin, Nicolas Pitre, Junio C Hamano, git,
Stephan Hennig, Andreas Ericsson
In-Reply-To: <m31w1wu1hc.fsf@localhost.localdomain>
Jakub Narebski <jnareb@gmail.com> wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> > On Mon, 14 Jul 2008, Shawn O. Pearce wrote:
>
> > > The only other alternative I can come up with is to change pack-objects
> > > (or at least its defaults) so we don't generate these massive delta
> > > chains. But there are already packs in the wild that use these chains,
> > > resulting in performance problems for clients.
> >
> > But the long chains make the pack actually as efficient as it is...
>
> Perhaps Shawn thought here about limiting delta chain not by its
> *length*, but by its *size* (as required when unpacking last object
> in a delta chanin).
>
> What do you think about this idea?
I was talking about that. Or at least thinking it. But its not a
good solution, as Dscho points out those long chains is what makes
the pack file such a powerful source code compressor.
IMHO the right solution is to continue to allow these long chains,
especially since they are typically used for the older, less often
accessed objects, and use bounded caching where necessary in the
readers to avoid unpacking costs. We already have a bounded cache
for the random access case used by most programs. What we missed
was the bounded cache in index-pack, and my 4 patch series is an
attempt to define that.
--
Shawn.
^ permalink raw reply
* Re: [PATCH 3/4] index-pack: Track the object_entry that creates each base_data
From: Nicolas Pitre @ 2008-07-15 3:42 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Junio C Hamano, git, Stephan Hennig, Andreas Ericsson
In-Reply-To: <20080715032008.GE1700@spearce.org>
On Tue, 15 Jul 2008, Shawn O. Pearce wrote:
> Nicolas Pitre <nico@cam.org> wrote:
> > On Sun, 13 Jul 2008, Shawn O. Pearce wrote:
> >
> > > If we free the data stored within a base_data we need the struct
> > > object_entry to get the data back again for use with another
> > > dependent delta. Storing the object_entry* makes it simple to call
> > > get_data_from_pack() to recover the compressed information.
> > >
> > > This however means we must add the missing baes object to the end
> >
> > Typo?
>
> Yea, Dscho also pointed it out.
>
> Junio, if you can, please fix this typo in the commit message.
>
> Its not really a big deal. I have no plans on posting a replacement
> patch just for this one small typo.
No, it is not a big deal. I do write crappy english myself. I
initially had some comments on the patch itself which whould have
warranted another patch and then changed my mind.
Nicolas
^ permalink raw reply
* Re: What's cooking in git.git (topics)
From: Geoffrey Irving @ 2008-07-15 3:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Johannes Schindelin
In-Reply-To: <7vtzetjbif.fsf@gitster.siamese.dyndns.org>
On Sun, Jul 13, 2008 at 10:11 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Here are the topics that have been cooking. Commits prefixed
> with '-' are only in 'pu' while commits prefixed with '+' are
> in 'next'.
>
> <snip>
>
> * gi/cherry-cache (Sat Jul 12 20:14:51 2008 -0700) 1 commit
> - cherry: cache patch-ids to avoid repeating work
>
> This does not seem to pass tests even on its own.
The problem (beyond the basic problem of me not having tried running
the tests) is that the current caching code isn't taking into account
the changing values of diff_options. t6007 computes a patch-id for a
commit with one value of options.paths, and then tries to compute a
_different_ patch-id for the same commit using a different value of
options.paths.
Here are a few different ways of fixing this:
1. Modify commit_patch_id in patch-ids.c to compute a sha1 of the
diff_options structure and xor it with the commit sha1 to get a truly
unique hash of the input. This means the optimization can be safely
applied for all patch-id computations regardless of the diff_options.
I can add a diff_options_sha1 function in diff.[ch] to compute the
checksum.
2. Restrict commit_patch_id in patch-ids.c to apply the optimization
only if options.nr_paths is zero, and perhaps a few other conditions.
This is rather fragile, since it would mean that the cache would break
if someone decided to change the default diff options.
3. Add a flag in struct patch_ids defaulting to false which turns the
caching on or off, and manually set the flag to true in cmd_cherry.
I'd lean towards (1), but wanted to check before writing the code to
make sure that it's reasonable to treat diff_options as stable enough
that computing a sha1 hash of it makes sense. According to "git help
patch-id", it is only "reasonable stable", which is sufficient as long
as we're confident that whenever the diff format changes, the
diff_options_sha1 function will be updated to reflect that change.
As an aside: is it correct that as long as the change is in pu, I
should be submitting complete (nonincremental) patches whenever I fix
bugs?
Thanks,
Geoffrey
^ permalink raw reply
* Re: Closing the merge window for 1.6.0
From: Nicolas Pitre @ 2008-07-15 3:30 UTC (permalink / raw)
To: Shawn O. Pearce
Cc: Gerrit Pape, Johannes Schindelin, Petr Baudis, Junio C Hamano,
git
In-Reply-To: <20080715025127.GC1700@spearce.org>
On Tue, 15 Jul 2008, Shawn O. Pearce wrote:
> Nicolas Pitre <nico@cam.org> wrote:
> > On Mon, 14 Jul 2008, Gerrit Pape wrote:
> >
> > > On Mon, Jul 14, 2008 at 12:57:56PM +0100, Johannes Schindelin wrote:
> > > > On Mon, 14 Jul 2008, Petr Baudis wrote:
> > > > > I'm saying this because I believe the best conservative upper bound for
> > > > > backwards compatibility is Git version in Debian stable. It gets
> > > > > probably the most stale from all the widely used software distributions
> > > > > using Git, and it *is* quite widely used. Etch carries v1.4.4.4, which
> > > > > fails miserably on the new packs.
> >
> > Maybe we can release 1.4.5 with the ability to read index v2? That
> > wouldn't be hard to backport the reading part of it.
>
> If we consider that supporting 1.4.4.4 clients is still a priority,
> due to the widespread distribution of that version in a popular
> version of Debian, we shouldn't be rushing the index v2 or OFS_DELTA
> functionality on by default in 1.6.0.
OFS_DELTA is supported by 1.4.4.4 so that's a non issue.
> Instead we would wait until Debian stable (and most other widely
> popular distributions) are on a modern enough version of Git to
> understand this format.
I don't think we should have git development be dictated by some
discutable policy from one distribution.
IMHO git prior 1.5.0 is so horrible as general usability goes, and so
different from what everybody is discussing on the net, that no one sane
should still be using it. Even ourselves (i.e. the git community) are
not supporting git 1.4.4 anymore so this hardly can be a priority.
As far as I know, there is no other widely popular distribution other
than Debian using git prior 1.5.0 in their latest release. If Debian
people want to support git 1.4.4 although we called thatversion obsolete
_long_ ago then that's their problem. We should not be bound by that
external policy to which we never agreed with.
Now I proposed a compromise which consists of making 1.4.4.4+1 able to
cope with index v2. That should fall into Debian's "major usability
fix" category. I think that is a far better compromize than delaying
index v2 even further.
> Really. As much as I'd love to see the switch to v2 made by default
> I don't think we can/should do it unless the majority of the user
> base will be able to grok it. And Debian etch sounds like it won't.
I truly hope the majority of the user is _not_ using 1.4.4.4. Otherwise
I may only have pity for them.
Nicolas
^ 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