Git development
 help / color / mirror / Atom feed
* Re: [PATCH 3/4] push: make non-fast-forward help message configurable
From: Jeff King @ 2009-09-09 11:22 UTC (permalink / raw)
  To: Uri Okrent
  Cc: Matthieu Moy, Nanako Shiraishi, Junio C Hamano, Teemu Likonen,
	Git
In-Reply-To: <4AA6A7A9.3080908@gmail.com>

On Tue, Sep 08, 2009 at 11:51:21AM -0700, Uri Okrent wrote:

> I know that by default I'd like to see new messages, and in case I'm
> doing something adventurous I'd like to see a message I may not have
> seen before. If I had turned off all messages because I got sick of
> seeing one or two in particular, then I'd never know.

Yeah, I think the 'advice.all' option, while clever, is not really
helping anyone. In my revised series, I am just omitting entirely (but
the code is still structured that adding it later would be very easy if
we change our minds).

> P.S. I never really introduced myself to the list... Uri Okrent here
> from L.A. Keep up the great work everyone!

Welcome. :)

-Peff

^ permalink raw reply

* Re: tracking branch for a rebase
From: Jeff King @ 2009-09-09 10:45 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Junio C Hamano, Björn Steinbrink, Michael J Gruber,
	Pete Wyckoff, git
In-Reply-To: <alpine.DEB.1.00.0909071126040.8306@pacific.mpi-cbg.de>

On Mon, Sep 07, 2009 at 11:29:50AM +0200, Johannes Schindelin wrote:

> > I think using @{} is a reasonable extension format.
> 
> Sorry to enter this thread that late, but I did not realize that it 
> touches my %<branch> work.
> 
> Your proposal leads to something like "master@{upstream}@{2.days.ago}", 
> which looks ugly.  And it is much more to type.
> 
> I still think that it is not too-much asked for to require the 
> "refs/heads/" prefix if somebody starts her branch names with "%".

I don't have a problem with restricting branch names starting with "%".
However, I do think "%.." is a bit ugly to read. And I am somewhat
concerned that we are eating the last reasonable available
meta-character for this feature, which will make things even harder next
time somebody suggests a clever feature. Which is why the discussion
turned to a generic extension syntax.

I wonder if it is worth adding @{upstream} now, which is fairly safe,
letting it cook for a while, and then adding a "%" alias later after the
concept has proved itself (and people say "I like this feature, but it
really is too much to type").

-Peff

^ permalink raw reply

* Re: [PATCH v2 3/3] INSTALL: Describe dependency knobs from Makefile
From: Junio C Hamano @ 2009-09-09  6:16 UTC (permalink / raw)
  To: Brian Gernhardt; +Cc: Git List
In-Reply-To: <1252461061-75840-4-git-send-email-brian@gernhardtsoftware.com>

Brian Gernhardt <brian@gernhardtsoftware.com> writes:

> +There are many options that can be configured in the makefile using either
> +command line defines or a config.mak file.  These options are documented at
> +the beginning of the Makefile.
> +

I did not like calling "make variables" "options", and also it was unclear
what good these "options" are for.  How about...

    The beginning of the Makefile documents many variables that affect the way
    git is built.  You can override them either from the command line, or in a
    config.mak file.

^ permalink raw reply

* Re: [PATCH v2 2/3] INSTALL: Reorder dependencies, split shell and Perl
From: Junio C Hamano @ 2009-09-09  6:01 UTC (permalink / raw)
  To: Brian Gernhardt; +Cc: Git List, Junio C Hamano
In-Reply-To: <1252461061-75840-3-git-send-email-brian@gernhardtsoftware.com>

Brian Gernhardt <brian@gernhardtsoftware.com> writes:

> The most important and non-optional dependencies should go first, so put
> them there.  While we're moving them, the descriptions for shell and perl
> were archaic, referring to "bare-bones Porcelainish scripts" that have
> become powerful and essential.
>
> Signed-off-by: Brian Gernhardt <brian@gernhardtsoftware.com>
> ---
>
>  Split from v1's 2/2 so the changes don't get confused with the re-order.
>
>  I kept the shell and perl re-wordings here since they're also somewhat of
>  a factual error instead of 3/3's additions.
>
>  INSTALL |   14 +++++++++-----
>  1 files changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/INSTALL b/INSTALL
> index 5267468..7ab2580 100644
> --- a/INSTALL
> +++ b/INSTALL
> @@ -52,6 +52,15 @@ Issues of note:
>  
>  	- "zlib", the compression library. Git won't build without it.
>  
> +	- "ssh" is used to push and pull over the net.
> +
> +	- A POSIX-compliant shell is required to run many scripts needed
> +	  for everyday use (e.g. "bisect", "pull").
>
> +	- "Perl" is needed to use some of the features (e.g. preparing a
> +	  partial commit using "git add -i/-p", interacting with svn
> +	  repositories with "git svn").
> +

This is a very minor point, but should we even make a distinction between
"scripts" vs "features" here?  It is a minor detail that "bisect" and
"pull" are implemented as scripts after all.  I dunno.

^ permalink raw reply

* [PATCH v2 3/3] INSTALL: Describe dependency knobs from Makefile
From: Brian Gernhardt @ 2009-09-09  1:51 UTC (permalink / raw)
  To: Git List; +Cc: Junio C Hamano
In-Reply-To: <1252461061-75840-1-git-send-email-brian@gernhardtsoftware.com>

We said that some of our dependencies were optional, but didn't say
how to turn them off.  Add information for that and mention where to
save the options close to the top of the file.

Also, standardize on both using quotes for the names of the dependencies
and tabs for indentation of the list.

Signed-off-by: Brian Gernhardt <brian@gernhardtsoftware.com>
---

 This has the most changes since v1, containing re-writes based on Junio's
 comments.  The paragraph changed in 1/1 gets changed again to include the
 Makefile switches and adapt to changes in the previous paragraph.

 I dropped the RFC since Junio seemed to appreciate the changes, but I kept
 the style changes here separate from the earlier factual changes both
 because I prefer to work that way and to allow the others to continue if
 discussion needs to continue on this one.

 INSTALL |   38 ++++++++++++++++++++++++--------------
 1 files changed, 24 insertions(+), 14 deletions(-)

diff --git a/INSTALL b/INSTALL
index 7ab2580..69c97b2 100644
--- a/INSTALL
+++ b/INSTALL
@@ -13,6 +13,10 @@ that uses $prefix, the built results have some paths encoded,
 which are derived from $prefix, so "make all; make prefix=/usr
 install" would not work.
 
+There are many options that can be configured in the makefile using either
+command line defines or a config.mak file.  These options are documented at
+the beginning of the Makefile.
+
 Alternatively you can use autoconf generated ./configure script to
 set up install paths (via config.mak.autogen), so you can write instead
 
@@ -48,7 +52,9 @@ Issues of note:
 	export GIT_EXEC_PATH PATH GITPERLLIB
 
  - Git is reasonably self-sufficient, but does depend on a few external
-   programs and libraries:
+   programs and libraries.  Git can be used without most of them by adding
+   the approriate "NO_<LIBRARY>=YesPlease" to the make command line or
+   config.mak file.
 
 	- "zlib", the compression library. Git won't build without it.
 
@@ -59,25 +65,29 @@ Issues of note:
 
 	- "Perl" is needed to use some of the features (e.g. preparing a
 	  partial commit using "git add -i/-p", interacting with svn
-	  repositories with "git svn").
+	  repositories with "git svn").  If you can live without these, use
+	  NO_PERL.
 
