* Re: Remove diff machinery dependency from read-cache
From: Junio C Hamano @ 2010-01-22 3:56 UTC (permalink / raw)
To: Nicolas Pitre
Cc: Junio C Hamano, Linus Torvalds, Shawn O. Pearce, Git Mailing List
In-Reply-To: <alpine.LFD.2.00.1001212131230.1726@xanadu.home>
Nicolas Pitre <nico@fluxnic.net> writes:
> I do use it, but for developing/debugging pack stuff. I don't suggest
> removing it, but I don't think making it a built-in has value either.
I thought people _might_ have used it for satistics purposes, but it
appears that the command doesn't even give in-pack size of objects nor
delta chain length, so probably anybody doing pack statistics would be
using "verify-pack -v" and wouldn't mind if it became test-show-index.
> So I really think that Linus' patch (which is missing hex.c btw) is a
> good thing to do, even if only for the cleanup value.
>
> Then, git-show-index could probably become test-show-index and no longer
> leave the build directory.
Yeah, I think that is a sensible long term plan, too.
^ permalink raw reply
* Re: Remove diff machinery dependency from read-cache
From: Linus Torvalds @ 2010-01-22 3:58 UTC (permalink / raw)
To: Junio C Hamano, Git Mailing List
In-Reply-To: <alpine.LFD.2.00.1001211119130.13231@localhost.localdomain>
On Thu, 21 Jan 2010, Linus Torvalds wrote:
>
> It's not just that one file either. I get:
>
> [torvalds@nehalem git]$ du -s /home/torvalds/libexec/git-core
> 45640 /home/torvalds/libexec/git-core (before)
> 33508 /home/torvalds/libexec/git-core (after)
>
> so we're talking 12MB of diskspace here.
Btw, with the few built-in patches I've sent, it's now
[torvalds@nehalem git]$ du -s /home/torvalds/libexec/git-core
27876 /home/torvalds/libexec/git-core
so they're worth about 5.5M of disk-space.
Of course, without debugging and with -Os, the difference is much smaller,
and libexec ends up being "just" 8.6MB in size. There are still a number
of trivial programs that could be made builtin.
Optimally, I suspect just things like 'git-daemon' and the remote helpers
would be actual separate binaries.
Linus
^ permalink raw reply
* Re: [PATCH 3/3] git-p4: improve submit performance on new P4 servers
From: Nicolas Pitre @ 2010-01-22 3:59 UTC (permalink / raw)
To: Pal-Kristian Engstad; +Cc: Simon Hausmann, Junio C Hamano, git@vger.kernel.org
In-Reply-To: <4B591455.7050409@naughtydog.com>
On Thu, 21 Jan 2010, Pal-Kristian Engstad wrote:
> Nicolas Pitre wrote:
> > On Thu, 21 Jan 2010, Pal-Kristian Engstad wrote:
> >
> >> Improve git-p4 submit performance on newer (from 2009.2) Perforce
> >> servers by changing "p4 diff -du" to "p4 diff -dub". This change is
> >> harmless since the output is only used for display purposes.
> >>
> >> Signed-off-by: Pal-Kristian Engstad <pal_engstad@naughtydog.com>
> >
> > Why is the b flag impacting performance?
>
> That's a very good question. The release notes say that they've been
> changing how 'p4 diff -du' works, but the net effect of it all is that
> it stats all files in the whole working set.
And so does git.
> For large projects, this
> takes forever. We say pauses of 3 minutes per submit...
This is abominable.
> > And even if for display purposes, why might you wish not to see
> > differences in whitespace changes?
>
> That's a good point, but what alternative is there?
Have the option of not seeing the diff in the submit template maybe?
I agree that P4 is slow. Painfully slow. I even needed to make git-p4
even slower by adding a time.sleep(1) in p4_build_cmd() otherwise the
server would randomly drop the connection with the client when being
contacted back to back.
I'm so glad Simon wrote this git-p4 nevertheless. Makes working with P4
almost enjoyable.
Nicolas
^ permalink raw reply
* [PATCH] git-mv: Fix error with multiple sources.
From: David Rydh @ 2010-01-21 20:39 UTC (permalink / raw)
To: git@vger.kernel.org; +Cc: David Rydh, Johannes Sixt
Commit b8f26269 (fix directory separator treatment on Windows,
30-06-2009) introduced a bug on Mac OS X. The problem is that
basename() may return a pointer to internal static storage space that
will be overwritten by subsequent calls:
> git mv dir/a.txt dir/b.txt other/
Checking rename of 'dir/a.txt' to 'other/b.txt'
Checking rename of 'dir/b.txt' to 'other/b.txt'
fatal: multiple sources for the same target,
source=dir/b.txt, destination=other/b.txt
This commit also fixed two potentially dangerous uses of
prefix_filename() -- which returns static storage space -- in
builtin-config.c and hash-object.c.
get_pathspec(): If called with an empty pathspec, allocate a new
pathspec with a copy of prefix. This is consistent with the behavior
when called with a non-empty pathspec and prevents potential errors.
Signed-off-by: David Rydh <dary@math.berkeley.edu>
---
This is my first patch submission to git. Perhaps this patch should
be split into two? The one-line change to builtin-mv.c suffices to
fix the bug.
builtin-config.c | 4 ++--
builtin-mv.c | 2 +-
hash-object.c | 2 +-
setup.c | 4 ++--
4 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/builtin-config.c b/builtin-config.c
index 2e3ef91..bf60f93 100644
--- a/builtin-config.c
+++ b/builtin-config.c
@@ -356,9 +356,9 @@ int cmd_config(int argc, const char **argv, const char *unused_prefix)
config_exclusive_filename = git_etc_gitconfig();
else if (given_config_file) {
if (!is_absolute_path(given_config_file) && prefix)
- config_exclusive_filename = prefix_filename(prefix,
+ config_exclusive_filename = xstrdup(prefix_filename(prefix,
strlen(prefix),
- argv[2]);
+ argv[2]));
else
config_exclusive_filename = given_config_file;
}
diff --git a/builtin-mv.c b/builtin-mv.c
index 8247186..1c1f8be 100644
--- a/builtin-mv.c
+++ b/builtin-mv.c
@@ -27,7 +27,7 @@ static const char **copy_pathspec(const char *prefix, const char **pathspec,
if (length > 0 && is_dir_sep(result[i][length - 1]))
result[i] = xmemdupz(result[i], length - 1);
if (base_name)
- result[i] = basename((char *)result[i]);
+ result[i] = xstrdup(basename((char *)result[i]));
}
return get_pathspec(prefix, result);
}
diff --git a/hash-object.c b/hash-object.c
index 9455dd0..3c509aa 100644
--- a/hash-object.c
+++ b/hash-object.c
@@ -91,7 +91,7 @@ int main(int argc, const char **argv)
prefix = setup_git_directory();
prefix_length = prefix ? strlen(prefix) : 0;
if (vpath && prefix)
- vpath = prefix_filename(prefix, prefix_length, vpath);
+ vpath = xstrdup(prefix_filename(prefix, prefix_length, vpath));
}
git_config(git_default_config, NULL);
diff --git a/setup.c b/setup.c
index 710e2f3..80cf535 100644
--- a/setup.c
+++ b/setup.c
@@ -132,8 +132,8 @@ const char **get_pathspec(const char *prefix, const char **pathspec)
return NULL;
if (!entry) {
- static const char *spec[2];
- spec[0] = prefix;
+ const char **spec = xmalloc(sizeof(char *) * 2);
+ spec[0] = xstrdup(prefix);
spec[1] = NULL;
return spec;
}
--
1.6.6.1
^ permalink raw reply related
* Re: Remove diff machinery dependency from read-cache
From: Linus Torvalds @ 2010-01-22 4:21 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nicolas Pitre, Shawn O. Pearce, Git Mailing List
In-Reply-To: <7vvdeubxnh.fsf@alter.siamese.dyndns.org>
On Thu, 21 Jan 2010, Junio C Hamano wrote:
> Nicolas Pitre <nico@fluxnic.net> writes:
>
> > I do use it, but for developing/debugging pack stuff. I don't suggest
> > removing it, but I don't think making it a built-in has value either.
>
> I thought people _might_ have used it for satistics purposes, but it
> appears that the command doesn't even give in-pack size of objects nor
> delta chain length, so probably anybody doing pack statistics would be
> using "verify-pack -v" and wouldn't mind if it became test-show-index.
>
> > So I really think that Linus' patch (which is missing hex.c btw) is a
> > good thing to do, even if only for the cleanup value.
> >
> > Then, git-show-index could probably become test-show-index and no longer
> > leave the build directory.
>
> Yeah, I think that is a sensible long term plan, too.
Note that there are other totally trivial git programs like 'git-var' that
have exactly the same issue as 'git-show-index'.
Same details: it doesn't really want any diff machinery, doesn't want any
git object handling, but can't avoid it because it uses xmalloc (through
environment.c and config.c etc), so a program that _should_ be pretty
trivially small again ends up being 200+kB in size even without debug
info (and almost a megabyte with it).
So here's another trivial builtin-ification patch.
Linus
---
Makefile | 2 +-
var.c => builtin-var.c | 4 +---
builtin.h | 1 +
git.c | 1 +
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/Makefile b/Makefile
index f9e4aa3..398b5fb 100644
--- a/Makefile
+++ b/Makefile
@@ -396,7 +396,6 @@ PROGRAMS += git-patch-id$X
PROGRAMS += git-shell$X
PROGRAMS += git-unpack-file$X
PROGRAMS += git-upload-pack$X
-PROGRAMS += git-var$X
PROGRAMS += git-http-backend$X
# List built-in command $C whose implementation cmd_$C() is not in
@@ -703,6 +702,7 @@ BUILTIN_OBJS += builtin-update-index.o
BUILTIN_OBJS += builtin-update-ref.o
BUILTIN_OBJS += builtin-update-server-info.o
BUILTIN_OBJS += builtin-upload-archive.o
+BUILTIN_OBJS += builtin-var.o
BUILTIN_OBJS += builtin-verify-pack.o
BUILTIN_OBJS += builtin-verify-tag.o
BUILTIN_OBJS += builtin-write-tree.o
diff --git a/var.c b/builtin-var.c
similarity index 96%
rename from var.c
rename to builtin-var.c
index d9892f8..2280518 100644
--- a/var.c
+++ b/builtin-var.c
@@ -72,7 +72,7 @@ static int show_config(const char *var, const char *value, void *cb)
return git_default_config(var, value, cb);
}
-int main(int argc, char **argv)
+int cmd_var(int argc, const char **argv, const char *prefix)
{
const char *val;
int nongit;
@@ -80,8 +80,6 @@ int main(int argc, char **argv)
usage(var_usage);
}
- git_extract_argv0_path(argv[0]);
-
setup_git_directory_gently(&nongit);
val = NULL;
diff --git a/builtin.h b/builtin.h
index 3aa6b6c..0c9c396 100644
--- a/builtin.h
+++ b/builtin.h
@@ -107,6 +107,7 @@ extern int cmd_update_ref(int argc, const char **argv, const char *prefix);
extern int cmd_update_server_info(int argc, const char **argv, const char *prefix);
extern int cmd_upload_archive(int argc, const char **argv, const char *prefix);
extern int cmd_upload_tar(int argc, const char **argv, const char *prefix);
+extern int cmd_var(int argc, const char **argv, const char *prefix);
extern int cmd_verify_tag(int argc, const char **argv, const char *prefix);
extern int cmd_version(int argc, const char **argv, const char *prefix);
extern int cmd_whatchanged(int argc, const char **argv, const char *prefix);
diff --git a/git.c b/git.c
index a952663..09d3272 100644
--- a/git.c
+++ b/git.c
@@ -373,6 +373,7 @@ static void handle_internal_command(int argc, const char **argv)
{ "update-ref", cmd_update_ref, RUN_SETUP },
{ "update-server-info", cmd_update_server_info, RUN_SETUP },
{ "upload-archive", cmd_upload_archive },
+ { "var", cmd_var },
{ "verify-tag", cmd_verify_tag, RUN_SETUP },
{ "version", cmd_version },
{ "whatchanged", cmd_whatchanged, RUN_SETUP | USE_PAGER },
^ permalink raw reply related
* Re: Remove diff machinery dependency from read-cache
From: Linus Torvalds @ 2010-01-22 4:31 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Junio C Hamano, Shawn O. Pearce, Git Mailing List
In-Reply-To: <alpine.LFD.2.00.1001212131230.1726@xanadu.home>
On Thu, 21 Jan 2010, Nicolas Pitre wrote:
>
> So I really think that Linus' patch (which is missing hex.c btw) is a
> good thing to do, even if only for the cleanup value.
Btw, it looks like the separate hex.c would fix not just git-show-index
(together with de-xmalloc), but also make git-patch-id shrink down. Except
git-patch-id for some reason does git_extract_argv0_path(), which brings
in exec_cmd.o, which brings in strbuf, and xmalloc, and now it's all the
same old pain again.
So rather than try to solve it all (xmalloc in particular is pretty
hairy), here's another patch.
Linus
---
Makefile | 2 +-
patch-id.c => builtin-patch-id.c | 4 +---
builtin.h | 1 +
git.c | 1 +
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/Makefile b/Makefile
index 398b5fb..5b614e4 100644
--- a/Makefile
+++ b/Makefile
@@ -392,7 +392,6 @@ PROGRAMS += git-index-pack$X
PROGRAMS += git-merge-index$X
PROGRAMS += git-mktag$X
PROGRAMS += git-pack-redundant$X
-PROGRAMS += git-patch-id$X
PROGRAMS += git-shell$X
PROGRAMS += git-unpack-file$X
PROGRAMS += git-upload-pack$X
@@ -674,6 +673,7 @@ BUILTIN_OBJS += builtin-mv.o
BUILTIN_OBJS += builtin-name-rev.o
BUILTIN_OBJS += builtin-pack-objects.o
BUILTIN_OBJS += builtin-pack-refs.o
+BUILTIN_OBJS += builtin-patch-id.o
BUILTIN_OBJS += builtin-prune-packed.o
BUILTIN_OBJS += builtin-prune.o
BUILTIN_OBJS += builtin-push.o
diff --git a/patch-id.c b/builtin-patch-id.c
similarity index 95%
rename from patch-id.c
rename to builtin-patch-id.c
index 0df4cb0..af0911e 100644
--- a/patch-id.c
+++ b/builtin-patch-id.c
@@ -75,13 +75,11 @@ static void generate_id_list(void)
static const char patch_id_usage[] = "git patch-id < patch";
-int main(int argc, char **argv)
+int cmd_patch_id(int argc, const char **argv, const char *prefix)
{
if (argc != 1)
usage(patch_id_usage);
- git_extract_argv0_path(argv[0]);
-
generate_id_list();
return 0;
}
diff --git a/builtin.h b/builtin.h
index 0c9c396..ab723f8 100644
--- a/builtin.h
+++ b/builtin.h
@@ -76,6 +76,7 @@ extern int cmd_mktree(int argc, const char **argv, const char *prefix);
extern int cmd_mv(int argc, const char **argv, const char *prefix);
extern int cmd_name_rev(int argc, const char **argv, const char *prefix);
extern int cmd_pack_objects(int argc, const char **argv, const char *prefix);
+extern int cmd_patch_id(int argc, const char **argv, const char *prefix);
extern int cmd_pickaxe(int argc, const char **argv, const char *prefix);
extern int cmd_prune(int argc, const char **argv, const char *prefix);
extern int cmd_prune_packed(int argc, const char **argv, const char *prefix);
diff --git a/git.c b/git.c
index 09d3272..e38f201 100644
--- a/git.c
+++ b/git.c
@@ -341,6 +341,7 @@ static void handle_internal_command(int argc, const char **argv)
{ "mv", cmd_mv, RUN_SETUP | NEED_WORK_TREE },
{ "name-rev", cmd_name_rev, RUN_SETUP },
{ "pack-objects", cmd_pack_objects, RUN_SETUP },
+ { "patch-id", cmd_patch_id },
{ "peek-remote", cmd_ls_remote },
{ "pickaxe", cmd_blame, RUN_SETUP },
{ "prune", cmd_prune, RUN_SETUP },
^ permalink raw reply related
* [PATCH] fix git-p4 editor invocation
From: Nicolas Pitre @ 2010-01-22 5:55 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
The strip() is required to remove the trailing newline character,
as already done elsewhere.
Signed-off-by: Nicolas Pitre <nico@fluxnic.net>
diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index 48059d0..d1512e0 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -732,7 +732,7 @@ class P4Submit(Command):
if os.environ.has_key("P4EDITOR"):
editor = os.environ.get("P4EDITOR")
else:
- editor = read_pipe("git var GIT_EDITOR")
+ editor = read_pipe("git var GIT_EDITOR").strip()
system(editor + " " + fileName)
response = "y"
^ permalink raw reply related
* Re: [PATCH] git-mv: Fix error with multiple sources.
From: Junio C Hamano @ 2010-01-22 5:57 UTC (permalink / raw)
To: David Rydh; +Cc: git@vger.kernel.org, Johannes Sixt
In-Reply-To: <718290.769818367-sendEmail@darysmbp>
"David Rydh" <dary@math.berkeley.edu> writes:
> Commit b8f26269 (fix directory separator treatment on Windows,
> 30-06-2009) introduced a bug on Mac OS X. The problem is that
> basename() may return a pointer to internal static storage space that
> will be overwritten by subsequent calls:
>
>> git mv dir/a.txt dir/b.txt other/
>
> Checking rename of 'dir/a.txt' to 'other/b.txt'
> Checking rename of 'dir/b.txt' to 'other/b.txt'
Good spotting. basename(3)/dirname(3) are tricky functions to use
correctly. In addition to the "static storage" implementation and its
associated problem you encountered, they can also be implemented to modify
the given string in place, and can even return a pointer _into_ the given
string, which means that if you do:
duped = strdup(string);
duped = basename(duped);
... use duped ...
free(duped);
you may be burned quite badly. The other call site of basename(3) is in
diff.c and it looks safe.
> diff --git a/setup.c b/setup.c
> index 710e2f3..80cf535 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -132,8 +132,8 @@ const char **get_pathspec(const char *prefix, const char **pathspec)
> return NULL;
>
> if (!entry) {
> - static const char *spec[2];
> - spec[0] = prefix;
> + const char **spec = xmalloc(sizeof(char *) * 2);
> + spec[0] = xstrdup(prefix);
> spec[1] = NULL;
> return spec;
> }
I don't understand this change. Because elements of returned pathspec
from this function are often simply the pathspec argument itself (which in
turn is often argv[] of the calling program), and other times allocated by
this function, the callers are never going to free() them.
^ permalink raw reply
* Re: [PATCH] git-mv: Fix error with multiple sources.
From: Junio C Hamano @ 2010-01-22 6:17 UTC (permalink / raw)
To: David Rydh; +Cc: git@vger.kernel.org, Johannes Sixt
In-Reply-To: <718290.769818367-sendEmail@darysmbp>
"David Rydh" <dary@math.berkeley.edu> writes:
> This commit also fixed two potentially dangerous uses of
> prefix_filename() -- which returns static storage space -- in
> builtin-config.c and hash-object.c.
This should probably be a separate patch. builtin-hash-object.c also uses
prefix_filename() first to obtain vpath without strdup() and then uses the
function to create arg, which seems to be unsafe to me. I've looked at
all the hits from
$ git grep -n -e prefix_filename\( -- '*.c'
and other places seem to be Ok.
^ permalink raw reply
* Re: [PATCH] git-mv: Fix error with multiple sources.
From: Junio C Hamano @ 2010-01-22 6:24 UTC (permalink / raw)
To: David Rydh; +Cc: git@vger.kernel.org, Johannes Sixt
In-Reply-To: <718290.769818367-sendEmail@darysmbp>
"David Rydh" <dary@math.berkeley.edu> writes:
> diff --git a/builtin-mv.c b/builtin-mv.c
> index 8247186..1c1f8be 100644
> --- a/builtin-mv.c
> +++ b/builtin-mv.c
> @@ -27,7 +27,7 @@ static const char **copy_pathspec(const char *prefix, const char **pathspec,
> if (length > 0 && is_dir_sep(result[i][length - 1]))
> result[i] = xmemdupz(result[i], length - 1);
> if (base_name)
> - result[i] = basename((char *)result[i]);
> + result[i] = xstrdup(basename((char *)result[i]));
> }
> return get_pathspec(prefix, result);
> }
Given that basename(3) is allowed to modify its parameter, I think the
above code is still not portable. casting constness away and feeding
result[i], especially when we didn't obtain our own copy by calling
xmemdupz(), is especially problematic.
Perhaps something ugly like this?
for (i = 0; i < count; i++) {
int length = strlen(result[i]);
int to_copy = length;
while (to_copy > 0 && is_dir_sep(result[i][to_copy - 1]))
to_copy--;
if (to_copy != length || basename) {
char *it = xmemdupz(result[i], to_copy);
result[i] = base_name ? strdup(basename(it)) : it;
}
}
^ permalink raw reply
* Re: [PATCH] git-mv: Fix error with multiple sources.
From: Matthieu Moy @ 2010-01-22 7:03 UTC (permalink / raw)
To: David Rydh; +Cc: git@vger.kernel.org, Johannes Sixt
In-Reply-To: <718290.769818367-sendEmail@darysmbp>
"David Rydh" <dary@math.berkeley.edu> writes:
> if (!is_absolute_path(given_config_file) && prefix)
> - config_exclusive_filename = prefix_filename(prefix,
> + config_exclusive_filename = xstrdup(prefix_filename(prefix,
> strlen(prefix),
> - argv[2]);
> + argv[2]));
Broken indentation on the last two lines.
Other than that, you use strdup without free-ing the result. Probably
not so serious, but it may be cleaner to actually call free, or to
explain why you don't (in commit message).
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: [PATCH] git-mv: Fix error with multiple sources.
From: Johannes Sixt @ 2010-01-22 7:29 UTC (permalink / raw)
To: David Rydh; +Cc: Git Mailing List
In-Reply-To: <718290.769818367-sendEmail@darysmbp>
David Rydh schrieb:
> diff --git a/builtin-mv.c b/builtin-mv.c
> index 8247186..1c1f8be 100644
> --- a/builtin-mv.c
> +++ b/builtin-mv.c
> @@ -27,7 +27,7 @@ static const char **copy_pathspec(const char *prefix, const char **pathspec,
> if (length > 0 && is_dir_sep(result[i][length - 1]))
> result[i] = xmemdupz(result[i], length - 1);
> if (base_name)
> - result[i] = basename((char *)result[i]);
> + result[i] = xstrdup(basename((char *)result[i]));
> }
> return get_pathspec(prefix, result);
> }
We are already leaking memory of magnitude O(argc*length of file names),
and IMO, this new leak of the same magnitude doesn't hurt.
If you want to avoid it, you can set NO_LIBGEN_H in Makefile.
The other changes in this patch should really be a separate patch. They do
not fix an immediate problem, right?
-- Hannes
^ permalink raw reply
* 1.7.0 release plans
From: Junio C Hamano @ 2010-01-22 8:08 UTC (permalink / raw)
To: git
As I've been repeating lately, I'd like to tag 1.7.0-rc0 this weekend.
We don't have any written, official-looking rules for patch acceptance
during the -rc period, and have been playing by ear. The rule of thumb is
that if a patch is not in 'master' by the time 1.7.0-rc0 is tagged, it
won't be in 'master' until 1.7.0 final is released, unless it is:
(1) a fix to a regression caused by changes since 1.6.6;
(2) an obvious fix to a new feature we added since 1.6.6; or
(3) an obvious fix to a problem we had even before 1.6.6.
When you have a patch that adds yet another new feature that you think
complements what we already added since 1.6.6, you could argue that the
patch falls into the second category, by saying something like: "the new
option to do X is not complete without configuration variable that tells
git to do X by default, and another option to override the configuration".
We need to discuss and judge if the the argument has merit, which would
largely depend on what that X is. So the above list is not completely
black-and-white.
We'll cook 1.7.0-rc0 for a week or so, accumulating fixes and then tag rc1
by the end of this month. After that, only fixes in category (1) and
perhaps really obvious ones in category (2) will be applied for rc2, which
should happen around Feb 10th. I am hoping that we can cut the final
release around mid February.
I don't know if I have enough bandwidth to review and queue patches that
fall outside of the above "meant for 1.7.0" criteria to 'next/pu' branches
during the feature freeze, but that doesn't mean I'd discourage people
from working on their own topics meant for post-1.7.0 cycle. Hopefully we
have enough qualified people to help reviewing them.
In any case, I'll be moving my focus to the 'master' branch from now on,
and I ask contributors to help hunting and fixing problems in 'master', so
that we can have a solid 1.7.0 release (on top of which you can build your
favorite shiny new toys ;-).
Thanks.
^ permalink raw reply
* Re: [PATCH] Handle double slashes in make_relative_path()
From: Johannes Sixt @ 2010-01-22 8:36 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Thomas Rast, git, Linus Torvalds
In-Reply-To: <7vmy06et5c.fsf@alter.siamese.dyndns.org>
Junio C Hamano schrieb:
> Credit for test script, motivation and initial patch goes to Thomas Rast,
> but the bugs in the implementation of this patch are mine..
And with this squashed in it has fewer of them ;-) and is more portable.
The bug was that /foo was incorrectly stripped from /foobar.
-- Hannes
diff --git a/path.c b/path.c
index 78ab54a..3cb19c7 100644
--- a/path.c
+++ b/path.c
@@ -396,15 +396,15 @@ const char *make_relative_path(const char *abs, const char *base)
static char buf[PATH_MAX + 1];
int i = 0, j = 0;
- if (!base)
+ if (!base || !base[0])
return abs;
while (base[i]) {
- if (base[i] == '/') {
- if (abs[j] != '/')
+ if (is_dir_sep(base[i])) {
+ if (!is_dir_sep(abs[j]))
return abs;
- while (base[i] == '/')
+ while (is_dir_sep(base[i]))
i++;
- while (abs[j] == '/')
+ while (is_dir_sep(abs[j]))
j++;
continue;
} else if (abs[j] != base[i]) {
@@ -413,7 +413,14 @@ const char *make_relative_path(const char *abs, const char *base)
i++;
j++;
}
- while (abs[j] == '/')
+ if (
+ /* "/foo" is a prefix of "/foo" */
+ abs[j] &&
+ /* "/foo" is not a prefix of "/foobar" */
+ !is_dir_sep(base[i-1]) && !is_dir_sep(abs[j])
+ )
+ return abs;
+ while (is_dir_sep(abs[j]))
j++;
if (!abs[j])
strcpy(buf, ".");
^ permalink raw reply related
* Re: [PATCH 1/1] Random documentation spelling fixes
From: Štěpán Němec @ 2010-01-22 8:36 UTC (permalink / raw)
To: Horst H. von Brand; +Cc: git, gitster
In-Reply-To: <1264126491-8273-2-git-send-email-vonbrand@inf.utfsm.cl>
Cool!
On Thu, Jan 21, 2010 at 11:14:51PM -0300, Horst H. von Brand wrote:
>
> * "git fast-export" did not export octopus merges correctly.
> diff --git a/Documentation/RelNotes-1.5.6.txt b/Documentation/RelNotes-1.5.6.txt
> index e143d8d..0614225 100644
> --- a/Documentation/RelNotes-1.5.6.txt
> +++ b/Documentation/RelNotes-1.5.6.txt
> @@ -25,7 +25,7 @@ Updates since v1.5.5
> * Many freestanding documentation pages have been converted and made
> available to "git help" (aka "man git<something>") as section 7 of
> the manual pages. This means bookmarks to some HTML documentation
> - files may need to be updated (eg "tutorial.html" became
> + files may need to be updated (eg, "tutorial.html" became
> "gittutorial.html").
What about "e.g.", when you're at it? (And I don't think the comma is
necessary -- and you didn't add it in the very first hunk of the patch
anyway).
> diff --git a/Documentation/git-bisect-lk2009.txt b/Documentation/git-bisect-lk2009.txt
> index 6b7b2e5..eb35f2f 100644
> --- a/Documentation/git-bisect-lk2009.txt
> +++ b/Documentation/git-bisect-lk2009.txt
> @@ -1320,7 +1320,7 @@ So git bisect is unconditional goodness - and feel free to quote that
> ;-)
> _____________
>
> -Acknowledgements
> +Acknowledgments
> ----------------
Both are correct, AFAIK.
Regards,
Štěpán
^ permalink raw reply
* Re: Remove diff machinery dependency from read-cache
From: Johannes Sixt @ 2010-01-22 8:43 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Junio C Hamano, Shawn O. Pearce, Git Mailing List
In-Reply-To: <alpine.LFD.2.00.1001211823590.13231@localhost.localdomain>
Linus Torvalds schrieb:
> @@ -347,8 +347,6 @@ int main(int argc, char **argv)
>
> git_extract_argv0_path(argv[0]);
This line must go away as well.
> - setup_git_directory();
> -
-- Hannes
^ permalink raw reply
* Re: [PATCH 1/1] Random documentation spelling fixes
From: Michael J Gruber @ 2010-01-22 9:18 UTC (permalink / raw)
To: Horst H. von Brand; +Cc: git, gitster
In-Reply-To: <1264126491-8273-2-git-send-email-vonbrand@inf.utfsm.cl>
Horst H. von Brand venit, vidit, dixit 22.01.2010 03:14:
> Signed-off-by: Horst H. von Brand <vonbrand@inf.utfsm.cl>
> ---
> Documentation/RelNotes-1.5.0.txt | 2 +-
> Documentation/RelNotes-1.5.1.2.txt | 2 +-
> Documentation/RelNotes-1.5.1.4.txt | 2 +-
> Documentation/RelNotes-1.5.2.3.txt | 2 +-
> Documentation/RelNotes-1.5.2.txt | 2 +-
> Documentation/RelNotes-1.5.3.8.txt | 2 +-
> Documentation/RelNotes-1.5.3.txt | 6 +++---
> Documentation/RelNotes-1.5.4.2.txt | 2 +-
> Documentation/RelNotes-1.5.5.3.txt | 2 +-
> Documentation/RelNotes-1.5.6.txt | 2 +-
> Documentation/RelNotes-1.6.0.txt | 4 ++--
> Documentation/RelNotes-1.6.1.3.txt | 2 +-
> Documentation/RelNotes-1.6.3.txt | 2 +-
> Documentation/RelNotes-1.6.4.2.txt | 2 +-
> Documentation/RelNotes-1.6.4.3.txt | 2 +-
> Documentation/RelNotes-1.6.4.4.txt | 2 +-
> Documentation/RelNotes-1.6.4.txt | 2 +-
> Documentation/RelNotes-1.6.5.1.txt | 2 +-
> Documentation/RelNotes-1.6.5.3.txt | 4 ++--
> Documentation/RelNotes-1.6.5.4.txt | 2 +-
> Documentation/RelNotes-1.6.5.txt | 2 +-
> Documentation/RelNotes-1.6.6.txt | 6 +++---
I don't think we should change release notes after the fact, should we?
> Documentation/RelNotes-1.7.0.txt | 2 +-
> Documentation/config.txt | 2 +-
> Documentation/diff-format.txt | 2 +-
> Documentation/git-bisect-lk2009.txt | 2 +-
> Documentation/git-cvsimport.txt | 4 ++--
> Documentation/git-cvsserver.txt | 2 +-
> Documentation/git-fast-export.txt | 2 +-
> Documentation/git-filter-branch.txt | 2 +-
> Documentation/git-fsck.txt | 4 ++--
> Documentation/git-http-backend.txt | 4 ++--
> Documentation/git-imap-send.txt | 4 ++--
> Documentation/git-index-pack.txt | 2 +-
> Documentation/git-init.txt | 2 +-
> Documentation/git-ls-files.txt | 2 +-
> Documentation/git-merge-index.txt | 2 +-
> Documentation/git-mktree.txt | 2 +-
> Documentation/git-name-rev.txt | 2 +-
> Documentation/git-pack-objects.txt | 2 +-
> Documentation/git-patch-id.txt | 2 +-
> Documentation/git-receive-pack.txt | 2 +-
> Documentation/git-reflog.txt | 4 ++--
> Documentation/git-remote-helpers.txt | 4 ++--
> Documentation/git-remote.txt | 2 +-
> Documentation/git-repack.txt | 2 +-
> Documentation/git-replace.txt | 4 ++--
> Documentation/git-rev-parse.txt | 10 +++++-----
> Documentation/git-show-branch.txt | 4 ++--
> Documentation/git-show-index.txt | 2 +-
> Documentation/git-show-ref.txt | 4 ++--
> Documentation/git-submodule.txt | 6 +++---
> Documentation/git-svn.txt | 8 ++++----
> Documentation/git-tag.txt | 2 +-
> Documentation/git-unpack-file.txt | 2 +-
> Documentation/git-update-index.txt | 4 ++--
> Documentation/git-verify-tag.txt | 2 +-
> Documentation/git.txt | 12 ++++++------
> Documentation/githooks.txt | 2 +-
> Documentation/gitk.txt | 2 +-
> Documentation/gitmodules.txt | 4 ++--
> Documentation/gitrepository-layout.txt | 2 +-
> Documentation/gittutorial-2.txt | 16 ++++++++--------
> Documentation/pretty-formats.txt | 4 ++--
> Documentation/user-manual.txt | 14 +++++++-------
> 65 files changed, 110 insertions(+), 110 deletions(-)
That's not only many files changed in one patch, but also several
semi-automatic changes. We've had a discussion about SHA-1 vs. SHA1 vs.
sha1 a while ago. I don't think it led to a result, but looking that up
would be worthwhile.
Also, please have the commit message in the patch, not in a preceding
e-mail, and mention all types of changes, and split at least by category
(sha1 issue, protocol names, project names). Otherwise nobody will be
reviewing your patch, which would be sad: I favor consistency - and
failed several times in my attempts to reach it ;)
Michael
^ permalink raw reply
* [PATCH 0/3] Fixes to mh/rebase-fixup
From: Michael Haggerty @ 2010-01-22 9:22 UTC (permalink / raw)
To: git; +Cc: gitster, Johannes.Schindelin, tarmigan+git, j.sixt,
Michael Haggerty
This patch series contains fixes to the mh/rebase-fixup patch series
as suggested by reviewers on the mailing list.
Since the mh/rebase-fixup patch series is already in "next", I have
written these fixup patches against "next". I hope that is OK.
The third patch removes the attempt to use one-shot export of
variables in do_with_author(), as the "$@" command that it calls might
be a shell function. This issue was discussed on the mailing list
[1]. This change has the side effect that the GIT_AUTHOR_* variables
remain exported after do_with_author() returns, but I don't think that
this is a problem in the context of git-rebase--interactive.sh.
However, additional review would be welcome here.
[1] http://marc.info/?l=git&m=126345851831751&w=2
Michael Haggerty (3):
rebase -i: Avoid non-portable "test X -a Y"
rebase -i: Enclose sed command substitution in quotes
rebase -i: Export GIT_AUTHOR_* variables explicitly
git-rebase--interactive.sh | 6 ++----
t/lib-rebase.sh | 2 +-
2 files changed, 3 insertions(+), 5 deletions(-)
^ permalink raw reply
* [PATCH 3/3] rebase -i: Export GIT_AUTHOR_* variables explicitly
From: Michael Haggerty @ 2010-01-22 9:22 UTC (permalink / raw)
To: git; +Cc: gitster, Johannes.Schindelin, tarmigan+git, j.sixt,
Michael Haggerty
In-Reply-To: <cover.1264151435.git.mhagger@alum.mit.edu>
As pointed out on the mailing list, one-shot shell variable exports do
not necessarily work with shell functions. So export the GIT_AUTHOR_*
variables explicitly using "export".
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
git-rebase--interactive.sh | 4 +---
1 files changed, 1 insertions(+), 3 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index c2f6089..c8603f0 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -215,9 +215,7 @@ has_action () {
# Run command with GIT_AUTHOR_NAME, GIT_AUTHOR_EMAIL, and
# GIT_AUTHOR_DATE exported from the current environment.
do_with_author () {
- GIT_AUTHOR_NAME="$GIT_AUTHOR_NAME" \
- GIT_AUTHOR_EMAIL="$GIT_AUTHOR_EMAIL" \
- GIT_AUTHOR_DATE="$GIT_AUTHOR_DATE" \
+ export GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL GIT_AUTHOR_DATE &&
"$@"
}
--
1.6.6
^ permalink raw reply related
* [PATCH 1/3] rebase -i: Avoid non-portable "test X -a Y"
From: Michael Haggerty @ 2010-01-22 9:22 UTC (permalink / raw)
To: git; +Cc: gitster, Johannes.Schindelin, tarmigan+git, j.sixt,
Michael Haggerty
In-Reply-To: <cover.1264151435.git.mhagger@alum.mit.edu>
Reported by: Eric Blake <ebb9@byu.net>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
git-rebase--interactive.sh | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index e551906..c2f6089 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -235,7 +235,7 @@ pick_one () {
parent_sha1=$(git rev-parse --verify $sha1^) ||
die "Could not get the parent of $sha1"
current_sha1=$(git rev-parse --verify HEAD)
- if test -z "$no_ff" -a "$current_sha1" = "$parent_sha1"
+ if test -z "$no_ff" && test "$current_sha1" = "$parent_sha1"
then
output git reset --hard $sha1
output warn Fast-forward to $(git rev-parse --short $sha1)
--
1.6.6
^ permalink raw reply related
* [PATCH 2/3] rebase -i: Enclose sed command substitution in quotes
From: Michael Haggerty @ 2010-01-22 9:22 UTC (permalink / raw)
To: git; +Cc: gitster, Johannes.Schindelin, tarmigan+git, j.sixt,
Michael Haggerty
In-Reply-To: <cover.1264151435.git.mhagger@alum.mit.edu>
Reported by: Johannes Sixt <j.sixt@viscovery.net>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
t/lib-rebase.sh | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh
index 2d922ae..6aefe27 100644
--- a/t/lib-rebase.sh
+++ b/t/lib-rebase.sh
@@ -27,7 +27,7 @@ set_fake_editor () {
case "$1" in
*/COMMIT_EDITMSG)
test -z "$EXPECT_HEADER_COUNT" ||
- test "$EXPECT_HEADER_COUNT" = $(sed -n '1s/^# This is a combination of \(.*\) commits\./\1/p' < "$1") ||
+ test "$EXPECT_HEADER_COUNT" = "$(sed -n '1s/^# This is a combination of \(.*\) commits\./\1/p' < "$1")" ||
exit
test -z "$FAKE_COMMIT_MESSAGE" || echo "$FAKE_COMMIT_MESSAGE" > "$1"
test -z "$FAKE_COMMIT_AMEND" || echo "$FAKE_COMMIT_AMEND" >> "$1"
--
1.6.6
^ permalink raw reply related
* [PATCH] bash: don't offer remote transport helpers as subcommands
From: SZEDER Gábor @ 2010-01-22 10:54 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git, SZEDER Gábor
Since commits a2d725b7 (Use an external program to implement fetching
with curl, 2009-08-05) and c9e388bb (Make the
"traditionally-supported" URLs a special case, 2009-09-03) remote
transport helpers like 'remote-ftp' and 'remote-curl' are offered by the
completion script as available subcommands. Not good, since they are
helpers, therefore should not be offered, so filter them out.
Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
Maybe maint-worthy? 1.6.5 was the first release with this bug, but
nobody complained since then.
contrib/completion/git-completion.bash | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 9651720..7def62c 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -568,6 +568,7 @@ __git_list_porcelain_commands ()
read-tree) : plumbing;;
receive-pack) : plumbing;;
reflog) : plumbing;;
+ remote-*) : transport;;
repo-config) : deprecated;;
rerere) : plumbing;;
rev-list) : plumbing;;
--
1.6.6.1.361.gc5121
^ permalink raw reply related
* Re: [PATCH 3/3] rebase -i: Export GIT_AUTHOR_* variables explicitly
From: Johannes Schindelin @ 2010-01-22 11:16 UTC (permalink / raw)
To: Michael Haggerty; +Cc: git, gitster, tarmigan+git, j.sixt
In-Reply-To: <c6efda03848abc00cf8bf8d84fc34ef0d652b64c.1264151435.git.mhagger@alum.mit.edu>
Hi,
On Fri, 22 Jan 2010, Michael Haggerty wrote:
> As pointed out on the mailing list, one-shot shell variable exports do
> not necessarily work with shell functions. So export the GIT_AUTHOR_*
> variables explicitly using "export".
This one's a bit hairy; I really was not sure about unintended side
effects, that is why I avoided the export.
Just imagine, for example, some git commit --amend which forgets to set
the author information; I am not saying that this is happening, but I
cannot be sure, because every possible code path to a git
commit/commit-tree has to be checked, and this does not mean that future
patches will not introduce such broken code, either.
It might also be possible that some people scripted rebase -i with a
custom "editor" as in the tests (I have done so in the past). They would
be affected.
Ciao,
Dscho
^ permalink raw reply
* [PATCH] merge-tree: remove unnecessary call of git_extract_argv0_path
From: Johannes Sixt @ 2010-01-22 11:47 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Junio C Hamano, Shawn O. Pearce, git, Johannes Sixt
In-Reply-To: <4B596519.8060508@viscovery.net>
This call should have been removed when the utility was made a builtin by
907a7cb.
Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
builtin-merge-tree.c | 2 --
1 files changed, 0 insertions(+), 2 deletions(-)
diff --git a/builtin-merge-tree.c b/builtin-merge-tree.c
index 8e16c3e..a4a4f2c 100644
--- a/builtin-merge-tree.c
+++ b/builtin-merge-tree.c
@@ -345,8 +345,6 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)
if (argc != 4)
usage(merge_tree_usage);
- git_extract_argv0_path(argv[0]);
-
buf1 = get_tree_descriptor(t+0, argv[1]);
buf2 = get_tree_descriptor(t+1, argv[2]);
buf3 = get_tree_descriptor(t+2, argv[3]);
--
1.6.6.1.372.g084d
^ permalink raw reply related
* [Patch 0/5] make more commands built-in
From: Linus Torvalds @ 2010-01-22 16:22 UTC (permalink / raw)
To: Junio C Hamano, Git Mailing List
Ok, so since I was working on this yesterday, I decided to just continue
until I was done with all the trivial files.
This series makes five more commands built-in: 'merge-index', 'mktag',
'unpack-file', 'pack-redundant' and 'index-pack'. It further shrinks my
git install footprint by about 20%:
[torvalds@nehalem git]$ du -s /home/torvalds/libexec/git-core
21424 /home/torvalds/libexec/git-core (before)
16920 /home/torvalds/libexec/git-core (after)
and if we didn't default to having debug info enabled, it would look
almost acceptable:
[torvalds@nehalem git]$ du -s /home/torvalds/libexec/git-core
5728 /home/torvalds/libexec/git-core (with "-g" removed)
5108 /home/torvalds/libexec/git-core (with -Os and no debugging)
There still remains a few big git commands after this, but this takes care
of most of the core ones.
All of these patches are trivial, the only one that has _any_ changes
apart from the obvious builtin conversion is 'pack-redundant' which
required some of the pack creation interfaces to now take 'const' index
pathnames etc.
But as you can see, it removes more lines than it adds, and considering
that yesterday the 'du' reported ~40MB of disk space for the git install,
the shrinking certainly matters (of course, I suspect most distros
wouldn't ship with debug info, so for most people it's about a few MB
rather than a few tens of MB, but still).
Linus
---
Linus Torvalds (5):
make "merge-index" a built-in
make "mktag" a built-in
make "git unpack-file" a built-in
make "git pack-redundant" a built-in
make "index-pack" a built-in
Makefile | 10 +++++-----
index-pack.c => builtin-index-pack.c | 16 +++++++---------
merge-index.c => builtin-merge-index.c | 7 ++-----
mktag.c => builtin-mktag.c | 6 +-----
builtin-pack-objects.c | 5 +++--
pack-redundant.c => builtin-pack-redundant.c | 8 ++------
unpack-file.c => builtin-unpack-file.c | 5 +----
builtin.h | 5 +++++
git.c | 5 +++++
pack-write.c | 4 ++--
pack.h | 2 +-
11 files changed, 34 insertions(+), 39 deletions(-)
rename index-pack.c => builtin-index-pack.c (98%)
rename merge-index.c => builtin-merge-index.c (94%)
rename mktag.c => builtin-mktag.c (98%)
rename pack-redundant.c => builtin-pack-redundant.c (99%)
rename unpack-file.c => builtin-unpack-file.c (89%)
^ 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