* [Security] gitweb local privilege escalation (fix)
From: Junio C Hamano @ 2008-12-20 6:46 UTC (permalink / raw)
To: git; +Cc: linux-kernel
In-Reply-To: <7vljub1h92.fsf@gitster.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 1050 bytes --]
Current gitweb has a possible local privilege escalation bug that allows a
malicious repository owner to run a command of his choice by specifying
diff.external configuration variable in his repository and running a
crafted gitweb query.
Recent (post 1.4.3) gitweb itself never generates a link that would result
in such a query, and the safest and cleanest fix to this issue is to
simply drop the support for it. Maintenance release v1.6.0.6, v1.5.6.6,
v1.5.5.6 and v1.5.4.7 are already available at k.org (see the announcement
for v1.6.0.6 I sent out a few minutes ago), and the master branch and
others pushed out tonight have the same fix.
This message contains two patches (credits go to Matt McCutchen, Jeff King
and Jakub Narebski) to do the fix yourself:
(1) for Git 1.5.4.X, 1.5.5.X, and 1.5.6.X, and
(2) for Git 1.6.0.X.
Distro packagers and people on the vendor security list have been notified
about this fix earlier this week; people running gitweb from vendor
supplied binaries should be able to get updates from them as well.
[-- Attachment #2: gitweb hotfix for 1.5.[456].X series --]
[-- Type: text/plain, Size: 2313 bytes --]
>From dfff4b7aa42de7e7d58caeebe2c6128449f09b76 Mon Sep 17 00:00:00 2001
From: Junio C Hamano <gitster@pobox.com>
Date: Tue, 16 Dec 2008 19:42:02 -0800
Subject: [PATCH] gitweb: do not run "git diff" that is Porcelain
Jakub says that legacy-style URI to view two blob differences are never
generated since 1.4.3. This codepath runs "git diff" Porcelain from the
gitweb, which is a no-no. It can trigger diff.external command that is
specified in the configuration file of the repository being viewed.
This patch applies to v1.5.4 and later.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
gitweb/gitweb.perl | 38 ++------------------------------------
1 files changed, 2 insertions(+), 36 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index b582332..86a6ced 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -4809,43 +4809,9 @@ sub git_blobdiff {
or die_error(undef, "Open git-diff-tree failed");
}
- # old/legacy style URI
- if (!%diffinfo && # if new style URI failed
- defined $hash && defined $hash_parent) {
- # fake git-diff-tree raw output
- $diffinfo{'from_mode'} = $diffinfo{'to_mode'} = "blob";
- $diffinfo{'from_id'} = $hash_parent;
- $diffinfo{'to_id'} = $hash;
- if (defined $file_name) {
- if (defined $file_parent) {
- $diffinfo{'status'} = '2';
- $diffinfo{'from_file'} = $file_parent;
- $diffinfo{'to_file'} = $file_name;
- } else { # assume not renamed
- $diffinfo{'status'} = '1';
- $diffinfo{'from_file'} = $file_name;
- $diffinfo{'to_file'} = $file_name;
- }
- } else { # no filename given
- $diffinfo{'status'} = '2';
- $diffinfo{'from_file'} = $hash_parent;
- $diffinfo{'to_file'} = $hash;
- }
-
- # non-textual hash id's can be cached
- if ($hash =~ m/^[0-9a-fA-F]{40}$/ &&
- $hash_parent =~ m/^[0-9a-fA-F]{40}$/) {
- $expires = '+1d';
- }
-
- # open patch output
- open $fd, "-|", git_cmd(), "diff", @diff_opts,
- '-p', ($format eq 'html' ? "--full-index" : ()),
- $hash_parent, $hash, "--"
- or die_error(undef, "Open git-diff failed");
- } else {
+ # old/legacy style URI -- not generated anymore since 1.4.3.
+ if (!%diffinfo) {
die_error('404 Not Found', "Missing one of the blob diff parameters")
- unless %diffinfo;
}
# header
--
1.6.1.rc3.19.g66a9
[-- Attachment #3: gitweb hotfix for 1.6.0.X series --]
[-- Type: text/plain, Size: 1630 bytes --]
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index ced7bb7..804670c 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -4863,43 +4863,9 @@ sub git_blobdiff {
or die_error(500, "Open git-diff-tree failed");
}
- # old/legacy style URI
- if (!%diffinfo && # if new style URI failed
- defined $hash && defined $hash_parent) {
- # fake git-diff-tree raw output
- $diffinfo{'from_mode'} = $diffinfo{'to_mode'} = "blob";
- $diffinfo{'from_id'} = $hash_parent;
- $diffinfo{'to_id'} = $hash;
- if (defined $file_name) {
- if (defined $file_parent) {
- $diffinfo{'status'} = '2';
- $diffinfo{'from_file'} = $file_parent;
- $diffinfo{'to_file'} = $file_name;
- } else { # assume not renamed
- $diffinfo{'status'} = '1';
- $diffinfo{'from_file'} = $file_name;
- $diffinfo{'to_file'} = $file_name;
- }
- } else { # no filename given
- $diffinfo{'status'} = '2';
- $diffinfo{'from_file'} = $hash_parent;
- $diffinfo{'to_file'} = $hash;
- }
-
- # non-textual hash id's can be cached
- if ($hash =~ m/^[0-9a-fA-F]{40}$/ &&
- $hash_parent =~ m/^[0-9a-fA-F]{40}$/) {
- $expires = '+1d';
- }
-
- # open patch output
- open $fd, "-|", git_cmd(), "diff", @diff_opts,
- '-p', ($format eq 'html' ? "--full-index" : ()),
- $hash_parent, $hash, "--"
- or die_error(500, "Open git-diff failed");
- } else {
- die_error(400, "Missing one of the blob diff parameters")
- unless %diffinfo;
+ # old/legacy style URI -- not generated anymore since 1.4.3.
+ if (!%diffinfo) {
+ die_error('404 Not Found', "Missing one of the blob diff parameters")
}
# header
^ permalink raw reply related
* [ANNOUNCE] GIT 1.6.0.6
From: Junio C Hamano @ 2008-12-20 6:40 UTC (permalink / raw)
To: git; +Cc: linux-kernel
The latest maintenance release GIT 1.6.0.6 is available at the
usual places:
http://www.kernel.org/pub/software/scm/git/
git-1.6.0.6.tar.{gz,bz2} (source tarball)
git-htmldocs-1.6.0.6.tar.{gz,bz2} (preformatted docs)
git-manpages-1.6.0.6.tar.{gz,bz2} (preformatted docs)
The RPM binary packages for a few architectures are also provided
as courtesy.
RPMS/$arch/git-*-1.6.0.6-1.fc9.$arch.rpm (RPM)
Among miscellaneous fixes, this contains a local gitweb security fix.
Maintenance releases for older versions (v1.5.4.7, v1.5.5.6 and v1.5.6.6)
are also available at the same place.
----------------------------------------------------------------
Changes since v1.6.0.5 are as follows:
David Aguilar (1):
git-mergetool: properly handle "git mergetool -- filename"
Deskin Miller (1):
git-svn: Make following parents atomic
Jim Meyering (1):
git-config.txt: fix a typo
Johannes Schindelin (2):
fast-import: close pack before unlinking it
fast-export: deal with tag objects that do not have a tagger
Junio C Hamano (6):
work around Python warnings from AsciiDoc
git-show: do not segfault when showing a bad tag
pager: do not dup2 stderr if it is already redirected
gitweb: do not run "git diff" that is Porcelain
GIT 1.5.4.7
fast-import: make tagger information optional
Linus Torvalds (1):
fsck: reduce stack footprint
Markus Heidelberg (1):
Documentation: fix typos, grammar, asciidoc syntax
Miklos Vajna (1):
SubmittingPatches: mention the usage of real name in Signed-off-by: lines
Nicolas Pitre (1):
make sure packs to be replaced are closed beforehand
Wu Fengguang (1):
git-send-email: handle email address with quoted comma
^ permalink raw reply
* [PATCH] Remove the requirement opaquelocktoken uri scheme
From: Kirill A. Korinskiy @ 2008-12-20 6:19 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, catap
In-Reply-To: <7vljucbows.fsf@gitster.siamese.dyndns.org>
The program flow of pushing over http is:
- call lock_remote() to issue a DAV_LOCK request to the server to lock
info/refs and branch refs being pushed into; handle_new_lock_ctx() is
used to parse its response to populate "struct remote_lock" that is
returned from lock_remote();
- send objects;
- call unlock_remote() to drop the lock.
The handle_new_lock_ctx() function assumed that the server will use a
lock token in opaquelocktoken URI scheme, which may have been an Ok
assumption under RFC 2518, but under RFC 4918 which obsoletes the older
standard it is not necessarily true.
This resulted in push failure (often resulted in "cannot lock existing
info/refs" error message) when talking to a server that does not use
opaquelocktoken URI scheme.
Signed-off-by: Kirill A. Korinskiy <catap@catap.ru>
---
http-push.c | 14 ++++++--------
1 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/http-push.c b/http-push.c
index 5cecef434a7740a3f853462978c3e071b4da7e74..7c6460919bf3eba10c46cede11ffdd9c53fd2dd2 100644
--- a/http-push.c
+++ b/http-push.c
@@ -595,7 +595,7 @@ static int refresh_lock(struct remote_lock *lock)
lock->refreshing = 1;
if_header = xmalloc(strlen(lock->token) + 25);
- sprintf(if_header, "If: (<opaquelocktoken:%s>)", lock->token);
+ sprintf(if_header, "If: (<%s>)", lock->token);
sprintf(timeout_header, "Timeout: Second-%ld", lock->timeout);
dav_headers = curl_slist_append(dav_headers, if_header);
dav_headers = curl_slist_append(dav_headers, timeout_header);
@@ -1120,10 +1120,8 @@ static void handle_new_lock_ctx(struct xml_ctx *ctx, int tag_closed)
lock->timeout =
strtol(ctx->cdata + 7, NULL, 10);
} else if (!strcmp(ctx->name, DAV_ACTIVELOCK_TOKEN)) {
- if (!prefixcmp(ctx->cdata, "opaquelocktoken:")) {
- lock->token = xmalloc(strlen(ctx->cdata) - 15);
- strcpy(lock->token, ctx->cdata + 16);
- }
+ lock->token = xmalloc(strlen(ctx->cdata) + 1);
+ strcpy(lock->token, ctx->cdata);
}
}
}
@@ -1308,7 +1306,7 @@ static int unlock_remote(struct remote_lock *lock)
int rc = 0;
lock_token_header = xmalloc(strlen(lock->token) + 31);
- sprintf(lock_token_header, "Lock-Token: <opaquelocktoken:%s>",
+ sprintf(lock_token_header, "Lock-Token: <%s>",
lock->token);
dav_headers = curl_slist_append(dav_headers, lock_token_header);
@@ -1722,7 +1720,7 @@ static int update_remote(unsigned char *sha1, struct remote_lock *lock)
struct curl_slist *dav_headers = NULL;
if_header = xmalloc(strlen(lock->token) + 25);
- sprintf(if_header, "If: (<opaquelocktoken:%s>)", lock->token);
+ sprintf(if_header, "If: (<%s>)", lock->token);
dav_headers = curl_slist_append(dav_headers, if_header);
strbuf_addf(&out_buffer.buf, "%s\n", sha1_to_hex(sha1));
@@ -1941,7 +1939,7 @@ static void update_remote_info_refs(struct remote_lock *lock)
add_remote_info_ref, &buffer.buf);
if (!aborted) {
if_header = xmalloc(strlen(lock->token) + 25);
- sprintf(if_header, "If: (<opaquelocktoken:%s>)", lock->token);
+ sprintf(if_header, "If: (<%s>)", lock->token);
dav_headers = curl_slist_append(dav_headers, if_header);
slot = get_active_slot();
--
1.5.6.5
^ permalink raw reply related
* Re: Git Notes idea.
From: Jeff King @ 2008-12-20 4:54 UTC (permalink / raw)
To: Govind Salinas; +Cc: Johannes Schindelin, Git Mailing List
In-Reply-To: <5d46db230812191424m14e82c5fx1c1c12027db901ed@mail.gmail.com>
On Fri, Dec 19, 2008 at 04:24:01PM -0600, Govind Salinas wrote:
> > I think so. Otherwise how will you push and pull notes? You won't even
> > know which one is the more recent tree, let alone handle any merges
> > caused by editing notes in two places.
>
> Couldn't you simply merge your tree and theirs even if there is no
> history. You would have to find a way to handle merges in any event
> since they could just as easily happen if you have a history.
Let's say I have a tree T1 like this:
$COMMIT_A -> $BLOB_A
$COMMIT_B -> $BLOB_B1
and a tree T2 like this:
$COMMIT_B -> $BLOB_B2
$COMMIT_C -> $BLOB_C
what is the correct merge? Was $COMMIT_A added in T1, or deleted in T2?
How about $COMMIT_C? Even if you went with a strategy like "always add
from both" (which I don't think is a good idea, because deleted notes
will keep popping back up) you have a conflict with $COMMIT_B. Should
it be B1 or B2? You can't tell if B1 became B2, vice versa, or if there
is a true merge conflict.
> > If the former, then you haven't solved the cruft accumulation problem.
> > You can get obsolete notes in your note history by rebasing on a branch
> > that is long-running (which is OK as long as you haven't published
> > _those particular_ commits). Or are you proposing to rebase and cleanup
> > the notes history every time you do a destructive operation?
>
> Yes, it does not solve that problem. But it does solve things like
>
> Dev1 and Dev2 both have branches A and topic branch B. and they
> are in refs/notes/public (or refs/notes or something not branch specific).
>
> Dev1 adds 100 notes to topic B, lets say half of them are obsolete due
> to rebases or whatever. Dev2 pulls A and updates their notes
> as well. Now Dev2 has acquired all the notes from Dev1 including the
> obsolete ones. So you have 100 commits, 100 blobs and all the new
> trees that go with them that the user was not interested in.
>
> Run this across 1000 users and you have a lot of cruft.
>
> Now, if instead we have a per-branch notes scheme, then you only get
> the cruft from the branches you were interested in. If you remove the
> history you could end up with no cruft because gc should handle it.
OK. But my point is that this is an incomplete solution. You can _still_
get cruft, and you _still_ have to deal with that cruft some other way.
So we will still end up having to implement something else. And I might
even be fine with a partial solution that helped some if it didn't come
with a cost, but I think the "notes stick to branches" behavior is
strictly worse.
> > If the latter, then I don't see how you've solved the push-pull and
> > merge problem (which you need history for).
>
> What git-fetch would have to do is say. This is a note. The remote
> sha is not the same as mine, i will treat this as a force and fetch the
> objects without checking history and then run a merge on the 2
> commits. The notes merge could have its own strategy that checked
> if an object exists before deciding to add a new item or delete a
> removed one. Then the user would only have to intervene if the
> notes where edited.
I don't like that because:
- the user is going to end up manually resolving merge conflicts for
things that _should_ have been fast forwards. But much worse, it's
going to be on content they may never even have seen before. How
will they decide which is which?
- how do you push notes? There's no opportunity to handle the merge
on the remote side. And you can't just pull, merge locally, and push
what is now a fast-forward, because there is no concept of
fast-forward without history.
- Suddenly pulling and pushing notes isn't just taken care of by the
usual ref transfer mechanisms. We have to implement a whole new
system.
> You are correct of course that it will just be wasted space. But I am
> concerned that it could end up being a lot of wasted space. I mean, what
> if every person who contributed to the kernel contributed note cruft. Users
What if every person who contributed to the kernel contributed history
cruft? It's really the same problem, and it is solved by people keeping
their trees clean (via rebase) and being picky about how data comes into
your tree (i.e., don't pull from people with cruft). I suspect Linus
wouldn't pull notes at all (and they wouldn't make it over patch
transmission anyway). But in a workflow that is pulling the notes, the
right time to clean up history is probably before publishing. That is,
you can rebase and clean up your notes history just before you push it
to somewhere public, just like you might clean up messy history.
> If you *really* don't think its something to be worried about then I
> am OK with that since you have a lot more experience with this, but it
> sounds hairy to me.
It is hairy, and I wish there were a better solution. But I think every
other option is much worse.
-Peff
^ permalink raw reply
* Re: [PATCH][RESEND] guilt: add option guilt.diffstat
From: Josef Jeff Sipek @ 2008-12-20 4:33 UTC (permalink / raw)
To: Wu Fengguang; +Cc: git@vger.kernel.org, Boyd Stephen Smith Jr.
In-Reply-To: <20081218112643.GA15416@localhost>
On Thu, Dec 18, 2008 at 07:26:43PM +0800, Wu Fengguang wrote:
> Introduce option guilt.diffstat so that we don't have to type
> "guilt refresh --diffstat" in its full form every time.
Applied.
Thanks,
Josef 'Jeff' Sipek.
--
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like
that.
- Linus Torvalds
^ permalink raw reply
* Re: [PATCH] guilt doc: use literal paragraphs for example commands
From: Josef Jeff Sipek @ 2008-12-20 4:33 UTC (permalink / raw)
To: Wu Fengguang; +Cc: git@vger.kernel.org
In-Reply-To: <20081218113934.GA16171@localhost>
On Thu, Dec 18, 2008 at 07:39:34PM +0800, Wu Fengguang wrote:
> Display example commands verbatim by indenting the paragraphs.
Applied.
Thanks,
Josef 'Jeff' Sipek.
--
Debugging is twice as hard as writing the code in the first place.
Therefore, if you write the code as cleverly as possible, you are, by
definition, not smart enough to debug it.
- Brian W. Kernighan
^ permalink raw reply
* Re: [PATCH 4/4] Add an expensive test for git-notes
From: Boyd Stephen Smith Jr. @ 2008-12-19 23:49 UTC (permalink / raw)
To: Git Mailing List; +Cc: Johannes Schindelin, Jeff King, Govind Salinas
In-Reply-To: <alpine.DEB.1.00.0812200035590.30769@pacific.mpi-cbg.de>
[-- Attachment #1: Type: text/plain, Size: 452 bytes --]
On Friday 2008 December 19 17:37:15 Johannes Schindelin wrote:
> + test_expect_success 'notes timing' "time_notes 100"
^^^
Probably should be ${count}.
--
Boyd Stephen Smith Jr. ,= ,-_-. =.
bss@iguanasuicide.net ((_/)o o(\_))
ICQ: 514984 YM/AIM: DaTwinkDaddy `-'(. .)`-'
http://iguanasuicide.net/ \_/
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* [PATCH] git-mergetool: properly handle "git mergetool -- filename"
From: David Aguilar @ 2008-12-20 1:01 UTC (permalink / raw)
To: gitster; +Cc: git, David Aguilar
Like many git commands, git-mergetool allows "--" to signal
the end of option processing. This adds a missing "shift"
statement so that this is correctly handled.
Signed-off-by: David Aguilar <davvid@gmail.com>
---
git-mergetool.sh | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/git-mergetool.sh b/git-mergetool.sh
index 94187c3..d4078a6 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -296,6 +296,7 @@ do
esac
;;
--)
+ shift
break
;;
-*)
--
1.6.1.rc3.38.gec9f0
^ permalink raw reply related
* Re: [PATCH] fast-import: make tagger information optional
From: Shawn O. Pearce @ 2008-12-20 0:59 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, Miklos Vajna, git, scott
In-Reply-To: <7vfxkj65y5.fsf@gitster.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> wrote:
> Even though newer Porcelain tools always record the tagger information
> when creating new tags, export/import pair should be able to faithfully
> reproduce ancient tag objects that lack tagger information.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Thanks Junio.
Acked-by: Shawn O. Pearce <spearce@spearce.org>
--
Shawn.
^ permalink raw reply
* Re: [PATCH] fast-import: make tagger information optional
From: Miklos Vajna @ 2008-12-20 0:59 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Shawn O. Pearce, Johannes Schindelin, git, scott
In-Reply-To: <7vfxkj65y5.fsf@gitster.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 477 bytes --]
On Fri, Dec 19, 2008 at 04:33:38PM -0800, Junio C Hamano <gitster@pobox.com> wrote:
> diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
> index 91b5ace..5a2aaf2 100755
> --- a/t/t9300-fast-import.sh
> +++ b/t/t9300-fast-import.sh
> @@ -56,6 +56,12 @@ M 644 :2 file2
> M 644 :3 file3
> M 755 :4 file4
>
> +tag series-A
> +from :5
> +data <<EOF
> +An annotated tag without an tagger
Minor nit: isn't this (here and later one more time) 'a tagger'?
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* [PATCH] SubmittingPatches: mention the usage of real name in Signed-off-by: lines
From: Miklos Vajna @ 2008-12-20 0:52 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nanako Shiraishi, jidanni, git
In-Reply-To: <7vbpv765oc.fsf@gitster.siamese.dyndns.org>
Especially with something that is supposed to hopefully have some legal
value down the line if somebody starts making noises, it really would be
nice to have a real person to associate things with. Suggest this in the
SubmittingPatches document.
Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---
On Fri, Dec 19, 2008 at 04:39:31PM -0800, Junio C Hamano <gitster@pobox.com> wrote:
> I would agree with the reasoning, but do we have to say "childish"
> just to
> be derogatory? I think that is counterproductive.
True.
Documentation/SubmittingPatches | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index f0295c6..ba07c8c 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -222,6 +222,9 @@ D-C-O. Indeed you are encouraged to do so. Do not forget to
place an in-body "From: " line at the beginning to properly attribute
the change to its true author (see (2) above).
+Also notice that a real name is used in the Signed-off-by: line. Please
+don't hide your real name.
+
Some people also put extra tags at the end.
"Acked-by:" says that the patch was reviewed by the person who
--
1.6.1.rc1.35.gae26e.dirty
^ permalink raw reply related
* Re: [PATCH] SubmittingPatches: mention the usage of real name in Signed-off-by: lines
From: Junio C Hamano @ 2008-12-20 0:39 UTC (permalink / raw)
To: Miklos Vajna; +Cc: Nanako Shiraishi, jidanni, git
In-Reply-To: <1229732427-14740-1-git-send-email-vmiklos@frugalware.org>
Miklos Vajna <vmiklos@frugalware.org> writes:
> Especially with something that is supposed to hopefully have some legal
> value down the line if somebody starts making noises, it really would be
> nice to have a real person to associate things with.
I would agree with the reasoning, but do we have to say "childish" just to
be derogatory? I think that is counterproductive.
^ permalink raw reply
* Re: just can't live without a user.name
From: jidanni @ 2008-12-20 0:36 UTC (permalink / raw)
To: vmiklos; +Cc: git
In-Reply-To: <20081220000803.GK21154@genesis.frugalware.org>
MV> I said you can use utf-8 in the user.name field and showed you an example
But I want to use null or (my favorite string of) ASCII.
Anyway no program so far on UNIX broke with just
$ grep $USER /etc/passwd
jidanni:x:1000:1000:,,,:/home/jidanni:/bin/bash
except git.
^ permalink raw reply
* Re: [PATCH 1/2] gitweb: allow access to forks with strict_export
From: Jakub Narebski @ 2008-12-20 0:35 UTC (permalink / raw)
To: Matt McCutchen; +Cc: Junio C Hamano, git, Petr Baudis
In-Reply-To: <1229220398.3360.66.camel@mattlaptop2.local>
On Sun, 14 Dec 2008 03:06, Matt McCutchen napisał:
> On Sat, 2008-12-13 at 23:51 +0100, Jakub Narebski wrote:
> >
> > "no_hide" (currently "include_forks") allows us to _not_ passing this
> > parameter in other places than project_in_list(); undef is falsy.
>
> Right. That's why I made the current parameter $for_strict_export (so
> only project_in_list passes it) rather than the negation.
Still I think $for_strict_export is singularly _bad_ name for
a parameter. $no_hide or $show_all would be much, much better.
> > By the way, doesn't git_project_index and perhaps git_opml also need
> > this parameter passed to git_get_projects_list?
>
> Yes, now that you mention it, I suppose they should show forks, though
> not hidden repositories. Then git_get_projects_list can be called in
> three different modes: include everything (project_in_list), include
> forks but not hidden (git_get_project_index and git_opml), or include
> neither forks nor hidden (git_project_list). Should we have two
> separate parameters to git_get_projects_list or a single three-valued
> one?
By the way, I am not sure if your idea of "hidden projects" and projects
list (projects index) file.
We have two way of specifying list of projects. One is scanning
projectroot directory for something that looks like git repository,
the other is having projects list file with project paths and project
owners.
If you use projects list file, you can have projects which are in not
on project list, but are accessible in filesystem. Those projects
(repositories) are hidden, i.e. they are not visible on projects_list
page, but still you can access a project. If you want to have projects
which are not on list to be not accessible at all, and not only hidden,
you have to use $strict_export... using which makes access to repo
views a tiny bit slower (and _only_ slows down gitweb for when scanning
directories for projects).
But why would one _want_ projects which are not visible, but still
accessible; "hidden" projects? Moreso what you want, the three class
of projects inside $projectroot: visible, hidden, and not exported.
With newly added $export_auth_hook you can limit accessibility of
projects in other ways, for example using .htaccess control files of
Apache web server. Check gitweb/README (or gitweb/INSTALL) for
example.
> That raises another point. I was going to change git_get_projects_list
> so that forks of a hidden project that are not themselves hidden appear
> on the parent project's page but not in the main project list. This
> way, users who know about the parent project can navigate to the fork,
> but the fork does not give away the existence of the parent project by
> appearing in the main list. Then I guess git_project_index and git_opml
> should omit forks of hidden projects, meaning that some fork-checking
> still has to take place with "include forks" on but "include hidden"
> off. This will make git_get_projects_list somewhat more complex but not
> unmanageably so, and I do think it's the behavior we want.
>
> I will send an updated patch.
So you see that having explicitly hidden files have yet another
complication. I wonder if this is really worth it...
--
Jakub Narebski
Poland
^ permalink raw reply
* [PATCH] fast-import: make tagger information optional
From: Junio C Hamano @ 2008-12-20 0:33 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Johannes Schindelin, Miklos Vajna, git, scott
In-Reply-To: <alpine.DEB.1.00.0812200059480.30769@pacific.mpi-cbg.de>
Even though newer Porcelain tools always record the tagger information
when creating new tags, export/import pair should be able to faithfully
reproduce ancient tag objects that lack tagger information.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* ... and corresponding patch to fast-import may look like this.
fast-import.c | 24 ++++++++++++++----------
t/t9300-fast-import.sh | 18 ++++++++++++++++++
2 files changed, 32 insertions(+), 10 deletions(-)
diff --git a/fast-import.c b/fast-import.c
index 201d4ff..d107896 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -2265,23 +2265,27 @@ static void parse_new_tag(void)
read_next_command();
/* tagger ... */
- if (prefixcmp(command_buf.buf, "tagger "))
- die("Expected tagger command, got %s", command_buf.buf);
- tagger = parse_ident(command_buf.buf + 7);
+ if (!prefixcmp(command_buf.buf, "tagger ")) {
+ tagger = parse_ident(command_buf.buf + 7);
+ read_next_command();
+ } else
+ tagger = NULL;
/* tag payload/message */
- read_next_command();
parse_data(&msg);
/* build the tag object */
strbuf_reset(&new_data);
+
strbuf_addf(&new_data,
- "object %s\n"
- "type %s\n"
- "tag %s\n"
- "tagger %s\n"
- "\n",
- sha1_to_hex(sha1), commit_type, t->name, tagger);
+ "object %s\n"
+ "type %s\n"
+ "tag %s\n",
+ sha1_to_hex(sha1), commit_type, t->name);
+ if (tagger)
+ strbuf_addf(&new_data,
+ "tagger %s\n", tagger);
+ strbuf_addch(&new_data, '\n');
strbuf_addbuf(&new_data, &msg);
free(tagger);
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index 91b5ace..5a2aaf2 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -56,6 +56,12 @@ M 644 :2 file2
M 644 :3 file3
M 755 :4 file4
+tag series-A
+from :5
+data <<EOF
+An annotated tag without an tagger
+EOF
+
INPUT_END
test_expect_success \
'A: create pack from stdin' \
@@ -102,6 +108,18 @@ test_expect_success \
'git cat-file blob master:file4 >actual && test_cmp expect actual'
cat >expect <<EOF
+object $(git rev-parse refs/heads/master)
+type commit
+tag series-A
+
+An annotated tag without an tagger
+EOF
+test_expect_success 'A: verify tag/series-A' '
+ git cat-file tag tags/series-A >actual &&
+ test_cmp expect actual
+'
+
+cat >expect <<EOF
:2 `git rev-parse --verify master:file2`
:3 `git rev-parse --verify master:file3`
:4 `git rev-parse --verify master:file4`
--
1.6.1.rc3.48.g2724
^ permalink raw reply related
* [PATCH] SubmittingPatches: mention the usage of real name in Signed-off-by: lines
From: Miklos Vajna @ 2008-12-20 0:20 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nanako Shiraishi, jidanni, git
In-Reply-To: <7vhc4z7oia.fsf@gitster.siamese.dyndns.org>
Especially with something that is supposed to hopefully have some legal
value down the line if somebody starts making noises, it really would be
nice to have a real person to associate things with. Suggest this in the
SubmittingPatches document.
Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---
On Fri, Dec 19, 2008 at 03:07:25PM -0800, Junio C Hamano <gitster@pobox.com> wrote:
> An earlier mistake does not justify adding new ones. Besides, I think
> ALASCM once revealed his "real name" on the list.
Ah, true ($gmane/7946). Then maybe something like this would be good to
have? The commit message is mostly copy&paste from Linus' message.
Documentation/SubmittingPatches | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index f0295c6..530aa25 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -222,6 +222,9 @@ D-C-O. Indeed you are encouraged to do so. Do not forget to
place an in-body "From: " line at the beginning to properly attribute
the change to its true author (see (2) above).
+Also notice that a real name is used in the Signed-off-by: line. Please
+don't be childish and don't hide your real name.
+
Some people also put extra tags at the end.
"Acked-by:" says that the patch was reviewed by the person who
--
1.6.1.rc1.35.gae26e.dirty
^ permalink raw reply related
* Re: just can't live without a user.name
From: Junio C Hamano @ 2008-12-20 0:18 UTC (permalink / raw)
To: Miklos Vajna; +Cc: jidanni, git
In-Reply-To: <20081220000803.GK21154@genesis.frugalware.org>
Miklos Vajna <vmiklos@frugalware.org> writes:
> On Sat, Dec 20, 2008 at 07:20:56AM +0800, jidanni@jidanni.org wrote:
>> Actually it's all git's fault for not working if user.name is null or
>> unset.
>
> Please don't turn this thread into a flame.
Just pay attention to messages in which he says something that adds value
to the discussion, and simply ignore the rest.
^ permalink raw reply
* Re: just can't live without a user.name
From: Miklos Vajna @ 2008-12-20 0:08 UTC (permalink / raw)
To: jidanni; +Cc: nanako3, gitster, git
In-Reply-To: <87fxkjbvl3.fsf_-_@jidanni.org>
[-- Attachment #1: Type: text/plain, Size: 615 bytes --]
On Sat, Dec 20, 2008 at 07:20:56AM +0800, jidanni@jidanni.org wrote:
> Actually it's all git's fault for not working if user.name is null or
> unset.
Please don't turn this thread into a flame.
If I understand correctly, you said you hide your name because you can't
use your real name and using some limited ASCII replacement makes you
unhappy.
I said you can use utf-8 in the user.name field and showed you an
example about this.
Instead of replying to this, you just did not quote any parts of my
message, but started to talk about you don't want to show your real name
at all. What's the point of doing so?
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* [PATCH v2] fast-export: deal with tag objects that do not have a tagger
From: Johannes Schindelin @ 2008-12-20 0:00 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Miklos Vajna, git, scott
In-Reply-To: <7viqphf4ua.fsf@gitster.siamese.dyndns.org>
When no tagger was found (old Git produced tags like this),
no "tagger" line is printed (but this is incompatible with the current
git fast-import).
Alternatively, you can pass the option --fake-missing-tagger, forcing
fast-export to fake a tagger
Unspecified Tagger <no-tagger>
with a tag date of the beginning of (Unix) time in the case of a missing
tagger, so that fast-import is still able to import the result.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
Documentation/git-fast-export.txt | 6 ++++++
builtin-fast-export.c | 21 ++++++++++++++++-----
t/t9301-fast-export.sh | 20 ++++++++++++++++++++
3 files changed, 42 insertions(+), 5 deletions(-)
diff --git a/Documentation/git-fast-export.txt b/Documentation/git-fast-export.txt
index 99a1c31..0c9eb56 100644
--- a/Documentation/git-fast-export.txt
+++ b/Documentation/git-fast-export.txt
@@ -65,6 +65,12 @@ If the backend uses a similar \--import-marks file, this allows for
incremental bidirectional exporting of the repository by keeping the
marks the same across runs.
+--fake-missing-tagger::
+ Some old repositories have tags without a tagger. The
+ fast-import protocol was pretty strict about that, and did not
+ allow that. So fake a tagger to be able to fast-import the
+ output.
+
EXAMPLES
--------
diff --git a/builtin-fast-export.c b/builtin-fast-export.c
index 7d5d57a..8386338 100644
--- a/builtin-fast-export.c
+++ b/builtin-fast-export.c
@@ -24,6 +24,7 @@ static const char *fast_export_usage[] = {
static int progress;
static enum { VERBATIM, WARN, STRIP, ABORT } signed_tag_mode = ABORT;
+static int fake_missing_tagger;
static int parse_opt_signed_tag_mode(const struct option *opt,
const char *arg, int unset)
@@ -297,10 +298,17 @@ static void handle_tag(const char *name, struct tag *tag)
message_size = strlen(message);
}
tagger = memmem(buf, message ? message - buf : size, "\ntagger ", 8);
- if (!tagger)
- die ("No tagger for tag %s", sha1_to_hex(tag->object.sha1));
- tagger++;
- tagger_end = strchrnul(tagger, '\n');
+ if (!tagger) {
+ if (fake_missing_tagger)
+ tagger = "tagger Unspecified Tagger "
+ "<unspecified-tagger> 0 +0000";
+ else
+ tagger = "";
+ tagger_end = tagger + strlen(tagger);
+ } else {
+ tagger++;
+ tagger_end = strchrnul(tagger, '\n');
+ }
/* handle signed tags */
if (message) {
@@ -326,9 +334,10 @@ static void handle_tag(const char *name, struct tag *tag)
if (!prefixcmp(name, "refs/tags/"))
name += 10;
- printf("tag %s\nfrom :%d\n%.*s\ndata %d\n%.*s\n",
+ printf("tag %s\nfrom :%d\n%.*s%sdata %d\n%.*s\n",
name, get_object_mark(tag->tagged),
(int)(tagger_end - tagger), tagger,
+ tagger == tagger_end ? "" : "\n",
(int)message_size, (int)message_size, message ? message : "");
}
@@ -483,6 +492,8 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
"Dump marks to this file"),
OPT_STRING(0, "import-marks", &import_filename, "FILE",
"Import marks from this file"),
+ OPT_BOOLEAN(0, "fake-missing-tagger", &fake_missing_tagger,
+ "Fake a tagger when tags lack one"),
OPT_END()
};
diff --git a/t/t9301-fast-export.sh b/t/t9301-fast-export.sh
index 2057435..9985721 100755
--- a/t/t9301-fast-export.sh
+++ b/t/t9301-fast-export.sh
@@ -239,4 +239,24 @@ test_expect_success 'fast-export | fast-import when master is tagged' '
'
+cat > tag-content << EOF
+object $(git rev-parse HEAD)
+type commit
+tag rosten
+EOF
+
+test_expect_success 'cope with tagger-less tags' '
+
+ TAG=$(git hash-object -t tag -w tag-content) &&
+ git update-ref refs/tags/sonnenschein $TAG &&
+ git fast-export -C -C --signed-tags=strip --all > output &&
+ test $(grep -c "^tag " output) = 4 &&
+ ! grep "Unspecified Tagger" output &&
+ git fast-export -C -C --signed-tags=strip --all \
+ --fake-missing-tagger > output &&
+ test $(grep -c "^tag " output) = 4 &&
+ grep "Unspecified Tagger" output
+
+'
+
test_done
--
1.6.1.rc3.412.ga72b
^ permalink raw reply related
* Re: Odd merge behaviour involving reverts
From: Junio C Hamano @ 2008-12-19 23:51 UTC (permalink / raw)
To: Nanako Shiraishi; +Cc: Linus Torvalds, Alan, git
In-Reply-To: <20081220081238.6117@nanako3.lavabit.com>
Nanako Shiraishi <nanako3@lavabit.com> writes:
> Quoting Junio C Hamano <gitster@pobox.com>:
>
>> Nanako Shiraishi <nanako3@lavabit.com> writes:
>>
>>> I think your explanation will help people if we make it part of the
>>> documentation. Especially because two different cases need two
>>> different recovery methods, and people need to learn which is which.
>>
>> Sure. It needs copyediting to make it readable standalone by not
>> mentioning "your misunderstanding", inlining "earlier Linus's suggestion",
>> etc., though.
>>
>> Patches welcome ;-)
>
> Okay, I'll send one later.
Thanks.
^ permalink raw reply
* [PATCH 4/4] Add an expensive test for git-notes
From: Johannes Schindelin @ 2008-12-19 23:37 UTC (permalink / raw)
To: Jeff King; +Cc: Govind Salinas, Git Mailing List
In-Reply-To: <alpine.DEB.1.00.0812192347261.30769@pacific.mpi-cbg.de>
git-notes have the potential of being pretty expensive, so test with
a lot of commits. A lot. So to make things cheaper, you have to
opt-in explicitely, by setting the environment variable
GIT_NOTES_TIMING_TESTS.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
I would appreciate other people running the tests, and maybe
profiling the code.
However, I will not be really online the next two weeks, so if you
feel like working on this series, go ahead.
Merry Christmas.
t/t3302-notes-index-expensive.sh | 98 ++++++++++++++++++++++++++++++++++++++
1 files changed, 98 insertions(+), 0 deletions(-)
create mode 100755 t/t3302-notes-index-expensive.sh
diff --git a/t/t3302-notes-index-expensive.sh b/t/t3302-notes-index-expensive.sh
new file mode 100755
index 0000000..00d27bf
--- /dev/null
+++ b/t/t3302-notes-index-expensive.sh
@@ -0,0 +1,98 @@
+#!/bin/sh
+#
+# Copyright (c) 2007 Johannes E. Schindelin
+#
+
+test_description='Test commit notes index (expensive!)'
+
+. ./test-lib.sh
+
+test -z "$GIT_NOTES_TIMING_TESTS" && {
+ say Skipping timing tests
+ test_done
+ exit
+}
+
+create_repo () {
+ number_of_commits=$1
+ nr=0
+ parent=
+ test -d .git || {
+ git init &&
+ tree=$(git write-tree) &&
+ while [ $nr -lt $number_of_commits ]; do
+ test_tick &&
+ commit=$(echo $nr | git commit-tree $tree $parent) ||
+ return
+ parent="-p $commit"
+ nr=$(($nr+1))
+ done &&
+ git update-ref refs/heads/master $commit &&
+ {
+ export GIT_INDEX_FILE=.git/temp;
+ git rev-list HEAD | cat -n | sed "s/^[ ][ ]*/ /g" |
+ while read nr sha1; do
+ blob=$(echo note $nr | git hash-object -w --stdin) &&
+ echo $sha1 | sed "s/^/0644 $blob 0 /"
+ done | git update-index --index-info &&
+ tree=$(git write-tree) &&
+ test_tick &&
+ commit=$(echo notes | git commit-tree $tree) &&
+ git update-ref refs/notes/commits $commit
+ } &&
+ git config core.notesRef refs/notes/commits
+ }
+}
+
+test_notes () {
+ count=$1 &&
+ git config core.notesRef refs/notes/commits &&
+ git log | grep "^ " > output &&
+ i=1 &&
+ while [ $i -le $count ]; do
+ echo " $(($count-$i))" &&
+ echo " note $i" &&
+ i=$(($i+1));
+ done > expect &&
+ git diff expect output
+}
+
+cat > time_notes << \EOF
+ mode=$1
+ i=1
+ while [ $i -lt $2 ]; do
+ case $1 in
+ no-notes)
+ export GIT_NOTES_REF=non-existing
+ ;;
+ notes)
+ unset GIT_NOTES_REF
+ ;;
+ esac
+ git log >/dev/null
+ i=$(($i+1))
+ done
+EOF
+
+time_notes () {
+ for mode in no-notes notes
+ do
+ echo $mode
+ /usr/bin/time sh ../time_notes $mode $1
+ done
+}
+
+for count in 10 100 1000 10000; do
+
+ mkdir $count
+ (cd $count;
+
+ test_expect_success "setup $count" "create_repo $count"
+
+ test_expect_success 'notes work' "test_notes $count"
+
+ test_expect_success 'notes timing' "time_notes 100"
+ )
+done
+
+test_done
--
1.6.1.rc3.368.g63acc
^ permalink raw reply related
* [PATCH 3/4] Speed up git notes lookup
From: Johannes Schindelin @ 2008-12-19 23:35 UTC (permalink / raw)
To: Jeff King; +Cc: Govind Salinas, Git Mailing List
In-Reply-To: <alpine.DEB.1.00.0812192347261.30769@pacific.mpi-cbg.de>
To avoid looking up each and every commit in the notes ref's tree
object, which is very expensive, speed things up by slurping the tree
object's contents into a hash_map.
The idea fo the hashmap singleton is from David Reiss, initial
benchmarking by Jeff King.
Note: the implementation allows for arbitrary entries in the notes
tree object, ignoring those that do not reference a valid object. This
allows you to annotate arbitrary branches, or objects.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
notes.c | 113 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++------
1 files changed, 102 insertions(+), 11 deletions(-)
diff --git a/notes.c b/notes.c
index 91ec77f..68bcb24 100644
--- a/notes.c
+++ b/notes.c
@@ -4,16 +4,112 @@
#include "refs.h"
#include "utf8.h"
#include "strbuf.h"
+#include "tree-walk.h"
+
+struct entry {
+ unsigned char commit_sha1[20];
+ unsigned char notes_sha1[20];
+};
+
+struct hash_map {
+ struct entry *entries;
+ off_t count, size;
+};
static int initialized;
+static struct hash_map hash_map;
+
+static int hash_index(struct hash_map *map, const unsigned char *sha1)
+{
+ int i = ((*(unsigned int *)sha1) % map->size);
+
+ for (;;) {
+ unsigned char *current = map->entries[i].commit_sha1;
+
+ if (!hashcmp(sha1, current))
+ return i;
+
+ if (is_null_sha1(current))
+ return -1 - i;
+
+ if (++i == map->size)
+ i = 0;
+ }
+}
+
+static void add_entry(const unsigned char *commit_sha1,
+ const unsigned char *notes_sha1)
+{
+ int index;
+
+ if (hash_map.count + 1 > hash_map.size >> 1) {
+ int i, old_size = hash_map.size;
+ struct entry *old = hash_map.entries;
+
+ hash_map.size = old_size ? old_size << 1 : 64;
+ hash_map.entries = (struct entry *)
+ xcalloc(sizeof(struct entry), hash_map.size);
+
+ for (i = 0; i < old_size; i++)
+ if (!is_null_sha1(old[i].commit_sha1)) {
+ index = -1 - hash_index(&hash_map,
+ old[i].commit_sha1);
+ memcpy(hash_map.entries + index, old + i,
+ sizeof(struct entry));
+ }
+ free(old);
+ }
+
+ index = hash_index(&hash_map, commit_sha1);
+ if (index < 0) {
+ index = -1 - index;
+ hash_map.count++;
+ }
+
+ hashcpy(hash_map.entries[index].commit_sha1, commit_sha1);
+ hashcpy(hash_map.entries[index].notes_sha1, notes_sha1);
+}
+
+static void initialize_hash_map(const char *notes_ref_name)
+{
+ unsigned char sha1[20], commit_sha1[20];
+ unsigned *mode;
+ struct tree_desc desc;
+ struct name_entry entry;
+ void *buf;
+
+ if (!notes_ref_name || read_ref(notes_ref_name, commit_sha1) ||
+ get_tree_entry(commit_sha1, "", sha1, mode))
+ return;
+
+ buf = fill_tree_descriptor(&desc, sha1);
+ if (!buf)
+ die ("Could not read %s for notes-index", sha1_to_hex(sha1));
+
+ while (tree_entry(&desc, &entry))
+ if (!get_sha1(entry.path, commit_sha1))
+ add_entry(commit_sha1, entry.sha1);
+ free(buf);
+}
+
+static unsigned char *lookup_notes(const unsigned char *commit_sha1)
+{
+ int index;
+
+ if (!hash_map.size)
+ return NULL;
+
+ index = hash_index(&hash_map, commit_sha1);
+ if (index < 0)
+ return NULL;
+ return hash_map.entries[index].notes_sha1;
+}
void get_commit_notes(const struct commit *commit, struct strbuf *sb,
const char *output_encoding)
{
static const char *utf8 = "utf-8";
- struct strbuf name = STRBUF_INIT;
- const char *hex;
- unsigned char sha1[20];
+ unsigned char *sha1;
char *msg;
unsigned long msgoffset, msglen;
enum object_type type;
@@ -24,17 +120,12 @@ void get_commit_notes(const struct commit *commit, struct strbuf *sb,
notes_ref_name = getenv(GIT_NOTES_REF_ENVIRONMENT);
else if (!notes_ref_name)
notes_ref_name = GIT_NOTES_DEFAULT_REF;
- if (notes_ref_name && read_ref(notes_ref_name, sha1))
- notes_ref_name = NULL;
+ initialize_hash_map(notes_ref_name);
initialized = 1;
}
- if (!notes_ref_name)
- return;
-
- strbuf_addf(&name, "%s:%s", notes_ref_name,
- sha1_to_hex(commit->object.sha1));
- if (get_sha1(name.buf, sha1))
+ sha1 = lookup_notes(commit->object.sha1);
+ if (!sha1)
return;
if (!(msg = read_sha1_file(sha1, &type, &msglen)) || !msglen ||
--
1.6.1.rc3.368.g63acc
^ permalink raw reply related
* [PATCH 2/4] Add a script to edit/inspect notes
From: Johannes Schindelin @ 2008-12-19 23:35 UTC (permalink / raw)
To: Jeff King; +Cc: Govind Salinas, Git Mailing List
In-Reply-To: <alpine.DEB.1.00.0812192347261.30769@pacific.mpi-cbg.de>
The script 'git notes' allows you to edit and show commit notes, by
calling either
git notes show <commit>
or
git notes edit <commit>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
.gitignore | 1 +
Documentation/git-notes.txt | 46 ++++++++++++++++++++++++++++++
Makefile | 1 +
command-list.txt | 1 +
git-notes.sh | 65 +++++++++++++++++++++++++++++++++++++++++++
t/t3301-notes.sh | 65 +++++++++++++++++++++++++++++++++++++++++++
6 files changed, 179 insertions(+), 0 deletions(-)
create mode 100644 Documentation/git-notes.txt
create mode 100755 git-notes.sh
create mode 100755 t/t3301-notes.sh
diff --git a/.gitignore b/.gitignore
index 90bbb2a..8b90af1 100644
--- a/.gitignore
+++ b/.gitignore
@@ -83,6 +83,7 @@ git-mktag
git-mktree
git-name-rev
git-mv
+git-notes
git-pack-redundant
git-pack-objects
git-pack-refs
diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt
new file mode 100644
index 0000000..3d93625
--- /dev/null
+++ b/Documentation/git-notes.txt
@@ -0,0 +1,46 @@
+git-notes(1)
+============
+
+NAME
+----
+git-notes - Add/inspect commit notes
+
+SYNOPSIS
+--------
+[verse]
+'git-notes' (edit | show) [commit]
+
+DESCRIPTION
+-----------
+This command allows you to add notes to commit messages, without
+changing the commit. To discern these notes from the message stored
+in the commit object, the notes are indented like the message, after
+an unindented line saying "Notes:".
+
+To disable commit notes, you have to set the config variable
+core.notesRef to the empty string. Alternatively, you can set it
+to a different ref, something like "refs/notes/bugzilla". This setting
+can be overridden by the environment variable "GIT_NOTES_REF".
+
+
+SUBCOMMANDS
+-----------
+
+edit::
+ Edit the notes for a given commit (defaults to HEAD).
+
+show::
+ Show the notes for a given commit (defaults to HEAD).
+
+
+Author
+------
+Written by Johannes Schindelin <johannes.schindelin@gmx.de>
+
+Documentation
+-------------
+Documentation by Johannes Schindelin
+
+GIT
+---
+Part of the gitlink:git[7] suite
diff --git a/Makefile b/Makefile
index 7871f52..4bdc86e 100644
--- a/Makefile
+++ b/Makefile
@@ -259,6 +259,7 @@ SCRIPT_SH += git-merge-octopus.sh
SCRIPT_SH += git-merge-one-file.sh
SCRIPT_SH += git-merge-resolve.sh
SCRIPT_SH += git-mergetool.sh
+SCRIPT_SH += git-notes.sh
SCRIPT_SH += git-parse-remote.sh
SCRIPT_SH += git-pull.sh
SCRIPT_SH += git-quiltimport.sh
diff --git a/command-list.txt b/command-list.txt
index 3583a33..2dc2c33 100644
--- a/command-list.txt
+++ b/command-list.txt
@@ -73,6 +73,7 @@ git-mktag plumbingmanipulators
git-mktree plumbingmanipulators
git-mv mainporcelain common
git-name-rev plumbinginterrogators
+git-notes mainporcelain
git-pack-objects plumbingmanipulators
git-pack-redundant plumbinginterrogators
git-pack-refs ancillarymanipulators
diff --git a/git-notes.sh b/git-notes.sh
new file mode 100755
index 0000000..bfdbaa8
--- /dev/null
+++ b/git-notes.sh
@@ -0,0 +1,65 @@
+#!/bin/sh
+
+USAGE="(edit | show) [commit]"
+. git-sh-setup
+
+test -n "$3" && usage
+
+test -z "$1" && usage
+ACTION="$1"; shift
+
+test -z "$GIT_NOTES_REF" && GIT_NOTES_REF="$(git config core.notesref)"
+test -z "$GIT_NOTES_REF" && GIT_NOTES_REF="refs/notes/commits"
+
+COMMIT=$(git rev-parse --verify --default HEAD "$@") ||
+die "Invalid commit: $@"
+
+MESSAGE="$GIT_DIR"/new-notes-$COMMIT
+trap '
+ test -f "$MESSAGE" && rm "$MESSAGE"
+' 0
+
+case "$ACTION" in
+edit)
+ GIT_NOTES_REF= git log -1 $COMMIT | sed "s/^/#/" > "$MESSAGE"
+
+ GIT_INDEX_FILE="$MESSAGE".idx
+ export GIT_INDEX_FILE
+
+ CURRENT_HEAD=$(git show-ref "$GIT_NOTES_REF" | cut -f 1 -d ' ')
+ if [ -z "$CURRENT_HEAD" ]; then
+ PARENT=
+ else
+ PARENT="-p $CURRENT_HEAD"
+ git read-tree "$GIT_NOTES_REF" || die "Could not read index"
+ git cat-file blob :$COMMIT >> "$MESSAGE" 2> /dev/null
+ fi
+
+ ${VISUAL:-${EDITOR:-vi}} "$MESSAGE"
+
+ grep -v ^# < "$MESSAGE" | git stripspace > "$MESSAGE".processed
+ mv "$MESSAGE".processed "$MESSAGE"
+ if [ -s "$MESSAGE" ]; then
+ BLOB=$(git hash-object -w "$MESSAGE") ||
+ die "Could not write into object database"
+ git update-index --add --cacheinfo 0644 $BLOB $COMMIT ||
+ die "Could not write index"
+ else
+ test -z "$CURRENT_HEAD" &&
+ die "Will not initialise with empty tree"
+ git update-index --force-remove $COMMIT ||
+ die "Could not update index"
+ fi
+
+ TREE=$(git write-tree) || die "Could not write tree"
+ NEW_HEAD=$(echo Annotate $COMMIT | git commit-tree $TREE $PARENT) ||
+ die "Could not annotate"
+ git update-ref -m "Annotate $COMMIT" \
+ "$GIT_NOTES_REF" $NEW_HEAD $CURRENT_HEAD
+;;
+show)
+ git show "$GIT_NOTES_REF":$COMMIT
+;;
+*)
+ usage
+esac
diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
new file mode 100755
index 0000000..ba42c45
--- /dev/null
+++ b/t/t3301-notes.sh
@@ -0,0 +1,65 @@
+#!/bin/sh
+#
+# Copyright (c) 2007 Johannes E. Schindelin
+#
+
+test_description='Test commit notes'
+
+. ./test-lib.sh
+
+cat > fake_editor.sh << \EOF
+echo "$MSG" > "$1"
+echo "$MSG" >& 2
+EOF
+chmod a+x fake_editor.sh
+VISUAL=./fake_editor.sh
+export VISUAL
+
+test_expect_success 'cannot annotate non-existing HEAD' '
+ ! MSG=3 git notes edit
+'
+
+test_expect_success setup '
+ : > a1 &&
+ git add a1 &&
+ test_tick &&
+ git commit -m 1st &&
+ : > a2 &&
+ git add a2 &&
+ test_tick &&
+ git commit -m 2nd
+'
+
+test_expect_success 'need valid notes ref' '
+ ! MSG=1 GIT_NOTES_REF='/' git notes edit &&
+ ! MSG=2 GIT_NOTES_REF='/' git notes show
+'
+
+test_expect_success 'create notes' '
+ git config core.notesRef refs/notes/commits &&
+ MSG=b1 git notes edit &&
+ test ! -f .git/new-notes &&
+ test 1 = $(git ls-tree refs/notes/commits | wc -l) &&
+ test b1 = $(git notes show) &&
+ git show HEAD^ &&
+ ! git notes show HEAD^
+'
+
+cat > expect << EOF
+commit 268048bfb8a1fb38e703baceb8ab235421bf80c5
+Author: A U Thor <author@example.com>
+Date: Thu Apr 7 15:14:13 2005 -0700
+
+ 2nd
+
+Notes:
+ b1
+EOF
+
+test_expect_success 'show notes' '
+ ! (git cat-file commit HEAD | grep b1) &&
+ git log -1 > output &&
+ git diff expect output
+'
+
+test_done
--
1.6.1.rc3.368.g63acc
^ permalink raw reply related
* [PATCH 1/4] Introduce commit notes
From: Johannes Schindelin @ 2008-12-19 23:35 UTC (permalink / raw)
To: Jeff King; +Cc: Govind Salinas, Git Mailing List
In-Reply-To: <alpine.DEB.1.00.0812192347261.30769@pacific.mpi-cbg.de>
Commit notes are blobs which are shown together with the commit
message. These blobs are taken from the notes ref, which you can
configure by the config variable core.notesRef, which in turn can
be overridden by the environment variable GIT_NOTES_REF.
The notes ref is a branch which contains "files" whose names are
the names of the corresponding commits (i.e. the SHA-1).
The rationale for putting this information into a ref is this: we
want to be able to fetch and possibly union-merge the notes,
maybe even look at the date when a note was introduced, and we
want to store them efficiently together with the other objects.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
Documentation/config.txt | 15 ++++++++++
Makefile | 2 +
cache.h | 3 ++
commit.c | 1 +
config.c | 5 +++
environment.c | 1 +
notes.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++
notes.h | 7 +++++
pretty.c | 5 +++
9 files changed, 107 insertions(+), 0 deletions(-)
create mode 100644 notes.c
create mode 100644 notes.h
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 4089362..3248524 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -431,6 +431,21 @@ core.inithook::
linkgit:git-init[1]. The hook is called with the argument
"reinit" if an existing repository is re-initialized.
+core.notesRef::
+ When showing commit messages, also show notes which are stored in
+ the given ref. This ref is expected to contain paths of the form
+ ??/*, where the directory name consists of the first two
+ characters of the commit name, and the base name consists of
+ the remaining 38 characters.
++
+If such a path exists in the given ref, the referenced blob is read, and
+appended to the commit message, separated by a "Notes:" line. If the
+given ref itself does not exist, it is not an error, but means that no
+notes should be print.
++
+This setting defaults to "refs/notes/commits", and can be overridden by
+the `GIT_NOTES_REF` environment variable.
+
alias.*::
Command aliases for the linkgit:git[1] command wrapper - e.g.
after defining "alias.last = cat-file commit HEAD", the invocation
diff --git a/Makefile b/Makefile
index 5e293fe..7871f52 100644
--- a/Makefile
+++ b/Makefile
@@ -371,6 +371,7 @@ LIB_H += ll-merge.h
LIB_H += log-tree.h
LIB_H += mailmap.h
LIB_H += merge-recursive.h
+LIB_H += notes.h
LIB_H += object.h
LIB_H += pack.h
LIB_H += pack-refs.h
@@ -454,6 +455,7 @@ LIB_OBJS += match-trees.o
LIB_OBJS += merge-file.o
LIB_OBJS += merge-recursive.o
LIB_OBJS += name-hash.o
+LIB_OBJS += notes.o
LIB_OBJS += object.o
LIB_OBJS += pack-check.o
LIB_OBJS += pack-refs.o
diff --git a/cache.h b/cache.h
index b393c2d..30981de 100644
--- a/cache.h
+++ b/cache.h
@@ -375,6 +375,8 @@ static inline enum object_type object_type(unsigned int mode)
#define GITATTRIBUTES_FILE ".gitattributes"
#define INFOATTRIBUTES_FILE "info/attributes"
#define ATTRIBUTE_MACRO_PREFIX "[attr]"
+#define GIT_NOTES_REF_ENVIRONMENT "GIT_NOTES_REF"
+#define GIT_NOTES_DEFAULT_REF "refs/notes/commits"
extern int is_bare_repository_cfg;
extern int is_bare_repository(void);
@@ -548,6 +550,7 @@ enum rebase_setup_type {
extern enum branch_track git_branch_track;
extern enum rebase_setup_type autorebase;
extern int keep_hard_links;
+extern char *notes_ref_name;
#define GIT_REPO_VERSION 0
extern int repository_format_version;
diff --git a/commit.c b/commit.c
index 00f4774..547b88f 100644
--- a/commit.c
+++ b/commit.c
@@ -5,6 +5,7 @@
#include "utf8.h"
#include "diff.h"
#include "revision.h"
+#include "notes.h"
int save_commit_buffer = 1;
diff --git a/config.c b/config.c
index 8ff2b4b..9ab9d21 100644
--- a/config.c
+++ b/config.c
@@ -469,6 +469,11 @@ static int git_default_core_config(const char *var, const char *value)
return 0;
}
+ if (!strcmp(var, "core.notesref")) {
+ notes_ref_name = xstrdup(value);
+ return 0;
+ }
+
if (!strcmp(var, "core.pager"))
return git_config_string(&pager_program, var, value);
diff --git a/environment.c b/environment.c
index fc91809..5724ad2 100644
--- a/environment.c
+++ b/environment.c
@@ -46,6 +46,7 @@ int keep_hard_links = 0;
/* Parallel index stat data preload? */
int core_preload_index = 0;
+char *notes_ref_name;
/* This is set by setup_git_dir_gently() and/or git_default_config() */
char *git_work_tree_cfg;
diff --git a/notes.c b/notes.c
new file mode 100644
index 0000000..91ec77f
--- /dev/null
+++ b/notes.c
@@ -0,0 +1,68 @@
+#include "cache.h"
+#include "commit.h"
+#include "notes.h"
+#include "refs.h"
+#include "utf8.h"
+#include "strbuf.h"
+
+static int initialized;
+
+void get_commit_notes(const struct commit *commit, struct strbuf *sb,
+ const char *output_encoding)
+{
+ static const char *utf8 = "utf-8";
+ struct strbuf name = STRBUF_INIT;
+ const char *hex;
+ unsigned char sha1[20];
+ char *msg;
+ unsigned long msgoffset, msglen;
+ enum object_type type;
+
+ if (!initialized) {
+ const char *env = getenv(GIT_NOTES_REF_ENVIRONMENT);
+ if (env)
+ notes_ref_name = getenv(GIT_NOTES_REF_ENVIRONMENT);
+ else if (!notes_ref_name)
+ notes_ref_name = GIT_NOTES_DEFAULT_REF;
+ if (notes_ref_name && read_ref(notes_ref_name, sha1))
+ notes_ref_name = NULL;
+ initialized = 1;
+ }
+
+ if (!notes_ref_name)
+ return;
+
+ strbuf_addf(&name, "%s:%s", notes_ref_name,
+ sha1_to_hex(commit->object.sha1));
+ if (get_sha1(name.buf, sha1))
+ return;
+
+ if (!(msg = read_sha1_file(sha1, &type, &msglen)) || !msglen ||
+ type != OBJ_BLOB)
+ return;
+
+ if (output_encoding && *output_encoding &&
+ strcmp(utf8, output_encoding)) {
+ char *reencoded = reencode_string(msg, output_encoding, utf8);
+ if (reencoded) {
+ free(msg);
+ msg = reencoded;
+ msglen = strlen(msg);
+ }
+ }
+
+ /* we will end the annotation by a newline anyway */
+ if (msglen && msg[msglen - 1] == '\n')
+ msglen--;
+
+ strbuf_addstr(sb, "\nNotes:\n");
+
+ for (msgoffset = 0; msgoffset < msglen;) {
+ int linelen = strchrnul(msg, '\n') - msg;
+
+ strbuf_addstr(sb, " ");
+ strbuf_add(sb, msg + msgoffset, linelen);
+ msgoffset += linelen;
+ }
+ free(msg);
+}
diff --git a/notes.h b/notes.h
new file mode 100644
index 0000000..79d21b6
--- /dev/null
+++ b/notes.h
@@ -0,0 +1,7 @@
+#ifndef NOTES_H
+#define NOTES_H
+
+void get_commit_notes(const struct commit *commit, struct strbuf *sb,
+ const char *output_encoding);
+
+#endif
diff --git a/pretty.c b/pretty.c
index 5f9a0c7..c2bf451 100644
--- a/pretty.c
+++ b/pretty.c
@@ -6,6 +6,7 @@
#include "string-list.h"
#include "mailmap.h"
#include "log-tree.h"
+#include "notes.h"
static char *user_format;
@@ -911,5 +912,9 @@ void pretty_print_commit(enum cmit_fmt fmt, const struct commit *commit,
*/
if (fmt == CMIT_FMT_EMAIL && sb->len <= beginning_of_body)
strbuf_addch(sb, '\n');
+
+ if (fmt != CMIT_FMT_ONELINE)
+ get_commit_notes(commit, sb, encoding);
+
free(reencoded);
}
--
1.6.1.rc3.368.g63acc
^ permalink raw reply related
* [PATCH 0/4] Notes reloaded
From: Johannes Schindelin @ 2008-12-19 23:34 UTC (permalink / raw)
To: Jeff King; +Cc: Govind Salinas, Git Mailing List
In-Reply-To: <20081216085108.GA3031@coredump.intra.peff.net>
Hi,
On Tue, 16 Dec 2008, Jeff King wrote:
> Johannes Schindelin's notes proposal (which is more or less
> the current proposal, but I think the on-disk notes index was not
> well liked):
> http://thread.gmane.org/gmane.comp.version-control.git/52598
I redid the benchmark (this time with a bit beefier machine), just
comparing no notes with David's/Peff's idea:
-- snip --
$ GIT_NOTES_TIMING_TESTS=1 sh t3302-notes-index-expensive.sh -i -v
Initialized empty Git repository in /home/gitte/git/t/trash directory.t3302-notes-index-expensive/.git/
* expecting success: create_repo 10
Initialized empty Git repository in /home/gitte/git/t/trash directory.t3302-notes-index-expensive/10/.git/
* ok 1: setup 10
* expecting success: test_notes 10
* ok 2: notes work
* expecting success: time_notes 100
no-notes
0.08user 0.10system 0:00.18elapsed 95%CPU (0avgtext+0avgdata 0maxresident)k
0inputs+0outputs (0major+58926minor)pagefaults 0swaps
notes
0.14user 0.07system 0:00.54elapsed 38%CPU (0avgtext+0avgdata 0maxresident)k
0inputs+0outputs (0major+60319minor)pagefaults 0swaps
* ok 3: notes timing
* expecting success: create_repo 100
Initialized empty Git repository in /home/gitte/git/t/trash directory.t3302-notes-index-expensive/100/.git/
* ok 1: setup 100
* expecting success: test_notes 100
* ok 2: notes work
* expecting success: time_notes 100
no-notes
0.23user 0.21system 0:00.45elapsed 96%CPU (0avgtext+0avgdata 0maxresident)k
0inputs+0outputs (0major+68043minor)pagefaults 0swaps
notes
0.38user 0.21system 0:00.59elapsed 99%CPU (0avgtext+0avgdata 0maxresident)k
0inputs+0outputs (0major+78829minor)pagefaults 0swaps
* ok 3: notes timing
* expecting success: create_repo 1000
Initialized empty Git repository in /home/gitte/git/t/trash directory.t3302-notes-index-expensive/1000/.git/
* ok 1: setup 1000
* expecting success: test_notes 1000
* ok 2: notes work
* expecting success: time_notes 100
no-notes
2.06user 0.95system 0:04.26elapsed 70%CPU (0avgtext+0avgdata 0maxresident)k
0inputs+0outputs (0major+159115minor)pagefaults 0swaps
notes
2.83user 1.54system 0:04.38elapsed 99%CPU (0avgtext+0avgdata 0maxresident)k
0inputs+0outputs (0major+267416minor)pagefaults 0swaps
* ok 3: notes timing
* expecting success: create_repo 10000
Initialized empty Git repository in /home/gitte/git/t/trash directory.t3302-notes-index-expensive/10000/.git/
* ok 1: setup 10000
* expecting success: test_notes 10000
* ok 2: notes work
* expecting success: time_notes 100
no-notes
20.46user 7.63system 0:28.30elapsed 99%CPU (0avgtext+0avgdata 0maxresident)k
0inputs+0outputs (0major+1083378minor)pagefaults 0swaps
notes
28.78user 13.74system 0:42.85elapsed 99%CPU (0avgtext+0avgdata 0maxresident)k
0inputs+0outputs (0major+2240296minor)pagefaults 0swaps
* ok 3: notes timing
* passed all 0 test(s)
-- snap --
Keep in mind that the tests run "git log" 99 times, and show the
accumulated time.
So it seems that an increase of roughly 40% in the user time, and roughly
70% in the system time is the price to have notes associated with every
single commit.
Note that in that very same repository, a single "git show" goes from
0.00user 0.00system 0:00.00elapsed 0%CPU (0avgtext+0avgdata
0maxresident)k
0inputs+0outputs (0major+561minor)pagefaults 0swaps
to this:
0.03user 0.02system 0:00.04elapsed 113%CPU (0avgtext+0avgdata
0maxresident)k
0inputs+0outputs (0major+2294minor)pagefaults 0swaps
(In another run, it only used 90%CPU)
That's not too shabby, given that Git needs to unpack double the number of
objects in this test when using notes vs. no notes.
For comparison, the numbers back then were something like 10% in user time
with a penalty of an extraordinary magnitude everytime the notes are
updated: around 800%.
Note: all these numbers are worst-case numbers, i.e. every commit has one
note.
To be frank, I do not completely understand why the numbers are that high.
I would have understood an increase roughly 4 seconds for reading the
quite large tree 99 times, and then the same ~0.20 seconds back then.
Maybe I made a huge mistake when implementing the thing.
And BTW, my code does not yet handle the case when
refs/notes/commits:$commit is a tree instead of a blob. That is left as
an exercise to the reader.
Johannes Schindelin (4):
Introduce commit notes
Add a script to edit/inspect notes
Speed up git notes lookup
Add an expensive test for git-notes
.gitignore | 1 +
Documentation/config.txt | 15 ++++
Documentation/git-notes.txt | 46 +++++++++++
Makefile | 3 +
cache.h | 3 +
command-list.txt | 1 +
commit.c | 1 +
config.c | 5 +
environment.c | 1 +
git-notes.sh | 65 +++++++++++++++
notes.c | 159 ++++++++++++++++++++++++++++++++++++++
notes.h | 7 ++
pretty.c | 5 +
t/t3301-notes.sh | 65 +++++++++++++++
t/t3302-notes-index-expensive.sh | 98 +++++++++++++++++++++++
15 files changed, 475 insertions(+), 0 deletions(-)
create mode 100644 Documentation/git-notes.txt
create mode 100755 git-notes.sh
create mode 100644 notes.c
create mode 100644 notes.h
create mode 100755 t/t3301-notes.sh
create mode 100755 t/t3302-notes-index-expensive.sh
^ 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