Git development
 help / color / mirror / Atom feed
* [JGIT PATCH 1/5] Include a progress meter for large uploads to Amazon S3
From: Shawn O. Pearce @ 2008-07-10  6:13 UTC (permalink / raw)
  To: Robin Rosenberg, Marek Zawirski; +Cc: git
In-Reply-To: <1215670403-19191-1-git-send-email-spearce@spearce.org>

If we are uploading a sizable pack file or idx file to the S3
service the upload happens after we have finished writing the pack
to a local temporary file.  This causes a long pause for the user
while they wait for the data transfer to complete, with no real
indication of progress happening.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../src/org/spearce/jgit/transport/AmazonS3.java   |   29 +++++++++++++++----
 .../spearce/jgit/transport/TransportAmazonS3.java  |    7 +++-
 .../org/spearce/jgit/transport/TransportSftp.java  |    5 +++-
 .../spearce/jgit/transport/WalkPushConnection.java |    5 ++-
 .../jgit/transport/WalkRemoteObjectDatabase.java   |   11 ++++++-
 5 files changed, 44 insertions(+), 13 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/AmazonS3.java b/org.spearce.jgit/src/org/spearce/jgit/transport/AmazonS3.java
index e8575aa..59337f8 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/AmazonS3.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/AmazonS3.java
@@ -75,6 +75,8 @@ import javax.crypto.spec.SecretKeySpec;
 
 import org.spearce.jgit.awtui.AwtAuthenticator;
 import org.spearce.jgit.lib.Constants;
+import org.spearce.jgit.lib.NullProgressMonitor;
+import org.spearce.jgit.lib.ProgressMonitor;
 import org.spearce.jgit.util.Base64;
 import org.spearce.jgit.util.HttpSupport;
 import org.spearce.jgit.util.TemporaryBuffer;
