Git development
 help / color / mirror / Atom feed
* Re: [PATCH] diffcore-rename: cache file deltas
From: Junio C Hamano @ 2007-09-25 21:29 UTC (permalink / raw)
  To: Jeff King; +Cc: Linus Torvalds, git
In-Reply-To: <20070925192941.GA8564@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> We find rename candidates by computing a fingerprint hash of
> each file, and then comparing those fingerprints. There are
> inherently O(n^2) comparisons, so it pays in CPU time to
> hoist the (rather expensive) computation of the fingerprint
> out of that loop (or to cache it once we have computed it once).
>
> Previously, we didn't keep the filespec information around
> because then we had the potential to consume a great deal of
> memory. However, instead of keeping all of the filespec
> data, we can instead just keep the fingerprint.
>
> This patch implements and uses diff_free_filespec_data_large
> to accomplish that goal. We also have to change
> estimate_similarity not to needlessly repopulate the
> filespec data when we already have the hash.
>
> Practical tests showed 4.5x speedup for a 10% memory usage
> increase.
>
> Signed-off-by: Jeff King <peff@peff.net>

Very nice.

> The implementation is a little less nice than I would like, but I was
> trying to be non-invasive. Specifically:
>
>  - the name diff_free_filespec_data_large is horrible, but this is based
>    on the fact that diff_free_filespec_data actually does too much (it
>    frees the data _and_ some other auxiliary data). And renaming that
>    would entail changing many callsites.

True.  But we can rename it to diff_file_filespec_blob() and
that would perfectly well describe what it does.  Will do so
when applying if it is Ok to you.

>  - It seems that a better place to call diffcore_populate_filespec
>    (rather than in estimate_similarity) would actually be in
>    diffcore_count_changes (when we _know_ that we need to populate the
>    contents data).
>
>  - The hash_chars() should arguably be tied into
>    diffcore_populate_filespec, which should have more of a "what
>    information do you want" interface. I.e., the "size_only" parameter
>    could be extended to a bitfield to say "populate this, and I need the
>    delta fingerprint, size, actual contents, etc". Then callers could
>    just use "populate" before looking at the filespec and it would
>    lazily load whatever they needed.

Both good points, but I agree with you that it is wise to do
that with a follow-up patch.

^ permalink raw reply

* Re: [PATCH] post-checkout hook, and related docs and tests
From: Junio C Hamano @ 2007-09-25 21:29 UTC (permalink / raw)
  To: Josh England; +Cc: git
In-Reply-To: <1190738473.6078.102.camel@beauty>

"Josh England" <jjengla@sandia.gov> writes:

> So this leads to my next question:  Should the post-merge patch be
> brought in under this same umbrella to form a single post-checkout hook,
> or should it stay a separate hook?

I think it is called would be inconvenient for the callee if you
call the same hook without telling the hook script why it is
called, so if you go in the unification route the caller of the
unified hook needs to supply an extra parameter and existing
hooks if any need to be updated --- neither sounds like a very
idea.  The writer of the hooks however can choose to call one
from the other if he wants the same action for both hooks, so it
looks to me that separate hooks for separate purposes is the way
to go.

^ permalink raw reply

* Re: [PATCH] diffcore-rename: cache file deltas
From: Jeff King @ 2007-09-25 21:42 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Linus Torvalds, git
In-Reply-To: <7vy7eu4eos.fsf@gitster.siamese.dyndns.org>

On Tue, Sep 25, 2007 at 02:29:07PM -0700, Junio C Hamano wrote:

> Very nice.

Thank you.

> True.  But we can rename it to diff_file_filespec_blob() and
> that would perfectly well describe what it does.  Will do so
> when applying if it is Ok to you.

Yes, that is a much better name (assuming s/file/free). Please do change
it.

> >  - The hash_chars() should arguably be tied into
> >    diffcore_populate_filespec, which should have more of a "what
> 
> Both good points, but I agree with you that it is wise to do
> that with a follow-up patch.

I think I will leave it, unless you are excited about the change. I feel
like there is a high chance of introducing some bug, and the benefit is
purely stylistic at this point (i.e., the diff code isn't really
changing much at this point, so I think it should just be left alone).

-Peff

^ permalink raw reply

* Re: [PATCH] post-checkout hook, and related docs and tests
From: Josh England @ 2007-09-25 21:47 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v4phi5t98.fsf@gitster.siamese.dyndns.org>

On Tue, 2007-09-25 at 14:29 -0700, Junio C Hamano wrote:
> "Josh England" <jjengla@sandia.gov> writes:
> 
> > So this leads to my next question:  Should the post-merge patch be
> > brought in under this same umbrella to form a single post-checkout hook,
> > or should it stay a separate hook?
> 
> I think it is called would be inconvenient for the callee if you
> call the same hook without telling the hook script why it is
> called, so if you go in the unification route the caller of the
> unified hook needs to supply an extra parameter and existing
> hooks if any need to be updated --- neither sounds like a very
> idea.  The writer of the hooks however can choose to call one
> from the other if he wants the same action for both hooks, so it
> looks to me that separate hooks for separate purposes is the way
> to go.

Yeah, I agree.  I'll rework post-checkout and send in both again.

-JE

^ permalink raw reply

* Re: [PATCH 0/5] gitweb: Fixes and improvements related to diffs
From: Junio C Hamano @ 2007-09-25 22:16 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git
In-Reply-To: <8fe92b430709211431m7a93a11cp915988a57801edd6@mail.gmail.com>

"Jakub Narebski" <jnareb@gmail.com> writes:

> This series of patches adds fixes, simplifications and improvements
> related to diff handling in gitweb ('commit', 'commitdiff' views).
>
> Becaus I didn't have access to Internet, this series is based
> on 1e61b7640d09015213dbcae3564fa27ac6a8c151 (v1.5.3.1-1-g1e61b76).
> As I am sending it via GMail WWW interface, patches are send as
> attachements.
>
> I send this series now, even without good Internet access,
> because it contains two bugfixes, and one code fix. I'll try to
> resend it rebase on top of current master later...

I've parked the first two in 'master' and was hoping to roll
them into 'maint'.  I haven't taken a look at the other three as
I was waiting for a resend.  If you think some of them are
bugfix material for 'maint', please mark them as such.

Thanks.

^ permalink raw reply

* Re: [PATCH 0/5] gitweb: Fixes and improvements related to diffs
From: Jakub Narebski @ 2007-09-25 22:21 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vodfq4ch0.fsf@gitster.siamese.dyndns.org>

