Git development
 help / color / mirror / Atom feed
* Re: [PATCH] Take care of errors reported from the server when upload command is started
From: Shawn O. Pearce @ 2008-06-22 23:01 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git, Marek Zawirski
In-Reply-To: <1214156797-29186-1-git-send-email-robin.rosenberg@dewire.com>

Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
> @@ -44,4 +46,31 @@ class EclipseSshSessionFactory extends SshSessionFactory {
> +	@Override
> +	public OutputStream getErrorStream() {
...
> +					Activator.logError(s, new Throwable());

I'm not sure what value the Throwable gives us here; it may be some
call stack deep within JSch, isn't it?  Is it useful to include?

We may also want the log records in Eclipse to say which remote the
message came from, which means passing in the URIish as a parameter.

> @@ -77,6 +78,7 @@ class TransportGitSsh extends PackTransport {
>  	}
>  
>  	final SshSessionFactory sch;
> +	OutputStream errStream;

Could we avoid putting the error stream as an instance member
of the transport by instead using channel.getErrStream() in the
exception case below?
  
> @@ -179,7 +181,8 @@ class TransportGitSsh extends PackTransport {
>  			cmd.append(' ');
>  			sqAlways(cmd, path);
>  			channel.setCommand(cmd.toString());
> -			channel.setErrStream(System.err);
> +			errStream = SshSessionFactory.getInstance().getErrorStream();

Use sch rather than SshSessionFactory.getInstance().  We store it in the
transport so that once the transport instance is created it always goes
to the same SshSessionFactory for anything it needs, even if the caller
has changed the global SshSessionFactory away on us.

> @@ -198,7 +201,12 @@ class TransportGitSsh extends PackTransport {
>  			try {
>  				session = openSession();
>  				channel = exec(session, getOptionUploadPack());
> -				init(channel.getInputStream(), channel.getOutputStream());
> +
> +				if (channel.isConnected())
> +					init(channel.getInputStream(), channel.getOutputStream());
> +				else
> +					throw new TransportException(errStream.toString());

I think you can say channel.getErrStream() here and not need the
instance member.

-- 
Shawn.

^ permalink raw reply

* Re: Shrink the git binary a bit by avoiding unnecessary inline functions
From: Linus Torvalds @ 2008-06-22 22:40 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <vpqej6p9ko2.fsf@bauges.imag.fr>



On Mon, 23 Jun 2008, Matthieu Moy wrote:
> 
> Wouldn't it be better to split that kind of function into two parts:
> 
> - inline the short common case
> - put the special case apart

It's almost certainly not worth it. These things really aren't big 
optimization wins, and the *big* wins from doing inlines are if it allows 
the call-site to do more optimizations (eg conditionals or calculations 
that just go away if inlined with constant arguments, for example), or if 
the lack of any function call at all allows the caller to do better 
register allocation around it.

Since these inlines contain a real call regardless, the second case never 
happens.

The first case can happen with things like "strlen()" being optimized away 
for constant-sized arguments, but for the one user where that was 
(xstrdup()) it really wasn't an issue. And while a constant size argument 
to "xmalloc()" is not unlikely, and would allow the compiler to optimize 
the

	if (!ret && !size)
		ret = malloc(1);

away statically as an inline, it's really not a big win.

		Linus

^ permalink raw reply

* git blame for a commit
From: Mircea Bardac @ 2008-06-22 22:32 UTC (permalink / raw)
  To: git

Hi everyone,

Is there any straightforward way of doing git blame for all the files 
that got changed in a commit. Problems are renames, deletes and copies.

I was also thinking of git diff with a huge number of context lines, but 
this one feels a bit hacking. "git diff" is also missing author info, so 
"git blame" is a bit more desirable.

Has anyone ever done this before?

Many thanks.

-- 
Mircea
http://mircea.bardac.net

^ permalink raw reply

* Re: Shrink the git binary a bit by avoiding unnecessary inline functions
From: Matthieu Moy @ 2008-06-22 22:20 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <alpine.LFD.1.10.0806221159140.2926@woody.linux-foundation.org>

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

> -static inline char* xstrdup(const char *str)
> -{
> -	char *ret = strdup(str);
> -	if (!ret) {
> -		release_pack_memory(strlen(str) + 1, -1);
> -		ret = strdup(str);
> -		if (!ret)
> -			die("Out of memory, strdup failed");
> -	}
> -	return ret;
> -}

Wouldn't it be better to split that kind of function into two parts:

- inline the short common case
- put the special case apart

like

static inline char* xstrdup(const char *str)
{
	char *ret = strdup(str);
	if (!ret)
		return xstrdup_failed(str);
	else
		return ret;
}

(with xstrdup_failed not being inline.)

?

-- 
Matthieu

^ permalink raw reply

* Re: linux-x86-tip: pilot error?
From: Paul E. McKenney @ 2008-06-22 22:21 UTC (permalink / raw)
  To: Björn Steinbrink; +Cc: Mikael Magnusson, mingo, git
In-Reply-To: <20080622214202.GA15311@atjola.homenet>

On Sun, Jun 22, 2008 at 11:42:02PM +0200, Björn Steinbrink wrote:
> On 2008.06.22 06:21:05 -0700, Paul E. McKenney wrote:
> > Trying "git-checkout -b tip-core-rcu tip-core-rcu-2008-06-16_09.23_Mon"
> > acts like it is doing something useful, but doesn't find the recent updates,
> > which I believe happened -before- June 16 2008.
> 
> Do you mean these?
> rcu: make rcutorture more vicious: reinstate boot-time testing
> rcu: make rcutorture more vicious: add stutter feature

Indeed those are the ones that I am looking for!

> I just fetched tip, and here, those two were committed on June 18 2008.
> They're in tip/core/rcu, but not in the tag you mentioned.
> 
> JFYI, I found those by using "git log --grep=stutter --all", and then
> passing the commit hash to "git describe":
> 
> $ git describe --contains --all 31a72bce0bd6f3e0114009288bccbc96376eeeca
> remotes/tip/core/rcu

Thank you -- that does find those commits for me as well.  <scratches
head>

							Thanx, Paul

^ permalink raw reply

* Re: [jgit PATCH] Paper bag fix quoting for SSH transport commands
From: Shawn O. Pearce @ 2008-06-22 22:15 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git, Marek Zawirski
In-Reply-To: <200806221954.08919.robin.rosenberg.lists@dewire.com>

Robin Rosenberg <robin.rosenberg.lists@dewire.com> wrote:
> söndagen den 22 juni 2008 03.36.40 skrev Shawn O. Pearce:
> > 
> > Testing concludes that git-shell requires the command name to never
> > be quoted, and the argument name to always be single quoted.  As
> > this is a long-standing behavior in the wild jgit needs to conform,
> > as git-shell and all git-shell work-a-likes such as gitosis may be
> > following the same convention.
> 
> Seems ok and works here. Error handling still has a paperbagish feel. See
> follow up patches.

Well, I wasn't trying to clean up error handling, I was just trying
to make `jgit fetch` work against repo.or.cz.  But I agree, the
error handling and feedback in the transport code could be improved.

> Maybe we should have a patch for git too so it will actually work
> with spaces in file names. What do people on Windows do? (those that
> actually get an SSH server up and running and sleep well overe it
> on that platform).

The issue only happens with the command name, which most people
use just git-upload-pack/git-receive-pack as they have git in their
$PATH on the remote side, or are running git-shell.  I suspect that
Windows users just don't run Git over SSH with paths that contain
spaces to access git-upload-pack remotely.

I'd patch Git to use the same rule as jgit and only quote the command
when really necessary, but its not high on my list of things to do
this month.  :-)

> As for pushing and signing. One way is for you (Shawn) and me is
> to sign-off and push each other's patches. I pushed this one.

Given that repo.or.cz doesn't show reflogs, I take it this is only
a way to make sure at least someone has reviewed the patch before
it goes into the main tree, since we both have write access?  I
can live with that.

-- 
Shawn.

^ permalink raw reply

* core.autocrlf and merge conflict output
From: Edward Z. Yang @ 2008-06-22 22:08 UTC (permalink / raw)
  To: git

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Apparently, the conflict information Git writes to the working copy
during merge doesn't respect core.autocrlf, magically converting files
to LF and causing problems when you try to commit the files, when
core.safecrlf is on.

I'm in the process of making a patch, but I'm having difficulty getting
builtin-merge-file.c and convert.c to play together. Part of the problem
is the lack of documentation in convert.c, which I'd also like to fix
(but in a different patch).

So, here's an appeal to the Git gods:

* Should I even bother trying to document this? While good docs are
always a definite plus, poor documentation can hurt code.

* Would crlf_to_worktree (apparently an internal function) or
convert_to_working_tree be more appropriate for this task?

* Where can I get a sensible value for *path in convert.c from
builtin-merge-file.c?

Thanks!
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFIXs17qTO+fYacSNoRAuAEAJ9LekfdUzU8WjbYxdhDOLaUfN67HACgg6gR
3kpT/5CbV3X7bbfkwJFDpAI=
=lega
-----END PGP SIGNATURE-----

^ permalink raw reply

* Re: linux-x86-tip: pilot error?
From: Björn Steinbrink @ 2008-06-22 21:42 UTC (permalink / raw)
  To: Paul E. McKenney; +Cc: Mikael Magnusson, mingo, git
In-Reply-To: <20080622132105.GD22569@linux.vnet.ibm.com>

On 2008.06.22 06:21:05 -0700, Paul E. McKenney wrote:
> Trying "git-checkout -b tip-core-rcu tip-core-rcu-2008-06-16_09.23_Mon"
> acts like it is doing something useful, but doesn't find the recent updates,
> which I believe happened -before- June 16 2008.

Do you mean these?
rcu: make rcutorture more vicious: reinstate boot-time testing
rcu: make rcutorture more vicious: add stutter feature

I just fetched tip, and here, those two were committed on June 18 2008.
They're in tip/core/rcu, but not in the tag you mentioned.

JFYI, I found those by using "git log --grep=stutter --all", and then
passing the commit hash to "git describe":

$ git describe --contains --all 31a72bce0bd6f3e0114009288bccbc96376eeeca
remotes/tip/core/rcu

Björn

^ permalink raw reply

* gitweb: Broken links in blame view
From: Lea Wiemann @ 2008-06-22 21:37 UTC (permalink / raw)
  To: Git Mailing List

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

Hi!

I (or rather, the link checker) has discovered another broken link, this 
time in gitweb's blame view, which happens if you blame files and it 
tries to get parents before the initial (first) commit in the repository:

1. Unpack test.git.tar.gz (attached) into gitweb's project root.

2. Make sure $feature{'blame'}{'default'} = [1]; is on in
gitweb_config.perl.

3. Open the following page:
http://localhost/.../gitweb.cgi?p=test.git;a=blame;f=file;h=f8c58d2c88cbcf660f473f8f123ee32db0d0d2be;hb=HEAD

4. Click the gray "1" line number -- you get a "400 - Invalid hash base 
parameter" response (or '403' instead of 400 if you're not on next).

(I've also reproduced this with gitweb revisions from 7 months ago, so 
it's not related to recent changes.)

Also, probably related, if you open the above blame view by calling 
gitweb.cgi on the command line, you'll see a bogus error message on 
STDERR: "fatal: ambiguous argument 'f891e539f...^': unknown revision" 
Note that f891... is the earliest revision in the repository.

-- Lea

[-- Attachment #2: test.git.tar.gz --]
[-- Type: application/gzip, Size: 9074 bytes --]

^ permalink raw reply

* Re: Shrink the git binary a bit by avoiding unnecessary inline functions
From: Christian MICHON @ 2008-06-22 21:32 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <alpine.LFD.1.10.0806221159140.2926@woody.linux-foundation.org>

On Sun, Jun 22, 2008 at 9:19 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> So I was looking at the disgusting size of the git binary, and even with
> the debugging removed, and using -Os instead of -O2, the size of the text
> section was pretty high. In this day and age I guess almost a megabyte of
> text isn't really all that surprising, but it still doesn't exactly make
> me think "lean and mean".

using gcc-3.4.6 and uclibc-0.9.29 (not exactly everyone's
configuration of course...)
I get different numbers with CFLAGS=-Os and NO_CURL, NO_ICONV on plain
git-1.5.6:

sh-3.2# ls -lh git
-rwxr-xr-x    3 root     root       699.7k Jun 22 23:26 git

sh-3.2# size git
   text    data     bss     dec     hex filename
 616544   10960  272272  899776   dbac0 git

after I use your patch, it goes to:

sh-3.2# ls -lh git
-rwxr-xr-x    1 root     root       652.6k Jun 22 23:30 git

sh-3.2# size git
   text    data     bss     dec     hex filename
 568124   10960  272272  851356   cfd9c git

So your patch obviously works here too but I get quite smaller figures too.

curl and iconv are not available on my distro detaolb, maybe it's a
big difference too...

Could your figures come from recent gcc/glibc versions ?

-- 
Christian
--
http://detaolb.sourceforge.net/, a linux distribution for Qemu with Git inside !

^ permalink raw reply

* Re: linux-x86-tip: pilot error?
From: Mikael Magnusson @ 2008-06-22 21:11 UTC (permalink / raw)
  To: paulmck; +Cc: mingo, git
In-Reply-To: <20080622132105.GD22569@linux.vnet.ibm.com>

2008/6/22 Paul E. McKenney <paulmck@linux.vnet.ibm.com>:
> On Sun, Jun 22, 2008 at 02:48:35PM +0200, Mikael Magnusson wrote:
>> 2008/6/22 Paul E. McKenney <paulmck@linux.vnet.ibm.com>:
>> > Hello, Ingo,
>> >
>> > I took the precaution of rebuilding my linux-2.6-tip from scratch as follows:
>> >
>> >  544  mkdir linux-2.6-tip
>> >  545  cd linux-2.6-tip
>> >  546  git-init-db
>> >  547  git-remote add linus git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
>> >  548  git-remote add tip git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip.git
>> >  549  git-remote update
>> >  550  git-checkout tip-core-rcu-2008-06-16_09.23_Mon
>>
>> When checking out remote branches, you have to specify the remote:
>> git checkout tip/tip-blabla
>> (it'll warn about detaching HEAD, this is normal).
>
> Thank you, Mikael!
>
> But when I try "git-checkout tip/tip-core-rcu-2008-06-16_09.23_Mon",
> it says:
>
>        error: pathspec 'tip/tip-core-rcu-2008-06-16_09.23_Mon' did not match any file(s) known to git.
>        Did you forget to 'git add'?
>
> Trying "git-checkout tip/tip-core-rcu" gets me:
>
>        error: pathspec 'tip/tip-core-rcu' did not match any file(s) known to git.
>        Did you forget to 'git add'?
>
> Trying "git-checkout -b tip-core-rcu tip/tip-core-rcu" gets me:
>
>        git checkout: updating paths is incompatible with switching branches/forcing
>        Did you intend to checkout 'tip/tip-core-rcu' which can not be resolved as commit?
>
> Trying "git-checkout -b tip-core-rcu tip/tip-core-rcu-2008-06-16_09.23_Mon"
> gets me:
>
>        git checkout: updating paths is incompatible with switching branches/forcing
>        Did you intend to checkout 'tip/tip-core-rcu-2008-06-16_09.23_Mon' which can not be resolved as commit?
>
> Trying "git-checkout -b tip-core-rcu tip-core-rcu-2008-06-16_09.23_Mon"
> acts like it is doing something useful, but doesn't find the recent updates,
> which I believe happened -before- June 16 2008.
>
> Help???

Oh, i didn't realize you were trying to check out a tag... In that case
the commands you gave were correct. I could successfully run your commands
here (though i have no idea if the file you talk about is up to date or not).

It's probably worth trying a newer version of git, could be a bug I guess.
Given the error message you could also try first checking out a branch, and
then the tag. ie git checkout -b master linus/master; git checkout tip-foo
It could also be that the tag just doesn't point to the commit you expect..

-- 
Mikael Magnusson

^ permalink raw reply

* [JGIT RFC PATCH] Add a stdio prompt for SSH connection information.
From: Robin Rosenberg @ 2008-06-22 21:06 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Marek Zawirski, git

With Java6 there is support for echo-less input of information
like passwords from the console.

Windows users must set the system headless promperty java.awt.headless
to true (-Djava.awt.headless=true) manually. Only unix system headless
operation is detected automatically via the DISPLAY environment variable.

Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---

Starting a command line utility like jgit and getting a graphical prompt is almost
an insult. The problem here is that Java 5, which we support, does not have a
portable way of disabling echoing of characters. Java 6 (and anythung newr)
does. There are several solutions involving non-portable tricks. Should we 
support an insecure practice of echoing passwords, or as I do here, only support
it if one is using Java 6. A downside of supporting it at all is that one needs a
JavaSE 6 compiler to build the thing.

btw, does anyone know if console() yields null when runnings as a Windows
service? I tentatively assume that it does without explicily setting the headless
property.

I'm also a little unsure about how to invoke the promptKeyboardInteractive method.

 .../jgit/transport/DefaultSshSessionFactory.java   |   86 +++++++++++++++++++-
 1 files changed, 84 insertions(+), 2 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/DefaultSshSessionFactory.java b/org.spearce.jgit/src/org/spearce/jgit/transport/DefaultSshSessionFactory.java
index 8a59904..c895988 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/DefaultSshSessionFactory.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/DefaultSshSessionFactory.java
@@ -39,6 +39,7 @@
 package org.spearce.jgit.transport;
 
 import java.awt.Container;
+import java.awt.GraphicsEnvironment;
 import java.awt.GridBagConstraints;
 import java.awt.GridBagLayout;
 import java.awt.Insets;
@@ -90,8 +91,18 @@ class DefaultSshSessionFactory extends SshSessionFactory {
 		final Session session = getUserJSch().getSession(user, host, port);
 		if (pass != null)
 			session.setPassword(pass);
-		else
-			session.setUserInfo(new AWT_UserInfo());
+		UserInfo userInfo = null;
+		try {
+			System.class.getMethod("console");
+			if (GraphicsEnvironment.isHeadless() || System.console() == null)
+				userInfo = new StdioUserInfo();
+		} catch (NoSuchMethodException e) {
+			throw new IllegalStateException(
+					"You will need JavaSE 6 for stdio based password prompts");
+		}
+		if (userInfo == null)
+			userInfo = new AWT_UserInfo();
+		session.setUserInfo(userInfo);
 		return session;
 	}
 
@@ -249,4 +260,75 @@ class DefaultSshSessionFactory extends SshSessionFactory {
 			return null; // cancel
 		}
 	}
+
+	static class StdioUserInfo implements UserInfo, UIKeyboardInteractive {
+
+		String password;
+
+		String passphrase;
+
+		public String getPassphrase() {
+			return passphrase;
+		}
+
+		public String getPassword() {
+			return password;
+		}
+
+		public boolean promptPassphrase(String msg) {
+			passphrase = null;
+			char[] readPassword = System.console().readPassword("Passphrase: ");
+			if (readPassword == null)
+				return false;
+			passphrase = new String(readPassword);
+			return true;
+		}
+
+		public boolean promptPassword(String msg) {
+			password = null;
+			char[] readPassword = System.console().readPassword("Password: ");
+			if (readPassword == null)
+				return false;
+			password = new String(readPassword);
+			return true;
+		}
+
+		public boolean promptYesNo(String msg) {
+			String readLine = System.console().readLine("%s [Y/n]: ", msg);
+			if (readLine == null)
+				return false;
+			if (readLine.indexOf("yY") > 0)
+				return true;
+			return false;
+		}
+
+		public void showMessage(String msg) {
+			System.console().format("%s\n", msg);
+		}
+
+		public String[] promptKeyboardInteractive(final String destination,
+				final String name, final String instruction,
+				final String[] prompt, final boolean[] echo) {
+			System.console().printf("%s\n", instruction);
+			System.console().printf("Information for %s@%s required.\n", name,
+					destination);
+			String[] ret = new String[prompt.length];
+			for (int i = 0; i < ret.length; ++i) {
+				String s;
+				if (echo[i])
+					s = System.console().readLine("%s :", prompt[i]);
+				else {
+					char[] cha = System.console().readPassword("%s :",
+							prompt[i]);
+					if (cha == null)
+						s = null;
+					else
+						s = new String(cha);
+				}
+				ret[i] = s;
+			}
+			return ret;
+		}
+
+	}
 }
-- 
1.5.5.1.178.g1f811

^ permalink raw reply related

* Usage with Subversion, externals
From: John Locke @ 2008-06-22 21:03 UTC (permalink / raw)
  To: Git Mailing List

Hi,

Brand new git user, trying to get my head around how to use this after 
years of Subversion usage.

I have a project that uses svn:externals to load a dependent library. 
The external repository, dojo toolkit, has a different layout for trunk 
than the tagged versions. Right now, I've got these svn externals defined:

> john@shasta:/opt/www/auriga$ svn pg svn:externals public_html/
> dojo http://svn.dojotoolkit.org/src/tags/release-1.1.1/
>
> john@shasta:/opt/www/auriga$ svn pg svn:externals public_html/dojo-trunk/
> dojo http://svn.dojotoolkit.org/src/dojo/trunk/
> dijit http://svn.dojotoolkit.org/src/dijit/trunk/
> dojox http://svn.dojotoolkit.org/src/dojox/trunk/
public_html/dojo contains dojo, dijit, dojox, and util directories, 
tagged to the specific version. The dojo svn repository stores trunk 
versions for each of these modules in src/<module>/trunk.

The problem I'm trying to solve by switching to git is that there are 
some API changes to Dojo that break the current production version of my 
application. So I'd like to branch my application and develop it against 
the Dojo trunk, while still being able to make changes to the mainline 
code using the stable Dojo tagged version.


I've used git svn clone to copy my main application, and I'm also 
pulling down the dojo svn repository (would be happy to post this 
somewhere when it's done).


So: Question 1: how do I get public_html/dojo in my working copy to 
contain dojo/trunk, dijit/trunk, and dojox/trunk from my external git 
repository? I'm assuming I use a submodule for this, and git submodule, 
but I'm wondering how I point git submodule to a particular path in the 
other repository.

... and question 2: How do I set up a different git submodule path (in 
the same external git repository) when I work on a different branch?


I'd like to be able to git checkout the development branch and have it 
switch the dojo submodule to contain dojo/trunk (aliased as dojo), 
dijit/trunk (aliased as dijit), and dojox/trunk (aliased as dojox). And 
then when I git checkout the main branch have it switch back to 
tags/release-1.1.1 (which contains dojo, dijit, and dojox without a 
trunk subdirectory).

Any thoughts?

-- 
John Locke
"Open Source Solutions for Small Business Problems"
published by Charles River Media, June 2004
http://www.freelock.com

^ permalink raw reply

* Re: [PATCH v2] git-imap-send: Add support for SSL.
From: Junio C Hamano @ 2008-06-22 20:41 UTC (permalink / raw)
  To: Alam Arias; +Cc: git
In-Reply-To: <20080622152747.77a0baee@gmail.com>

Alam Arias <Alam.GBC@gmail.com> writes:

> Allow SSL to be used when a new config setting, imap.ssl, is set to
> true.
>
> Also, automatically use TLS when imap.ssl is not set by using the IMAP
> STARTTLS command, if the server supports it.
>
> Tested with Courier and Gimap IMAP servers.
>
> Signed-off-by: Robert Shearman <robertshearman@gmail.com>
> ---
>  Documentation/git-imap-send.txt |    1 +
>  Makefile                        |    4 +-
>  imap-send.c                     |  163
> +++++++++++++++++++++++++++++++++++---- 3 files changed, 152
> insertions(+), 16 deletions(-)
>

Next time please do _not_ attach *.diff but follow the style of patch
submission other people do (see recent patch from Linus for example).

> diff --git a/Documentation/git-imap-send.txt b/Documentation/git-imap-send.txt
> index f4fdc24..ecf2958 100644
> --- a/Documentation/git-imap-send.txt
> +++ b/Documentation/git-imap-send.txt
> @@ -41,6 +41,7 @@ configuration file (shown with examples):
>      User = bob
>      Pass = pwd
>      Port = 143
> +    Ssl  = false
>  ..........................

This is "start talking plain imap to the standard imap port and then say
STARTTLS to start SSL", not "imap over ssl (aka imaps = 993/tcp)", right?

Is support for the latter (1) widely needed, and/or (2) easy to add on top
of this?  I presume the latter would use imaps:// URL scheme (in which
case the user does not need an extra config)?

> diff --git a/Makefile b/Makefile
> index 6a31c9f..0bd18fa 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1157,7 +1157,9 @@ endif
>  git-%$X: %.o $(GITLIBS)
>  	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS)
>  
> -git-imap-send$X: imap-send.o $(LIB_FILE)
> +git-imap-send$X: imap-send.o $(GITLIBS)
> +	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
> +		$(LIBS) $(OPENSSL_LINK) $(OPENSSL_LIBSSL)

This looks enough to make it link both with and without NO_OPENSSL, but
has it actually been tested with both configurations?

> diff --git a/imap-send.c b/imap-send.c
> index 1ec1310..7c95c5c 100644
> --- a/imap-send.c
> +++ b/imap-send.c
> @@ -225,19 +235,104 @@ static const char *Flags[] = {
>  	"Deleted",
>  };
>  
> +#ifndef NO_OPENSSL
> +static void
> +ssl_socket_perror( const char *func )
> +{
> +	fprintf( stderr, "%s: %s\n", func, ERR_error_string(ERR_get_error(), 0));
> +}
> +#endif

The original code has tons of style violations like this, but please do
not introduce more of them.  I'd even like a follow-up patch after this
one to clean up the style of existing code (you can choose to do the other
way around, first clean up the style of existing code without adding
anything new, and then this patch without the style violations).

 * function name in definition does not start a new line, but follows its
   return type on the same line;

 * open and close parentheses for function parameter list and argument
   list are not followed/preceded by any space;

>  static void
>  socket_perror( const char *func, Socket_t *sock, int ret )
>  {
> -	if (ret < 0)
> -		perror( func );
> +#ifndef NO_OPENSSL
> +	if (sock->ssl) {
> +		int sslerr = SSL_get_error( sock->ssl, ret );
> +		switch (sslerr) {
> +			case SSL_ERROR_NONE:
> +				break;
> +			case SSL_ERROR_SYSCALL:
> +				perror( "SSL_connect" );
> +				break;
> +			default:
> +				ssl_socket_perror( "SSL_connect" );
> +				break;
> +		}

 * "case" arms of switch statement align with "switch" without extra
   indentation;

> +	/* FIXME! Add a config option for this */
> +	if (0)
> +		SSL_CTX_set_verify( ctx, SSL_VERIFY_PEER, NULL );

Indeed ;-).

> +	if (!SSL_CTX_set_default_verify_paths( ctx )) {
> +		ssl_socket_perror( "SSL_CTX_set_default_verify_paths" );
> +		return 1;
> +	}
> +	sock->ssl = SSL_new( ctx );
> +	if (!sock->ssl) {
> +		ssl_socket_perror( "SSL_new" );
> +		return 1;
> +	}

 * We usually signal error by returning negative (e.g. -1) unless there
   otherwise a reason not to.

> @@ -1014,7 +1142,10 @@ imap_open_store( imap_server_conf_t *srvc )
>  			fprintf( stderr, "Skipping account %s@%s, server forbids LOGIN\n", srvc->user, srvc->host );
>  			goto bail;
>  		}
> -		imap_warn( "*** IMAP Warning *** Password is being sent in the clear\n" );
> +#ifndef NO_OPENSSL
> +		if (!imap->buf.sock.ssl)
> +#endif

Hmm.  If NO_OPENSSL compilation had ".ssl" member that is a dummy "int" or
something, you can use this ifndef and it might make it easier to read.

^ permalink raw reply

* gitweb: broken link in atom view
From: Lea Wiemann @ 2008-06-22 20:16 UTC (permalink / raw)
  To: Git Mailing List

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

Linkchecking gitweb's atom feed on a test repository reveals a 
(solitary) broken link under certain conditions.  I can't deal with it 
right now, but maybe someone wants to look into it?  Here's how to 
reproduce it:

1. Unpack test.git.tar.gz (attached) into gitweb's project root.

2. Make sure $feature{'blame'}{'default'} = [1]; is on in 
gitweb_config.perl.

3. Download http://localhost/.../gitweb.cgi?p=test.git;a=atom

4. Locate the following link in the atom feed: 
http://localhost/.../gitweb.cgi?p=test.git;a=blame;f=renamed_file;hb=037554ed1a38b5112181208ac5a02ab461c4d305

5. Access the link -- it yields 404.

-- Lea

[-- Attachment #2: test.git.tar.gz --]
[-- Type: application/gzip, Size: 9074 bytes --]

^ permalink raw reply

* [PATCH v2] git-imap-send: Add support for SSL.
From: Alam Arias @ 2008-06-22 19:27 UTC (permalink / raw)
  To: git
In-Reply-To: <1096648c0806010829n71de92dcmc19ddb87da19931d@mail.gmail.com>

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


Allow SSL to be used when a new config setting, imap.ssl, is set to
true.

Also, automatically use TLS when imap.ssl is not set by using the IMAP
STARTTLS command, if the server supports it.

Tested with Courier and Gimap IMAP servers.

Signed-off-by: Robert Shearman <robertshearman@gmail.com>
---
 Documentation/git-imap-send.txt |    1 +
 Makefile                        |    4 +-
 imap-send.c                     |  163
+++++++++++++++++++++++++++++++++++---- 3 files changed, 152
insertions(+), 16 deletions(-)

[-- Attachment #2: af89f503ddce94c25529b50b3374a80913ca18cc.diff --]
[-- Type: text/x-patch, Size: 7512 bytes --]

diff --git a/Documentation/git-imap-send.txt b/Documentation/git-imap-send.txt
index f4fdc24..ecf2958 100644
--- a/Documentation/git-imap-send.txt
+++ b/Documentation/git-imap-send.txt
@@ -41,6 +41,7 @@ configuration file (shown with examples):
     User = bob
     Pass = pwd
     Port = 143
+    Ssl  = false
 ..........................
 
 
diff --git a/Makefile b/Makefile
index 6a31c9f..0bd18fa 100644
--- a/Makefile
+++ b/Makefile
@@ -1157,7 +1157,9 @@ endif
 git-%$X: %.o $(GITLIBS)
 	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS)
 
-git-imap-send$X: imap-send.o $(LIB_FILE)
+git-imap-send$X: imap-send.o $(GITLIBS)
+	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
+		$(LIBS) $(OPENSSL_LINK) $(OPENSSL_LIBSSL)
 
 http.o http-walker.o http-push.o transport.o: http.h
 
diff --git a/imap-send.c b/imap-send.c
index 1ec1310..7c95c5c 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -23,6 +23,10 @@
  */
 
 #include "cache.h"
+#ifndef NO_OPENSSL
+# include <openssl/ssl.h>
+# include <openssl/err.h>
+#endif
 
 typedef struct store_conf {
 	char *name;
@@ -129,6 +133,7 @@ typedef struct imap_server_conf {
 	int port;
 	char *user;
 	char *pass;
+	int use_ssl;
 } imap_server_conf_t;
 
 typedef struct imap_store_conf {
@@ -148,6 +153,9 @@ typedef struct _list {
 
 typedef struct {
 	int fd;
+#ifndef NO_OPENSSL
+	SSL *ssl;
+#endif
 } Socket_t;
 
 typedef struct {
@@ -201,6 +209,7 @@ enum CAPABILITY {
 	UIDPLUS,
 	LITERALPLUS,
 	NAMESPACE,
+	STARTTLS,
 };
 
 static const char *cap_list[] = {
@@ -208,6 +217,7 @@ static const char *cap_list[] = {
 	"UIDPLUS",
 	"LITERAL+",
 	"NAMESPACE",
+	"STARTTLS",
 };
 
 #define RESP_OK    0
@@ -225,19 +235,104 @@ static const char *Flags[] = {
 	"Deleted",
 };
 
+#ifndef NO_OPENSSL
+static void
+ssl_socket_perror( const char *func )
+{
+	fprintf( stderr, "%s: %s\n", func, ERR_error_string(ERR_get_error(), 0));
+}
+#endif
+
 static void
 socket_perror( const char *func, Socket_t *sock, int ret )
 {
-	if (ret < 0)
-		perror( func );
+#ifndef NO_OPENSSL
+	if (sock->ssl) {
+		int sslerr = SSL_get_error( sock->ssl, ret );
+		switch (sslerr) {
+			case SSL_ERROR_NONE:
+				break;
+			case SSL_ERROR_SYSCALL:
+				perror( "SSL_connect" );
+				break;
+			default:
+				ssl_socket_perror( "SSL_connect" );
+				break;
+		}
+	} else
+#endif
+	{
+		if (ret < 0)
+			perror( func );
+		else
+			fprintf( stderr, "%s: unexpected EOF\n", func );
+	}
+}
+
+static int
+ssl_socket_connect( Socket_t *sock, int use_tls_only )
+{
+#ifdef NO_OPENSSL
+	fprintf( stderr, "SSL requested but SSL support not compiled in\n" );
+	return 1;
+#else
+	SSL_METHOD *meth;
+	SSL_CTX *ctx;
+	int ret;
+
+	SSL_library_init();
+	SSL_load_error_strings();
+
+	if (use_tls_only)
+		meth = TLSv1_method();
 	else
-		fprintf( stderr, "%s: unexpected EOF\n", func );
+		meth = SSLv23_method();
+
+	if (!meth) {
+		ssl_socket_perror( "SSLv23_method" );
+		return 1;
+	}
+
+	ctx = SSL_CTX_new( meth );
+
+	/* FIXME! Add a config option for this */
+	if (0)
+		SSL_CTX_set_verify( ctx, SSL_VERIFY_PEER, NULL );
+
+	if (!SSL_CTX_set_default_verify_paths( ctx )) {
+		ssl_socket_perror( "SSL_CTX_set_default_verify_paths" );
+		return 1;
+	}
+	sock->ssl = SSL_new( ctx );
+	if (!sock->ssl) {
+		ssl_socket_perror( "SSL_new" );
+		return 1;
+	}
+	if (!SSL_set_fd( sock->ssl, sock->fd )) {
+		ssl_socket_perror( "SSL_set_fd" );
+		return 1;
+	}
+
+	ret = SSL_connect( sock->ssl );
+	if (ret <= 0) {
+		socket_perror( "SSL_connect", sock, ret );
+		return 1;
+	}
+
+	return 0;
+#endif
 }
 
 static int
 socket_read( Socket_t *sock, char *buf, int len )
 {
-	ssize_t n = xread( sock->fd, buf, len );
+	int n;
+#ifndef NO_OPENSSL
+	if (sock->ssl)
+		n = SSL_read( sock->ssl, buf, len );
+	else
+#endif
+		n = xread( sock->fd, buf, len );
 	if (n <= 0) {
 		socket_perror( "read", sock, n );
 		close( sock->fd );
@@ -249,7 +344,13 @@ socket_read( Socket_t *sock, char *buf, int len )
 static int
 socket_write( Socket_t *sock, const char *buf, int len )
 {
-	int n = write_in_full( sock->fd, buf, len );
+	int n;
+#ifndef NO_OPENSSL
+	if (sock->ssl)
+		n = SSL_write( sock->ssl, buf, len );
+	else
+#endif
+		n = write_in_full( sock->fd, buf, len );
 	if (n != len) {
 		socket_perror( "write", sock, n );
 		close( sock->fd );
@@ -258,6 +359,18 @@ socket_write( Socket_t *sock, const char *buf, int len )
 	return n;
 }
 
+static void
+socket_shutdown( Socket_t *sock )
+{
+#ifndef NO_OPENSSL
+	if (sock->ssl) {
+		SSL_shutdown( sock->ssl );
+		SSL_free( sock->ssl );
+	}
+#endif
+	close( sock->fd );
+}
+
 /* simple line buffering */
 static int
 buffer_gets( buffer_t * b, char **s )
@@ -875,7 +988,7 @@ imap_close_server( imap_store_t *ictx )
 
 	if (imap->buf.sock.fd != -1) {
 		imap_exec( ictx, NULL, "LOGOUT" );
-		close( imap->buf.sock.fd );
+		socket_shutdown( &imap->buf.sock );
 	}
 	free_list( imap->ns_personal );
 	free_list( imap->ns_other );
@@ -958,10 +1071,15 @@ imap_open_store( imap_server_conf_t *srvc )
 			perror( "connect" );
 			goto bail;
 		}
-		imap_info( "ok\n" );
 
 		imap->buf.sock.fd = s;
 
+		if (srvc->use_ssl && ssl_socket_connect( &imap->buf.sock, 0 )) {
+			close( s );
+			goto bail;
+		}
+		imap_info( "ok\n" );
+
 	}
 
 	/* read the greeting string */
@@ -986,7 +1104,17 @@ imap_open_store( imap_server_conf_t *srvc )
 		goto bail;
 
 	if (!preauth) {
-
+#ifndef NO_OPENSSL
+		if (!srvc->use_ssl && CAP(STARTTLS)) {
+			if (imap_exec( ctx, 0, "STARTTLS" ) != RESP_OK)
+				goto bail;
+			if (ssl_socket_connect( &imap->buf.sock, 1 ))
+				goto bail;
+			/* capabilities may have changed, so get the new capabilities */
+			if (imap_exec( ctx, 0, "CAPABILITY" ) != RESP_OK)
+				goto bail;
+		}
+#endif
 		imap_info ("Logging in...\n");
 		if (!srvc->user) {
 			fprintf( stderr, "Skipping server %s, no user\n", srvc->host );
@@ -1014,7 +1142,10 @@ imap_open_store( imap_server_conf_t *srvc )
 			fprintf( stderr, "Skipping account %s@%s, server forbids LOGIN\n", srvc->user, srvc->host );
 			goto bail;
 		}
-		imap_warn( "*** IMAP Warning *** Password is being sent in the clear\n" );
+#ifndef NO_OPENSSL
+		if (!imap->buf.sock.ssl)
+#endif
+			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" );
 			goto bail;
@@ -1242,6 +1373,7 @@ static imap_server_conf_t server =
 	0,	/* port */
 	NULL,	/* user */
 	NULL,	/* pass */
+	0,	/* use_ssl */
 };
 
 static char *imap_folder;
@@ -1262,12 +1394,8 @@ git_imap_config(const char *key, const char *val, void *cb)
 	if (!strcmp( "folder", key )) {
 		imap_folder = xstrdup( val );
 	} else if (!strcmp( "host", key )) {
-		{
-			if (!prefixcmp(val, "imap:"))
-				val += 5;
-			if (!server.port)
-				server.port = 143;
-		}
+		if (!prefixcmp(val, "imap:"))
+			val += 5;
 		if (!prefixcmp(val, "//"))
 			val += 2;
 		server.host = xstrdup( val );
@@ -1280,6 +1408,8 @@ git_imap_config(const char *key, const char *val, void *cb)
 		server.port = git_config_int( key, val );
 	else if (!strcmp( "tunnel", key ))
 		server.tunnel = xstrdup( val );
+	else if (!strcmp( "ssl", key ))
+		server.use_ssl = git_config_bool( key, val );
 	return 0;
 }
 
@@ -1298,6 +1428,9 @@ main(int argc, char **argv)
 
 	git_config(git_imap_config, NULL);
 
+	if (!server.port)
+		server.port = server.use_ssl ? 993 : 143;
+
 	if (!imap_folder) {
 		fprintf( stderr, "no imap store specified\n" );
 		return 1;

^ permalink raw reply related

* Shrink the git binary a bit by avoiding unnecessary inline functions
From: Linus Torvalds @ 2008-06-22 19:19 UTC (permalink / raw)
  To: Junio C Hamano, Git Mailing List


So I was looking at the disgusting size of the git binary, and even with 
the debugging removed, and using -Os instead of -O2, the size of the text 
section was pretty high. In this day and age I guess almost a megabyte of 
text isn't really all that surprising, but it still doesn't exactly make 
me think "lean and mean".

With -Os, a surprising amount of text space is wasted on inline functions 
that end up just being replicated multiple times, and where performance 
really isn't a valid reason to inline them. In particular, the trivial 
wrapper functions like "xmalloc()" are used _everywhere_, and making them 
inline just duplicates the text (and the string we use to 'die()' on 
failure) unnecessarily.

So this just moves them into a "wrapper.c" file, getting rid of a tiny bit 
of unnecessary bloat. The following numbers are both with "CFLAGS=-Os":

Before:
	[torvalds@woody git]$ size git
	   text	   data	    bss	    dec	    hex	filename
	 700460	  15160	 292184	1007804	  f60bc	git

After:
	[torvalds@woody git]$ size git
	   text	   data	    bss	    dec	    hex	filename
	 670540	  15160	 292184	 977884	  eebdc	git

so it saves almost 30k of text-space (it actually saves more than that 
with the default -O2, but I don't think that's necessarily a very relevant 
number from a "try to shrink git" standpoint).

It might conceivably have a performance impact, but none of this should be 
_that_ performance critical. The real cost is not generally in the wrapper 
anyway, but in the code it wraps (ie the cost of "xread()" is all in the 
read itself, not in the trivial wrapping of it).

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
---

There are probably other things we should do, but this was the obvious 
low-hanging fruit.

NOTE! When looking at the actual size of the binary on-disk, all the size 
issues are dwarfed by the cost of teh debug information, so don't be 
fooled by doing an "ls -l" unless you strip it first, or remove the "-g". 

If you just do a "ls -l git" with the default CFLAGS options, you'll think 
that the git binary is 3+MB in size:

	$ ls -lh git
	-rwxrwxr-x 88 torvalds torvalds 3.4M 2008-06-22 12:14 git

but that's mostly debug and symbol info:

	$ strip git
	$ ls -lh git
	-rwxrwxr-x 88 torvalds torvalds 811K 2008-06-22 12:15 git

so it's not wuite *that* scary. With -Os (this patch applied in all 
cases), it shrinks to

	$ ls -lh git
	-rwxrwxr-x 88 torvalds torvalds 2.9M 2008-06-22 12:17 git

	$ strip git
	$ ls -lh git
	-rwxrwxr-x 88 torvalds torvalds 680K 2008-06-22 12:18 git

respectively.

 Makefile          |    1 +
 git-compat-util.h |  167 ++++-------------------------------------------------
 wrapper.c         |  160 ++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 173 insertions(+), 155 deletions(-)

diff --git a/Makefile b/Makefile
index 6a31c9f..408ec33 100644
--- a/Makefile
+++ b/Makefile
@@ -467,6 +467,7 @@ LIB_OBJS += unpack-trees.o
 LIB_OBJS += usage.o
 LIB_OBJS += utf8.o
 LIB_OBJS += walker.o
+LIB_OBJS += wrapper.o
 LIB_OBJS += write_or_die.o
 LIB_OBJS += ws.o
 LIB_OBJS += wt-status.o
diff --git a/git-compat-util.h b/git-compat-util.h
index c04e8ba..6f94a81 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -240,161 +240,18 @@ static inline char *gitstrchrnul(const char *s, int c)
 
 extern void release_pack_memory(size_t, int);
 
-static inline char* xstrdup(const char *str)
-{
-	char *ret = strdup(str);
-	if (!ret) {
-		release_pack_memory(strlen(str) + 1, -1);
-		ret = strdup(str);
-		if (!ret)
-			die("Out of memory, strdup failed");
-	}
-	return ret;
-}
-
-static inline void *xmalloc(size_t size)
-{
-	void *ret = malloc(size);
-	if (!ret && !size)
-		ret = malloc(1);
-	if (!ret) {
-		release_pack_memory(size, -1);
-		ret = malloc(size);
-		if (!ret && !size)
-			ret = malloc(1);
-		if (!ret)
-			die("Out of memory, malloc failed");
-	}
-#ifdef XMALLOC_POISON
-	memset(ret, 0xA5, size);
-#endif
-	return ret;
-}
-
-/*
- * xmemdupz() allocates (len + 1) bytes of memory, duplicates "len" bytes of
- * "data" to the allocated memory, zero terminates the allocated memory,
- * and returns a pointer to the allocated memory. If the allocation fails,
- * the program dies.
- */
-static inline void *xmemdupz(const void *data, size_t len)
-{
-	char *p = xmalloc(len + 1);
-	memcpy(p, data, len);
-	p[len] = '\0';
-	return p;
-}
-
-static inline char *xstrndup(const char *str, size_t len)
-{
-	char *p = memchr(str, '\0', len);
-	return xmemdupz(str, p ? p - str : len);
-}
-
-static inline void *xrealloc(void *ptr, size_t size)
-{
-	void *ret = realloc(ptr, size);
-	if (!ret && !size)
-		ret = realloc(ptr, 1);
-	if (!ret) {
-		release_pack_memory(size, -1);
-		ret = realloc(ptr, size);
-		if (!ret && !size)
-			ret = realloc(ptr, 1);
-		if (!ret)
-			die("Out of memory, realloc failed");
-	}
-	return ret;
-}
-
-static inline void *xcalloc(size_t nmemb, size_t size)
-{
-	void *ret = calloc(nmemb, size);
-	if (!ret && (!nmemb || !size))
-		ret = calloc(1, 1);
-	if (!ret) {
-		release_pack_memory(nmemb * size, -1);
-		ret = calloc(nmemb, size);
-		if (!ret && (!nmemb || !size))
-			ret = calloc(1, 1);
-		if (!ret)
-			die("Out of memory, calloc failed");
-	}
-	return ret;
-}
-
-static inline void *xmmap(void *start, size_t length,
-	int prot, int flags, int fd, off_t offset)
-{
-	void *ret = mmap(start, length, prot, flags, fd, offset);
-	if (ret == MAP_FAILED) {
-		if (!length)
-			return NULL;
-		release_pack_memory(length, fd);
-		ret = mmap(start, length, prot, flags, fd, offset);
-		if (ret == MAP_FAILED)
-			die("Out of memory? mmap failed: %s", strerror(errno));
-	}
-	return ret;
-}
-
-/*
- * xread() is the same a read(), but it automatically restarts read()
- * operations with a recoverable error (EAGAIN and EINTR). xread()
- * DOES NOT GUARANTEE that "len" bytes is read even if the data is available.
- */
-static inline ssize_t xread(int fd, void *buf, size_t len)
-{
-	ssize_t nr;
-	while (1) {
-		nr = read(fd, buf, len);
-		if ((nr < 0) && (errno == EAGAIN || errno == EINTR))
-			continue;
-		return nr;
-	}
-}
-
-/*
- * xwrite() is the same a write(), but it automatically restarts write()
- * operations with a recoverable error (EAGAIN and EINTR). xwrite() DOES NOT
- * GUARANTEE that "len" bytes is written even if the operation is successful.
- */
-static inline ssize_t xwrite(int fd, const void *buf, size_t len)
-{
-	ssize_t nr;
-	while (1) {
-		nr = write(fd, buf, len);
-		if ((nr < 0) && (errno == EAGAIN || errno == EINTR))
-			continue;
-		return nr;
-	}
-}
-
-static inline int xdup(int fd)
-{
-	int ret = dup(fd);
-	if (ret < 0)
-		die("dup failed: %s", strerror(errno));
-	return ret;
-}
-
-static inline FILE *xfdopen(int fd, const char *mode)
-{
-	FILE *stream = fdopen(fd, mode);
-	if (stream == NULL)
-		die("Out of memory? fdopen failed: %s", strerror(errno));
-	return stream;
-}
-
-static inline int xmkstemp(char *template)
-{
-	int fd;
-
-	fd = mkstemp(template);
-	if (fd < 0)
-		die("Unable to create temporary file: %s", strerror(errno));
-	return fd;
-}
+extern char *xstrdup(const char *str);
+extern void *xmalloc(size_t size);
+extern void *xmemdupz(const void *data, size_t len);
+extern char *xstrndup(const char *str, size_t len);
+extern void *xrealloc(void *ptr, size_t size);
+extern void *xcalloc(size_t nmemb, size_t size);
+extern void *xmmap(void *start, size_t length, int prot, int flags, int fd, off_t offset);
+extern ssize_t xread(int fd, void *buf, size_t len);
+extern ssize_t xwrite(int fd, const void *buf, size_t len);
+extern int xdup(int fd);
+extern FILE *xfdopen(int fd, const char *mode);
+extern int xmkstemp(char *template);
 
 static inline size_t xsize_t(off_t len)
 {
diff --git a/wrapper.c b/wrapper.c
new file mode 100644
index 0000000..4e04f76
--- /dev/null
+++ b/wrapper.c
@@ -0,0 +1,160 @@
+/*
+ * Various trivial helper wrappers around standard functions
+ */
+#include "cache.h"
+
+char *xstrdup(const char *str)
+{
+	char *ret = strdup(str);
+	if (!ret) {
+		release_pack_memory(strlen(str) + 1, -1);
+		ret = strdup(str);
+		if (!ret)
+			die("Out of memory, strdup failed");
+	}
+	return ret;
+}
+
+void *xmalloc(size_t size)
+{
+	void *ret = malloc(size);
+	if (!ret && !size)
+		ret = malloc(1);
+	if (!ret) {
+		release_pack_memory(size, -1);
+		ret = malloc(size);
+		if (!ret && !size)
+			ret = malloc(1);
+		if (!ret)
+			die("Out of memory, malloc failed");
+	}
+#ifdef XMALLOC_POISON
+	memset(ret, 0xA5, size);
+#endif
+	return ret;
+}
+
+/*
+ * xmemdupz() allocates (len + 1) bytes of memory, duplicates "len" bytes of
+ * "data" to the allocated memory, zero terminates the allocated memory,
+ * and returns a pointer to the allocated memory. If the allocation fails,
+ * the program dies.
+ */
+void *xmemdupz(const void *data, size_t len)
+{
+	char *p = xmalloc(len + 1);
+	memcpy(p, data, len);
+	p[len] = '\0';
+	return p;
+}
+
+char *xstrndup(const char *str, size_t len)
+{
+	char *p = memchr(str, '\0', len);
+	return xmemdupz(str, p ? p - str : len);
+}
+
+void *xrealloc(void *ptr, size_t size)
+{
+	void *ret = realloc(ptr, size);
+	if (!ret && !size)
+		ret = realloc(ptr, 1);
+	if (!ret) {
+		release_pack_memory(size, -1);
+		ret = realloc(ptr, size);
+		if (!ret && !size)
+			ret = realloc(ptr, 1);
+		if (!ret)
+			die("Out of memory, realloc failed");
+	}
+	return ret;
+}
+
+void *xcalloc(size_t nmemb, size_t size)
+{
+	void *ret = calloc(nmemb, size);
+	if (!ret && (!nmemb || !size))
+		ret = calloc(1, 1);
+	if (!ret) {
+		release_pack_memory(nmemb * size, -1);
+		ret = calloc(nmemb, size);
+		if (!ret && (!nmemb || !size))
+			ret = calloc(1, 1);
+		if (!ret)
+			die("Out of memory, calloc failed");
+	}
+	return ret;
+}
+
+void *xmmap(void *start, size_t length,
+	int prot, int flags, int fd, off_t offset)
+{
+	void *ret = mmap(start, length, prot, flags, fd, offset);
+	if (ret == MAP_FAILED) {
+		if (!length)
+			return NULL;
+		release_pack_memory(length, fd);
+		ret = mmap(start, length, prot, flags, fd, offset);
+		if (ret == MAP_FAILED)
+			die("Out of memory? mmap failed: %s", strerror(errno));
+	}
+	return ret;
+}
+
+/*
+ * xread() is the same a read(), but it automatically restarts read()
+ * operations with a recoverable error (EAGAIN and EINTR). xread()
+ * DOES NOT GUARANTEE that "len" bytes is read even if the data is available.
+ */
+ssize_t xread(int fd, void *buf, size_t len)
+{
+	ssize_t nr;
+	while (1) {
+		nr = read(fd, buf, len);
+		if ((nr < 0) && (errno == EAGAIN || errno == EINTR))
+			continue;
+		return nr;
+	}
+}
+
+/*
+ * xwrite() is the same a write(), but it automatically restarts write()
+ * operations with a recoverable error (EAGAIN and EINTR). xwrite() DOES NOT
+ * GUARANTEE that "len" bytes is written even if the operation is successful.
+ */
+ssize_t xwrite(int fd, const void *buf, size_t len)
+{
+	ssize_t nr;
+	while (1) {
+		nr = write(fd, buf, len);
+		if ((nr < 0) && (errno == EAGAIN || errno == EINTR))
+			continue;
+		return nr;
+	}
+}
+
+int xdup(int fd)
+{
+	int ret = dup(fd);
+	if (ret < 0)
+		die("dup failed: %s", strerror(errno));
+	return ret;
+}
+
+FILE *xfdopen(int fd, const char *mode)
+{
+	FILE *stream = fdopen(fd, mode);
+	if (stream == NULL)
+		die("Out of memory? fdopen failed: %s", strerror(errno));
+	return stream;
+}
+
+int xmkstemp(char *template)
+{
+	int fd;
+
+	fd = mkstemp(template);
+	if (fd < 0)
+		die("Unable to create temporary file: %s", strerror(errno));
+	return fd;
+}

^ permalink raw reply related

* Re: Is git-imap-send able to use SSL?
From: Andreas Ericsson @ 2008-06-22 19:11 UTC (permalink / raw)
  To: Alam Arias; +Cc: git
In-Reply-To: <20080621011622.39fd1625@gmail.com>

Alam Arias wrote:
> On Fri, 20 Jun 2008 18:08:42 +0200
> Cristian Peraferrer <corellian.c@gmail.com> wrote:
> 
>> I am trying to use git-imap-send to send a Draft to my GMail account  
>> which uses SSL to connect, I have put the correct port (993 in that  
>> case) in the config file but it seems it doesn't work. I figure that  
>> git-imap-send is not able to connect using SSL.
>>
>  well, there a patch by for SSL support for git-imap-send by Rob
> Shearman "robertshearman@gmail.com" over two weeks ago but it did not
> apply cleanly on master at the time, well, maybe I can send
> my version of this patch? or do I need to ask Rob Shearman?
> 

Since the patch was made against the current git sources, which are GPL,
the patch itself and the produced sources are also GPL. As such, you're
free to continue the work he sent you and submit it back to git, but you
may not alter the license for your submitted code.

That is: "Yes, you may work on Rob Shearmans patch, but the result must
be GPL'd".

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

^ permalink raw reply

* Re: [PATCH 1/2] parse_options: Add flag to prevent errors for further processing
From: Johannes Schindelin @ 2008-06-22 19:07 UTC (permalink / raw)
  To: Shawn Bohrer; +Cc: Junio C Hamano, Jeff King, git, madcoder
In-Reply-To: <20080619142527.GA8429@mediacenter>

Hi,

On Thu, 19 Jun 2008, Shawn Bohrer wrote:

> On Wed, Jun 18, 2008 at 11:52:42AM -0700, Junio C Hamano wrote:
> > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> > 
> > > On Tue, 17 Jun 2008, Junio C Hamano wrote:
> > >
> > >> Jeff King <peff@peff.net> writes:
> > >> 
> > >> > I think the only right way to accomplish this is to convert the 
> > >> > revision and diff parameters into a parseopt-understandable 
> > >> > format.
> > >> 
> > >> Not necessarily.  You could structure individual option parsers 
> > >> like how diff option parsers are done.  You iterate over argv[], 
> > >> feed diff option parser the current index into argv[] and ask if it 
> > >> is an option diff understands, have diff eat the option (and 
> > >> possibly its parameter) to advance the index, or allow diff option 
> > >> to say "I do not understand this", and then handle it yourself or 
> > >> hand it to other parsers.
> > >
> > > AFAIR Pierre tried a few ways, and settled with a macro to introduce 
> > > the diff options into a caller's options.
> > >
> > > IOW it would look something like this:
> > >
> > > static struct option builtin_what_options[] = {
> > > 	[... options specific to this command ...]
> > > 	DIFF__OPT(&diff_options)
> > > };
> > 
> > I think that is the more painful approach Jeff mentioned, and my 
> > comment was to show that it is not the only way.
> > 
> 
> It seems to me that you could implement Jeff's 
> PARSE_OPT_STOP_AT_UNKNOWN, and then if multiple option parsers are 
> needed you would simply loop over parse_options for each of the 
> commands, waiting for argc to stop changing.  Of course Jeff's flag 
> would also need to stop parse_options from eating the first argument. Is 
> this sort of what you are suggesting Junio?

I believe not.  I think that Junio prefers some callback that can handle a 
whole bunch of options (as opposed to the callback we can have now, to 
handle arguments for a specific option).

However, I am not sure what that would buy us over the approach Pierre 
settled.  Junio, maybe you thought that the option parsing macro would 
live in parse-options.h?  It was supposed to live in diff.h and 
revision.h, respectively.

Ciao,
Dscho

^ permalink raw reply

* [PATCH] Take care of errors reported from the server when upload command is started
From: Robin Rosenberg @ 2008-06-22 17:46 UTC (permalink / raw)
  To: git; +Cc: Shawn O. Pearce, Marek Zawirski, Robin Rosenberg
In-Reply-To: <20080622013640.GA18629@spearce.org>

When JSch cannot launch the command our SSH Channel will close immediately
in connect, resulting in a NullPointerException and a closed System.err
silencing an errors.

Using System.err for errors also means we get no feedback when using from
Eclipse.
---
 .../spearce/egit/ui/EclipseSshSessionFactory.java  |   29 ++++++++++++++++++++
 .../jgit/transport/DefaultSshSessionFactory.java   |    6 ++++
 .../spearce/jgit/transport/SshSessionFactory.java  |   10 +++++++
 .../spearce/jgit/transport/TransportGitSsh.java    |   12 +++++++-
 4 files changed, 55 insertions(+), 2 deletions(-)

diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/EclipseSshSessionFactory.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/EclipseSshSessionFactory.java
index 5005494..144d47d 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/EclipseSshSessionFactory.java
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/EclipseSshSessionFactory.java
@@ -8,6 +8,8 @@
  *******************************************************************************/
 package org.spearce.egit.ui;
 
+import java.io.IOException;
+import java.io.OutputStream;
 import java.security.AccessController;
 import java.security.PrivilegedAction;
 
@@ -44,4 +46,31 @@ class EclipseSshSessionFactory extends SshSessionFactory {
 			}
 		});
 	}
+
+	@Override
+	public OutputStream getErrorStream() {
+		return new OutputStream() {
+
+			StringBuilder all = new StringBuilder();
+			StringBuilder sb = new StringBuilder();
+
+			public String toString() {
+				return all.toString();
+			}
+
+			@Override
+			public void write(int b) throws IOException {
+				if (b == '\r')
+					return;
+				sb.append((char) b);
+				if (b == '\n') {
+					String s = sb.toString();
+					all.append(s);
+					sb = new StringBuilder();
+					Activator.logError(s, new Throwable());
+				}
+			}
+		};
+	}
+
 }
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/DefaultSshSessionFactory.java b/org.spearce.jgit/src/org/spearce/jgit/transport/DefaultSshSessionFactory.java
index 8a59904..5924a04 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/DefaultSshSessionFactory.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/DefaultSshSessionFactory.java
@@ -46,6 +46,7 @@ import java.io.File;
 import java.io.FileInputStream;
 import java.io.FileNotFoundException;
 import java.io.IOException;
+import java.io.OutputStream;
 import java.security.AccessController;
 import java.security.PrivilegedAction;
 
@@ -249,4 +250,9 @@ class DefaultSshSessionFactory extends SshSessionFactory {
 			return null; // cancel
 		}
 	}
+
+	@Override
+	public OutputStream getErrorStream() {
+		return System.err;
+	}
 }
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/SshSessionFactory.java b/org.spearce.jgit/src/org/spearce/jgit/transport/SshSessionFactory.java
index 7f8136d..4c9eae0 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/SshSessionFactory.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/SshSessionFactory.java
@@ -38,6 +38,8 @@
 
 package org.spearce.jgit.transport;
 
+import java.io.OutputStream;
+
 import com.jcraft.jsch.JSchException;
 import com.jcraft.jsch.Session;
 
@@ -121,4 +123,12 @@ public abstract class SshSessionFactory {
 		if (session.isConnected())
 			session.disconnect();
 	}
+
+	/**
+	 * Find or create an OutputStream for Ssh to use. For a command line client
+	 * this is probably System.err.
+	 * 
+	 * @return an OutputStream to receive the SSH error stream.
+	 */
+	public abstract OutputStream getErrorStream();
 }
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportGitSsh.java b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportGitSsh.java
index 8944df7..82723a3 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportGitSsh.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportGitSsh.java
@@ -39,6 +39,7 @@
 package org.spearce.jgit.transport;
 
 import java.io.IOException;
+import java.io.OutputStream;
 import java.net.ConnectException;
 import java.net.UnknownHostException;
 
@@ -77,6 +78,7 @@ class TransportGitSsh extends PackTransport {
 	}
 
 	final SshSessionFactory sch;
+	OutputStream errStream;
 
 	TransportGitSsh(final Repository local, final URIish uri) {
 		super(local, uri);
@@ -179,7 +181,8 @@ class TransportGitSsh extends PackTransport {
 			cmd.append(' ');
 			sqAlways(cmd, path);
 			channel.setCommand(cmd.toString());
-			channel.setErrStream(System.err);
+			errStream = SshSessionFactory.getInstance().getErrorStream();
+			channel.setErrStream(errStream, true); 
 			channel.connect();
 			return channel;
 		} catch (JSchException je) {
@@ -198,7 +201,12 @@ class TransportGitSsh extends PackTransport {
 			try {
 				session = openSession();
 				channel = exec(session, getOptionUploadPack());
-				init(channel.getInputStream(), channel.getOutputStream());
+
+				if (channel.isConnected())
+					init(channel.getInputStream(), channel.getOutputStream());
+				else
+					throw new TransportException(errStream.toString());
+					
 			} catch (TransportException err) {
 				close();
 				throw err;
-- 
1.5.5.1.178.g1f811

^ permalink raw reply related

* [PATCH] Clone: If url is changed was changed, forget the old value
From: Robin Rosenberg @ 2008-06-22 17:46 UTC (permalink / raw)
  To: git; +Cc: Shawn O. Pearce, Marek Zawirski, Robin Rosenberg
In-Reply-To: <1214156797-29186-2-git-send-email-robin.rosenberg@dewire.com>

This fixes a bug found by some monkey testing where the dialog
get stuck in a state where changing connection parameters have
no effect.

Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
 .../egit/ui/internal/clone/SourceBranchPage.java   |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/clone/SourceBranchPage.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/clone/SourceBranchPage.java
index 615be32..b704aaa 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/clone/SourceBranchPage.java
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/clone/SourceBranchPage.java
@@ -75,8 +75,10 @@ class SourceBranchPage extends WizardPage {
 		setImageDescriptor(UIIcons.WIZBAN_IMPORT_REPO);
 		sourcePage.addURIishChangeListener(new URIishChangeListener() {
 			public void uriishChanged(final URIish newURI) {
-				if (newURI == null || !newURI.equals(validated))
+				if (newURI == null || !newURI.equals(validated)) {
+					validated = null;
 					setPageComplete(false);
+				}
 			}
 		});
 		branchChangeListeners = new ArrayList<BranchChangeListener>(3);
-- 
1.5.5.1.178.g1f811

^ permalink raw reply related

* [PATCH] Clone: Handle cancel in clone dialog specially
From: Robin Rosenberg @ 2008-06-22 17:46 UTC (permalink / raw)
  To: git; +Cc: Shawn O. Pearce, Marek Zawirski, Robin Rosenberg
In-Reply-To: <1214156797-29186-1-git-send-email-robin.rosenberg@dewire.com>

Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
 .../src/org/spearce/egit/ui/UIText.java            |    3 +++
 .../egit/ui/internal/clone/SourceBranchPage.java   |   18 ++++++++++++------
 .../src/org/spearce/egit/ui/uitext.properties      |    2 ++
 3 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/UIText.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/UIText.java
index 9ccf606..4adb99c 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/UIText.java
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/UIText.java
@@ -122,6 +122,9 @@ public class UIText extends NLS {
 	public static String SourceBranchPage_cannotListBranches;
 
 	/** */
+	public static String SourceBranchPage_remoteListingCancelled;
+
+	/** */
 	public static String CloneDestinationPage_title;
 
 	/** */
diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/clone/SourceBranchPage.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/clone/SourceBranchPage.java
index b2f1a18..615be32 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/clone/SourceBranchPage.java
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/clone/SourceBranchPage.java
@@ -20,6 +20,7 @@ import java.util.List;
 import org.eclipse.core.runtime.IProgressMonitor;
 import org.eclipse.core.runtime.IStatus;
 import org.eclipse.core.runtime.NullProgressMonitor;
+import org.eclipse.core.runtime.OperationCanceledException;
 import org.eclipse.core.runtime.Status;
 import org.eclipse.jface.dialogs.ErrorDialog;
 import org.eclipse.jface.operation.IRunnableWithProgress;
@@ -269,12 +270,17 @@ class SourceBranchPage extends WizardPage {
 			});
 		} catch (InvocationTargetException e) {
 			Throwable why = e.getCause();
-			ErrorDialog.openError(getShell(),
-					UIText.SourceBranchPage_transportError,
-					UIText.SourceBranchPage_cannotListBranches, new Status(
-							IStatus.ERROR, Activator.getPluginId(), 0, why
-									.getMessage(), why.getCause()));
-			transportError(why.getMessage());
+			if ((why instanceof OperationCanceledException)) {
+				transportError(UIText.SourceBranchPage_remoteListingCancelled);
+				return;
+			} else {
+				ErrorDialog.openError(getShell(),
+						UIText.SourceBranchPage_transportError,
+						UIText.SourceBranchPage_cannotListBranches, new Status(
+								IStatus.ERROR, Activator.getPluginId(), 0, why
+										.getMessage(), why.getCause()));
+				transportError(why.getMessage());
+			}
 			return;
 		} catch (InterruptedException e) {
 			transportError(UIText.SourceBranchPage_interrupted);
diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/uitext.properties b/org.spearce.egit.ui/src/org/spearce/egit/ui/uitext.properties
index 18f8c28..9516aa0 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/uitext.properties
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/uitext.properties
@@ -55,6 +55,8 @@ SourceBranchPage_errorBranchRequired=At least one branch must be selected.
 SourceBranchPage_transportError=Transport Error
 SourceBranchPage_cannotListBranches=Cannot list the available branches.
 SourceBranchPage_interrupted=Connection attempt interrupted.
+SourceBranchPage_remoteListingCancelled=Operation cancelled
+
 
 CloneDestinationPage_title=Local Destination
 CloneDestinationPage_description=Configure the local storage location for {0}.
-- 
1.5.5.1.178.g1f811

^ permalink raw reply related

* Re: [q] git-diff --reverse 7def2be1..7def2be1^
From: Andreas Ericsson @ 2008-06-22 18:52 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: Jakub Narebski, git
In-Reply-To: <20080620135236.GC8135@elte.hu>

Ingo Molnar wrote:
> * Jakub Narebski <jnareb@gmail.com> wrote:
> 
>>>    But Git didnt recognize that as a valid commit range.
>> There is shortcut for rev^..rev, namely rev^! (I'm not sure if it is 
>> documented anywhere, though), so you could have used
>>
>>         git diff 7def2be1^!
> 
> nice! :-)
> 

And for just a single revision, you can do

	git show -R 7def2be1

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

^ permalink raw reply

* [PATCH] Add 'git-p4.allowSubmit' to git-p4
From: Jing Xue @ 2008-06-22 18:12 UTC (permalink / raw)
  To: Junio C Hamano, Simon Hausmann; +Cc: git, jingxue

I'm working with a perforce repo using git-p4. There are some config
files which I need to change locally according to my environment. I'm
using a 'local' git branch to park these changes. And I want to avoid
accidentally checking them into p4 just by doing "git p4 submit"
mindlessly without realizing which branch I'm actually on.

This patch adds a new git config, 'git-p4.allowSubmit', which is a
whitelist of branch names. "git p4 submit" will only allow submissions
from local branches on the list. Useful for preventing inadvertently
submitting from a strictly local branch.

For backward compatibility, if this config is not set at all,
submissions from all branches are allowed.

Signed-off-by: Jing Xue <jingxue@digizenstudio.com>
---
 contrib/fast-import/git-p4 |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index d8de9f6..87ca51e 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -687,6 +687,10 @@ class P4Submit(Command):
         else:
             return False
 
+        allowSubmit = gitConfig("git-p4.allowSubmit")
+        if len(allowSubmit) > 0 and not self.master in allowSubmit.split(","):
+            die("%s is not in git-p4.allowSubmit" % self.master)
+
         [upstream, settings] = findUpstreamBranchPoint()
         self.depotPath = settings['depot-paths'][0]
         if len(self.origin) == 0:
-- 
1.5.6.13.g2ae4ef

^ permalink raw reply related

* Re: [jgit PATCH] Paper bag fix quoting for SSH transport commands
From: Robin Rosenberg @ 2008-06-22 17:54 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git, Marek Zawirski
In-Reply-To: <20080622013640.GA18629@spearce.org>

söndagen den 22 juni 2008 03.36.40 skrev Shawn O. Pearce:
> Not all Git-over-SSH servers run a Bourne shell on the remote side
> to evaluate the command we are sending.  Some servers run git-shell,
> which will fail to execute git-upload-pack if we feed it a quoted
> string for the name git-upload-pack.
> 
> Testing concludes that git-shell requires the command name to never
> be quoted, and the argument name to always be single quoted.  As
> this is a long-standing behavior in the wild jgit needs to conform,
> as git-shell and all git-shell work-a-likes such as gitosis may be
> following the same convention.
> 
> Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
> ---
> 
>  If there are no arguments I'll push this into the public tree.
>  It seems right on the surface, and is necessary to use jgit against
>  repo.or.cz, and probably many other sites like it.

Seems ok and works here. Error handling still has a paperbagish feel. See
follow up patches.

Maybe we should have a patch for git too so it will actually work with spaces in file names. What do people on Windows do? (those that actually get an SSH server up and running and sleep well overe it on that platform).

As for pushing and signing. One way is for you (Shawn) and me is to sign-off and push each other's patches. I pushed this one.

-- robin

^ permalink raw reply


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