@@ -376,7 +378,7 @@ public class AmazonS3 {
 			// We have to copy to produce the cipher text anyway so use
 			// the large object code path as it supports that behavior.
 			//
-			final OutputStream os = beginPut(bucket, key);
+			final OutputStream os = beginPut(bucket, key, null, null);
 			os.write(data);
 			os.close();
 			return;
@@ -430,11 +432,17 @@ public class AmazonS3 {
 	 *            name of the bucket storing the object.
 	 * @param key
 	 *            key of the object within its bucket.
+	 * @param monitor
+	 *            (optional) progress monitor to post upload completion to
+	 *            during the stream's close method.
+	 * @param monitorTask
+	 *            (optional) task name to display during the close method.
 	 * @return a stream which accepts the new data, and transmits once closed.
 	 * @throws IOException
 	 *             if encryption was enabled it could not be configured.
 	 */
-	public OutputStream beginPut(final String bucket, final String key)
+	public OutputStream beginPut(final String bucket, final String key,
+			final ProgressMonitor monitor, final String monitorTask)
 			throws IOException {
 		final MessageDigest md5 = newMD5();
 		final TemporaryBuffer buffer = new TemporaryBuffer() {
@@ -442,7 +450,8 @@ public class AmazonS3 {
 			public void close() throws IOException {
 				super.close();
 				try {
-					putImpl(bucket, key, md5.digest(), this);
+					putImpl(bucket, key, md5.digest(), this, monitor,
+							monitorTask);
 				} finally {
 					destroy();
 				}
@@ -452,7 +461,13 @@ public class AmazonS3 {
 	}
 
 	private void putImpl(final String bucket, final String key,
-			final byte[] csum, final TemporaryBuffer buf) throws IOException {
+			final byte[] csum, final TemporaryBuffer buf,
+			ProgressMonitor monitor, String monitorTask) throws IOException {
+		if (monitor == null)
+			monitor = NullProgressMonitor.INSTANCE;
+		if (monitorTask == null)
+			monitorTask = "Uploading " + key;
+
 		final String md5str = Base64.encodeBytes(csum);
 		final long len = buf.length();
 		final String lenstr = String.valueOf(len);
@@ -465,10 +480,12 @@ public class AmazonS3 {
 			authorize(c);
 			c.setDoOutput(true);
 			c.setFixedLengthStreamingMode((int) len);
+			monitor.beginTask(monitorTask, (int) (len / 1024));
 			final OutputStream os = c.getOutputStream();
 			try {
-				buf.writeTo(os, null);
+				buf.writeTo(os, monitor);
 			} finally {
+				monitor.endTask();
 				os.close();
 			}
 
@@ -641,7 +658,7 @@ public class AmazonS3 {
 		} else if ("rm".equals(op) || "delete".equals(op)) {
 			s3.delete(bucket, key);
 		} else if ("put".equals(op)) {
-			final OutputStream os = s3.beginPut(bucket, key);
+			final OutputStream os = s3.beginPut(bucket, key, null, null);
 			final byte[] tmp = new byte[2048];
 			int n;
 			while ((n = System.in.read(tmp)) > 0)
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportAmazonS3.java b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportAmazonS3.java
index d2f4c83..cd62c5b 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportAmazonS3.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportAmazonS3.java
@@ -54,6 +54,7 @@ import java.util.TreeMap;
 import org.spearce.jgit.errors.NotSupportedException;
 import org.spearce.jgit.errors.TransportException;
 import org.spearce.jgit.lib.ObjectId;
+import org.spearce.jgit.lib.ProgressMonitor;
 import org.spearce.jgit.lib.Ref;
 import org.spearce.jgit.lib.Repository;
 import org.spearce.jgit.lib.Ref.Storage;
@@ -236,8 +237,10 @@ class TransportAmazonS3 extends WalkTransport {
 		}
 
 		@Override
-		OutputStream writeFile(final String path) throws IOException {
-			return s3.beginPut(bucket, resolveKey(path));
+		OutputStream writeFile(final String path,
+				final ProgressMonitor monitor, final String monitorTask)
+				throws IOException {
+			return s3.beginPut(bucket, resolveKey(path), monitor, monitorTask);
 		}
 
 		@Override
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportSftp.java b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportSftp.java
index a33406b..c2cbe6a 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportSftp.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportSftp.java
@@ -54,6 +54,7 @@ import java.util.TreeMap;
 
 import org.spearce.jgit.errors.TransportException;
 import org.spearce.jgit.lib.ObjectId;
+import org.spearce.jgit.lib.ProgressMonitor;
 import org.spearce.jgit.lib.Ref;
 import org.spearce.jgit.lib.Repository;
 import org.spearce.jgit.lib.Ref.Storage;
@@ -294,7 +295,9 @@ class TransportSftp extends WalkTransport {
 		}
 
 		@Override
-		OutputStream writeFile(final String path) throws IOException {
+		OutputStream writeFile(final String path,
+				final ProgressMonitor monitor, final String monitorTask)
+				throws IOException {
 			try {
 				return ftp.put(path);
 			} catch (SftpException je) {
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/WalkPushConnection.java b/org.spearce.jgit/src/org/spearce/jgit/transport/WalkPushConnection.java
index bb5a653..904a699 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/WalkPushConnection.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/WalkPushConnection.java
@@ -230,14 +230,15 @@ class WalkPushConnection extends BaseConnection implements PushConnection {
 			// Write the pack file, then the index, as readers look the
 			// other direction (index, then pack file).
 			//
-			OutputStream os = dest.writeFile(pathPack);
+			final String wt = "Put " + base.substring(0, 12);
+			OutputStream os = dest.writeFile(pathPack, monitor, wt + "..pack");
 			try {
 				pw.writePack(os);
 			} finally {
 				os.close();
 			}
 
-			os = dest.writeFile(pathIdx);
+			os = dest.writeFile(pathIdx, monitor, wt + "..idx");
 			try {
 				pw.writeIndex(os);
 			} finally {
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/WalkRemoteObjectDatabase.java b/org.spearce.jgit/src/org/spearce/jgit/transport/WalkRemoteObjectDatabase.java
index 915faac..c5a5199 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/WalkRemoteObjectDatabase.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/WalkRemoteObjectDatabase.java
@@ -52,6 +52,7 @@ import java.util.Map;
 import org.spearce.jgit.errors.TransportException;
 import org.spearce.jgit.lib.Constants;
 import org.spearce.jgit.lib.ObjectId;
+import org.spearce.jgit.lib.ProgressMonitor;
 import org.spearce.jgit.lib.Ref;
 import org.spearce.jgit.util.NB;
 
@@ -213,11 +214,17 @@ abstract class WalkRemoteObjectDatabase {
 	 *         complete the write request. The stream is not buffered and each
 	 *         write may cause a network request/response so callers should
 	 *         buffer to smooth out small writes.
+	 * @param monitor
+	 *            (optional) progress monitor to post write completion to during
+	 *            the stream's close method.
+	 * @param monitorTask
+	 *            (optional) task name to display during the close method.
 	 * @throws IOException
 	 *             writing is not supported, or attempting to write the file
 	 *             failed, possibly due to permissions or remote disk full, etc.
 	 */
-	OutputStream writeFile(final String path) throws IOException {
+	OutputStream writeFile(final String path, final ProgressMonitor monitor,
+			final String monitorTask) throws IOException {
 		throw new IOException("Writing of '" + path + "' not supported.");
 	}
 
@@ -247,7 +254,7 @@ abstract class WalkRemoteObjectDatabase {
 	 *             failed, possibly due to permissions or remote disk full, etc.
 	 */
 	void writeFile(final String path, final byte[] data) throws IOException {
-		final OutputStream os = writeFile(path);
+		final OutputStream os = writeFile(path, null, null);
 		try {
 			os.write(data);
 		} finally {
-- 
1.5.6.2.393.g45096

^ permalink raw reply related

* [JGIT PATCH 0/5] Yet another round of transport fixes
From: Shawn O. Pearce @ 2008-07-10  6:13 UTC (permalink / raw)
  To: Robin Rosenberg, Marek Zawirski; +Cc: git

Apparently tweaking minor details is never done.  This short
series improves minor issues I have with the transport code
in relation to error and progress reporting.  We also get a
small performance boost for tag fetching over SSH.

Shawn O. Pearce (5):
  Include a progress meter for large uploads to Amazon S3
  Don't display passwords on the console in fetch/push output
  Reuse the same SSH connection when automatically fetching tags
  Report remote SSH execution errors during push via TransportException
  Explicitly capture the stderr from a failed SSH fetch or push

 .../org/spearce/egit/core/op/CloneOperation.java   |   10 ++-
 .../spearce/egit/ui/EclipseSshSessionFactory.java  |    5 +-
 .../egit/ui/internal/clone/SourceBranchPage.java   |    1 +
 .../spearce/jgit/transport/PushProcessTest.java    |    5 +
 .../org/spearce/jgit/transport/TransportTest.java  |   10 ++
 .../jgit/errors/NoRemoteRepositoryException.java   |   59 +++++++++++
 .../src/org/spearce/jgit/pgm/Fetch.java            |   19 +++--
 .../src/org/spearce/jgit/pgm/LsRemote.java         |    1 +
 .../src/org/spearce/jgit/pgm/Push.java             |    2 +-
 .../src/org/spearce/jgit/transport/AmazonS3.java   |   29 +++++-
 .../spearce/jgit/transport/BasePackConnection.java |    3 +-
 .../jgit/transport/DefaultSshSessionFactory.java   |   30 ++++++-
 .../src/org/spearce/jgit/transport/Transport.java  |   10 ++
 .../spearce/jgit/transport/TransportAmazonS3.java  |   12 ++-
 .../spearce/jgit/transport/TransportBundle.java    |    5 +
 .../spearce/jgit/transport/TransportGitAnon.java   |    5 +
 .../spearce/jgit/transport/TransportGitSsh.java    |  103 +++++++++++++-------
 .../org/spearce/jgit/transport/TransportHttp.java  |    5 +
 .../org/spearce/jgit/transport/TransportLocal.java |    5 +
 .../org/spearce/jgit/transport/TransportSftp.java  |   57 ++++++-----
 .../spearce/jgit/transport/WalkPushConnection.java |    5 +-
 .../jgit/transport/WalkRemoteObjectDatabase.java   |   11 ++-
 22 files changed, 303 insertions(+), 89 deletions(-)
 create mode 100644 org.spearce.jgit/src/org/spearce/jgit/errors/NoRemoteRepositoryException.java

^ permalink raw reply

* Re: [PATCH] Correct pack memory leak causing git gc to try to exceed ulimit
From: Nicolas Pitre @ 2008-07-10  5:37 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Junio C Hamano, git
In-Reply-To: <20080709071007.GA31974@spearce.org>

On Wed, 9 Jul 2008, Shawn O. Pearce wrote:

> When recursing to unpack a delta base we must unuse_pack() so that
> the pack window for the current object does not remain pinned in
> memory while the delta base is itself being unpacked and materialized
> for our use.
> 
> On a long delta chain of 50 objects we may need to access 6 different
> windows from a very large (>3G) pack file in order to obtain all
> of the delta base content.  If the process ulimit permits us to
> map/allocate only 1.5G we must release windows during this recursion
> to ensure we stay within the ulimit and transition memory from pack
> cache to standard malloc, or other mmap needs.
> 
> Inserting an unuse_pack() call prior to the recursion allows us to
> avoid pinning the current window, making it available for garbage
> collection if memory runs low.
> 
> This has been broken since at least before 1.5.1-rc1, and very
> likely earlier than that.  Its fixed now.  :)

Well well...  I suspect this might have been the cause of our strange 
out-of-memory issue when trying to repack the gcc repository a while 
ago.

I updated my copy of git://git.infradead.org/gcc.git and re-attempted a 
full repack with window=500 and depth=500, and this time it worked for 
me on a 32-bit machine!  Took about 6h30 single-threaded, and produced a 
mere 331MB pack file containing 1254664 objects across 142328 commits.


Nicolas

^ permalink raw reply

* Re: [PATCH 2/4] git-imap-send: Add support for SSL.
From: Mike Hommey @ 2008-07-10  5:33 UTC (permalink / raw)
  To: Robert Shearman; +Cc: git, Robert Shearman
In-Reply-To: <1215638942-25010-2-git-send-email-robertshearman@gmail.com>

On Wed, Jul 09, 2008 at 10:29:00PM +0100, Robert Shearman wrote:
> From: Robert Shearman <rob@codeweavers.com>
> 
> Allow SSL to be used when a imaps:// URL is used for the host name.
> 
> Also, automatically use TLS when not using imaps:// by using the IMAP STARTTLS command, if the server supports it.
> 
> Tested with Courier and Gimap IMAP servers.

Please do an alternative implementation using GnuTLS. Why ? Because curl
can be built against openssl or gnutls, and it would be sad if git
ended up depending on both indirectly.

Mike

^ permalink raw reply

* Re: [PATCH] apply: fix copy/rename breakage
From: Stephan Beyer @ 2008-07-10  4:21 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Linus Torvalds, Don Zickus
In-Reply-To: <7vy74aqvr1.fsf@gitster.siamese.dyndns.org>

Hi,

On Wed, Jul 09, 2008 at 08:10:58PM -0700, Junio C Hamano wrote:
> 
> 	diff --git a/A b/A
> 	--- a/A
> 	+++ b/A
> 	... change text here ...
> 	diff --git a/A b/B
> 	copy from A
> 	copy to B
> 	--- a/A
> 	+++ b/B
> 	... change text here ...

Big thanks! Now my patch applies cleanly again and many others, too. So:

Tested-by: Stephan Beyer <s-beyer@gmx.net>

;)

Regards,
  Stephan

-- 
Stephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F

^ permalink raw reply

* [PATCH] bisect: test merge base if good rev is not an ancestor of bad rev
From: Christian Couder @ 2008-07-10  3:41 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, Michael Haggerty, Jeff King, git

Before this patch, "git bisect", when it was given some good revs that
are not ancestor of the bad rev, didn't check if the merge bases were
good. "git bisect" just supposed that the user knew what he was doing,
and that, when he said the revs were good, he knew that it meant that
all the revs in the history leading to the good revs were also
considered good.

But in pratice, the user may not know that a good rev is not an
ancestor of the bad rev, or he may not know/remember that all revs
leading to the good rev will be considered good. So he may give a good
rev that is a sibling, instead of an ancestor, of the bad rev, when in
fact there can be one rev becoming good in the branch of the good rev
(because the bug was already fixed there for example) instead of one
rev becoming bad in the branch of the bad rev.

For example, if there is the following history:

A-B-C-D
   \E-F

and we launch "git bisect start D F" then only C and D would have been
considered as possible first bad commit before this patch. This may be
wrong because A, B and E may be bad too if the bug exists everywhere
except in F that fixes it.

The purpose of this patch is to detect when "git bisect" is passed
some good revs that are not ancestor of the bad rev, and then to first
ask the user to test the merge bases between the good and bad revs.

If the merge bases are good then all is fine, we can continue
bisecting. Otherwise, if one merge base is bad, it means that the
assumption that all revs leading to the good one are good too is
wrong and we error out. In the case where one merge base is skipped we
issue a warning and then continue bisecting anyway.

These checks will also catch the case where good and bad have been
mistaken. This means that we can remove the check that was done latter
on the output of "git rev-list --bisect-vars".

In the current implementation though, only one merge base between a
good and a bad rev is tested.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 git-bisect.sh               |  137 +++++++++++++++++++++++++++++++++++--------
 t/t6030-bisect-porcelain.sh |   41 +++++++++++++
 2 files changed, 153 insertions(+), 25 deletions(-)

	Le mardi 1 juillet 2008, Junio C Hamano a écrit :
	> Aside from the "test a trial merge" idea I floated in the other message,
	> when we detect such a fork, perhaps we can suggest testing the merge base
	> version (B in your picture) first?  We would immediately know as the user
	> would say "B is bad" if the topology is problematic.
	>
	> Then, we can suggest the user that breakage at D may not be a regression
	> but a longstanding bug that was recently fixed somewhere between B and F.

	Here is a patch to do that.

diff --git a/git-bisect.sh b/git-bisect.sh
index 991b2ef..50f912f 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -242,33 +242,18 @@ bisect_auto_next() {
 	bisect_next_check && bisect_next || :
 }
 
-eval_rev_list() {
-	_eval="$1"
-
-	eval $_eval
-	res=$?
-
-	if [ $res -ne 0 ]; then
-		echo >&2 "'git rev-list --bisect-vars' failed:"
-		echo >&2 "maybe you mistake good and bad revs?"
-		exit $res
-	fi
-
-	return $res
-}
-
 filter_skipped() {
 	_eval="$1"
 	_skip="$2"
 
 	if [ -z "$_skip" ]; then
-		eval_rev_list "$_eval"
+		eval "$_eval"
 		return
 	fi
 
 	# Let's parse the output of:
 	# "git rev-list --bisect-vars --bisect-all ..."
-	eval_rev_list "$_eval" | while read hash line
+	eval "$_eval" | while read hash line
 	do
 		case "$VARS,$FOUND,$TRIED,$hash" in
 			# We display some vars.
@@ -331,20 +316,122 @@ exit_if_skipped_commits () {
 	fi
 }
 
+checkout() {
+	_rev="$1"
+	_msg="$2"
+	echo "Bisecting: $_msg"
+	git checkout -q "$_rev" || exit
+	git show-branch "$_rev"
+}
+
+is_among() {
+	_rev="$1"
+	_list="$2"
+	case "$_list" in *$_rev*) return 0 ;; esac
+	return 1
+}
+
+is_merge_base_ok() {
+	grep "^$1 $2 ok$" "$GIT_DIR/BISECT_MERGE_BASES" >/dev/null 2>&1
+}
+
+mark_merge_base_ok() {
+	echo "$1 $2 ok" >> "$GIT_DIR/BISECT_MERGE_BASES"
+}
+
+is_testing_merge_base() {
+	grep "^testing $1$" "$GIT_DIR/BISECT_MERGE_BASES" >/dev/null 2>&1
+}
+
+mark_testing_merge_base() {
+	echo "testing $1" >> "$GIT_DIR/BISECT_MERGE_BASES"
+}
+
+handle_bad_merge_base() {
+	_badmb="$1"
+	_g="$2"
+	if is_testing_merge_base "$_badmb"; then
+		cat >&2 <<EOF
+The merge base $_badmb is bad.
+This means the bug has been fixed between $_badmb and $_g.
+EOF
+		exit 3
+	else
+		cat >&2 <<EOF
+Some good revs are not ancestor of the bad rev.
+git bisect cannot work properly in this case.
+Maybe you mistake good and bad revs?
+EOF
+		exit 1
+	fi
+}
+
+handle_skipped_merge_base() {
+	_bad="$1"
+	_g="$2"
+	_mb="$3"
+	cat >&2 <<EOF
+Warning: the merge base between $_bad and $_g must be skipped.
+So we cannot be sure the first bad commit is between $_mb and $_bad.
+We continue anyway.
+EOF
+	mark_merge_base_ok "$_bad" "$_g"
+}
+
+check_merge_bases() {
+	_bad="$1"
+	_good="$2"
+	_skip="$3"
+	for _g in $_good; do
+		is_merge_base_ok "$_bad" "$_g" && continue
+		_mb=$(git merge-base $_g $_bad)
+		if test "$_mb" = "$_g" || is_among "$_mb" "$_good"; then
+			mark_merge_base_ok "$_bad" "$_g"
+		elif test "$_mb" = "$_bad"; then
+			handle_bad_merge_base "$_bad" "$_g"
+		elif is_among "$_mb" "$_skip"; then
+			handle_skipped_merge_base "$_bad" "$_g" "_mb"
+		else
+			mark_testing_merge_base "$_mb"
+			checkout "$_mb" "a merge base must be tested"
+			checkout_done=1
+			break
+		fi
+	done
+}
+
+check_good_are_ancestors_of_bad() {
+	test -f "$GIT_DIR/BISECT_ANCESTORS_OK" && return
+	_bad="$1"
+	_good=$(echo $2 | sed -e 's/\^//g')
+	_skip="$3"
+	_side=$(git rev-list $_good ^$_bad)
+	if test -n "$_side"; then
+		check_merge_bases "$_bad" "$_good" "$_skip" || return
+		test "$checkout_done" -eq "1" && return
+	fi
+	: > "$GIT_DIR/BISECT_ANCESTORS_OK"
+}
+
 bisect_next() {
 	case "$#" in 0) ;; *) usage ;; esac
 	bisect_autostart
 	bisect_next_check good
 
+	# Get bad, good and skipped revs
+	bad=$(git rev-parse --verify refs/bisect/bad) &&
+	good=$(git for-each-ref --format='^%(objectname)' \
+		"refs/bisect/good-*" | tr '\012' ' ') &&
 	skip=$(git for-each-ref --format='%(objectname)' \
-		"refs/bisect/skip-*" | tr '\012' ' ') || exit
+		"refs/bisect/skip-*" | tr '\012' ' ') &&
 
+	# Maybe some merge bases must be tested first
+	check_good_are_ancestors_of_bad "$bad" "$good" "$skip" || exit
+	test "$checkout_done" -eq "1" && checkout_done='' && return
+
+	# Get bisection information
 	BISECT_OPT=''
 	test -n "$skip" && BISECT_OPT='--bisect-all'
-
-	bad=$(git rev-parse --verify refs/bisect/bad) &&
-	good=$(git for-each-ref --format='^%(objectname)' \
-		"refs/bisect/good-*" | tr '\012' ' ') &&
 	eval="git rev-list --bisect-vars $BISECT_OPT $good $bad --" &&
 	eval="$eval $(cat "$GIT_DIR/BISECT_NAMES")" &&
 	eval=$(filter_skipped "$eval" "$skip") &&
@@ -365,9 +452,7 @@ bisect_next() {
 	# commit is also a "skip" commit (see above).
 	exit_if_skipped_commits "$bisect_rev"
 
-	echo "Bisecting: $bisect_nr revisions left to test after this"
-	git checkout -q "$bisect_rev" || exit
-	git show-branch "$bisect_rev"
+	checkout "$bisect_rev" "$bisect_nr revisions left to test after this"
 }
 
 bisect_visualize() {
@@ -414,6 +499,8 @@ bisect_clean_state() {
 	do
 		git update-ref -d $ref $hash || exit
 	done
+	rm -f "$GIT_DIR/BISECT_MERGE_BASES" &&
+	rm -f "$GIT_DIR/BISECT_ANCESTORS_OK" &&
 	rm -f "$GIT_DIR/BISECT_LOG" &&
 	rm -f "$GIT_DIR/BISECT_NAMES" &&
 	rm -f "$GIT_DIR/BISECT_RUN" &&
diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index 0626544..bf24b9a 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -350,6 +350,47 @@ test_expect_success 'bisect does not create a "bisect" branch' '
 	git branch -D bisect
 '
 
+# This creates a "side" branch to test "siblings" cases.
+test_expect_success 'side branch creation' '
+	git bisect reset &&
+	git checkout -b side $HASH4 &&
+	add_line_into_file "5(side): first line on a side branch" hello &&
+	SIDE_HASH5=$(git rev-parse --verify HEAD) &&
+	add_line_into_file "6(side): second line on a side branch" hello &&
+	SIDE_HASH6=$(git rev-parse --verify HEAD) &&
+	add_line_into_file "7(side): third line on a side branch" hello &&
+	SIDE_HASH7=$(git rev-parse --verify HEAD)
+'
+
+test_expect_success 'good merge base when good and bad are siblings' '
+	git bisect start "$HASH7" "$SIDE_HASH7" > my_bisect_log.txt &&
+	grep "merge base must be tested" my_bisect_log.txt &&
+	grep $HASH4 my_bisect_log.txt &&
+	git bisect good > my_bisect_log.txt &&
+	test_must_fail grep "merge base must be tested" my_bisect_log.txt &&
+	grep $HASH6 my_bisect_log.txt &&
+	git bisect reset
+'
+test_expect_success 'skipped merge base when good and bad are siblings' '
+	git bisect start "$SIDE_HASH7" "$HASH7" > my_bisect_log.txt &&
+	grep "merge base must be tested" my_bisect_log.txt &&
+	grep $HASH4 my_bisect_log.txt &&
+	git bisect skip > my_bisect_log.txt 2>&1 &&
+	grep "Warning" my_bisect_log.txt &&
+	grep $SIDE_HASH6 my_bisect_log.txt &&
+	git bisect reset
+'
+
+test_expect_success 'bad merge base when good and bad are siblings' '
+	git bisect start "$HASH7" HEAD > my_bisect_log.txt &&
+	grep "merge base must be tested" my_bisect_log.txt &&
+	grep $HASH4 my_bisect_log.txt &&
+	test_must_fail git bisect bad > my_bisect_log.txt 2>&1 &&
+	grep "merge base $HASH4 is bad" my_bisect_log.txt &&
+	grep "fixed between $HASH4 and $SIDE_HASH7" my_bisect_log.txt &&
+	git bisect reset
+'
+
 #
 #
 test_done
-- 
1.5.6.2.221.g6ad9a

^ permalink raw reply related

* [PATCH] cherry: cache patch-ids to avoid repeating work
From: Geoffrey Irving @ 2008-07-10  3:34 UTC (permalink / raw)
  To: Junio C Hamano, Johannes Schindelin, git@vger.kernel.org
In-Reply-To: <7vprpnlglh.fsf@gitster.siamese.dyndns.org>

Add cached-sha-map.[ch] implementing a persistent hash map from sha1 to
sha1.  The map is read with mmap, and completely rewritten if any entries
change.  It would be good to add incremental update to handle the usual case
where only a few entries change.

This structure is used by patch-ids.c to cache the mapping from commit to
patch-id into $GIT_DIR/patch-id-cache.  In the one case I've tested so far,
this speeds up the second invocation of git-cherry by two orders of
magnitude.  The caching can be disabled by setting cherry.cachepatchids to
false.

Original code cannibalized from Johannes Schindelin's notes-index structure.

Signed-off-by: Geoffrey Irving <irving@naml.us>
---

Note: there are at least two "holes" in this code.  First, it is impossible
to verify the validity of the entries (this is impossible to fix).  Second,
it is possible to write a malicious patch-id-cache file that causes git-cherry
to go into an infinite loop.  Fixing the loop requires either traversing every
entry on load (bad) or adding a second loop termination condition to
find_helper.  Since looping forever is better than returning incorrect
results, I figured fixing the weaker hole would just result in a false sense
of security.

I'll await the next round of comments. :)

 Documentation/config.txt |    5 +
 Makefile                 |    2 +
 builtin-log.c            |   12 ++
 cached-sha1-map.c        |  269 ++++++++++++++++++++++++++++++++++++++++++++++
 cached-sha1-map.h        |   45 ++++++++
 patch-ids.c              |   26 +++++-
 patch-ids.h              |    2 +
 7 files changed, 360 insertions(+), 1 deletions(-)
 create mode 100644 cached-sha1-map.c
 create mode 100644 cached-sha1-map.h

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 838794d..02b8113 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -468,6 +468,11 @@ browser.<tool>.path::
 	browse HTML help (see '-w' option in linkgit:git-help[1]) or a
 	working repository in gitweb (see linkgit:git-instaweb[1]).

+cherry.cachepatchids::
+	If true, linkgit:git-cherry will store a cache of computed patch-ids
+	in $GIT_DIR/patch-id-cache in order to make repeated invocations faster.
+	Defaults to true.
+
 clean.requireForce::
 	A boolean to make git-clean do nothing unless given -f
 	or -n.   Defaults to true.
diff --git a/Makefile b/Makefile
index 4796565..f7360e1 100644
--- a/Makefile
+++ b/Makefile
@@ -356,6 +356,7 @@ LIB_H += pack-refs.h
 LIB_H += pack-revindex.h
 LIB_H += parse-options.h
 LIB_H += patch-ids.h
+LIB_H += cached-sha1-map.h
 LIB_H += path-list.h
 LIB_H += pkt-line.h
 LIB_H += progress.h
@@ -436,6 +437,7 @@ LIB_OBJS += pager.o
 LIB_OBJS += parse-options.o
 LIB_OBJS += patch-delta.o
 LIB_OBJS += patch-ids.o
+LIB_OBJS += cached-sha1-map.o
 LIB_OBJS += path-list.o
 LIB_OBJS += path.o
 LIB_OBJS += pkt-line.o
diff --git a/builtin-log.c b/builtin-log.c
index 430d876..fbfefbd 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -1081,6 +1081,16 @@ static int add_pending_commit(const char *arg,
struct rev_info *revs, int flags)
 	return -1;
 }

+static int git_cherry_config(const char *var, const char *value, void *cb)
+{
+	if (!strcmp(var, "cherry.cachepatchids")) {
+		cache_patch_ids = git_config_bool(var, value);
+		return 0;
+	}
+
+	return 0;
+}
+
 static const char cherry_usage[] =
 "git-cherry [-v] <upstream> [<head>] [<limit>]";
 int cmd_cherry(int argc, const char **argv, const char *prefix)
@@ -1094,6 +1104,8 @@ int cmd_cherry(int argc, const char **argv,
const char *prefix)
 	const char *limit = NULL;
 	int verbose = 0;

+	git_config(git_cherry_config, NULL);
+
 	if (argc > 1 && !strcmp(argv[1], "-v")) {
 		verbose = 1;
 		argc--;
diff --git a/cached-sha1-map.c b/cached-sha1-map.c
new file mode 100644
index 0000000..3ac5474
--- /dev/null
+++ b/cached-sha1-map.c
@@ -0,0 +1,269 @@
+#include "cached-sha1-map.h"
+
+union cached_sha1_map_header {
+	struct {
+		char signature[4]; /* CS1M */
+		uint32_t version;
+		uint32_t count;
+		uint32_t size;
+	} u;
+	struct cached_sha1_entry pad; /* pad header out to 40 bytes */
+};
+
+static const char *signature = "CS1M";
+static const uint32_t version = 1;
+
+static int init_empty_map(struct cached_sha1_map *cache, uint32_t size)
+{
+	cache->count = 0;
+	cache->size = size;
+	cache->initialized = 1;
+	cache->mmapped = 0;
+	cache->dirty = 1;
+
+	cache->entries = calloc(size, sizeof(struct cached_sha1_entry));
+	if (!cache->entries) {
+		warning("failed to allocate empty map of size %"PRIu32" for %s",
+			size, git_path(cache->filename));
+		cache->size = 0;
+		cache->dirty = 0;
+		return -1;
+	}
+	return 0;
+}
+
+static int grow_map(struct cached_sha1_map *cache)
+{
+	struct cached_sha1_map new_cache;
+	uint32_t i;
+
+	if (cache->size * 2 == 0) {
+		warning("%s overflowed, so resetting to empty",
+			git_path(cache->filename));
+		return init_empty_map(cache, 64);
+	}
+
+	/* allocate cache with twice the size */
+	new_cache.filename = cache->filename;
+	if (init_empty_map(&new_cache, cache->size * 2)) {
+		warning("failed to grow %s to size %"PRIu32,
+			git_path(cache->filename), cache->size * 2);
+		return init_empty_map(cache, 64);
+	}
+
+	/* reinsert all entries */
+ 	for (i = 0; i < cache->size; i++)
+		if (!is_null_sha1(cache->entries[i].key))
+			set_cached_sha1_entry(&new_cache,
+				cache->entries[i].key, cache->entries[i].value);
+	/* finish */
+	free_cached_sha1_map(cache);
+	*cache = new_cache;
+	return 0;
+}
+
+/* Any errors that occur result in the cache being initialized to empty */
+static int init_cached_sha1_map(struct cached_sha1_map *cache)
+{
+	int fd;
+	union cached_sha1_map_header header;
+	const char *filename;
+	size_t map_size;
+
+	if (cache->initialized)
+		return cache->size ? 0 : -1;
+
+	filename = git_path(cache->filename);
+	fd = open(filename, O_RDONLY);
+	if (fd < 0) {
+		if (errno != ENOENT)
+			warning("failed to read '%s': %s", filename,
+				strerror(errno));
+		goto empty;
+	}
+
+	if (read_in_full(fd, &header, sizeof(header)) != sizeof(header))
+	{
+		warning("cannot read %s header", filename);
+		goto empty;
+	}
+
+	if (memcmp(header.u.signature, signature, 4))
+	{
+		warning("%s has invalid header", filename);
+		goto empty;
+	}
+
+	if (ntohl(header.u.version) != version)
+	{
+		warning("%s has unrecognized version %"PRIu32, filename,
+			ntohl(header.u.version));
+		goto empty;
+	}
+
+	cache->count = ntohl(header.u.count);
+	cache->size = ntohl(header.u.size);
+
+	if (cache->size & (cache->size-1))
+	{
+		warning("%s is corrupt: size %"PRIu32" is not a power of two",
+			filename, cache->size);
+		goto empty;
+	}
+
+	if (cache->count >= cache->size)
+	{
+		warning("%s is corrupt: count %"PRIu32" >= size %"PRIu32,
+			filename, cache->count, cache->size);
+		goto empty;
+	}
+
+	cache->dirty = 0;
+	cache->initialized = 1;
+	cache->mmapped = 1;
+
+	/* mmap entire file so that file / memory blocks are aligned */
+	map_size = sizeof(struct cached_sha1_entry) * (cache->size + 1);
+	cache->entries = mmap(NULL, map_size,
+		PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0);
+	if (cache->entries == MAP_FAILED) {
+		/* this is just a cache, so don't free pack memory and retry */
+		warning("%s mmap failed: %s", filename, strerror(errno));
+		goto empty;
+	}
+	cache->entries += 1; /* skip header */
+	return 0;
+
+empty:
+	if (fd >= 0)
+		close(fd);
+	return init_empty_map(cache, 64);
+}
+
+int write_cached_sha1_map(struct cached_sha1_map *cache)
+{
+	union cached_sha1_map_header header;
+	struct lock_file update_lock;
+	int fd;
+	size_t map_size;
+	const char *filename;
+
+	if (!cache->initialized || !cache->dirty)
+		return 0;
+
+	filename = git_path(cache->filename);
+	fd = hold_lock_file_for_update(&update_lock, filename, 0);
+
+	if (fd < 0)
+	{
+		warning("could not construct %s", filename);
+		return -1;
+	}
+
+	memcpy(header.u.signature, signature, 4);
+	header.u.version = htonl(version);
+	header.u.count = htonl(cache->count);
+	header.u.size = htonl(cache->size);
+	map_size = sizeof(struct cached_sha1_entry) * cache->size;
+	if (write_in_full(fd, &header, sizeof(header)) != sizeof(header)
+		|| write_in_full(fd, cache->entries, map_size) != map_size)
+	{
+		warning("could not write %s", filename);
+		return -1;
+	}
+
+	if (commit_lock_file(&update_lock) < 0)
+	{
+		warning("could not write %s", filename);
+		return -1;
+	}
+
+	cache->dirty = 0;
+	return 0;
+}
+
+void free_cached_sha1_map(struct cached_sha1_map *cache)
+{
+	if (!cache->initialized)
+		return;
+
+	if (cache->mmapped)
+		munmap(cache->entries - 1,
+			sizeof(struct cached_sha1_entry) * (cache->size + 1));
+	else
+		free(cache->entries);
+}
+
+/* The fact that size is a power of two means count-1 <= INT32_MAX, so it
+ * is safe to return signed integers here. */
+static int32_t get_hash_index(const unsigned char *sha1)
+{
+	/* this is alignment safe since 40 is a multiple of 4 */
+	return ntohl(*(uint32_t*)sha1);
+}
+
+/*
+ * Returns the index if the entry exists, and the complemented index of
+ * the next free entry otherwise.
+ */
+static int32_t find_helper(struct cached_sha1_map *cache,
+	const unsigned char *key)
+{
+	int32_t i, mask;
+
+	mask = cache->size - 1;
+
+	for (i = get_hash_index(key) & mask; ; i = (i+1) & mask) {
+		if (!hashcmp(key, cache->entries[i].key))
+			return i;
+		else if (is_null_sha1(cache->entries[i].key))
+			return ~i;
+	}
+}
+
+int get_cached_sha1_entry(struct cached_sha1_map *cache,
+	const unsigned char *key, unsigned char *value)
+{
+	int32_t i;
+
+	if (init_cached_sha1_map(cache))
+		return -1;
+
+	i = find_helper(cache, key);
+	if(i < 0)
+		return -1;
+
+	/* entry found, return value */
+	hashcpy(value, cache->entries[i].value);
+	return 0;
+}
+
+int set_cached_sha1_entry(struct cached_sha1_map *cache,
+	const unsigned char *key, const unsigned char *value)
+{
+	int32_t i;
+	struct cached_sha1_entry *entry;
+
+	if (init_cached_sha1_map(cache))
+		return -1;
+
+	i = find_helper(cache, key);
+
+	if (i < 0) { /* write new entry */
+		entry = cache->entries + ~i;
+		hashcpy(entry->key, key);
+		hashcpy(entry->value, value);
+		cache->count++;
+		cache->dirty = 1;
+	} else { /* overwrite existing entry */
+		entry = cache->entries + i;
+		if (hashcmp(value, entry->value)) {
+			hashcpy(entry->value, value);
+			cache->dirty = 1;
+		}
+	}
+
+	if (4*cache->count >= 3*cache->size)
+		return grow_map(cache);
+	return 0;
+}
diff --git a/cached-sha1-map.h b/cached-sha1-map.h
new file mode 100644
index 0000000..296c17c
--- /dev/null
+++ b/cached-sha1-map.h
@@ -0,0 +1,45 @@
+#ifndef CACHED_SHA1_MAP_H
+#define CACHED_SHA1_MAP_H
+
+#include "cache.h"
+
+/*
+ * A cached-sha1-map is a file storing a hash map from sha1 to sha1.
+ *
+ * The file is mmap'ed, updated in memory during operation, and flushed
+ * back to disk when freed.  Currently the entire file is rewritten for
+ * any change.  This could be a significant bottleneck for common uses,
+ * so it would be good to fix this later if possible.
+ *
+ * The performance of a hash map depends highly on a good hashing
+ * algorithm, to avoid collisions.  Lucky us!  SHA-1 is a pretty good
+ * hashing algorithm.
+ */
+
+struct cached_sha1_entry {
+	unsigned char key[20];
+	unsigned char value[20];
+};
+
+struct cached_sha1_map {
+	const char *filename; /* relative to GIT_DIR */
+
+	/* rest is for internal use */
+	uint32_t count, size;
+	unsigned int initialized : 1;
+	unsigned int dirty : 1;
+	unsigned int mmapped : 1;
+	struct cached_sha1_entry *entries; /* pointer to mmap'ed memory + 1 */
+};
+
+extern int get_cached_sha1_entry(struct cached_sha1_map *cache,
+	const unsigned char *key,unsigned char *value);
+
+extern int set_cached_sha1_entry(struct cached_sha1_map *cache,
+	const unsigned char *key, const unsigned char *value);
+
+extern int write_cached_sha1_map(struct cached_sha1_map *cache);
+
+extern void free_cached_sha1_map(struct cached_sha1_map *cache);
+
+#endif
diff --git a/patch-ids.c b/patch-ids.c
index 3be5d31..663ffee 100644
--- a/patch-ids.c
+++ b/patch-ids.c
@@ -2,17 +2,36 @@
 #include "diff.h"
 #include "commit.h"
 #include "patch-ids.h"
+#include "cached-sha1-map.h"
+
+int cache_patch_ids = 1;
+static struct cached_sha1_map patch_id_cache;

 static int commit_patch_id(struct commit *commit, struct diff_options *options,
 		    unsigned char *sha1)
 {
+	int ret;
+
+	/* pull patch-id out of the cache if possible */
+	patch_id_cache.filename = "patch-id-cache";
+	if (cache_patch_ids && !get_cached_sha1_entry(&patch_id_cache,
+			commit->object.sha1, sha1))
+		return 0;
+
 	if (commit->parents)
 		diff_tree_sha1(commit->parents->item->object.sha1,
 		               commit->object.sha1, "", options);
 	else
 		diff_root_tree_sha1(commit->object.sha1, "", options);
 	diffcore_std(options);
-	return diff_flush_patch_id(options, sha1);
+	ret = diff_flush_patch_id(options, sha1);
+	if (ret)
+		return ret;
+
+	/* record commit, patch-id pair in cache */
+	if (cache_patch_ids)
+		set_cached_sha1_entry(&patch_id_cache, commit->object.sha1, sha1);
+	return 0;
 }

 static uint32_t take2(const unsigned char *id)
@@ -136,6 +155,11 @@ int free_patch_ids(struct patch_ids *ids)
 		next = patches->next;
 		free(patches);
 	}
+
+	/* write cached patch-ids and ignore any errors that arise
+	 * (e.g. if the repository is write protected) */
+	if (cache_patch_ids)
+		write_cached_sha1_map(&patch_id_cache);
 	return 0;
 }

diff --git a/patch-ids.h b/patch-ids.h
index c8c7ca1..c0ebdc1 100644
--- a/patch-ids.h
+++ b/patch-ids.h
@@ -18,4 +18,6 @@ int free_patch_ids(struct patch_ids *);
 struct patch_id *add_commit_patch_id(struct commit *, struct patch_ids *);
 struct patch_id *has_commit_patch_id(struct commit *, struct patch_ids *);

+extern int cache_patch_ids;
+
 #endif /* PATCH_IDS_H */
-- 
1.5.6.2.256.g47cb9.dirty

^ permalink raw reply related

* Re: [PATCH] apply: fix copy/rename breakage
From: Junio C Hamano @ 2008-07-10  3:13 UTC (permalink / raw)
  To: git; +Cc: Linus Torvalds, Don Zickus
In-Reply-To: <7vy74aqvr1.fsf@gitster.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> Recently, 7ebd52a (Merge branch 'dz/apply-again', 2008-07-01) taught
> "git-apply" to grok a (non-git) patch that is a concatenation of separate
> patches that touch the same file number of files, by recording the

Eh, s/files/times/;

> postimage of patch application of previous round and using it as the
> preimage for later rounds.
>
> However, this "incremental" mode of patch application contradicts with the
> way git rename/copy patches are fundamentally designed....
> a patch to produce new files out of existing file by copying to fix this
> issue.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>
>  * Applies to 'master'.  I am CC'ing Linus not because he is in any way
>    responsible for this breakage, but because this breakage can affect
>    heavy users of "git apply".

^ permalink raw reply

* [PATCH] apply: fix copy/rename breakage
From: Junio C Hamano @ 2008-07-10  3:10 UTC (permalink / raw)
  To: git; +Cc: Linus Torvalds, Don Zickus

Recently, 7ebd52a (Merge branch 'dz/apply-again', 2008-07-01) taught
"git-apply" to grok a (non-git) patch that is a concatenation of separate
patches that touch the same file number of files, by recording the
postimage of patch application of previous round and using it as the
preimage for later rounds.

However, this "incremental" mode of patch application contradicts with the
way git rename/copy patches are fundamentally designed.  When a git patch
talks about a file A getting modified, and a new file B created out of B,
like this:

	diff --git a/A b/A
	--- a/A
	+++ b/A
	... change text here ...
	diff --git a/A b/B
	copy from A
	copy to B
	--- a/A
	+++ b/B
	... change text here ...

the second change to produce B does not depend on what is done to A with
the first change (this is explicitly done so for reviewability of
individual patches).

With this patch, we disable the postimage record 'fn_table' when applying
a patch to produce new files out of existing file by copying to fix this
issue.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * Applies to 'master'.  I am CC'ing Linus not because he is in any way
   responsible for this breakage, but because this breakage can affect
   heavy users of "git apply".

 builtin-apply.c |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/builtin-apply.c b/builtin-apply.c
index b3fc290..d13313f 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -2296,7 +2296,8 @@ static int apply_data(struct patch *patch, struct stat *st, struct cache_entry *
 
 	strbuf_init(&buf, 0);
 
-	if ((tpatch = in_fn_table(patch->old_name)) != NULL) {
+	if (!(patch->is_copy || patch->is_rename) &&
+	    ((tpatch = in_fn_table(patch->old_name)) != NULL)) {
 		if (tpatch == (struct patch *) -1) {
 			return error("patch %s has been renamed/deleted",
 				patch->old_name);
@@ -2375,7 +2376,7 @@ static int verify_index_match(struct cache_entry *ce, struct stat *st)
 static int check_preimage(struct patch *patch, struct cache_entry **ce, struct stat *st)
 {
 	const char *old_name = patch->old_name;
-	struct patch *tpatch;
+	struct patch *tpatch = NULL;
 	int stat_ret = 0;
 	unsigned st_mode = 0;
 
@@ -2389,7 +2390,9 @@ static int check_preimage(struct patch *patch, struct cache_entry **ce, struct s
 		return 0;
 
 	assert(patch->is_new <= 0);
-	if ((tpatch = in_fn_table(old_name)) != NULL) {
+
+	if (!(patch->is_copy || patch->is_rename) &&
+	    (tpatch = in_fn_table(old_name)) != NULL) {
 		if (tpatch == (struct patch *) -1) {
 			return error("%s: has been deleted/renamed", old_name);
 		}
@@ -2399,6 +2402,7 @@ static int check_preimage(struct patch *patch, struct cache_entry **ce, struct s
 		if (stat_ret && errno != ENOENT)
 			return error("%s: %s", old_name, strerror(errno));
 	}
+
 	if (check_index && !tpatch) {
 		int pos = cache_name_pos(old_name, strlen(old_name));
 		if (pos < 0) {
-- 
1.5.6.2.291.g7eef3

^ permalink raw reply related

* What's cooking in git.git (topics)
From: Junio C Hamano @ 2008-07-10  2:32 UTC (permalink / raw)
  To: git
In-Reply-To: <7vtzf1w0rj.fsf@gitster.siamese.dyndns.org>

Here are the topics that have been cooking.  Commits prefixed
with '-' are only in 'pu' while commits prefixed with '+' are
in 'next'.

The topics list the commits in reverse chronological order.  The topics
meant to be applied to the maintenance series have "maint-" in their
names.

It already is beginning to become clear what 1.6.0 will look like.  What's
already in 'next' all are well intentioned (I do not guarantee they are
already bug-free --- that is what cooking them in 'next' is for) and are
good set of feature enhancements.  Bigger changes will be:

 * Port for MinGW.

 * With the default Makefile settings, most of the programs will be
   installed outside your $PATH, except for "git", "gitk", "git-gui" and
   some server side programs that need to be accessible for technical
   reasons.  Invoking a git subcommand as "git-xyzzy" from the command
   line has been deprecated since early 2006 (and officially announced in
   1.5.4 release notes); use of them from your scripts after adding
   output from "git --exec-path" to the $PATH will still be supported in
   1.6.0, but users are again strongly encouraged to adjust their
   scripts to use "git xyzzy" form, as we will stop installing
   "git-xyzzy" hardlinks for built-in commands in later releases.

 * git-merge will be rewritten in C.

 * default pack and idx versions will be updated as scheduled for some
   time ago.

 * GIT_CONFIG, which was only documented as affecting "git config", but
   actually affected all git commands, now only affects "git config".
   GIT_LOCAL_CONFIG, also only documented as affecting "git config" and
   not different from GIT_CONFIG in a useful way, is removed.

----------------------------------------------------------------
[New Topics]

* xx/merge-in-c-into-next (Wed Jul 9 13:51:46 2008 -0700) 4 commits
 + Teach git-merge -X<option> again.
 + Merge branch 'jc/merge-theirs' into xx/merge-in-c-into-next
 + builtin-merge.c: use parse_options_step() "incremental parsing"
   machinery
 + Merge branch 'ph/parseopt-step-blame' into xx/merge-in-c-into-next

I've described what this is in a separate message.

* jc/branch-merged (Tue Jul 8 17:55:47 2008 -0700) 3 commits
 + branch --merged/--no-merged: allow specifying arbitrary commit
 + branch --contains: default to HEAD
 + parse-options: add PARSE_OPT_LASTARG_DEFAULT flag

This builds on top of the parse-options enhancement series that
has been cooking in 'next' for some time.

* rs/imap (Wed Jul 9 22:29:02 2008 +0100) 5 commits
 + Documentation: Improve documentation for git-imap-send(1)
 + imap-send.c: more style fixes
 + imap-send.c: style fixes
 + git-imap-send: Support SSL
 + git-imap-send: Allow the program to be run from subdirectories of
   a git tree

* om/rerere-careful (Mon Jul 7 14:42:48 2008 +0200) 1 commit
 + builtin-rerere: more carefully find conflict markers

----------------------------------------------------------------
[Will merge to master soon]

* js/pick-root (Fri Jul 4 16:19:52 2008 +0100) 1 commit
 + Allow cherry-picking root commits

* ab/bundle (Sat Jul 5 17:26:40 2008 -0400) 1 commit
 + Teach git-bundle to read revision arguments from stdin like git-
   rev-list.

* sg/stash-k-i (Tue Jul 8 00:40:56 2008 -0700) 2 commits
 + Documentation: tweak use case in "git stash save --keep-index"
 + stash: introduce 'stash save --keep-index' option

One weakness of our "partial commit" workflow support used to be that the
user can incrementally build what is to be committed in the index but that
state cannot be tested as a whole in the working tree.  This allows you to
temporarily stash the remaining changes in the working tree so that the
index state before running "stash save --keep-index" can be seen in the
working tree to be tested and then committed.

* am/stash-branch (Mon Jul 7 02:50:10 2008 +0530) 2 commits
 + Add a test for "git stash branch"
 + Implement "git stash branch <newbranch> <stash>"

Creates a new branch out of the stashed state, after returning from the
interrupt that forced you to create the stash in the first place.

* tr/add-i-e (Thu Jul 3 00:00:00 2008 +0200) 3 commits
 + git-add--interactive: manual hunk editing mode
 + git-add--interactive: remove hunk coalescing
 + git-add--interactive: replace hunk recounting with apply --recount

Adds 'e/dit' action to interactive add command.

* jc/report-tracking (Sun Jul 6 02:54:56 2008 -0700) 5 commits
 + branch -r -v: do not spit out garbage
 + stat_tracking_info(): clear object flags used during counting
 + git-branch -v: show the remote tracking statistics
 + git-status: show the remote tracking statistics
 + Refactor "tracking statistics" code used by "git checkout"

Makes the "your branch is ahead of the tracked one by N commits" logic and
messages available to other commands; status and branch are updated.

----------------------------------------------------------------
[Actively Cooking]

* jc/rebase-orig-head (Tue Jul 8 00:12:22 2008 -0400) 2 commits
 + Documentation: mention ORIG_HEAD in am, merge, and rebase
 + Teach "am" and "rebase" to mark the original position with
   ORIG_HEAD

* ph/parseopt-step-blame (Wed Jul 9 23:38:34 2008 +0200) 18 commits
 + revisions: refactor handle_revision_opt into parse_revision_opt.
 + git-shortlog: migrate to parse-options partially.
 + git-blame: fix lapsus
 + git-blame: migrate to incremental parse-option [2/2]
 + git-blame: migrate to incremental parse-option [1/2]
 + revisions: split handle_revision_opt() from setup_revisions()
 + Merge branch 'jc/blame' (early part) into HEAD
 + parse-opt: add PARSE_OPT_KEEP_ARGV0 parser option.
 + parse-opt: fake short strings for callers to believe in.
 + parse-opt: do not print errors on unknown options, return -2
   intead.
 + parse-opt: create parse_options_step.
 + parse-opt: Export a non NORETURN usage dumper.
 + parse-opt: have parse_options_{start,end}.
 + git-blame --reverse
 + builtin-blame.c: allow more than 16 parents
 + builtin-blame.c: move prepare_final() into a separate function.
 + rev-list --children
 + revision traversal: --children option

Became active again ;-) This probably is ready for 'master' already,
except for the last two which I only looked at the patch and have not
used heavily in production yet.

* jc/merge-theirs (Mon Jun 30 22:18:57 2008 -0700) 5 commits
 + Make "subtree" part more orthogonal to the rest of merge-
   recursive.
 + Teach git-pull to pass -X<option> to git-merge
 + Teach git-merge to pass -X<option> to the backend strategy module
 + git-merge-recursive-{ours,theirs}
 + git-merge-file --ours, --theirs

Punting a merge by discarding your own work in conflicting parts but still
salvaging the parts that are cleanly automerged.  It is likely that this
will result in nonsense mishmash, but somehow often people want this, so
here they are.  The interface to the backends is updated so that you can
say "git merge -Xours -Xsubtree=foo/bar/baz -s recursive other" now.

* mv/merge-in-c (Mon Jul 7 19:24:20 2008 +0200) 15 commits
 + Build in merge
 + Fix t7601-merge-pull-config.sh on AIX
 + git-commit-tree: make it usable from other builtins
 + Add new test case to ensure git-merge prepends the custom merge
   message
 + Add new test case to ensure git-merge reduces octopus parents when
   possible
 + Introduce reduce_heads()
 + Introduce get_merge_bases_many()
 + Add new test to ensure git-merge handles more than 25 refs.
 + Introduce get_octopus_merge_bases() in commit.c
 + git-fmt-merge-msg: make it usable from other builtins
 + Move read_cache_unmerged() to read-cache.c
 + Add new test to ensure git-merge handles pull.twohead and
   pull.octopus
 + Move parse-options's skip_prefix() to git-compat-util.h
 + Move commit_list_count() to commit.c
 + Move split_cmdline() to alias.c

----------------------------------------------------------------
[Graduated to "master"]

* js/apply-root (Sun Jul 6 18:36:01 2008 -0700) 3 commits
 + git-apply --directory: make --root more similar to GNU diff
 + apply --root: thinkofix.
 + Teach "git apply" to prepend a prefix with "--root=<root>"

* jc/reflog-expire (Sat Jun 28 22:24:49 2008 -0700) 2 commits
 + Make default expiration period of reflog used for stash infinite
 + Per-ref reflog expiry configuration

As 1.6.0 will be a good time to make backward incompatible changes, the
tip commit makes the default expiry period of stash 'never', unless you
configure them to expire explicitly using gc.refs/stash.* variables.
Needs consensus, but I am guessing that enough people would want stash
that does not expire.

* jk/pager-config (Thu Jul 3 07:46:57 2008 -0400) 1 commit
 + Allow per-command pager config

----------------------------------------------------------------
[On Hold]

* sg/merge-options (Sun Apr 6 03:23:47 2008 +0200) 1 commit
 + merge: remove deprecated summary and diffstat options and config
   variables

This was previously in "will be in master soon" category, but it turns out
that the synonyms to the ones this one deletes are fairly new invention
that happend in 1.5.6 timeframe, and we cannot do this just yet.  Perhaps
in 1.7.0.

* jc/dashless (Thu Jun 26 16:43:34 2008 -0700) 2 commits
 + Revert "Make clients ask for "git program" over ssh and local
   transport"
 + Make clients ask for "git program" over ssh and local transport

This is the "botched" one.  Will be resurrected during 1.7.0 or 1.8.0
timeframe.

* jk/renamelimit (Sat May 3 13:58:42 2008 -0700) 1 commit
 - diff: enable "too large a rename" warning when -M/-C is explicitly
   asked for

This would be the right thing to do for command line use, but gitk will be
hit due to tcl/tk's limitation, so I am holding this back for now.

----------------------------------------------------------------
[Stalled/Needs more work]

* sb/sequencer (Tue Jul 1 04:38:34 2008 +0200) 4 commits
 . Migrate git-am to use git-sequencer
 . Add git-sequencer test suite (t3350)
 . Add git-sequencer prototype documentation
 . Add git-sequencer shell prototype

* jc/grafts (Wed Jul 2 17:14:12 2008 -0700) 1 commit
 - [BROKEN wrt shallow clones] Ignore graft during object transfer

Cloning or fetching from a repository from grafts did not send objects
that are hidden by grafts, but the commits in the resulting repository do
need these to pass fsck.  This fixes object transfer to ignore grafts.

Another fix is needed to git-prune so that it ignores grafts but treats
commits that are mentioned in grafts as reachable.

* jc/blame (Wed Jun 4 22:58:40 2008 -0700) 7 commits
 - blame: show "previous" information in --porcelain/--incremental
   format
 - git-blame: refactor code to emit "porcelain format" output
 + git-blame --reverse
 + builtin-blame.c: allow more than 16 parents
 + builtin-blame.c: move prepare_final() into a separate function.
 + rev-list --children
 + revision traversal: --children option

The blame that finds where each line in the original lines moved to.  This
may help a GSoC project that wants to gather statistical overview of the
history.  The final presentation may need tweaking (see the log message of
the commit ""git-blame --reverse" on the series).

The tip two commits are for peeling to see what's behind the blamed
commit, which we should be able to separate out into an independent topic
from the rest.

^ permalink raw reply

* Re: git protocol specification
From: Shawn O. Pearce @ 2008-07-10  2:32 UTC (permalink / raw)
  To: Tom Werner; +Cc: Git Mailing List
In-Reply-To: <48750231.4020805@rubyisawesome.com>

Tom Werner <pubsub@rubyisawesome.com> wrote:
> I'm working on a flexible Erlang replacement for git-daemon and would  
> like to know if there is a specification available for the protocol that  
> git-upload-pack and git-receive-pack use. I've reverse engineered it but  
> have come across some points that seem inconsistent. Before I dig into  
> the C code in earnest I wanted to see if there's anything like spec  
> document for the system. Thanks!

I'm curious, are you planning on implementing git-upload-pack and
git-receive-pack in Erlang as well?  Or are you just doing the
git-daemon frontend and spawn the C git binaries as port processes?

git-receive-pack isn't too much trouble, the protocol on the server
side without the tell-me-more is fairly simple to implement, once
you have something like index-pack or unpack-objects implemented.
But that is somewhat non-trivial to do.

git-upload-pack is much more complex on the server side as you
must actually create the packfile, which means you also need to
compute the set of objects the client is missing, as well as tell
the client when you have enough "have" lines in order to compute
the smallest reasonable pack possible for the data transfer.

I know Erlang is really good at protocol handling and binary
formatting/parsing, but I don't know if you have a good git object
navigation library available to you in Erlang.  Its been years
since I last worked with it, or was involved in its community...  ;-)

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH 3/4] imap-send.c: Clean up coding style to better match the rest of the git codebase.
From: Junio C Hamano @ 2008-07-10  2:18 UTC (permalink / raw)
  To: Robert Shearman; +Cc: git
In-Reply-To: <1215638942-25010-3-git-send-email-robertshearman@gmail.com>

Robert Shearman <robertshearman@gmail.com> writes:

> -	else if (!strcmp( "ssl_verify", key ))
> -		server.ssl_verify = git_config_bool( key, val );
> +	else if (!strcmp("sslverify", key))
> +		server.ssl_verify = git_config_bool(key, val);

This is not a style fix.  Output from "size imap-send.o" before and after
a style fix patch should exactly match.

I'll fix up in the earlier one [2/4] so that this becomes a pure style
fix, so no need to resend.

^ permalink raw reply

* Re: [PATCH 2/4] git-imap-send: Add support for SSL.
From: Junio C Hamano @ 2008-07-10  2:02 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Robert Shearman, git, Robert Shearman
In-Reply-To: <alpine.LFD.1.10.0807091825500.11076@woody.linux-foundation.org>

Linus Torvalds <torvalds@linux-foundation.org> writes:

> We should be able to safely include <ctype.h> as-is.
>
> It should just happen _before_ including git-compat-util.h. That's why 
> git-compat-util.h does all those #undef's - exactly because ctype.h does 
> get included on various systems by various header files when 
> git-compat-util.h includes all those other files.

Eh, that's true,... although I am not convinced we should keep doing this.

With and without NO_OPENSSL, on top of Robert's patch, these seem to pass
compile test.

--

 git-compat-util.h |    5 +++++
 imap-send.c       |    4 ----
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/git-compat-util.h b/git-compat-util.h
index 545df59..65c4671 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -99,6 +99,11 @@
 #include <iconv.h>
 #endif
 
+#ifndef NO_OPENSSL
+#include <openssl/ssl.h>
+#include <openssl/err.h>
+#endif
+
 /* On most systems <limits.h> would have given us this, but
  * not on some systems (e.g. GNU/Hurd).
  */
diff --git a/imap-send.c b/imap-send.c
index 9dc5d08..8026334 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -25,9 +25,6 @@
 #include "cache.h"
 #ifdef NO_OPENSSL
 typedef void *SSL;
-#else
-# include <openssl/ssl.h>
-# include <openssl/err.h>
 #endif
 
 typedef struct store_conf {

^ permalink raw reply related

* Re: git protocol specification
From: Shawn O. Pearce @ 2008-07-10  1:47 UTC (permalink / raw)
  To: Tom Werner; +Cc: Johannes Schindelin, Git Mailing List
In-Reply-To: <alpine.DEB.1.00.0807092044120.4010@eeepc-johanness>

Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> On Wed, 9 Jul 2008, Tom Werner wrote:
> 
> > I'm working on a flexible Erlang replacement for git-daemon and would 
> > like to know if there is a specification available for the protocol that 
> > git-upload-pack and git-receive-pack use.
> 
> How about Documentation/technical/pack-protocol.txt?

You may also want to look at jgit's source code.  Its a cleaner
implementation of the protocol in 100% Java.  Not something you
would want to try and call directly from in Erlang, but it may
be easier to follow.

  http://repo.or.cz/w/egit.git/

Look inside of the org.spearce.jgit/src/org/spearce/jgit/transport
package.  In particular the client side of the protocol for git://
is in the TransportGitAnon class as two inner subclasses, one for
fetch ('git-upload-pack') and another for push ('git-receive-pack').

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH 2/4] git-imap-send: Add support for SSL.
From: Linus Torvalds @ 2008-07-10  1:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Robert Shearman, git, Robert Shearman
In-Reply-To: <7v8wwa5ycf.fsf@gitster.siamese.dyndns.org>



On Wed, 9 Jul 2008, Junio C Hamano wrote:
> 
> A hack like the one attached below would make it "work" but it is too
> ugly.  Probably we need to bite the bullet and rename ours not to
> collide, so that external library headers can safely include <ctype.h>.

We should be able to safely include <ctype.h> as-is.

It should just happen _before_ including git-compat-util.h. That's why 
git-compat-util.h does all those #undef's - exactly because ctype.h does 
get included on various systems by various header files when 
git-compat-util.h includes all those other files.

So the problem with Robert's patch is that it happens after "cache.h". The 
system headers should be included at the top of git-compat-util.h, or 
before it.

		Linus

^ permalink raw reply

* Re: [PATCH 2/4] git-imap-send: Add support for SSL.
From: Junio C Hamano @ 2008-07-10  1:20 UTC (permalink / raw)
  To: Robert Shearman; +Cc: git, Robert Shearman, Linus Torvalds
In-Reply-To: <1215638942-25010-2-git-send-email-robertshearman@gmail.com>

Robert Shearman <robertshearman@gmail.com> writes:

> diff --git a/imap-send.c b/imap-send.c
> index 24d76a7..26d1dba 100644
> --- a/imap-send.c
> +++ b/imap-send.c
> @@ -23,6 +23,12 @@
>   */
>  
>  #include "cache.h"
> +#ifdef NO_OPENSSL
> +typedef void *SSL;
> +#else
> +# include <openssl/ssl.h>
> +# include <openssl/err.h>
> +#endif

This unfortunately is causing compilation issues.  <openssl/ssl.h> wants
to include <ctype.h> and gets upset by seeing our isalpha() and friends
that are defined indirectly in "cache.h" expanded.

In <ctype.h> (on FC9), isCHARACTERISTC() are defined like this:

	#define __exctype(name) extern int name (int) __THROW
	...
        __exctype (isalnum);
        __exctype (isalpha);

but we have been using our own locale agonistic and signed-chars safe
macros defined in git-compat-util.h.  Including <ctype.h> breaks at the
syntax level, but more importantly if the system <ctype.h> redefines
isalpha() and friends as macro, then it would break _our_ code that expect
these macros are the sane_ctype[] based ones we have.

A hack like the one attached below would make it "work" but it is too
ugly.  Probably we need to bite the bullet and rename ours not to
collide, so that external library headers can safely include <ctype.h>.

Sigh...

diff --git a/git-compat-util.h b/git-compat-util.h
index 8c7e114..0af6406 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -302,6 +302,7 @@ static inline int has_extension(const char *filename, const char *ext)
 }
 
 /* Sane ctype - no locale, and works with signed chars */
+#define _CTYPE_H
 #undef isspace
 #undef isdigit
 #undef isalpha

^ permalink raw reply related

* [PATCH] git-submodule - register submodule URL if adding in place
From: Mark Levedahl @ 2008-07-10  1:05 UTC (permalink / raw)
  To: gitster, Johannes.Schindelin
  Cc: git, sylvain.joyeux, hjemli, pkufranky, Mark Levedahl
In-Reply-To: <1215651941-3460-1-git-send-email-mlevedahl@gmail.com>

When adding a new submodule in place, meaning the user created the
submodule as a git repo in the superproject's tree first, we don't go
through "git submodule init" to register the module.  Thus, the
submodule's origin repository URL is not stored in .git/config, and no
subsequent submodule operation will ever do so.  In this case, assume the
URL the user supplies to "submodule add" is the one that should be
registered, and do so.

Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
Dscho's issue, so far as I can tell, was not really with this patch
but is addressed by the preceding patch eliminating the
confusing single argument "submodule add" format that lead to the
error he wanted to avoid. This patch is not changed except that it
is rebased.

 git-submodule.sh |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index c2ce2fb..9228f56 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -172,6 +172,16 @@ cmd_add()
 		else
 			die "'$path' already exists and is not a valid git repo"
 		fi
+
+		case "$repo" in
+		./*|../*)
+			url=$(resolve_relative_url "$repo") || exit
+		    ;;
+		*)
+			url="$repo"
+			;;
+		esac
+		git config submodule."$path".url "$url"
 	else

 		module_clone "$path" "$realrepo" || exit
--
1.5.6.2.271.g73ad8

^ permalink raw reply related

* [PATCH] git-submodule - make "submodule add" more strict, and document it
From: Mark Levedahl @ 2008-07-10  1:05 UTC (permalink / raw)
  To: gitster, Johannes.Schindelin
  Cc: git, sylvain.joyeux, hjemli, pkufranky, Mark Levedahl
In-Reply-To: <7vwsjvlhjw.fsf@gitster.siamese.dyndns.org>

This change makes "submodule add" much more strict in the arguments it
takes, and is intended to address confusion as recently noted on the
git-list. With this change, the required syntax is:

	$ git submodule add URL path

Specifically, this eliminates the form

	$ git submodule add URL

which was confused by more than one person as

	$ git submodule add path

With this patch, the URL locating the submodule's origin repository can be
either an absolute URL, or (if it begins with ./ or ../) can express the
submodule's repository location relative to the superproject's origin.

This patch also eliminates a third form of URL, which was relative to the
superproject's top-level directory (not its repository).  Any URL that was
neither absolute nor matched ./*|../* was assumed to point to a
subdirectory of the superproject as the location of the submodule's origin
repository.  This URL form was confusing and does not seem to correspond
to an important use-case.  Specifically, no-one has identified the need to
clone from a repository already in the superproject's tree, but if this is
needed it is easily done using an absolute URL: $(pwd)/relative-path.  So,
no functionality is lost with this patch. (t6008-rev-list-submodule.sh did
rely upon this relative URL, fixed by using $(pwd).)

Following this change, there are exactly four variants of
submodule-add, as both arguments have two flavors:

URL can be absolute, or can begin with ./|../ and thus names the
submodule's origin relative to the superproject's origin.

Note: With this patch, "submodule add" discerns an absolute URL as
matching /*|*:*: e.g., URL begins with /, or it contains a :.  This works
for all valid URLs, an absolute path in POSIX, as well as an absolute path
on Windows).

path can either already exist as a valid git repo, or will be cloned from
the given URL.  The first form here eases creation of a new submodule in
an existing superproject as the submodule can be added and tested in-tree
before pushing to the public repository.  However, the more usual form is
the second, where the repo is cloned from the given URL.

This specifically addresses the issue of

	$ git submodule add a/b/c

attempting to clone from a repository at "a/b/c" to create a new module
in "c". This also simplifies description of "relative URL" as there is now
exactly *one* form: a URL relative to the parent's origin repo.

Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
This should address Junio's issues. In addition, I found one test
affected by this change and fixed that (the test actually used
the relative path URL to clone, replaced with $(pwd)).

 Documentation/git-submodule.txt |   36 +++++++++++++++++++------
 git-submodule.sh                |   55 ++++++++++++++------------------------
 t/t6008-rev-list-submodule.sh   |    2 +-
 3 files changed, 48 insertions(+), 45 deletions(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 105fc2d..1fe13a6 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -9,7 +9,7 @@ git-submodule - Initialize, update or inspect submodules
 SYNOPSIS
 --------
 [verse]
-'git submodule' [--quiet] add [-b branch] [--] <repository> [<path>]
+'git submodule' [--quiet] add [-b branch] [--] <repository> <path>
 'git submodule' [--quiet] status [--cached] [--] [<path>...]
 'git submodule' [--quiet] init [--] [<path>...]
 'git submodule' [--quiet] update [--init] [--] [<path>...]
@@ -20,14 +20,31 @@ COMMANDS
 --------
 add::
 	Add the given repository as a submodule at the given path
-	to the changeset to be committed next.  If path is a valid
-	repository within the project, it is added as is. Otherwise,
-	repository is cloned at the specified path. path is added to the
-	changeset and registered in .gitmodules.   If no path is
-	specified, the path is deduced from the repository specification.
-	If the repository url begins with ./ or ../, it is stored as
-	given but resolved as a relative path from the main project's
-	url when cloning.
+	to the changeset to be committed next to the current
+	project: the current project is termed termed the "superproject".
+
+	This requires two arguments: <repository> and <path>.
+
+	<repository> is the URL of the new submodule's origin repository.
+	This may be either an absolute URL, or (if it begins with ./
+	or ../), the location relative to the superproject's origin
+	repository.
+
+	<path> is the relative location for the cloned submodule to
+	exist in the superproject. If <path> does not exist, then the
+	submodule is created by cloning from the named URL. If <path> does
+	exist and is already a valid git repository, then this is added
+	to the changeset without cloning. This second form is provided
+	to ease creating a new submodule from scratch, and presumes
+	the user will later push the submodule to the given URL.
+
+	In either case, the given URL is recorded into .gitmodules for
+	use by subsequent users cloning the superproject. If the URL is
+	given relative to the superproject's repository, the presumption
+	is the superproject and submodule repositories will be kept
+	together in the same relative location, and only the
+	superproject's URL need be provided: git-submodule will correctly
+	locate the submodule using the relative URL in .gitmodules.

 status::
 	Show the status of the submodules. This will print the SHA-1 of the
@@ -85,6 +102,7 @@ OPTIONS
 <path>::
 	Path to submodule(s). When specified this will restrict the command
 	to only operate on the submodules found at the specified paths.
+	(This argument is required with add).

 FILES
 -----
diff --git a/git-submodule.sh b/git-submodule.sh
index 099a7d7..c2ce2fb 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -5,7 +5,7 @@
 # Copyright (c) 2007 Lars Hjemli

 USAGE="[--quiet] [--cached] \
-[add <repo> [-b branch]|status|init|update [-i|--init]|summary [-n|--summary-limit <n>] [<commit>]] \
+[add <repo> [-b branch] <path>]|[status|init|update [-i|--init]|summary [-n|--summary-limit <n>] [<commit>]] \
 [--] [<path>...]"
 OPTIONS_SPEC=
 . git-sh-setup
@@ -27,18 +27,6 @@ say()
 	fi
 }

-# NEEDSWORK: identical function exists in get_repo_base in clone.sh
-get_repo_base() {
-	(
-		cd "`/bin/pwd`" &&
-		cd "$1" || cd "$1.git" &&
-		{
-			cd .git
-			pwd
-		}
-	) 2>/dev/null
-}
-
 # Resolve relative url by appending to parent's url
 resolve_relative_url ()
 {
@@ -115,7 +103,7 @@ module_clone()
 #
 # Add a new submodule to the working tree, .gitmodules and the index
 #
-# $@ = repo [path]
+# $@ = repo path
 #
 # optional branch is stored in global branch variable
 #
@@ -150,16 +138,27 @@ cmd_add()
 	repo=$1
 	path=$2

-	if test -z "$repo"; then
+	if test -z "$repo" -o -z "$path"; then
 		usage
 	fi

-	# Guess path from repo if not specified or strip trailing slashes
-	if test -z "$path"; then
-		path=$(echo "$repo" | sed -e 's|/*$||' -e 's|:*/*\.git$||' -e 's|.*[/:]||g')
-	else
-		path=$(echo "$path" | sed -e 's|/*$||')
-	fi
+	# assure repo is absolute or relative to parent
+	case "$repo" in
+	./*|../*)
+		# dereference source url relative to parent's url
+		realrepo=$(resolve_relative_url "$repo") || exit
+		;;
+	*:*|/*)
+		# absolute url
+		realrepo=$repo
+		;;
+	*)
+		die "repo URL: '$repo' must be absolute or begin with ./|../"
+	;;
+	esac
+
+	# strip trailing slashes from path
+	path=$(echo "$path" | sed -e 's|/*$||')

 	git ls-files --error-unmatch "$path" > /dev/null 2>&1 &&
 	die "'$path' already exists in the index"
@@ -174,20 +173,6 @@ cmd_add()
 			die "'$path' already exists and is not a valid git repo"
 		fi
 	else
-		case "$repo" in
-		./*|../*)
-			# dereference source url relative to parent's url
-			realrepo=$(resolve_relative_url "$repo") || exit
-			;;
-		*)
-			# Turn the source into an absolute path if
-			# it is local
-			if base=$(get_repo_base "$repo"); then
-				repo="$base"
-			fi
-			realrepo=$repo
-			;;
-		esac

 		module_clone "$path" "$realrepo" || exit
 		(unset GIT_DIR; cd "$path" && git checkout -q ${branch:+-b "$branch" "origin/$branch"}) ||
diff --git a/t/t6008-rev-list-submodule.sh b/t/t6008-rev-list-submodule.sh
index 88e96fb..c4af9ca 100755
--- a/t/t6008-rev-list-submodule.sh
+++ b/t/t6008-rev-list-submodule.sh
@@ -23,7 +23,7 @@ test_expect_success 'setup' '

 	: > super-file &&
 	git add super-file &&
-	git submodule add . sub &&
+	git submodule add "$(pwd)" sub &&
 	git symbolic-ref HEAD refs/heads/super &&
 	test_tick &&
 	git commit -m super-initial &&
--
1.5.6.2.271.g73ad8

^ permalink raw reply related

* Re: [RFC/PATCH (WIP)] Git.pm: Add get_config() method and related subroutines
From: Jakub Narebski @ 2008-07-09 23:33 UTC (permalink / raw)
  To: Petr Baudis; +Cc: git, Lea Wiemann
In-Reply-To: <20080709160303.GL10151@machine.or.cz>

On Wed, Jul 09, 2008, Petr "Pasky" Baudis wrote:
> On Thu, Jul 03, 2008 at 06:24:53PM +0200, Jakub Narebski wrote:
> >
> > There are also a few things which I'd like some comments about:
> > 
> >  * Do config_val_to_bool and config_val_to_int should be exported
> >    by default?
> 
>   Yes with the current API, not with object API (see below). But if
> exported by default, there should be definitely a git_ prefix.

Object API for config is definitely good idea; some more reasons are
given below.

> >  * Should config_val_to_bool and config_val_to_int throw error or
> >    just return 'undef' on invalid values?  One can check if variable
> >    is defined using "exists($config_hash{'varname'})".
> 
>   I think that it's more reasonable to throw an error here (as long as
> you don't throw an error on undef argument). This particular case is
> clearly a misconfiguration by the user and you rarely need to handle
> this more gracefully, I believe.

If we follow git-config convention (and I guess we should), it would be
value of appropriate type if variable value is of appropriate type,
'undef' (no output in the case of git-config) when variable does not 
exists, and throwing error (status and "fatal: ..." message on STDERR
in the case of git-config) if variable is not of given type.

> >  * How config_val_to_bool etc. should be named? Perhaps just
> >    config_to_bool, like in gitweb?
> 
>   What about Git::Config::bool()? ;-) (See below.)
> 
> >  * Is "return wantarray ? %config : \%config;" DWIM-mery good style?
> >    I am _not_ a Perl hacker...
> 
> I maybe wouldn't be as liberal myself, but I can't tell if it's a bad
> style either.

This won't matter, I think, in the case of object API (returning 
Git::Config instead of hash or hashref).

> >  * Should ->get_config() use ->command_output_pipe, or simpler
> >    ->command() method, reading whole config into array?
> 
> You have the code written already, I'd stick with it.

This is simple rewrite of code which is in gitweb, changing open '-|'
into ->command_output_pipe, but we could just read the whole config
into array using ->command(), which would be a bit simpler.

> >  * What should ->get_config() have as an optional parameter:
> >    PREFIX (/^$prefix/o), or simply SECTION (/^(?:$section)\./o)?
> 
>   Do we even _need_ a parameter like that? I don't understand what is
> this interface trying to address.

For example if one wants to access _all_ variables in gitweb.* section
(or in gitcvs.* section), and _only_ config variables in given section.

> >  * Should we perltie hash?
> 
>   I agree with Lea that we should actually bless it. :-) Returning a
> Git::Config object provides a more flexible interface, and you can
> also do $repo->get_config()->bool('key') which is quite more elegant
> way than the val_ functions, I think.

Yes it is.

Also it does allow to use any capitalization of the section name and 
variable name (which are case insensitive), so you can write either

  $repo->get_config()->get('gitcvs.dbtablenameprefix');

but also

  $repo->get_config()->get('gitcvs.dbTableNamePrefix');

or even better, as below:

  $config = $repo->get_config();
  $config->get('gitcvs.dbTableNamePrefix');

BTW. what should non-typecasting should be named? $c->get(<VAR>), 
$c->value(<VAR>), $c->param(<VAR>), or something yet different?

> Also, having accessors for special types lets you return undef when
> the type really isn't defined, instead of e.g. true with current
> config_val_bool, which is clearly bogus and requires complicated code
> on the user side. 
 
I don't follow you.  Didn't we agree on casting an error when variable
is not of given type?

By the way, bool values are processed a bit strangely.  Value is true
if it there is no value[*1*], if it is 'true' or 'yes' case insensitive, 
or of it is integer != 0.  Value is false if it has empty value[*2*], 
if it is 'false' or 'no' case insensitive, and if it is integer of 
value '0'.  All other values are invalid (and should cause throwing an 
error).

Both current config_val_to_bool() and config_val_to_int() implementation 
are too forbidding.

> > As this is an RFC I have not checked if manpage (generated from
> > embedded POD documentation) renders correctly.
> 
>   By the way, unless you know already, you can do that trivially by
> issuing:
> 
> 	perldoc perl/Git.pm

Thanks, I didn't knew this.

> -- 
> 				Petr "Pasky" Baudis
> The last good thing written in C++ was the Pachelbel Canon. 
>                                                  -- J. Olson 

Eh? Isn't it written C# rather?

-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: [PATCH] git-send-email: Fix authenticating on some servers when using TLS.
From: Junio C Hamano @ 2008-07-09 22:51 UTC (permalink / raw)
  To: Robert Shearman; +Cc: git
In-Reply-To: <1215639580-25040-1-git-send-email-robertshearman@gmail.com>

Robert Shearman <robertshearman@gmail.com> writes:

> Send hello after a successful STARTTLS command to refresh the list of extensions. These may be different to what is returned over a clear connection (for example the AUTH command may be accepted over a secure connection, but not over a clear connection).
>
> Furthermore, this behaviour is recommended by RFC 2487 (http://www.ietf.org/rfc/rfc2487.txt).

Very true.  Thanks.

^ permalink raw reply

* [PATCH] git-send-email: Fix authenticating on some servers when using TLS.
From: Robert Shearman @ 2008-07-09 21:39 UTC (permalink / raw)
  To: git; +Cc: Robert Shearman

Send hello after a successful STARTTLS command to refresh the list of extensions. These may be different to what is returned over a clear connection (for example the AUTH command may be accepted over a secure connection, but not over a clear connection).

Furthermore, this behaviour is recommended by RFC 2487 (http://www.ietf.org/rfc/rfc2487.txt).

Signed-off-by: Robert Shearman <robertshearman@gmail.com>
---
 git-send-email.perl |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 3564419..6adb669 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -773,6 +773,9 @@ X-Mailer: git-send-email $gitversion
 					$smtp = Net::SMTP::SSL->start_SSL($smtp)
 						or die "STARTTLS failed! ".$smtp->message;
 					$smtp_encryption = '';
+					# Send EHLO again to receive fresh
+					# supported commands
+					$smtp->hello();
 				} else {
 					die "Server does not support STARTTLS! ".$smtp->message;
 				}
-- 
1.5.6.2.224.g26efb.dirty

^ permalink raw reply related

* [PATCH] git-shortlog: migrate to parse-options partially.
From: Pierre Habouzit @ 2008-07-09 21:38 UTC (permalink / raw)
  To: git; +Cc: gitster, Pierre Habouzit

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
 builtin-shortlog.c |  135 +++++++++++++++++++++++++++++----------------------
 1 files changed, 77 insertions(+), 58 deletions(-)

diff --git a/builtin-shortlog.c b/builtin-shortlog.c
index e6a2865..9107bff 100644
--- a/builtin-shortlog.c
+++ b/builtin-shortlog.c
@@ -7,9 +7,14 @@
 #include "utf8.h"
 #include "mailmap.h"
 #include "shortlog.h"
+#include "parse-options.h"
 
-static const char shortlog_usage[] =
-"git-shortlog [-n] [-s] [-e] [-w] [<commit-id>... ]";
+static char const * const shortlog_usage[] = {
+	"git-shortlog [-n] [-s] [-e] [-w] [rev-opts] [--] [<commit-id>... ]",
+	"",
+	"[rev-opts] are documented in git-rev-list(1)",
+	NULL
+};
 
 static int compare_by_number(const void *a1, const void *a2)
 {
@@ -164,21 +169,19 @@ static void get_from_rev(struct rev_info *rev, struct shortlog *log)
 		shortlog_add_commit(log, commit);
 }
 
-static int parse_uint(char const **arg, int comma)
+static int parse_uint(char const **arg, int comma, int defval)
 {
 	unsigned long ul;
 	int ret;
 	char *endp;
 
 	ul = strtoul(*arg, &endp, 10);
-	if (endp != *arg && *endp && *endp != comma)
+	if (*endp && *endp != comma)
 		return -1;
-	ret = (int) ul;
-	if (ret != ul)
+	if (ul > INT_MAX)
 		return -1;
-	*arg = endp;
-	if (**arg)
-		(*arg)++;
+	ret = *arg == endp ? defval : (int)ul;
+	*arg = *endp ? endp + 1 : endp;
 	return ret;
 }
 
@@ -187,30 +190,30 @@ static const char wrap_arg_usage[] = "-w[<width>[,<indent1>[,<indent2>]]]";
 #define DEFAULT_INDENT1 6
 #define DEFAULT_INDENT2 9
 
-static void parse_wrap_args(const char *arg, int *in1, int *in2, int *wrap)
+static int parse_wrap_args(const struct option *opt, const char *arg, int unset)
 {
-	arg += 2; /* skip -w */
-
-	*wrap = parse_uint(&arg, ',');
-	if (*wrap < 0)
-		die(wrap_arg_usage);
-	*in1 = parse_uint(&arg, ',');
-	if (*in1 < 0)
-		die(wrap_arg_usage);
-	*in2 = parse_uint(&arg, '\0');
-	if (*in2 < 0)
-		die(wrap_arg_usage);
-
-	if (!*wrap)
-		*wrap = DEFAULT_WRAPLEN;
-	if (!*in1)
-		*in1 = DEFAULT_INDENT1;
-	if (!*in2)
-		*in2 = DEFAULT_INDENT2;
-	if (*wrap &&
-	    ((*in1 && *wrap <= *in1) ||
-	     (*in2 && *wrap <= *in2)))
-		die(wrap_arg_usage);
+	struct shortlog *log = opt->value;
+
+	log->wrap_lines = !unset;
+	if (unset)
+		return 0;
+	if (!arg) {
+		log->wrap = DEFAULT_WRAPLEN;
+		log->in1 = DEFAULT_INDENT1;
+		log->in2 = DEFAULT_INDENT2;
+		return 0;
+	}
+
+	log->wrap = parse_uint(&arg, ',', DEFAULT_WRAPLEN);
+	log->in1 = parse_uint(&arg, ',', DEFAULT_INDENT1);
+	log->in2 = parse_uint(&arg, '\0', DEFAULT_INDENT2);
+	if (log->wrap < 0 || log->in1 < 0 || log->in2 < 0)
+		return error(wrap_arg_usage);
+	if (log->wrap &&
+	    ((log->in1 && log->wrap <= log->in1) ||
+	     (log->in2 && log->wrap <= log->in2)))
+		return error(wrap_arg_usage);
+	return 0;
 }
 
 void shortlog_init(struct shortlog *log)
@@ -227,38 +230,54 @@ void shortlog_init(struct shortlog *log)
 
 int cmd_shortlog(int argc, const char **argv, const char *prefix)
 {
-	struct shortlog log;
-	struct rev_info rev;
+	static struct shortlog log;
+	static struct rev_info rev;
 	int nongit;
 
+	static const struct option options[] = {
+		OPT_BOOLEAN('n', "numbered", &log.sort_by_number,
+			    "sort output according to the number of commits per author"),
+		OPT_BOOLEAN('s', "summary", &log.summary,
+			    "Suppress commit descriptions, only provides commit count"),
+		OPT_BOOLEAN('e', "email", &log.email,
+			    "Show the email address of each author"),
+		{ OPTION_CALLBACK, 'w', NULL, &log, "w[,i1[,i2]]",
+			"Linewrap output", PARSE_OPT_OPTARG, &parse_wrap_args },
+		OPT_END(),
+	};
+
+	struct parse_opt_ctx_t ctx;
+
 	prefix = setup_git_directory_gently(&nongit);
 	shortlog_init(&log);
-
-	/* since -n is a shadowed rev argument, parse our args first */
-	while (argc > 1) {
-		if (!strcmp(argv[1], "-n") || !strcmp(argv[1], "--numbered"))
-			log.sort_by_number = 1;
-		else if (!strcmp(argv[1], "-s") ||
-				!strcmp(argv[1], "--summary"))
-			log.summary = 1;
-		else if (!strcmp(argv[1], "-e") ||
-			 !strcmp(argv[1], "--email"))
-			log.email = 1;
-		else if (!prefixcmp(argv[1], "-w")) {
-			log.wrap_lines = 1;
-			parse_wrap_args(argv[1], &log.in1, &log.in2, &log.wrap);
+	init_revisions(&rev, prefix);
+	parse_options_start(&ctx, argc, argv, PARSE_OPT_KEEP_DASHDASH |
+			    PARSE_OPT_KEEP_ARGV0);
+
+	for (;;) {
+		int n;
+		switch (parse_options_step(&ctx, options, shortlog_usage)) {
+		case PARSE_OPT_HELP:
+			exit(129);
+		case PARSE_OPT_DONE:
+			goto parse_done;
 		}
-		else if (!strcmp(argv[1], "-h") || !strcmp(argv[1], "--help"))
-			usage(shortlog_usage);
-		else
-			break;
-		argv++;
-		argc--;
+		n = handle_revision_opt(&rev, ctx.argc, ctx.argv,
+					&ctx.cpidx, ctx.out);
+		if (n <= 0) {
+			error("unknown option `%s'", ctx.argv[0]);
+			usage_with_options(shortlog_usage, options);
+		}
+		ctx.argv += n;
+		ctx.argc -= n;
+	}
+parse_done:
+	argc = parse_options_end(&ctx);
+
+	if (setup_revisions(argc, argv, &rev, NULL) != 1) {
+		error("unrecognized argument: %s", argv[1]);
+		usage_with_options(shortlog_usage, options);
 	}
-	init_revisions(&rev, prefix);
-	argc = setup_revisions(argc, argv, &rev, NULL);
-	if (argc > 1)
-		die ("unrecognized argument: %s", argv[1]);
 
 	/* assume HEAD if from a tty */
 	if (!nongit && !rev.pending.nr && isatty(0))
-- 
1.5.6.2.428.gdce6.dirty

^ permalink raw reply related

* [PATCH] revisions: refactor handle_revision_opt into parse_revision_opt.
From: Pierre Habouzit @ 2008-07-09 21:38 UTC (permalink / raw)
  To: git; +Cc: gitster, Pierre Habouzit
In-Reply-To: <1215639514-1612-1-git-send-email-madcoder@debian.org>

It seems we're using handle_revision_opt the same way each time, have a
wrapper around it that does the 9-liner we copy each time instead.

handle_revision_opt can be static in the module for now, it's always
possible to make it public again if needed.

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
 builtin-blame.c    |   11 +----------
 builtin-shortlog.c |   10 +---------
 revision.c         |   18 ++++++++++++++++--
 revision.h         |    7 +++++--
 4 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/builtin-blame.c b/builtin-blame.c
index 73d26c6..06c7de4 100644
--- a/builtin-blame.c
+++ b/builtin-blame.c
@@ -2305,8 +2305,6 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 	parse_options_start(&ctx, argc, argv, PARSE_OPT_KEEP_DASHDASH |
 			    PARSE_OPT_KEEP_ARGV0);
 	for (;;) {
-		int n;
-
 		switch (parse_options_step(&ctx, options, blame_opt_usage)) {
 		case PARSE_OPT_HELP:
 			exit(129);
@@ -2320,14 +2318,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 			ctx.argv[0] = "--children";
 			reverse = 1;
 		}
-		n = handle_revision_opt(&revs, ctx.argc, ctx.argv,
-					&ctx.cpidx, ctx.out);
-		if (n <= 0) {
-			error("unknown option `%s'", ctx.argv[0]);
-			usage_with_options(blame_opt_usage, options);
-		}
-		ctx.argv += n;
-		ctx.argc -= n;
+		parse_revision_opt(&revs, &ctx, options, blame_opt_usage);
 	}
 parse_done:
 	argc = parse_options_end(&ctx);
diff --git a/builtin-shortlog.c b/builtin-shortlog.c
index 9107bff..0136202 100644
--- a/builtin-shortlog.c
+++ b/builtin-shortlog.c
@@ -255,21 +255,13 @@ int cmd_shortlog(int argc, const char **argv, const char *prefix)
 			    PARSE_OPT_KEEP_ARGV0);
 
 	for (;;) {
-		int n;
 		switch (parse_options_step(&ctx, options, shortlog_usage)) {
 		case PARSE_OPT_HELP:
 			exit(129);
 		case PARSE_OPT_DONE:
 			goto parse_done;
 		}
-		n = handle_revision_opt(&rev, ctx.argc, ctx.argv,
-					&ctx.cpidx, ctx.out);
-		if (n <= 0) {
-			error("unknown option `%s'", ctx.argv[0]);
-			usage_with_options(shortlog_usage, options);
-		}
-		ctx.argv += n;
-		ctx.argc -= n;
+		parse_revision_opt(&rev, &ctx, options, shortlog_usage);
 	}
 parse_done:
 	argc = parse_options_end(&ctx);
diff --git a/revision.c b/revision.c
index 9d5d933..bbd563e 100644
--- a/revision.c
+++ b/revision.c
@@ -974,8 +974,8 @@ static void add_ignore_packed(struct rev_info *revs, const char *name)
 	revs->ignore_packed[num] = NULL;
 }
 
-int handle_revision_opt(struct rev_info *revs, int argc, const char **argv,
-			int *unkc, const char **unkv)
+static int handle_revision_opt(struct rev_info *revs, int argc, const char **argv,
+			       int *unkc, const char **unkv)
 {
 	const char *arg = argv[0];
 
@@ -1180,6 +1180,20 @@ int handle_revision_opt(struct rev_info *revs, int argc, const char **argv,
 	return 1;
 }
 
+void parse_revision_opt(struct rev_info *revs, struct parse_opt_ctx_t *ctx,
+			const struct option *options,
+			const char * const usagestr[])
+{
+	int n = handle_revision_opt(revs, ctx->argc, ctx->argv,
+				    &ctx->cpidx, ctx->out);
+	if (n <= 0) {
+		error("unknown option `%s'", ctx->argv[0]);
+		usage_with_options(usagestr, options);
+	}
+	ctx->argv += n;
+	ctx->argc -= n;
+}
+
 /*
  * Parse revision information, filling in the "rev_info" structure,
  * and removing the used arguments from the argument list.
diff --git a/revision.h b/revision.h
index cc80fcd..fa68c65 100644
--- a/revision.h
+++ b/revision.h
@@ -1,6 +1,8 @@
 #ifndef REVISION_H
 #define REVISION_H
 
+#include "parse-options.h"
+
 #define SEEN		(1u<<0)
 #define UNINTERESTING   (1u<<1)
 #define TREESAME	(1u<<2)
@@ -122,8 +124,9 @@ volatile show_early_output_fn_t show_early_output;
 
 extern void init_revisions(struct rev_info *revs, const char *prefix);
 extern int setup_revisions(int argc, const char **argv, struct rev_info *revs, const char *def);
-extern int handle_revision_opt(struct rev_info *revs, int argc, const char **argv,
-			       int *unkc, const char **unkv);
+extern void parse_revision_opt(struct rev_info *revs, struct parse_opt_ctx_t *ctx,
+				 const struct option *options,
+				 const char * const usagestr[]);
 extern int handle_revision_arg(const char *arg, struct rev_info *revs,int flags,int cant_be_filename);
 
 extern int prepare_revision_walk(struct rev_info *revs);
-- 
1.5.6.2.428.gdce6.dirty

^ permalink raw reply related

* [PATCH 4/4] Documentation: Improve documentation for git-imap-send(1).
From: Robert Shearman @ 2008-07-09 21:29 UTC (permalink / raw)
  To: git; +Cc: Robert Shearman

Change the description to be similar to that used for git-send-email(1) to give a better description of what the tool can be used for and sound more user-friendly.

Document the configuration variables used by git-imap-send, split the example into tunnel and direct examples. Rephrase other parts of the git-imap-send documentation to use better grammar and to be clearer.

Signed-off-by: Robert Shearman <robertshearman@gmail.com>
---
 Documentation/git-imap-send.txt |   77 +++++++++++++++++++++++++++++++++------
 1 files changed, 65 insertions(+), 12 deletions(-)

diff --git a/Documentation/git-imap-send.txt b/Documentation/git-imap-send.txt
index 136c82b..f9e7288 100644
--- a/Documentation/git-imap-send.txt
+++ b/Documentation/git-imap-send.txt
@@ -3,7 +3,7 @@ git-imap-send(1)
 
 NAME
 ----
-git-imap-send - Dump a mailbox from stdin into an imap folder
+git-imap-send - Send a collection of patches from stdin to an IMAP folder
 
 
 SYNOPSIS
@@ -13,9 +13,9 @@ SYNOPSIS
 
 DESCRIPTION
 -----------
-This command uploads a mailbox generated with git-format-patch
-into an imap drafts folder.  This allows patches to be sent as
-other email is sent with mail clients that cannot read mailbox
+This command uploads a mailbox generated with git format-patch
+into an IMAP drafts folder.  This allows patches to be sent as
+other email is when using mail clients that cannot read mailbox
 files directly.
 
 Typical usage is something like:
@@ -26,21 +26,74 @@ git format-patch --signoff --stdout --attach origin | git imap-send
 CONFIGURATION
 -------------
 
-'git-imap-send' requires the following values in the repository
-configuration file (shown with examples):
+To use the tool imap.folder and either imap.tunnel or imap.host must be set
+to appropriate values.
+
+Variables
+~~~~~~~~~
+
+imap.folder::
+	The folder to drop the mails into, which is typically the Drafts
+	folder. For example: "INBOX.Drafts", "INBOX/Drafts" or
+	"[Gmail]/Drafts". Required to use imap-send.
+
+imap.tunnel::
+	Command used to setup a tunnel to the IMAP server through which
+	commands will be piped instead of using a direct network connection
+	to the server. Required when imap.host is not set to use imap-send.
+
+imap.host::
+	A URL identifying the server. Use a `imap://` prefix for non-secure
+	connections and a `imaps://` prefix for secure connections.
+	Ignored when imap.tunnel is set, but required to use imap-send
+	otherwise.
+
+imap.user::
+	The username to use when logging in to the server.
+
+imap.password::
+	The password to use when logging in to the server.
+ 
+imap.port::
+	An integer port number to connect to on the server.
+	Defaults to 143 for imap:// hosts and 993 for imaps:// hosts.
+	Ignored when imap.tunnel is set.
+
+imap.sslverify::
+	A boolean to enable/disable verification of the server certificate
+	used by the SSL/TLS connection. Default is `true`. Ignored when
+	imap.tunnel is set.
+
+Examples
+~~~~~~~~
+
+Using tunnel mode:
 
 ..........................
 [imap]
-    Folder = "INBOX.Drafts"
+    folder = "INBOX.Drafts"
+    tunnel = "ssh -q -C user@example.com /usr/bin/imapd ./Maildir 2> /dev/null"
+..........................
 
+Using direct mode:
+
+.........................
 [imap]
-    Tunnel = "ssh -q user@server.com /usr/bin/imapd ./Maildir 2> /dev/null"
+    folder = "INBOX.Drafts"
+    host = imap://imap.example.com
+    user = bob
+    pass = p4ssw0rd
+..........................
+
+Using direct mode with SSL:
 
+.........................
 [imap]
-    Host = imap://imap.example.com
-    User = bob
-    Pass = pwd
-    Port = 143
+    folder = "INBOX.Drafts"
+    host = imaps://imap.example.com
+    user = bob
+    pass = p4ssw0rd
+    port = 123
     sslverify = false
 ..........................
 
-- 
1.5.6.2.224.g26efb.dirty

^ permalink raw reply related

* [PATCH 3/4] imap-send.c: Clean up coding style to better match the rest of the git codebase.
From: Robert Shearman @ 2008-07-09 21:29 UTC (permalink / raw)
  To: git; +Cc: Robert Shearman


Signed-off-by: Robert Shearman <robertshearman@gmail.com>
---
 imap-send.c |  629 ++++++++++++++++++++++++++++-------------------------------
 1 files changed, 300 insertions(+), 329 deletions(-)

diff --git a/imap-send.c b/imap-send.c
index 26d1dba..ae8189f 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -102,14 +102,14 @@ typedef struct {
 
 static int Verbose, Quiet;
 
-static void imap_info( const char *, ... );
-static void imap_warn( const char *, ... );
+static void imap_info(const char *, ...);
+static void imap_warn(const char *, ...);
 
-static char *next_arg( char ** );
+static char *next_arg(char **);
 
-static void free_generic_messages( message_t * );
+static void free_generic_messages(message_t *);
 
-static int nfsnprintf( char *buf, int blen, const char *fmt, ... );
+static int nfsnprintf(char *buf, int blen, const char *fmt, ...);
 
 static int nfvasprintf(char **strp, const char *fmt, va_list ap)
 {
@@ -125,8 +125,8 @@ static int nfvasprintf(char **strp, const char *fmt, va_list ap)
 	return len;
 }
 
-static void arc4_init( void );
-static unsigned char arc4_getbyte( void );
+static void arc4_init(void);
+static unsigned char arc4_getbyte(void);
 
 typedef struct imap_server_conf {
 	char *name;
@@ -187,8 +187,8 @@ typedef struct imap_store {
 } imap_store_t;
 
 struct imap_cmd_cb {
-	int (*cont)( imap_store_t *ctx, struct imap_cmd *cmd, const char *prompt );
-	void (*done)( imap_store_t *ctx, struct imap_cmd *cmd, int response);
+	int (*cont)(imap_store_t *ctx, struct imap_cmd *cmd, const char *prompt);
+	void (*done)(imap_store_t *ctx, struct imap_cmd *cmd, int response);
 	void *ctx;
 	char *data;
 	int dlen;
@@ -225,7 +225,7 @@ static const char *cap_list[] = {
 #define RESP_NO    1
 #define RESP_BAD   2
 
-static int get_cmd_result( imap_store_t *ctx, struct imap_cmd *tcmd );
+static int get_cmd_result(imap_store_t *ctx, struct imap_cmd *tcmd);
 
 
 static const char *Flags[] = {
@@ -243,8 +243,7 @@ static void ssl_socket_perror(const char *func)
 }
 #endif
 
-static void
-socket_perror( const char *func, Socket_t *sock, int ret )
+static void socket_perror(const char *func, Socket_t *sock, int ret)
 {
 #ifndef NO_OPENSSL
 	if (sock->ssl) {
@@ -321,8 +320,7 @@ static int ssl_socket_connect(Socket_t *sock, int use_tls_only, int verify)
 #endif
 }
 
-static int
-socket_read( Socket_t *sock, char *buf, int len )
+static int socket_read(Socket_t *sock, char *buf, int len)
 {
 	ssize_t n;
 #ifndef NO_OPENSSL
@@ -330,17 +328,16 @@ socket_read( Socket_t *sock, char *buf, int len )
 		n = SSL_read(sock->ssl, buf, len);
 	else
 #endif
-		n = xread( sock->fd, buf, len );
+		n = xread(sock->fd, buf, len);
 	if (n <= 0) {
-		socket_perror( "read", sock, n );
-		close( sock->fd );
+		socket_perror("read", sock, n);
+		close(sock->fd);
 		sock->fd = -1;
 	}
 	return n;
 }
 
-static int
-socket_write( Socket_t *sock, const char *buf, int len )
+static int socket_write(Socket_t *sock, const char *buf, int len)
 {
 	int n;
 #ifndef NO_OPENSSL
@@ -348,10 +345,10 @@ socket_write( Socket_t *sock, const char *buf, int len )
 		n = SSL_write(sock->ssl, buf, len);
 	else
 #endif
-		n = write_in_full( sock->fd, buf, len );
+		n = write_in_full(sock->fd, buf, len);
 	if (n != len) {
-		socket_perror( "write", sock, n );
-		close( sock->fd );
+		socket_perror("write", sock, n);
+		close(sock->fd);
 		sock->fd = -1;
 	}
 	return n;
@@ -369,8 +366,7 @@ static void socket_shutdown(Socket_t *sock)
 }
 
 /* simple line buffering */
-static int
-buffer_gets( buffer_t * b, char **s )
+static int buffer_gets(buffer_t * b, char **s)
 {
 	int n;
 	int start = b->offset;
@@ -384,7 +380,7 @@ buffer_gets( buffer_t * b, char **s )
 				/* shift down used bytes */
 				*s = b->buf;
 
-				assert( start <= b->bytes );
+				assert(start <= b->bytes);
 				n = b->bytes - start;
 
 				if (n)
@@ -394,8 +390,8 @@ buffer_gets( buffer_t * b, char **s )
 				start = 0;
 			}
 
-			n = socket_read( &b->sock, b->buf + b->bytes,
-					 sizeof(b->buf) - b->bytes );
+			n = socket_read(&b->sock, b->buf + b->bytes,
+					 sizeof(b->buf) - b->bytes);
 
 			if (n <= 0)
 				return -1;
@@ -404,12 +400,12 @@ buffer_gets( buffer_t * b, char **s )
 		}
 
 		if (b->buf[b->offset] == '\r') {
-			assert( b->offset + 1 < b->bytes );
+			assert(b->offset + 1 < b->bytes);
 			if (b->buf[b->offset + 1] == '\n') {
 				b->buf[b->offset] = 0;  /* terminate the string */
 				b->offset += 2; /* next line */
 				if (Verbose)
-					puts( *s );
+					puts(*s);
 				return 0;
 			}
 		}
@@ -419,39 +415,36 @@ buffer_gets( buffer_t * b, char **s )
 	/* not reached */
 }
 
-static void
-imap_info( const char *msg, ... )
+static void imap_info(const char *msg, ...)
 {
 	va_list va;
 
 	if (!Quiet) {
-		va_start( va, msg );
-		vprintf( msg, va );
-		va_end( va );
-		fflush( stdout );
+		va_start(va, msg);
+		vprintf(msg, va);
+		va_end(va);
+		fflush(stdout);
 	}
 }
 
-static void
-imap_warn( const char *msg, ... )
+static void imap_warn(const char *msg, ...)
 {
 	va_list va;
 
 	if (Quiet < 2) {
-		va_start( va, msg );
-		vfprintf( stderr, msg, va );
-		va_end( va );
+		va_start(va, msg);
+		vfprintf(stderr, msg, va);
+		va_end(va);
 	}
 }
 
-static char *
-next_arg( char **s )
+static char *next_arg(char **s)
 {
 	char *ret;
 
 	if (!s || !*s)
 		return NULL;
-	while (isspace( (unsigned char) **s ))
+	while (isspace((unsigned char) **s))
 		(*s)++;
 	if (!**s) {
 		*s = NULL;
@@ -460,10 +453,10 @@ next_arg( char **s )
 	if (**s == '"') {
 		++*s;
 		ret = *s;
-		*s = strchr( *s, '"' );
+		*s = strchr(*s, '"');
 	} else {
 		ret = *s;
-		while (**s && !isspace( (unsigned char) **s ))
+		while (**s && !isspace((unsigned char) **s))
 			(*s)++;
 	}
 	if (*s) {
@@ -475,27 +468,25 @@ next_arg( char **s )
 	return ret;
 }
 
-static void
-free_generic_messages( message_t *msgs )
+static void free_generic_messages(message_t *msgs)
 {
 	message_t *tmsg;
 
 	for (; msgs; msgs = tmsg) {
 		tmsg = msgs->next;
-		free( msgs );
+		free(msgs);
 	}
 }
 
-static int
-nfsnprintf( char *buf, int blen, const char *fmt, ... )
+static int nfsnprintf(char *buf, int blen, const char *fmt, ...)
 {
 	int ret;
 	va_list va;
 
-	va_start( va, fmt );
-	if (blen <= 0 || (unsigned)(ret = vsnprintf( buf, blen, fmt, va )) >= (unsigned)blen)
-		die( "Fatal: buffer too small. Please report a bug.\n");
-	va_end( va );
+	va_start(va, fmt);
+	if (blen <= 0 || (unsigned)(ret = vsnprintf(buf, blen, fmt, va)) >= (unsigned)blen)
+		die("Fatal: buffer too small. Please report a bug.\n");
+	va_end(va);
 	return ret;
 }
 
@@ -503,21 +494,20 @@ static struct {
 	unsigned char i, j, s[256];
 } rs;
 
-static void
-arc4_init( void )
+static void arc4_init(void)
 {
 	int i, fd;
 	unsigned char j, si, dat[128];
 
-	if ((fd = open( "/dev/urandom", O_RDONLY )) < 0 && (fd = open( "/dev/random", O_RDONLY )) < 0) {
-		fprintf( stderr, "Fatal: no random number source available.\n" );
-		exit( 3 );
+	if ((fd = open("/dev/urandom", O_RDONLY)) < 0 && (fd = open("/dev/random", O_RDONLY)) < 0) {
+		fprintf(stderr, "Fatal: no random number source available.\n");
+		exit(3);
 	}
-	if (read_in_full( fd, dat, 128 ) != 128) {
-		fprintf( stderr, "Fatal: cannot read random number source.\n" );
-		exit( 3 );
+	if (read_in_full(fd, dat, 128) != 128) {
+		fprintf(stderr, "Fatal: cannot read random number source.\n");
+		exit(3);
 	}
-	close( fd );
+	close(fd);
 
 	for (i = 0; i < 256; i++)
 		rs.s[i] = i;
@@ -533,8 +523,7 @@ arc4_init( void )
 		arc4_getbyte();
 }
 
-static unsigned char
-arc4_getbyte( void )
+static unsigned char arc4_getbyte(void)
 {
 	unsigned char si, sj;
 
@@ -547,54 +536,54 @@ arc4_getbyte( void )
 	return rs.s[(si + sj) & 0xff];
 }
 
-static struct imap_cmd *
-v_issue_imap_cmd( imap_store_t *ctx, struct imap_cmd_cb *cb,
-		  const char *fmt, va_list ap )
+static struct imap_cmd *v_issue_imap_cmd(imap_store_t *ctx,
+					 struct imap_cmd_cb *cb,
+					 const char *fmt, va_list ap)
 {
 	imap_t *imap = ctx->imap;
 	struct imap_cmd *cmd;
 	int n, bufl;
 	char buf[1024];
 
-	cmd = xmalloc( sizeof(struct imap_cmd) );
-	nfvasprintf( &cmd->cmd, fmt, ap );
+	cmd = xmalloc(sizeof(struct imap_cmd));
+	nfvasprintf(&cmd->cmd, fmt, ap);
 	cmd->tag = ++imap->nexttag;
 
 	if (cb)
 		cmd->cb = *cb;
 	else
-		memset( &cmd->cb, 0, sizeof(cmd->cb) );
+		memset(&cmd->cb, 0, sizeof(cmd->cb));
 
 	while (imap->literal_pending)
-		get_cmd_result( ctx, NULL );
+		get_cmd_result(ctx, NULL);
 
-	bufl = nfsnprintf( buf, sizeof(buf), cmd->cb.data ? CAP(LITERALPLUS) ?
+	bufl = nfsnprintf(buf, sizeof(buf), cmd->cb.data ? CAP(LITERALPLUS) ?
 			   "%d %s{%d+}\r\n" : "%d %s{%d}\r\n" : "%d %s\r\n",
-			   cmd->tag, cmd->cmd, cmd->cb.dlen );
+			   cmd->tag, cmd->cmd, cmd->cb.dlen);
 	if (Verbose) {
 		if (imap->num_in_progress)
-			printf( "(%d in progress) ", imap->num_in_progress );
-		if (memcmp( cmd->cmd, "LOGIN", 5 ))
-			printf( ">>> %s", buf );
+			printf("(%d in progress) ", imap->num_in_progress);
+		if (memcmp(cmd->cmd, "LOGIN", 5))
+			printf(">>> %s", buf);
 		else
-			printf( ">>> %d LOGIN <user> <pass>\n", cmd->tag );
+			printf(">>> %d LOGIN <user> <pass>\n", cmd->tag);
 	}
-	if (socket_write( &imap->buf.sock, buf, bufl ) != bufl) {
-		free( cmd->cmd );
-		free( cmd );
+	if (socket_write(&imap->buf.sock, buf, bufl) != bufl) {
+		free(cmd->cmd);
+		free(cmd);
 		if (cb)
-			free( cb->data );
+			free(cb->data);
 		return NULL;
 	}
 	if (cmd->cb.data) {
 		if (CAP(LITERALPLUS)) {
-			n = socket_write( &imap->buf.sock, cmd->cb.data, cmd->cb.dlen );
-			free( cmd->cb.data );
+			n = socket_write(&imap->buf.sock, cmd->cb.data, cmd->cb.dlen);
+			free(cmd->cb.data);
 			if (n != cmd->cb.dlen ||
-			    (n = socket_write( &imap->buf.sock, "\r\n", 2 )) != 2)
+			    (n = socket_write(&imap->buf.sock, "\r\n", 2)) != 2)
 			{
-				free( cmd->cmd );
-				free( cmd );
+				free(cmd->cmd);
+				free(cmd);
 				return NULL;
 			}
 			cmd->cb.data = NULL;
@@ -609,109 +598,106 @@ v_issue_imap_cmd( imap_store_t *ctx, struct imap_cmd_cb *cb,
 	return cmd;
 }
 
-static struct imap_cmd *
-issue_imap_cmd( imap_store_t *ctx, struct imap_cmd_cb *cb, const char *fmt, ... )
+static struct imap_cmd *issue_imap_cmd(imap_store_t *ctx,
+				       struct imap_cmd_cb *cb,
+				       const char *fmt, ...)
 {
 	struct imap_cmd *ret;
 	va_list ap;
 
-	va_start( ap, fmt );
-	ret = v_issue_imap_cmd( ctx, cb, fmt, ap );
-	va_end( ap );
+	va_start(ap, fmt);
+	ret = v_issue_imap_cmd(ctx, cb, fmt, ap);
+	va_end(ap);
 	return ret;
 }
 
-static int
-imap_exec( imap_store_t *ctx, struct imap_cmd_cb *cb, const char *fmt, ... )
+static int imap_exec(imap_store_t *ctx, struct imap_cmd_cb *cb,
+		     const char *fmt, ...)
 {
 	va_list ap;
 	struct imap_cmd *cmdp;
 
-	va_start( ap, fmt );
-	cmdp = v_issue_imap_cmd( ctx, cb, fmt, ap );
-	va_end( ap );
+	va_start(ap, fmt);
+	cmdp = v_issue_imap_cmd(ctx, cb, fmt, ap);
+	va_end(ap);
 	if (!cmdp)
 		return RESP_BAD;
 
-	return get_cmd_result( ctx, cmdp );
+	return get_cmd_result(ctx, cmdp);
 }
 
-static int
-imap_exec_m( imap_store_t *ctx, struct imap_cmd_cb *cb, const char *fmt, ... )
+static int imap_exec_m(imap_store_t *ctx, struct imap_cmd_cb *cb,
+		       const char *fmt, ...)
 {
 	va_list ap;
 	struct imap_cmd *cmdp;
 
-	va_start( ap, fmt );
-	cmdp = v_issue_imap_cmd( ctx, cb, fmt, ap );
-	va_end( ap );
+	va_start(ap, fmt);
+	cmdp = v_issue_imap_cmd(ctx, cb, fmt, ap);
+	va_end(ap);
 	if (!cmdp)
 		return DRV_STORE_BAD;
 
-	switch (get_cmd_result( ctx, cmdp )) {
+	switch (get_cmd_result(ctx, cmdp)) {
 	case RESP_BAD: return DRV_STORE_BAD;
 	case RESP_NO: return DRV_MSG_BAD;
 	default: return DRV_OK;
 	}
 }
 
-static int
-is_atom( list_t *list )
+static int is_atom(list_t *list)
 {
 	return list && list->val && list->val != NIL && list->val != LIST;
 }
 
-static int
-is_list( list_t *list )
+static int is_list(list_t *list)
 {
 	return list && list->val == LIST;
 }
 
-static void
-free_list( list_t *list )
+static void free_list(list_t *list)
 {
 	list_t *tmp;
 
 	for (; list; list = tmp) {
 		tmp = list->next;
-		if (is_list( list ))
-			free_list( list->child );
-		else if (is_atom( list ))
-			free( list->val );
-		free( list );
+		if (is_list(list))
+			free_list(list->child);
+		else if (is_atom(list))
+			free(list->val);
+		free(list);
 	}
 }
 
-static int
-parse_imap_list_l( imap_t *imap, char **sp, list_t **curp, int level )
+static int parse_imap_list_l(imap_t *imap, char **sp, list_t **curp, int level)
 {
 	list_t *cur;
 	char *s = *sp, *p;
 	int n, bytes;
 
 	for (;;) {
-		while (isspace( (unsigned char)*s ))
+		while (isspace((unsigned char)*s))
 			s++;
 		if (level && *s == ')') {
 			s++;
 			break;
 		}
-		*curp = cur = xmalloc( sizeof(*cur) );
+		*curp = cur = xmalloc(sizeof(*cur));
 		curp = &cur->next;
 		cur->val = NULL; /* for clean bail */
 		if (*s == '(') {
 			/* sublist */
 			s++;
 			cur->val = LIST;
-			if (parse_imap_list_l( imap, &s, &cur->child, level + 1 ))
+			if (parse_imap_list_l(imap, &s, &cur->child, level + 1))
 				goto bail;
 		} else if (imap && *s == '{') {
 			/* literal */
-			bytes = cur->len = strtol( s + 1, &s, 10 );
+			bytes = cur->len = strtol(s + 1, &s, 10);
 			if (*s != '}')
 				goto bail;
 
-			s = cur->val = xmalloc( cur->len );
+			s = cur->val = xmalloc(cur->len);
 
 			/* dump whats left over in the input buffer */
 			n = imap->buf.bytes - imap->buf.offset;
@@ -720,7 +706,7 @@ parse_imap_list_l( imap_t *imap, char **sp, list_t **curp, int level )
 				/* the entire message fit in the buffer */
 				n = bytes;
 
-			memcpy( s, imap->buf.buf + imap->buf.offset, n );
+			memcpy(s, imap->buf.buf + imap->buf.offset, n);
 			s += n;
 			bytes -= n;
 
@@ -729,13 +715,13 @@ parse_imap_list_l( imap_t *imap, char **sp, list_t **curp, int level )
 
 			/* now read the rest of the message */
 			while (bytes > 0) {
-				if ((n = socket_read (&imap->buf.sock, s, bytes)) <= 0)
+				if ((n = socket_read(&imap->buf.sock, s, bytes)) <= 0)
 					goto bail;
 				s += n;
 				bytes -= n;
 			}
 
-			if (buffer_gets( &imap->buf, &s ))
+			if (buffer_gets(&imap->buf, &s))
 				goto bail;
 		} else if (*s == '"') {
 			/* quoted string */
@@ -750,11 +736,11 @@ parse_imap_list_l( imap_t *imap, char **sp, list_t **curp, int level )
 		} else {
 			/* atom */
 			p = s;
-			for (; *s && !isspace( (unsigned char)*s ); s++)
+			for (; *s && !isspace((unsigned char)*s); s++)
 				if (level && *s == ')')
 					break;
 			cur->len = s - p;
-			if (cur->len == 3 && !memcmp ("NIL", p, 3)) {
+			if (cur->len == 3 && !memcmp("NIL", p, 3)) {
 				cur->val = NIL;
 			} else {
 				cur->val = xmemdupz(p, cur->len);
@@ -775,39 +761,36 @@ parse_imap_list_l( imap_t *imap, char **sp, list_t **curp, int level )
 	return -1;
 }
 
-static list_t *
-parse_imap_list( imap_t *imap, char **sp )
+static list_t *parse_imap_list(imap_t *imap, char **sp)
 {
 	list_t *head;
 
-	if (!parse_imap_list_l( imap, sp, &head, 0 ))
+	if (!parse_imap_list_l(imap, sp, &head, 0))
 		return head;
-	free_list( head );
+	free_list(head);
 	return NULL;
 }
 
-static list_t *
-parse_list( char **sp )
+static list_t *parse_list(char **sp)
 {
-	return parse_imap_list( NULL, sp );
+	return parse_imap_list(NULL, sp);
 }
 
-static void
-parse_capability( imap_t *imap, char *cmd )
+static void parse_capability(imap_t *imap, char *cmd)
 {
 	char *arg;
 	unsigned i;
 
 	imap->caps = 0x80000000;
-	while ((arg = next_arg( &cmd )))
+	while ((arg = next_arg(&cmd)))
 		for (i = 0; i < ARRAY_SIZE(cap_list); i++)
-			if (!strcmp( cap_list[i], arg ))
+			if (!strcmp(cap_list[i], arg))
 				imap->caps |= 1 << i;
 	imap->rcaps = imap->caps;
 }
 
-static int
-parse_response_code( imap_store_t *ctx, struct imap_cmd_cb *cb, char *s )
+static int parse_response_code(imap_store_t *ctx, struct imap_cmd_cb *cb,
+			       char *s)
 {
 	imap_t *imap = ctx->imap;
 	char *arg, *p;
@@ -815,43 +798,42 @@ parse_response_code( imap_store_t *ctx, struct imap_cmd_cb *cb, char *s )
 	if (*s != '[')
 		return RESP_OK;		/* no response code */
 	s++;
-	if (!(p = strchr( s, ']' ))) {
-		fprintf( stderr, "IMAP error: malformed response code\n" );
+	if (!(p = strchr(s, ']'))) {
+		fprintf(stderr, "IMAP error: malformed response code\n");
 		return RESP_BAD;
 	}
 	*p++ = 0;
-	arg = next_arg( &s );
-	if (!strcmp( "UIDVALIDITY", arg )) {
-		if (!(arg = next_arg( &s )) || !(ctx->gen.uidvalidity = atoi( arg ))) {
-			fprintf( stderr, "IMAP error: malformed UIDVALIDITY status\n" );
+	arg = next_arg(&s);
+	if (!strcmp("UIDVALIDITY", arg)) {
+		if (!(arg = next_arg(&s)) || !(ctx->gen.uidvalidity = atoi(arg))) {
+			fprintf(stderr, "IMAP error: malformed UIDVALIDITY status\n");
 			return RESP_BAD;
 		}
-	} else if (!strcmp( "UIDNEXT", arg )) {
-		if (!(arg = next_arg( &s )) || !(imap->uidnext = atoi( arg ))) {
-			fprintf( stderr, "IMAP error: malformed NEXTUID status\n" );
+	} else if (!strcmp("UIDNEXT", arg)) {
+		if (!(arg = next_arg(&s)) || !(imap->uidnext = atoi(arg))) {
+			fprintf(stderr, "IMAP error: malformed NEXTUID status\n");
 			return RESP_BAD;
 		}
-	} else if (!strcmp( "CAPABILITY", arg )) {
-		parse_capability( imap, s );
-	} else if (!strcmp( "ALERT", arg )) {
+	} else if (!strcmp("CAPABILITY", arg)) {
+		parse_capability(imap, s);
+	} else if (!strcmp("ALERT", arg)) {
 		/* RFC2060 says that these messages MUST be displayed
 		 * to the user
 		 */
-		for (; isspace( (unsigned char)*p ); p++);
-		fprintf( stderr, "*** IMAP ALERT *** %s\n", p );
-	} else if (cb && cb->ctx && !strcmp( "APPENDUID", arg )) {
-		if (!(arg = next_arg( &s )) || !(ctx->gen.uidvalidity = atoi( arg )) ||
-		    !(arg = next_arg( &s )) || !(*(int *)cb->ctx = atoi( arg )))
+		for (; isspace((unsigned char)*p); p++);
+		fprintf(stderr, "*** IMAP ALERT *** %s\n", p);
+	} else if (cb && cb->ctx && !strcmp("APPENDUID", arg)) {
+		if (!(arg = next_arg(&s)) || !(ctx->gen.uidvalidity = atoi(arg)) ||
+		    !(arg = next_arg(&s)) || !(*(int *)cb->ctx = atoi(arg)))
 		{
-			fprintf( stderr, "IMAP error: malformed APPENDUID status\n" );
+			fprintf(stderr, "IMAP error: malformed APPENDUID status\n");
 			return RESP_BAD;
 		}
 	}
 	return RESP_OK;
 }
 
-static int
-get_cmd_result( imap_store_t *ctx, struct imap_cmd *tcmd )
+static int get_cmd_result(imap_store_t *ctx, struct imap_cmd *tcmd)
 {
 	imap_t *imap = ctx->imap;
 	struct imap_cmd *cmdp, **pcmdp, *ncmdp;
@@ -859,38 +841,38 @@ get_cmd_result( imap_store_t *ctx, struct imap_cmd *tcmd )
 	int n, resp, resp2, tag;
 
 	for (;;) {
-		if (buffer_gets( &imap->buf, &cmd ))
+		if (buffer_gets(&imap->buf, &cmd))
 			return RESP_BAD;
 
-		arg = next_arg( &cmd );
+		arg = next_arg(&cmd);
 		if (*arg == '*') {
-			arg = next_arg( &cmd );
+			arg = next_arg(&cmd);
 			if (!arg) {
-				fprintf( stderr, "IMAP error: unable to parse untagged response\n" );
+				fprintf(stderr, "IMAP error: unable to parse untagged response\n");
 				return RESP_BAD;
 			}
 
-			if (!strcmp( "NAMESPACE", arg )) {
-				imap->ns_personal = parse_list( &cmd );
-				imap->ns_other = parse_list( &cmd );
-				imap->ns_shared = parse_list( &cmd );
-			} else if (!strcmp( "OK", arg ) || !strcmp( "BAD", arg ) ||
-				   !strcmp( "NO", arg ) || !strcmp( "BYE", arg )) {
-				if ((resp = parse_response_code( ctx, NULL, cmd )) != RESP_OK)
+			if (!strcmp("NAMESPACE", arg)) {
+				imap->ns_personal = parse_list(&cmd);
+				imap->ns_other = parse_list(&cmd);
+				imap->ns_shared = parse_list(&cmd);
+			} else if (!strcmp("OK", arg) || !strcmp("BAD", arg) ||
+				   !strcmp("NO", arg) || !strcmp("BYE", arg)) {
+				if ((resp = parse_response_code(ctx, NULL, cmd)) != RESP_OK)
 					return resp;
-			} else if (!strcmp( "CAPABILITY", arg ))
-				parse_capability( imap, cmd );
-			else if ((arg1 = next_arg( &cmd ))) {
-				if (!strcmp( "EXISTS", arg1 ))
-					ctx->gen.count = atoi( arg );
-				else if (!strcmp( "RECENT", arg1 ))
-					ctx->gen.recent = atoi( arg );
+			} else if (!strcmp("CAPABILITY", arg))
+				parse_capability(imap, cmd);
+			else if ((arg1 = next_arg(&cmd))) {
+				if (!strcmp("EXISTS", arg1))
+					ctx->gen.count = atoi(arg);
+				else if (!strcmp("RECENT", arg1))
+					ctx->gen.recent = atoi(arg);
 			} else {
-				fprintf( stderr, "IMAP error: unable to parse untagged response\n" );
+				fprintf(stderr, "IMAP error: unable to parse untagged response\n");
 				return RESP_BAD;
 			}
 		} else if (!imap->in_progress) {
-			fprintf( stderr, "IMAP error: unexpected reply: %s %s\n", arg, cmd ? cmd : "" );
+			fprintf(stderr, "IMAP error: unexpected reply: %s %s\n", arg, cmd ? cmd : "");
 			return RESP_BAD;
 		} else if (*arg == '+') {
 			/* This can happen only with the last command underway, as
@@ -898,30 +880,30 @@ get_cmd_result( imap_store_t *ctx, struct imap_cmd *tcmd )
 			cmdp = (struct imap_cmd *)((char *)imap->in_progress_append -
 			       offsetof(struct imap_cmd, next));
 			if (cmdp->cb.data) {
-				n = socket_write( &imap->buf.sock, cmdp->cb.data, cmdp->cb.dlen );
-				free( cmdp->cb.data );
+				n = socket_write(&imap->buf.sock, cmdp->cb.data, cmdp->cb.dlen);
+				free(cmdp->cb.data);
 				cmdp->cb.data = NULL;
 				if (n != (int)cmdp->cb.dlen)
 					return RESP_BAD;
 			} else if (cmdp->cb.cont) {
-				if (cmdp->cb.cont( ctx, cmdp, cmd ))
+				if (cmdp->cb.cont(ctx, cmdp, cmd))
 					return RESP_BAD;
 			} else {
-				fprintf( stderr, "IMAP error: unexpected command continuation request\n" );
+				fprintf(stderr, "IMAP error: unexpected command continuation request\n");
 				return RESP_BAD;
 			}
-			if (socket_write( &imap->buf.sock, "\r\n", 2 ) != 2)
+			if (socket_write(&imap->buf.sock, "\r\n", 2) != 2)
 				return RESP_BAD;
 			if (!cmdp->cb.cont)
 				imap->literal_pending = 0;
 			if (!tcmd)
 				return DRV_OK;
 		} else {
-			tag = atoi( arg );
+			tag = atoi(arg);
 			for (pcmdp = &imap->in_progress; (cmdp = *pcmdp); pcmdp = &cmdp->next)
 				if (cmdp->tag == tag)
 					goto gottag;
-			fprintf( stderr, "IMAP error: unexpected tag %s\n", arg );
+			fprintf(stderr, "IMAP error: unexpected tag %s\n", arg);
 			return RESP_BAD;
 		  gottag:
 			if (!(*pcmdp = cmdp->next))
@@ -929,26 +911,26 @@ get_cmd_result( imap_store_t *ctx, struct imap_cmd *tcmd )
 			imap->num_in_progress--;
 			if (cmdp->cb.cont || cmdp->cb.data)
 				imap->literal_pending = 0;
-			arg = next_arg( &cmd );
-			if (!strcmp( "OK", arg ))
+			arg = next_arg(&cmd);
+			if (!strcmp("OK", arg))
 				resp = DRV_OK;
 			else {
-				if (!strcmp( "NO", arg )) {
-					if (cmdp->cb.create && cmd && (cmdp->cb.trycreate || !memcmp( cmd, "[TRYCREATE]", 11 ))) { /* SELECT, APPEND or UID COPY */
-						p = strchr( cmdp->cmd, '"' );
-						if (!issue_imap_cmd( ctx, NULL, "CREATE \"%.*s\"", strchr( p + 1, '"' ) - p + 1, p )) {
+				if (!strcmp("NO", arg)) {
+					if (cmdp->cb.create && cmd && (cmdp->cb.trycreate || !memcmp(cmd, "[TRYCREATE]", 11))) { /* SELECT, APPEND or UID COPY */
+						p = strchr(cmdp->cmd, '"');
+						if (!issue_imap_cmd(ctx, NULL, "CREATE \"%.*s\"", strchr(p + 1, '"') - p + 1, p)) {
 							resp = RESP_BAD;
 							goto normal;
 						}
 						/* not waiting here violates the spec, but a server that does not
 						   grok this nonetheless violates it too. */
 						cmdp->cb.create = 0;
-						if (!(ncmdp = issue_imap_cmd( ctx, &cmdp->cb, "%s", cmdp->cmd ))) {
+						if (!(ncmdp = issue_imap_cmd(ctx, &cmdp->cb, "%s", cmdp->cmd))) {
 							resp = RESP_BAD;
 							goto normal;
 						}
-						free( cmdp->cmd );
-						free( cmdp );
+						free(cmdp->cmd);
+						free(cmdp);
 						if (!tcmd)
 							return 0;	/* ignored */
 						if (cmdp == tcmd)
@@ -956,21 +938,21 @@ get_cmd_result( imap_store_t *ctx, struct imap_cmd *tcmd )
 						continue;
 					}
 					resp = RESP_NO;
-				} else /*if (!strcmp( "BAD", arg ))*/
+				} else /*if (!strcmp("BAD", arg))*/
 					resp = RESP_BAD;
-				fprintf( stderr, "IMAP command '%s' returned response (%s) - %s\n",
-					 memcmp (cmdp->cmd, "LOGIN", 5) ?
+				fprintf(stderr, "IMAP command '%s' returned response (%s) - %s\n",
+					 memcmp(cmdp->cmd, "LOGIN", 5) ?
 							cmdp->cmd : "LOGIN <user> <pass>",
 							arg, cmd ? cmd : "");
 			}
-			if ((resp2 = parse_response_code( ctx, &cmdp->cb, cmd )) > resp)
+			if ((resp2 = parse_response_code(ctx, &cmdp->cb, cmd)) > resp)
 				resp = resp2;
 		  normal:
 			if (cmdp->cb.done)
-				cmdp->cb.done( ctx, cmdp, resp );
-			free( cmdp->cb.data );
-			free( cmdp->cmd );
-			free( cmdp );
+				cmdp->cb.done(ctx, cmdp, resp);
+			free(cmdp->cb.data);
+			free(cmdp->cmd);
+			free(cmdp);
 			if (!tcmd || tcmd == cmdp)
 				return resp;
 		}
@@ -978,31 +960,28 @@ get_cmd_result( imap_store_t *ctx, struct imap_cmd *tcmd )
 	/* not reached */
 }
 
-static void
-imap_close_server( imap_store_t *ictx )
+static void imap_close_server(imap_store_t *ictx)
 {
 	imap_t *imap = ictx->imap;
 
 	if (imap->buf.sock.fd != -1) {
-		imap_exec( ictx, NULL, "LOGOUT" );
-		socket_shutdown( &imap->buf.sock );
+		imap_exec(ictx, NULL, "LOGOUT");
+		socket_shutdown(&imap->buf.sock);
 	}
-	free_list( imap->ns_personal );
-	free_list( imap->ns_other );
-	free_list( imap->ns_shared );
-	free( imap );
+	free_list(imap->ns_personal);
+	free_list(imap->ns_other);
+	free_list(imap->ns_shared);
+	free(imap);
 }
 
-static void
-imap_close_store( store_t *ctx )
+static void imap_close_store(store_t *ctx)
 {
-	imap_close_server( (imap_store_t *)ctx );
-	free_generic_messages( ctx->msgs );
-	free( ctx );
+	imap_close_server((imap_store_t *)ctx);
+	free_generic_messages(ctx->msgs);
+	free(ctx);
 }
 
-static store_t *
-imap_open_store( imap_server_conf_t *srvc )
+static store_t *imap_open_store(imap_server_conf_t *srvc)
 {
 	imap_store_t *ctx;
 	imap_t *imap;
@@ -1012,9 +991,9 @@ imap_open_store( imap_server_conf_t *srvc )
 	int s, a[2], preauth;
 	pid_t pid;
 
-	ctx = xcalloc( sizeof(*ctx), 1 );
+	ctx = xcalloc(sizeof(*ctx), 1);
 
-	ctx->imap = imap = xcalloc( sizeof(*imap), 1 );
+	ctx->imap = imap = xcalloc(sizeof(*imap), 1);
 	imap->buf.sock.fd = -1;
 	imap->buf.sock.ssl = NULL;
 	imap->in_progress_append = &imap->in_progress;
@@ -1022,51 +1001,51 @@ imap_open_store( imap_server_conf_t *srvc )
 	/* open connection to IMAP server */
 
 	if (srvc->tunnel) {
-		imap_info( "Starting tunnel '%s'... ", srvc->tunnel );
+		imap_info("Starting tunnel '%s'... ", srvc->tunnel);
 
-		if (socketpair( PF_UNIX, SOCK_STREAM, 0, a )) {
-			perror( "socketpair" );
-			exit( 1 );
+		if (socketpair(PF_UNIX, SOCK_STREAM, 0, a)) {
+			perror("socketpair");
+			exit(1);
 		}
 
 		pid = fork();
 		if (pid < 0)
-			_exit( 127 );
+			_exit(127);
 		if (!pid) {
-			if (dup2( a[0], 0 ) == -1 || dup2( a[0], 1 ) == -1)
-				_exit( 127 );
-			close( a[0] );
-			close( a[1] );
-			execl( "/bin/sh", "sh", "-c", srvc->tunnel, NULL );
-			_exit( 127 );
+			if (dup2(a[0], 0) == -1 || dup2(a[0], 1) == -1)
+				_exit(127);
+			close(a[0]);
+			close(a[1]);
+			execl("/bin/sh", "sh", "-c", srvc->tunnel, NULL);
+			_exit(127);
 		}
 
-		close (a[0]);
+		close(a[0]);
 
 		imap->buf.sock.fd = a[1];
 
-		imap_info( "ok\n" );
+		imap_info("ok\n");
 	} else {
-		memset( &addr, 0, sizeof(addr) );
-		addr.sin_port = htons( srvc->port );
+		memset(&addr, 0, sizeof(addr));
+		addr.sin_port = htons(srvc->port);
 		addr.sin_family = AF_INET;
 
-		imap_info( "Resolving %s... ", srvc->host );
-		he = gethostbyname( srvc->host );
+		imap_info("Resolving %s... ", srvc->host);
+		he = gethostbyname(srvc->host);
 		if (!he) {
-			perror( "gethostbyname" );
+			perror("gethostbyname");
 			goto bail;
 		}
-		imap_info( "ok\n" );
+		imap_info("ok\n");
 
 		addr.sin_addr.s_addr = *((int *) he->h_addr_list[0]);
 
-		s = socket( PF_INET, SOCK_STREAM, 0 );
+		s = socket(PF_INET, SOCK_STREAM, 0);
 
-		imap_info( "Connecting to %s:%hu... ", inet_ntoa( addr.sin_addr ), ntohs( addr.sin_port ) );
-		if (connect( s, (struct sockaddr *)&addr, sizeof(addr) )) {
-			close( s );
-			perror( "connect" );
+		imap_info("Connecting to %s:%hu... ", inet_ntoa(addr.sin_addr), ntohs(addr.sin_port));
+		if (connect(s, (struct sockaddr *)&addr, sizeof(addr))) {
+			close(s);
+			perror("connect");
 			goto bail;
 		}
 		
@@ -1077,28 +1056,28 @@ imap_open_store( imap_server_conf_t *srvc )
 			close(s);
 			goto bail;
 		}
-		imap_info( "ok\n" );
+		imap_info("ok\n");
 	}
 
 	/* read the greeting string */
-	if (buffer_gets( &imap->buf, &rsp )) {
-		fprintf( stderr, "IMAP error: no greeting response\n" );
+	if (buffer_gets(&imap->buf, &rsp)) {
+		fprintf(stderr, "IMAP error: no greeting response\n");
 		goto bail;
 	}
-	arg = next_arg( &rsp );
-	if (!arg || *arg != '*' || (arg = next_arg( &rsp )) == NULL) {
-		fprintf( stderr, "IMAP error: invalid greeting response\n" );
+	arg = next_arg(&rsp);
+	if (!arg || *arg != '*' || (arg = next_arg(&rsp)) == NULL) {
+		fprintf(stderr, "IMAP error: invalid greeting response\n");
 		goto bail;
 	}
 	preauth = 0;
-	if (!strcmp( "PREAUTH", arg ))
+	if (!strcmp("PREAUTH", arg))
 		preauth = 1;
-	else if (strcmp( "OK", arg ) != 0) {
-		fprintf( stderr, "IMAP error: unknown greeting response\n" );
+	else if (strcmp("OK", arg) != 0) {
+		fprintf(stderr, "IMAP error: unknown greeting response\n");
 		goto bail;
 	}
-	parse_response_code( ctx, NULL, rsp );
-	if (!imap->caps && imap_exec( ctx, NULL, "CAPABILITY" ) != RESP_OK)
+	parse_response_code(ctx, NULL, rsp);
+	if (!imap->caps && imap_exec(ctx, NULL, "CAPABILITY") != RESP_OK)
 		goto bail;
 
 	if (!preauth) {
@@ -1114,38 +1093,38 @@ imap_open_store( imap_server_conf_t *srvc )
 				goto bail;
 		}
 #endif
-		imap_info ("Logging in...\n");
+		imap_info("Logging in...\n");
 		if (!srvc->user) {
-			fprintf( stderr, "Skipping server %s, no user\n", srvc->host );
+			fprintf(stderr, "Skipping server %s, no user\n", srvc->host);
 			goto bail;
 		}
 		if (!srvc->pass) {
 			char prompt[80];
-			sprintf( prompt, "Password (%s@%s): ", srvc->user, srvc->host );
-			arg = getpass( prompt );
+			sprintf(prompt, "Password (%s@%s): ", srvc->user, srvc->host);
+			arg = getpass(prompt);
 			if (!arg) {
-				perror( "getpass" );
-				exit( 1 );
+				perror("getpass");
+				exit(1);
 			}
 			if (!*arg) {
-				fprintf( stderr, "Skipping account %s@%s, no password\n", srvc->user, srvc->host );
+				fprintf(stderr, "Skipping account %s@%s, no password\n", srvc->user, srvc->host);
 				goto bail;
 			}
 			/*
 			 * getpass() returns a pointer to a static buffer.  make a copy
 			 * for long term storage.
 			 */
-			srvc->pass = xstrdup( arg );
+			srvc->pass = xstrdup(arg);
 		}
 		if (CAP(NOLOGIN)) {
-			fprintf( stderr, "Skipping account %s@%s, server forbids LOGIN\n", srvc->user, srvc->host );
+			fprintf(stderr, "Skipping account %s@%s, server forbids LOGIN\n", srvc->user, srvc->host);
 			goto bail;
 		}
 		if (!imap->buf.sock.ssl)
 			imap_warn( "*** IMAP Warning *** Password is being "
 				   "sent in the clear\n" );
-		if (imap_exec( ctx, NULL, "LOGIN \"%s\" \"%s\"", srvc->user, srvc->pass ) != RESP_OK) {
-			fprintf( stderr, "IMAP error: LOGIN failed\n" );
+		if (imap_exec(ctx, NULL, "LOGIN \"%s\" \"%s\"", srvc->user, srvc->pass) != RESP_OK) {
+			fprintf(stderr, "IMAP error: LOGIN failed\n");
 			goto bail;
 		}
 	} /* !preauth */
@@ -1155,12 +1134,11 @@ imap_open_store( imap_server_conf_t *srvc )
 	return (store_t *)ctx;
 
   bail:
-	imap_close_store( &ctx->gen );
+	imap_close_store(&ctx->gen);
 	return NULL;
 }
 
-static int
-imap_make_flags( int flags, char *buf )
+static int imap_make_flags(int flags, char *buf)
 {
 	const char *s;
 	unsigned i, d;
@@ -1179,8 +1157,7 @@ imap_make_flags( int flags, char *buf )
 
 #define TUIDL 8
 
-static int
-imap_store_msg( store_t *gctx, msg_data_t *data, int *uid )
+static int imap_store_msg(store_t *gctx, msg_data_t *data, int *uid)
 {
 	imap_store_t *ctx = (imap_store_t *)gctx;
 	imap_t *imap = ctx->imap;
@@ -1191,7 +1168,7 @@ imap_store_msg( store_t *gctx, msg_data_t *data, int *uid )
 	int start, sbreak = 0, ebreak = 0;
 	char flagstr[128], tuid[TUIDL * 2 + 1];
 
-	memset( &cb, 0, sizeof(cb) );
+	memset(&cb, 0, sizeof(cb));
 
 	fmap = data->data;
 	len = data->len;
@@ -1207,18 +1184,18 @@ imap_store_msg( store_t *gctx, msg_data_t *data, int *uid )
 					sbreak = ebreak = i - 2 + nocr;
 					goto mktid;
 				}
-				if (!memcmp( fmap + start, "X-TUID: ", 8 )) {
+				if (!memcmp(fmap + start, "X-TUID: ", 8)) {
 					extra -= (ebreak = i) - (sbreak = start) + nocr;
 					goto mktid;
 				}
 				goto nloop;
 			}
 		/* invalid message */
-		free( fmap );
+		free(fmap);
 		return DRV_MSG_BAD;
 	 mktid:
 		for (j = 0; j < TUIDL; j++)
-			sprintf( tuid + j * 2, "%02x", arc4_getbyte() );
+			sprintf(tuid + j * 2, "%02x", arc4_getbyte());
 		extra += 8 + TUIDL * 2 + 2;
 	}
 	if (nocr)
@@ -1227,7 +1204,7 @@ imap_store_msg( store_t *gctx, msg_data_t *data, int *uid )
 				extra++;
 
 	cb.dlen = len + extra;
-	buf = cb.data = xmalloc( cb.dlen );
+	buf = cb.data = xmalloc(cb.dlen);
 	i = 0;
 	if (!CAP(UIDPLUS) && uid) {
 		if (nocr) {
@@ -1238,12 +1215,12 @@ imap_store_msg( store_t *gctx, msg_data_t *data, int *uid )
 				} else
 					*buf++ = fmap[i];
 		} else {
-			memcpy( buf, fmap, sbreak );
+			memcpy(buf, fmap, sbreak);
 			buf += sbreak;
 		}
-		memcpy( buf, "X-TUID: ", 8 );
+		memcpy(buf, "X-TUID: ", 8);
 		buf += 8;
-		memcpy( buf, tuid, TUIDL * 2 );
+		memcpy(buf, tuid, TUIDL * 2);
 		buf += TUIDL * 2;
 		*buf++ = '\r';
 		*buf++ = '\n';
@@ -1257,13 +1234,13 @@ imap_store_msg( store_t *gctx, msg_data_t *data, int *uid )
 			} else
 				*buf++ = fmap[i];
 	} else
-		memcpy( buf, fmap + i, len - i );
+		memcpy(buf, fmap + i, len - i);
 
-	free( fmap );
+	free(fmap);
 
 	d = 0;
 	if (data->flags) {
-		d = imap_make_flags( data->flags, flagstr );
+		d = imap_make_flags(data->flags, flagstr);
 		flagstr[d++] = ' ';
 	}
 	flagstr[d] = 0;
@@ -1276,11 +1253,11 @@ imap_store_msg( store_t *gctx, msg_data_t *data, int *uid )
 			imap->caps = imap->rcaps & ~(1 << LITERALPLUS);
 	} else {
 		box = gctx->name;
-		prefix = !strcmp( box, "INBOX" ) ? "" : ctx->prefix;
+		prefix = !strcmp(box, "INBOX") ? "" : ctx->prefix;
 		cb.create = 0;
 	}
 	cb.ctx = uid;
-	ret = imap_exec_m( ctx, &cb, "APPEND \"%s%s\" %s", prefix, box, flagstr );
+	ret = imap_exec_m(ctx, &cb, "APPEND \"%s%s\" %s", prefix, box, flagstr);
 	imap->caps = imap->rcaps;
 	if (ret != DRV_OK)
 		return ret;
@@ -1294,8 +1271,7 @@ imap_store_msg( store_t *gctx, msg_data_t *data, int *uid )
 
 #define CHUNKSIZE 0x1000
 
-static int
-read_message( FILE *f, msg_data_t *msg )
+static int read_message(FILE *f, msg_data_t *msg)
 {
 	struct strbuf buf;
 
@@ -1312,8 +1288,7 @@ read_message( FILE *f, msg_data_t *msg )
 	return msg->len;
 }
 
-static int
-count_messages( msg_data_t *msg )
+static int count_messages(msg_data_t *msg)
 {
 	int count = 0;
 	char *p = msg->data;
@@ -1323,7 +1298,7 @@ count_messages( msg_data_t *msg )
 			count++;
 			p += 5;
 		}
-		p = strstr( p+5, "\nFrom ");
+		p = strstr(p+5, "\nFrom ");
 		if (!p)
 			break;
 		p++;
@@ -1331,22 +1306,21 @@ count_messages( msg_data_t *msg )
 	return count;
 }
 
-static int
-split_msg( msg_data_t *all_msgs, msg_data_t *msg, int *ofs )
+static int split_msg(msg_data_t *all_msgs, msg_data_t *msg, int *ofs)
 {
 	char *p, *data;
 
-	memset( msg, 0, sizeof *msg );
+	memset(msg, 0, sizeof *msg);
 	if (*ofs >= all_msgs->len)
 		return 0;
 
-	data = &all_msgs->data[ *ofs ];
+	data = &all_msgs->data[*ofs];
 	msg->len = all_msgs->len - *ofs;
 
 	if (msg->len < 5 || prefixcmp(data, "From "))
 		return 0;
 
-	p = strchr( data, '\n' );
+	p = strchr(data, '\n');
 	if (p) {
 		p = &p[1];
 		msg->len -= p-data;
@@ -1354,7 +1328,7 @@ split_msg( msg_data_t *all_msgs, msg_data_t *msg, int *ofs )
 		data = p;
 	}
 
-	p = strstr( data, "\nFrom " );
+	p = strstr(data, "\nFrom ");
 	if (p)
 		msg->len = &p[1] - data;
 
@@ -1363,8 +1337,7 @@ split_msg( msg_data_t *all_msgs, msg_data_t *msg, int *ofs )
 	return 1;
 }
 
-static imap_server_conf_t server =
-{
+static imap_server_conf_t server = {
 	NULL,	/* name */
 	NULL,	/* tunnel */
 	NULL,	/* host */
@@ -1377,12 +1350,11 @@ static imap_server_conf_t server =
 
 static char *imap_folder;
 
-static int
-git_imap_config(const char *key, const char *val, void *cb)
+static int git_imap_config(const char *key, const char *val, void *cb)
 {
 	char imap_key[] = "imap.";
 
-	if (strncmp( key, imap_key, sizeof imap_key - 1 ))
+	if (strncmp(key, imap_key, sizeof imap_key - 1))
 		return 0;
 
 	if (!val)
@@ -1390,9 +1362,9 @@ git_imap_config(const char *key, const char *val, void *cb)
 
 	key += sizeof imap_key - 1;
 
-	if (!strcmp( "folder", key )) {
-		imap_folder = xstrdup( val );
-	} else if (!strcmp( "host", key )) {
+	if (!strcmp("folder", key)) {
+		imap_folder = xstrdup(val);
+	} else if (!strcmp("host", key)) {
 		if (!prefixcmp(val, "imap:"))
 			val += 5;
 		else if (!prefixcmp(val, "imaps:")) {
@@ -1401,23 +1373,22 @@ git_imap_config(const char *key, const char *val, void *cb)
 		}
 		if (!prefixcmp(val, "//"))
 			val += 2;
-		server.host = xstrdup( val );
+		server.host = xstrdup(val);
 	}
-	else if (!strcmp( "user", key ))
-		server.user = xstrdup( val );
-	else if (!strcmp( "pass", key ))
-		server.pass = xstrdup( val );
-	else if (!strcmp( "port", key ))
-		server.port = git_config_int( key, val );
-	else if (!strcmp( "tunnel", key ))
-		server.tunnel = xstrdup( val );
-	else if (!strcmp( "ssl_verify", key ))
-		server.ssl_verify = git_config_bool( key, val );
+	else if (!strcmp("user", key))
+		server.user = xstrdup(val);
+	else if (!strcmp("pass", key))
+		server.pass = xstrdup(val);
+	else if (!strcmp("port", key))
+		server.port = git_config_int(key, val);
+	else if (!strcmp("tunnel", key))
+		server.tunnel = xstrdup(val);
+	else if (!strcmp("sslverify", key))
+		server.ssl_verify = git_config_bool(key, val);
 	return 0;
 }
 
-int
-main(int argc, char **argv)
+int main(int argc, char **argv)
 {
 	msg_data_t all_msgs, msg;
 	store_t *ctx = NULL;
@@ -1437,50 +1408,50 @@ main(int argc, char **argv)
 		server.port = server.use_ssl ? 993 : 143;
 
 	if (!imap_folder) {
-		fprintf( stderr, "no imap store specified\n" );
+		fprintf(stderr, "no imap store specified\n");
 		return 1;
 	}
 	if (!server.host) {
 		if (!server.tunnel) {
-			fprintf( stderr, "no imap host specified\n" );
+			fprintf(stderr, "no imap host specified\n");
 			return 1;
 		}
 		server.host = "tunnel";
 	}
 
 	/* read the messages */
-	if (!read_message( stdin, &all_msgs )) {
+	if (!read_message(stdin, &all_msgs)) {
 		fprintf(stderr,"nothing to send\n");
 		return 1;
 	}
 
-	total = count_messages( &all_msgs );
+	total = count_messages(&all_msgs);
 	if (!total) {
 		fprintf(stderr,"no messages to send\n");
 		return 1;
 	}
 
 	/* write it to the imap server */
-	ctx = imap_open_store( &server );
+	ctx = imap_open_store(&server);
 	if (!ctx) {
-		fprintf( stderr,"failed to open store\n");
+		fprintf(stderr,"failed to open store\n");
 		return 1;
 	}
 
-	fprintf( stderr, "sending %d message%s\n", total, (total!=1)?"s":"" );
+	fprintf(stderr, "sending %d message%s\n", total, (total!=1)?"s":"");
 	ctx->name = imap_folder;
 	while (1) {
 		unsigned percent = n * 100 / total;
-		fprintf( stderr, "%4u%% (%d/%d) done\r", percent, n, total );
-		if (!split_msg( &all_msgs, &msg, &ofs ))
+		fprintf(stderr, "%4u%% (%d/%d) done\r", percent, n, total);
+		if (!split_msg(&all_msgs, &msg, &ofs))
 			break;
-		r = imap_store_msg( ctx, &msg, &uid );
+		r = imap_store_msg(ctx, &msg, &uid);
 		if (r != DRV_OK) break;
 		n++;
 	}
-	fprintf( stderr,"\n" );
+	fprintf(stderr, "\n");
 
-	imap_close_store( ctx );
+	imap_close_store(ctx);
 
 	return 0;
 }
-- 
1.5.6.2.224.g26efb.dirty

^ permalink raw reply related


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