* Re: [PATCH] fix for "index-pack: rationalize delta resolution code"
From: Harvey Harrison @ 2008-10-20 19:07 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Junio C Hamano, git, Jeff King
In-Reply-To: <590657100810201137m477b834cr9c940851b1a599d8@mail.gmail.com>
On Mon, Oct 20, 2008 at 11:37 AM, Harvey Harrison
<harvey.harrison@gmail.com> wrote:
> On Mon, Oct 20, 2008 at 11:12 AM, Nicolas Pitre <nico@cam.org> wrote:
>> My bad. A small detail went through the crack: the real_type of
>> a delta object is the real_type of its base object.
>>
>> Without this, the created index will be wrong as the actual object SHA1
>> won't match the object.
>>
>> Signed-off-by: Nicolas Pitre <nico@cam.org>
>
> This fixes it for me, thanks for the quick fix.
>
> Tested-by: Harvey Harrison <harvey.harrison@gmail.com>
>
Scratch that, it's back to failing again on my next update.
Harvey
^ permalink raw reply
* Re: [PATCH] fix for "index-pack: rationalize delta resolution code"
From: Marco Roeland @ 2008-10-20 19:14 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Junio C Hamano, git, Jeff King
In-Reply-To: <alpine.LFD.2.00.0810201357340.26244@xanadu.home>
On Monday October 2008 at 14:12 Nicolas Pitre wrote:
> My bad. A small detail went through the crack: the real_type of
> a delta object is the real_type of its base object.
>
> Without this, the created index will be wrong as the actual object SHA1
> won't match the object.
>
> Signed-off-by: Nicolas Pitre <nico@cam.org>
> ---
>
> If you got a corrupted .idx file because of this ('git verify-pack'
> should tell) then just toss it and recreate with a fixed 'git
> index-pack'.
>
> Could anyone having problems fetching from kernel.org with git from the
> next branch confirm that this also fixes that? Thanks.
I still seem to have the same problem after patching:
$ git pull
remote: Counting objects: 279, done.
remote: Compressing objects: 100% (78/78), done.
remote: Total 177 (delta 136), reused 135 (delta 99)
Receiving objects: 100% (177/177), 66.59 KiB, done.
fatal: pack has bad object at offset 53487: failed to apply delta
fatal: index-pack failed
'git verify-pack' does _not_ report an error for either pack or index.
This is with git from branch next at 8f0e41f379d486dd27766d84d994eb1da5b8319d
trying to pull from git://git.kernel.org/pub/scm/git/git.git
This is on Debian 'sid' with an AMD64 architecture.
I've put the whole ".git" directory (warning: almost 35MB) for
investigation at:
http://www.xs4all.nl/~fiberbit/http://www.xs4all.nl/~fiberbit/git-next-8f0e41f3-bad-index.tgz
I hope I've patched correctly. After applying (cleanly) and rebuilding
simply executing "./git" from the workdirectory still uses the old
version. Only after using "make install" I get the patched version,
which as shown above still gives an error, from the die() at line 528 in
index-pack.c: bad_object(delta_obj->idx.offset, "failed to apply
delta");
Not much more time tonight here, but perhaps it's easier to reproduce
now with the copy of an affected .git directory.
--
Marco Roeland
^ permalink raw reply
* Re: [PATCH] fix for "index-pack: rationalize delta resolution code"
From: Nicolas Pitre @ 2008-10-20 19:27 UTC (permalink / raw)
To: Marco Roeland; +Cc: Junio C Hamano, git, Jeff King
In-Reply-To: <20081020192051.GA21770@fiberbit.xs4all.nl>
On Mon, 20 Oct 2008, Marco Roeland wrote:
> Op maandag 20 oktober 2008 om 21:14 uur schreef Marco Roeland het volgende:
>
> > This is on Debian 'sid' with an AMD64 architecture.
> >
> > I've put the whole ".git" directory (warning: almost 35MB) for
> > investigation at:
> >
> > http://www.xs4all.nl/~fiberbit/http://www.xs4all.nl/~fiberbit/git-next-8f0e41f3-bad-index.tgz
>
> Gah, I can't even copy-and-paste:
>
> http://www.xs4all.nl/~fiberbit/git-next-8f0e41f3-bad-index.tgz
Don't worry -- I figured it out and was able to reproduce the problem
already. Thanks a lot!
> This is on a quadcore. I recently experimented with "git config
> pack.threads 0" but as it didn't seem to speedup anything I removed
> it again. Just mention it on the infinitesimal chance it might be
> important.
It is not. And the speedup should be noticeable when you repack, not
when you fetch.
Nicolas
^ permalink raw reply
* Re: [PATCH] fix for "index-pack: rationalize delta resolution code"
From: Marco Roeland @ 2008-10-20 19:20 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Junio C Hamano, git, Jeff King
In-Reply-To: <20081020191400.GA18743@fiberbit.xs4all.nl>
Op maandag 20 oktober 2008 om 21:14 uur schreef Marco Roeland het volgende:
> This is on Debian 'sid' with an AMD64 architecture.
>
> I've put the whole ".git" directory (warning: almost 35MB) for
> investigation at:
>
> http://www.xs4all.nl/~fiberbit/http://www.xs4all.nl/~fiberbit/git-next-8f0e41f3-bad-index.tgz
Gah, I can't even copy-and-paste:
http://www.xs4all.nl/~fiberbit/git-next-8f0e41f3-bad-index.tgz
This is on a quadcore. I recently experimented with "git config
pack.threads 0" but as it didn't seem to speedup anything I removed
it again. Just mention it on the infinitesimal chance it might be
important.
--
Marco Roeland
^ permalink raw reply
* Re: [PATCH] fix for "index-pack: rationalize delta resolution code"
From: Marco Roeland @ 2008-10-20 19:36 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Junio C Hamano, git, Jeff King
In-Reply-To: <alpine.LFD.2.00.0810201525540.26244@xanadu.home>
On Monday Oktober 20th 2008 at 15:27 Nicolas Pitre wrote:
> > This is on a quadcore. I recently experimented with "git config
> > pack.threads 0" but as it didn't seem to speedup anything I removed
> > it again. Just mention it on the infinitesimal chance it might be
> > important.
>
> It is not. And the speedup should be noticeable when you repack, not
> when you fetch.
No offense meant! I tried a few "git gc" and "git repack" and only
watched the Gnome CPU applet; perhaps everything was already nicely
packed. I'm certainly going to retry now. Thanks for all your good work.
--
Marco Roeland
^ permalink raw reply
* Re: [PATCH] fix for "index-pack: rationalize delta resolution code"
From: Nicolas Pitre @ 2008-10-20 20:04 UTC (permalink / raw)
To: Marco Roeland; +Cc: Junio C Hamano, git, Jeff King
In-Reply-To: <20081020193652.GA22123@fiberbit.xs4all.nl>
On Mon, 20 Oct 2008, Marco Roeland wrote:
> On Monday Oktober 20th 2008 at 15:27 Nicolas Pitre wrote:
>
> > > This is on a quadcore. I recently experimented with "git config
> > > pack.threads 0" but as it didn't seem to speedup anything I removed
> > > it again. Just mention it on the infinitesimal chance it might be
> > > important.
> >
> > It is not. And the speedup should be noticeable when you repack, not
> > when you fetch.
>
> No offense meant!
Oh certainly not.
> I tried a few "git gc" and "git repack" and only
> watched the Gnome CPU applet; perhaps everything was already nicely
> packed. I'm certainly going to retry now. Thanks for all your good work.
If you want to make the difference really visible, try with
'git repack -a -f --window=100'.
Nicolas
^ permalink raw reply
* Re: [PATCH] fix for "index-pack: rationalize delta resolution code"
From: Jeff King @ 2008-10-20 20:10 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.LFD.2.00.0810201357340.26244@xanadu.home>
On Mon, Oct 20, 2008 at 02:12:04PM -0400, Nicolas Pitre wrote:
> Could anyone having problems fetching from kernel.org with git from the
> next branch confirm that this also fixes that? Thanks.
Nope, this does not fix it for me.
-Peff
^ permalink raw reply
* Re: [PATCH] fix for "index-pack: rationalize delta resolution code"
From: Marco Roeland @ 2008-10-20 20:12 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Junio C Hamano, git, Jeff King
In-Reply-To: <alpine.LFD.2.00.0810201601480.26244@xanadu.home>
On Monday Oktober 20th 2008 at 16:04 Nicolas Pitre wrote:
> If you want to make the difference really visible, try with
> 'git repack -a -f --window=100'.
Impressive, yes. Thanks very much.
marco@sirius:~/src/git (next) $ time git repack -a -f --window=100
Counting objects: 85713, done.
Compressing objects: 100% (84207/84207), done.
Writing objects: 100% (85713/85713), done.
Total 85713 (delta 62371), reused 0 (delta 0)
real 1m2.775s
user 1m1.848s
sys 0m0.176s
marco@sirius:~/src/git (next) $ git config pack.threads 0
marco@sirius:~/src/git (next) $ time git repack -a -f --window=100
Counting objects: 85713, done.
Compressing objects: 100% (84207/84207), done.
Writing objects: 100% (85713/85713), done.
Total 85713 (delta 62363), reused 0 (delta 0)
real 0m21.348s
user 1m2.948s
sys 0m0.432s
marco@sirius:~/src/git (next) $ git config --unset pack.threads
marco@sirius:~/src/git (next) $ time git repack -a -f --window=100
Counting objects: 85713, done.
Compressing objects: 100% (84207/84207), done.
Writing objects: 100% (85713/85713), done.
Total 85713 (delta 62371), reused 0 (delta 0)
real 1m1.904s
user 1m1.476s
sys 0m0.184s
This on Intel(R) Core(TM)2 Quad CPU Q9450 @ 2.66GHz.
--
Marco Roeland
^ permalink raw reply
* [PATCH] fix multiple issues in index-pack
From: Nicolas Pitre @ 2008-10-20 20:46 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Since commit 9441b61dc5, two issues affected correct behavior of
index-pack:
1) The real_type of a delta object is the 'real_type' of its base, not
the 'type' which can be a "delta type". Consequence of this is a
corrupted pack index file which only needs to be recreated with a
good index-pack command ('git verify-pack' will flag those).
2( The code sequence:
result->data = patch_delta(get_base_data(base), base->obj->size,
delta_data, delta_size, &result->size);
has two issues of its own since base->obj->size should instead be
base->size as we want the size of the actual object data and not
the size of the delta object it is represented by. Except that
simply replacing base->obj->size with base->size won't make the
code more correct as the C language doesn't enforce a particular
ordering for the evaluation of needed arguments for a function call,
hence base->size could be pushed on the stack before get_base_data()
which initializes base->size is called.
Signed-off-by: Nicolas Pitre <nico@cam.org>
---
Damn... this one was subtle. And I'm still wondering how the hell the
test suite is able to pass with this. I'll try to figure out why and
come up with better tests.
diff --git a/index-pack.c b/index-pack.c
index 0a917d7..e179bd9 100644
--- a/index-pack.c
+++ b/index-pack.c
@@ -514,15 +514,14 @@ static void *get_base_data(struct base_data *c)
static void resolve_delta(struct object_entry *delta_obj,
struct base_data *base, struct base_data *result)
{
- void *delta_data;
- unsigned long delta_size;
+ void *base_data, *delta_data;
- delta_obj->real_type = base->obj->type;
+ delta_obj->real_type = base->obj->real_type;
delta_data = get_data_from_pack(delta_obj);
- delta_size = delta_obj->size;
+ base_data = get_base_data(base);
result->obj = delta_obj;
- result->data = patch_delta(get_base_data(base), base->obj->size,
- delta_data, delta_size, &result->size);
+ result->data = patch_delta(base_data, base->size,
+ delta_data, delta_obj->size, &result->size);
free(delta_data);
if (!result->data)
bad_object(delta_obj->idx.offset, "failed to apply delta");
^ permalink raw reply related
* Re: [BUG?] Fail to pull from kernel.org: pack has bad object
From: Thomas Rast @ 2008-10-20 20:47 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Johan Herland, Jeff King, git
In-Reply-To: <alpine.LFD.2.00.0810201112360.26244@xanadu.home>
[-- Attachment #1: Type: text/plain, Size: 567 bytes --]
Nicolas Pitre wrote:
> On Mon, 20 Oct 2008, Thomas Rast wrote:
> > http://n.ethz.ch/~trast/download/tmp_pack_NMj69p
>
> Yes, except I can't resolve any of the deltas in there... Is it from
> the Linux kernel repository?
No, that was in my own git.git. Here's the entire repo, if that
context helps. 51M with all temporaries and everything, sorry; I'm
too tired to clean it up. I doubt I have anything to hide in there
however.
http://n.ethz.ch/~trast/download/git-with-broken-pack.tar.bz2
--
Thomas Rast
trast@{inf,student}.ethz.ch
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: [BUG?] Fail to pull from kernel.org: pack has bad object
From: Thomas Rast @ 2008-10-20 20:48 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Johan Herland, Jeff King, git
In-Reply-To: <200810202247.33372.trast@student.ethz.ch>
[-- Attachment #1: Type: text/plain, Size: 369 bytes --]
Thomas Rast wrote:
> No, that was in my own git.git. Here's the entire repo, if that
> context helps. 51M with all temporaries and everything, sorry; I'm
> too tired to clean it up. I doubt I have anything to hide in there
> however.
Obviously I should read all mail before replying. Sorry for the
noise.
--
Thomas Rast
trast@{inf,student}.ethz.ch
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: [BUG?] Fail to pull from kernel.org: pack has bad object
From: Nicolas Pitre @ 2008-10-20 20:51 UTC (permalink / raw)
To: Thomas Rast; +Cc: Johan Herland, Jeff King, git
In-Reply-To: <200810202247.33372.trast@student.ethz.ch>
On Mon, 20 Oct 2008, Thomas Rast wrote:
> Nicolas Pitre wrote:
> > On Mon, 20 Oct 2008, Thomas Rast wrote:
> > > http://n.ethz.ch/~trast/download/tmp_pack_NMj69p
> >
> > Yes, except I can't resolve any of the deltas in there... Is it from
> > the Linux kernel repository?
>
> No, that was in my own git.git. Here's the entire repo, if that
> context helps.
Thanks but I think I already fixed it now. See the latest patch I just
posted to the list.
Nicolas
^ permalink raw reply
* Re: [PATCH] fix multiple issues in index-pack
From: Jeff King @ 2008-10-20 20:56 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.LFD.2.00.0810201609300.26244@xanadu.home>
On Mon, Oct 20, 2008 at 04:46:19PM -0400, Nicolas Pitre wrote:
> 2( The code sequence:
>
> result->data = patch_delta(get_base_data(base), base->obj->size,
> delta_data, delta_size, &result->size);
>
> has two issues of its own since base->obj->size should instead be
> base->size as we want the size of the actual object data and not
> the size of the delta object it is represented by. Except that
This one fixes my problem.
Tested-by: Jeff King <peff@peff.net>
-Peff
^ permalink raw reply
* Re: [PATCH] fix multiple issues in index-pack
From: Marco Roeland @ 2008-10-20 20:59 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.LFD.2.00.0810201609300.26244@xanadu.home>
On Monday October 20th 2008 at 16:46 Nicolas Pitre wrote:
> Since commit 9441b61dc5, two issues affected correct behavior of
> index-pack:
>
> 1) The real_type of a delta object is the 'real_type' of its base, not
> the 'type' which can be a "delta type". Consequence of this is a
> corrupted pack index file which only needs to be recreated with a
> good index-pack command ('git verify-pack' will flag those).
>
> 2( The code sequence:
>
> result->data = patch_delta(get_base_data(base), base->obj->size,
> delta_data, delta_size, &result->size);
>
> has two issues of its own since base->obj->size should instead be
> base->size as we want the size of the actual object data and not
> the size of the delta object it is represented by. Except that
> simply replacing base->obj->size with base->size won't make the
> code more correct as the C language doesn't enforce a particular
> ordering for the evaluation of needed arguments for a function call,
> hence base->size could be pushed on the stack before get_base_data()
> which initializes base->size is called.
>
> Signed-off-by: Nicolas Pitre <nico@cam.org>
This patch works for me. Thanks and good detective work.
--
Marco Roeland
^ permalink raw reply
* [RFC PATCH] builtin-blame: Reencode commit messages according to git-log rules.
From: Alexander Gavrilov @ 2008-10-20 21:24 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
Currently git-blame outputs text from the commit messages
(e.g. the author name and the summary string) as-is, without
even providing any information about the encoding used for
the data. It makes interpreting the data in multilingual
environment very difficult.
This commit changes the blame implementation to recode the
messages using the rules used by other commands like git-log.
Namely, the target encoding can be specified through the
i18n.commitEncoding or i18n.logOutputEncoding options, or
directly on the command line using the --encoding parameter.
Converting the encoding before output seems to be more
friendly to the porcelain tools than simply providing the
value of the encoding header, and does not require changing
the output format.
If anybody needs the old behavior, it is possible to
achieve it by specifying --encoding=none.
Signed-off-by: Alexander Gavrilov <angavrilov@gmail.com>
---
Documentation/blame-options.txt | 7 +++++++
Documentation/i18n.txt | 6 +++---
builtin-blame.c | 16 +++++++++++-----
commit.h | 2 ++
pretty.c | 21 +++++++++++++++------
5 files changed, 38 insertions(+), 14 deletions(-)
diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt
index 5428111..51e7f9f 100644
--- a/Documentation/blame-options.txt
+++ b/Documentation/blame-options.txt
@@ -49,6 +49,13 @@ of lines before or after the line given by <start>.
Show the result incrementally in a format designed for
machine consumption.
+--encoding=<encoding>::
+ Specifies the encoding used to output author names
+ and commit summaries. Setting it to `none` makes blame
+ output unconverted data. For more information see the
+ discussion about encoding in the linkgit:git-log[1]
+ manual page.
+
--contents <file>::
When <rev> is not specified, the command annotates the
changes starting backwards from the working tree copy.
diff --git a/Documentation/i18n.txt b/Documentation/i18n.txt
index d2970f8..2cdacd9 100644
--- a/Documentation/i18n.txt
+++ b/Documentation/i18n.txt
@@ -37,9 +37,9 @@ of `i18n.commitencoding` in its `encoding` header. This is to
help other people who look at them later. Lack of this header
implies that the commit log message is encoded in UTF-8.
-. 'git-log', 'git-show' and friends looks at the `encoding`
- header of a commit object, and tries to re-code the log
- message into UTF-8 unless otherwise specified. You can
+. 'git-log', 'git-show', 'git-blame' and friends look at the
+ `encoding` header of a commit object, and try to re-code the
+ log message into UTF-8 unless otherwise specified. You can
specify the desired output encoding with
`i18n.logoutputencoding` in `.git/config` file, like this:
+
diff --git a/builtin-blame.c b/builtin-blame.c
index 48cc0c1..a806036 100644
--- a/builtin-blame.c
+++ b/builtin-blame.c
@@ -1431,7 +1431,7 @@ static void get_commit_info(struct commit *commit,
int detailed)
{
int len;
- char *tmp, *endp;
+ char *tmp, *endp, *reencoded, *message;
static char author_buf[1024];
static char committer_buf[1024];
static char summary_buf[1024];
@@ -1449,24 +1449,29 @@ static void get_commit_info(struct commit *commit,
die("Cannot read commit %s",
sha1_to_hex(commit->object.sha1));
}
+ reencoded = reencode_commit_message(commit, NULL);
+ message = reencoded ? reencoded : commit->buffer;
ret->author = author_buf;
- get_ac_line(commit->buffer, "\nauthor ",
+ get_ac_line(message, "\nauthor ",
sizeof(author_buf), author_buf, &ret->author_mail,
&ret->author_time, &ret->author_tz);
- if (!detailed)
+ if (!detailed) {
+ free(reencoded);
return;
+ }
ret->committer = committer_buf;
- get_ac_line(commit->buffer, "\ncommitter ",
+ get_ac_line(message, "\ncommitter ",
sizeof(committer_buf), committer_buf, &ret->committer_mail,
&ret->committer_time, &ret->committer_tz);
ret->summary = summary_buf;
- tmp = strstr(commit->buffer, "\n\n");
+ tmp = strstr(message, "\n\n");
if (!tmp) {
error_out:
sprintf(summary_buf, "(%s)", sha1_to_hex(commit->object.sha1));
+ free(reencoded);
return;
}
tmp += 2;
@@ -1478,6 +1483,7 @@ static void get_commit_info(struct commit *commit,
goto error_out;
memcpy(summary_buf, tmp, len);
summary_buf[len] = 0;
+ free(reencoded);
}
/*
diff --git a/commit.h b/commit.h
index 4c05864..c73fb2d 100644
--- a/commit.h
+++ b/commit.h
@@ -65,6 +65,8 @@ enum cmit_fmt {
extern int non_ascii(int);
struct rev_info; /* in revision.h, it circularly uses enum cmit_fmt */
+extern char *reencode_commit_message(const struct commit *commit,
+ const char **encoding_p);
extern void get_commit_format(const char *arg, struct rev_info *);
extern void format_commit_message(const struct commit *commit,
const void *format, struct strbuf *sb,
diff --git a/pretty.c b/pretty.c
index 1e79943..58c8c11 100644
--- a/pretty.c
+++ b/pretty.c
@@ -783,6 +783,20 @@ void pp_remainder(enum cmit_fmt fmt,
}
}
+char *reencode_commit_message(const struct commit *commit, const char **encoding_p)
+{
+ const char *encoding;
+
+ encoding = (git_log_output_encoding
+ ? git_log_output_encoding
+ : git_commit_encoding);
+ if (!encoding)
+ encoding = "utf-8";
+ if (encoding_p)
+ *encoding_p = encoding;
+ return logmsg_reencode(commit, encoding);
+}
+
void pretty_print_commit(enum cmit_fmt fmt, const struct commit *commit,
struct strbuf *sb, int abbrev,
const char *subject, const char *after_subject,
@@ -799,12 +813,7 @@ void pretty_print_commit(enum cmit_fmt fmt, const struct commit *commit,
return;
}
- encoding = (git_log_output_encoding
- ? git_log_output_encoding
- : git_commit_encoding);
- if (!encoding)
- encoding = "utf-8";
- reencoded = logmsg_reencode(commit, encoding);
+ reencoded = reencode_commit_message(commit, &encoding);
if (reencoded) {
msg = reencoded;
}
--
1.6.0.20.g6148bc
^ permalink raw reply related
* [RFC PATCH] commit: Warn about encodings unsupported by iconv.
From: Alexander Gavrilov @ 2008-10-20 21:25 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Paul Mackerras
Currently git-commit and git-commit-tree silently allow
usage of encodings that are unknown to iconv. This may
confuse the user, who then won't be able to use automatic
encoding conversion in git-log and friends without any
immediately obvious reason. Note that the difference
between a supported and an unsupported name may be as
small as CP1251 vs CP-1251, or Shift-JIS vs ShiftJIS.
This commit adds a check for such cases, which produces
a warning similar to the one issued when a message claims
to be utf-8, but actually is not.
Signed-off-by: Alexander Gavrilov <angavrilov@gmail.com>
---
I wonder how common such situation may actually be, and
whether gitk & git-gui (or core git itself) should explicitly
provide some way to deal with it in old commits. I personally
hit it during experimenting, and wrongly concluded that gitk
does not support using multiple encodings in one repository.
Current gitk implementation generally allows working around
it by setting i18n.commitencoding to a valid name of the
encoding used in the mislabeled commits. However, the
downside is that if the selected encoding cannot represent
some characters of an otherwise completely valid commit,
they come out as garbage. Always using --encoding=utf-8
from GUI and relying on conversion done by git-log should
fix this case, but it breaks the workaround.
-- Alexander
builtin-commit-tree.c | 25 +++++++++++++++++++++++--
1 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/builtin-commit-tree.c b/builtin-commit-tree.c
index 0453425..7f325a3 100644
--- a/builtin-commit-tree.c
+++ b/builtin-commit-tree.c
@@ -45,6 +45,28 @@ static const char commit_utf8_warn[] =
"You may want to amend it after fixing the message, or set the config\n"
"variable i18n.commitencoding to the encoding your project uses.\n";
+static const char commit_bad_encoding_warn[] =
+"Warning: commit encoding '%s' is not supported.\n"
+"You may want to change the value of the i18n.commitencoding config\n"
+"variable, and redo the commit. Use 'iconv --list' to see the list of\n"
+"available encoding names.\n";
+
+static void verify_commit_encoding(const char *text, const char *encoding)
+{
+ if (is_encoding_utf8(encoding)) {
+ if (!is_utf8(text))
+ fprintf(stderr, commit_utf8_warn);
+ }
+#ifndef NO_ICONV
+ else {
+ char *conv = reencode_string("", "utf-8", encoding);
+ if (!conv)
+ fprintf(stderr, commit_bad_encoding_warn, encoding);
+ free(conv);
+ }
+#endif
+}
+
int commit_tree(const char *msg, unsigned char *tree,
struct commit_list *parents, unsigned char *ret,
const char *author)
@@ -87,8 +109,7 @@ int commit_tree(const char *msg, unsigned char *tree,
strbuf_addstr(&buffer, msg);
/* And check the encoding */
- if (encoding_is_utf8 && !is_utf8(buffer.buf))
- fprintf(stderr, commit_utf8_warn);
+ verify_commit_encoding(buffer.buf, git_commit_encoding);
result = write_sha1_file(buffer.buf, buffer.len, commit_type, ret);
strbuf_release(&buffer);
--
1.6.0.20.g6148bc
^ permalink raw reply related
* Re: [PATCH] fix multiple issues in index-pack
From: Junio C Hamano @ 2008-10-20 21:31 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: git
In-Reply-To: <alpine.LFD.2.00.0810201609300.26244@xanadu.home>
Nicolas Pitre <nico@cam.org> writes:
> Damn... this one was subtle. And I'm still wondering how the hell the
> test suite is able to pass with this. I'll try to figure out why and
> come up with better tests.
Thanks; much appreciated.
^ permalink raw reply
* Re: Feedback outside of the user survey
From: Junio C Hamano @ 2008-10-20 22:41 UTC (permalink / raw)
To: Christian Jaeger; +Cc: Andreas Ericsson, Garry Dolley, Richard Hartmann, git
In-Reply-To: <48FCB87B.1080207@jaeger.mine.nu>
Christian Jaeger <christian@jaeger.mine.nu> writes:
>> That's partially implemented, I think (google for Nguy (or something, I'm
>> not very god with asian names),
>
> That's not enough information for me to find what you've had in
> mind. "stump Nguy site:marc.info" doesn't yield a result with Google.
I think Andreas is referring to nd/narrow topic currently parked in 'pu'.
$ git log next..36aa66d^2
^ permalink raw reply
* Re: gitk: Turn short SHA1 names into links too
From: Paul Mackerras @ 2008-10-20 23:20 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Git Mailing List
In-Reply-To: <alpine.LFD.1.10.0809251657080.3265@nehalem.linux-foundation.org>
Linus Torvalds writes:
> And the thing I wanted to work was to have the abbreviated SHA1's that
> have started to get more common in the kernel commit logs work as links in
> gitk too, just the way a full 40-character SHA1 link works.
I just pushed out a commit to gitk that makes this work, and fixes the
other bugs you mentioned.
Paul.
^ permalink raw reply
* Re: [GITK PATCH v2] Add menu accelerators
From: Paul Mackerras @ 2008-10-20 23:21 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: git
In-Reply-To: <200810200000.33035.robin.rosenberg@dewire.com>
Robin Rosenberg writes:
> Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
> ---
> gitk | 80 +++++++++++++++++++++++++++++++++---------------------------------
> 1 files changed, 40 insertions(+), 40 deletions(-)
>
> This one is slightly better than the first one. Works with gitk id...id too.
With that patch, I'm finding that pressing Alt-f pops up the File
menu, but also triggers the binding for 'f' in the main window (scroll
diff window to the next file), which is annoying. I'm not sure why, I
will need to track that down.
Paul.
^ permalink raw reply
* Re: [PATCH] rebase-i-p: only list commits that require rewriting in todo
From: Junio C Hamano @ 2008-10-20 23:36 UTC (permalink / raw)
To: Jeff King; +Cc: Stephen Haberman, gitster, git
In-Reply-To: <20081020115003.GA11309@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Wed, Oct 15, 2008 at 02:44:38AM -0500, Stephen Haberman wrote:
>
>> + cat "$TODO" | grep -v "${rev:0:7}" > "${TODO}2" ; mv "${TODO}2" "$TODO"
>
> Substring expansion (like ${rev:0:7}) is not portable. At least it
> doesn't work on FreeBSD /bin/sh, and "it's not even in POSIX", I
> believe.
True.
I do not remember the individual patches in the series, but I have to say
that the script at the tip of the topic is, eh, less than ideal.
Here is a small untested patch to fix a few issues I spotted while reading
it for two minutes.
* Why filter output from "rev-list --left-right A...B" and look for the
ones that begin with ">"? Wouldn't "rev-list A..B" give that?
* The abbreviated SHA-1 are made with "rev-list --abbrev=7" into $TODO in
an earlier invocation, and it can be more than 7 letters to avoid
ambiguity. Not just that "${r:0:7} is not even in POSIX", but use of
it here is actively wrong.
* There is no point in catting a single file and piping it into grep.
git-rebase--interactive.sh | 7 ++++---
1 files changed, 4 insertions(+), 3 deletions(-)
diff --git i/git-rebase--interactive.sh w/git-rebase--interactive.sh
index 848fbe7..a563dea 100755
--- i/git-rebase--interactive.sh
+++ w/git-rebase--interactive.sh
@@ -635,8 +635,8 @@ first and then run 'git rebase --continue' again."
sed -n "s/^>//p" > "$DOTEST"/not-cherry-picks
# Now all commits and note which ones are missing in
# not-cherry-picks and hence being dropped
- git rev-list $UPSTREAM...$HEAD --left-right | \
- sed -n "s/^>//p" | while read rev
+ git rev-list $UPSTREAM..$HEAD |
+ while read rev
do
if test -f "$REWRITTEN"/$rev -a "$(grep "$rev" "$DOTEST"/not-cherry-picks)" = ""
then
@@ -645,7 +645,8 @@ first and then run 'git rebase --continue' again."
# just the history of its first-parent for others that will
# be rebasing on top of it
git rev-list --parents -1 $rev | cut -d' ' -f2 > "$DROPPED"/$rev
- cat "$TODO" | grep -v "${rev:0:7}" > "${TODO}2" ; mv "${TODO}2" "$TODO"
+ short=$(git rev-list -1 --abbrev-commit --abbrev=7 $rev)
+ grep -v "^[a-z][a-z]* $short" <"$TODO" > "${TODO}2" ; mv "${TODO}2" "$TODO"
rm "$REWRITTEN"/$rev
fi
done
^ permalink raw reply related
* Re: [PATCH, RFC] diff: add option to show context between close chunks
From: Junio C Hamano @ 2008-10-20 23:43 UTC (permalink / raw)
To: René Scharfe; +Cc: Git Mailing List, Davide Libenzi
In-Reply-To: <48FB757B.9030105@lsrfire.ath.cx>
René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
> I think it makes sense to make 1, or even 3, the default for this
> option for all commands that create patches intended for human
> consumption. The patch keeps the default at 0, though.
I think defaulting to 1 would make sense, or alternatively, just
hardcoding that behaviour without any new option. That would give you
more information with the same number of patch lines, iow, upside without
any downside.
diff --git a/xdiff/xemit.c b/xdiff/xemit.c
index d3d9c84..3bf2581 100644
--- a/xdiff/xemit.c
+++ b/xdiff/xemit.c
@@ -60,9 +60,9 @@ static int xdl_emit_record(xdfile_t *xdf,
*/
static xdchange_t *xdl_get_hunk(xdchange_t *xscr, xdemitconf_t const *xecfg) {
xdchange_t *xch, *xchp;
for (xchp = xscr, xch = xscr->next; xch; xchp = xch, xch = xch->next)
- if (xch->i1 - (xchp->i1 + xchp->chg1) > 2 * xecfg->ctxlen)
+ if (xch->i1 - (xchp->i1 + xchp->chg1) > 2 * xecfg->ctxlen + 1)
break;
return xchp;
^ permalink raw reply related
* Re: git add --patch newfile doesn't add newfile to cache ?
From: Jeff King @ 2008-10-20 23:50 UTC (permalink / raw)
To: Marc Weber; +Cc: git
In-Reply-To: <20081020143636.GB3988@gmx.de>
On Mon, Oct 20, 2008 at 04:36:36PM +0200, Marc Weber wrote:
> Is this desired behaviour?
> [...]
> git init
> echo test > test
> git add --patch test
> echo "running status, nothing has been added"
> git status
I think your example makes sense, but nobody ever really tried it
before. I use "git add -p" all the time, but almost always when I am
adding a new file, I add the whole contents.
I think there are two ways to go about fixing it:
- in git-add--interactive.perl, the function patch_update_cmd
explicitly looks at the list of modified files. It would have to
also check for untracked files, which is easy. But we also need to
keep track of which files are modified and which are untracked
through the whole patching procedure, which is a bit more invasive.
- the recently-added "git add -N" adds an empty file into the index,
at which point we could add content in the normal way. So:
git add -N test
git add -p test
should just work (but obviously requires two steps from the user).
You could do something more automatic like the patch below, but I
think the semantics aren't quite right. If you stage nothing for a
newly added file, then you still end up with an empty version of the
staged file in the index. I would expect the semantics to be:
1. if you stage any content, then the file is added to the index
with that content
2. if you stage no content, then the file remains untracked
---
diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index da768ee..72f8a67 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -811,6 +811,12 @@ EOF
}
sub patch_update_cmd {
+ my @new = list_untracked();
+ if (@new) {
+ system(qw(git add -N), @new)
+ and die "git add reported failure";
+ }
+
my @mods = grep { !($_->{BINARY}) } list_modified('file-only');
my @them;
^ permalink raw reply related
* Re: [PATCH] Teach/Fix -q/-v to pull/fetch
From: Junio C Hamano @ 2008-10-20 23:52 UTC (permalink / raw)
To: Tuncer Ayaz; +Cc: git
In-Reply-To: <4ac8254d0810191133v79ed73b7tf09a282f44d302dd@mail.gmail.com>
"Tuncer Ayaz" <tuncer.ayaz@gmail.com> writes:
> it doesn't work as I expected when you supply -q and -v.
> I have to redo it, I guess.
Probably adding a few new tests to one of the test scripts in t/ is in
order, then.
^ permalink raw reply
* Re: [PATCH] Teach/Fix pull/fetch -q/-v options
From: Junio C Hamano @ 2008-10-20 23:54 UTC (permalink / raw)
To: Tuncer Ayaz; +Cc: Junio C Hamano, git
In-Reply-To: <4ac8254d0810200935sf7ad873tea53c0fb53bbe1c0@mail.gmail.com>
"Tuncer Ayaz" <tuncer.ayaz@gmail.com> writes:
> On Sun, Oct 19, 2008 at 11:26 PM, Junio C Hamano <gitster@pobox.com> wrote:
> ...
>>> @@ -23,6 +24,10 @@ rebase=$(git config --bool branch.$curr_branch_short.rebase)
>>> while :
>>> do
>>> case "$1" in
>>> + -q|--quiet)
>>> + verbosity="$verbosity -q" ;;
>>> + -v|--verbose)
>>> + verbosity="$verbosity -v" ;;
>>
>> You know verbosity flags (-q and -v) are "the last one wins", so I do not
>> see much point in this concatenation.
>
> Without concatenation I would need to analyze the content
> of the variable each time the option is passed to the shell
> script. Do you know of a simpler/better way still keeping the
> functionality that
> $ git pull -q -v --quiet --verbose --quiet gives verbosity=QUIET
> and
> $ git pull -q -v --quiet --verbose --quiet -v yields verbosity=VERBOSE
> ?
Wouldn't
verbosity=
while :
do
case "$1" in
-q|--quiet) verbosity=-q ;;
-v|--verbose) verbosity=-v ;;
... others ...
esac
shift
done
git pull $verbosity other options
give the -q for the former and -v for the latter to "git pull"?
^ 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