-	- "openssl".  Unless you specify otherwise, you'll get the SHA1
-	  library from here.
+	- "openssl" library is used by git-imap-send to use IMAP over SSL.
+	  If you don't need it, use NO_OPENSSL.
 
-	  If you don't have openssl, you can use one of the SHA1 libraries
-	  that come with git (git includes one inspired by Mozilla's and a
-	  PowerPC optimized one too - see the Makefile).
+	  By default, git uses OpenSSL for SHA1 but it will use it's own
+	  library (inspired by Mozilla's) with either NO_OPENSSL or
+	  BLK_SHA1.  Also included is a version optimized for PowerPC
+	  (PPC_SHA1).
 
-	- libcurl library; git-http-fetch and git-fetch use them.  You
+	- "libcurl" library is used by git-http-fetch and git-fetch.  You
 	  might also want the "curl" executable for debugging purposes.
-	  If you do not use http transfer, you are probably OK if you
-	  do not have them.
+	  If you do not use http:// or https:// repositories, you do not
+	  have to have them (use NO_CURL).
 
-	- expat library; git-http-push uses it for remote lock
-	  management over DAV.  Similar to "curl" above, this is optional.
+	- "expat" library; git-http-push uses it for remote lock
+	  management over DAV.  Similar to "curl" above, this is optional
+	  (with NO_EXPAT).
 
-        - "wish", the Tcl/Tk windowing shell is used in gitk to show the
-          history graphically, and in git-gui.
+	- "wish", the Tcl/Tk windowing shell is used in gitk to show the
+	  history graphically, and in git-gui.  If you don't want gitk or
+	  git-gui, you can use NO_TCLTK.
 
  - Some platform specific issues are dealt with Makefile rules,
    but depending on your specific installation, you may not
-- 
1.6.4.2.420.g30ecf

^ permalink raw reply related

* [PATCH v2 1/3] INSTALL: Update description of our SHA-1 code
From: Brian Gernhardt @ 2009-09-09  1:50 UTC (permalink / raw)
  To: Git List; +Cc: Junio C Hamano
In-Reply-To: <1252461061-75840-1-git-send-email-brian@gernhardtsoftware.com>

We haven't had Mozilla's code or an ARM optimized algorithm since
30ae47b.  Reword the paragraph to give credit but not authorship to
Mozilla.

Signed-off-by: Brian Gernhardt <brian@gernhardtsoftware.com>
---

 The only change from v1 here is "based on" -> "inspired by".

 INSTALL |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/INSTALL b/INSTALL
index ae7f750..5267468 100644
--- a/INSTALL
+++ b/INSTALL
@@ -56,8 +56,8 @@ Issues of note:
 	  library from here.
 
 	  If you don't have openssl, you can use one of the SHA1 libraries
-	  that come with git (git includes the one from Mozilla, and has
-	  its own PowerPC and ARM optimized ones too - see the Makefile).
+	  that come with git (git includes one inspired by Mozilla's and a
+	  PowerPC optimized one too - see the Makefile).
 
 	- libcurl library; git-http-fetch and git-fetch use them.  You
 	  might also want the "curl" executable for debugging purposes.
-- 
1.6.4.2.420.g30ecf

^ permalink raw reply related

* [PATCH v2 2/3] INSTALL: Reorder dependencies, split shell and Perl
From: Brian Gernhardt @ 2009-09-09  1:51 UTC (permalink / raw)
  To: Git List; +Cc: Junio C Hamano
In-Reply-To: <1252461061-75840-1-git-send-email-brian@gernhardtsoftware.com>

The most important and non-optional dependencies should go first, so put
them there.  While we're moving them, the descriptions for shell and perl
were archaic, referring to "bare-bones Porcelainish scripts" that have
become powerful and essential.

Signed-off-by: Brian Gernhardt <brian@gernhardtsoftware.com>
---

 Split from v1's 2/2 so the changes don't get confused with the re-order.

 I kept the shell and perl re-wordings here since they're also somewhat of
 a factual error instead of 3/3's additions.

 INSTALL |   14 +++++++++-----
 1 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/INSTALL b/INSTALL
index 5267468..7ab2580 100644
--- a/INSTALL
+++ b/INSTALL
@@ -52,6 +52,15 @@ Issues of note:
 
 	- "zlib", the compression library. Git won't build without it.
 
+	- "ssh" is used to push and pull over the net.
+
+	- A POSIX-compliant shell is required to run many scripts needed
+	  for everyday use (e.g. "bisect", "pull").
+
+	- "Perl" is needed to use some of the features (e.g. preparing a
+	  partial commit using "git add -i/-p", interacting with svn
+	  repositories with "git svn").
+
 	- "openssl".  Unless you specify otherwise, you'll get the SHA1
 	  library from here.
 
@@ -70,11 +79,6 @@ Issues of note:
         - "wish", the Tcl/Tk windowing shell is used in gitk to show the
           history graphically, and in git-gui.
 
-	- "ssh" is used to push and pull over the net
-
-	- "perl" and POSIX-compliant shells are needed to use most of
-	  the bare-bones Porcelainish scripts.
-
  - Some platform specific issues are dealt with Makefile rules,
    but depending on your specific installation, you may not
    have all the libraries/tools needed, or you may have
-- 
1.6.4.2.420.g30ecf

^ permalink raw reply related

* [PATCH v2 0/3] INSTALL updates
From: Brian Gernhardt @ 2009-09-09  1:50 UTC (permalink / raw)
  To: Git List; +Cc: Junio C Hamano

I'm now up to a three patch series for this minor documentation update.
:-/

The first one simply removes the references to code that no longer exists
in our tree.  This paragraph will be changed again in 3/3 so this patch
could be dropped, but I left it since the factual error is a different
issue than the later enhancements.

The second patch mostly reorders the text so that later changes are more
obvious.  It does contain the changes Junio suggested regarding shell and
perl, since the old wording is archaic and confusing to new users.

The third contains the additions concerning the Makefile switches.  It has
been updated based on Junio's commends.

Brian Gernhardt (3):
  INSTALL: Update description of our SHA-1 code
  INSTALL: Reorder dependencies, split shell and Perl
  INSTALL: Describe dependency knobs from Makefile

 INSTALL |   48 +++++++++++++++++++++++++++++++-----------------
 1 files changed, 31 insertions(+), 17 deletions(-)

^ permalink raw reply

* Re: [PATCH] Makefile: Add NEEDS_CRYPTO_WITH_SSL
From: Brian Gernhardt @ 2009-09-09  0:40 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git List
In-Reply-To: <7vhbvdyysq.fsf@alter.siamese.dyndns.org>


On Sep 8, 2009, at 4:19 PM, Junio C Hamano wrote:

> The patch makes sense to me, but as the result, depending on platforms
> and configuration, we would use three variations when linking imap- 
> send
> with no NO_OPENSSL defined:
>
> * Both -lcrypto -lssl
> * Only -lssl
> * Only -lcrypto
>
> I wonder if we can simplify this in some way (not a 1.6.5 topic).
>
> I am suspecting that the reason we do not say "always both" is because
> depending on the vintage of OpenSSL one or the other is missing?

It just occurred to me that I have a Debian machine to test this on.   
I generally don't use my web server for testing, but testing a compile  
should be safe enough.

Sure enough, it compiles without this patch.  It may be a difference  
between the linkers.  It appears that ERR_error_string() is in  
libcrypto for both OS X and Debian and that libssl.{so,dylib} both  
have load commands for libcrypto.{so,dylib}, but OS X ld is pickier  
than Debian ld.  Odd.

I don't see a scenario where only -lcrypto would be used for imap- 
send, actually.  That would imply that we're using OpenSSL's SHA-1  
algorithm but not using it for IMAPS access.  SSL_WITH_CRYPTO is about  
needing -lssl when using -lcrypto for SHA-1 while CRYPTO_WITH_SSL is  
about needing -lcrypto when using -lssl for SSL.

The only simplification I can think of is to make  
NEEDS_CRYPTO_WITH_SSL and NEEDS_SSL_WITH_CRYPTO be the same, so that  
it will include both any time it includes either one.   
NEEDS_SSL_WITH_CRYPTO is set for UnixWare, SCO_SV, and OS X.  I only  
have the latter to test with so I don't know if the symmetry is true  
for the other two.

~~ Brian

^ permalink raw reply

* Re: [RFC PATCH 2/2] INSTALL: Describe a few knobs from the Makefile
From: Brian Gernhardt @ 2009-09-09  0:12 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git List
In-Reply-To: <7vy6opxjx8.fsf@alter.siamese.dyndns.org>


On Sep 8, 2009, at 4:26 PM, Junio C Hamano wrote:

> [rewrite suggestions snipped]

I'll re-roll this and the other INSTALL patch as per your suggested re- 
wordings.  I tried to add the minimum needed instead of changing  
things, but many of your suggestions make sense.  And since I'm in  
here anyway, might as well change them.

> Brian Gernhardt <brian@gernhardtsoftware.com> writes:
>> +	- "openssl".  Unless you specify otherwise (with NO_OPENSSL),
>> +	  you'll get the SHA1 library from here.
>
> It is not very clear what will be affected by disabling this.
>
> - SHA-1 is not used from OpenSSL, as stated;
> - imap-send won't be able to talk over SSL;
>
> Do we still able to walk https:// URLs?  If your cURL library is  
> linked
> with gnutls I think we can, but I never tried the combination.

Glancing at http*.[ch], it looks like we depend on curl's ability to  
speak https instead of using OpenSSL ourselves.

>> +	- "perl" is used for several scripts that are useful, but not
>> +	  required for git (e.g. "git add -i" and "git difftool").  If you
>> +	  don't need the *.perl scripts or the library contained in perl/,
>> +	  then use NO_PERL.
>
> I do not think moving "Perl" this low in the requirement level is  
> such a
> good idea, at least for now.  I'd suggest movign it back immediately  
> after
> POSIX-compliant shell, and would say something like this:
>
> 	- "Perl" is needed to use some of the features (e.g. preparing a
>          partial commit using "git add -i/-p", interacting with svn
>          repositories with "git svn").  If you can live without these,
>          say NO_PERL.
>
> Maybe they are Ruby, github, and general acceptance by many open  
> source
> projects these days, but it used to be that the initial entry points  
> to
> git were "git cvsimport" and "git svn" for a surprisingly large  
> number of
> people.

I didn't move it this low so much as not move it when I moved the  
POSIX shell requirement that we can't live without.  Although I use  
git-svn on a regular basis, I don't think of it as core, but you're  
right that it's one of the main tools for many users.  Will move.

~~ Brian

^ permalink raw reply

* Re: tracking branch for a rebase
From: Julian Phillips @ 2009-09-08 23:17 UTC (permalink / raw)
  To: Michael J Gruber
  Cc: Johannes Schindelin, Jeff King, Junio C Hamano,
	Björn Steinbrink, Pete Wyckoff, git
In-Reply-To: <4AA4D81A.1030203@drmicha.warpmail.net>

On Mon, 7 Sep 2009, Michael J Gruber wrote:

> Solution needs to be:
> - practical (short, clear)
> - not too brutal (on existing users of legal "exotic" refnames)
> - extensible (for later uses)

How about '/^' and 'ref/^'?  Since you can't create a ref ending / ...

-- 
Julian

  ---
When you ask a question, do you truly want to know the answer, or are you
merely flaunting your power?

   -- DMITRI HARKONNEN, Notes to My Sons

^ permalink raw reply

* Re: [PATCH] git.el: Use git-add-file for unmerged files, remove git-resolve-file
From: Junio C Hamano @ 2009-09-08 23:10 UTC (permalink / raw)
  To: Martin Nordholts; +Cc: git, Alexandre Julliard
In-Reply-To: <4AA026AC.10907@gmail.com>

Martin Nordholts <enselic@gmail.com> writes:

> Use `git-add-file' to mark unmerged files as resolved in the
> *git-status* buffer to be consistent with git's CLI instructions. Also
> remove `git-resolve-file' to make it clear that that "R" is a now a
> free keybinding.

I do not know all the details of how Emacs keybinding works, but I had an
impression that something-x sequence is triggered if you type something-X
and you do not have an explicit binding for something-X but you do have a
binding for something-x.

IOW, if I only have

	(define-key global-map "\C-xc" 'compile)

then both "\C-xc" and "\C-xC" runs "compile", but in addition to the
above if I also have

	(define-key global-map "\C-xC" 'grep-find)

then I can invoke these two commands with lower- and upper- case 'c/C'
after control-x.

If people have been relying on the historical behaviour that typing "R"
marked the path resolved (which may internally have been implemented with
whatever way), and if you are removing that binding, wouldn't that now
expose them to whatever happens to be bound to "r"?

And worse, it is bound to git-remove-file.  Shouldn't you at least rebind
the "R" to something safer, like beep?

Am I grossly misunderstanding how Emacs keybindings work?  Is the
experiment I did in the initial part of the message with the global-map
relying on some "magic" that only appears in global-map, and there is no
problem doing this in the map this patch is about?

> Signed-off-by: Martin Nordholts <martinn@src.gnome.org>
> Acked-by: Alexandre Julliard <julliard@winehq.org>
> ---
>  contrib/emacs/git.el |   13 +------------
>  1 files changed, 1 insertions(+), 12 deletions(-)
>
> diff --git a/contrib/emacs/git.el b/contrib/emacs/git.el
> index 8c70ad8..214930a 100644
> --- a/contrib/emacs/git.el
> +++ b/contrib/emacs/git.el
> @@ -1046,7 +1046,7 @@ The FILES list must be sorted."
>  (defun git-add-file ()
>    "Add marked file(s) to the index cache."
>    (interactive)
> -  (let ((files (git-get-filenames (git-marked-files-state 'unknown 'ignored))))
> +  (let ((files (git-get-filenames (git-marked-files-state 'unknown 'ignored 'unmerged))))
>      ;; FIXME: add support for directories
>      (unless files
>        (push (file-relative-name (read-file-name "File to add: " nil nil t)) files))
> @@ -1119,15 +1119,6 @@ The FILES list must be sorted."
>                (when buffer (with-current-buffer buffer (revert-buffer t t t)))))
>            (git-success-message "Reverted" names))))))
>  
> -(defun git-resolve-file ()
> -  "Resolve conflicts in marked file(s)."
> -  (interactive)
> -  (let ((files (git-get-filenames (git-marked-files-state 'unmerged))))
> -    (when files
> -      (when (apply 'git-call-process-display-error "update-index" "--" files)
> -        (git-update-status-files files)
> -        (git-success-message "Resolved" files)))))
> -
>  (defun git-remove-handled ()
>    "Remove handled files from the status list."
>    (interactive)
> @@ -1556,7 +1547,6 @@ amended version of it."
>      (define-key map "P"   'git-prev-unmerged-file)
>      (define-key map "q"   'git-status-quit)
>      (define-key map "r"   'git-remove-file)
> -    (define-key map "R"   'git-resolve-file)
>      (define-key map "t"    toggle-map)
>      (define-key map "T"   'git-toggle-all-marks)
>      (define-key map "u"   'git-unmark-file)
> @@ -1598,7 +1588,6 @@ amended version of it."
>        ("Merge"
>  	["Next Unmerged File" git-next-unmerged-file t]
>  	["Prev Unmerged File" git-prev-unmerged-file t]
> -	["Mark as Resolved" git-resolve-file t]
>  	["Interactive Merge File" git-find-file-imerge t]
>  	["Diff Against Common Base File" git-diff-file-base t]
>  	["Diff Combined" git-diff-file-combined t]
> -- 
> 1.6.2.5

^ permalink raw reply

* Re: [StGit PATCH] Add import -p option
From: Karl Wiberg @ 2009-09-08 22:37 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: Gustav Hållberg, Git Mailing List
In-Reply-To: <b0943d9e0909081443g47978cb7mee241d74dc52adf7@mail.gmail.com>

On 2009-09-08 22:43:39 +0100, Catalin Marinas wrote:

> This patch renames some of the existing import options and adds the
> -p (--strip) option which allows stripping the leading slashes of
> the diff paths.

Looks good (and the intent is very good). The import test should
probably be augmented with a test case for -pN, though. (And since you
didn't have to change the test in this patch, presumably the two
options you renamed weren't tested either.)

> +    if strip:
> +        cmd += ['-p', strip]

This test should probably be "if strip != None". It doesn't _really_
matter, technically, since -p0 is the default, but still ...

-- 
Karl Wiberg, kha@treskal.com
    www.treskal.com/kalle

^ permalink raw reply

* Re: [PATCHv5 00/14] git notes
From: Johan Herland @ 2009-09-08 22:46 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: git, Junio C Hamano, trast, tavestbo, git, chriscool, spearce
In-Reply-To: <alpine.DEB.1.00.0909081741590.4330@intel-tinevez-2-302>

On Tuesday 08 September 2009, Johannes Schindelin wrote:
> On Tue, 8 Sep 2009, Johan Herland wrote:
> > On Tuesday 08 September 2009, Johannes Schindelin wrote:
> > > I can see that some people may think that date-based fan-out is the
> > > cat's ass, but I have to warn that we have no idea how notes will be
> > > used,
> >
> > I don't agree. Although we will certainly see many more use cases for
> > notes, I believe that the vast majority of them can be placed in one of
> > two categories:
> 
> My experience with Git is that having beliefs how my work is used was a
> constant source of surprise.

And you believe that a system that only allows SHA1-based fanout schemes is 
better equipped to tackle such surprises than a system that provides both 
date-based and SHA1-based fanout schemes?

> > > - I find the restriction to commits rather limiting.
> >
> > I see your point, but I don't agree until I see a compelling case for
> > annotating a non-commit.
> 
> My point is that it is too late by then, if you don't allow for a
>  flexible and still efficient scheme.

As I replied to Junio, we could use the epoch as a "commit date" for tree 
and blob objects, thus making them representable in a date-based fanout 
scheme (although if there are a signficant number of non-commit notes, the 
code should be smart enough to find a better (SHA1-based probably) fanout 
scheme for those notes).

> > > - most of the performance difference between the date-based and the
> > >   SHA-1 based fan-out looks to me as if the issue was the top-level
> > >   tree. Basically, this tree has to be read _every_ time _anybody_
> > >   wants to read a note.
> >
> > Not sure what you're trying to say here. The top-level notes tree is
> > read (as in fill_tree_descriptor()) exactly _once_. After that, it is
> > cached by the internal data structure (until free_commit_notes() or
> > end-of-process).
> 
> By that reasoning, we do not need any fan-out scheme.
> 
> Keep in mind: reading a large tree object takes a long time.  That's why
> we started fan-out.  Reading a large number of tree objects also takes a
> long time.  That's why I propagated flexible fan-out that is only read-in
> on demand.

Not sure where you're going with this. Of course we want to strike an 
optimal balance between the size of tree objects and the number of tree 
objects. Nobody is arguing about that. Both SHA1-based and (in the most 
common cases) date-based schemes can be used to achieve this balance. But 
using date-based fanout has the added advantage of providing better 
performance (both runtime- and memory-wise) when looking up notes in a 
chronological order.

> > > But I think that having a dynamic fan-out that can even put blobs
> > > into the top-level tree (nothing prevents us from doing that, right?)
> >
> > Well, the "flexible" code does add the new requirement that all entries
> > in a notes (sub)tree object must follow the same scheme, i.e. you
> > cannot have:
> >
> >   /12/34567890123456789012345678901234567890
> >   /2345/678901234567890123456789012345678901
> >
> > but you can have
> >
> >   /12/34567890123456789012345678901234567890
> >   /23/45/678901234567890123456789012345678901
> 
> Umm, why?  Is there any good technical reason?

In the date-based parts of notes tree, there are very good reasons for doing 
so: The code peeks at the first tree entry in order to determine what kind 
of date-based fanout is used in the current tree object. Subsequent entries 
(in that tree object) that do not follow the same format are 
skipped/ignored.

The SHA1-based fanout code has not changed since the last iteration, so this 
extra requirement is not absolutely necessary for the SHA1-based parts of 
the notes tree. However, the extra requirement does guarantee that commit 
notes have exactly one unique location in the notes tree, and thus relieves 
us of having to keep searching for alternative notes locations, and 
concatenate the notes found.

> > > The real question for me, therefore, is: what is the optimal way to
> > > strike the balance between size of the tree objects (which we want to
> > > be small, so that unpacking them is fast)  and depth of the fan-out
> > > (which we want to be shallow to avoid reading worst-case 39 tree
> > > objects to get at one single note).
> >
> > s/39/19/ (each fanout must use at least 2 chars of the 40-char SHA1)
> 
> That is another unnecessary restriction that could cost you dearly.  Just
> think what happens if it turns out that the optimal number of tree items
> is closer to 16 than to 255...

The code can easily be rewritten to allow for "odd" fanouts (1/39, 1/1/38, 
etc.). Feel free to submit a patch.

I was, however, naive enough to assume that when git.git decided on using 
2/38 fanout for its loose objects, then some performance-related thoughts
went into that decision. If there are indications that multiple-of-2-type 
fanouts are not optimal, we should probably reconsider.


Have fun! :)

...Johan

-- 
Johan Herland, <johan@herland.net>
www.herland.net

^ permalink raw reply

* Re: [PATCH 5/6 (v4)] full integration of rev-cache into git,  completed test suite
From: Nick Edelen @ 2009-09-08 22:24 UTC (permalink / raw)
  To: Sam Vilain
  Cc: Junio C Hamano, Nicolas Pitre, Johannes Schindelin,
	Michael J Gruber, Jeff King, Shawn O. Pearce, Andreas Ericsson,
	Christian Couder, git@vger.kernel.org
In-Reply-To: <1252357564.5969.4.camel@maia.lan>

> ^^ You don't need to write comments like "This patch"; in the history
> such words are meaningless.

I had meant to delete that...

> "tweak" ?

Yeah I had modified the messages and didn't replace the additional
info I deleted.  It's not really important, as it's modified again
(for the last time) in the name-related patch, but here it's revised
to take advantage of the size storage.

>>  - more fluid handling of damaged cache slices
>
> What does this mean?

That it remembers/is aware of bad slices, instead of dumbly attempting
to load them upon each commit.

> This is quite a long commit message.  Is the above detail all useful?
> Can it be split into one patch for each of the above integrations?

Erm, I suppose they could be split, but the changes to revision and
list-objects aren't very big, so I figured it'd be easier/cleaner to
just put everything required for smooth integration into a single
patch.  I dunno, it dosn't seem hugely necessary; the bits modifying
git code are relatively small and already obviously seperate in the
patch.

^ permalink raw reply

* Re: [PATCHv5 00/14] git notes
From: Sverre Rabbelier @ 2009-09-08 21:57 UTC (permalink / raw)
  To: Shawn O. Pearce
  Cc: Junio C Hamano, Johannes Schindelin, Johan Herland, git, trast,
	tavestbo, git, chriscool
In-Reply-To: <20090908213944.GX1033@spearce.org>

Heya,

On Tue, Sep 8, 2009 at 23:39, Shawn O. Pearce<spearce@spearce.org> wrote:
> Uh, but the natural way to index those is by commit, and each
> different revision of a change is a different commit.  Why delete
> the prior revision information and move it to the final commit note?

Ah, I didn't realize you would push the notes before the final revision is made.

> Someone who has the prior revisions in their reflog and is doing
> `git log -g --notes` might want to see that annotation.

It would make more sense to have multiple notes then, but wouldn't you
want them to annotate the original commit, rather than the final one?

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* Re: [PATCH] post-receive-email: do not call sendmail if no mail was generated
From: Junio C Hamano @ 2009-09-08 21:49 UTC (permalink / raw)
  To: Lars Noschinski; +Cc: Junio C Hamano, git, andyparkins, pape
In-Reply-To: <20090908205935.GB3858@lars.home.noschinski.de>

Lars Noschinski <lars-2008-2@usenet.noschinski.de> writes:

> Actually, there are a two cases in the case statement before, where
> generate_email would return:
>
>     refs/remotes/*,commit)
>         # tracking branch
>         refname_type="tracking branch"
>         short_refname=${refname##refs/remotes/}
>         echo >&2 "*** Push-update of tracking branch, $refname"
>         echo >&2 "***  - no email generated."
>         exit 0
>         ;;
>     *)
>         # Anything else (is there anything else?)
>         echo >&2 "*** Unknown type of update to $refname ($rev_type)"
>         echo >&2 "***  - no email generated"
>         exit 1
>         ;;

Ok, that justifies the existence of the patch.

^ permalink raw reply

* [StGit PATCH] Add import -p option
From: Catalin Marinas @ 2009-09-08 21:43 UTC (permalink / raw)
  To: Gustav Hållberg, Karl Hasselström, Git Mailing List

This patch renames some of the existing import options and adds the -p
(--strip) option which allows stripping the leading slashes of the diff
paths.

Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---
 stgit/commands/imprt.py |   13 ++++++++-----
 stgit/git.py            |    4 +++-
 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/stgit/commands/imprt.py b/stgit/commands/imprt.py
index 8067beb..0bbacbc 100644
--- a/stgit/commands/imprt.py
+++ b/stgit/commands/imprt.py
@@ -57,7 +57,9 @@ options = [
         short = 'Import a patch from a URL'),
     opt('-n', '--name',
         short = 'Use NAME as the patch name'),
-    opt('-t', '--strip', action = 'store_true',
+    opt('-p', '--strip', type = 'int', metavar = 'N',
+        short = 'Remove N leading slashes from diff paths (default 1)'),
+    opt('-t', '--stripname', action = 'store_true',
         short = 'Strip numbering and extension from patch name'),
     opt('-i', '--ignore', action = 'store_true',
         short = 'Ignore the applied patches in the series'),
@@ -69,7 +71,7 @@ options = [
         short = 'leave the rejected hunks in corresponding *.rej files'),
     opt('-e', '--edit', action = 'store_true',
         short = 'Invoke an editor for the patch description'),
-    opt('-p', '--showpatch', action = 'store_true',
+    opt('-d', '--showdiff', action = 'store_true',
         short = 'Show the patch content in the editor buffer'),
     opt('-a', '--author', metavar = '"NAME <EMAIL>"',
         short = 'Use "NAME <EMAIL>" as the author details'),
@@ -104,7 +106,7 @@ def __create_patch(filename, message, author_name,
author_email,
         patch = os.path.basename(filename)
     else:
         patch = ''
-    if options.strip:
+    if options.stripname:
         patch = __strip_patch_name(patch)

     if not patch:
@@ -152,9 +154,10 @@ def __create_patch(filename, message,
author_name, author_email,
             base = git_id(crt_series, options.base)
         else:
             base = None
-        git.apply_patch(diff = diff, base = base, reject = options.reject)
+        git.apply_patch(diff = diff, base = base, reject = options.reject,
+                        strip = options.strip)
         crt_series.refresh_patch(edit = options.edit,
-                                 show_patch = options.showpatch,
+                                 show_patch = options.showdiff,
                                  sign_str = options.sign_str,
                                  backup = False)
         out.done()
diff --git a/stgit/git.py b/stgit/git.py
index 268c44b..0886207 100644
--- a/stgit/git.py
+++ b/stgit/git.py
@@ -818,7 +818,7 @@ def repack():
     GRun('repack', '-a', '-d', '-f').run()

 def apply_patch(filename = None, diff = None, base = None,
-                fail_dump = True, reject = False):
+                fail_dump = True, reject = False, strip = None):
     """Apply a patch onto the current or given index. There must not
     be any local changes in the tree, otherwise the command fails
     """
@@ -840,6 +840,8 @@ def apply_patch(filename = None, diff = None, base = None,
     cmd = ['apply', '--index']
     if reject:
         cmd += ['--reject']
+    if strip:
+        cmd += ['-p', strip]
     try:
         GRun(*cmd).raw_input(diff).no_output()
     except GitRunException:

^ permalink raw reply related

* Re: [PATCHv5 00/14] git notes
From: Johan Herland @ 2009-09-08 21:40 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Johannes Schindelin, trast, tavestbo, git, chriscool,
	spearce
In-Reply-To: <7vocplxjov.fsf@alter.siamese.dyndns.org>

On Tuesday 08 September 2009, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> > On Tue, 8 Sep 2009, Johan Herland wrote:
> >> Algorithm / Notes tree   git log -n10 (x100)   git log --all
> >> ------------------------------------------------------------
> >> next / no-notes                4.77s              63.84s
> >>
> >> before / no-notes              4.78s              63.90s
> >> before / no-fanout            56.85s              65.69s
> >>
> >> 16tree / no-notes              4.77s              64.18s
> >> 16tree / no-fanout            30.35s              65.39s
> >> 16tree / 2_38                  5.57s              65.42s
> >> 16tree / 2_2_36                5.19s              65.76s
> >>
> >> flexible / no-notes            4.78s              63.91s
> >> flexible / no-fanout          30.34s              65.57s
> >> flexible / 2_38                5.57s              65.46s
> >> flexible / 2_2_36              5.18s              65.72s
> >> flexible / ym                  5.13s              65.66s
> >> flexible / ym_2_38             5.08s              65.63s
> >> flexible / ymd                 5.30s              65.45s
> >> flexible / ymd_2_38            5.29s              65.90s
> >> flexible / y_m                 5.11s              65.72s
> >> flexible / y_m_2_38            5.08s              65.67s
> >> flexible / y_m_d               5.06s              65.50s
> >> flexible / y_m_d_2_38          5.07s              65.79s
> >
> > I can see that some people may think that date-based fan-out is the
> > cat's ass,
> 
> Actually, my knee-jerk reaction was that 4.77 (next) vs 5.57 (16tree with
> 2_38) is already a good enough performance/simplicity tradeoff, and 5.57
> vs 5.08 (16tree with ym_2_38) probably does not justify the risk of worst
> case behaviour that can come from possible mismatch between the access
> pattern and the date-optimized tree layout.

Yes, 16tree / 2_38 looks like a reasonable tradeoff when you look at the 
absolute numbers, but it's also interesting to highlight the actual cost of 
doing the notes lookup. In that case, we see that 16tree / 2_38 costs 0.80s, 
whereas flexible / ym_2_38 only costs 0.31s, i.e. less than half the cost of 
the former...

> But that only argues against supporting _only_ date-optimized layout.
> 
> Support of "flexible layout" is not that flexible as its name suggests;
> one single note tree needs to have a uniform fanout strategy.

Actually, the uniform strategy is only required at each separate level. You 
are free to vary the strategy within independent subtrees. I.e. in the case 
where you have 1 note from 2007, and 1000 notes from 2008, you are free to 
use a mix of date-based and SHA1-based structures, like this:

  y2007/1234567...
  y2008/m01/d01/2345678...
  y2008/m01/d01/3456789...
  y2008/m01/d02/45/67890...
  y2008/m01/d02/56/78901...
  y2008/m01/d02/67/89012...
  ...

> > - I find the restriction to commits rather limiting.
> 
> Yeah, we would not want to be surprised to find many people want to
> annotate non-commits with this mechanism.

We could arbitrarily set the "commit date" for non-commit objects to the 
epoch, so that they can still be represented in a date-based fanout. (Of 
course, the notes code should be smart enough to choose a more optimal 
fanout if the number of non-commit notes is significant).

> > - most of the performance difference between the date-based and the
> > SHA-1 based fan-out looks to me as if the issue was the top-level tree.
> > Basically, this tree has to be read _every_ time _anybody_ wants to
> > read a note.
> 
> A comparison between 'next' and another algorithm that opens the
>  top-level notes tree object and returns "I did not find any note"
>  without doing anything else would reveal that cost.  But when you are
>  doing "log -n10" (or "log --all"), you would read the notes top-level
>  tree once, and it is likely to be cached in the obj_hash[] (or in
>  delta_base cache) already for the remaining invocations, even if notes
>  mechanism does not do its own cache, which I think it does, no?

Yes it does, since Dscho's original hash_map based implementation, in fact.


...Johan

-- 
Johan Herland, <johan@herland.net>
www.herland.net

^ permalink raw reply

* Re: [PATCHv5 00/14] git notes
From: Shawn O. Pearce @ 2009-09-08 21:39 UTC (permalink / raw)
  To: Sverre Rabbelier
  Cc: Junio C Hamano, Johannes Schindelin, Johan Herland, git, trast,
	tavestbo, git, chriscool
In-Reply-To: <fabb9a1e0909081436x72c71b0bv51d8f198cd82f289@mail.gmail.com>

Sverre Rabbelier <srabbelier@gmail.com> wrote:
> On Tue, Sep 8, 2009 at 23:10, Shawn O. Pearce<spearce@spearce.org> wrote:
> > So most commits (66%) would have only 1 version (and 1 note)
> > related to them in the note tree, but if I use the same note tree
> > for final commits as individual revisions considered, at least 18%
> > of the commits in the final history of the project would actually
> > have two notes, and 7.5% would have 3 notes.
> 
> You could however store all that information in one note, yes? Since
> the 'latest version' is the one committed, you can include the notes
> for all the previous ones at commit time?

Uh, but the natural way to index those is by commit, and each
different revision of a change is a different commit.  Why delete
the prior revision information and move it to the final commit note?
Someone who has the prior revisions in their reflog and is doing
`git log -g --notes` might want to see that annotation.

-- 
Shawn.

^ permalink raw reply

* Re: [PATCHv5 00/14] git notes
From: Sverre Rabbelier @ 2009-09-08 21:36 UTC (permalink / raw)
  To: Shawn O. Pearce
  Cc: Junio C Hamano, Johannes Schindelin, Johan Herland, git, trast,
	tavestbo, git, chriscool
In-Reply-To: <20090908211046.GV1033@spearce.org>

Heya,

On Tue, Sep 8, 2009 at 23:10, Shawn O. Pearce<spearce@spearce.org> wrote:
> So most commits (66%) would have only 1 version (and 1 note)
> related to them in the note tree, but if I use the same note tree
> for final commits as individual revisions considered, at least 18%
> of the commits in the final history of the project would actually
> have two notes, and 7.5% would have 3 notes.

You could however store all that information in one note, yes? Since
the 'latest version' is the one committed, you can include the notes
for all the previous ones at commit time?

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* Re: [PATCH] post-receive-email: do not call sendmail if no mail was generated
From: Andy Parkins @ 2009-09-08 21:12 UTC (permalink / raw)
  To: git
In-Reply-To: <7v4ord19da.fsf@alter.siamese.dyndns.org>

Thanks for CCing me in - I don't monitor the list closely enough these days 
:-)

Junio C Hamano wrote:

> If generate_email results in an empty output in this codepath:
> 
> # Check if we've got anyone to send to
> if [ -z "$recipients" ]; then
> ...
> echo >&2 "*** $config_name is not set so no email will be sent"
> echo >&2 "*** for $refname update $oldrev->$newrev"
> exit 0
> fi
> 
> shouldn't we rather receive an error e-mail than let the
> misconfiguration go undetected?

I don't know if it's still the case, but when I wrote it, anything that went 
to standard error appeared on the client terminal, however, it could 
probably do with being a better description of who is generating the 
message, otherwise it'll be some anonymous error during a push, giving the 
user no clue as to how to fix it.

> Before this check, I do not see anywhere generate_email would return nor
> exit, and after this check, there is a call to generate_email_header and
> that guarantees that the output from the generate_email function is not
> empty, so it looks to me that triggering this check is the only case your
> patch would change the behaviour of the script.

There is also a check for the validity of the update type above the 
recipients check.

I'm wondering actually if all of these should be "return"s rather than 
"exit"s.  Better still would be if there were some sort of exception 
throwing mechanism in shell script - anyone know if there is?



Andy

P.S. Hope you're all keeping well.

-- 
Dr Andy Parkins
andyparkins@gmail.com

^ permalink raw reply

* Re: [PATCHv5 00/14] git notes
From: Shawn O. Pearce @ 2009-09-08 21:10 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Johannes Schindelin, Johan Herland, git, trast, tavestbo, git,
	chriscool
In-Reply-To: <7vocplxjov.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> wrote:
> Also Gitney talked about annotating commits in the code-review thing.
> What's the expected notes density and distribution in that application?

Uh, try one note per commit in a project.  A few merges won't need
a note, but nearly every single non-merge commit would.

Consider a project with a velocity of about 200 non-merge
commits/day; the object count goes up fast.

One idea we are starting to kick around might double or quadruple
that number.  If we store metadata about every version of every
commit ever proposed to a project, we need a lot more notes than
commits.  Right now we have this sort of distribution from one of
our servers:

versions | commits 
---------+---------
       1 |    9262
       2 |    2626
       3 |    1053
       4 |     424
       5 |     224
       6 |     124
       7 |      57
       8 |      38
       9 |      28
      10 |      14
      11 |      12
      12 |      10
      13 |       5
      14 |       6
      15 |       2
      16 |       3
      17 |       2
      21 |       1
      32 |       1

So most commits (66%) would have only 1 version (and 1 note)
related to them in the note tree, but if I use the same note tree
for final commits as individual revisions considered, at least 18%
of the commits in the final history of the project would actually
have two notes, and 7.5% would have 3 notes.

-- 
Shawn.

^ permalink raw reply

* Re: [SCuMD]
From: Michael Gaffney @ 2009-09-08 21:09 UTC (permalink / raw)
  To: Christian Senkowski; +Cc: git
In-Reply-To: <4A965799.6050204@gmx.de>



Christian Senkowski wrote:
> Hi,
>
> I cloned SCuMD directly via git and started it but got following error:
>
> ~/scumd$ ./run.sh
> Exception in thread "main" java.lang.NoClassDefFoundError:
> com.asolutions.scmsshd.SCuMD
>    at gnu.java.lang.MainThread.run(libgcj.so.90)
> Caused by: java.lang.ClassNotFoundException:
> com.asolutions.scmsshd.SCuMD not found in
> gnu.gcj.runtime.SystemClassLoader{urls=[file:depend/lib/jgit.jar,file:depend/lib/minasshd.jar,file:lib/aopalliance-1.0.jar,file:lib/bcprov-jdk15-140.jar,file:lib/commons-io-1.4.jar,file:lib/commons-logging-1.0.4.jar,file:./,file:lib/jline-0.9.1.jar,file:lib/jline-0.9.94.jar,file:lib/jpam-1.1.jar,file:lib/jsch-0.1.40.jar,file:lib/jzlib-1.0.7.jar,file:lib/log4j-1.2.13.jar,file:lib/slf4j-api-1.5.2.jar,file:lib/slf4j-log4j12-1.4.3.jar,file:lib/spring-beans-2.5.5.jar,file:lib/spring-context-2.5.5.jar,file:lib/spring-core-2.5.5.jar],
> parent=gnu.gcj.runtime.ExtensionClassLoader{urls=[], parent=null}}
>    at java.net.URLClassLoader.findClass(libgcj.so.90)
>    at gnu.gcj.runtime.SystemClassLoader.findClass(libgcj.so.90)
>    at java.lang.ClassLoader.loadClass(libgcj.so.90)
>    at java.lang.ClassLoader.loadClass(libgcj.so.90)
>    at gnu.java.lang.MainThread.run(libgcj.so.90)
>
>
> Please help me out :)
>
>
> Kind regards,
> C. Senkowski
>
>   

Christian,
  Sorry for the late reply, yes, you need java 6 for this as it depends 
on Mina2 and JGit in the bowles. You should be able to get ahold of the 
openJDK these days on most distros.

-Mike

^ permalink raw reply

* Re: [PATCH] post-receive-email: do not call sendmail if no mail was generated
From: Lars Noschinski @ 2009-09-08 20:59 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, andyparkins, pape
In-Reply-To: <7v4ord19da.fsf@alter.siamese.dyndns.org>

* Junio C Hamano <gitster@pobox.com> [09-09-08 22:15]:
> Lars Noschinski <lars@public.noschinski.de> writes:
> 
> > contrib/hooks/post-receive-email used to call the send_mail function
> > (and thus, /usr/sbin/sendmail), even if generate_mail generated no
> > output.  This is problematic, as the sendmail binary provided by exim4
> > generates an error mail if provided with an empty input.
> 
> I actually have a bigger question, not about the implementation but about
> the cause.
> 
> If generate_email results in an empty output in this codepath:
> 
> 	# Check if we've got anyone to send to
> 	if [ -z "$recipients" ]; then
> 		...
> 		echo >&2 "*** $config_name is not set so no email will be sent"
> 		echo >&2 "*** for $refname update $oldrev->$newrev"
> 		exit 0
> 	fi
> 
> shouldn't we rather receive an error e-mail than let the
> misconfiguration go undetected?

Probably not. The error message is displayed to the user who did the
push. Normally (if no explicit From: address is configured), this is the
same user, which would receive the error mail.

> Before this check, I do not see anywhere generate_email would return nor
> exit, and after this check, there is a call to generate_email_header and
> that guarantees that the output from the generate_email function is not
> empty, so it looks to me that triggering this check is the only case your
> patch would change the behaviour of the script.

Actually, there are a two cases in the case statement before, where
generate_email would return:

    refs/remotes/*,commit)
        # tracking branch
        refname_type="tracking branch"
        short_refname=${refname##refs/remotes/}
        echo >&2 "*** Push-update of tracking branch, $refname"
        echo >&2 "***  - no email generated."
        exit 0
        ;;
    *)
        # Anything else (is there anything else?)
        echo >&2 "*** Unknown type of update to $refname ($rev_type)"
        echo >&2 "***  - no email generated"
        exit 1
        ;;

i.e. if we are pushing to a branch neither in refs/tags nor refs/heads.

In our setting, the build process pushes to refs/builds, so we can track
code changes between different builds, without displaying a whole lot of
mostly useless branches or tags to the user.

> > diff --git a/contrib/hooks/post-receive-email b/contrib/hooks/post-receive-email
> > index 2a66063..c855c31 100755
> > --- a/contrib/hooks/post-receive-email
> > +++ b/contrib/hooks/post-receive-email
> > @@ -637,6 +637,16 @@ show_new_revisions()
> >  
> >  send_mail()
> >  {
> > +	OIFS=$IFS
> > +	IFS='
> > +'
> > +	read FIRSTLINE || exit 1
> 
> Shouldn't this be merely a "return"?  The caller looks like this:

Yes.

I'll fix it in the next patch (when there are further comments); but you
may fold it in (and add the SOB if forgot), if you prefer.

^ 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