* [JGIT PATCH 1/2] Fix the UnpackedObjectCache hash function to prevent overflow
From: Shawn O. Pearce @ 2008-09-30 7:47 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: git
Sometimes we got negative array indexes inside of the cache due
to WindowedFile.hash having a negative value assigned by the JVM.
We now use only the lower 8 bits as the cache is fixed to at most
256 entries.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
.../org/spearce/jgit/lib/UnpackedObjectCache.java | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/UnpackedObjectCache.java b/org.spearce.jgit/src/org/spearce/jgit/lib/UnpackedObjectCache.java
index 03cf674..ee6a680 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/UnpackedObjectCache.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/UnpackedObjectCache.java
@@ -48,9 +48,9 @@
private static int hash(final WindowedFile pack, final long position) {
int h = pack.hash + (int) position;
- h += h >> 16;
- h += h >> 8;
- return h % CACHE_SZ;
+ h += h >>> 16;
+ h += h >>> 8;
+ return h & (CACHE_SZ - 1);
}
private static int maxByteCount;
--
1.6.0.2.463.g7f0eb
^ permalink raw reply related
* Re: [PATCH] Builtin-commit: show on which branch a commit was added
From: Jeff King @ 2008-09-30 7:09 UTC (permalink / raw)
To: Wincent Colaiuta
Cc: Andreas Ericsson, Pieter de Bie, Junio C Hamano, Git Mailinglist
In-Reply-To: <836C204F-F5AF-4887-99C9-04E70FEEB998@wincent.com>
On Tue, Sep 30, 2008 at 08:37:00AM +0200, Wincent Colaiuta wrote:
>> "commit" is just noise.
>
> Excellent point on the noise. Independently of whether the branch info
> gets added the word "commit" should probably be dropped.
The branch info has already been added, if you count it being in next
(in the form of "on $branch: ").
> As far as long-line-wrapping goes, I don't really think this is a problem
> for Git to solve (by truncation or any other means); it's more of a user
> behaviour thing where one would hope that users would get into the habit
> of using concise subject lines and branch names.
How concise must we be? I wrap my commit messages at 60 characters,
which I consider quite conservative. But
Created commit abcd1234 on jk/my-topic-branch:
takes up over half of an 80-column terminal. Is that a long branch name?
Browsing "git log --grep=Merge.branch --pretty=format:%s origin/next"
suggests it's not terribly out of line (at least by Junio's standards).
Dropping "commit " will help some. But given how much width is still
used, and the fact that this message is really just to say "yes, I
confirm that we just created the commit you asked for", I think
truncating (with dots) to keep it within 80 characters is reasonable.
-Peff
^ permalink raw reply
* [PATCH] git-gui: Do not automatically stage file after merge tool finishes
From: Johannes Sixt @ 2008-09-30 6:43 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Alexander Gavrilov, Git Mailing List
From: Johannes Sixt <johannes.sixt@telecom.at>
If a merge tool was invoked on a conflicted file and the tool completed,
then the conflicted file was staged automatically. However, the fact that
the user closed the merge tool cannot be understood as the unequivocal
sign that the conflict was completely resolved. For example, the user
could have decided to postpone the resolution of the conflict, or could
have accidentally closed the tool. We better leave the file unstaged and
let the user stage it explicitly.
Since the file is not staged anyway, the check for an unmodified
timestamp is pointless and removed.
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
I had sent this patch last week (but marked as RFC). Here it is again
without 'RFC' because I think it is a necessary change.
-- Hannes
lib/mergetool.tcl | 10 +---------
1 files changed, 1 insertions(+), 9 deletions(-)
diff --git a/lib/mergetool.tcl b/lib/mergetool.tcl
index 6ab5701..8d1ee5b 100644
--- a/lib/mergetool.tcl
+++ b/lib/mergetool.tcl
@@ -375,14 +375,6 @@ proc merge_tool_finish {fd} {
}
}
- # Check the modification time of the target file
- if {!$failed && [file mtime $mtool_target] eq $mtool_mtime} {
- if {[ask_popup [mc "File %s unchanged, still accept as resolved?" \
- [short_path $mtool_target]]] ne {yes}} {
- set failed 1
- }
- }
-
# Finish
if {$failed} {
file rename -force -- $backup $mtool_target
@@ -395,6 +387,6 @@ proc merge_tool_finish {fd} {
delete_temp_files $mtool_tmpfiles
- merge_add_resolution $mtool_target
+ reshow_diff
}
}
--
1.6.0.2.1262.ge466e
^ permalink raw reply related
* [PATCH] git-gui: Remove space from the end of aspell's reply before processing
From: Johannes Sixt @ 2008-09-30 6:39 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Git Mailing List
From: Johannes Sixt <johannes.sixt@telecom.at>
When git gui processes a reply from aspell it explicitly ignores an empty
line. The Windows version of aspell, however, terminates lines with CRLF,
but TCL's 'gets' does not remove CR, hence, a "visibly" empty line was not
actually recognized as empty. With this change we explicitly trim off
whitespace before the line is further processed.
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
There's probably a better solution by having TCL do the translation of
line endings, but I don't know how to do that.
-- Hannes
lib/spellcheck.tcl | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/lib/spellcheck.tcl b/lib/spellcheck.tcl
index a479b2f..e612030 100644
--- a/lib/spellcheck.tcl
+++ b/lib/spellcheck.tcl
@@ -314,6 +314,7 @@ method _run {} {
method _read {} {
while {[gets $s_fd line] >= 0} {
set lineno [lindex $s_pending 0 0]
+ set line [string trim $line]
if {$s_clear} {
$w_text tag remove misspelled "$lineno.0" "$lineno.end"
--
1.6.0.2.1262.ge466e
^ permalink raw reply related
* Re: [PATCH] Builtin-commit: show on which branch a commit was added
From: Wincent Colaiuta @ 2008-09-30 6:37 UTC (permalink / raw)
To: Andreas Ericsson
Cc: Jeff King, Pieter de Bie, Junio C Hamano, Git Mailinglist
In-Reply-To: <48E1C39F.4070906@op5.se>
El 30/9/2008, a las 8:13, Andreas Ericsson escribió:
> Jeff King wrote:
>> On Mon, Sep 29, 2008 at 10:09:17PM +0200, Pieter de Bie wrote:
>>> How about something like
>>>
>>> Created commit abcd1234 on widget -- "subwidget: one line summary"
>> I think that is probably just trading one visual problem for another.
>> That is, there are other people will have the same problem with "--"
>> that I had with ": ".
>
> Created 6207abc (subwidget: one quite long line of sum...) on <branch>
>
> "commit" is just noise.
Excellent point on the noise. Independently of whether the branch info
gets added the word "commit" should probably be dropped.
As far as long-line-wrapping goes, I don't really think this is a
problem for Git to solve (by truncation or any other means); it's more
of a user behaviour thing where one would hope that users would get
into the habit of using concise subject lines and branch names.
Cheers,
Wincent
^ permalink raw reply
* Re: [PATCH] Builtin-commit: show on which branch a commit was added
From: Jeff King @ 2008-09-30 6:16 UTC (permalink / raw)
To: Andreas Ericsson; +Cc: Pieter de Bie, Junio C Hamano, Git Mailinglist
In-Reply-To: <48E1C39F.4070906@op5.se>
On Tue, Sep 30, 2008 at 08:13:51AM +0200, Andreas Ericsson wrote:
> Created 6207abc (subwidget: one quite long line of sum...) on <branch>
>
> "commit" is just noise. Parentheses are often used to extemporize when
> using normal written language so it should work well here too.
I do like that better, and there is some precedent in the way we mention
commits in emails (I know Junio even has an alias that formats it as
$hash ($subject)).
By your "..." are you suggesting to truncate the subject?
-Peff
^ permalink raw reply
* Re: [PATCH] Builtin-commit: show on which branch a commit was added
From: Andreas Ericsson @ 2008-09-30 6:13 UTC (permalink / raw)
To: Jeff King; +Cc: Pieter de Bie, Junio C Hamano, Git Mailinglist
In-Reply-To: <20080929224430.GA11545@sigill.intra.peff.net>
Jeff King wrote:
> On Mon, Sep 29, 2008 at 10:09:17PM +0200, Pieter de Bie wrote:
>
>> How about something like
>>
>> Created commit abcd1234 on widget -- "subwidget: one line summary"
>
> I think that is probably just trading one visual problem for another.
> That is, there are other people will have the same problem with "--"
> that I had with ": ".
>
Created 6207abc (subwidget: one quite long line of sum...) on <branch>
"commit" is just noise. Parentheses are often used to extemporize when
using normal written language so it should work well here too.
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply
* Re: [PATCH] explicitly set LANG to 'C' in for guilt run-tests
From: Josef 'Jeff' Sipek @ 2008-09-30 4:42 UTC (permalink / raw)
To: Mikael Magnusson; +Cc: Scott Moser, git
In-Reply-To: <237967ef0809291325p7a0e3581vac348a1e99dbd4ed@mail.gmail.com>
On Mon, Sep 29, 2008 at 10:25:48PM +0200, Mikael Magnusson wrote:
...
> If I'm not mistaken, $LANG is used as the ultimate fallback, while LC_ALL is
> the one that overrides all others, so you probably want to set LC_ALL. I'm
> unsure which off the specific ones would apply here, but very likely it's
> LC_COLLATE. In other words, if LC_ALL is set, it is used, otherwise if
> LC_COLLATE is set it is used, otherwise if LANG is set, it is used,
> otherwise, "POSIX" is used.
I fixed up the patch to set LC_ALL instead, and committed it.
Thanks,
Josef 'Jeff' Sipek.
--
Humans were created by water to transport it upward.
^ permalink raw reply
* [JGIT PATCH] index-pack: Detect SHA-1 hash collisions to avoid replacing objects
From: Shawn O. Pearce @ 2008-09-30 3:54 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: git
When indexing a pack file coming in from the network there may
be an object contained in the pack that we already have. Its
only safe to retain that object in our newly stored pack if it
exactly matches the content we already have on disk. Storing
a different content for the same object name runs the risk of
allowing a malicious attacker to replace an object in our store.
Data validation is performed the first time we have the object
in memory, which occurs when we first compute its SHA-1. This
is the earliest opportunity we have to validate that there is
no collision, and it reduces the number of times we need to do
a read for an object. However our memory usage when we process
a whole object is now increased as the collision test is done
by loading the entire object into memory. Future improvements
may permit a streaming compare of the objects.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
This is only the first in what will be a series to add basic fsck
support to jgit. My day-job application is accepting untrusted
input through IndexPack so I need to fully validate the objects
are sane before I can allow them to enter the local repository.
In the end we'll honor receive.fsckObjects. But right now this
is enough to detect evil SHA-1 collisions, and is the same level
of protection that git.git's index-pack has.
I'm offering this up for comment because its done and ready for
inclusion. The rest of the object fsck rules are going to take
a bit more time, but I hope to have most of them done tomorrow or
the day after.
.../src/org/spearce/jgit/transport/IndexPack.java | 63 +++++++++++++++++---
1 files changed, 54 insertions(+), 9 deletions(-)
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java b/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java
index bc52896..8a66ec9 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java
@@ -375,6 +375,7 @@ private void resolveDeltas(final long pos, final int oldCRC, int type,
objectDigest.update(data);
tempObjectId.fromRaw(objectDigest.digest(), 0);
+ verifyNoCollision(type, data);
oe = new PackedObjectInfo(pos, crc32, tempObjectId);
entries[entryCount++] = oe;
}
@@ -622,7 +623,7 @@ private void indexOneObject() throws IOException {
r = new ArrayList<UnresolvedDelta>(8);
baseByPos.put(base, r);
}
- inflateFromInput(false);
+ skipInflateFromInput(sz);
r.add(new UnresolvedDelta(pos, (int) crc.getValue()));
deltaCount++;
break;
@@ -637,7 +638,7 @@ private void indexOneObject() throws IOException {
r = new ArrayList<UnresolvedDelta>(8);
baseById.put(base, r);
}
- inflateFromInput(false);
+ skipInflateFromInput(sz);
r.add(new UnresolvedDelta(pos, (int) crc.getValue()));
deltaCount++;
break;
@@ -649,17 +650,30 @@ private void indexOneObject() throws IOException {
private void whole(final int type, final long pos, final long sz)
throws IOException {
+ final byte[] data = inflateFromInput(sz);
objectDigest.update(Constants.encodedTypeString(type));
objectDigest.update((byte) ' ');
objectDigest.update(Constants.encodeASCII(sz));
objectDigest.update((byte) 0);
- inflateFromInput(true);
+ objectDigest.update(data);
tempObjectId.fromRaw(objectDigest.digest(), 0);
+ verifyNoCollision(type, data);
final int crc32 = (int) crc.getValue();
entries[entryCount++] = new PackedObjectInfo(pos, crc32, tempObjectId);
}
+ private void verifyNoCollision(final int type, final byte[] data)
+ throws IOException {
+ final ObjectLoader ldr = repo.openObject(tempObjectId);
+ if (ldr != null) {
+ final byte[] existingData = ldr.getCachedBytes();
+ if (ldr.getType() != type || !Arrays.equals(data, existingData)) {
+ throw new IOException("collision in " + tempObjectId.name());
+ }
+ }
+ }
+
// Current position of {@link #bOffset} within the entire file.
private long position() {
return bBase + bOffset;
@@ -747,7 +761,7 @@ private void sync() throws IOException {
bOffset = 0;
}
- private void inflateFromInput(final boolean digest) throws IOException {
+ private void skipInflateFromInput(long sz) throws IOException {
final Inflater inf = inflater;
try {
final byte[] dst = objectData;
@@ -765,21 +779,52 @@ private void inflateFromInput(final boolean digest) throws IOException {
int free = dst.length - n;
if (free < 8) {
- if (digest)
- objectDigest.update(dst, 0, n);
+ sz -= n;
n = 0;
free = dst.length;
}
-
n += inf.inflate(dst, n, free);
}
- if (digest)
- objectDigest.update(dst, 0, n);
+ if (n != sz)
+ throw new DataFormatException("wrong decompressed length");
+ n = bAvail - inf.getRemaining();
+ if (n > 0) {
+ crc.update(buf, p, n);
+ use(n);
+ }
+ } catch (DataFormatException dfe) {
+ throw corrupt(dfe);
+ } finally {
+ inf.reset();
+ }
+ }
+
+ private byte[] inflateFromInput(final long sz) throws IOException {
+ final byte[] dst = new byte[(int) sz];
+ final Inflater inf = inflater;
+ try {
+ int n = 0;
+ int p = -1;
+ while (!inf.finished()) {
+ if (inf.needsInput()) {
+ if (p >= 0) {
+ crc.update(buf, p, bAvail);
+ use(bAvail);
+ }
+ p = fillFromInput(1);
+ inf.setInput(buf, p, bAvail);
+ }
+
+ n += inf.inflate(dst, n, dst.length - n);
+ }
+ if (n != sz)
+ throw new DataFormatException("wrong decompressed length");
n = bAvail - inf.getRemaining();
if (n > 0) {
crc.update(buf, p, n);
use(n);
}
+ return dst;
} catch (DataFormatException dfe) {
throw corrupt(dfe);
} finally {
--
1.6.0.2.513.g6dbd
^ permalink raw reply related
* Re: Bug? git svn fetch: "unable to create temporary sha1 filename /home/andres/git/public/crystal.git/objects/96: File exists"
From: Andres Freund @ 2008-09-30 0:23 UTC (permalink / raw)
To: git
In-Reply-To: <200809300210.00285.andres@anarazel.de>
[-- Attachment #1: Type: text/plain, Size: 825 bytes --]
On Tuesday 30 September 2008, Andres Freund wrote in "Bug? git svn fetch:
"unable to create temporary sha1 filename
/home/andres/git/public/crystal.git/objects/96: File exists"":
> First failing svn fetch:
> M plugins/cscript/pycore/coremod.cpp
> M plugins/cscript/cspython/pytocs.cpp
> error: unable to create temporary sha1 filename
> /home/andres/git/public/crystal.git/objects/96: File exists
>
> fatal: Unable to add /tmp/Git_zIIUG4 to database
> hash-object -w --stdin-paths: command returned error: 128
>
> error closing pipe: Bad file descriptor at
> /usr/local/libexec/git-core/git-svn line 0
> error closing pipe: Bad file descriptor at
> /usr/local/libexec/git-core/git-svn line 0
After repacking the repository the problem is gone. Really rather strange.
Andres
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: [PATCH 1/6] gitweb: action in path with use_pathinfo
From: Jakub Narebski @ 2008-09-30 0:21 UTC (permalink / raw)
To: Giuseppe Bilotta; +Cc: git, Petr Baudis, Lea Wiemann
In-Reply-To: <cb7bb73a0809290722w5ed92171v98d6b83a7dae8f8b@mail.gmail.com>
On Mon, 29 Sep 2008, Giuseppe Bilotta wrote:
> On Mon, Sep 29, 2008 at 3:03 AM, Jakub Narebski <jnareb@gmail.com> wrote:
>> On Sun, 21 Sep 2008, Giuseppe Bilotta wrote:
>> Also, from what I understand, generated pathinfo links now always
>> use action, so they are a tiny little bit longer.
>
> Is that a problem, by the way? I've had half-thoughts about making the
> action implicit when possible, but I'm afraid that's prone to make the
> code way more complex and the path info handling much less robust.
No, I don't think there is a problem; if I remember correctly action
is omitted for default actions with and without project, i.e. for
projects list, and for 'summary' view for a project, which is default
view in absence of other parameters.
I would explain difference between then and now in the patch adding
support for _creating_ wider range of path_info links (I don't know,
perhaps you did that in new version):
* path_info URL were always without action, and were possible only
for the case of default action (projects list, summary), and in the
case of implicit action ('tree' for trees i.e. filename ending in
'/'; 'blob_plain' for ordinary files i.e. filename but no '/' at end;
'shortlog' for bare ref, assuming branch).
* now that pathinfo can contain action, wider range of URL can be done
as purely path_info links; in other way more of parameters can be put
in path_info part.
Perhaps not in so large and detailed form... I guess explanation of
using ':/' as separator should be put there as well, if you plan to
squash those patches.
>>> ---
>>> gitweb/gitweb.perl | 109 ++++++++++++++++++++++++++++++++++------------------
>>> 1 files changed, 72 insertions(+), 37 deletions(-)
>>>
>>> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
>>> index da474d0..e783d12 100755
>>> --- a/gitweb/gitweb.perl
>>> +++ b/gitweb/gitweb.perl
>> Well, you could instead split hash declaration from defining it,
>> in the form of
>>
>> my %actions = ();
>> ...
>> %actions = (
>> ...
>> );
>>
>> but I guess moving declaration earlier is good solution.
>
> Is there some coding style recommendation wrt this situations, or is
> it just a matter of making the patch smaller?
I think that moving %actions earlier is a better solution.
>>> # now read PATH_INFO and use it as alternative to parameters
>>> sub evaluate_path_info {
>>> return if defined $project;
>>> @@ -512,6 +543,16 @@ sub evaluate_path_info {
>>> # do not change any parameters if an action is given using the query string
>>> return if $action;
>>> $path_info =~ s,^\Q$project\E/*,,;
>>> +
>>> + # next comes the action
>>> + $action = $path_info;
>>> + $action =~ s,/.*$,,;
>>
>> I would use perhaps "($action) = ($path_info =~ m!^([^/]+)!);"
>> But that is Perl, so TIMTOWDI.
>
> Well, Perl is not my native language so I tend to stay away from
> complex expressions if possible ;-)
What I meant is instead of "copy and strip" use "find match".
I tried to use one-liner, but it could be written instead as:
+ # next comes the action
+ if ($path_info =~ m!^([^/]+)!) {;
+ $action = $1;
+ }
But I guess your approach is equally valid. I don't think of myself
being a Perl expert, either.
>>> @@ -525,10 +566,12 @@ sub evaluate_path_info {
>>> }
>>> $hash_base ||= validate_refname($refname);
>>> $file_name ||= validate_pathname($pathname);
>>> + $hash ||= git_get_hash_by_path($hash_base, $file_name);
>>
>> I don't understand why you feel that you need to do this (this is
>> additional git command fork, as git_get_hash_by_path calls Git, to
>> be more exact it calls git-ls-tree (it could call git-rev-parse
>> instead). Moreover, I don't understand why you need to do this _here_,
>> instead of just before where you would have to have $hash variable set.
>
> Hm. I must confess that I honestly don't remember. The same holds for
> the other chunks you have perplexities on. When I started writing
> these patches I came across a few situations where $hash wouldn't
> carry over properly, but now I can't seem to recreate those issues
> anymore, which leads me to suspect it was a problem with hand-crafted
> links (i.e. before I coded the link generation part too). I'll resend
> without these chunks.
That is the problem, but not as large a problem as having similar code
calling git_get_hash_by_path() during link generation, in href(...)
subroutine (at least without Lea's gitweb caching, the part that reuses
"git cat-file --batch"). This is called once per view (page); that
was called once per generated gitweb link...
>>> @@ -624,8 +636,13 @@ sub href (%) {
>>> if ($params{-replay}) {
>>> while (my ($name, $symbol) = each %mapping) {
>>> if (!exists $params{$name}) {
>>> - # to allow for multivalued params we use arrayref form
>>> - $params{$name} = [ $cgi->param($symbol) ];
>>> + if ($cgi->param($symbol)) {
>>> + # to allow for multivalued params we use arrayref form
>>> + $params{$name} = [ $cgi->param($symbol) ];
>>> + } else {
>>> + no strict 'refs';
>>> + $params{$name} = $$name if $$name;
>>> + }
>>> }
>>> }
>>> }
>>
>> What this change is about? And why this change is _here_, in this
>> commit? It is I think unrelated, and wrong change.
>
> This is about being able to recycle CGI parameters that came through
> as part of path_info instead of the CGI parameter list. It might not
> be the best way to recover it, though. I *did* have a few thoughts
> about an alternative way that consisted of build a parameter list
> merging CGI and path-info parameter, but since this approach seemed to
> work, I went with it.
Fact, I have totally forgot about this.
>> href(..., -replay=>1) is all about reusing current URL, perhaps with
>> a few parameters changed, like for example pagination links differ only
>> in page number param change. For example if we had only hb= and f=
>> parameters, -replay=>1 links should use only those, and not add h=
>> parameter because somewhere we felt that we need $hash to be calculated.
>
> Assume for example that you are to an url such as
>
> http://git.oblomov.eu/git/tree/refs/remotes/origin/master:gitweb
>
> Without this patch, the 'history' link on the second header line links
> to ARRAY(0xblah)ARRAY(0xblah). With this patch, it shows the proper
> link. So either replay is being abused somewhere in the link
> generation code, or this CGI+path_info parameter retrieval is
> necessary, one way or the other.
Ah. Now I understand.
When creating code for href(..., -replay=>1), which by the way I thought
would be more useful than actually is, I have forgot that parameters to
gitweb could be passed in other way that through CGI parameters
(CGI query)[1].
Using
$params{$name} = [ $cgi->param($symbol) ];
is a cute hack, but it doesn't work for arguments passed via path_info
(was: project, hash_base and file_name; while now it is project, action,
hash_base (in full) and file_name).
The solution I thought about and abandoned in favor of this cute hack
was to have additional hash (in addition to %mapping), which would map
action names to references to variables holding the value for parameter.
This has the same problem as your proposed solution of putting some
parameters which didn't come from URL but were filled from other info.
$hash parameter is most likely to be culprit here.
On the other hand it is more generic and doesn't rely on knowledge that
there is no multi-valued parameter which can be expressed in path_info
(currently only 'opt' parameter can be multi-valued, and requires
arrayref, but also 'opt' parameter is and cannot be put in path_info).
I am talking there about the following solution:
my %action_vars = (
project => \$project,
action => \$action,
# ...
extra_options => \@extra_options,
);
# ...
while (my ($name, $symbol) = each %mapping) {
if (!exists $params{$name}) {
$params{$name} = ${$action_vars{$name}};
}
}
This avoids cure hack of (from your code)
} else {
no strict 'refs';
$params{$name} = $$name if $$name;
}
I think that gitweb should use single source, not CGI query parameters
or variable saving [sanitized] value.
[*1*] Currently parameters can be passed either as CGI query parameters
(which I remember about), but also (with some restrictions) in the
path_info part of gitweb URLs. If we implement command line
switches (for example to generate directory listing in format
expected by gitweb for file $projects_list, or for off-line
generation of RSS feed), there could be yet another source.
>>> @@ -636,10 +653,28 @@ sub href (%) {
>>> + # next, we put either hash_base:file_name or hash
>>> + if (defined $params{'hash_base'}) {
>>> + $href .= "/".esc_url($params{'hash_base'});
>>> + if (defined $params{'file_name'}) {
>>> + $href .= ":".esc_url($params{'file_name'});
>>> + delete $params{'hash'} if $params{'hash'} eq git_get_hash_by_path($params{'hash_base'},$params{'file_name'});
>>
>> First, this page has around 140 characters. That is too long, much too long.
>> Please try to wrap code around 80-characters.
>>
>> Second, following Petr 'Pasky' Baudis suggestion of reducing complexity
>> and shortening gitweb URLs, we could unconditionally remove redundant
>> 'hash' parameter if we have both 'hash_base' and 'file_name'
>> parameters. This would also simplify and speed up (lack of extra fork)
>> gitweb code.
>
> If it's indeed guaranteed that hash is not needed in these cases, it's
> surely the best course of action. I changed the code to that effect.
Hash is not needed for hash_base:file_name, unless you messed up
something (by hand-crafting URL). If they do not much, you have more
problems than that...
>> Hmmm... now I am not so sure if it wouldn't be better to split this
>> patch in pathinfo parsing and pathinfo generation. The first part
>> would be obvious, the second part would be smaller and easier to review.
>
> Ok, I'll split parsing from generation. Since it's what I did for
> subsequent extensions (such as the parent..current thing) it also fits
> nicely with the patchflow.
Thanks.
--
Jakub Narebski
Poland
^ permalink raw reply
* Bug? git svn fetch: "unable to create temporary sha1 filename /home/andres/git/public/crystal.git/objects/96: File exists"
From: Andres Freund @ 2008-09-30 0:09 UTC (permalink / raw)
To: git
[-- Attachment #1: Type: text/plain, Size: 2164 bytes --]
Hi,
I regularly run git-svn fetch on some mirrors of svn repositories - which
works quite well.
But now I started to get this error:
Last successfull git svn fetch:
M src/z3c/form/browser/file-testing.txt
r91613 = 27c991067847b4a54a4d8bc71d440c74893475af (trunk)
From .
c7c32b3..27c9910 trunk -> trunk
First failing svn fetch:
M plugins/cscript/pycore/coremod.cpp
M plugins/cscript/cspython/pytocs.cpp
error: unable to create temporary sha1 filename
/home/andres/git/public/crystal.git/objects/96: File exists
fatal: Unable to add /tmp/Git_zIIUG4 to database
hash-object -w --stdin-paths: command returned error: 128
error closing pipe: Bad file descriptor at /usr/local/libexec/git-core/git-svn
line 0
error closing pipe: Bad file descriptor at /usr/local/libexec/git-core/git-svn
line 0
M src/z3c/form/tests/test_doc.py
r91624 = 6a9a5c03c44618ca1cc34cf428c94b25009c6fc4 (trunk)
A src/z3c/form/testing.txt
r91625 = a10508daf71d4e8aaad5531f5bcf8cb4ef0695e8 (trunk)
From .
27c9910..a10508d trunk -> trunk
All further git svn fetch invocation result in something alike:
Index mismatch: c8858cc0b587ab638af9ed52e7c03c8966b7db63 !=
c32a46c51a046500a3765fe573b95e2eb2f44b2a
rereading 87e2a16d8c259eba290bde0512af4d6bc1f64d1f
M plugins/cscript/pycore/coremod.cpp
M plugins/cscript/cspython/pytocs.cpp
error: unable to create temporary sha1 filename
/home/andres/git/public/crystal.git/objects/96: File exists
fatal: Unable to add /tmp/Git_YSlrAs to database
hash-object -w --stdin-paths: command returned error: 128
error closing pipe: Bad file descriptor at /usr/local/libexec/git-core/git-svn
line 0
error closing pipe: Bad file descriptor at /usr/local/libexec/git-core/git-svn
line 0
svn fetch is always invocated as "git --bare svn fetch".
Git repository is located at:
git://git.anarazel.de/crystal.git
http://git.anarazel.de/git/crystal.git
Original svn repository is located at:
https://crystal.svn.sourceforge.net/svnroot/crystal
Any idea?
Thanks,
Andres
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* [PATCH v2] Add OS X support to the pre-auto-gc example hook
From: Jonathan del Strother @ 2008-09-29 23:36 UTC (permalink / raw)
To: git; +Cc: Miklos Vajna, Jonathan del Strother
In-Reply-To: <57518fd10809270253s4c07318bjd54c7d86460ce7d7@mail.gmail.com>
Signed-off-by: Jonathan del Strother <jon.delStrother@bestbefore.tv>
---
Second attempt - this simplifies the test while making it more specific (it will only pack when on AC power, rather than, say, UPS).
contrib/hooks/pre-auto-gc-battery | 10 +++++++---
1 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/contrib/hooks/pre-auto-gc-battery b/contrib/hooks/pre-auto-gc-battery
index 0096f57..b0a8caa 100644
--- a/contrib/hooks/pre-auto-gc-battery
+++ b/contrib/hooks/pre-auto-gc-battery
@@ -1,9 +1,9 @@
#!/bin/sh
#
# An example hook script to verify if you are on battery, in case you
-# are running Linux. Called by git-gc --auto with no arguments. The hook
-# should exit with non-zero status after issuing an appropriate message
-# if it wants to stop the auto repacking.
+# are running Linux or OS X. Called by git-gc --auto with no arguments.
+# The hook should exit with non-zero status after issuing an appropriate
+# message if it wants to stop the auto repacking.
#
# This hook is stored in the contrib/hooks directory. Your distribution
# may have put this somewhere else. If you want to use this hook, you
@@ -30,6 +30,10 @@ then
elif grep -q '0x01$' /proc/apm 2>/dev/null
then
exit 0
+elif test -x /usr/bin/pmset && /usr/bin/pmset -g batt |
+ grep -q "Currently drawing from 'AC Power'"
+then
+ exit 0
fi
echo "Auto packing deferred; not on AC"
--
1.6.0.2.308.gd442a.dirty
^ permalink raw reply related
* [PATCH 1/1] Replace svn.foo.org with svn.example.com in git-svn docs (RFC 2606)
From: Michael Prokop @ 2008-09-29 23:01 UTC (permalink / raw)
To: git; +Cc: Michael Prokop
foo.org is an existing domain, use RFC 2606 complying example.com instead
as used in other docs as well.
Signed-off-by: Michael Prokop <mika@grml.org>
---
Documentation/git-svn.txt | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
index 1e644ca..82d03b4 100644
--- a/Documentation/git-svn.txt
+++ b/Documentation/git-svn.txt
@@ -473,7 +473,7 @@ Tracking and contributing to the trunk of a Subversion-managed project:
------------------------------------------------------------------------
# Clone a repo (like git clone):
- git svn clone http://svn.foo.org/project/trunk
+ git svn clone http://svn.example.com/project/trunk
# Enter the newly cloned directory:
cd trunk
# You should be on master branch, double-check with git-branch
@@ -495,7 +495,7 @@ Tracking and contributing to an entire Subversion-managed project
------------------------------------------------------------------------
# Clone a repo (like git clone):
- git svn clone http://svn.foo.org/project -T trunk -b branches -t tags
+ git svn clone http://svn.example.com/project -T trunk -b branches -t tags
# View all branches and tags you have cloned:
git branch -r
# Reset your master to trunk (or any other branch, replacing 'trunk'
@@ -514,7 +514,7 @@ have each person clone that repository with 'git-clone':
------------------------------------------------------------------------
# Do the initial import on a server
- ssh server "cd /pub && git svn clone http://svn.foo.org/project
+ ssh server "cd /pub && git svn clone http://svn.example.com/project
# Clone locally - make sure the refs/remotes/ space matches the server
mkdir project
cd project
@@ -523,7 +523,7 @@ have each person clone that repository with 'git-clone':
git config --add remote.origin.fetch '+refs/remotes/*:refs/remotes/*'
git fetch
# Initialize git-svn locally (be sure to use the same URL and -T/-b/-t options as were used on server)
- git svn init http://svn.foo.org/project
+ git svn init http://svn.example.com/project
# Pull the latest changes from Subversion
git svn rebase
------------------------------------------------------------------------
--
1.5.6.5
^ permalink raw reply related
* Re: [PATCH 2/6] gitweb: use_pathinfo filenames start with /
From: Jakub Narebski @ 2008-09-29 23:20 UTC (permalink / raw)
To: Giuseppe Bilotta; +Cc: git, Petr Baudis, Lea Wiemann
In-Reply-To: <cb7bb73a0809290712g324ec015r70fd868b91673645@mail.gmail.com>
On Mon, 29 Sep 2008, Giuseppe Bilotta wrote:
> On Mon, Sep 29, 2008 at 3:08 AM, Jakub Narebski <jnareb@gmail.com> wrote:
>> On Sun, 21 Sep 2008, Giuseppe Bilotta wrote:
>>> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
>>> index e783d12..18da484 100755
>>> --- a/gitweb/gitweb.perl
>>> +++ b/gitweb/gitweb.perl
>>> @@ -664,7 +664,7 @@ sub href (%) {
>>> if (defined $params{'hash_base'}) {
>>> $href .= "/".esc_url($params{'hash_base'});
>>> if (defined $params{'file_name'}) {
>>> - $href .= ":".esc_url($params{'file_name'});
>>> + $href .= ":/".esc_url($params{'file_name'});
>>> delete $params{'hash'} if $params{'hash'} eq git_get_hash_by_path($params{'hash_base'},$params{'file_name'});
>>> delete $params{'file_name'};
>>> } else {
>>> --
>>> 1.5.6.5
>>
>> Is there reason why this change is separate (not squashed) from
>> previous commit?
>
> Historical reason (i.e. I came up with the idea later on). I'll squash it.
Hn. Now I am not sure if it should be squashed, or should be separate.
Good change.
Please don't forget to describe this decision, i.e. why gitweb is
using "branch:/filename" when creating path_info links instead of
simple "branch:name" for relative URLs in HTML in 'raw' ('blob_plain')
view, in the commit message. Thanks in advance.
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [PATCH] Builtin-commit: show on which branch a commit was added
From: Jeff King @ 2008-09-29 22:44 UTC (permalink / raw)
To: Pieter de Bie; +Cc: Junio C Hamano, Git Mailinglist
In-Reply-To: <A36A4B61-D223-4821-9969-FA76EAECD1EC@ai.rug.nl>
On Mon, Sep 29, 2008 at 10:09:17PM +0200, Pieter de Bie wrote:
> How about something like
>
> Created commit abcd1234 on widget -- "subwidget: one line summary"
I think that is probably just trading one visual problem for another.
That is, there are other people will have the same problem with "--"
that I had with ": ".
And of course it doesn't deal with the line length issues.
Anyway, I seem to be the only one complaining, so perhaps it should just
be left as-is.
-Peff
^ permalink raw reply
* [PATCH] diff.c: remove duplicate bibtex pattern introduced by merge 92bb9785
From: Brandon Casey @ 2008-09-29 21:52 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Git Mailing List
In-Reply-To: <ZOivOHIiBa1yoDqFPq18uB0VuTttUsV4lS5k7YcyEsM@cipher.nrlssc.navy.mil>
Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
---
diff.c | 2 --
1 files changed, 0 insertions(+), 2 deletions(-)
diff --git a/diff.c b/diff.c
index b001d7b..7c982b4 100644
--- a/diff.c
+++ b/diff.c
@@ -1439,8 +1439,6 @@ static const struct funcname_pattern_entry builtin_funcname_pattern[] = {
{ "python", "^[ \t]*((class|def)[ \t].*)$", REG_EXTENDED },
{ "ruby", "^[ \t]*((class|module|def)[ \t].*)$",
REG_EXTENDED },
- { "bibtex", "(@[a-zA-Z]{1,}[ \t]*\\{{0,1}[ \t]*[^ \t\"@',\\#}{~%]*).*$",
- REG_EXTENDED },
{ "tex",
"^(\\\\((sub)*section|chapter|part)\\*{0,1}\\{.*)$",
REG_EXTENDED },
--
1.6.0.2.323.g7c850
^ permalink raw reply related
* Re: [PATCH] explicitly set LANG to 'C' in for guilt run-tests
From: Scott Moser @ 2008-09-29 21:01 UTC (permalink / raw)
To: Josef Jeff Sipek; +Cc: Mikael Magnusson, git
In-Reply-To: <20080929204958.GD31590@josefsipek.net>
On Mon, 29 Sep 2008, Josef Jeff Sipek wrote:
> > If I'm not mistaken, $LANG is used as the ultimate fallback, while LC_ALL is
> > the one that overrides all others, so you probably want to set LC_ALL. I'm
> > unsure which off the specific ones would apply here, but very likely it's
> > LC_COLLATE. In other words, if LC_ALL is set, it is used, otherwise if
> > LC_COLLATE is set it is used, otherwise if LANG is set, it is used,
> > otherwise, "POSIX" is used.
>
> IIRC, my devel system has all of them set to UTF8, _except_ LC_COLLATE (I
> like the case sensitive sort of filenames in ls(1)) which I have set to "C".
> So chances are that the minimum required is LC_COLLATE=C, but overriding
> everything might be safer overall.
>
Yeah, LC_ALL I sprobably correct. I'm not very "LANG" aware at all. I
just noticed that test 052 didn't run on my system, and figured out that
was why.
You want a re-send of this patch with LC_ALL ? Or do you want to make
the modification yourself and apply?
Scott
^ permalink raw reply
* Re: [PATCH] explicitly set LANG to 'C' in for guilt run-tests
From: Josef Jeff Sipek @ 2008-09-29 20:49 UTC (permalink / raw)
To: Mikael Magnusson; +Cc: Scott Moser, git
In-Reply-To: <237967ef0809291325p7a0e3581vac348a1e99dbd4ed@mail.gmail.com>
On Mon, Sep 29, 2008 at 10:25:48PM +0200, Mikael Magnusson wrote:
> 2008/9/29 Scott Moser <smoser@brickies.net>:
> > The output of guilt's run-tests is dependent on LANG due to reliance on a
> > given sorting algorithm. Currently, the test '052' will fail if LANG is
> > set to 'en_US.UTF-8' (and likely others values).
> >
> > Remove the assumption by explicitly setting this in run-tests.
> >
> > Signed-off-by: Scott Moser <smoser@brickies.net>
> > ---
> > regression/run-tests | 1 +
> > 1 files changed, 1 insertions(+), 0 deletions(-)
> >
> > diff --git a/regression/run-tests b/regression/run-tests
> > index 8f572eb..945150b 100755
> > --- a/regression/run-tests
> > +++ b/regression/run-tests
> > @@ -2,6 +2,7 @@
> >
> > export REG_DIR="$PWD"
> > export PATH="$PWD/bin:$PATH"
> > +export LANG=C
> >
> > source scaffold
> >
> > --
> > 1.5.6.3
>
> If I'm not mistaken, $LANG is used as the ultimate fallback, while LC_ALL is
> the one that overrides all others, so you probably want to set LC_ALL. I'm
> unsure which off the specific ones would apply here, but very likely it's
> LC_COLLATE. In other words, if LC_ALL is set, it is used, otherwise if
> LC_COLLATE is set it is used, otherwise if LANG is set, it is used,
> otherwise, "POSIX" is used.
IIRC, my devel system has all of them set to UTF8, _except_ LC_COLLATE (I
like the case sensitive sort of filenames in ls(1)) which I have set to "C".
So chances are that the minimum required is LC_COLLATE=C, but overriding
everything might be safer overall.
Josef 'Jeff' Sipek.
--
Penguin : Linux version 2.6.25.4 on an i386 machine (6135.73 BogoMips).
^ permalink raw reply
* Re: [PATCH] explicitly set LANG to 'C' in for guilt run-tests
From: Mikael Magnusson @ 2008-09-29 20:25 UTC (permalink / raw)
To: Scott Moser; +Cc: Josef Jeff Sipek, git
In-Reply-To: <1222714272-9557-1-git-send-email-smoser@brickies.net>
2008/9/29 Scott Moser <smoser@brickies.net>:
> The output of guilt's run-tests is dependent on LANG due to reliance on a
> given sorting algorithm. Currently, the test '052' will fail if LANG is
> set to 'en_US.UTF-8' (and likely others values).
>
> Remove the assumption by explicitly setting this in run-tests.
>
> Signed-off-by: Scott Moser <smoser@brickies.net>
> ---
> regression/run-tests | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/regression/run-tests b/regression/run-tests
> index 8f572eb..945150b 100755
> --- a/regression/run-tests
> +++ b/regression/run-tests
> @@ -2,6 +2,7 @@
>
> export REG_DIR="$PWD"
> export PATH="$PWD/bin:$PATH"
> +export LANG=C
>
> source scaffold
>
> --
> 1.5.6.3
If I'm not mistaken, $LANG is used as the ultimate fallback, while LC_ALL is
the one that overrides all others, so you probably want to set LC_ALL. I'm
unsure which off the specific ones would apply here, but very likely it's
LC_COLLATE. In other words, if LC_ALL is set, it is used, otherwise if
LC_COLLATE is set it is used, otherwise if LANG is set, it is used,
otherwise, "POSIX" is used.
--
Mikael Magnusson
^ permalink raw reply
* [PATCH] Typos.
From: Ralf Wildenhues @ 2008-09-29 20:30 UTC (permalink / raw)
To: git
---
Documentation/RelNotes-1.6.1.txt | 4 ++--
Documentation/git-read-tree.txt | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/Documentation/RelNotes-1.6.1.txt b/Documentation/RelNotes-1.6.1.txt
index 421e569..906932c 100644
--- a/Documentation/RelNotes-1.6.1.txt
+++ b/Documentation/RelNotes-1.6.1.txt
@@ -59,7 +59,7 @@ on.
* "git daemon" learned --max-connections=<count> option.
-* "git diff" learned to mimick --suppress-blank-empty from GNU diff via a
+* "git diff" learned to mimic --suppress-blank-empty from GNU diff via a
configuration option.
* "git diff" learned to put more sensible hunk headers for Python and
@@ -121,7 +121,7 @@ release, unless otherwise noted.
is a path in it).
* "git diff --stdin" used to take two trees on a line and compared them,
- but we droppped support for such a use case long time ago. This has
+ but we dropped support for such a use case long time ago. This has
been resurrected.
* "git filter-branch" failed to rewrite a tag name with slashes in it.
diff --git a/Documentation/git-read-tree.txt b/Documentation/git-read-tree.txt
index 309deac..7160fa1 100644
--- a/Documentation/git-read-tree.txt
+++ b/Documentation/git-read-tree.txt
@@ -212,7 +212,7 @@ output after two-tree merge.
Case #3 is slightly tricky and needs explanation. The result from this
rule logically should be to remove the path if the user staged the removal
-of the path and then swiching to a new branch. That however will prevent
+of the path and then switching to a new branch. That however will prevent
the initial checkout from happening, so the rule is modified to use M (new
tree) only when the contents of the index is empty. Otherwise the removal
of the path is kept as long as $H and $M are the same.
--
1.6.0.1.309.g48068
^ permalink raw reply related
* Re: [PATCH] remove vim syntax highlighting in favor of upstream
From: Jeff King @ 2008-09-29 20:12 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: Shawn O. Pearce, git
In-Reply-To: <20080929200814.GA19840@neumann>
On Mon, Sep 29, 2008 at 10:08:14PM +0200, SZEDER Gábor wrote:
> Here it is. Since significant parts of the patch and the commit
> message are from Jeff, maybe he should sign off, too?
I think all of my changes were deletions, so I'm not sure there is any
copyright to claim. ;) But:
Signed-off-by: Jeff King <peff@peff.net>
> Note, that this patch is slightly different from the previous one, as
> it proposes writing the auto-detect commands into ~/.vim/filetype.vim
> instead of ~/.vimrc. It's not quite clear to me why, but it seems to
> resolve the filetype confusion I mentioned in my previous email.
I haven't really tested this at all, as I have been running vim 7.2 for
a while now. But presumably it works for you, and hey, it's only
contrib/. :)
-Peff
^ permalink raw reply
* Re: [PATCH] Builtin-commit: show on which branch a commit was added
From: Pieter de Bie @ 2008-09-29 20:09 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Git Mailinglist
In-Reply-To: <20080921104238.GA9217@sigill.intra.peff.net>
(Sorry for a late response)
On 21 sep 2008, at 12:42, Jeff King wrote:
> OK, I have lived with it for a little while, and I am still
> annoyed. ;)
>
> My complaints are:
>
> 1. It wastes more horizontal screen real estate, making it more
> likely
> that the line will wrap.
>
> 2. In almost all of my projects (including git), I use the subject
> line convention of "subsystem: one line summary". So you end up
> with the visually confusing:
>
> Created commit abcd1234 on master: subsystem: one line summary
>
> which is even worse on a topic branch which is meaningful to the
> project:
>
> Created commit abcd1234 on widget: subwidget: one line summary
>
> which has literally left me scratching my head wondering why I put
> "widget" into the commit message.
How about something like
Created commit abcd1234 on widget -- "subwidget: one line summary"
?
> Maybe it is better to simply break the line, which solves both
> problems.
> Something like:
I don't like a multi-line approach.. I tried it myself, and the second
line
makes the first line easier to overlook
- Pieter
^ permalink raw reply
* [PATCH] remove vim syntax highlighting in favor of upstream
From: SZEDER Gábor @ 2008-09-29 20:08 UTC (permalink / raw)
To: Shawn O. Pearce, Jeff King; +Cc: vim, git
In-Reply-To: <20080929145542.GA18340@spearce.org>
As of version 7.2, vim ships with its own syntax
highlighting for git commit messages, which is:
1. more comprehensive in splitting up the various
components of the file
2. in accordance with the usual vim behavior for syntax
highlighting (e.g., respecting b:current_syntax)
3. presumably better maintained (I have not been using
what's in git's contrib/ directory for some time in
favor of the upstream version)
Furthermore, vim upsream also provides syntax highlighting
for other git filetypes (gitconfig, rebase, send-email).
This patch gets rid of our local version and just points
interested parties to the upstream version.
The code for auto-detecting filetypes is taken from vim's
runtime/filetype.vim.
Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
On Mon, Sep 29, 2008 at 07:55:42AM -0700, Shawn O. Pearce wrote:
> Missing SBO line?
Here it is. Since significant parts of the patch and the commit
message are from Jeff, maybe he should sign off, too?
Note, that this patch is slightly different from the previous one, as
it proposes writing the auto-detect commands into ~/.vim/filetype.vim
instead of ~/.vimrc. It's not quite clear to me why, but it seems to
resolve the filetype confusion I mentioned in my previous email.
contrib/vim/README | 38 ++++++++++++++++++++++++++++++--------
contrib/vim/syntax/gitcommit.vim | 18 ------------------
2 files changed, 30 insertions(+), 26 deletions(-)
delete mode 100644 contrib/vim/syntax/gitcommit.vim
diff --git a/contrib/vim/README b/contrib/vim/README
index 9e7881f..c487346 100644
--- a/contrib/vim/README
+++ b/contrib/vim/README
@@ -1,8 +1,30 @@
-To syntax highlight git's commit messages, you need to:
- 1. Copy syntax/gitcommit.vim to vim's syntax directory:
- $ mkdir -p $HOME/.vim/syntax
- $ cp syntax/gitcommit.vim $HOME/.vim/syntax
- 2. Auto-detect the editing of git commit files:
- $ cat >>$HOME/.vimrc <<'EOF'
- autocmd BufNewFile,BufRead COMMIT_EDITMSG set filetype=gitcommit
- EOF
+Syntax highlighting for git commit messages, config files, etc. is
+included with the vim distribution as of vim 7.2, and should work
+automatically.
+
+If you have an older version of vim, you can get the latest syntax
+files from the vim project:
+
+ http://vim.svn.sourceforge.net/viewvc/vim/trunk/runtime/syntax/git.vim
+ http://vim.svn.sourceforge.net/viewvc/vim/trunk/runtime/syntax/gitcommit.vim
+ http://vim.svn.sourceforge.net/viewvc/vim/trunk/runtime/syntax/gitconfig.vim
+ http://vim.svn.sourceforge.net/viewvc/vim/trunk/runtime/syntax/gitrebase.vim
+ http://vim.svn.sourceforge.net/viewvc/vim/trunk/runtime/syntax/gitsendemail.vim
+
+To install:
+
+ 1. Copy these files to vim's syntax directory $HOME/.vim/syntax
+ 2. To auto-detect the editing of various git-related filetypes:
+ $ cat >>$HOME/.vim/filetype.vim <<'EOF'
+ autocmd BufNewFile,BufRead *.git/COMMIT_EDITMSG setf gitcommit
+ autocmd BufNewFile,BufRead *.git/config,.gitconfig setf gitconfig
+ autocmd BufNewFile,BufRead git-rebase-todo setf gitrebase
+ autocmd BufNewFile,BufRead .msg.[0-9]*
+ \ if getline(1) =~ '^From.*# This line is ignored.$' |
+ \ setf gitsendemail |
+ \ endif
+ autocmd BufNewFile,BufRead *.git/**
+ \ if getline(1) =~ '^\x\{40\}\>\|^ref: ' |
+ \ setf git |
+ \ endif
+ EOF
diff --git a/contrib/vim/syntax/gitcommit.vim b/contrib/vim/syntax/gitcommit.vim
deleted file mode 100644
index 332121b..0000000
--- a/contrib/vim/syntax/gitcommit.vim
+++ /dev/null
@@ -1,18 +0,0 @@
-syn region gitLine start=/^#/ end=/$/
-syn region gitCommit start=/^# Changes to be committed:$/ end=/^#$/ contains=gitHead,gitCommitFile
-syn region gitHead contained start=/^# (.*)/ end=/^#$/
-syn region gitChanged start=/^# Changed but not updated:/ end=/^#$/ contains=gitHead,gitChangedFile
-syn region gitUntracked start=/^# Untracked files:/ end=/^#$/ contains=gitHead,gitUntrackedFile
-
-syn match gitCommitFile contained /^#\t.*/hs=s+2
-syn match gitChangedFile contained /^#\t.*/hs=s+2
-syn match gitUntrackedFile contained /^#\t.*/hs=s+2
-
-hi def link gitLine Comment
-hi def link gitCommit Comment
-hi def link gitChanged Comment
-hi def link gitHead Comment
-hi def link gitUntracked Comment
-hi def link gitCommitFile Type
-hi def link gitChangedFile Constant
-hi def link gitUntrackedFile Constant
--
1.6.0.2.330.gcef5c
^ permalink raw reply related
* [PATCH] fix guilt-pop and push to fail if no relevant patches
From: Scott Moser @ 2008-09-29 18:51 UTC (permalink / raw)
To: Josef "Jeff" Sipek; +Cc: git, Scott Moser
currently guilt-pop and guilt-push will exit with '0' if there are no more
relevant patches in the series (ie, if you've pushed or popped all of them)
This means that you cannot do something like:
while guilt-push; do
guilt refresh || break
done
for reference, quilt does exit with non-zero in those cases:
$ quilt push -a && quilt push
File series fully applied, ends at patch my.patch
$ echo $?
1
$ quilt pop -a; quilt pop
No patch removed
$ echo $?
2
Signed-off-by: Scott Moser <smoser@brickies.net>
---
guilt-pop | 3 +--
guilt-push | 43 ++++++++++++++++++++++++-------------------
regression/t-021.out | 3 +++
3 files changed, 28 insertions(+), 21 deletions(-)
diff --git a/guilt-pop b/guilt-pop
index db8473e..8a83fdb 100755
--- a/guilt-pop
+++ b/guilt-pop
@@ -45,8 +45,7 @@ patch="$1"
[ ! -z "$all" ] && patch="-a"
if [ ! -s "$applied" ]; then
- disp "No patches applied."
- exit 0
+ die "No patches applied."
elif [ "$patch" = "-a" ]; then
# we are supposed to pop all patches
diff --git a/guilt-push b/guilt-push
index 018f9ac..48f886b 100755
--- a/guilt-push
+++ b/guilt-push
@@ -97,22 +97,27 @@ fi
sidx=`wc -l < $applied`
sidx=`expr $sidx + 1`
-get_series | sed -n -e "${sidx},${eidx}p" | while read p
-do
- disp "Applying patch..$p"
- if [ ! -f "$GUILT_DIR/$branch/$p" ]; then
- die "Patch $p does not exist. Aborting."
- fi
-
- push_patch "$p" $abort_flag
-
- # bail if necessary
- if [ $? -eq 0 ]; then
- disp "Patch applied."
- elif [ -z "$abort_flag" ]; then
- die "Patch applied with rejects. Fix it up, and refresh."
- else
- die "To force apply this patch, use 'guilt push -f'"
- fi
-done
-
+get_series | sed -n -e "${sidx},${eidx}p" |
+ {
+ did_patch=0
+ while read p
+ do
+ disp "Applying patch..$p"
+ if [ ! -f "$GUILT_DIR/$branch/$p" ]; then
+ die "Patch $p does not exist. Aborting."
+ fi
+
+ push_patch "$p" $abort_flag
+
+ # bail if necessary
+ if [ $? -eq 0 ]; then
+ disp "Patch applied."
+ elif [ -z "$abort_flag" ]; then
+ die "Patch applied with rejects. Fix it up, and refresh."
+ else
+ die "To force apply this patch, use 'guilt push -f'"
+ fi
+ did_patch=1
+ done
+ [ $did_patch -ge 1 ] || die "no patches to apply"
+ }
diff --git a/regression/t-021.out b/regression/t-021.out
index cd8ae96..44771cb 100644
--- a/regression/t-021.out
+++ b/regression/t-021.out
@@ -822,6 +822,7 @@ index 0000000..8baef1b
@@ -0,0 +1 @@
+abc
% guilt-push --all
+no patches to apply
% guilt-pop -n -1
Invalid number of patches to pop.
% list_files
@@ -908,6 +909,7 @@ index 0000000..8baef1b
@@ -0,0 +1 @@
+abc
% guilt-push --all
+no patches to apply
% guilt-pop -n 0
No patches requested to be removed.
% list_files
@@ -994,6 +996,7 @@ index 0000000..8baef1b
@@ -0,0 +1 @@
+abc
% guilt-push --all
+no patches to apply
% guilt-pop -n 1
Now at remove.
% list_files
--
1.5.6.3
^ permalink raw reply related
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