On 9/26/07, Junio C Hamano <gitster@pobox.com> wrote:
> "Jakub Narebski" <jnareb@gmail.com> writes:
>
> > This series of patches adds fixes, simplifications and improvements
> > related to diff handling in gitweb ('commit', 'commitdiff' views).
> >
> > Becaus I didn't have access to Internet, this series is based
> > on 1e61b7640d09015213dbcae3564fa27ac6a8c151 (v1.5.3.1-1-g1e61b76).
> > As I am sending it via GMail WWW interface, patches are send as
> > attachements.
> >
> > I send this series now, even without good Internet access,
> > because it contains two bugfixes, and one code fix. I'll try to
> > resend it rebase on top of current master later...
>
> I've parked the first two in 'master' and was hoping to roll
> them into 'maint'.  I haven't taken a look at the other three as
> I was waiting for a resend.  If you think some of them are
> bugfix material for 'maint', please mark them as such.

Third fixes a rare bug when gitweb screwed up names of files
in the patchset part of commitdiff view. This took place only
(diagnosed by pasky) when there were different files which
had the same preimage and the same postimage.

-- 
Jakub Narebski

^ permalink raw reply

* [PATCH] User Manual: document import-tars.perl
From: Miklos Vajna @ 2007-09-25 22:37 UTC (permalink / raw)
  To: J. Bruce Fields; +Cc: Junio C Hamano, git, Miklos Vajna
In-Reply-To: <20070925201306.GW30845@fieldses.org>

Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---

On Tue, Sep 25, 2007 at 04:13:06PM -0400, "J. Bruce Fields" <bfields@fieldses.org> wrote:
> One exception--the "series of tarballs" thing--I think it's cool that
> you can just unpack a bunch of tarballs and string them together into a
> git history.  It gives a good sense of how git works, and I don't think
> it's documented explicitly anywhere.  I think that might be kinda fun to
> write up.  But I haven't tried.

something like this?

VMiklos

 Documentation/user-manual.txt |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
index a085ca1..f722932 100644
--- a/Documentation/user-manual.txt
+++ b/Documentation/user-manual.txt
@@ -978,6 +978,19 @@ $ git add . # include everything below ./ in the first commit:
 $ git commit
 -------------------------------------------------
 
