Git development
 help / color / mirror / Atom feed
* [PATCH] simple euristic for further free packing improvements
From: Nicolas Pitre @ 2006-05-15 15:40 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0605132305580.18071@localhost.localdomain>

Given that the early eviction of objects with maximum delta depth 
may exhibit bad packing on its own, why not considering a bias against 
deep base objects in try_delta() to mitigate that bad behavior.

This patch adjust the MAX_size allowed for a delta based on the depth of 
the base object as well as enabling the early eviction of max depth 
objects from the object window.  When used separately, those two things 
produce slightly better and much worse results respectively.  But their 
combined effect is a surprising significant packing improvement.

With this really simple patch the GIT repo gets nearly 15% smaller, and 
the Linux kernel repo about 5% smaller, with no significantly measurable 
CPU usage difference.

Signed-off-by: Nicolas Pitre <nico@cam.org>

---

diff --git a/pack-objects.c b/pack-objects.c
index 523a1c7..b0388d7 100644
--- a/pack-objects.c
+++ b/pack-objects.c
@@ -1038,8 +1038,8 @@ static int try_delta(struct unpacked *tr
 
 	/* Now some size filtering euristics. */
 	size = trg_entry->size;
-	max_size = size / 2 - 20;
-	if (trg_entry->delta)
+	max_size = (size/2 - 20) / (src_entry->depth + 1);
+	if (trg_entry->delta && trg_entry->delta_size <= max_size)
 		max_size = trg_entry->delta_size-1;
 	src_size = src_entry->size;
 	sizediff = src_size < size ? size - src_size : 0;
@@ -1128,15 +1128,12 @@ static void find_deltas(struct object_en
 			if (try_delta(n, m, m->index, depth) < 0)
 				break;
 		}
-#if 0
 		/* if we made n a delta, and if n is already at max
 		 * depth, leaving it in the window is pointless.  we
 		 * should evict it first.
-		 * ... in theory only; somehow this makes things worse.
 		 */
 		if (entry->delta && depth <= entry->depth)
 			continue;
-#endif
 		idx++;
 		if (idx >= window)
 			idx = 0;

^ permalink raw reply related

* 1.3.2.gde1d fails to build on OpenBSD
From: Randal L. Schwartz @ 2006-05-15 14:24 UTC (permalink / raw)
  To: git


GIT_VERSION = 1.3.2.gde1d
gcc -o sha1_file.o -c -g -O2 -Wall -I/usr/local/include -DSHA1_HEADER='<openssl/sha.h>' -DNO_STRCASESTR sha1_file.c
sha1_file.c:16:20: stdint.h: No such file or directory
gmake: *** [sha1_file.o] Error 1

I think you want

        #include <sys/types.h>

on OpenBSD.

-- 
Randal L. Schwartz - Stonehenge Consulting Services, Inc. - +1 503 777 0095
<merlyn@stonehenge.com> <URL:http://www.stonehenge.com/merlyn/>
Perl/Unix/security consulting, Technical writing, Comedy, etc. etc.
See PerlTraining.Stonehenge.com for onsite and open-enrollment Perl training!

^ permalink raw reply

* pack-object mystery explained
From: Nicolas Pitre @ 2006-05-15 15:29 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

[-- Attachment #1: Type: TEXT/PLAIN, Size: 7928 bytes --]

As Junio certainly knows since he added it himself, there is some code 
currently surrounded with #if 0 ... #endif in pack-objects.c that was 
meant to improve delta window usage.

The idea was to _not_ keep any object in the object window which delta 
depth reached the maximum depth since it obviously cannot be used as a 
base for any further deltification anyway.  In doing so the object 
window would always be filled only with objects that are not too deep 
and therefore increase the possibility for better delta match.

In theory... only.

Because, in practice, doing so produces pack files that are between 20% 
to 60% *larger* than without this "optimization".

This is just so counter-intuitive that I wanted to find out why.  So I 
added a few lines to output the target object sha1, and the sha1 of each 
object in the object window as well as the return code from try_delta(), 
the object depth and the size of the best delta, something like the 
attached patch.  Then diffing the output between a run with the code as 
is and another run with the "optimization" enabled produce a really 
interesting result.

Looking at the diff:

--- /tmp/l1      2006-05-13 22:38:06.000000000 -0400
+++ /tmp/l2     2006-05-13 23:04:53.000000000 -0400
@@ -1255,7 +1255,6 @@
  0 src 57cefdea530171c71c72229a85aa0212dedb2c5a depth 5 delta_sz 115
 nbdelta = 34 deltasz = 25794
 trg f24cfd2b46a65bde53610eecb6998fa615b62363
- 0 src 0b90fb9ae3396d0f2b0a9ebf9acad8374f3db317 depth 10 delta_sz 0
  1 src 8866189332568a317183f6f895014519515f67e7 depth 9 delta_sz 20146
  1 src 4c1b0c3039a84c4c786a09ba70a84df46f995186 depth 8 delta_sz 3357
  0 src 226d71966d4a0e8a8611ea09e35719688a987ceb depth 7 delta_sz 3357
@@ -1265,10 +1264,10 @@
  0 src e44310f5a8115011eafce41362d1752c4ee0f72f depth 3 delta_sz 1365
  0 src b35d400ee1a2e8b5c97ca6ad82f6d91818519456 depth 2 delta_sz 1365
  0 src b60fa8d2417c32cf72331e9869d4d3a3208e953f depth 2 delta_sz 1365
+ 0 src 57cefdea530171c71c72229a85aa0212dedb2c5a depth 5 delta_sz 1365
 nbdelta = 35 deltasz = 27159
 trg 553e1e1749ab537a71c7a1cfb97f18f85dc9ef1f
  1 src f24cfd2b46a65bde53610eecb6998fa615b62363 depth 6 delta_sz 990
- 0 src 0b90fb9ae3396d0f2b0a9ebf9acad8374f3db317 depth 10 delta_sz 990
  0 src 8866189332568a317183f6f895014519515f67e7 depth 9 delta_sz 990
  0 src 4c1b0c3039a84c4c786a09ba70a84df46f995186 depth 8 delta_sz 990
  0 src 226d71966d4a0e8a8611ea09e35719688a987ceb depth 7 delta_sz 990
@@ -1277,11 +1276,11 @@
  0 src 42b0d59e8c15dff0d72b0d3a486d0248bd767dfe depth 4 delta_sz 248
  0 src e44310f5a8115011eafce41362d1752c4ee0f72f depth 3 delta_sz 248
  0 src b35d400ee1a2e8b5c97ca6ad82f6d91818519456 depth 2 delta_sz 248
+ 0 src b60fa8d2417c32cf72331e9869d4d3a3208e953f depth 2 delta_sz 248
 nbdelta = 36 deltasz = 27407
[...]

So the object that reached a delta depth of 10 is clearly not added to 
the window in the second log.

But here's the interesting part:

 trg f3c92c971e65e9df72fafdab52b4935866a0a794
- 0 src e85f1c17089a1da95b264909cfc50235c74471da depth 10 delta_sz 0
- 0 src 134d405ba2e6dd41c7501aa9e078a8879ba8025f depth 10 delta_sz 0
- 0 src 6a241aab054a8d6b5c021aba77bba795d7684196 depth 10 delta_sz 0
- 0 src 85cd595802ca565f380a9851efd5a3551c959db0 depth 10 delta_sz 0
- 0 src 89fda42efb678d2f9f9e802454fe561b4452a167 depth 10 delta_sz 0
- 0 src c10067c17ff9eef3ce0f01e5c14ba86bd2273d54 depth 10 delta_sz 0
- 0 src 8e953a67596cf0f5c12f2d8c66055759a7d2201c depth 10 delta_sz 0
  1 src 553e1e1749ab537a71c7a1cfb97f18f85dc9ef1f depth 6 delta_sz 6223
  0 src f24cfd2b46a65bde53610eecb6998fa615b62363 depth 6 delta_sz 6223
- 0 src 0b90fb9ae3396d0f2b0a9ebf9acad8374f3db317 depth 10 delta_sz 6223
-nbdelta = 44 deltasz = 40076
+ 1 src 8866189332568a317183f6f895014519515f67e7 depth 9 delta_sz 5135
+ 0 src 4c1b0c3039a84c4c786a09ba70a84df46f995186 depth 8 delta_sz 5135
+ 0 src 226d71966d4a0e8a8611ea09e35719688a987ceb depth 7 delta_sz 5135
+ 0 src 78129ec0cd5fb9767caefee8a8dc1a17dc095bdb depth 6 delta_sz 5135
+ 0 src 93a50b4444e3bba210f6879795979d0894ad5aaf depth 5 delta_sz 5135
+ 0 src 42b0d59e8c15dff0d72b0d3a486d0248bd767dfe depth 4 delta_sz 5135
+ 0 src e44310f5a8115011eafce41362d1752c4ee0f72f depth 3 delta_sz 5135
+ 0 src b35d400ee1a2e8b5c97ca6ad82f6d91818519456 depth 2 delta_sz 5135
+nbdelta = 44 deltasz = 38988
 trg fe925609b4024119c6171dd32350a6570bea8516
- 1 src f3c92c971e65e9df72fafdab52b4935866a0a794 depth 7 delta_sz 99
- 0 src e85f1c17089a1da95b264909cfc50235c74471da depth 10 delta_sz 99
- 0 src 134d405ba2e6dd41c7501aa9e078a8879ba8025f depth 10 delta_sz 99
- 0 src 6a241aab054a8d6b5c021aba77bba795d7684196 depth 10 delta_sz 99
- 0 src 85cd595802ca565f380a9851efd5a3551c959db0 depth 10 delta_sz 99
- 0 src 89fda42efb678d2f9f9e802454fe561b4452a167 depth 10 delta_sz 99
- 0 src c10067c17ff9eef3ce0f01e5c14ba86bd2273d54 depth 10 delta_sz 99
- 0 src 8e953a67596cf0f5c12f2d8c66055759a7d2201c depth 10 delta_sz 99
- 0 src 553e1e1749ab537a71c7a1cfb97f18f85dc9ef1f depth 6 delta_sz 99
- 0 src f24cfd2b46a65bde53610eecb6998fa615b62363 depth 6 delta_sz 99
-nbdelta = 45 deltasz = 40175
+ 1 src 553e1e1749ab537a71c7a1cfb97f18f85dc9ef1f depth 6 delta_sz 6006
+ 0 src f24cfd2b46a65bde53610eecb6998fa615b62363 depth 6 delta_sz 6006
+ 1 src 8866189332568a317183f6f895014519515f67e7 depth 9 delta_sz 5027
+ 0 src 4c1b0c3039a84c4c786a09ba70a84df46f995186 depth 8 delta_sz 5027
+ 0 src 226d71966d4a0e8a8611ea09e35719688a987ceb depth 7 delta_sz 5027
+ 0 src 78129ec0cd5fb9767caefee8a8dc1a17dc095bdb depth 6 delta_sz 5027
+ 0 src 93a50b4444e3bba210f6879795979d0894ad5aaf depth 5 delta_sz 5027
+ 0 src 42b0d59e8c15dff0d72b0d3a486d0248bd767dfe depth 4 delta_sz 5027
+ 0 src e44310f5a8115011eafce41362d1752c4ee0f72f depth 3 delta_sz 5027
+ 0 src b35d400ee1a2e8b5c97ca6ad82f6d91818519456 depth 2 delta_sz 5027
+nbdelta = 45 deltasz = 44015
[...]

Here we can see that f3c92c97 benefits from a window that has none of 
those objects with a depth of 10 so it can find a better delta base.  
But in doing so it becomes itself depth 10 and is therefore discarded 
from the object window right away.  And the unfortunate fact is that 
f3c92c97 would have been the best base for the next object (fe925609) by 
far with a delta size of 99 bytes instead of 5027 bytes.

And then the story repeats itself for a while since fe925609 also 
becomes depth 10 which means the object window doesn't get refreshed as 
long as best match are based on 88661893 with a delta size around 5000 
bytes instead of 100 bytes if f3c92c97 didn't reach depth 10 initially.

So it is like a priority inversion problem. Instead of evicting max 
depth objects early to always have a full window of potential usable 
base object to delta against andimprove the delta packing, this early 
eviction of objects with max depth may enter a perverse state where the 
object window is not recycled with new objects for a while, which is 
especially bad if the window content is the source of suboptimal delta 
matches.  And that perverse state will remain until the delta match 
becomes bad enough to break the link with 88661893.

I thought about recording the second best delta base for any object that 
reached maximum delta depth and reverting to that base whenever an 
object of maximum depth would allow for a delta of greater saving if it 
wasn't of max depth already and could be used as a delta base again.  
But this quickly gets complicated if we want to retroactively consider 
previous objects that didn't constitute a good enough saving to motivate 
the switch of given object to its second best base but could benefit 
from such a switch if some other objects are making that switch anyway, 
and then it would be necessary to make sure not to exceed the depth of 
objects already deltified from the object we're considering moving, 
etc.  In short it gets really twisted for an incertain gain.

But... there is still a way to improve things with a really really 
simple euristic...


Nicolas

[-- Attachment #2: Type: TEXT/PLAIN, Size: 1108 bytes --]

diff --git a/pack-objects.c b/pack-objects.c
index 523a1c7..8ff7f30 100644
--- a/pack-objects.c
+++ b/pack-objects.c
@@ -1116,8 +1116,10 @@ static void find_deltas(struct object_en
 		if (!n->index)
 			die("out of memory");
 
+fprintf(stderr, "trg %s\n", sha1_to_hex(entry->sha1));
 		j = window;
 		while (--j > 0) {
+			int rc;
 			unsigned int other_idx = idx + j;
 			struct unpacked *m;
 			if (other_idx >= window)
@@ -1125,10 +1127,15 @@ static void find_deltas(struct object_en
 			m = array + other_idx;
 			if (!m->entry)
 				break;
-			if (try_delta(n, m, m->index, depth) < 0)
+			rc = try_delta(n, m, m->index, depth);
+fprintf(stderr, "% d src %s depth %d delta_sz %d\n", rc, sha1_to_hex(m->entry->sha1), m->entry->depth, n->entry->delta_size);
+			if (rc < 0)
 				break;
 		}
+static int nbdelta=0, deltasz=0;
+if (n->entry->delta) nbdelta++, deltasz += n->entry->delta_size;
+fprintf(stderr, "nbdelta = %d deltasz = %d\n", nbdelta, deltasz);
 #if 1
 		/* if we made n a delta, and if n is already at max
 		 * depth, leaving it in the window is pointless.  we
 		 * should evict it first.

^ permalink raw reply related

* Re: how to display file history?
From: Linus Torvalds @ 2006-05-15 15:15 UTC (permalink / raw)
  To: Brown, Len; +Cc: git
In-Reply-To: <CFF307C98FEABE47A452B27C06B85BB670F4F8@hdsmsx411.amr.corp.intel.com>



On Mon, 15 May 2006, Brown, Len wrote:
>
> it is tiresome to access kernel.org/git tree display
> to see the list of commits that changed a particular file.
> (and for files on my local disk, this isn't available).
> 
> How do I print the list of commits that change a particular file
> on my local disk?

Just do

	git whatchanged -p <file>

which has worked pretty much since day one. In fact, it's much better than 
that. The "file" can be any abritrary combination of files and/or 
directories, and it will track them all at the same time. So

	git whatchanged -p arch/ia64 include/asm-ia64

will track the ia64-specific changes.

Newer git versions (ie 1.3.x+) support this in "git log" too (it worked on 
older gits, but it was unacceptably slow, so you might as well consider it 
"nonworking"). So

	git log [-p] <filespec>

is your friend.

Finally, as usual, "gitk" is just a fancier log viewer. So just do

	gitk arch/ia64 include/asm-ia64

and enjoy.

You really shouldn't go to gitweb. The history view of gitweb is much less 
capable than any local view.

		Linus

^ permalink raw reply

* [PATCH] gitk: Display commit messages with word wrap (try 2)
From: Sergey Vlasov @ 2006-05-15 15:13 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: git
In-Reply-To: <17511.48749.631725.358279@cargo.ozlabs.ibm.com>

Some people put very long strings into commit messages, which then
become invisible in gitk (word wrapping in the commit details window is
turned off, and there is no horizontal scroll bar).  Enabling word wrap
for just the commit message looks much better.

Wrapping is controlled by the "wrapcomment" option in ~/.gitk.  By
default this option is set to "none", which disables wrapping; setting
it to "word" enables word wrap for commit messages.

Signed-off-by: Sergey Vlasov <vsu@altlinux.ru>

---

 gitk |   26 ++++++++++++++------------
 1 files changed, 14 insertions(+), 12 deletions(-)

 The text about the "comment" variable IMHO just does not fit in the
 commit message above - if you prefer, I'll just make two separate
 patches, the first which leaves the "comment" name in place (ignoring the
 fact that the variable is no longer holding the comment text), and the
 second which just renames it to "headers".

f23c00577c9a4379c794313f8e54132a159f7f43
diff --git a/gitk b/gitk
index 4aa57c0..8d046af 100755
--- a/gitk
+++ b/gitk
@@ -380,7 +380,7 @@ proc makewindow {} {
     global findtype findtypemenu findloc findstring fstring geometry
     global entries sha1entry sha1string sha1but
     global maincursor textcursor curtextcursor
-    global rowctxmenu mergemax
+    global rowctxmenu mergemax wrapcomment
 
     menu .bar
     .bar add cascade -label "File" -menu .bar.file
@@ -527,6 +527,7 @@ proc makewindow {} {
     pack $ctext -side left -fill both -expand 1
     .ctop.cdet add .ctop.cdet.left
 
+    $ctext tag conf comment -wrap $wrapcomment
     $ctext tag conf filesep -font [concat $textfont bold] -back "#aaaaaa"
     $ctext tag conf hunksep -fore blue
     $ctext tag conf d0 -fore red
@@ -696,7 +697,7 @@ proc savestuff {w} {
     global stuffsaved findmergefiles maxgraphpct
     global maxwidth
     global viewname viewfiles viewargs viewperm nextviewnum
-    global cmitmode
+    global cmitmode wrapcomment
 
     if {$stuffsaved} return
     if {![winfo viewable .]} return
@@ -709,6 +710,7 @@ proc savestuff {w} {
 	puts $f [list set maxgraphpct $maxgraphpct]
 	puts $f [list set maxwidth $maxwidth]
 	puts $f [list set cmitmode $cmitmode]
+	puts $f [list set wrapcomment $wrapcomment]
 	puts $f "set geometry(width) [winfo width .ctop]"
 	puts $f "set geometry(height) [winfo height .ctop]"
 	puts $f "set geometry(canv1) [expr {[winfo width $canv]-2}]"
@@ -3222,11 +3224,11 @@ proc commit_descriptor {p} {
 
 # append some text to the ctext widget, and make any SHA1 ID
 # that we know about be a clickable link.
-proc appendwithlinks {text} {
+proc appendwithlinks {text tags} {
     global ctext commitrow linknum curview
 
     set start [$ctext index "end - 1c"]
-    $ctext insert end $text
+    $ctext insert end $text $tags
     $ctext insert end "\n"
     set links [regexp -indices -all -inline {[0-9a-f]{40}} $text]
     foreach l $links {
@@ -3354,7 +3356,7 @@ proc selectline {l isnew} {
 	$ctext insert end "\n"
     }
  
-    set comment {}
+    set headers {}
     set olds [lindex $parentlist $l]
     if {[llength $olds] > 1} {
 	set np 0
@@ -3365,23 +3367,22 @@ proc selectline {l isnew} {
 		set tag m$np
 	    }
 	    $ctext insert end "Parent: " $tag
-	    appendwithlinks [commit_descriptor $p]
+	    appendwithlinks [commit_descriptor $p] {}
 	    incr np
 	}
     } else {
 	foreach p $olds {
-	    append comment "Parent: [commit_descriptor $p]\n"
+	    append headers "Parent: [commit_descriptor $p]\n"
 	}
     }
 
     foreach c [lindex $childlist $l] {
-	append comment "Child:  [commit_descriptor $c]\n"
+	append headers "Child:  [commit_descriptor $c]\n"
     }
-    append comment "\n"
-    append comment [lindex $info 5]
 
     # make anything that looks like a SHA1 ID be a clickable link
-    appendwithlinks $comment
+    appendwithlinks $headers {}
+    appendwithlinks [lindex $info 5] {comment}
 
     $ctext tag delete Comments
     $ctext tag remove found 1.0 end
@@ -4504,7 +4505,7 @@ proc showtag {tag isnew} {
     } else {
 	set text "Tag: $tag\nId:  $tagids($tag)"
     }
-    appendwithlinks $text
+    appendwithlinks $text {}
     $ctext conf -state disabled
     init_flist {}
 }
@@ -4890,6 +4891,7 @@ set downarrowlen 7
 set mingaplen 30
 set flistmode "flat"
 set cmitmode "patch"
+set wrapcomment "none"
 
 set colors {green red blue magenta darkgrey brown orange}
 
-- 
1.3.2.g10c1

^ permalink raw reply related

* Re: Simplify "git reset --hard"
From: Linus Torvalds @ 2006-05-15 15:09 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0605150752490.3866@g5.osdl.org>



On Mon, 15 May 2006, Linus Torvalds wrote:
> 
> Ack. On the other hand, I wonder if it might not make sense to have this 
> part potentially depend on the "--reset" flag.
> 
> That way you wouldn't even have to apologize for it.

Ie, something like this..

This should allow a two-way or three-way merge to akso ignore any dirty 
state when you use "--reset", because they use "verify_uptodate()", which 
now would set the CD_UPDATE flag on the ce rather than complain about it 
being different (if it survives the merge, of course - often it won't, but 
then we won't care).

This is all totally untested, of course.

		Linus

----
diff --git a/read-tree.c b/read-tree.c
index 7c83031..f2d674c 100644
--- a/read-tree.c
+++ b/read-tree.c
@@ -13,6 +13,7 @@ #include "cache-tree.h"
 #include <sys/time.h>
 #include <signal.h>
 
+static int reset = 0;
 static int merge = 0;
 static int update = 0;
 static int index_only = 0;
@@ -419,6 +420,10 @@ static void verify_uptodate(struct cache
 			return;
 		errno = 0;
 	}
+	if (reset) {
+		ce->ce_flags |= htons(CE_UPDATE);
+		return;
+	}
 	if (errno == ENOENT)
 		return;
 	die("Entry '%s' not uptodate. Cannot merge.", ce->name);
@@ -723,6 +728,11 @@ static int oneway_merge(struct cache_ent
 		return deleted_entry(old, NULL);
 	}
 	if (old && same(old, a)) {
+		if (reset) {
+			struct stat st;
+			if (lstat(old->name, &st) || ce_match_stat(old, &st, 1))
+				old->ce_flags |= htons(CE_UPDATE);
+		}
 		return keep_entry(old);
 	}
 	return merged_entry(a, NULL);
@@ -790,7 +800,7 @@ static struct cache_file cache_file;
 
 int main(int argc, char **argv)
 {
-	int i, newfd, reset, stage = 0;
+	int i, newfd, stage = 0;
 	unsigned char sha1[20];
 	merge_fn_t fn = NULL;
 

^ permalink raw reply related

* Re: [PATCH] gitk: Display commit messages with word wrap
From: Sergey Vlasov @ 2006-05-15 15:07 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v4pzs18wl.fsf@assigned-by-dhcp.cox.net>

[-- Attachment #1: Type: text/plain, Size: 660 bytes --]

On Sun, May 14, 2006 at 04:30:50PM -0700, Junio C Hamano wrote:
> Sergey Vlasov <vsu@altlinux.ru> writes:
> 
> > Some people put very long strings into commit messages, which then
> > become invisible in gitk (word wrapping in the commit details window is
> > turned off, and there is no horizontal scroll bar).  Enabling word wrap
> > for just the commit message looks much better.
> 
> I do not mind it myself but IIRC gitk's "no wrapping" was made
> because people found wrapping annoying; maybe a runtime
> configuration option?

OK, added a hidden option in ~/.gitk which defaults to "no wrapping", so
most users will not notice anything...

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* Re: [PATCH] gitk: Display commit messages with word wrap
From: Sergey Vlasov @ 2006-05-15 15:06 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: git
In-Reply-To: <17511.48749.631725.358279@cargo.ozlabs.ibm.com>

[-- Attachment #1: Type: text/plain, Size: 1489 bytes --]

On Mon, May 15, 2006 at 09:34:05AM +1000, Paul Mackerras wrote:
> Sergey Vlasov writes:
> 
> > Some people put very long strings into commit messages, which then
> > become invisible in gitk (word wrapping in the commit details window is
> > turned off, and there is no horizontal scroll bar).  Enabling word wrap
> > for just the commit message looks much better.
> 
> Well... you can scroll in any direction with mouse button 2, but ok...

I completely forgot about this obscure feature of Tk (and is it only me
who thinks that it scrolls in the wrong direction?).

> > +    $ctext insert end "\n" {}
> 
> Why are you adding the superfluous {} ?

Because I was paranoid about not letting the tag leak into subsequent
text...  but apparently this does not happen even without that {}, so I'll
remove it.

> > -    set comment {}
> > +    set headers {}
> 
> Why are you changing the name here?  Your commit description doesn't
> address either of these points.

Previously the "comment" variable contained both the commit headers
("Parent:" and "Child:" lines) and the commit message, and all this text
was inserted into $ctext by a single call to "appendwithlinks".  Now I
need to insert these parts separately (wrapped "Parent:" and "Child:"
lines look bad, I want to wrap only the commit message), therefore only
headers are collected in that variable - so I renamed it to reflect this
new usage.

I'll send the updated patch in a separate message.

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* Re: Simplify "git reset --hard"
From: Linus Torvalds @ 2006-05-15 14:55 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vr72vvigy.fsf@assigned-by-dhcp.cox.net>



On Mon, 15 May 2006, Junio C Hamano wrote:
>
> read-tree -u one-way merge fix to check out locally modified paths.
> 
> The "-u" flag means "update the working tree files", but to
> other types of merges, it also implies "I want to keep my local
> changes" -- because they prevent local changes from getting lost
> by using verify_uptodate.  The one-way merge is different from
> other merges in that its purpose is opposite of doing something
> else while keeping unrelated local changes.  The point of
> one-way merge is to nuke local changes.  So while it feels
> somewhat wrong that this actively loses local changes, it is the
> right thing to do.

Ack. On the other hand, I wonder if it might not make sense to have this 
part potentially depend on the "--reset" flag.

That way you wouldn't even have to apologize for it.

		Linus

^ permalink raw reply

* Re: [PATCH] Fix compilation on newer NetBSD systems
From: Dennis Stosberg @ 2006-05-15 12:41 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vu07rx3a9.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano wrote:

> Dennis Stosberg <dennis@stosberg.net> writes:
> 
> > +	ifeq ($(shell test `uname -r | sed -e 's/^\([0-9]\).*/\1/'` -lt 2 && echo y),y)
> > +		NEEDS_LIBICONV = YesPlease
> > +	endif
> 
> This looks rather ugly.  I do not know if NetBSD has 0.xx
> versions, but perhaps something like this?
> 
> 	ifeq ($(shell expr "$(uname_R)" : '[01]\.'),2)

Admittedly, that looks a lot nicer.  And it works correctly with
NetBSD's "expr", too.

Regards,
Dennis

^ permalink raw reply

* Re: [PATCH 2/3] Handle branch names with slashes
From: Karl Hasselström @ 2006-05-15 10:58 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: Wartan Hachaturow, git
In-Reply-To: <b0943d9e0605150322w684785d5n9b17dccda6b29ac1@mail.gmail.com>

On 2006-05-15 11:22:08 +0100, Catalin Marinas wrote:

> There is one problem with killing "/" entirely (or maybe we could
> use other character than "#"). I tend to write quite often "stg diff
> -r /bottom" to see how the whole patch looks like before refreshing.
> With "#", the shell ignores "#bottom" as being a comment.
>
> Otherwise, I'm OK with changing "/" with something else or just
> keeping both (though I prefer to have a singe way of specifying it).
> It looks like ^ and ~ are already used by GIT. It leaves us with %
> and !. Do you have any preference? The exclamation mark looks OK to
> me.

Ah, right. Well, I would prefer %, since ! is used for some kind of
shell history searching, but % is not touched by the shell, I think.

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle

^ permalink raw reply

* Re: [PATCH] send-email: allow sendmail binary to be used instead of SMTP
From: Martin Langhoff @ 2006-05-15 10:37 UTC (permalink / raw)
  To: Eric Wong; +Cc: Junio C Hamano, git
In-Reply-To: <20060515101142.GD6855@localdomain>

On 5/15/06, Eric Wong <normalperson@yhbt.net> wrote:
> Junio C Hamano <junkio@cox.net> wrote:
> > I am not opposed to have an option to run a local submission
> > agent binary (I said I like that if(){}else{} there, didn't I?).
> > The ability to do so is a good thing.  I am not however sure
> > about changing the default when no option is specified on the
> > command line.
>
> By "I believe this is what Martin wanted", I meant changing the default to
> sendmail: <46a038f90604271804j195d62f3x93ae816e809f4ffd@mail.gmail.com>
>
>         > Oh, it should just work with sendmail if it's there and we don't

Thanks Eric! git-send-email used to default to using local binaries.
It was only with the switch to Net::SMTP that the default changed to
localhost:25.
IMHO the developer's machine is more likely to have a working
/usr/sbin/sendmail than an SMTP server (specially looking at current
linux desktop configurations).

OTOH, as long as I can override it to use sendmail, it's all good.


martin

^ permalink raw reply

* Re: [PATCH 2/3] Handle branch names with slashes
From: Catalin Marinas @ 2006-05-15 10:22 UTC (permalink / raw)
  To: Karl Hasselström; +Cc: Wartan Hachaturow, git
In-Reply-To: <20060515095440.GA11412@backpacker.hemma.treskal.com>

Karl,

On 15/05/06, Karl Hasselström <kha@treskal.com> wrote:
> Teach stgit to handle branch names with slashes in them; that is,
> branches living in a subdirectory of .git/refs/heads.

Thanks for the patches. I'll add them, probably tomorrow evening as
I'm busy today.

> I had to change the patch@branch/top command-line syntax to
> patch@branch#top, in order to get sane parsing. The /top variant is
> still available for repositories that have no slashy branches; it is
> disabled as soon as there exists at least one subdirectory of
> refs/heads. Preferably, this compatibility hack can be killed some
> time in the future.

There is one problem with killing "/" entirely (or maybe we could use
other character than "#"). I tend to write quite often "stg diff -r
/bottom" to see how the whole patch looks like before refreshing. With
"#", the shell ignores "#bottom" as being a comment.

Otherwise, I'm OK with changing "/" with something else or just
keeping both (though I prefer to have a singe way of specifying it).
It looks like ^ and ~ are already used by GIT. It leaves us with % and
!. Do you have any preference? The exclamation mark looks OK to me.

-- 
Catalin

^ permalink raw reply

* Re: [PATCH] send-email: allow sendmail binary to be used instead of SMTP
From: Eric Wong @ 2006-05-15 10:11 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Martin Langhoff
In-Reply-To: <7vmzdjtya4.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano <junkio@cox.net> wrote:
> Eric Wong <normalperson@yhbt.net> writes:
> 
> > I believe this is what Martin wanted.  I think it's a good idea since
> > sendmail binaries tend to be more flexible, but I'm ok with it either
> > way.
> 
> I am not opposed to have an option to run a local submission
> agent binary (I said I like that if(){}else{} there, didn't I?).
> The ability to do so is a good thing.  I am not however sure
> about changing the default when no option is specified on the
> command line.

By "I believe this is what Martin wanted", I meant changing the default to
sendmail: <46a038f90604271804j195d62f3x93ae816e809f4ffd@mail.gmail.com>

	> Oh, it should just work with sendmail if it's there and we don't
	> provide --smtp-server ;-)

> >> > +	if ($smtp_server =~ m#^/#) {
> >> 
> >> I like this if(){}else{} here, but have a feeling that the
> >> logging part should be placed outside it to be shared.
> >
> > Cleaned that up a bit, patch coming.  Also removed the Port: printout
> > completely, as it's rather redundant (see below).
> >
> >> While we are at it, we might want to enhance $smtp_server parsing
> >> to take host:port notation so that people can use message
> >> submission port 587/tcp (RFC 4409) instead.
> >
> > This already works, IO::Socket::INET (behind Net::SMTP) takes care of
> > it :)
> 
> Thanks.  Maybe the next option would be delivery to a file (or
> even SMTP batch)? ;-)

Sure :)  Authentication and SSL may be worth looking into, though.  I'm
pretty content these days with my autossh tunnel to my mail server,
though.

-- 
Eric Wong

^ permalink raw reply

* [PATCH 3/3] Tests for branch names with slashes
From: Karl Hasselström @ 2006-05-15  9:55 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: Wartan Hachaturow, git
In-Reply-To: <20060510060040.GA3034@diana.vm.bytemark.co.uk>

Test a number of operations on a repository that has branch names
containing slashes (that is, branches living in a subdirectory of
.git/refs/heads).

Signed-off-by: Karl Hasselström <kha@treskal.com>


---

 t/t0001-subdir-branches.sh |   59 ++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 59 insertions(+), 0 deletions(-)
 create mode 100644 t/t0001-subdir-branches.sh

58badd265645874165379bc841453a44b64e1906
diff --git a/t/t0001-subdir-branches.sh b/t/t0001-subdir-branches.sh
new file mode 100644
index 0000000..e3608de
--- /dev/null
+++ b/t/t0001-subdir-branches.sh
@@ -0,0 +1,59 @@
+#!/bin/sh
+#
+# Copyright (c) 2006 Karl Hasselström
+#
+
+test_description='Branch names containing slashes
+
+Test a number of operations on a repository that has branch names
+containing slashes (that is, branches living in a subdirectory of
+.git/refs/heads).'
+
+. ./test-lib.sh
+
+test_expect_success 'Create a patch' \
+  'stg init &&
+   echo "foo" > foo.txt &&
+   stg add foo.txt &&
+   stg new foo -m "Add foo.txt" &&
+   stg refresh'
+
+test_expect_success 'Old and new id with non-slashy branch' \
+  'stg id foo &&
+   stg id foo# &&
+   stg id foo/ &&
+   stg id foo#top &&
+   stg id foo/top &&
+   stg id foo@master &&
+   stg id foo@master#top &&
+   stg id foo@master/top'
+
+test_expect_success 'Clone branch to slashier name' \
+  'stg branch --clone x/y/z'
+
+test_expect_success 'Try new form of id with slashy branch' \
+  'stg id foo &&
+   stg id foo# &&
+   stg id foo#top &&
+   stg id foo@x/y/z &&
+   stg id foo@x/y/z#top'
+
+test_expect_failure 'Try old id with slashy branch' \
+  'stg id foo/ ||
+   stg id foo/top ||
+   stg id foo@x/y/z/top'
+
+test_expect_success 'Create patch in slashy branch' \
+  'echo "bar" >> foo.txt &&
+   stg new bar -m "Add another line" &&
+   stg refresh'
+
+test_expect_success 'Rename branches' \
+  'stg branch --rename master goo/gaa &&
+   test ! -e .git/refs/heads/master &&
+   stg branch --rename goo/gaa x1/x2/x3/x4 &&
+   test ! -e .git/refs/heads/goo &&
+   stg branch --rename x1/x2/x3/x4 servant &&
+   test ! -e .git/refs/heads/x1'
+
+test_done
-- 
1.3.2.g639c


-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle

^ permalink raw reply related

* [PATCH 2/3] Handle branch names with slashes
From: Karl Hasselström @ 2006-05-15  9:54 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: Wartan Hachaturow, git
In-Reply-To: <20060510060040.GA3034@diana.vm.bytemark.co.uk>

Teach stgit to handle branch names with slashes in them; that is,
branches living in a subdirectory of .git/refs/heads.

I had to change the patch@branch/top command-line syntax to
patch@branch#top, in order to get sane parsing. The /top variant is
still available for repositories that have no slashy branches; it is
disabled as soon as there exists at least one subdirectory of
refs/heads. Preferably, this compatibility hack can be killed some
time in the future.

Signed-off-by: Karl Hasselström <kha@treskal.com>


---

 stgit/commands/branch.py |    5 ++
 stgit/commands/common.py |  103 ++++++++++++++++++++++++++--------------------
 stgit/commands/diff.py   |   12 +++--
 stgit/commands/files.py  |    4 +-
 stgit/commands/id.py     |    2 -
 stgit/commands/mail.py   |    8 ++--
 stgit/git.py             |   42 +++++++++----------
 stgit/stack.py           |   21 ++++++---
 stgit/utils.py           |   88 +++++++++++++++++++++++++++++++++++++--
 9 files changed, 193 insertions(+), 92 deletions(-)

466723a40f8d40b84a23fe720447e1938e22c0a5
diff --git a/stgit/commands/branch.py b/stgit/commands/branch.py
index 2218bbb..d348409 100644
--- a/stgit/commands/branch.py
+++ b/stgit/commands/branch.py
@@ -172,7 +172,10 @@ def func(parser, options, args):
         if len(args) != 0:
             parser.error('incorrect number of arguments')
 
-        branches = os.listdir(os.path.join(basedir.get(), 'refs', 'heads'))
+        branches = []
+        basepath = os.path.join(basedir.get(), 'refs', 'heads')
+        for path, files, dirs in walk_tree(basepath):
+            branches += [os.path.join(path, f) for f in files]
         branches.sort()
 
         if branches:
diff --git a/stgit/commands/common.py b/stgit/commands/common.py
index c6ca514..9439976 100644
--- a/stgit/commands/common.py
+++ b/stgit/commands/common.py
@@ -18,7 +18,7 @@ along with this program; if not, write t
 Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
 """
 
-import sys, os, re
+import sys, os, os.path, re
 from optparse import OptionParser, make_option
 
 from stgit.utils import *
@@ -34,54 +34,69 @@ class CmdException(Exception):
 
 
 # Utility functions
+class RevParseException(Exception):
+    """Revision spec parse error."""
+    pass
+
+def parse_rev(rev):
+    """Parse a revision specification into its
+    patchname@branchname#patch_id parts. If no branch name has a slash
+    in it, also accept / instead of #."""
+    files, dirs = list_files_and_dirs(os.path.join(basedir.get(),
+                                                   'refs', 'heads'))
+    if len(dirs) != 0:
+        # We have branch names with / in them.
+        branch_chars = '[^@#]'
+        patch_id_mark = '#'
+    else:
+        # No / in branch names.
+        branch_chars = '[^@#/]'
+        patch_id_mark = '[/#]'
+    patch_re = r'(?P<patch>[^@/#]+)'
+    branch_re = r'@(?P<branch>%s+)' % branch_chars
+    patch_id_re = r'%s(?P<patch_id>[a-z.]*)' % patch_id_mark
+
+    # Try #patch_id.
+    m = re.match(r'^%s$' % patch_id_re, rev)
+    if m:
+        return None, None, m.group('patch_id')
+
+    # Try patch[@branch][#patch_id].
+    m = re.match(r'^%s(%s)?(%s)?$' % (patch_re, branch_re, patch_id_re), rev)
+    if m:
+        return m.group('patch'), m.group('branch'), m.group('patch_id')
+
+    # No, we can't parse that.
+    raise RevParseException
+
 def git_id(rev):
     """Return the GIT id
     """
     if not rev:
         return None
-    
-    rev_list = rev.split('/')
-    if len(rev_list) == 2:
-        patch_id = rev_list[1]
-        if not patch_id:
-            patch_id = 'top'
-    elif len(rev_list) == 1:
-        patch_id = 'top'
-    else:
-        patch_id = None
-
-    patch_branch = rev_list[0].split('@')
-    if len(patch_branch) == 1:
-        series = crt_series
-    elif len(patch_branch) == 2:
-        series = stack.Series(patch_branch[1])
-    else:
-        raise CmdException, 'Unknown id: %s' % rev
-
-    patch_name = patch_branch[0]
-    if not patch_name:
-        patch_name = series.get_current()
-        if not patch_name:
-            raise CmdException, 'No patches applied'
-
-    # patch
-    if patch_name in series.get_applied() \
-           or patch_name in series.get_unapplied():
-        if patch_id == 'top':
-            return series.get_patch(patch_name).get_top()
-        elif patch_id == 'bottom':
-            return series.get_patch(patch_name).get_bottom()
-        # Note we can return None here.
-        elif patch_id == 'top.old':
-            return series.get_patch(patch_name).get_old_top()
-        elif patch_id == 'bottom.old':
-            return series.get_patch(patch_name).get_old_bottom()
-
-    # base
-    if patch_name == 'base' and len(rev_list) == 1:
-        return read_string(series.get_base_file())
-
-    # anything else failed
+    try:
+        patch, branch, patch_id = parse_rev(rev)
+        if branch == None:
+            series = crt_series
+        else:
+            series = stack.Series(branch)
+        if patch == None:
+            patch = series.get_current()
+            if not patch:
+                raise CmdException, 'No patches applied'
+        if patch in series.get_applied() or patch in series.get_unapplied():
+            if patch_id in ['top', '', None]:
+                return series.get_patch(patch).get_top()
+            elif patch_id == 'bottom':
+                return series.get_patch(patch).get_bottom()
+            elif patch_id == 'top.old':
+                return series.get_patch(patch).get_old_top()
+            elif patch_id == 'bottom.old':
+                return series.get_patch(patch).get_old_bottom()
+        if patch == 'base' and patch_id == None:
+            return read_string(series.get_base_file())
+    except RevParseException:
+        pass
     return git.rev_parse(rev + '^{commit}')
 
 def check_local_changes():
diff --git a/stgit/commands/diff.py b/stgit/commands/diff.py
index 7dc6c5d..c3ab7d8 100644
--- a/stgit/commands/diff.py
+++ b/stgit/commands/diff.py
@@ -33,12 +33,12 @@ or a tree-ish object and another tree-is
 be given to restrict the diff output. The tree-ish object can be a
 standard git commit, tag or tree. In addition to these, the command
 also supports 'base', representing the bottom of the current stack,
-and '[patch]/[bottom | top]' for the patch boundaries (defaulting to
+and '[patch]#[bottom | top]' for the patch boundaries (defaulting to
 the current one):
 
-rev = '([patch]/[bottom | top]) | <tree-ish> | base'
+rev = '([patch]#[bottom | top]) | <tree-ish> | base'
 
-If neither bottom or top are given but a '/' is present, the command
+If neither bottom or top are given but a '#' is present, the command
 shows the specified patch (defaulting to the current one)."""
 
 options = [make_option('-r', metavar = 'rev1[:[rev2]]', dest = 'revs',
@@ -55,10 +55,10 @@ def func(parser, options, args):
         rev_list = options.revs.split(':')
         rev_list_len = len(rev_list)
         if rev_list_len == 1:
-            if rev_list[0][-1] == '/':
+            if rev_list[0][-1] in ['/', '#']:
                 # the whole patch
-                rev1 = rev_list[0] + 'bottom'
-                rev2 = rev_list[0] + 'top'
+                rev1 = rev_list[0][:-1] + '#bottom'
+                rev2 = rev_list[0][:-1] + '#top'
             else:
                 rev1 = rev_list[0]
                 rev2 = None
diff --git a/stgit/commands/files.py b/stgit/commands/files.py
index 0694d83..1d5126e 100644
--- a/stgit/commands/files.py
+++ b/stgit/commands/files.py
@@ -53,8 +53,8 @@ def func(parser, options, args):
     else:
         parser.error('incorrect number of arguments')
 
-    rev1 = git_id('%s/bottom' % patch)
-    rev2 = git_id('%s/top' % patch)
+    rev1 = git_id('%s#bottom' % patch)
+    rev2 = git_id('%s#top' % patch)
 
     if options.stat:
         print git.diffstat(rev1 = rev1, rev2 = rev2)
diff --git a/stgit/commands/id.py b/stgit/commands/id.py
index 1cf6ea6..5202aa4 100644
--- a/stgit/commands/id.py
+++ b/stgit/commands/id.py
@@ -28,7 +28,7 @@ usage = """%prog [options] [id]
 
 Print the hash value of a GIT id (defaulting to HEAD). In addition to
 the standard GIT id's like heads and tags, this command also accepts
-'base[@<branch>]' and '[<patch>[@<branch>]][/(bottom | top)]'. If no
+'base[@<branch>]' and '[<patch>[@<branch>]][#(bottom | top)]'. If no
 'top' or 'bottom' are passed and <patch> is a valid patch name, 'top'
 will be used by default."""
 
diff --git a/stgit/commands/mail.py b/stgit/commands/mail.py
index 5e01ea1..937efa3 100644
--- a/stgit/commands/mail.py
+++ b/stgit/commands/mail.py
@@ -324,10 +324,10 @@ def __build_message(tmpl, patch, patch_n
                  'shortdescr':   short_descr,
                  'longdescr':    long_descr,
                  'endofheaders': headers_end,
-                 'diff':         git.diff(rev1 = git_id('%s/bottom' % patch),
-                                          rev2 = git_id('%s/top' % patch)),
-                 'diffstat':     git.diffstat(rev1 = git_id('%s/bottom'%patch),
-                                              rev2 = git_id('%s/top' % patch)),
+                 'diff':         git.diff(rev1 = git_id('%s#bottom' % patch),
+                                          rev2 = git_id('%s#top' % patch)),
+                 'diffstat':     git.diffstat(rev1 = git_id('%s#bottom'%patch),
+                                              rev2 = git_id('%s#top' % patch)),
                  'date':         email.Utils.formatdate(localtime = True),
                  'version':      version_str,
                  'patchnr':      patch_nr_str,
diff --git a/stgit/git.py b/stgit/git.py
index 2884f36..716609c 100644
--- a/stgit/git.py
+++ b/stgit/git.py
@@ -225,7 +225,8 @@ def get_head():
 def get_head_file():
     """Returns the name of the file pointed to by the HEAD link
     """
-    return os.path.basename(_output_one_line('git-symbolic-ref HEAD'))
+    return strip_prefix('refs/heads/',
+                        _output_one_line('git-symbolic-ref HEAD'))
 
 def set_head_file(ref):
     """Resets HEAD to point to a new ref
@@ -233,7 +234,8 @@ def set_head_file(ref):
     # head cache flushing is needed since we might have a different value
     # in the new head
     __clear_head_cache()
-    if __run('git-symbolic-ref HEAD', [ref]) != 0:
+    if __run('git-symbolic-ref HEAD',
+             [os.path.join('refs', 'heads', ref)]) != 0:
         raise GitException, 'Could not set head to "%s"' % ref
 
 def __set_head(val):
@@ -272,6 +274,7 @@ def rev_parse(git_id):
 def branch_exists(branch):
     """Existence check for the named branch
     """
+    branch = os.path.join('refs', 'heads', branch)
     for line in _output_lines('git-rev-parse --symbolic --all 2>&1'):
         if line.strip() == branch:
             return True
@@ -282,12 +285,11 @@ def branch_exists(branch):
 def create_branch(new_branch, tree_id = None):
     """Create a new branch in the git repository
     """
-    new_head = os.path.join('refs', 'heads', new_branch)
-    if branch_exists(new_head):
+    if branch_exists(new_branch):
         raise GitException, 'Branch "%s" already exists' % new_branch
 
     current_head = get_head()
-    set_head_file(new_head)
+    set_head_file(new_branch)
     __set_head(current_head)
 
     # a checkout isn't needed if new branch points to the current head
@@ -297,22 +299,22 @@ def create_branch(new_branch, tree_id = 
     if os.path.isfile(os.path.join(basedir.get(), 'MERGE_HEAD')):
         os.remove(os.path.join(basedir.get(), 'MERGE_HEAD'))
 
-def switch_branch(name):
+def switch_branch(new_branch):
     """Switch to a git branch
     """
     global __head
 
-    new_head = os.path.join('refs', 'heads', name)
-    if not branch_exists(new_head):
-        raise GitException, 'Branch "%s" does not exist' % name
+    if not branch_exists(new_branch):
+        raise GitException, 'Branch "%s" does not exist' % new_branch
 
-    tree_id = rev_parse(new_head + '^{commit}')
+    tree_id = rev_parse(os.path.join('refs', 'heads', new_branch)
+                        + '^{commit}')
     if tree_id != get_head():
         refresh_index()
         if __run('git-read-tree -u -m', [get_head(), tree_id]) != 0:
             raise GitException, 'git-read-tree failed (local changes maybe?)'
         __head = tree_id
-    set_head_file(new_head)
+    set_head_file(new_branch)
 
     if os.path.isfile(os.path.join(basedir.get(), 'MERGE_HEAD')):
         os.remove(os.path.join(basedir.get(), 'MERGE_HEAD'))
@@ -320,25 +322,23 @@ def switch_branch(name):
 def delete_branch(name):
     """Delete a git branch
     """
-    branch_head = os.path.join('refs', 'heads', name)
-    if not branch_exists(branch_head):
+    if not branch_exists(name):
         raise GitException, 'Branch "%s" does not exist' % name
-    os.remove(os.path.join(basedir.get(), branch_head))
+    remove_file_and_dirs(os.path.join(basedir.get(), 'refs', 'heads'),
+                         name)
 
 def rename_branch(from_name, to_name):
     """Rename a git branch
     """
-    from_head = os.path.join('refs', 'heads', from_name)
-    if not branch_exists(from_head):
+    if not branch_exists(from_name):
         raise GitException, 'Branch "%s" does not exist' % from_name
-    to_head = os.path.join('refs', 'heads', to_name)
-    if branch_exists(to_head):
+    if branch_exists(to_name):
         raise GitException, 'Branch "%s" already exists' % to_name
 
     if get_head_file() == from_name:
-        set_head_file(to_head)
-    os.rename(os.path.join(basedir.get(), from_head), \
-              os.path.join(basedir.get(), to_head))
+        set_head_file(to_name)
+    rename(os.path.join(basedir.get(), 'refs', 'heads'),
+           from_name, to_name)
 
 def add(names):
     """Add the files or recursively add the directory contents
diff --git a/stgit/stack.py b/stgit/stack.py
index f83161b..49b50e7 100644
--- a/stgit/stack.py
+++ b/stgit/stack.py
@@ -443,8 +443,7 @@ class Series:
 
         os.makedirs(self.__patch_dir)
 
-        if not os.path.isdir(bases_dir):
-            os.makedirs(bases_dir)
+        create_dirs(bases_dir)
 
         create_empty_file(self.__applied_file)
         create_empty_file(self.__unapplied_file)
@@ -502,11 +501,14 @@ class Series:
         git.rename_branch(self.__name, to_name)
 
         if os.path.isdir(self.__series_dir):
-            os.rename(self.__series_dir, to_stack.__series_dir)
+            rename(os.path.join(self.__base_dir, 'patches'),
+                   self.__name, to_stack.__name)
         if os.path.exists(self.__base_file):
-            os.rename(self.__base_file, to_stack.__base_file)
+            rename(os.path.join(self.__base_dir, 'refs', 'bases'),
+                   self.__name, to_stack.__name)
         if os.path.exists(self.__refs_dir):
-            os.rename(self.__refs_dir, to_stack.__refs_dir)
+            rename(os.path.join(self.__base_dir, 'refs', 'patches'),
+                   self.__name, to_stack.__name)
 
         self.__init__(to_name)
 
@@ -560,16 +562,19 @@ class Series:
             else:
                 print 'Patch directory %s is not empty.' % self.__name
             if not os.listdir(self.__series_dir):
-                os.rmdir(self.__series_dir)
+                remove_dirs(os.path.join(self.__base_dir, 'patches'),
+                            self.__name)
             else:
                 print 'Series directory %s is not empty.' % self.__name
             if not os.listdir(self.__refs_dir):
-                os.rmdir(self.__refs_dir)
+                remove_dirs(os.path.join(self.__base_dir, 'refs', 'patches'),
+                            self.__name)
             else:
                 print 'Refs directory %s is not empty.' % self.__refs_dir
 
         if os.path.exists(self.__base_file):
-            os.remove(self.__base_file)
+            remove_file_and_dirs(
+                os.path.join(self.__base_dir, 'refs', 'bases'), self.__name)
 
     def refresh_patch(self, files = None, message = None, edit = False,
                       show_patch = False,
diff --git a/stgit/utils.py b/stgit/utils.py
index 5749b3b..68b8f58 100644
--- a/stgit/utils.py
+++ b/stgit/utils.py
@@ -1,6 +1,8 @@
 """Common utility functions
 """
 
+import errno, os, os.path
+
 __copyright__ = """
 Copyright (C) 2005, Catalin Marinas <catalin.marinas@gmail.com>
 
@@ -18,6 +20,12 @@ along with this program; if not, write t
 Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
 """
 
+def mkdir_file(filename, mode):
+    """Opens filename with the given mode, creating the directory it's
+    in if it doesn't already exist."""
+    create_dirs(os.path.dirname(filename))
+    return file(filename, mode)
+
 def read_string(filename, multiline = False):
     """Reads the first line from a file
     """
@@ -32,7 +40,7 @@ def read_string(filename, multiline = Fa
 def write_string(filename, line, multiline = False):
     """Writes 'line' to file and truncates it
     """
-    f = file(filename, 'w+')
+    f = mkdir_file(filename, 'w+')
     if multiline:
         f.write(line)
     else:
@@ -42,7 +50,7 @@ def write_string(filename, line, multili
 def append_strings(filename, lines):
     """Appends 'lines' sequence to file
     """
-    f = file(filename, 'a+')
+    f = mkdir_file(filename, 'a+')
     for line in lines:
         print >> f, line
     f.close()
@@ -50,14 +58,14 @@ def append_strings(filename, lines):
 def append_string(filename, line):
     """Appends 'line' to file
     """
-    f = file(filename, 'a+')
+    f = mkdir_file(filename, 'a+')
     print >> f, line
     f.close()
 
 def insert_string(filename, line):
     """Inserts 'line' at the beginning of the file
     """
-    f = file(filename, 'r+')
+    f = mkdir_file(filename, 'r+')
     lines = f.readlines()
     f.seek(0); f.truncate()
     print >> f, line
@@ -67,4 +75,74 @@ def insert_string(filename, line):
 def create_empty_file(name):
     """Creates an empty file
     """
-    file(name, 'w+').close()
+    mkdir_file(name, 'w+').close()
+
+def list_files_and_dirs(path):
+    """Return the sets of filenames and directory names in a
+    directory."""
+    files, dirs = [], []
+    for fd in os.listdir(path):
+        full_fd = os.path.join(path, fd)
+        if os.path.isfile(full_fd):
+            files.append(fd)
+        elif os.path.isdir(full_fd):
+            dirs.append(fd)
+    return files, dirs
+
+def walk_tree(basedir):
+    """Starting in the given directory, iterate through all its
+    subdirectories. For each subdirectory, yield the name of the
+    subdirectory (relative to the base directory), the list of
+    filenames in the subdirectory, and the list of directory names in
+    the subdirectory."""
+    subdirs = ['']
+    while subdirs:
+        subdir = subdirs.pop()
+        files, dirs = list_files_and_dirs(os.path.join(basedir, subdir))
+        for d in dirs:
+            subdirs.append(os.path.join(subdir, d))
+        yield subdir, files, dirs
+
+def strip_prefix(prefix, string):
+    """Return string, without the prefix. Blow up if string doesn't
+    start with prefix."""
+    assert string.startswith(prefix)
+    return string[len(prefix):]
+
+def remove_dirs(basedir, dirs):
+    """Starting at join(basedir, dirs), remove the directory if empty,
+    and try the same with its parent, until we find a nonempty
+    directory or reach basedir."""
+    path = dirs
+    while path:
+        try:
+            os.rmdir(os.path.join(basedir, path))
+        except OSError:
+            return # can't remove nonempty directory
+        path = os.path.dirname(path)
+
+def remove_file_and_dirs(basedir, file):
+    """Remove join(basedir, file), and then remove the directory it
+    was in if empty, and try the same with its parent, until we find a
+    nonempty directory or reach basedir."""
+    os.remove(os.path.join(basedir, file))
+    remove_dirs(basedir, os.path.dirname(file))
+
+def create_dirs(directory):
+    """Create the given directory, if the path doesn't already exist."""
+    if directory:
+        create_dirs(os.path.dirname(directory))
+        try:
+            os.mkdir(directory)
+        except OSError, e:
+            if e.errno != errno.EEXIST:
+                raise e
+
+def rename(basedir, file1, file2):
+    """Rename join(basedir, file1) to join(basedir, file2), not
+    leaving any empty directories behind and creating any directories
+    necessary."""
+    full_file2 = os.path.join(basedir, file2)
+    create_dirs(os.path.dirname(full_file2))
+    os.rename(os.path.join(basedir, file1), full_file2)
+    remove_dirs(basedir, os.path.dirname(file1))
-- 
1.3.2.g639c


-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle

^ permalink raw reply related

* [PATCH 1/3] Don't die when there are no branches
From: Karl Hasselström @ 2006-05-15  9:53 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: Wartan Hachaturow, git
In-Reply-To: <20060510060040.GA3034@diana.vm.bytemark.co.uk>

Signed-off-by: Karl Hasselström <kha@treskal.com>


---

 stgit/commands/branch.py |   11 +++++++----
 1 files changed, 7 insertions(+), 4 deletions(-)

c32f6b7bfd81bdbdb136ff72a4ad073e162ab97c
diff --git a/stgit/commands/branch.py b/stgit/commands/branch.py
index c7561a8..2218bbb 100644
--- a/stgit/commands/branch.py
+++ b/stgit/commands/branch.py
@@ -174,11 +174,14 @@ def func(parser, options, args):
 
         branches = os.listdir(os.path.join(basedir.get(), 'refs', 'heads'))
         branches.sort()
-        max_len = max([len(i) for i in branches])
 
-        print 'Available branches:'
-        for i in branches:
-            __print_branch(i, max_len)
+        if branches:
+            print 'Available branches:'
+            max_len = max([len(i) for i in branches])
+            for i in branches:
+                __print_branch(i, max_len)
+        else:
+            print 'No branches'
         return
 
     elif options.protect:
-- 
1.3.2.g639c


-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle

^ permalink raw reply related

* Re: Tracking branch history
From: Shawn Pearce @ 2006-05-15  9:53 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <20060515063849.GA28337@spearce.org>

Shawn Pearce <spearce@spearce.org> wrote:
> Junio C Hamano <junkio@cox.net> wrote:
> > Shawn Pearce <spearce@spearce.org> writes:
> > 
> > > This is all well and good but its sort of useless without the diffcore
> > > being able to lookup what SHA1 was valid on a given branch at a given
> > > point in time.  :-)
> > >
> > > I'm thinking about extending the 'extended SHA1' syntax to accept
> > > a date (or date expression) as a suffix:
> > >
> > > 	HEAD@'2 hours ago'
> > > 	HEAD@'2006-04-20'
> > > 	HEAD@'2006-04-20 14:12'
> > >
> > > etc... This would be merged into get_sha1 (sha1_name.c) so its
> > > usable pretty much anywhere.  Does this seem reasonable?  If so
> > > I'll work up a patch for it.

This is a preliminary patch for this syntax.  I haven't handled the
absolute date parsing yet; I was hoping to use the same syntax
accepted by GIT_AUTHOR_DATE/GIT_COMMITTER_DATE but looking at the
code in date.c it wasn't going to be easily reused.  I'll work
on it more tomorrow, right now I have to go do my day job to pay
the rent.  :-)

Hmm... A quick look at date.c indicates I should be able to clean up
this parse_date_spec function quite a bit by using code from date.c.
I'll look at it more later.

-- >8 -
Support 'master@2 hours ago' syntax

Extended sha1 expressions may now include date specifications
which indicate a point in time within the local repository's
history.  If the ref indicated to the left of '@' has a log in
$GIT_DIR/logs/<ref> then the value of the ref at the time indicated
by the specification is obtained from the ref's log.

---

 Documentation/git-rev-parse.txt |    6 ++
 refs.c                          |   52 ++++++++++++++
 refs.h                          |    3 +
 sha1_name.c                     |  145 ++++++++++++++++++++++++++++++++++-----
 4 files changed, 189 insertions(+), 17 deletions(-)

1f16364fd8cadb6cdeb0b14a6f5439f02b578924
diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
index ab896fc..df308c3 100644
--- a/Documentation/git-rev-parse.txt
+++ b/Documentation/git-rev-parse.txt
@@ -124,6 +124,12 @@ syntax.
   happen to have both heads/master and tags/master, you can
   explicitly say 'heads/master' to tell git which one you mean.
 
+* A suffix '@' followed by a date specification such as 'yesterday'
+  (24 hours ago) or '1 month 2 weeks 3 days 1 hour 1 second ago'
+  to specify the value of the ref at a prior point in time.
+  This suffix may only be used immediately following a ref name
+  and the ref must have an existing log ($GIT_DIR/logs/<ref>).
+
 * A suffix '{caret}' to a revision parameter means the first parent of
   that commit object.  '{caret}<n>' means the <n>th parent (i.e.
   'rev{caret}'
diff --git a/refs.c b/refs.c
index a50ea8f..da009ac 100644
--- a/refs.c
+++ b/refs.c
@@ -440,3 +440,55 @@ int log_ref_update(const char *ref, cons
 	close(logfd);
 	return 0;
 }
+
+int read_ref_at(const char *ref, unsigned long at_time, unsigned char *sha1)
+{
+	const char *logfile, *logdata, *rec, *c;
+	char *tz_c;
+	int logfd, tz;
+	struct stat st;
+	unsigned long date;
+	unsigned char oldsha1[20];
+
+	logfile = git_path("logs/%s", ref);
+	logfd = open(logfile, O_RDONLY, 0);
+	if (logfd < 0)
+		die("Unable to read log %s: %s", logfile, strerror(errno));
+	fstat(logfd, &st);
+	if (!st.st_size)
+		die("Log %s is empty.", logfile);
+	logdata = mmap(NULL, st.st_size, PROT_READ, MAP_PRIVATE, logfd, 0);
+	close(logfd);
+
+	rec = logdata + st.st_size;
+	while (logdata < rec) {
+		if (logdata < rec && *(rec-1) == '\n')
+			rec--;
+		while (logdata < rec && *(rec-1) != '\n')
+			rec--;
+		c = rec;
+		while (*c++ != '>')
+			/* nada */;
+		date = strtoul(c, NULL, 10);
+		if (date <= at_time) {
+			if (get_sha1_hex(rec, oldsha1))
+				die("Log %s is corrupt.", logfile);
+			if (get_sha1_hex(rec + 41, sha1))
+				die("Log %s is corrupt.", logfile);
+			munmap((void*)logdata, st.st_size);
+			return 0;
+		}
+	}
+
+	c = logdata;
+	while (*c++ != '>')
+		/* nada */;
+	date = strtoul(c, &tz_c, 10);
+	tz = strtoul(tz_c, NULL, 10);
+	if (get_sha1_hex(logdata, sha1))
+		die("Log %s is corrupt.", logfile);
+	munmap((void*)logdata, st.st_size);
+	fprintf(stderr, "warning: Log %s only goes back to %s.\n",
+		logfile, show_rfc2822_date(date, tz));
+	return 0;
+}
diff --git a/refs.h b/refs.h
index de3cb92..4831cdb 100644
--- a/refs.h
+++ b/refs.h
@@ -31,4 +31,7 @@ extern int check_ref_format(const char *
 /** If logging is enabled logs the change made to the ref. **/
 extern int log_ref_update(const char *ref, const unsigned char *currsha1, const unsigned char *newsha1, const char *msg);
 
+/** Reads log for the value of ref during at_time. **/
+extern int read_ref_at(const char *ref, unsigned long at_time, unsigned char *sha1);
+
 #endif /* REFS_H */
diff --git a/sha1_name.c b/sha1_name.c
index dc68355..5f33aea 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -4,6 +4,7 @@ #include "commit.h"
 #include "tree.h"
 #include "blob.h"
 #include "tree-walk.h"
+#include "refs.h"
 
 static int find_short_object_filename(int len, const char *name, unsigned char *sha1)
 {
@@ -234,6 +235,98 @@ static int ambiguous_path(const char *pa
 	return slash;
 }
 
+static unsigned long parse_date_spec(const char *str, int len)
+{
+	long delta;
+	time_t now;
+
+	time(&now);
+	if (len == 9 && !strncasecmp("yesterday", str, 9))
+		return now - 24 * 60 * 60;
+	if (len > 4 && !strncasecmp(" ago", str + (len - 4), 4)) {
+		len -= 4;
+		while (len) {
+			if (len > 2 && !strncasecmp("a ", str, 2)) {
+				delta = 1;
+				len -= 2;
+				str += 2;
+			}
+			else if (len > 3 && !strncasecmp("an ", str, 3)) {
+				delta = 1;
+				len -= 2;
+				str += 2;
+			} else {
+				delta = 0;
+				while (len && isdigit(*str)) {
+					if (delta)
+						delta *= 10;
+					delta += *str++ - '0';
+					len--;
+				}
+				if (!delta)
+					return (time_t)-1;
+				while (len && isspace(*str)) {
+					str++;
+					len--;
+				}
+			}
+
+			if (len >= 5 && !strncasecmp("month", str, 5)) {
+				len -= 5;
+				str += 5;
+				now -= 30 * 24 * 60 * 60 * delta;
+			}
+			else if (len >= 4 && !strncasecmp("week", str, 4)) {
+				len -= 4;
+				str += 4;
+				now -= 7 * 24 * 60 * 60 * delta;
+			}
+			else if (len >= 3 && !strncasecmp("day", str, 3)) {
+				len -= 3;
+				str += 3;
+				now -= 24 * 60 * 60 * delta;
+			}
+			else if (len >= 4 && !strncasecmp("hour", str, 4)) {
+				len -= 4;
+				str += 4;
+				now -= 60 * 60 * delta;
+			}
+			else if (len >= 6 && !strncasecmp("minute", str, 6)) {
+				len -= 6;
+				str += 6;
+				now -= 60 * delta;
+			}
+			else if (len >= 3 && !strncasecmp("min", str, 3)) {
+				len -= 3;
+				str += 3;
+				now -= 60 * delta;
+			}
+			else if (len >= 6 && !strncasecmp("second", str, 6)) {
+				len -= 6;
+				str += 6;
+				now -= delta;
+			}
+			else if (len >= 3 && !strncasecmp("sec", str, 3)) {
+				len -= 3;
+				str += 3;
+				now -= delta;
+			}
+
+			if (len && *str == 's') {
+				len -= 1;
+				str += 1;
+			}
+
+			while (len && isspace(*str)) {
+				str++;
+				len--;
+			}
+		}
+		return now;
+	}
+	return (time_t)-1;
+}
+
 static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
 {
 	static const char *fmt[] = {
@@ -245,36 +338,54 @@ static int get_sha1_basic(const char *st
 		"refs/remotes/%.*s/HEAD",
 		NULL
 	};
-	const char **p;
-	const char *warning = "warning: refname '%.*s' is ambiguous.\n";
-	char *pathname;
-	int already_found = 0;
+	static const char *warning = "warning: refname '%.*s' is ambiguous.\n";
+	const char **p, *pathname;
+	char *real_path = NULL;
+	int refs_found = 0, at_mark;
+	unsigned long at_time = (unsigned long)-1;
 	unsigned char *this_result;
 	unsigned char sha1_from_ref[20];
 
 	if (len == 40 && !get_sha1_hex(str, sha1))
 		return 0;
 
+	/* At a given period of time? "@2 hours ago" */
+	for (at_mark = 1; at_mark < len; at_mark++) {
+		if (str[at_mark] == '@') {
+			at_time = parse_date_spec(str + at_mark + 1, len - at_mark - 1);
+			if (at_time == (unsigned long)-1)
+				die("Invalid date spec after @ in '%.*s'", len, str);
+			len = at_mark;
+		}
+	}
+
 	/* Accept only unambiguous ref paths. */
 	if (ambiguous_path(str, len))
 		return -1;
 
 	for (p = fmt; *p; p++) {
-		this_result = already_found ? sha1_from_ref : sha1;
-		pathname = git_path(*p, len, str);
-		if (!read_ref(pathname, this_result)) {
-			if (warn_ambiguous_refs) {
-				if (already_found)
-					fprintf(stderr, warning, len, str);
-				already_found++;
-			}
-			else
-				return 0;
+		this_result = refs_found ? sha1_from_ref : sha1;
+		pathname = resolve_ref(git_path(*p, len, str), this_result, 1);
+		if (pathname) {
+			if (!refs_found++)
+				real_path = strdup(pathname);
+			if (!warn_ambiguous_refs)
+				break;
 		}
 	}
-	if (already_found)
-		return 0;
-	return -1;
+
+	if (!refs_found)
+		return -1;
+
+	if (warn_ambiguous_refs && refs_found > 1)
+		fprintf(stderr, warning, len, str);
+
+	if (at_time != (unsigned long)-1) {
+		read_ref_at(real_path + strlen(git_path(".")) - 1, at_time, sha1);
+	}
+
+	free(real_path);
+	return 0;
 }
 
 static int get_sha1_1(const char *name, int len, unsigned char *sha1);
-- 
1.3.2.g7278

^ permalink raw reply related

* Re: [PATCH] send-email: allow sendmail binary to be used instead of SMTP
From: Junio C Hamano @ 2006-05-15  9:47 UTC (permalink / raw)
  To: Eric Wong; +Cc: git
In-Reply-To: <20060515092704.GB6855@localdomain>

Eric Wong <normalperson@yhbt.net> writes:

> I believe this is what Martin wanted.  I think it's a good idea since
> sendmail binaries tend to be more flexible, but I'm ok with it either
> way.

I am not opposed to have an option to run a local submission
agent binary (I said I like that if(){}else{} there, didn't I?).
The ability to do so is a good thing.  I am not however sure
about changing the default when no option is specified on the
command line.

>> > +	if ($smtp_server =~ m#^/#) {
>> 
>> I like this if(){}else{} here, but have a feeling that the
>> logging part should be placed outside it to be shared.
>
> Cleaned that up a bit, patch coming.  Also removed the Port: printout
> completely, as it's rather redundant (see below).
>
>> While we are at it, we might want to enhance $smtp_server parsing
>> to take host:port notation so that people can use message
>> submission port 587/tcp (RFC 4409) instead.
>
> This already works, IO::Socket::INET (behind Net::SMTP) takes care of
> it :)

Thanks.  Maybe the next option would be delivery to a file (or
even SMTP batch)? ;-)

^ permalink raw reply

* [PATCH] send-email: quiet some warnings, reject invalid addresses
From: Eric Wong @ 2006-05-15  9:41 UTC (permalink / raw)
  To: git, Junio C Hamano, Martin Langhoff, Ryan Anderson, Greg KH; +Cc: Eric Wong
In-Reply-To: <7vlkt3x1qz.fsf@assigned-by-dhcp.cox.net>

I'm not sure why we never actually rejected invalid addresses in
the first place.  We just seemed to be using our email validity
checkers to kill duplicates.

Now we just drop invalid email addresses completely and warn
the user about it.

Since we support local sendmail, we'll also accept username-only
addresses.

Signed-off-by: Eric Wong <normalperson@yhbt.net>

---

 git-send-email.perl |   15 ++++++++++++---
 1 files changed, 12 insertions(+), 3 deletions(-)

f069e9a9cee726d82dfeee1d477152a0fe0651c1
diff --git a/git-send-email.perl b/git-send-email.perl
index 0540e93..312a4ea 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -307,6 +307,10 @@ our ($message_id, $cc, %mail, $subject, 
 
 sub extract_valid_address {
 	my $address = shift;
+
+	# check for a local address:
+	return $address if ($address =~ /^([\w\-]+)$/);
+
 	if ($have_email_valid) {
 		return Email::Valid->address($address);
 	} else {
@@ -498,9 +502,14 @@ sub unique_email_list(@) {
 	my @emails;
 
 	foreach my $entry (@_) {
-		my $clean = extract_valid_address($entry);
-		next if $seen{$clean}++;
-		push @emails, $entry;
+		if (my $clean = extract_valid_address($entry)) {
+			$seen{$clean} ||= 0;
+			next if $seen{$clean}++;
+			push @emails, $entry;
+		} else {
+			print STDERR "W: unable to extract a valid address",
+					" from: $entry\n";
+		}
 	}
 	return @emails;
 }
-- 
1.3.2.g7d11

^ permalink raw reply related

* [PATCH] send-email: allow sendmail binary to be used instead of SMTP
From: Eric Wong @ 2006-05-15  9:34 UTC (permalink / raw)
  To: Junio C Hamano, Martin Langhoff, Ryan Anderson, Greg KH; +Cc: Eric Wong
In-Reply-To: <20060515092704.GB6855@localdomain>

This should make local mailing possible for machines without
a connection to an SMTP server.

It'll default to using /usr/sbin/sendmail or /usr/lib/sendmail
if no SMTP server is specified (the default).  If it can't find
either of those paths, it'll fall back to connecting to an SMTP
server on localhost.

Signed-off-by: Eric Wong <normalperson@yhbt.net>

---

 git-send-email.perl |   60 ++++++++++++++++++++++++++++++++++-----------------
 1 files changed, 40 insertions(+), 20 deletions(-)

d4248a8ab7c883ab0f4dc080374bf60dc582f0f4
diff --git a/git-send-email.perl b/git-send-email.perl
index d8c4b1f..0540e93 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -40,7 +40,8 @@ # Variables we fill in automatically, or
 my (@to,@cc,@initial_cc,$initial_reply_to,$initial_subject,@files,$from,$compose,$time);
 
 # Behavior modification variables
-my ($chain_reply_to, $smtp_server, $quiet, $suppress_from, $no_signed_off_cc) = (1, "localhost", 0, 0, 0);
+my ($chain_reply_to, $quiet, $suppress_from, $no_signed_off_cc) = (1, 0, 0, 0);
+my $smtp_server;
 
 # Example reply to:
 #$initial_reply_to = ''; #<20050203173208.GA23964@foobar.com>';
@@ -179,8 +180,14 @@ if (!defined $initial_reply_to && $promp
 	$initial_reply_to =~ s/(^\s+|\s+$)//g;
 }
 
-if (!defined $smtp_server) {
-	$smtp_server = "localhost";
+if (!$smtp_server) {
+	foreach (qw( /usr/sbin/sendmail /usr/lib/sendmail )) {
+		if (-x $_) {
+			$smtp_server = $_;
+			last;
+		}
+	}
+	$smtp_server ||= 'localhost'; # could be 127.0.0.1, too... *shrug*
 }
 
 if ($compose) {
@@ -358,26 +365,39 @@ X-Mailer: git-send-email $gitversion
 ";
 	$header .= "In-Reply-To: $reply_to\n" if $reply_to;
 
-	$smtp ||= Net::SMTP->new( $smtp_server );
-	$smtp->mail( $from ) or die $smtp->message;
-	$smtp->to( @recipients ) or die $smtp->message;
-	$smtp->data or die $smtp->message;
-	$smtp->datasend("$header\n$message") or die $smtp->message;
-	$smtp->dataend() or die $smtp->message;
-	$smtp->ok or die "Failed to send $subject\n".$smtp->message;
-
+	if ($smtp_server =~ m#^/#) {
+		my $pid = open my $sm, '|-';
+		defined $pid or die $!;
+		if (!$pid) {
+			exec($smtp_server,'-i',@recipients) or die $!;
+		}
+		print $sm "$header\n$message";
+		close $sm or die $?;
+	} else {
+		$smtp ||= Net::SMTP->new( $smtp_server );
+		$smtp->mail( $from ) or die $smtp->message;
+		$smtp->to( @recipients ) or die $smtp->message;
+		$smtp->data or die $smtp->message;
+		$smtp->datasend("$header\n$message") or die $smtp->message;
+		$smtp->dataend() or die $smtp->message;
+		$smtp->ok or die "Failed to send $subject\n".$smtp->message;
+	}
 	if ($quiet) {
 		printf "Sent %s\n", $subject;
 	} else {
-		print "OK. Log says:
-Date: $date
-Server: $smtp_server Port: 25
-From: $from
-Subject: $subject
-Cc: $cc
-To: $to
-
-Result: ", $smtp->code, ' ', ($smtp->message =~ /\n([^\n]+\n)$/s), "\n";
+		print "OK. Log says:\nDate: $date\n";
+		if ($smtp) {
+			print "Server: $smtp_server\n";
+		} else {
+			print "Sendmail: $smtp_server\n";
+		}
+		print "From: $from\nSubject: $subject\nCc: $cc\nTo: $to\n\n";
+		if ($smtp) {
+			print "Result: ", $smtp->code, ' ',
+				($smtp->message =~ /\n([^\n]+\n)$/s), "\n";
+		} else {
+			print "Result: OK\n";
+		}
 	}
 }
 
-- 
1.3.2.g7d11

^ permalink raw reply related

* Re: [PATCH] send-email: allow sendmail binary to be used instead of SMTP
From: Eric Wong @ 2006-05-15  9:27 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Martin Langhoff, Greg KH, Ryan Anderson
In-Reply-To: <7vpsifx2b7.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano <junkio@cox.net> wrote:
> Eric Wong <normalperson@yhbt.net> writes:
> 
> > This should make local mailing possible for machines without
> > a connection to an SMTP server.
> 
> Which is a good thing, but
> 
> > It'll default to using /usr/sbin/sendmail or /usr/lib/sendmail
> > if no SMTP server is specified (the default).  If it can't find
> > either of those paths, it'll fall back to connecting to an SMTP
> > server on localhost.
> 
> I do not know if it is OK to change the default to first prefer
> local MDA executable and then "localhost".  That is, ...
> 
> > @@ -179,8 +180,14 @@ if (!defined $initial_reply_to && $promp
> >  	$initial_reply_to =~ s/(^\s+|\s+$)//g;
> >  }
> >  
> > -if (!defined $smtp_server) {
> > -	$smtp_server = "localhost";
> > +if (!$smtp_server) {
> > +	foreach (qw( /usr/sbin/sendmail /usr/lib/sendmail )) {
> > +		if (-x $_) {
> > +			$smtp_server = $_;
> > +			last;
> > +		}
> > +	}
> > +	$smtp_server ||= 'localhost'; # could be 127.0.0.1, too... *shrug*
> >  }
> >  
> >  if ($compose) {
> 
> Without this hunk, people who did not specify --smtp-server=host
> could get away with having anything that listens to 25/tcp on
> the localhost that is not either of the above two paths; now
> they have to explicitly say --smtp-server=localhost to defeat
> what this hunk does.  I do not know if it is a big deal, though.

I believe this is what Martin wanted.  I think it's a good idea since
sendmail binaries tend to be more flexible, but I'm ok with it either
way.

Of course, Greg and Ryan were the original authors of this, so I'd
like their take on it, too.

> > +	if ($smtp_server =~ m#^/#) {
> 
> I like this if(){}else{} here, but have a feeling that the
> logging part should be placed outside it to be shared.

Cleaned that up a bit, patch coming.  Also removed the Port: printout
completely, as it's rather redundant (see below).

> While we are at it, we might want to enhance $smtp_server parsing
> to take host:port notation so that people can use message
> submission port 587/tcp (RFC 4409) instead.

This already works, IO::Socket::INET (behind Net::SMTP) takes care of
it :)

-- 
Eric Wong

^ permalink raw reply

* align git diff a..b semantics with git log a..b
From: Salikh Zakirov @ 2006-05-15  8:41 UTC (permalink / raw)
  To: git

Hi,

Currently, git-diff accepts

	git-diff A..B

syntax, and the output seems to be equivalent of 'git-diff A B'.

IMHO, this conflicts with A..B semantics defined for git-log family of commands.
Consider following tree

O--X--Y--A1--A2  (a)
       \
        B1--B2  (b)

The command 'git log a..b' will show B1,B2, so I would find it
intuitive (and useful) to have 'git diff a..b' show B1+B2,
rather than -A2-A1+B1+B2.

So, I suggest to change semantics of 'git diff a..b'
to 'git diff `git-merge-base a b` b'.

I could contribute the documentation change if the idea is accepted and implemented.

Thanks a lot!

^ permalink raw reply

* (Not) What's in git.git
From: Junio C Hamano @ 2006-05-15  8:46 UTC (permalink / raw)
  To: git

Over the weekend everybody was busy feeding me patches, so I've
added much stuff in the "next", but without really looking deep
into each of them.  It still passes the testsuite, but that
tells us more about the sparseness of the tests not the quality
of what are in the branch.

I'll review them in the coming week and have them graduate to
"master", perhaps slowly.  Also what's currently in "maint" will
be tagged as 1.3.3.

I expect myself to be a bit slower for a couple of days at
least; I am still adjusting to the new development environment.
I haven't fully installed what I need nor configuired it
properly yet.

^ permalink raw reply

* Re: The git newbie experience
From: Junio C Hamano @ 2006-05-15  8:39 UTC (permalink / raw)
  To: Shawn Pearce, Tommi Virtanen; +Cc: git
In-Reply-To: <20060515053133.GB28068@spearce.org>

Shawn Pearce <spearce@spearce.org> writes:

>> I'd rather do that with a diff file that can be used to do a
>> 3-way (see how rebase does it with --full-index diff with am -3).
>> No point creating and forgetting to remove a throw away branch
>> and getting more complaints.
>
> How is a quick stash different from a topic branch?

The original version of my message in response to TV looked like
this.

 - Jack is a beginning user of git and does not (want to) understand
   the index (right now).

 - Jack works on branch X, say his HEAD points to X1. He has an edited,
   uncommitted files with the names A, B and C.

 - Jack wants to pull new changes made by others to his branch.
   But "git merge" invoked from "git pull" says he needs to stash
   away the local changes to do the merge.

 - Jack stashes away what he has been working on and cleans up
   his mess.

   git checkout -b stash ;# risks error when "stash" exists
   git commit -a -m 'Stashing WIP'
   git checkout master ;# assuming that was where he was

 - Jack then pulls.  There are merge conflicts in files D, E, ..., Z.

 - Jack resolves the merge conflicts and is ready to commit the resulting
   merge. Note files A, B and C do not have his unfinished work.

   There is no "if Jack does this or that" problem; he says "git
   commit -a" because that is the only "commit" command he knows
   about.

 - Jack then reapplies what he stashed away, and keeps working.

   git pull . --no-commit stash
   git branch -D stash

You have to teach the new user to (1) name something, only to
immediately discard it when he returns to what he was in the
middle of, (2) remember to clean up the temporary thing once he
is done lest he forgets to clean it up (and common names like
"stash", "tmp" will be reused by accident causing grief next
time he needs to do another stash), and (3) use of --no-commit
pull.

On the other hand, "git stash/unstash" workflow would be quite
simple:

	$ git stash >my.precious.state
        ... do whatever you want to deviate to
        $ git unstash <my.precious.state

Merge resolve might be needed while unstashing, but 
we are talking about pulling somebody else's work in "do
whatever" part, so that is something the user knows how to
perform anyway.

A quick and dirty stash implementation would go like this:

Stash is easy.

        #!/bin/sh
        # git stash
        git diff --binary HEAD
        git reset --hard

Unstash is a bit involved.

        #!/bin/sh
        # git unstash
        . git-sh-setup
        O_OBJECT=`cd "$GIT_OBJECT_DIRECTORY" && pwd`
        O_DIR=`cd "$GIT_DIR" && pwd`
        stash="$O_DIR/.stash$$"
        rm -fr "$stash.*"
        trap 'rm -rf $stash.*' 0
        cat >"$stash.patch"
        git-apply -z --index-info <"$stash.patch" >"$stash.list"
        GIT_INDEX_FILE="$stash.index"  \
        GIT_OBJECT_DIRECTORY="$O_OBJECT" \
        (
                mkdir -p "$stash.tmp" &&
                git-update-index -z --index-info <"$stash.list" &&
                git-write-tree >"$stash.base" &&
                cd "$stash.tmp" &&
                git-apply --binary --index <"$stash.patch" &&
                git-write-tree >"$stash.his"
        )
        his_tree=$(cat "$stash.his")
        orig_tree=$(cat "$stash.base")
        rm -fr "$stash.*"
        git-merge-resolve $orig_tree -- HEAD $his_tree

This is essentially the core of "am -3" logic; if you are going
to use this for real, you would probably want to see if the
patch applies cleanly before falling back on the three-way
merge, though.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox