* Re: bug: git-stash save and symbolic links
From: Jeff King @ 2008-10-20 17:55 UTC (permalink / raw)
To: Simon Strandgaard; +Cc: git
In-Reply-To: <df1390cc0810200839q5eddad1cp4bc14762724d8848@mail.gmail.com>
On Mon, Oct 20, 2008 at 05:39:31PM +0200, Simon Strandgaard wrote:
> mkdir test &&
> echo content >test/file2 &&
> git add test/file2 &&
> git commit -m two &&
> cd .. &&
> ln -s repo/test linked &&
Ah, OK. You have a symlink into a subdirectory of the repository. Git
correctly finds the repository's .git directory, but then when we
attempt to 'cd' to the top-level of the working tree, we end up outside
of the repository. The culprit is actually "git rev-parse --show-cdup".
This demonstrates it more simply:
mkdir repo &&
(cd repo && git init) &&
mkdir repo/dir &&
ln -s repo/dir linked &&
cd linked && git rev-parse --show-cdup
which prints "../". But that's not right; we actually need to go to
"../repo".
I think the right solution in this situation is that we should _not_ be
setting is_inside_work_tree in setup_git_directory_gently, so that we
chdir to its absolute path. But I don't think we ever actually detect
the symlink during this setup, so I'm not sure how best to realize we
are _in_ this situation.
-Peff
^ permalink raw reply
* Re: [BUG?] Fail to pull from kernel.org: pack has bad object
From: Jeff King @ 2008-10-20 17:56 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Thomas Rast, Johan Herland, git
In-Reply-To: <alpine.LFD.2.00.0810201112360.26244@xanadu.home>
On Mon, Oct 20, 2008 at 01:49:33PM -0400, Nicolas Pitre 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?
I don't know about Thomas's example, but this one is from git.git:
http://peff.net/tmp_pack_c7KnC6
-Peff
^ permalink raw reply
* Re: [PATCH, RFC] diff: add option to show context between close chunks
From: René Scharfe @ 2008-10-20 18:06 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Git Mailing List, Davide Libenzi
In-Reply-To: <48FC9685.8030704@viscovery.net>
Johannes Sixt schrieb:
> Why can't you just use -U6 instead instead of --inter-chunk-context=3? If
> this is intended for human consumption anyway, then you can just as well
> increase the overall number of context lines: You get extra context lines
> in the places where hunks are not fused, but this cannot be a disadvantage
> for the targeted audience.
The extra lines _are_ a disadvantage for me, since each chunk gets them
while only close ones need them -- that's wasted space. The intent is
to save time by not having to apply and rediff a patch on a mailing list
in order to see the hidden text; part of the savings would be lost if
the reader had to skip over extra lines.
Besides, this option is probably most useful if set by default on
machines not controlled by me. E.g. I wouldn't want to change an option
on a gitweb page, I'd want to get fused chunks by default. I wouldn't
want to write "best viewed with -U6" in a patch description, I'd want it
to just work (e.g. for gitk users).
I have to admit my main motivation was that one line gap, where a chunk
header hid an interesting line of context. Showing it didn't change the
length of the patch, so I found this to be a sad wastage.
Optimizing patches for readability makes sense since they are such an
important part of our communication here. I think --i-c-c=1 results in
an obvious win without much of a downside. I was surprised to find that
almost 4% of the chunks in git 1.6.0 would be eliminated by that option:
$ git log -p v1.6.0 | grep -c '^@@ '
60544
$ git log -p v1.6.0 --inter-chunk-context=1 | grep -c '^@@ '
58471
But perhaps a higher value would be even better?
$ git log v1.6.0 | wc -l
225441
$ git log -p v1.6.0 | wc -l
1736188
$ git log -p --inter-chunk-context=10 v1.6.0 | wc -l
1779048
$ git log -p -U8 v1.6.0 | wc -l
2214448
Well, I don't know if those patches are easier to read (haven't looked
at them), but I imagine that some related changes are presented with the
full context between them (e.g. changes to loop header and footer,
function signature and body). The numbers only show that it's
affordable (3% more lines with -i-c-c=10, 30% more lines with the
comparable -U8).
(Why 10? With the default of -U3, a chunk is 6 lines of context plus at
least one line of actual change. Two chunks are at least 14 lines long,
thus only 10 more fit into 24 lines, i.e. a terminal window..)
René
^ permalink raw reply
* [PATCH] fix for "index-pack: rationalize delta resolution code"
From: Nicolas Pitre @ 2008-10-20 18:12 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jeff King
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.
diff --git a/index-pack.c b/index-pack.c
index 0a917d7..8287ebf 100644
--- a/index-pack.c
+++ b/index-pack.c
@@ -517,7 +517,7 @@ static void resolve_delta(struct object_entry *delta_obj,
void *delta_data;
unsigned long delta_size;
- 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;
result->obj = delta_obj;
^ permalink raw reply related
* How to push to http(s) repository with authentication?
From: Josef Wolf @ 2008-10-20 18:25 UTC (permalink / raw)
To: git
Hello,
I have set up a repository as described in
http://www.kernel.org/pub/software/scm/git/docs/howto/setup-git-server-over-http.txt
over SSL with basic authentication. DAV access works fine with konqueror,
cadaver and and curl, using this .curlrc:
$ cat ~/.curlrc
--cacert /etc/cacerts/myca.pem
--user user
$ curl https://repo.host.org/git/test/HEAD
Enter host password for user 'user':
ref: refs/heads/master
$
But when called from git, this file seems to be ignored. I have checked
google, but the only hint I could find was to put credentials into
~/.netrc. IMHO, this is a bad workaround, since this implies to have
unencrypted passwords lying around on the disk.
Any hints?
PS: I have asked a similar question a couple of days ago. Here is a link
to my original mail with more information how I set up the server:
http://marc.info/?l=git&m=122426078301793&w=2
^ permalink raw reply
* Re: [PATCH] fix for "index-pack: rationalize delta resolution code"
From: Harvey Harrison @ 2008-10-20 18:37 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 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>
^ permalink raw reply
* Re: [PATCH] git-fetch should not strip off ".git" extension
From: Junio C Hamano @ 2008-10-20 18:37 UTC (permalink / raw)
To: Andreas Ericsson; +Cc: SLONIK.AZ, git
In-Reply-To: <48FC5F1B.1050608@op5.se>
Andreas Ericsson <ae@op5.se> writes:
>>...
>>
>> builtin-fetch--tool.c | 2 --
>> builtin-fetch.c | 2 --
>> 2 files changed, 0 insertions(+), 4 deletions(-)
>>
>> diff --git a/builtin-fetch--tool.c b/builtin-fetch--tool.c
>> index 7460ab7..5d0b95f 100644
>> --- a/builtin-fetch--tool.c
>> +++ b/builtin-fetch--tool.c
>> @@ -160,8 +160,6 @@ static int append_fetch_head(FILE *fp,
>> for (i = remote_len - 1; remote[i] == '/' && 0 <= i; i--)
>> ;
>> remote_len = i + 1;
>> - if (4 < i && !strncmp(".git", remote + i - 3, 4))
>> - remote_len = i - 3;
>>
>> note_len = 0;
>> if (*what) {
>> diff --git a/builtin-fetch.c b/builtin-fetch.c
>> index ee93d3a..28123a5 100644
>> --- a/builtin-fetch.c
>> +++ b/builtin-fetch.c
>> @@ -348,8 +348,6 @@ static int store_updated_refs(const char *url,
>> const char *remote_name,
>> for (i = url_len - 1; url[i] == '/' && 0 <= i; i--)
>> ;
>> url_len = i + 1;
>> - if (4 < i && !strncmp(".git", url + i - 3, 4))
>> - url_len = i - 3;
>>
>
> Will this still play nicely with
>
> git clone foo.git
>
> ?
I think it would.
As far as I can tell, the only thing the patch changes is to disable the
long established "repository name clean-up" feature in the autogenerated
merge messages (iow, input to "fmt-merge-msg").
^ permalink raw reply
* [EGIT PATCH] git property page for project properties.
From: Tomi Pakarinen @ 2008-10-20 19:06 UTC (permalink / raw)
To: spearce; +Cc: git, Tomi Pakarinen
Show git property page in project's properties, if project
has git repository provider.
Signed-off-by: Tomi Pakarinen <tomi.pakarinen@iki.fi>
---
This is similar to CVS's property page. Current
information on page is very limited and non editable.
Tomi.
org.spearce.egit.ui/plugin.properties | 2 +
org.spearce.egit.ui/plugin.xml | 13 ++
.../preferences/GitProjectPropertyPage.java | 147 ++++++++++++++++++++
3 files changed, 162 insertions(+), 0 deletions(-)
create mode 100644 org.spearce.egit.ui/src/org/spearce/egit/ui/internal/preferences/GitProjectPropertyPage.java
diff --git a/org.spearce.egit.ui/plugin.properties b/org.spearce.egit.ui/plugin.properties
index da91e48..fa043f1 100644
--- a/org.spearce.egit.ui/plugin.properties
+++ b/org.spearce.egit.ui/plugin.properties
@@ -1,6 +1,8 @@
plugin_name=Git Team Provider (UI)
provider_name=spearce.org
+Git=Git
+
Git_clone_category=Git
Git_clone_wizard=Git Repository
Git_clone_description=Clone an existing Git repository.
diff --git a/org.spearce.egit.ui/plugin.xml b/org.spearce.egit.ui/plugin.xml
index ee8a5a0..e6e3762 100644
--- a/org.spearce.egit.ui/plugin.xml
+++ b/org.spearce.egit.ui/plugin.xml
@@ -194,6 +194,19 @@
id="org.spearce.egit.ui.internal.preferences.WindowCachePreferencePage" />
</extension>
+ <extension point="org.eclipse.ui.propertyPages">
+ <page
+ objectClass="org.eclipse.core.resources.IProject"
+ adaptable="true"
+ name="%Git"
+ class="org.spearce.egit.ui.internal.preferences.GitProjectPropertyPage"
+ id="org.spearce.egit.ui.preferences.GitProjectPropertyPage">
+ <filter
+ name="projectPersistentProperty"
+ value="org.eclipse.team.core.repository=org.spearce.egit.core.GitProvider">
+ </filter>
+ </page>
+ </extension>
<extension point="org.eclipse.ui.decorators">
<decorator
lightweight="true"
diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/preferences/GitProjectPropertyPage.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/preferences/GitProjectPropertyPage.java
new file mode 100644
index 0000000..fd7e14a
--- /dev/null
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/preferences/GitProjectPropertyPage.java
@@ -0,0 +1,147 @@
+/*******************************************************************************
+ * Copyright (C) 2008, Tomi Pakarinen <tomi.pakarinen@iki.fi>
+ *
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * See LICENSE for the full license text, also available.
+ *******************************************************************************/
+package org.spearce.egit.ui.internal.preferences;
+
+import java.io.IOException;
+
+import org.eclipse.core.resources.IProject;
+import org.eclipse.core.runtime.IAdaptable;
+import org.eclipse.swt.SWT;
+import org.eclipse.swt.layout.GridData;
+import org.eclipse.swt.layout.GridLayout;
+import org.eclipse.swt.widgets.Composite;
+import org.eclipse.swt.widgets.Control;
+import org.eclipse.swt.widgets.Display;
+import org.eclipse.swt.widgets.Label;
+import org.eclipse.swt.widgets.Text;
+import org.eclipse.ui.dialogs.PropertyPage;
+import org.spearce.egit.core.project.RepositoryMapping;
+import org.spearce.jgit.lib.ObjectId;
+import org.spearce.jgit.lib.Repository;
+
+/**
+ * Property page to be shown in project properties, if project is shared using
+ * git provider. Currently there aren't any modifiable element.
+ */
+public class GitProjectPropertyPage extends PropertyPage {
+
+ Text gitDir;
+
+ Text branch;
+
+ Text id;
+
+ Text state;
+
+ Text workDir;
+
+ @Override
+ protected Control createContents(Composite parent) {
+ final Composite composite = new Composite(parent, SWT.NULL);
+
+ composite.setLayoutData(new GridData(SWT.FILL, SWT.FILL, true, true));
+ final GridLayout layout = new GridLayout();
+ layout.numColumns = 2;
+ layout.marginHeight = 0;
+ layout.marginWidth = 0;
+ layout.horizontalSpacing = 0;
+ layout.verticalSpacing = 0;
+ composite.setLayout(layout);
+
+ gitDir = createLabeledReadOnlyText(composite, "Git directory:");
+ workDir = createLabeledReadOnlyText(composite, "Working directory:");
+ branch = createLabeledReadOnlyText(composite, "Branch:");
+ id = createLabeledReadOnlyText(composite, "Id:");
+ state = createLabeledReadOnlyText(composite, "Current state:");
+
+ // Get the project that is the source of this property page
+ IProject project = null;
+ final IAdaptable element = getElement();
+ if (element instanceof IProject) {
+ project = (IProject) element;
+ } else {
+ Object adapter = element.getAdapter(IProject.class);
+ if (adapter instanceof IProject) {
+ project = (IProject) adapter;
+ }
+ }
+
+ Repository repository = RepositoryMapping.getMapping(project)
+ .getRepository();
+
+ if (repository != null) {
+ try {
+ fillValues(repository);
+ } catch (IOException e) {
+ e.printStackTrace();
+ }
+ }
+
+ return composite;
+ }
+
+ private void fillValues(Repository repository) throws IOException {
+ gitDir.setText(repository.getDirectory().getAbsolutePath());
+ branch.setText(repository.getBranch());
+ workDir.setText(repository.getWorkDir().getAbsolutePath());
+
+ switch (repository.getRepositoryState()) {
+ case BISECTING:
+ state.setText("Bisecting");
+ break;
+ case MERGING:
+ state.setText("Merging");
+ break;
+ case REBASING:
+ state.setText("Rebasing");
+ break;
+ case REBASING_INTERACTIVE:
+ state.setText("Rebasing interactive");
+ break;
+ case REBASING_MERGE:
+ state.setText("Rebasing merge");
+ break;
+ case SAFE:
+ state.setText("Safe");
+ break;
+ }
+
+ final ObjectId objectId = repository
+ .resolve(repository.getFullBranch());
+ id.setText(objectId.name());
+ }
+
+ /**
+ * Create a read only text field with a label
+ *
+ * @param parent
+ * the parent composite for new widgets
+ * @param labelText
+ * text for label
+ * @return the new read only text field
+ */
+ protected Text createLabeledReadOnlyText(Composite parent,
+ final String labelText) {
+ Label label = new Label(parent, SWT.LEFT);
+ label.setText(labelText);
+ GridData data = new GridData();
+ data.horizontalSpan = 1;
+ data.horizontalAlignment = GridData.FILL;
+ label.setLayoutData(data);
+
+ Text text = new Text(parent, SWT.LEFT | SWT.READ_ONLY);
+ text.setBackground(Display.getDefault().getSystemColor(
+ SWT.COLOR_WIDGET_BACKGROUND));
+ data = new GridData();
+ data.horizontalSpan = 1;
+ data.horizontalAlignment = GridData.FILL;
+ text.setLayoutData(data);
+ return text;
+ }
+
+}
--
1.6.0.2
^ permalink raw reply related
* 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
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