+If you already have a series of tarballs (typically previous releases without
+using a version control system):
+
+-------------------------------------------------
+$ mkdir project
+$ cd project
+$ git init
+$ perl import-tars.perl /path/to/tarballs/*.tar.bz2
+$ git checkout import-tars
+-------------------------------------------------
+
+You can find `import-tars.perl` in the `contrib/fast-import/` directory.
+
 [[how-to-make-a-commit]]
 How to make a commit
 --------------------
-- 
1.5.3.2.80.g077d6f-dirty

^ permalink raw reply related

* [PATCH] Add ability to specify SMTP server port when using git-send-email.
From: Glenn Rempe @ 2007-09-25 22:38 UTC (permalink / raw)
  To: git; +Cc: Glenn Rempe

Add ability to specify custom SMTP server port using
smtpserverport config value or --smtp-server-port command
line option.

Ability to specify custom SMTP server port using
--smtp-server host:port syntax.

Will default to port 25 if smtpssl config is set
to false or --smtp-ssl command line is unset and port
is not explicitly defined.

Will default to port 465 if smtpssl config is set
to true or --smtp-ssl is set and port is not explicity
defined.

Users should be aware that sending auth info over non-ssl
connections may be unsafe or just may not work at all
depending on SMTP server config.

Added some negative test cases.

Signed-off-by: Glenn Rempe <glenn@rempe.us>
---
 git-send-email.perl   |   73 ++++++++++++++++++++++++++++++++++++++++---------
 t/t9001-send-email.sh |   12 ++++++++
 2 files changed, 72 insertions(+), 13 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 4031e86..969cb39 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -77,7 +77,10 @@ Options:
                   the default section.
 
    --smtp-server  If set, specifies the outgoing SMTP server to use.
-                  Defaults to localhost.
+                  Defaults to localhost.  Port number can be specified here with
+                  hostname:port format or by using --smtp-server-port option.
+
+   --smtp-server-port Specify a port on the outgoing SMTP server to connect to.
 
    --smtp-user    The username for SMTP-AUTH.
 
@@ -172,8 +175,8 @@ my ($quiet, $dry_run) = (0, 0);
 
 # Variables with corresponding config settings
 my ($thread, $chain_reply_to, $suppress_from, $signed_off_cc, $cc_cmd);
-my ($smtp_server, $smtp_authuser, $smtp_authpass, $smtp_ssl);
-my ($identity, $aliasfiletype, @alias_files);
+my ($smtp_server, $smtp_server_port, $smtp_authuser, $smtp_authpass, $smtp_ssl);
+my ($identity, $aliasfiletype, @alias_files, @smtp_host_parts);
 
 my %config_bool_settings = (
     "thread" => [\$thread, 1],
@@ -185,6 +188,7 @@ my %config_bool_settings = (
 
 my %config_settings = (
     "smtpserver" => \$smtp_server,
+    "smtpserverport" => \$smtp_server_port,
     "smtpuser" => \$smtp_authuser,
     "smtppass" => \$smtp_authpass,
     "cccmd" => \$cc_cmd,
@@ -204,6 +208,7 @@ my $rc = GetOptions("sender|from=s" => \$sender,
 		    "bcc=s" => \@bcclist,
 		    "chain-reply-to!" => \$chain_reply_to,
 		    "smtp-server=s" => \$smtp_server,
+		    "smtp-server-port=s" => \$smtp_server_port,
 		    "smtp-user=s" => \$smtp_authuser,
 		    "smtp-pass=s" => \$smtp_authpass,
 		    "smtp-ssl!" => \$smtp_ssl,
@@ -375,6 +380,29 @@ if (!defined $smtp_server) {
 	$smtp_server ||= 'localhost'; # could be 127.0.0.1, too... *shrug*
 }
 
+# don't allow BOTH forms of port definition to work since we can't guess which one is right.
+if (($smtp_server =~ /:\d+/) && (defined $smtp_server_port)) {
+  die "You must specify the port using either hostname:port OR --smtp-server-port but not both!"
+}
+
+# setup smtp_server var if it was passed in as host:port format
+if ( $smtp_server =~ /:\d+/) {
+  # if they do pass a host:port form then split it and use the parts
+  @smtp_host_parts = split(/:/, $smtp_server);
+  $smtp_server = $smtp_host_parts[0];
+  $smtp_server_port = $smtp_host_parts[1];
+}
+
+# setup reasonable defaults if neither host:port or --smtp-server-port were passed
+if ( !defined $smtp_server_port) {
+  if ($smtp_ssl) {
+    $smtp_server_port = 465  # SSL port
+  } else {
+    $smtp_server_port = 25  # Non-SSL port
+  }
+}
+
+
 if ($compose) {
 	# Note that this does not need to be secure, but we will make a small
 	# effort to have it be unique
@@ -602,22 +630,41 @@ X-Mailer: git-send-email $gitversion
 		print $sm "$header\n$message";
 		close $sm or die $?;
 	} else {
+
+		if (!defined $smtp_server) {
+		  die "The required SMTP server is not properly defined."
+		}
+
+		if (!defined $smtp_server_port || !$smtp_server_port =~ /^\d+$/ ) {
+		  die "The required SMTP server port is not properly defined."
+		}
+
 		if ($smtp_ssl) {
 			require Net::SMTP::SSL;
-			$smtp ||= Net::SMTP::SSL->new( $smtp_server, Port => 465 );
+			$smtp ||= Net::SMTP::SSL->new( $smtp_server, Port => $smtp_server_port );
 		}
 		else {
 			require Net::SMTP;
-			$smtp ||= Net::SMTP->new( $smtp_server );
+			$smtp ||= Net::SMTP->new($smtp_server . ":" . $smtp_server_port);
 		}
-		$smtp->auth( $smtp_authuser, $smtp_authpass )
-			or die $smtp->message if (defined $smtp_authuser);
-		$smtp->mail( $raw_from ) or die $smtp->message;
-		$smtp->to( @recipients ) or die $smtp->message;
-		$smtp->data or die $smtp->message;
-		$smtp->datasend("$header\n$message") or die $smtp->message;
-		$smtp->dataend() or die $smtp->message;
-		$smtp->ok or die "Failed to send $subject\n".$smtp->message;
+    
+    # we'll get an ugly error if $smtp was undefined above.
+    # If so we'll catch it and present something friendlier.
+    if (!$smtp) {
+      die "Unable to initialize SMTP properly.  Is there something wrong with your config?";
+    }
+
+    if ((defined $smtp_authuser) && (defined $smtp_authpass)) {
+      $smtp->auth( $smtp_authuser, $smtp_authpass ) or die $smtp->message;
+    }
+
+    $smtp->mail( $raw_from ) or die $smtp->message;
+    $smtp->to( @recipients ) or die $smtp->message;
+    $smtp->data or die $smtp->message;
+    $smtp->datasend("$header\n$message") or die $smtp->message;
+    $smtp->dataend() or die $smtp->message;
+    $smtp->ok or die "Failed to send $subject\n".$smtp->message;
+
 	}
 	if ($quiet) {
 		printf (($dry_run ? "Dry-" : "")."Sent %s\n", $subject);
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 83f9470..d32907d 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -41,4 +41,16 @@ test_expect_success \
     'Verify commandline' \
     'diff commandline expected'
 
+test_expect_failure 'Passing in both host:port form AND --smtp-server-port' '
+  git send-email --from="Example <nobody@example.com>" --to=nobody@example.com --smtp-server smtp.foo.com:66 --smtp-server-port 77" $patches 2>errors
+'
+
+test_expect_failure 'Passing in non-numeric server port with host:port form' '
+  git send-email --from="Example <nobody@example.com>" --to=nobody@example.com --smtp-server smtp.foo.com:bar" $patches 2>errors
+'
+
+test_expect_failure 'Passing in non-numeric server port with --smtp-server-port form' '
+  git send-email --from="Example <nobody@example.com>" --to=nobody@example.com --smtp-server smtp.foo.com --smtp-server-port bar" $patches 2>errors
+'
+
 test_done
-- 
1.5.3.2.105.g23fe

^ permalink raw reply related

* Re: Workflow question
From: Andreas Ericsson @ 2007-09-25 22:38 UTC (permalink / raw)
  To: Russ Brown; +Cc: git
In-Reply-To: <46F96493.8000607@gmail.com>

Russ Brown wrote:
> Andreas Ericsson wrote:
>> Russ Brown wrote:
>>> Hi,
>>>
>>> I've been trying to think of a git workflow that we could use to replace
>>> our current svn/svk setup without simply using git in exactly the same
>>> way that we use svn/svk.
>>>
>>> Basically, we develop, maintain and enhance a website. On the central
>>> repo is trunk which represents live, and any number of project branches.
>>> Developers don't use local branches: they check out the project branches
>>> they're working on and commit to those. Developers merge from trunk to
>>> project branch from time to time to keep them current, and when a
>>> project rolls out the branch is merged to trunk.
>>>
>>> In addition to the obvious advantages that git would give us (such as
>>> properly tracking that code author as opposed to the person who did the
>>> merge), I'm wanting to gain the following benefits:
>>>
>>>  * The repository is very large (multiple gigabytes) and mirroring using
>>> svk obviously takes a lot of time and space, so I'm keen to bring that
>>> down, most likely by the developer not needing to mirror branches he
>>> doesn't care about, or by being able to throw away branches he's done
>>> with.
>>>  * The repository is full of revisions that fail review (or break
>>> things) and are fixed by subsequent revisions. We'd much rather be able
>>> to have the developer fix his revisions before they get committed
>>> 'upstream' (whatever that ends up meaning).
>>>
>>> I asked earlier about the email-based model that git itself uses, and
>>> while it appears to work very well for a widely-dispersed open-source
>>> project, I think it will be too cumbersome and slow for a fast-paced
>>> internal development team who make a number of live releases every day.
>>>
>> We came to the same conclusion at our workplace. Email works great, but
>> it's faster and better to just walk over to your colleague and ask what
>> he/she thinks about something.
>>
> 
> Very true, with the minor exception that at my place there are
> developers working at different sites, so the walk-over method will only
> work for specific subsets of the team. :)
> 

So do what Peff suggested and use emails for code-review and git-pull/push
for shoveling the code around. He's got some valid points. Where I work,
there's mostly three or at most four developers at any one team, so paired
programming or walking over works quite well. Otherwise we'd likely use the
patch-emails-for-review thing.


> Here's a question: is the creation and deletion of a branch also version
> controlled as it is in Subversion? In other words, if I create a branch,
> develop on it and delete it without merging it anywhere, will the
> revisions hang around and get pulled down by future people cloning the
> repository, or do they get thrown away?
> 

They don't get thrown away unless you garbage-collect them, using git-gc,
but they won't get propagated unless they can't be reached from a 'ref'
or a ref's log. The refs that can be propagated between repos are tags
and branches. There are others too, but they are less user-visible and
oftentimes more for housekeeping than anything else.

>>
>>>  4. Developers create a local branch of the project they
>>> are working on and commit to that.
>>>  5. Once they think they're done, they publish their branch to the
>>> development repo and request for comments.
>> Using topic-branches is a much better strategy, usually, since that
>> allows each feature to be evaluated and improved on on its own, rather
>> than having to merge *all* of a particular developers changes just to
>> get desirable feature X. Note that cherry-pick provides ways of doing
>> that anyways, but in a much less elegant way, and your integrator/
>> release engineer will likely tear his hair out on a daily basis without
>> topic branches.
>>
> 
> I've seen the term 'topic-branch' used here quite a bit but it's
> unfamiliar to me. It is basically synonymous with what we call a
> 'project branch'? i.e. Management decide that feature X is required so
> we create a branch to develop it on which all developers on the project
> commit to.
> 

Pretty much, yes. 'feature branch' and 'topic branch' are interchangeable.

> Note that in step 4 above I mean the developer takes a local branch of
> the topic branch. For example, we start projectX/main, and create branch
> projectX on the shared repo. Developer 'jeff' works on the project and
> so creates local branch projectX/jeff and begins work. In step 5 they
> push this local branch to the shared repo so everyone can see it
> (alternative to the 'walk-over' method or emailing). Note that all
> changes jeff making in projectX/jeff are specific to the project branch,
> so he can rebase against other changes that get committed to that
> project branch.
> 
> If colleagues don't like his changes he can create projectXjeff2 and try
> again.
> 
> Jeff can also have other local branches to keep separate changes he is
> making on the other projects he is involved in.
> 
> That's how I'd thought of it happening...
> 

Ye, that sounds about right.

>>>  6. If all is not well, the developer creates a new local branch and
>>> moves good revisions from his previous one to the new one, modifying
>>> things as he goes, and republishes his new branch.
>>>  7. If all is well, their works gets merged or rebased onto the main
>>> project branch, and once that's ready it gets pushed to the master and
>>> rolled to live. The developer's individual branches get deleted from the
>>> dev repo since they're no longer required.
>> Topic branches would work the same, basically, except they can be pushed up
>> for review a lot faster.
>>
>> If all the pushing gets cumbersome, it also makes it easy to send the
>> patches
>> out as emails for discussion. It's usually easier to let git handle the
>> actual code transmissions, but discussing patches in emails works quite
>> well if it's intended for a wider audience.
>>
> 
> Yes, I am going to experiment with this a little too to see just how
> much work it would involve for the developers (if it's too much they
> won't do it) :)
> 

It's a slightly steeper starting cost, but the maintenance costs go way
down, so the developers can spend more of their time doing fun and new
stuff rather than figuring out how to get their already-ancient fixes
merged to the master-branch.

>>>  8. From time to time the master branch gets merged to the project
>>> branches. Developer's local branches can be rebased against the project
>>> branch as they please.
>>>
>> criss-cross merging can turn kinda nasty though, as you may have a hard
>> time
>> finding *the* common point when you run into that rogue merge with conflict
>> markers everywhere (it happens for everyone sooner or later).
>>
>> I'd suggest you rebase the developer/topic branches onto master with
>> regular
>> intervals instead.
>>
> 
> Having been using git-svn for a while I really like the clean history
> result that rebase gives, however my understanding was that you should
> never rebase any published branch as it could screw up clones of that
> branch. In fact, this is what has me the most confused: how to rebase a
> project branch that is on a shared repository against master when
> everyone will have it cloned? Or is this something that I clearly don't
> understand properly?
> 

Well, afaiu you want every developer to have his own topic-branch, in
which case they can simply rebase those topic-branches onto master,
which will make sure they always know they're conflict-free with the
latest bugfixes and whatnot.

They don't really have to push their changes upstream until they're
ready for review.

> (Thanks for your answers BTW Andreas)
> 

np. Happy to help :)

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

^ permalink raw reply

* Re: History over-simplification
From: Junio C Hamano @ 2007-09-25 22:42 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Linus Torvalds, git
In-Reply-To: <20070923044628.GA3099@spearce.org>

"Shawn O. Pearce" <spearce@spearce.org> writes:

> I don't really like the patch to revision.c because it winds up
> showing trivial merges too.  What I really want is to have the
> "--full-history" option include a merge if either of the following
> is true:
>
>  a) The resulting path does not match _any_ of the parents.  We
>     already handle this case correctly in revision.c and correctly
>     show the "evil" merge.
>
>  b) The resulting path matches one of the parents but not one of
>     the others.  In such a case the merge should still be output if
>     a 3-way read-tree would not have chosen this result by default.

I am not sure (b) is useful in general.  Merging two branches
that fix the same issue but in different ways (think: 'maint'
and 'master' have different infrastructure and a fix initially
made on 'master' was backported to 'maint', and then later
'maint' needed to be merged to 'master' to carry forward other
fixes) is a norm, and in such cases taking the version from the
merged-to branch is almost always what happens.

Also it sounds to me by "if read-tree would not have chosen this
result by default" you mean this feature would not just need to
run merge-base but also recursive merge-base synthesis, and also
recreate the structural merge (aka "rename detection") there as
well.  Even if (b) is useful, it sounds like a very expensive
option, and the current merge-recursive code is structured in
such a way to be easily reused for this purpose.

^ permalink raw reply

* Git-tag's over-aggressive pipework fails badly on fresh Ubuntu, over NFS.
From: Samium Gromoff @ 2007-09-25 22:34 UTC (permalink / raw)
  To: git

Hello folks,

Without the following applied, git-tag -s gets zeroes into TAG_TMP
in place of the .asc sig, resulting in an unsigned commit.

My repository is located on a NFS share, and I didn't check with NFS factored
out, so it might be more extensive.

I'm not sure what's the policy -- "go and fix Ubuntu, your filesystem
and your kernel", or "let's go the safe way", so there it is.

--- a/git-tag   2007-09-07 22:12:11.000000000 +0400
+++ b/git-tag   2007-09-26 02:19:42.000000000 +0400
@@ -143,7 +143,8 @@
     rm -f "$GIT_DIR"/TAG_TMP.asc "$GIT_DIR"/TAG_FINALMSG
     if [ "$signed" ]; then
        gpg -bsa -u "$username" "$GIT_DIR"/TAG_TMP &&
-       cat "$GIT_DIR"/TAG_TMP.asc >>"$GIT_DIR"/TAG_TMP ||
+       cat "$GIT_DIR"/TAG_TMP "$GIT_DIR"/TAG_TMP.asc > "$GIT_DIR"/TAG_TMP_RELAY &&
+       mv "$GIT_DIR"/TAG_TMP_RELAY "$GIT_DIR"/TAG_TMP ||
        die "failed to sign the tag with GPG."
     fi
     object=$(git-mktag < "$GIT_DIR"/TAG_TMP)

Git is 1.5.2.5-2build1/Gutsy.

regards, Samium Gromoff

^ permalink raw reply

* [PATCH] Documentation for post-checkout and post-merge hooks
From: Josh England @ 2007-09-25 22:49 UTC (permalink / raw)
  To: git; +Cc: Josh England
In-Reply-To: <1190760563-32453-2-git-send-email-jjengla@sandia.gov>

Signed-off-by: Josh England <jjengla@sandia.gov>
---
 Documentation/hooks.txt |   25 +++++++++++++++++++++++++
 1 files changed, 25 insertions(+), 0 deletions(-)

diff --git a/Documentation/hooks.txt b/Documentation/hooks.txt
index c39edc5..37a3e5a 100644
--- a/Documentation/hooks.txt
+++ b/Documentation/hooks.txt
@@ -87,6 +87,31 @@ parameter, and is invoked after a commit is made.
 This hook is meant primarily for notification, and cannot affect
 the outcome of `git-commit`.
 
+post-checkout
+-----------
+
+This hook is invoked when a `git-checkout` is run after having updated the
+worktree.  The hook is given two parameters: the ref of the previous HEAD and
+the ref of the new HEAD, which may or may not have changed.  This hook cannot
+affect the outcome of `git-checkout`.
+
+This hook can be used to perform repository validity checks, auto-display
+differences from the previous HEAD if different, or set working dir metadata
+properties.
+
+post-merge
+-----------
+
+This hook is invoked by `git-merge`, which happens when a `git pull`
+is done on a local repository.  The hook takes a single parameter, a status
+flag specifying whether or not the merge being done was a squash merge.
+This hook cannot affect the outcome of `git-merge`.
+
+This hook can be used in conjunction with a corresponding pre-commit hook to
+save and restore any form of metadata associated with the working tree
+(eg: permissions/ownership, ACLS, etc).  See contrib/hooks/setgitperms.pl
+for an example of how to do this.
+
 [[pre-receive]]
 pre-receive
 -----------
-- 
1.5.3.2.89.g296e

^ permalink raw reply related

* [PATCH] post-merge hook and related tests
From: Josh England @ 2007-09-25 22:49 UTC (permalink / raw)
  To: git; +Cc: Josh England
In-Reply-To: <1190760563-32453-1-git-send-email-jjengla@sandia.gov>

Signed-off-by: Josh England <jjengla@sandia.gov>
---
 git-merge.sh               |   13 ++++++++++
 t/t5402-post-merge-hook.sh |   56 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 69 insertions(+), 0 deletions(-)
 create mode 100644 t/t5402-post-merge-hook.sh

diff --git a/git-merge.sh b/git-merge.sh
index 3a01db0..66e48b3 100755
--- a/git-merge.sh
+++ b/git-merge.sh
@@ -97,6 +97,19 @@ finish () {
 		fi
 		;;
 	esac
+
+	# Run a post-merge hook
+        if test -x "$GIT_DIR"/hooks/post-merge
+        then
+	    case "$squash" in
+	    t)
+                "$GIT_DIR"/hooks/post-merge 1
+		;;
+	    '')
+                "$GIT_DIR"/hooks/post-merge 0
+		;;
+	    esac
+        fi
 }
 
 merge_name () {
diff --git a/t/t5402-post-merge-hook.sh b/t/t5402-post-merge-hook.sh
new file mode 100644
index 0000000..2a7a097
--- /dev/null
+++ b/t/t5402-post-merge-hook.sh
@@ -0,0 +1,56 @@
+#!/bin/sh
+#
+# Copyright (c) 2006 Josh England
+#
+
+test_description='Test the post-merge hook.'
+. ./test-lib.sh
+
+test_expect_success setup '
+	echo Data for commit0. >a &&
+	git update-index --add a &&
+	tree0=$(git write-tree) &&
+	commit0=$(echo setup | git commit-tree $tree0) &&
+	echo Changed data for commit1. >a &&
+	git update-index a &&
+	tree1=$(git write-tree) &&
+	commit1=$(echo modify | git commit-tree $tree1 -p $commit0) &&
+        git update-ref refs/heads/master $commit0 &&     
+	git-clone ./. clone1 &&
+	GIT_DIR=clone1/.git git update-index --add a &&
+	git-clone ./. clone2 &&
+	GIT_DIR=clone2/.git git update-index --add a
+'
+
+for clone in 1 2; do 
+    cat >clone${clone}/.git/hooks/post-merge <<'EOF'
+#!/bin/sh
+echo $@ >> $GIT_DIR/post-merge.args
+EOF
+    chmod u+x clone${clone}/.git/hooks/post-merge
+done
+
+test_expect_failure 'post-merge does not run for up-to-date ' '
+        GIT_DIR=clone1/.git git merge $commit0 &&
+ 	test -e clone1/.git/post-merge.args
+'
+
+test_expect_success 'post-merge runs as expected ' '
+        GIT_DIR=clone1/.git git merge $commit1 &&
+ 	test -e clone1/.git/post-merge.args
+'
+
+test_expect_success 'post-merge from normal merge receives the right argument ' '
+        grep 0 clone1/.git/post-merge.args
+'
+
+test_expect_success 'post-merge from squash merge runs as expected ' '
+        GIT_DIR=clone2/.git git merge --squash $commit1 &&
+ 	test -e clone2/.git/post-merge.args
+'
+
+test_expect_success 'post-merge from squash merge receives the right argument ' '
+        grep 1 clone2/.git/post-merge.args
+'
+
+test_done
-- 
1.5.3.2.89.g296e

^ permalink raw reply related

* [PATCH] post-checkout hooks and related tests
From: Josh England @ 2007-09-25 22:49 UTC (permalink / raw)
  To: git; +Cc: Josh England

Signed-off-by: Josh England <jjengla@sandia.gov>
---
 git-checkout.sh               |   12 +++++++
 t/t5403-post-checkout-hook.sh |   70 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 82 insertions(+), 0 deletions(-)
 create mode 100755 t/t5403-post-checkout-hook.sh

diff --git a/git-checkout.sh b/git-checkout.sh
index 17f4392..78355eb 100755
--- a/git-checkout.sh
+++ b/git-checkout.sh
@@ -137,6 +137,13 @@ Did you intend to checkout '$@' which can not be resolved as commit?"
 	git ls-files --error-unmatch -- "$@" >/dev/null || exit
 	git ls-files -- "$@" |
 	git checkout-index -f -u --stdin
+
+        # Run a post-checkout hook -- the HEAD does not change so the
+	# current HEAD is passed in for both args
+	if test -x "$GIT_DIR"/hooks/post-checkout; then
+	    "$GIT_DIR"/hooks/post-checkout $old $old
+	fi
+
 	exit $?
 else
 	# Make sure we did not fall back on $arg^{tree} codepath
@@ -284,3 +291,8 @@ if [ "$?" -eq 0 ]; then
 else
 	exit 1
 fi
+
+# Run a post-checkout hook
+if test -x "$GIT_DIR"/hooks/post-checkout; then
+        "$GIT_DIR"/hooks/post-checkout $old $new
+fi
diff --git a/t/t5403-post-checkout-hook.sh b/t/t5403-post-checkout-hook.sh
new file mode 100755
index 0000000..663f8d7
--- /dev/null
+++ b/t/t5403-post-checkout-hook.sh
@@ -0,0 +1,70 @@
+#!/bin/sh
+#
+# Copyright (c) 2006 Josh England
+#
+
+test_description='Test the post-checkout hook.'
+. ./test-lib.sh
+
+test_expect_success setup '
+	echo Data for commit0. >a &&
+	echo Data for commit0. >b &&
+	git update-index --add a &&
+	git update-index --add b &&
+	tree0=$(git write-tree) &&
+	commit0=$(echo setup | git commit-tree $tree0) &&
+        git update-ref refs/heads/master $commit0 &&
+	git-clone ./. clone1 &&
+	git-clone ./. clone2 &&
+        GIT_DIR=clone2/.git git branch -a new2 &&
+       	echo Data for commit1. >clone2/b &&
+	GIT_DIR=clone2/.git git add clone2/b &&
+	GIT_DIR=clone2/.git git commit -m new2
+'
+
+for clone in 1 2; do 
+    cat >clone${clone}/.git/hooks/post-checkout <<'EOF'
+#!/bin/sh
+echo $@ > $GIT_DIR/post-checkout.args
+EOF
+    chmod u+x clone${clone}/.git/hooks/post-checkout
+done
+
+test_expect_success 'post-checkout runs as expected ' '
+        GIT_DIR=clone1/.git git checkout master &&
+ 	test -e clone1/.git/post-checkout.args
+'
+
+test_expect_success 'post-checkout receives the right arguments with HEAD unchanged ' '
+         old=$(awk "{print \$1}" clone1/.git/post-checkout.args) &&
+         new=$(awk "{print \$2}" clone1/.git/post-checkout.args) &&
+         test $old = $new
+'
+
+test_expect_success 'post-checkout runs as expected ' '
+        GIT_DIR=clone1/.git git checkout master &&
+ 	test -e clone1/.git/post-checkout.args
+'
+
+test_expect_success 'post-checkout args are correct with git checkout -b ' '
+        GIT_DIR=clone1/.git git checkout -b new1 &&
+        old=$(awk "{print \$1}" clone1/.git/post-checkout.args) &&
+        new=$(awk "{print \$2}" clone1/.git/post-checkout.args) &&
+        test $old = $new
+'
+
+test_expect_success 'post-checkout receives the right args with HEAD changed ' '
+        GIT_DIR=clone2/.git git checkout new2 &&
+        old=$(awk "{print \$1}" clone2/.git/post-checkout.args) &&
+        new=$(awk "{print \$2}" clone2/.git/post-checkout.args) &&
+        test $old != $new
+'
+
+test_expect_success 'post-checkout receives the right args when not switching branches ' '
+        GIT_DIR=clone2/.git git checkout master b &&
+        old=$(awk "{print \$1}" clone2/.git/post-checkout.args) &&
+        new=$(awk "{print \$2}" clone2/.git/post-checkout.args) &&
+        test $old == $new
+'
+
+test_done
-- 
1.5.3.2.89.g296e

^ permalink raw reply related

* Re: Git-tag's over-aggressive pipework fails badly on fresh Ubuntu, over NFS.
From: Johannes Schindelin @ 2007-09-25 23:01 UTC (permalink / raw)
  To: Samium Gromoff; +Cc: git
In-Reply-To: <87hclifk7i.wl@betelheise.deep.net>

Hi,

On Wed, 26 Sep 2007, Samium Gromoff wrote:

> Without the following applied, git-tag -s gets zeroes into TAG_TMP
> in place of the .asc sig, resulting in an unsigned commit.

You'll be delighted to hear that it is no longer necessary to patch 
git-tag.sh (git-tag is not the right place), since git-tag is now a 
builtin (without much in the way of pipes...)

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] Add ability to specify SMTP server port when using git-send-email.
From: Johannes Schindelin @ 2007-09-25 23:05 UTC (permalink / raw)
  To: Glenn Rempe; +Cc: git
In-Reply-To: <1190759927-19493-1-git-send-email-glenn@rempe.us>

Hi,

On Tue, 25 Sep 2007, Glenn Rempe wrote:

> +if (($smtp_server =~ /:\d+/) && (defined $smtp_server_port)) {

Not that I want to be a PITA, but this breaks down with IPv6, right?

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] Add ability to specify SMTP server port when using git-send-email.
From: Junio C Hamano @ 2007-09-25 23:12 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Glenn Rempe, git
In-Reply-To: <Pine.LNX.4.64.0709260004090.28395@racer.site>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> On Tue, 25 Sep 2007, Glenn Rempe wrote:
>
>> +if (($smtp_server =~ /:\d+/) && (defined $smtp_server_port)) {
>
> Not that I want to be a PITA, but this breaks down with IPv6, right?

Right.  Do we care about symbolic "server.addre.ss:smtp"
notation as well, I wonder?

^ permalink raw reply

* Re: [PATCH] post-checkout hooks and related tests
From: Junio C Hamano @ 2007-09-25 23:17 UTC (permalink / raw)
  To: Josh England; +Cc: git
In-Reply-To: <1190760563-32453-1-git-send-email-jjengla@sandia.gov>

"Josh England" <jjengla@sandia.gov> writes:

> diff --git a/git-checkout.sh b/git-checkout.sh
> index 17f4392..78355eb 100755
> --- a/git-checkout.sh
> +++ b/git-checkout.sh
> @@ -137,6 +137,13 @@ Did you intend to checkout '$@' which can not be resolved as commit?"
>  	git ls-files --error-unmatch -- "$@" >/dev/null || exit
>  	git ls-files -- "$@" |
>  	git checkout-index -f -u --stdin
> +
> +        # Run a post-checkout hook -- the HEAD does not change so the
> +	# current HEAD is passed in for both args
> +	if test -x "$GIT_DIR"/hooks/post-checkout; then

It is usually a good idea to view your patch in your MUA before
sending them out.

You will spot HT vs SP indentation inconsistencies right away.

HEAD did not change but don't you want to differenciate if the
checkout was from the index or from the HEAD?  If not why not?

^ permalink raw reply

* Re: [PATCH] post-merge hook and related tests
From: Junio C Hamano @ 2007-09-25 23:21 UTC (permalink / raw)
  To: Josh England; +Cc: git
In-Reply-To: <1190760563-32453-2-git-send-email-jjengla@sandia.gov>

"Josh England" <jjengla@sandia.gov> writes:

> Signed-off-by: Josh England <jjengla@sandia.gov>
> ---
>  git-merge.sh               |   13 ++++++++++
>  t/t5402-post-merge-hook.sh |   56 ++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 69 insertions(+), 0 deletions(-)
>  create mode 100644 t/t5402-post-merge-hook.sh

I think this is part of the 'master' for at least a few days, so
is a half of the Documentation patch.

^ permalink raw reply

* Re: [PATCH] Add ability to specify SMTP server port when using git-send-email.
From: Junio C Hamano @ 2007-09-25 23:25 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Glenn Rempe, git
In-Reply-To: <7vzlza2vcl.fsf@gitster.siamese.dyndns.org>

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

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
>> On Tue, 25 Sep 2007, Glenn Rempe wrote:
>>
>>> +if (($smtp_server =~ /:\d+/) && (defined $smtp_server_port)) {
>>
>> Not that I want to be a PITA, but this breaks down with IPv6, right?
>
> Right.  Do we care about symbolic "server.addre.ss:smtp"
> notation as well, I wonder?

Well, does it break?

BTW, I do not think we care about ":smtp"; it was a
tongue-in-cheek comment.

^ permalink raw reply

* Unable to connect to the server
From: Kaushalya Satharasinghe @ 2007-09-21  9:10 UTC (permalink / raw)
  To: git

Hi,

I want to checkout a git package. I installed git and when I give the
command
"/root/bin/git clone
git://git.kernel.org/pub/scm/linux/kernel/git/drzeus/mmc.git /home/Kaushalya/moreWork/git_test
"
I got the result as

Initialized empty Git repository
in /home/Kaushalya/moreWork/git_test/.git/
git.kernel.org[0: 140.211.167.38]: errno=Connection timed out
fatal: unable to connect a socket (Connection timed out)
fetch-pack from
'git://git.kernel.org/pub/scm/linux/kernel/git/drzeus/mmc.git' failed.

I can ping the 140.211.167.38. I am using Fedora Core 5 hosted through
vmware 5.5. Can anyone help me out here?

Thanks and regards,
-- 
Kaushalya Satharasinghe <kaushalyas@zone24x7.com>
Zone24x7

^ permalink raw reply

* Re: Unable to connect to the server
From: Johannes Schindelin @ 2007-09-25 23:51 UTC (permalink / raw)
  To: Kaushalya Satharasinghe; +Cc: git
In-Reply-To: <1190365837.5497.4.camel@localhost>

Hi,

On Fri, 21 Sep 2007, Kaushalya Satharasinghe wrote:

> I want to checkout a git package. I installed git and when I give the
> command
> "/root/bin/git clone
> git://git.kernel.org/pub/scm/linux/kernel/git/drzeus/mmc.git /home/Kaushalya/moreWork/git_test
> "
> I got the result as
> 
> Initialized empty Git repository
> in /home/Kaushalya/moreWork/git_test/.git/
> git.kernel.org[0: 140.211.167.38]: errno=Connection timed out
> fatal: unable to connect a socket (Connection timed out)
> fetch-pack from
> 'git://git.kernel.org/pub/scm/linux/kernel/git/drzeus/mmc.git' failed.
> 
> I can ping the 140.211.167.38. I am using Fedora Core 5 hosted through
> vmware 5.5. Can anyone help me out here?

You're probably quite a few hops away from that machine.  However, mmc.git 
is a fork of the kernel tree, so if you can get ahold of any other kernel 
repository (for example from repo.or.cz), you can easen the load by using 
the --reference option to git-clone.

Hth,
Dscho

^ permalink raw reply

* Re: Workflow question
From: Russ Brown @ 2007-09-26  0:01 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vabra5tah.fsf@gitster.siamese.dyndns.org>

Junio C Hamano wrote:
> Russ Brown <pickscrape@gmail.com> writes:
> 
>> I keep reading things similar to this and bit by bit I'm starting to get
>> it. :) I suppose this is one case in which it's definitely a
>> disadvantage to have a good understanding of svn before coming to git...
>>
>> <yoda>You must unlearn what you have learned</yoda>
> 
> You do not have to unlearn; if Jeff truly unlearned he wouldn't
> have spotted you were trapped in SVN mentality.  You just need
> to learn there could be other ways ;-).
> 

I suppose what I really mean is you need to stop assuming what you've
already learned. :)

>> If you delete a branch that has commits on it that aren't referenced by
>> any other branches, will those commits be removed by something like git
>> pack or git gc?
> 
> Yes, eventually.
> 
>> I suppose what has me the most confused is how a developer works with a
>> remote branch: I've come to understand that a developer should never
>> check out and work on a remote branch, and always create a local one and
>> work on that. If he does that using the above hierarchy, there then
>> becomes main->projectX->featureY->jeff_local_branch_of_featureY. Or is
>> is possible for a developer to work directory on a remote branch?
> 
> The statement in the last sentence does not make any sense.
> Remote is called remote because it is remote and supposed to be
> out of reach ;-)
> 

Ah. I think I was a little confused by the fact that git does let you
checkout remote branches, through I see that it does warn you about it
when you do it.

> More seriously, remotes are used as reference points so if you
> "work directly on them", you cannot use them as reference points
> any more; you defeat the sole purpose of existence of remotes.
> 
> You can work _without_ using remote tracking branches, but that
> is mostly for merge based workflow.  It appears that you are
> leaning towards rebase-heavy workflow, so I do not think it is
> applicable to your project.

Right, I think we're going to be aiming for that, though as I say I'm
going to be experimenting a bit to see how things work when using both
approaches.

Thanks.

-- 

Russ

^ permalink raw reply

* Re: [PATCH] Add ability to specify SMTP server port when using git-send-email.
From: Glenn Rempe @ 2007-09-26  0:23 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, Glenn Rempe, git
In-Reply-To: <7vmyva2uqd.fsf@gitster.siamese.dyndns.org>


On Sep 25, 2007, at 4:25 PM, Junio C Hamano wrote:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>>
>>> On Tue, 25 Sep 2007, Glenn Rempe wrote:
>>>
>>>> +if (($smtp_server =~ /:\d+/) && (defined $smtp_server_port)) {
>>>
>>> Not that I want to be a PITA, but this breaks down with IPv6, right?
>>
>> Right.  Do we care about symbolic "server.addre.ss:smtp"
>> notation as well, I wonder?
>
> Well, does it break?
>
> BTW, I do not think we care about ":smtp"; it was a
> tongue-in-cheek comment.

Unfortunately, I know little about IPv6 and whether this breaks IPv6  
addressing or not.  So I'll leave that question for others. Does the  
unpatched code work with IPv6?  Does anyone currently use it that way?

Junio, are you suggesting that I should remove the host:port form  
support entirely and leave only the --smtp-server port option as  
valid?  I kind of liked that this new method allows both forms of  
specifying port as equal citizens.  :-)  If you are suggesting that  
it be removed, I think we would have to reject the host:port form  
smtp-server addresses so we don't break when both --smtp- 
server=host:port and -smtp-ssl are provided. (which brings us back to  
the ipv6 question).  No?

^ permalink raw reply

* Re: [PATCH] Add ability to specify SMTP server port when using git-send-email.
From: Junio C Hamano @ 2007-09-26  0:27 UTC (permalink / raw)
  To: Glenn Rempe; +Cc: git
In-Reply-To: <1190759927-19493-1-git-send-email-glenn@rempe.us>

I'm inclined to do this on top of yours.

 * As you mentioned, ssmtp plus undocumented "server:port"
   syntax never worked.  There is no point supporting the
   combination.  Also I do not think it is worth additional
   lines of code to even say "server:port" plus --server-port
   option are not meant to be used together.

 * People who used the undocumented "server:port" syntax did not
   use the new --smtp-server-port option anyway.

 * If somebody goes over plain smtp specifies server without
   port, we can let the Net::SMTP to handle the default port.
   No need for _us_ to say that the default is 25.

 * I do not see much point insisting that port to be numeric; I
   do not know what Net::SMTP accepts, but if it accepts
   my.isp.com:smtp instead of my.isp.com:25, that is fine. This
   has the side effect of keeping people's existing set-up
   working.

 * The indentation was horrible.  Maybe your tabstop is set
   incorrectly?

 * As I am inclined not to insist on numeric port numbers,
   the additional tests become pointless.

The result is much simpler, and I think it is more readable.

---

 git-send-email.perl   |   64 +++++++++++++-----------------------------------
 t/t9001-send-email.sh |   12 ---------
 2 files changed, 18 insertions(+), 58 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 886f78f..62e1429 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -380,29 +380,6 @@ if (!defined $smtp_server) {
 	$smtp_server ||= 'localhost'; # could be 127.0.0.1, too... *shrug*
 }
 
-# don't allow BOTH forms of port definition to work since we can't guess which one is right.
-if (($smtp_server =~ /:\d+/) && (defined $smtp_server_port)) {
-  die "You must specify the port using either hostname:port OR --smtp-server-port but not both!"
-}
-
-# setup smtp_server var if it was passed in as host:port format
-if ( $smtp_server =~ /:\d+/) {
-  # if they do pass a host:port form then split it and use the parts
-  @smtp_host_parts = split(/:/, $smtp_server);
-  $smtp_server = $smtp_host_parts[0];
-  $smtp_server_port = $smtp_host_parts[1];
-}
-
-# setup reasonable defaults if neither host:port or --smtp-server-port were passed
-if ( !defined $smtp_server_port) {
-  if ($smtp_ssl) {
-    $smtp_server_port = 465  # SSL port
-  } else {
-    $smtp_server_port = 25  # Non-SSL port
-  }
-}
-
-
 if ($compose) {
 	# Note that this does not need to be secure, but we will make a small
 	# effort to have it be unique
@@ -632,39 +609,34 @@ X-Mailer: git-send-email $gitversion
 	} else {
 
 		if (!defined $smtp_server) {
-		  die "The required SMTP server is not properly defined."
-		}
-
-		if (!defined $smtp_server_port || !$smtp_server_port =~ /^\d+$/ ) {
-		  die "The required SMTP server port is not properly defined."
+			die "The required SMTP server is not properly defined."
 		}
 
 		if ($smtp_ssl) {
+			$smtp_server_port ||= 465; # ssmtp
 			require Net::SMTP::SSL;
-			$smtp ||= Net::SMTP::SSL->new( $smtp_server, Port => $smtp_server_port );
+			$smtp ||= Net::SMTP::SSL->new($smtp_server, Port => $smtp_server_port);
 		}
 		else {
 			require Net::SMTP;
-			$smtp ||= Net::SMTP->new($smtp_server . ":" . $smtp_server_port);
+			$smtp ||= Net::SMTP->new((defined $smtp_server_port)
+						 ? "$smtp_server:$smtp_server_port"
+						 : $smtp_server);
 		}
 
-    # we'll get an ugly error if $smtp was undefined above.
-    # If so we'll catch it and present something friendlier.
-    if (!$smtp) {
-      die "Unable to initialize SMTP properly.  Is there something wrong with your config?";
-    }
-
-    if ((defined $smtp_authuser) && (defined $smtp_authpass)) {
-      $smtp->auth( $smtp_authuser, $smtp_authpass ) or die $smtp->message;
-    }
-
-    $smtp->mail( $raw_from ) or die $smtp->message;
-    $smtp->to( @recipients ) or die $smtp->message;
-    $smtp->data or die $smtp->message;
-    $smtp->datasend("$header\n$message") or die $smtp->message;
-    $smtp->dataend() or die $smtp->message;
-    $smtp->ok or die "Failed to send $subject\n".$smtp->message;
+		if (!$smtp) {
+			die "Unable to initialize SMTP properly.  Is there something wrong with your config?";
+		}
 
+		if ((defined $smtp_authuser) && (defined $smtp_authpass)) {
+			$smtp->auth( $smtp_authuser, $smtp_authpass ) or die $smtp->message;
+		}
+		$smtp->mail( $raw_from ) or die $smtp->message;
+		$smtp->to( @recipients ) or die $smtp->message;
+		$smtp->data or die $smtp->message;
+		$smtp->datasend("$header\n$message") or die $smtp->message;
+		$smtp->dataend() or die $smtp->message;
+		$smtp->ok or die "Failed to send $subject\n".$smtp->message;
 	}
 	if ($quiet) {
 		printf (($dry_run ? "Dry-" : "")."Sent %s\n", $subject);
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index d32907d..83f9470 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -41,16 +41,4 @@ test_expect_success \
     'Verify commandline' \
     'diff commandline expected'
 
-test_expect_failure 'Passing in both host:port form AND --smtp-server-port' '
-  git send-email --from="Example <nobody@example.com>" --to=nobody@example.com --smtp-server smtp.foo.com:66 --smtp-server-port 77" $patches 2>errors
-'
-
-test_expect_failure 'Passing in non-numeric server port with host:port form' '
-  git send-email --from="Example <nobody@example.com>" --to=nobody@example.com --smtp-server smtp.foo.com:bar" $patches 2>errors
-'
-
-test_expect_failure 'Passing in non-numeric server port with --smtp-server-port form' '
-  git send-email --from="Example <nobody@example.com>" --to=nobody@example.com --smtp-server smtp.foo.com --smtp-server-port bar" $patches 2>errors
-'
-
 test_done

^ 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