Git development
 help / color / mirror / Atom feed
* [PATCH v2] Add --track option to git clone
From: David Soria Parra @ 2009-12-01 22:51 UTC (permalink / raw)
  To: git

Update: 
 + added a test

The following series adds a --track option to git clone. If the --track option
is specified only the given remote branch will be received and checked out.

It tries to make the following usecase possible:
Imagine you are working on a project that has 1.x and a 2.x branch. The project
itself requires a complex setup (webserver, configuration files, etc). Setting up
1.x and 2.x branch requires a lot of work, but a developer needs to maintain both.
He'll use the --track option to clone the 2.x branch into a directory and does the same
with the 1.x branch, where he setup the project. He can use locally separate repositories
while still being able to push to just one remote repository.

I'm aware that it's not possible to give more than one --track option. Implementing
the possibility to specify multiple --track option would certainly a good improvment
later, but would also require a lot more work as far as I understand the clone code.

Being able to specify just one --track option is a compromise of doing a small change
and implementing this feature.

^ permalink raw reply

* [PATCH v2 1/2] Teach clone to clone just one remote branch using --track
From: David Soria Parra @ 2009-12-01 22:51 UTC (permalink / raw)
  To: git; +Cc: David Soria Parra
In-Reply-To: <1259707865-6561-1-git-send-email-sn_@gmx.net>

From: David Soria Parra <dsp@php.net>

Add a --track option that can be used to clone just the
given branch from the remote and nothing else. This is done
by setting the remote.<branch>.fetch option before cloning.
This option cannot be used together with --mirror.
For example using

    git clone --track next git://git.kernel.org/pub/scm/git/git.git

will just clone the next branch from the git.git repository.

The option is called --track to ensure clean wording with
'git remote add --track'.

Signed-off-by: David Soria Parra <dsp@php.net>
---
 builtin-clone.c        |   12 +++++++++++-
 t/t5708-clone-track.sh |   43 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+), 1 deletions(-)
 create mode 100755 t/t5708-clone-track.sh

diff --git a/builtin-clone.c b/builtin-clone.c
index 5df8b0f..bc335ee 100644
--- a/builtin-clone.c
+++ b/builtin-clone.c
@@ -43,6 +43,7 @@ static char *option_template, *option_reference, *option_depth;
 static char *option_origin = NULL;
 static char *option_branch = NULL;
 static char *option_upload_pack = "git-upload-pack";
+static char *option_track = NULL;
 static int option_verbose;
 
 static struct option builtin_clone_options[] = {
@@ -76,6 +77,8 @@ static struct option builtin_clone_options[] = {
 		   "path to git-upload-pack on the remote"),
 	OPT_STRING(0, "depth", &option_depth, "depth",
 		    "create a shallow clone of that depth"),
+	OPT_STRING('t', "track", &option_track, "branch",
+			"remote branche to track"),
 
 	OPT_END()
 };
@@ -483,7 +486,14 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 		strbuf_addf(&branch_top, "refs/remotes/%s/", option_origin);
 	}
 
-	strbuf_addf(&value, "+%s*:%s*", src_ref_prefix, branch_top.buf);
+	if (option_track) {
+		if (option_mirror)
+			return error("Cannot use --track together with --mirror");
+		strbuf_addf(&value, "+%s%s:%s%s", src_ref_prefix, option_track, branch_top.buf, option_track);
+		option_branch = option_track;
+	} else {
+		strbuf_addf(&value, "+%s*:%s*", src_ref_prefix, branch_top.buf);
+	}
 
 	if (option_mirror || !option_bare) {
 		/* Configure the remote */
diff --git a/t/t5708-clone-track.sh b/t/t5708-clone-track.sh
new file mode 100755
index 0000000..71b8461
--- /dev/null
+++ b/t/t5708-clone-track.sh
@@ -0,0 +1,43 @@
+#!/bin/sh
+
+test_description='clone --track option'
+. ./test-lib.sh
+
+check_HEAD() {
+	echo refs/heads/"$1" >expect &&
+	git symbolic-ref HEAD >actual &&
+	test_cmp expect actual
+}
+
+check_file() {
+	echo "$1" >expect &&
+	test_cmp expect file
+}
+
+test_expect_success 'setup' '
+	mkdir parent &&
+	(cd parent && git init &&
+	 echo one >file && git add file && git commit -m one &&
+	 git checkout -b two &&
+	 echo two >file && git add file && git commit -m two &&
+	 git checkout master)
+'
+
+test_expect_success 'vanilla clone has both branches' '
+	git clone parent clone &&
+	(cd clone &&
+	git branch -r | grep master &&
+	git branch -r | grep two
+	)
+'
+
+test_expect_success 'clone -t chooses specified remote branch' '
+	git clone -t two parent clone-two &&
+	(cd clone-two &&
+	!(git branch -r | grep master) &&
+	git branch -r | grep two &&
+	check_HEAD two
+	)
+'
+
+test_done
-- 
1.6.6.rc0.268.g1c272

^ permalink raw reply related

* [PATCH v2 2/2] Documentation: Add --track option to the git clone manpage
From: David Soria Parra @ 2009-12-01 22:51 UTC (permalink / raw)
  To: git; +Cc: David Soria Parra
In-Reply-To: <1259707865-6561-1-git-send-email-sn_@gmx.net>

From: David Soria Parra <dsp@php.net>

Signed-off-by: David Soria Parra <dsp@php.net>
---
 Documentation/git-clone.txt |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
index 7ccd742..c2ab645 100644
--- a/Documentation/git-clone.txt
+++ b/Documentation/git-clone.txt
@@ -11,7 +11,7 @@ SYNOPSIS
 [verse]
 'git clone' [--template=<template_directory>]
 	  [-l] [-s] [--no-hardlinks] [-q] [-n] [--bare] [--mirror]
-	  [-o <name>] [-b <name>] [-u <upload-pack>] [--reference <repository>]
+	  [-o <name>] [-b <name>] [-t <name>] [-u <upload-pack>] [--reference <repository>]
 	  [--depth <depth>] [--recursive] [--] <repository> [<directory>]
 
 DESCRIPTION
@@ -135,6 +135,12 @@ objects from the source repository into a pack in the cloned repository.
 	instead. In a non-bare repository, this is the branch that will
 	be checked out.
 
+--track <name>::
+-t <name>::
+	Instead of cloning the complete remote repository, only the given
+	remote branch `<name>` will be tracked and checked out.
+	This implies --branch `<name>`.
+
 --upload-pack <upload-pack>::
 -u <upload-pack>::
 	When given, and the repository to clone from is accessed
-- 
1.6.6.rc0.268.g1c272

^ permalink raw reply related

* [PATCH] git-pull.sh: Fix call to git-merge for new command format
From: Horst H. von Brand @ 2009-12-01 22:44 UTC (permalink / raw)
  To: git, gitster; +Cc: Horst H. von Brand

Signed-off-by: Horst H. von Brand <vonbrand@inf.utfsm.cl>
---
 git-pull.sh |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/git-pull.sh b/git-pull.sh
index bfeb4a0..a875809 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -218,5 +218,5 @@ merge_name=$(git fmt-merge-msg $log_arg <"$GIT_DIR/FETCH_HEAD") || exit
 test true = "$rebase" &&
 	exec git-rebase $diffstat $strategy_args --onto $merge_head \
 	${oldremoteref:-$merge_head}
-exec git-merge $diffstat $no_commit $squash $no_ff $ff_only $log_arg $strategy_args \
-	"$merge_name" HEAD $merge_head $verbosity
+exec git-merge  $verbosity -m $diffstat $no_commit $squash $no_ff $ff_only $log_arg $strategy_args \
+	"$merge_name" $merge_head
-- 
1.6.6.rc0.114.gc8648

^ permalink raw reply related

* Re: [RFC PATCH 0/8] Git remote helpers to implement smart transports.
From: Shawn O. Pearce @ 2009-12-01 23:20 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Ilari Liusvaara, Sverre Rabbelier, git
In-Reply-To: <7vskbuwhmy.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> wrote:
> It is somewhat unfortunate that a few changes I liked (e.g. the "debug"
> bit), even though it was somewhat painful to read them due to coding style
> differences, were not at the beginning of the series but instead buried
> after changes that are much bigger and controversial (e.g. [6/8]).

I agree about that debug patch, I actually thought that was
interesting, and wish I had done more of that sort of work during
smart-http.  It would have helped me to debug it in the early stages.

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH] git-pull.sh: Fix call to git-merge for new command format
From: Junio C Hamano @ 2009-12-01 23:54 UTC (permalink / raw)
  To: Horst H. von Brand; +Cc: git
In-Reply-To: <1259707451-20661-1-git-send-email-vonbrand@inf.utfsm.cl>

"Horst H. von Brand" <vonbrand@inf.utfsm.cl> writes:

> Signed-off-by: Horst H. von Brand <vonbrand@inf.utfsm.cl>
> ---
>  git-pull.sh |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/git-pull.sh b/git-pull.sh
> index bfeb4a0..a875809 100755
> --- a/git-pull.sh
> +++ b/git-pull.sh
> @@ -218,5 +218,5 @@ merge_name=$(git fmt-merge-msg $log_arg <"$GIT_DIR/FETCH_HEAD") || exit
>  test true = "$rebase" &&
>  	exec git-rebase $diffstat $strategy_args --onto $merge_head \
>  	${oldremoteref:-$merge_head}
> -exec git-merge $diffstat $no_commit $squash $no_ff $ff_only $log_arg $strategy_args \
> -	"$merge_name" HEAD $merge_head $verbosity
> +exec git-merge  $verbosity -m $diffstat $no_commit $squash $no_ff $ff_only $log_arg $strategy_args \
> +	"$merge_name" $merge_head
> -- 
> 1.6.6.rc0.114.gc8648

Heh, embarrasing.

But I think you wanted to have -m immediately before "$merge_name", no?

^ permalink raw reply

* Re: multiple working directories for long-running builds (was: "git merge" merges too much!)
From: Dmitry Potapov @ 2009-12-02  0:10 UTC (permalink / raw)
  To: The Git Mailing List
In-Reply-To: <m1NFbSE-000kn2C@most.weird.com>

On Tue, Dec 01, 2009 at 05:44:15PM -0500, Greg A. Woods wrote:
> At Wed, 2 Dec 2009 00:18:30 +0300, Dmitry Potapov <dpotapov@gmail.com> wrote:
> Subject: Re: multiple working directories for long-running builds (was:	"git merge" merges too much!)
> > 
> > AFAIK, "git archive" is cheaper than git clone.
> 
> It depends on what you mean by "cheaper"

You said:

>>> "git archive" is likely even more
>>> impossible for some large projects to use than "git clone" 

My point was that I do not see why you believe "git archive" is more
expensive than "git clone". Accordingly to Jeff Epler's numbers,
"git archive" is 20% faster than "git clone"...

> It's clearly going to require
> less disk space.  However it's also clearly going to require more disk
> bandwidth, potentially a _LOT_ more disk bandwidth.

Well, it does, but as I said earlier it is not the case where I expect
things being instantaneous. If you do not a full build and test, then it
is going to take a lot of time anyway, and git-archive is not likely to
be a big overhead in relative terms.

> > I do not say it is fast
> > for huge project, but if you want to run a process such as clean build
> > and test that takes a long time anyway, it does not add much to the
> > total time.
> 
> I think you need to try throwing around an archive of, say, 50,000 small
> files a few times simultaneously on your system to appreciate the issue.
> 
> (i.e. consider the load on a storage subsystem, say a SAN or NAS, where
> with your suggestion there might be a dozen or more developers running
> "git archive" frequently enough that even three or four might be doing
> it at the same time, and this on top of all the i/o bandwidth required
> for the builds all of the other developers are also running at the same
> time.)

First of all, I do not see why it should be done frequently. The full
build and test may be run once or twice a day, and the full test and
build may take an hour. git-archive will take probably less than minute
for 50,000 small files (especially if you use tmpfs). In other words, it
is 1% overhead, but you get a clean build, which is fully tested. You
can sure that no garbage left in the worktree that could influence the
result.

> 
> > > Disk bandwidth is almost always more expensive than disk space.
> > 
> > Disk bandwidth is certainly more expensive than disk space, and the
> > whole point was to avoid a lot of disk bandwidth by using hot cache.
> 
> Huh?  Throwing around the archive has nothing to do with the build
> system in this case.

"git archive" to do full build and test, which is rarely done. Normally,
you just switch between branches, which means a few files are changed
and rebuild, and no archive is involved here.

> 
> I'm just not willing to even consider using what would really be the
> most simplistic and most expensive form of updating a working directory
> as could ever be imagined.  "Git archive" is truly unintelligent, as-is.

"git archive" is NOT for updating your working tree. You use "git
checkout" to switch between branches. "git checkout" is intelligent
enough to overwrite only those files that actually differ between
two versions.

> A major further advantage of multiple working directories is that this
> eliminates one more point of failure -- i.e. you don't end up with
> multiple copies of the repo that _should_ be effectively read-only for
> everything but "push", and perhaps then only to one branch.

Multiple copies of the same repo is never a problem (except taking some
disks space). I really do not understand why you say that some copies
should be effectively read-only... You can start to work on some feature
at one place (using one repo) and then continue in another place using
another repo. (Obviously, it will require to fetch changes from the
first repo, before you will be able to continue, but it is just one
command). In other words, I really do not understand what are you
talking about here.


> 
> I know of at least three very real-world projects where there are tens
> of thousands of small files that really must be managed as one unit, and
> where running a build in that tree could take a whole day or two on even
> the fastest currently available dedicated build server.  Eg. pkgsrc.

Tens of thousands files should not be a problem... For instance, the
Linux kernel has around 30 thousands and Git works very well in this
case. But I would consider to split if it has hundrends of thousands...


Dmitry

^ permalink raw reply

* Re: "git merge" merges too much!
From: Dmitry Potapov @ 2009-12-02  0:22 UTC (permalink / raw)
  To: The Git Mailing List
In-Reply-To: <m1NFak0-000kn2C@most.weird.com>

On Tue, Dec 01, 2009 at 04:58:34PM -0500, Greg A. Woods wrote:
> At Tue, 1 Dec 2009 23:50:57 +0300, Dmitry Potapov <dpotapov@gmail.com> wrote:
> Subject: Re: "git merge" merges too much!
> > 
> > > > 
> > > > $ git branch new-foo foo
> > > > 
> > > > $ git rebase --onto newbase oldbase new-foo
> > > 
> > > Hmmm.... I'll have to think about that.  It makes some sense, but I
> > > don't intuitively read the command-line parameters well enough to
> > > predict the outcome in all of the scenarios I'm interested in.
> > > 
> > > what is "oldbase" there?  I'm guessing it means "base of foo" (and for
> > > the moment, "new-foo" too)?
> > 
> > You have:
> > 
> >  o---o---o---o---o  newbase
> >        \
> >         o---o---o---o---o  oldbase
> >                          \
> >                           o---o---o  foo
> 
> Yes, sort of -- in the ideal situation, but not in my particular example
> where "oldbase" is just a tag, not a real branch.

It does not matter whether it is tag or branch or just SHA-1. You can
use any two reference as newbase and oldbase. They specify two points
in DAG. The only thing that has to be a branch in my example is new-foo.

> 
> So yes, "oldbase" is in fact "base of foo".  Trickier still is when the
> "oldbase" branch has one or more commits newer then "base of foo".  Does
> Git not have a symbolic name for the true base of a branch?  I.e. is
> there not some form of symbolic name for "N" in the following?
> 
>    o---o---o---o---o---o---o---o  master
>             \
>              o---o---N---o---o  release-1
>                       \
>                        o---o---o  local-release-1

You can always find SHA-1 for N using the following command:

  git merge-base release-1 local-release-1

but you do not have to do that to rebase your changes. You just can run:

   # create a copy of local-release-1, so it will not disappear
   git branch copy-release-1 local-release-1

   # rebase the branch to master
   git rebase --onto master release-1 copy-release-1


Dmitry

^ permalink raw reply

* [StGit PATCH v2 1/6] stg mail: Refactor __send_message and friends
From: Alex Chiang @ 2009-12-02  0:46 UTC (permalink / raw)
  To: catalin.marinas; +Cc: git, Karl Wiberg
In-Reply-To: <20091202003503.7737.51579.stgit@bob.kio>

Instead of passing all the various smtp* args to __send_message
individually, let's just pass the options list instead.

The main motivation is for future patches. The end goal is to
thin out stg mail's implementation and make it a minimal wrapper
around git send-email. By passing the options list to __send_message
we prepare to pass options directly to git send-email.

As a bonus, this change results in a cleaner internal API.

Finally, it also pushes the smtp logic where it belongs, viz. into
__send_message_smtp, instead of cluttering up the main body of
mail.func().

Cc: Karl Wiberg <kha@treskal.com>
Signed-off-by: Alex Chiang <achiang@hp.com>
---

 stgit/commands/mail.py |   43 +++++++++++++++++++------------------------
 1 files changed, 19 insertions(+), 24 deletions(-)

diff --git a/stgit/commands/mail.py b/stgit/commands/mail.py
index abd42e4..a38e3e6 100644
--- a/stgit/commands/mail.py
+++ b/stgit/commands/mail.py
@@ -190,10 +190,20 @@ def __send_message_sendmail(sendmail, msg):
     cmd = sendmail.split()
     Run(*cmd).raw_input(msg).discard_output()
 
-def __send_message_smtp(smtpserver, from_addr, to_addr_list, msg,
-                        smtpuser, smtppassword, use_tls):
+def __send_message_smtp(smtpserver, from_addr, to_addr_list, msg, options):
     """Send the message using the given SMTP server
     """
+    smtppassword = options.smtp_password or config.get('stgit.smtppassword')
+    smtpuser = options.smtp_user or config.get('stgit.smtpuser')
+    smtpusetls = options.smtp_tls or config.get('stgit.smtptls') == 'yes'
+
+    if (smtppassword and not smtpuser):
+        raise Exception('SMTP password supplied, username needed')
+    if (smtpusetls and not smtpuser):
+        raise Exception('SMTP over TLS requested, username needed')
+    if (smtpuser and not smtppassword):
+        smtppassword = getpass.getpass("Please enter SMTP password: ")
+
     try:
         s = smtplib.SMTP(smtpserver)
     except Exception, err:
@@ -203,7 +213,7 @@ def __send_message_smtp(smtpserver, from_addr, to_addr_list, msg,
     try:
         if smtpuser and smtppassword:
             s.ehlo()
-            if use_tls:
+            if smtpusetls:
                 if not hasattr(socket, 'ssl'):
                     raise CmdException,  "cannot use TLS - no SSL support in Python"
                 s.starttls()
@@ -218,17 +228,17 @@ def __send_message_smtp(smtpserver, from_addr, to_addr_list, msg,
 
     s.quit()
 
-def __send_message(smtpserver, from_addr, to_addr_list, msg,
-                   smtpuser, smtppassword, use_tls):
+def __send_message(from_addr, to_addr_list, msg, options):
     """Message sending dispatcher.
     """
+    smtpserver = options.smtp_server or config.get('stgit.smtpserver')
+
     if smtpserver.startswith('/'):
         # Use the sendmail tool
         __send_message_sendmail(smtpserver, msg)
     else:
         # Use the SMTP server (we have host and port information)
-        __send_message_smtp(smtpserver, from_addr, to_addr_list, msg,
-                            smtpuser, smtppassword, use_tls)
+        __send_message_smtp(smtpserver, from_addr, to_addr_list, msg, options)
 
 def __build_address_headers(msg, options, extra_cc = []):
     """Build the address headers and check existing headers in the
@@ -543,8 +553,6 @@ def func(parser, options, args):
     """Send the patches by e-mail using the patchmail.tmpl file as
     a template
     """
-    smtpserver = options.smtp_server or config.get('stgit.smtpserver')
-
     applied = crt_series.get_applied()
 
     if options.all:
@@ -564,17 +572,6 @@ def func(parser, options, args):
             raise CmdException, 'Cannot send empty patch "%s"' % p
     out.done()
 
-    smtppassword = options.smtp_password or config.get('stgit.smtppassword')
-    smtpuser = options.smtp_user or config.get('stgit.smtpuser')
-    smtpusetls = options.smtp_tls or config.get('stgit.smtptls') == 'yes'
-
-    if (smtppassword and not smtpuser):
-        raise CmdException, 'SMTP password supplied, username needed'
-    if (smtpusetls and not smtpuser):
-        raise CmdException, 'SMTP over TLS requested, username needed'
-    if (smtpuser and not smtppassword):
-        smtppassword = getpass.getpass("Please enter SMTP password: ")
-
     total_nr = len(patches)
     if total_nr == 0:
         raise CmdException, 'No patches to send'
@@ -616,8 +613,7 @@ def func(parser, options, args):
             out.stdout_raw(msg_string + '\n')
         else:
             out.start('Sending the cover message')
-            __send_message(smtpserver, from_addr, to_addr_list, msg_string,
-                           smtpuser, smtppassword, smtpusetls)
+            __send_message(from_addr, to_addr_list, msg_string, options)
             time.sleep(sleep)
             out.done()
 
@@ -648,8 +644,7 @@ def func(parser, options, args):
             out.stdout_raw(msg_string + '\n')
         else:
             out.start('Sending patch "%s"' % p)
-            __send_message(smtpserver, from_addr, to_addr_list, msg_string,
-                           smtpuser, smtppassword, smtpusetls)
+            __send_message(from_addr, to_addr_list, msg_string, options)
             # give recipients a chance of receiving related patches in the
             # correct order.
             if patch_nr < total_nr:

^ permalink raw reply related

* [StGit PATCH v2 0/6] add support for git send-email
From: Alex Chiang @ 2009-12-02  0:46 UTC (permalink / raw)
  To: catalin.marinas; +Cc: git, Karl Wiberg

This is v2 of the series that starts teaching stg mail how to
call git send-email.

I made all the changes that Karl recommended, and made sure to
run the t1900-mail.sh test at every point in the series (passes
successfully).

I also experimented with adding another test case for --git
mode, basically duplicating t1900-mail.sh, and then adding the
--git argument wherever it made sense.

However, that resulted in failure of the last 3 test cases,
which is due to the fact that we no longer parse To/Cc/Bcc
command line args in --git mode, and the resulting mbox file was
missing the expected recipient addresses.

I played around with that for a while, thinking that I could use
git send-email --dry-run to do something equivalent, but then
realized that git send-email's run-run mode is definitely not
analogous to stg mail's --mbox mode.

The upshot is that in stg mail, --git and --mbox don't interact
well, and the resulting mbox file will lack the recipients. This
might be fixed in the future if we teach git send-email how to
generate mbox files, but then we introduce a versioning problem.

So let's just accept this wart for now, and say, if you want an
mbox file generated, don't use --git. That seems reasonable to
me.

This mail was sent with the following command line:

	./stg mail --git -a -e --auto -v v2 --prefix=StGit
	--to=catalin.marinas@gmail.com --cc=git

Note that the --cc= contains an alias for the git mailing list
that I defined in my ~/.mutt.aliases file (and specified in
.gitconfig -> sendemail.aliasesfile and sendemail.aliasfiletype.

Thanks,
/ac

---

Alex Chiang (6):
      stg mail: Refactor __send_message and friends
      stg mail: reorder __build_[message|cover] parameters
      stg mail: make __send_message do more
      stg mail: factor out __update_header
      stg mail: add basic support for git send-email
      stg mail: don't parse To/Cc/Bcc in --git mode


 stgit/commands/mail.py |  196 +++++++++++++++++++++++++++---------------------
 1 files changed, 112 insertions(+), 84 deletions(-)

^ permalink raw reply

* [StGit PATCH v2 4/6] stg mail: factor out __update_header
From: Alex Chiang @ 2009-12-02  0:46 UTC (permalink / raw)
  To: catalin.marinas; +Cc: git, Karl Wiberg
In-Reply-To: <20091202003503.7737.51579.stgit@bob.kio>

Factor __update_header out of __build_address_headers.

Headers like Reply-To, Mail-Reply-To, and Mail-Followup-To are now
handled in __build_extra_headers.

We make this change because in the future, we do not want to call
__build_address_headers if using git send-email but we will always
want to call __build_extra_headers.

Cc: Karl Wiberg <kha@treskal.com>
Signed-off-by: Alex Chiang <achiang@hp.com>
---

 stgit/commands/mail.py |   48 +++++++++++++++++++++++++-----------------------
 1 files changed, 25 insertions(+), 23 deletions(-)

diff --git a/stgit/commands/mail.py b/stgit/commands/mail.py
index edff878..f430a13 100644
--- a/stgit/commands/mail.py
+++ b/stgit/commands/mail.py
@@ -262,25 +262,25 @@ def __send_message(type, tmpl, options, *args):
     out.done()
     return msg_id
 
-def __build_address_headers(msg, options, extra_cc = []):
-    """Build the address headers and check existing headers in the
-    template.
-    """
+def __update_header(msg, header, addr = '', ignore = ()):
     def __addr_pairs(msg, header, extra):
         pairs = email.Utils.getaddresses(msg.get_all(header, []) + extra)
         # remove pairs without an address and resolve the aliases
         return [address_or_alias(p) for p in pairs if p[1]]
 
-    def __update_header(header, addr = '', ignore = ()):
-        addr_pairs = __addr_pairs(msg, header, [addr])
-        del msg[header]
-        # remove the duplicates and filter the addresses
-        addr_dict = dict((addr, email.Utils.formataddr((name, addr)))
-                         for name, addr in addr_pairs if addr not in ignore)
-        if addr_dict:
-            msg[header] = ', '.join(addr_dict.itervalues())
-        return set(addr_dict.iterkeys())
+    addr_pairs = __addr_pairs(msg, header, [addr])
+    del msg[header]
+    # remove the duplicates and filter the addresses
+    addr_dict = dict((addr, email.Utils.formataddr((name, addr)))
+                     for name, addr in addr_pairs if addr not in ignore)
+    if addr_dict:
+        msg[header] = ', '.join(addr_dict.itervalues())
+    return set(addr_dict.iterkeys())
 
+def __build_address_headers(msg, options, extra_cc = []):
+    """Build the address headers and check existing headers in the
+    template.
+    """
     to_addr = ''
     cc_addr = ''
     extra_cc_addr = ''
@@ -300,18 +300,14 @@ def __build_address_headers(msg, options, extra_cc = []):
         bcc_addr = autobcc
 
     # if an address is on a header, ignore it from the rest
-    to_set = __update_header('To', to_addr)
-    cc_set = __update_header('Cc', cc_addr, to_set)
-    bcc_set = __update_header('Bcc', bcc_addr, to_set.union(cc_set))
+    to_set = __update_header(msg, 'To', to_addr)
+    cc_set = __update_header(msg, 'Cc', cc_addr, to_set)
+    bcc_set = __update_header(msg, 'Bcc', bcc_addr, to_set.union(cc_set))
 
     # --auto generated addresses, don't include the sender
-    from_set = __update_header('From')
-    __update_header('Cc', extra_cc_addr, to_set.union(bcc_set).union(from_set))
-
-    # update other address headers
-    __update_header('Reply-To')
-    __update_header('Mail-Reply-To')
-    __update_header('Mail-Followup-To')
+    from_set = __update_header(msg, 'From')
+    __update_header(msg, 'Cc', extra_cc_addr,
+                    to_set.union(bcc_set).union(from_set))
 
 def __get_signers_list(msg):
     """Return the address list generated from signed-off-by and
@@ -349,6 +345,12 @@ def __build_extra_headers(msg, msg_id, ref_id = None):
         msg['References'] = ref_id
     msg['User-Agent'] = 'StGit/%s' % version.version
 
+    # update other address headers
+    __update_header(msg, 'Reply-To')
+    __update_header(msg, 'Mail-Reply-To')
+    __update_header(msg, 'Mail-Followup-To')
+
+
 def __encode_message(msg):
     # 7 or 8 bit encoding
     charset = email.Charset.Charset('utf-8')

^ permalink raw reply related

* [StGit PATCH v2 2/6] stg mail: reorder __build_[message|cover] parameters
From: Alex Chiang @ 2009-12-02  0:46 UTC (permalink / raw)
  To: catalin.marinas; +Cc: git, Karl Wiberg
In-Reply-To: <20091202003503.7737.51579.stgit@bob.kio>

Reorder the argument lists for both __build_cover and __build_message.

This change will aid readability of a subsequent refactoring patch.

Cc: Karl Wiberg <kha@treskal.com>
Signed-off-by: Alex Chiang <achiang@hp.com>
---

 stgit/commands/mail.py |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/stgit/commands/mail.py b/stgit/commands/mail.py
index a38e3e6..35194a8 100644
--- a/stgit/commands/mail.py
+++ b/stgit/commands/mail.py
@@ -370,7 +370,7 @@ def __edit_message(msg):
 
     return msg
 
-def __build_cover(tmpl, patches, msg_id, options):
+def __build_cover(tmpl, msg_id, options, patches):
     """Build the cover message (series description) to be sent via SMTP
     """
     sender = __get_sender()
@@ -439,7 +439,7 @@ def __build_cover(tmpl, patches, msg_id, options):
 
     return msg
 
-def __build_message(tmpl, patch, patch_nr, total_nr, msg_id, ref_id, options):
+def __build_message(tmpl, msg_id, options, patch, patch_nr, total_nr, ref_id):
     """Build the message to be sent via SMTP
     """
     p = crt_series.get_patch(patch)
@@ -600,7 +600,7 @@ def func(parser, options, args):
                 raise CmdException, 'No cover message template file found'
 
         msg_id = email.Utils.make_msgid('stgit')
-        msg = __build_cover(tmpl, patches, msg_id, options)
+        msg = __build_cover(tmpl, msg_id, options, patches)
         from_addr, to_addr_list = __parse_addresses(msg)
 
         msg_string = msg.as_string(options.mbox)
@@ -630,8 +630,8 @@ def func(parser, options, args):
 
     for (p, patch_nr) in zip(patches, range(1, total_nr + 1)):
         msg_id = email.Utils.make_msgid('stgit')
-        msg = __build_message(tmpl, p, patch_nr, total_nr, msg_id, ref_id,
-                              options)
+        msg = __build_message(tmpl, msg_id, options, p, patch_nr, total_nr,
+                              ref_id)
         from_addr, to_addr_list = __parse_addresses(msg)
 
         msg_string = msg.as_string(options.mbox)

^ permalink raw reply related

* [StGit PATCH v2 5/6] stg mail: add basic support for git send-email
From: Alex Chiang @ 2009-12-02  0:46 UTC (permalink / raw)
  To: catalin.marinas; +Cc: git, Karl Wiberg
In-Reply-To: <20091202003503.7737.51579.stgit@bob.kio>

This is the first step in turning stg mail into a wrapper for
git send-email. It requires passing the --git option to stg mail
for now.

Only a few basic options are supported for now, namely To/Cc/Bcc.

git send-email options used:
  --suppress-cc=self	prevent further information prompts
  --quiet		reduce git send-email output

Cc: Karl Wiberg <kha@treskal.com>
Signed-off-by: Alex Chiang <achiang@hp.com>
---

 stgit/commands/mail.py |   50 ++++++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 46 insertions(+), 4 deletions(-)

diff --git a/stgit/commands/mail.py b/stgit/commands/mail.py
index f430a13..8a33c22 100644
--- a/stgit/commands/mail.py
+++ b/stgit/commands/mail.py
@@ -140,7 +140,9 @@ options = [
     opt('-b', '--branch', args = [argparse.stg_branches],
         short = 'Use BRANCH instead of the default branch'),
     opt('-m', '--mbox', action = 'store_true',
-        short = 'Generate an mbox file instead of sending')
+        short = 'Generate an mbox file instead of sending'),
+    opt('--git', action = 'store_true',
+        short = 'Use git send-email (EXPERIMENTAL)')
     ] + argparse.diff_opts_option()
 
 directory = DirectoryHasRepository(log = False)
@@ -228,6 +230,42 @@ def __send_message_smtp(smtpserver, from_addr, to_addr_list, msg, options):
 
     s.quit()
 
+def __send_message_git(msg, options):
+    """Send the message using git send-email
+    """
+    from subprocess import call
+    from tempfile import mkstemp
+
+    cmd = ["git", "send-email", "--from=%s" % msg['From']]
+    cmd.append("--quiet")
+    cmd.append("--suppress-cc=self")
+    if not options.auto:
+        cmd.append("--suppress-cc=body")
+
+    # We only support To/Cc/Bcc in git send-email for now.
+    for x in ['to', 'cc', 'bcc']:
+        if getattr(options, x):
+            cmd.extend('--%s=%s' % (x, a) for a in getattr(options, x))
+
+    # XXX: hack for now so that we don't duplicate To/Cc/Bcc headers
+    # in the mail, as git send-email inserts those for us.
+    del msg['To']
+    del msg['Cc']
+    del msg['Bcc']
+
+    (fd, path) = mkstemp()
+    os.write(fd, msg.as_string(options.mbox))
+    os.close(fd)
+
+    try:
+        try:
+            cmd.append(path)
+            call(cmd)
+        except Exception, err:
+            raise CmdException, str(err)
+    finally:
+        os.unlink(path)
+
 def __send_message(type, tmpl, options, *args):
     """Message sending dispatcher.
     """
@@ -245,10 +283,13 @@ def __send_message(type, tmpl, options, *args):
         out.stdout_raw(msg_str + '\n')
         return msg_id
 
-    out.start('Sending ' + outstr)
+    if not options.git:
+        out.start('Sending ' + outstr)
 
     smtpserver = options.smtp_server or config.get('stgit.smtpserver')
-    if smtpserver.startswith('/'):
+    if options.git:
+        __send_message_git(msg, options)
+    elif smtpserver.startswith('/'):
         # Use the sendmail tool
         __send_message_sendmail(smtpserver, msg_str)
     else:
@@ -259,7 +300,8 @@ def __send_message(type, tmpl, options, *args):
     if type == 'cover' or (type == 'patch' and patch_nr < total_nr):
         sleep = options.sleep or config.getint('stgit.smtpdelay')
         time.sleep(sleep)
-    out.done()
+    if not options.git:
+        out.done()
     return msg_id
 
 def __update_header(msg, header, addr = '', ignore = ()):

^ permalink raw reply related

* [StGit PATCH v2 3/6] stg mail: make __send_message do more
From: Alex Chiang @ 2009-12-02  0:46 UTC (permalink / raw)
  To: catalin.marinas; +Cc: git, Karl Wiberg
In-Reply-To: <20091202003503.7737.51579.stgit@bob.kio>

Factor out the common code required to send either a cover mail
or patch, and implement it in __send_message.

WRY? DRY.

Cc: Karl Wiberg <kha@treskal.com>
Signed-off-by: Alex Chiang <achiang@hp.com>
---

 stgit/commands/mail.py |   65 +++++++++++++++++++++---------------------------
 1 files changed, 29 insertions(+), 36 deletions(-)

diff --git a/stgit/commands/mail.py b/stgit/commands/mail.py
index 35194a8..edff878 100644
--- a/stgit/commands/mail.py
+++ b/stgit/commands/mail.py
@@ -228,17 +228,39 @@ def __send_message_smtp(smtpserver, from_addr, to_addr_list, msg, options):
 
     s.quit()
 
-def __send_message(from_addr, to_addr_list, msg, options):
+def __send_message(type, tmpl, options, *args):
     """Message sending dispatcher.
     """
-    smtpserver = options.smtp_server or config.get('stgit.smtpserver')
+    (build, outstr) = {'cover': (__build_cover, 'the cover message'),
+                       'patch': (__build_message, 'patch "%s"' % args[0])}[type]
+    if type == 'patch':
+        (patch_nr, total_nr) = (args[1], args[2])
+
+    msg_id = email.Utils.make_msgid('stgit')
+    msg = build(tmpl, msg_id, options, *args)
+
+    from_addr, to_addrs = __parse_addresses(msg)
+    msg_str = msg.as_string(options.mbox)
+    if options.mbox:
+        out.stdout_raw(msg_str + '\n')
+        return msg_id
+
+    out.start('Sending ' + outstr)
 
+    smtpserver = options.smtp_server or config.get('stgit.smtpserver')
     if smtpserver.startswith('/'):
         # Use the sendmail tool
-        __send_message_sendmail(smtpserver, msg)
+        __send_message_sendmail(smtpserver, msg_str)
     else:
         # Use the SMTP server (we have host and port information)
-        __send_message_smtp(smtpserver, from_addr, to_addr_list, msg, options)
+        __send_message_smtp(smtpserver, from_addr, to_addrs, msg_str, options)
+
+    # give recipients a chance of receiving related patches in correct order
+    if type == 'cover' or (type == 'patch' and patch_nr < total_nr):
+        sleep = options.sleep or config.getint('stgit.smtpdelay')
+        time.sleep(sleep)
+    out.done()
+    return msg_id
 
 def __build_address_headers(msg, options, extra_cc = []):
     """Build the address headers and check existing headers in the
@@ -584,7 +606,6 @@ def func(parser, options, args):
     else:
         ref_id = None
 
-    sleep = options.sleep or config.getint('stgit.smtpdelay')
 
     # send the cover message (if any)
     if options.cover or options.edit_cover:
@@ -599,24 +620,12 @@ def func(parser, options, args):
             if not tmpl:
                 raise CmdException, 'No cover message template file found'
 
-        msg_id = email.Utils.make_msgid('stgit')
-        msg = __build_cover(tmpl, msg_id, options, patches)
-        from_addr, to_addr_list = __parse_addresses(msg)
-
-        msg_string = msg.as_string(options.mbox)
+        msg_id = __send_message('cover', tmpl, options, patches)
 
         # subsequent e-mails are seen as replies to the first one
         if not options.noreply:
             ref_id = msg_id
 
-        if options.mbox:
-            out.stdout_raw(msg_string + '\n')
-        else:
-            out.start('Sending the cover message')
-            __send_message(from_addr, to_addr_list, msg_string, options)
-            time.sleep(sleep)
-            out.done()
-
     # send the patches
     if options.template:
         tmpl = file(options.template).read()
@@ -628,25 +637,9 @@ def func(parser, options, args):
         if not tmpl:
             raise CmdException, 'No e-mail template file found'
 
-    for (p, patch_nr) in zip(patches, range(1, total_nr + 1)):
-        msg_id = email.Utils.make_msgid('stgit')
-        msg = __build_message(tmpl, msg_id, options, p, patch_nr, total_nr,
-                              ref_id)
-        from_addr, to_addr_list = __parse_addresses(msg)
-
-        msg_string = msg.as_string(options.mbox)
+    for (p, n) in zip(patches, range(1, total_nr + 1)):
+        msg_id = __send_message('patch', tmpl, options, p, n, total_nr, ref_id)
 
         # subsequent e-mails are seen as replies to the first one
         if not options.noreply and not options.unrelated and not ref_id:
             ref_id = msg_id
-
-        if options.mbox:
-            out.stdout_raw(msg_string + '\n')
-        else:
-            out.start('Sending patch "%s"' % p)
-            __send_message(from_addr, to_addr_list, msg_string, options)
-            # give recipients a chance of receiving related patches in the
-            # correct order.
-            if patch_nr < total_nr:
-                time.sleep(sleep)
-            out.done()

^ permalink raw reply related

* [StGit PATCH v2 6/6] stg mail: don't parse To/Cc/Bcc in --git mode
From: Alex Chiang @ 2009-12-02  0:46 UTC (permalink / raw)
  To: catalin.marinas; +Cc: git, Karl Wiberg
In-Reply-To: <20091202003503.7737.51579.stgit@bob.kio>

When using stg mail in --git mode, do not parse command-line To/Cc/Bcc
addresses.

Instead, we pass them directly to git send-email.

This allows us to leverage git send-email's support for email aliases.

Cc: Karl Wiberg <kha@treskal.com>
Signed-off-by: Alex Chiang <achiang@hp.com>
---

 stgit/commands/mail.py |   14 +++++---------
 1 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/stgit/commands/mail.py b/stgit/commands/mail.py
index 8a33c22..d108e75 100644
--- a/stgit/commands/mail.py
+++ b/stgit/commands/mail.py
@@ -247,12 +247,6 @@ def __send_message_git(msg, options):
         if getattr(options, x):
             cmd.extend('--%s=%s' % (x, a) for a in getattr(options, x))
 
-    # XXX: hack for now so that we don't duplicate To/Cc/Bcc headers
-    # in the mail, as git send-email inserts those for us.
-    del msg['To']
-    del msg['Cc']
-    del msg['Bcc']
-
     (fd, path) = mkstemp()
     os.write(fd, msg.as_string(options.mbox))
     os.close(fd)
@@ -277,13 +271,13 @@ def __send_message(type, tmpl, options, *args):
     msg_id = email.Utils.make_msgid('stgit')
     msg = build(tmpl, msg_id, options, *args)
 
-    from_addr, to_addrs = __parse_addresses(msg)
     msg_str = msg.as_string(options.mbox)
     if options.mbox:
         out.stdout_raw(msg_str + '\n')
         return msg_id
 
     if not options.git:
+        from_addr, to_addrs = __parse_addresses(msg)
         out.start('Sending ' + outstr)
 
     smtpserver = options.smtp_server or config.get('stgit.smtpserver')
@@ -499,7 +493,8 @@ def __build_cover(tmpl, msg_id, options, patches):
     except Exception, ex:
         raise CmdException, 'template parsing error: %s' % str(ex)
 
-    __build_address_headers(msg, options)
+    if not options.git:
+        __build_address_headers(msg, options)
     __build_extra_headers(msg, msg_id, options.refid)
     __encode_message(msg)
 
@@ -609,7 +604,8 @@ def __build_message(tmpl, msg_id, options, patch, patch_nr, total_nr, ref_id):
     else:
         extra_cc = []
 
-    __build_address_headers(msg, options, extra_cc)
+    if not options.git:
+        __build_address_headers(msg, options, extra_cc)
     __build_extra_headers(msg, msg_id, ref_id)
     __encode_message(msg)
 

^ permalink raw reply related

* [PATCH] Fixed typo
From: Richard Hartmann @ 2009-12-02  1:35 UTC (permalink / raw)
  To: git

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

Hi all,

please see attached.


Richard

[-- Attachment #2: 0001-Typos-commiting-committing.patch --]
[-- Type: text/x-diff, Size: 1751 bytes --]

From 138b95638693b47251d61ed5f316b6e68002b766 Mon Sep 17 00:00:00 2001
From: Richard Hartmann <richih.mailinglist@gmail.com>
Date: Wed, 2 Dec 2009 02:33:17 +0100
Subject: [PATCH] Typos: commiting -> committing

---
 builtin-revert.c |    2 +-
 t/t7001-mv.sh    |    8 ++++----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/builtin-revert.c b/builtin-revert.c
index 151aa6a..5708908 100644
--- a/builtin-revert.c
+++ b/builtin-revert.c
@@ -216,7 +216,7 @@ static char *help_msg(const unsigned char *sha1)
 
 	if (action == CHERRY_PICK) {
 		sprintf(helpbuf + strlen(helpbuf),
-			"\nWhen commiting, use the option "
+			"\nWhen committing, use the option "
 			"'-c %s' to retain authorship and message.",
 			find_unique_abbrev(sha1, DEFAULT_ABBREV));
 	}
diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh
index 10b8f8c..ad93a97 100755
--- a/t/t7001-mv.sh
+++ b/t/t7001-mv.sh
@@ -16,7 +16,7 @@ test_expect_success \
 
 # in path0 currently
 test_expect_success \
-    'commiting the change' \
+    'committing the change' \
     'cd .. && git commit -m move-out -a'
 
 test_expect_success \
@@ -30,7 +30,7 @@ test_expect_success \
 
 # in path0 currently
 test_expect_success \
-    'commiting the change' \
+    'committing the change' \
     'cd .. && git commit -m move-in -a'
 
 test_expect_success \
@@ -82,7 +82,7 @@ test_expect_success \
     'git mv path0 path2'
 
 test_expect_success \
-    'commiting the change' \
+    'committing the change' \
     'git commit -m dir-move -a'
 
 test_expect_success \
@@ -101,7 +101,7 @@ test_expect_success \
     'git mv path2 path1'
 
 test_expect_success \
-    'commiting the change' \
+    'committing the change' \
     'git commit -m dir-move -a'
 
 test_expect_success \
-- 
1.6.5.2


^ permalink raw reply related

* Re: [PATCH/RFC] Allow curl to rewind the RPC read buffer at any time
From: Tay Ray Chuan @ 2009-12-02  2:03 UTC (permalink / raw)
  To: Daniel Stenberg
  Cc: Shawn O. Pearce, Martin Storsj?, git, Nicholas Miell, gsky51,
	Clemens Buchacher, Mark Lodato, Johannes Schindelin
In-Reply-To: <alpine.DEB.2.00.0912011914270.30348@tvnag.unkk.fr>

Hi,

On Wed, Dec 2, 2009 at 2:18 AM, Daniel Stenberg <daniel@haxx.se> wrote:
> If '#@!*@!*' is your pattern for matching libcurl or curl, then sure libcurl
> certainly has no problem at all to send as many requests you like
> back-to-back.

I have a feeling Shawn's referring to the git http library on top of that. ;)

> The rewinding business is only really necessary for multipass authentication
> when Expect: 100-continue doesn't work (and thus libcurl has started to send
> data that the server will discard and thus is needed to get sent again). And
> that's not something you can blame "the #@!*@!* library" for, but rather
> your server end and/or how HTTP is defined to work.

According to Martin, Expect: 100-continue is not working due to libcurl.

I quote him:

Date: Tue, 1 Dec 2009 12:28:26 +0200 (EET)
Subject: Re: [PATCH 0/2] http: allow multi-pass authentication

On Tue, Dec 1, 2009 at 6:28 PM, Martin Storsjö <martin@martin.st> wrote:
> Normally, libcurl should add the Expect: 100-continue header
> automatically, but for some reason
> (http://article.gmane.org/gmane.comp.web.curl.library/25992) it doesn't,
> so that's probably why we're manually adding that header in
> remote-curl.c:371 at the moment. libcurl doesn't detect this at the moment
> (http://article.gmane.org/gmane.comp.web.curl.library/25991) so it won't
> wait for the 100 continue response before starting to send the body data.

But, again, don't read my blaming of libcurl for this 100 business as
a criticism of curl.

-- 
Cheers,
Ray Chuan

^ permalink raw reply

* Re: multiple working directories for long-running builds
From: Junio C Hamano @ 2009-12-02  2:09 UTC (permalink / raw)
  To: Dmitry Potapov; +Cc: The Git Mailing List
In-Reply-To: <20091201211830.GE11235@dpotapov.dyndns.org>

Dmitry Potapov <dpotapov@gmail.com> writes:

> On Tue, Dec 01, 2009 at 01:58:05PM -0500, Greg A. Woods wrote:
>> 
>> > > I just disagreed that "git archive" was a reasonable alternative to
>> > > leaving the working directory alone during the entire time of the build.
>> > 
>> > Using "git archive" allows you avoid running long time procedure such as
>> > full clean build and testing in the working tree. Also, it is guaranteed
>> > that you test exactly what you put in Git and some other garbage in your
>> > working tree does not affect the result.
>> 
>> Sure, but let's be very clear here:  "git archive" is likely even more
>> impossible for some large projects to use than "git clone" would be to
>> use to create build directories.
>
> AFAIK, "git archive" is cheaper than git clone. I do not say it is fast
> for huge project, but if you want to run a process such as clean build
> and test that takes a long time anyway, it does not add much to the
> total time.

I do not understand people who advocate for "git archive" to be used in
this manner at all.

I do use a set of separate build directories, and I typically run 5 to 10
full builds (in each) per day, but I rarely if ever make fix in them.
Perhaps the usage pattern expected by people who want others to use "git
archive" to prepare separate build directories may be different from how I
use them for.

I see two downsides in using "git archive":

 - "archive" piped to "tar xf -" will overwrite _all_ files every time you
   refresh the build area, causing extra work on "make" and any build
   procedure based on file timestamps.  Sure, you can work it around by
   using ccache but why make your life complicated?

 - When a build in these separate build areas fails, you would want to go
   there and try to diagnose or even fix the problem in there, not in your
   primary working area (after all, the whole point of keeping a separate
   build area is so that you do not have to switch branches too much in
   the primary working area).  A directory structure prepared by "archive"
   piped to "tar xf -" however is not a work tree, and any experimental
   changes (e.g. "debugf()") or fixes you make there need to be reverted
   or taken back manually to be placed in the primary working area.

If your build area is prepared with new-workdir, then you share the
history and you even share the ref namespace, so that "reset --hard" will
remove all the debugf() added while diagnosing, and "diff" will give you
the patch you need to take home.

You could even make a commit from your build area, but this cuts both
ways.  You need to be aware that after committing on a branch in one
repository other repositories that have the same branch checked out will
become out of sync.  It is however less of an issue in practice, because
the build areas are typically used to check out integration branches
(e.g. 'master' and 'next' in git.git) that you do not directly commit
anyway, and you will get very aware of the tentative nature of the tree,
as the update procedure for such a build area prepared with new-workdir is
always:

    cd /buildfarm/<branch>/ && git reset --hard

This will not touch any file that do not have to get updated, so your
"make" won't get confused.

^ permalink raw reply

* [RFC/PATCHv9 00/11] git notes
From: Johan Herland @ 2009-12-02  2:09 UTC (permalink / raw)
  To: git; +Cc: gitster, johan, spearce

Hi,

Here is the 9th iteration of the git-notes series. Changes in this
iteration are as follows:

Changes to existing patches:

- Rebased back onto the early part of the jh/notes series, as
  suggested by Junio.

- Minor style fixes to some of the patches.

- The git-fast-import patch has been heavily rewritten after
  suggestions from Shawn. The new patch is much less intrusive,
  by way of NOT interfacing with the notes API, but doing the
  fanout strategy directly in fast-import.c instead.

- The new fast-import patch no longer depends on changes in the
  notes API. The fast-import-related patches are therefore moved
  to the head of the patch series, and the notes API changes that
  are no longer necessary (but still worthwhile, IMHO) are moved
  to the tail.

If Shawn is OK with the fast-import patch, I believe that at least
patches #1 - #3 (and possibly #4 - #5) are ready for 'next'.

Patches #6 - #11 drastically extend the notes API. Since there are
currently no users of that API, and it has not been discussed much
on the list (although these patches have already been present in a
couple of iterations), I would still consider them RFC quality.


TODO:
- Builtin-ify git-notes shell script to take advantage of notes API
- Garbage collect notes whose referenced object is unreachable (gc_notes())
- Handle note objects that are not blobs, but trees


Have fun! :)

...Johan


Johan Herland (11):
  fast-import: Proper notes tree manipulation
  Rename t9301 to t9350, to make room for more fast-import tests
  Add more testcases to test fast-import of notes
  Minor style fixes to notes.c
  Notes API: get_commit_notes() -> format_note() + remove the commit restriction
  Notes API: init_notes(): Initialize the notes tree from the given notes ref
  Notes API: add_note(): Add note objects to the internal notes tree structure
  Notes API: get_note(): Return the note annotating the given object
  Notes API: for_each_note(): Traverse the entire notes tree with a callback
  Notes API: Allow multiple concurrent notes trees with new struct notes_tree
  Refactor notes concatenation into a flexible interface for combining notes

 fast-import.c                                    |  141 +++++-
 notes.c                                          |  345 ++++++++++----
 notes.h                                          |  114 ++++-
 pretty.c                                         |    9 +-
 t/t9300-fast-import.sh                           |  156 ++++++-
 t/t9301-fast-import-notes.sh                     |  578 ++++++++++++++++++++++
 t/{t9301-fast-export.sh => t9350-fast-export.sh} |    0
 7 files changed, 1218 insertions(+), 125 deletions(-)
 create mode 100755 t/t9301-fast-import-notes.sh
 rename t/{t9301-fast-export.sh => t9350-fast-export.sh} (100%)

^ permalink raw reply

* [RFC/PATCHv9 02/11] Rename t9301 to t9350, to make room for more fast-import tests
From: Johan Herland @ 2009-12-02  2:09 UTC (permalink / raw)
  To: git; +Cc: gitster, johan, spearce
In-Reply-To: <1259719783-4674-1-git-send-email-johan@herland.net>

Signed-off-by: Johan Herland <johan@herland.net>
---
 t/{t9301-fast-export.sh => t9350-fast-export.sh} |    0
 1 files changed, 0 insertions(+), 0 deletions(-)
 rename t/{t9301-fast-export.sh => t9350-fast-export.sh} (100%)

diff --git a/t/t9301-fast-export.sh b/t/t9350-fast-export.sh
similarity index 100%
rename from t/t9301-fast-export.sh
rename to t/t9350-fast-export.sh
-- 
1.6.5.3.433.g11067

^ permalink raw reply

* [RFC/PATCHv9 01/11] fast-import: Proper notes tree manipulation
From: Johan Herland @ 2009-12-02  2:09 UTC (permalink / raw)
  To: git; +Cc: gitster, johan, spearce
In-Reply-To: <1259719783-4674-1-git-send-email-johan@herland.net>

This patch teaches 'git fast-import' to automatically organize note objects
in a fast-import stream into an appropriate fanout structure. The notes API
in notes.h is NOT used to accomplish this, because trying to keep the
fast-import and notes data structures in sync would yield a significantly
larger patch with higher complexity.

Note objects are added to the fast-import tree structure with special mode
bits set, so that they can be recognized and restructured on-demand. The
special mode bits are ignored when generating the containing tree object,
hence the special mode bits are never visible externally.

The code keeps track of the number of note objects per branch through a
simple counter, and if/when the number of notes warrant a different fanout
level, the branch tree is traversed, renaming note objects into the
location dictated by the new fanout level.

Since note objects are stored in the same tree structure as other objects,
the unloading and reloading of a fast-import branches handle note objects
transparently.

Signed-off-by: Johan Herland <johan@herland.net>
---

On Thursday 26 November 2009, Shawn O. Pearce wrote:
> Yea, I agree, I'm not happy with the amount of complex code added
> to implement this.  But I can't say there's a better way to do it
> and still reuse the notes code.  Maybe its just worth breaking away
> from the notes code altogether?  fast-import also implements its
> own pack formatting functions because reusing them from pack-objects
> was just too ugly.

Ok, here is the promised rewrite that does it all within fast-import
instead of reusing the notes code. It's much smaller, both in size,
and in the impact on the existing code. I'm certainly a lot happier
with this patch, than with the previous iteration.


Have fun! :)

...Johan


 fast-import.c          |  141 +++++++++++++++++++++++++++++++++++++++++---
 t/t9300-fast-import.sh |  156 ++++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 276 insertions(+), 21 deletions(-)

diff --git a/fast-import.c b/fast-import.c
index b41d29f..b51ffbc 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -161,6 +161,7 @@ Format of STDIN stream:
 #define MAX_PACK_ID ((1<<PACK_ID_BITS)-1)
 #define DEPTH_BITS 13
 #define MAX_DEPTH ((1<<DEPTH_BITS)-1)
+#define NOTE_MODE 0170000

 struct object_entry
 {
@@ -245,6 +246,7 @@ struct branch
 	const char *name;
 	struct tree_entry branch_tree;
 	uintmax_t last_commit;
+	unsigned int num_notes;
 	unsigned active : 1;
 	unsigned pack_id : PACK_ID_BITS;
 	unsigned char sha1[20];
@@ -693,6 +695,7 @@ static struct branch *new_branch(const char *name)
 	b->table_next_branch = branch_table[hc];
 	b->branch_tree.versions[0].mode = S_IFDIR;
 	b->branch_tree.versions[1].mode = S_IFDIR;
+	b->num_notes = 0;
 	b->active = 0;
 	b->pack_id = MAX_PACK_ID;
 	branch_table[hc] = b;
@@ -1306,10 +1309,12 @@ static void mktree(struct tree_content *t, int v, struct strbuf *b)
 	strbuf_grow(b, maxlen);
 	for (i = 0; i < t->entry_count; i++) {
 		struct tree_entry *e = t->entries[i];
-		if (!e->versions[v].mode)
+		unsigned int mode = (unsigned int) e->versions[v].mode;
+		if (!mode)
 			continue;
-		strbuf_addf(b, "%o %s%c", (unsigned int)e->versions[v].mode,
-					e->name->str_dat, '\0');
+		else if ((mode & NOTE_MODE) == NOTE_MODE)
+			mode = (mode & ~NOTE_MODE) | S_IFREG;
+		strbuf_addf(b, "%o %s%c", mode, e->name->str_dat, '\0');
 		strbuf_add(b, e->versions[v].sha1, 20);
 	}
 }
@@ -1860,6 +1865,115 @@ static void load_branch(struct branch *b)
 	}
 }

+static unsigned char convert_num_notes_to_fanout(unsigned int num_notes)
+{
+	unsigned char fanout = 0;
+	while ((num_notes >>= 8))
+		fanout++;
+	return fanout;
+}
+
+static void construct_path_with_fanout(const char *hex_sha1,
+		unsigned char fanout, char *path)
+{
+	unsigned int i = 0, j = 0;
+	assert(fanout < 20);
+	while (fanout) {
+		path[i++] = hex_sha1[j++];
+		path[i++] = hex_sha1[j++];
+		path[i++] = '/';
+		fanout--;
+	}
+	memcpy(path + i, hex_sha1 + j, 40 - j);
+	path[i + 40 - j] = '\0';
+}
+
+static int adjust_num_notes(struct tree_entry *root, const char *p,
+		const unsigned char *sha1)
+{
+	/* Return -1/0/1 indicating how storing sha1 at p affects #notes */
+	struct tree_entry leaf;
+	int delete_note = is_null_sha1(sha1) ? 1 : 0;
+	int nonexisting_note = !(
+		tree_content_get(root, p, &leaf) &&
+		!is_null_sha1(leaf.versions[1].sha1) &&
+		(leaf.versions[1].mode & NOTE_MODE) == NOTE_MODE) ? 1 : 0;
+	return nonexisting_note - delete_note;
+}
+
+static unsigned int do_change_note_fanout(
+		struct tree_entry *orig_root, struct tree_entry *root,
+		char *hex_sha1, unsigned int hex_sha1_len,
+		char *fullpath, unsigned int fullpath_len,
+		unsigned char fanout)
+{
+	struct tree_content *t = root->tree;
+	struct tree_entry *e, leaf;
+	unsigned int i, tmp_hex_sha1_len, tmp_fullpath_len, num_notes = 0;
+	unsigned char sha1[20];
+	char realpath[60];
+	int is_note;
+
+	for (i = 0; i < t->entry_count; i++) {
+		e = t->entries[i];
+		is_note = (e->versions[1].mode & NOTE_MODE) == NOTE_MODE;
+		tmp_hex_sha1_len = hex_sha1_len + e->name->str_len;
+		tmp_fullpath_len = fullpath_len;
+
+		if (tmp_hex_sha1_len <= 40 && e->name->str_len >= 2) {
+			memcpy(hex_sha1 + hex_sha1_len, e->name->str_dat,
+				e->name->str_len);
+			if (tmp_fullpath_len)
+				fullpath[tmp_fullpath_len++] = '/';
+			memcpy(fullpath + tmp_fullpath_len, e->name->str_dat,
+				e->name->str_len);
+			tmp_fullpath_len += e->name->str_len;
+			assert(tmp_fullpath_len < 60);
+			fullpath[tmp_fullpath_len] = '\0';
+		} else {
+			assert(!is_note);
+			continue;
+		}
+
+		if (is_note) {
+			num_notes++;
+			assert(tmp_hex_sha1_len == 40);
+			if (get_sha1_hex(hex_sha1, sha1))
+				die("Invalid SHA1 sum %.40s", hex_sha1);
+			construct_path_with_fanout(hex_sha1, fanout, realpath);
+			if (!strcmp(fullpath, realpath))
+				continue; /* note is already in right place */
+
+			/* Rename fullpath to realpath */
+			if (!tree_content_remove(orig_root, fullpath, &leaf))
+				die("Failed to remove path %s", fullpath);
+			if (!leaf.versions[1].mode)
+				die("Path %s not in branch", fullpath);
+			tree_content_set(orig_root, realpath,
+				leaf.versions[1].sha1,
+				leaf.versions[1].mode,
+				leaf.tree);
+		} else if (tmp_hex_sha1_len < 40 && S_ISDIR(e->versions[1].mode)) {
+			/* Found a subdir that may contain a note */
+			num_notes += do_change_note_fanout(orig_root, e,
+				hex_sha1, tmp_hex_sha1_len,
+				fullpath, tmp_fullpath_len, fanout);
+		}
+
+		/* The above may have reallocated the current tree_content */
+		if (t != root->tree)
+			t = root->tree;
+	}
+	return num_notes;
+}
+
+static unsigned int change_note_fanout(struct tree_entry *root,
+		unsigned char fanout)
+{
+	char hex_sha1[40], path[60];
+	return do_change_note_fanout(root, root, hex_sha1, 0, path, 0, fanout);
+}
+
 static void file_change_m(struct branch *b)
 {
 	const char *p = command_buf.buf + 2;
@@ -2010,14 +2124,15 @@ static void file_change_cr(struct branch *b, int rename)
 		leaf.tree);
 }

-static void note_change_n(struct branch *b)
+static void note_change_n(struct branch *b, unsigned char fanout)
 {
 	const char *p = command_buf.buf + 2;
 	static struct strbuf uq = STRBUF_INIT;
 	struct object_entry *oe = oe;
 	struct branch *s;
 	unsigned char sha1[20], commit_sha1[20];
-	uint16_t inline_data = 0;
+	char path[60];
+	uint16_t inline_data = 0, mode;

 	/* <dataref> or 'inline' */
 	if (*p == ':') {
@@ -2080,8 +2195,10 @@ static void note_change_n(struct branch *b)
 			    typename(type), command_buf.buf);
 	}

-	tree_content_set(&b->branch_tree, sha1_to_hex(commit_sha1), sha1,
-		S_IFREG | 0644, NULL);
+	construct_path_with_fanout(sha1_to_hex(commit_sha1), fanout, path);
+	b->num_notes += adjust_num_notes(&b->branch_tree, path, sha1);
+	mode = (is_null_sha1(sha1) ? S_IFREG : NOTE_MODE) | 0644;
+	tree_content_set(&b->branch_tree, path, sha1, mode, NULL);
 }

 static void file_change_deleteall(struct branch *b)
@@ -2090,6 +2207,7 @@ static void file_change_deleteall(struct branch *b)
 	hashclr(b->branch_tree.versions[0].sha1);
 	hashclr(b->branch_tree.versions[1].sha1);
 	load_tree(&b->branch_tree);
+	b->num_notes = 0;
 }

 static void parse_from_commit(struct branch *b, char *buf, unsigned long size)
@@ -2213,6 +2331,7 @@ static void parse_new_commit(void)
 	char *committer = NULL;
 	struct hash_list *merge_list = NULL;
 	unsigned int merge_count;
+	unsigned char prev_fanout, new_fanout;

 	/* Obtain the branch name from the rest of our command */
 	sp = strchr(command_buf.buf, ' ') + 1;
@@ -2243,6 +2362,8 @@ static void parse_new_commit(void)
 		load_branch(b);
 	}

+	prev_fanout = convert_num_notes_to_fanout(b->num_notes);
+
 	/* file_change* */
 	while (command_buf.len > 0) {
 		if (!prefixcmp(command_buf.buf, "M "))
@@ -2254,7 +2375,7 @@ static void parse_new_commit(void)
 		else if (!prefixcmp(command_buf.buf, "C "))
 			file_change_cr(b, 0);
 		else if (!prefixcmp(command_buf.buf, "N "))
-			note_change_n(b);
+			note_change_n(b, prev_fanout);
 		else if (!strcmp("deleteall", command_buf.buf))
 			file_change_deleteall(b);
 		else {
@@ -2265,6 +2386,10 @@ static void parse_new_commit(void)
 			break;
 	}

+	new_fanout = convert_num_notes_to_fanout(b->num_notes);
+	if (new_fanout != prev_fanout)
+		b->num_notes = change_note_fanout(&b->branch_tree, new_fanout);
+
 	/* build the tree and the commit */
 	store_tree(&b->branch_tree);
 	hashcpy(b->branch_tree.versions[0].sha1,
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index b49815d..bf8c509 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -1092,9 +1092,12 @@ test_expect_success 'P: fail on blob mark in gitlink' '
 ### series Q (notes)
 ###

-note1_data="Note for the first commit"
-note2_data="Note for the second commit"
-note3_data="Note for the third commit"
+note1_data="The first note for the first commit"
+note2_data="The first note for the second commit"
+note3_data="The first note for the third commit"
+note1b_data="The second note for the first commit"
+note1c_data="The third note for the first commit"
+note2b_data="The second note for the second commit"

 test_tick
 cat >input <<INPUT_END
@@ -1169,7 +1172,45 @@ data <<EOF
 $note3_data
 EOF

+commit refs/notes/foobar
+mark :10
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+notes (:10)
+COMMIT
+
+N inline :3
+data <<EOF
+$note1b_data
+EOF
+
+commit refs/notes/foobar2
+mark :11
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+notes (:11)
+COMMIT
+
+N inline :3
+data <<EOF
+$note1c_data
+EOF
+
+commit refs/notes/foobar
+mark :12
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+notes (:12)
+COMMIT
+
+deleteall
+N inline :5
+data <<EOF
+$note2b_data
+EOF
+
 INPUT_END
+
 test_expect_success \
 	'Q: commit notes' \
 	'git fast-import <input &&
@@ -1224,8 +1265,8 @@ committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
 notes (:9)
 EOF
 test_expect_success \
-	'Q: verify notes commit' \
-	'git cat-file commit refs/notes/foobar | sed 1d >actual &&
+	'Q: verify first notes commit' \
+	'git cat-file commit refs/notes/foobar~2 | sed 1d >actual &&
 	test_cmp expect actual'

 cat >expect.unsorted <<EOF
@@ -1235,23 +1276,112 @@ cat >expect.unsorted <<EOF
 EOF
 cat expect.unsorted | sort >expect
 test_expect_success \
-	'Q: verify notes tree' \
-	'git cat-file -p refs/notes/foobar^{tree} | sed "s/ [0-9a-f]*	/ /" >actual &&
+	'Q: verify first notes tree' \
+	'git cat-file -p refs/notes/foobar~2^{tree} | sed "s/ [0-9a-f]*	/ /" >actual &&
 	 test_cmp expect actual'

 echo "$note1_data" >expect
 test_expect_success \
-	'Q: verify note for first commit' \
-	'git cat-file blob refs/notes/foobar:$commit1 >actual && test_cmp expect actual'
+	'Q: verify first note for first commit' \
+	'git cat-file blob refs/notes/foobar~2:$commit1 >actual && test_cmp expect actual'

 echo "$note2_data" >expect
 test_expect_success \
-	'Q: verify note for second commit' \
-	'git cat-file blob refs/notes/foobar:$commit2 >actual && test_cmp expect actual'
+	'Q: verify first note for second commit' \
+	'git cat-file blob refs/notes/foobar~2:$commit2 >actual && test_cmp expect actual'
+
+echo "$note3_data" >expect
+test_expect_success \
+	'Q: verify first note for third commit' \
+	'git cat-file blob refs/notes/foobar~2:$commit3 >actual && test_cmp expect actual'
+
+cat >expect <<EOF
+parent `git rev-parse --verify refs/notes/foobar~2`
+author $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+
+notes (:10)
+EOF
+test_expect_success \
+	'Q: verify second notes commit' \
+	'git cat-file commit refs/notes/foobar^ | sed 1d >actual &&
+	test_cmp expect actual'
+
+cat >expect.unsorted <<EOF
+100644 blob $commit1
+100644 blob $commit2
+100644 blob $commit3
+EOF
+cat expect.unsorted | sort >expect
+test_expect_success \
+	'Q: verify second notes tree' \
+	'git cat-file -p refs/notes/foobar^^{tree} | sed "s/ [0-9a-f]*	/ /" >actual &&
+	 test_cmp expect actual'
+
+echo "$note1b_data" >expect
+test_expect_success \
+	'Q: verify second note for first commit' \
+	'git cat-file blob refs/notes/foobar^:$commit1 >actual && test_cmp expect actual'
+
+echo "$note2_data" >expect
+test_expect_success \
+	'Q: verify first note for second commit' \
+	'git cat-file blob refs/notes/foobar^:$commit2 >actual && test_cmp expect actual'

 echo "$note3_data" >expect
 test_expect_success \
-	'Q: verify note for third commit' \
-	'git cat-file blob refs/notes/foobar:$commit3 >actual && test_cmp expect actual'
+	'Q: verify first note for third commit' \
+	'git cat-file blob refs/notes/foobar^:$commit3 >actual && test_cmp expect actual'
+
+cat >expect <<EOF
+author $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+
+notes (:11)
+EOF
+test_expect_success \
+	'Q: verify third notes commit' \
+	'git cat-file commit refs/notes/foobar2 | sed 1d >actual &&
+	test_cmp expect actual'
+
+cat >expect.unsorted <<EOF
+100644 blob $commit1
+EOF
+cat expect.unsorted | sort >expect
+test_expect_success \
+	'Q: verify third notes tree' \
+	'git cat-file -p refs/notes/foobar2^{tree} | sed "s/ [0-9a-f]*	/ /" >actual &&
+	 test_cmp expect actual'
+
+echo "$note1c_data" >expect
+test_expect_success \
+	'Q: verify third note for first commit' \
+	'git cat-file blob refs/notes/foobar2:$commit1 >actual && test_cmp expect actual'
+
+cat >expect <<EOF
+parent `git rev-parse --verify refs/notes/foobar^`
+author $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+
+notes (:12)
+EOF
+test_expect_success \
+	'Q: verify fourth notes commit' \
+	'git cat-file commit refs/notes/foobar | sed 1d >actual &&
+	test_cmp expect actual'
+
+cat >expect.unsorted <<EOF
+100644 blob $commit2
+EOF
+cat expect.unsorted | sort >expect
+test_expect_success \
+	'Q: verify fourth notes tree' \
+	'git cat-file -p refs/notes/foobar^{tree} | sed "s/ [0-9a-f]*	/ /" >actual &&
+	 test_cmp expect actual'
+
+echo "$note2b_data" >expect
+test_expect_success \
+	'Q: verify second note for second commit' \
+	'git cat-file blob refs/notes/foobar:$commit2 >actual && test_cmp expect actual'

 test_done
--
1.6.5.3.433.g11067

^ permalink raw reply related

* [RFC/PATCHv9 07/11] Notes API: add_note(): Add note objects to the internal notes tree structure
From: Johan Herland @ 2009-12-02  2:09 UTC (permalink / raw)
  To: git; +Cc: gitster, johan, spearce
In-Reply-To: <1259719783-4674-1-git-send-email-johan@herland.net>

Signed-off-by: Johan Herland <johan@herland.net>
---
 notes.c |   11 +++++++++++
 notes.h |    4 ++++
 2 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/notes.c b/notes.c
index 23e62dd..3c8a6e0 100644
--- a/notes.c
+++ b/notes.c
@@ -366,6 +366,17 @@ void init_notes(const char *notes_ref, int flags)
 	load_subtree(&root_tree, &root_node, 0);
 }
 
+void add_note(const unsigned char *object_sha1, const unsigned char *note_sha1)
+{
+	struct leaf_node *l;
+
+	assert(initialized);
+	l = (struct leaf_node *) xmalloc(sizeof(struct leaf_node));
+	hashcpy(l->key_sha1, object_sha1);
+	hashcpy(l->val_sha1, note_sha1);
+	note_tree_insert(&root_node, 0, l, PTR_TYPE_NOTE);
+}
+
 static unsigned char *lookup_notes(const unsigned char *object_sha1)
 {
 	struct leaf_node *found = note_tree_find(&root_node, 0, object_sha1);
diff --git a/notes.h b/notes.h
index 6b52799..5f22852 100644
--- a/notes.h
+++ b/notes.h
@@ -21,6 +21,10 @@
  */
 void init_notes(const char *notes_ref, int flags);
 
+/* Add the given note object to the internal notes tree structure */
+void add_note(const unsigned char *object_sha1,
+		const unsigned char *note_sha1);
+
 /* Free (and de-initialize) the internal notes tree structure */
 void free_notes(void);
 
-- 
1.6.5.3.433.g11067

^ permalink raw reply related

* [RFC/PATCHv9 03/11] Add more testcases to test fast-import of notes
From: Johan Herland @ 2009-12-02  2:09 UTC (permalink / raw)
  To: git; +Cc: gitster, johan, spearce
In-Reply-To: <1259719783-4674-1-git-send-email-johan@herland.net>

This patch adds testcases verifying correct behaviour in several scenarios
regarding fast-import of notes:
- using a mixture of 'N' and 'M' commands
- updating existing notes
- concatenation of notes
- 'deleteall' also removes notes
- fanout schemes is added/removed when needed
- git-fast-import's branch unload/reload preserves notes

Signed-off-by: Johan Herland <johan@herland.net>
---
 t/t9301-fast-import-notes.sh |  578 ++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 578 insertions(+), 0 deletions(-)
 create mode 100755 t/t9301-fast-import-notes.sh

diff --git a/t/t9301-fast-import-notes.sh b/t/t9301-fast-import-notes.sh
new file mode 100755
index 0000000..5a08a76
--- /dev/null
+++ b/t/t9301-fast-import-notes.sh
@@ -0,0 +1,578 @@
+#!/bin/sh
+#
+# Copyright (c) 2009 Johan Herland
+#
+
+test_description='test git fast-import of notes objects'
+. ./test-lib.sh
+
+
+test_tick
+cat >input <<INPUT_END
+commit refs/heads/master
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+first commit
+COMMIT
+
+M 644 inline foo
+data <<EOF
+file foo in first commit
+EOF
+
+M 755 inline bar
+data <<EOF
+file bar in first commit
+EOF
+
+M 644 inline baz/xyzzy
+data <<EOF
+file baz/xyzzy in first commit
+EOF
+
+commit refs/heads/master
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+second commit
+COMMIT
+
+M 644 inline foo
+data <<EOF
+file foo in second commit
+EOF
+
+M 755 inline baz/xyzzy
+data <<EOF
+file baz/xyzzy in second commit
+EOF
+
+commit refs/heads/master
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+third commit
+COMMIT
+
+M 644 inline foo
+data <<EOF
+file foo in third commit
+EOF
+
+commit refs/heads/master
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+fourth commit
+COMMIT
+
+M 755 inline bar
+data <<EOF
+file bar in fourth commit
+EOF
+
+INPUT_END
+
+test_expect_success 'set up master branch' '
+
+	git fast-import <input &&
+	git whatchanged master
+'
+
+commit4=$(git rev-parse refs/heads/master)
+commit3=$(git rev-parse "$commit4^")
+commit2=$(git rev-parse "$commit4~2")
+commit1=$(git rev-parse "$commit4~3")
+
+test_tick
+cat >input <<INPUT_END
+commit refs/notes/test
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+first notes commit
+COMMIT
+
+M 644 inline $commit1
+data <<EOF
+first note for first commit
+EOF
+
+M 755 inline $commit2
+data <<EOF
+first note for second commit
+EOF
+
+INPUT_END
+
+cat >expect <<EXPECT_END
+    fourth commit
+    third commit
+    second commit
+    first note for second commit
+    first commit
+    first note for first commit
+EXPECT_END
+
+test_expect_success 'add notes with simple M command' '
+
+	git fast-import <input &&
+	GIT_NOTES_REF=refs/notes/test git log | grep "^    " > actual &&
+	test_cmp expect actual
+
+'
+
+test_tick
+cat >input <<INPUT_END
+commit refs/notes/test
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+second notes commit
+COMMIT
+
+from refs/notes/test^0
+N inline $commit3
+data <<EOF
+first note for third commit
+EOF
+
+N inline $commit4
+data <<EOF
+first note for fourth commit
+EOF
+
+INPUT_END
+
+cat >expect <<EXPECT_END
+    fourth commit
+    first note for fourth commit
+    third commit
+    first note for third commit
+    second commit
+    first note for second commit
+    first commit
+    first note for first commit
+EXPECT_END
+
+test_expect_success 'add notes with simple N command' '
+
+	git fast-import <input &&
+	GIT_NOTES_REF=refs/notes/test git log | grep "^    " > actual &&
+	test_cmp expect actual
+
+'
+
+test_tick
+cat >input <<INPUT_END
+commit refs/notes/test
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+third notes commit
+COMMIT
+
+from refs/notes/test^0
+N inline $commit1
+data <<EOF
+second note for first commit
+EOF
+
+N inline $commit2
+data <<EOF
+second note for second commit
+EOF
+
+N inline $commit3
+data <<EOF
+second note for third commit
+EOF
+
+N inline $commit4
+data <<EOF
+second note for fourth commit
+EOF
+
+INPUT_END
+
+cat >expect <<EXPECT_END
+    fourth commit
+    second note for fourth commit
+    third commit
+    second note for third commit
+    second commit
+    second note for second commit
+    first commit
+    second note for first commit
+EXPECT_END
+
+test_expect_success 'update existing notes with N command' '
+
+	git fast-import <input &&
+	GIT_NOTES_REF=refs/notes/test git log | grep "^    " > actual &&
+	test_cmp expect actual
+
+'
+
+test_tick
+cat >input <<INPUT_END
+commit refs/notes/test
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+fourth notes commit
+COMMIT
+
+from refs/notes/test^0
+M 644 inline $(echo "$commit3" | sed "s|^..|&/|")
+data <<EOF
+prefix of note for third commit
+EOF
+
+M 644 inline $(echo "$commit4" | sed "s|^..|&/|")
+data <<EOF
+prefix of note for fourth commit
+EOF
+
+M 644 inline $(echo "$commit4" | sed "s|^\(..\)\(..\)|\1/\2/|")
+data <<EOF
+pre-prefix of note for fourth commit
+EOF
+
+N inline $commit1
+data <<EOF
+third note for first commit
+EOF
+
+N inline $commit2
+data <<EOF
+third note for second commit
+EOF
+
+N inline $commit3
+data <<EOF
+third note for third commit
+EOF
+
+N inline $commit4
+data <<EOF
+third note for fourth commit
+EOF
+
+
+INPUT_END
+
+cat >expect <<EXPECT_END
+    fourth commit
+    pre-prefix of note for fourth commit
+    prefix of note for fourth commit
+    third note for fourth commit
+    third commit
+    prefix of note for third commit
+    third note for third commit
+    second commit
+    third note for second commit
+    first commit
+    third note for first commit
+EXPECT_END
+
+test_expect_success 'add concatentation notes with M command' '
+
+	git fast-import <input &&
+	GIT_NOTES_REF=refs/notes/test git log | grep "^    " > actual &&
+	test_cmp expect actual
+
+'
+
+test_tick
+cat >input <<INPUT_END
+commit refs/notes/test
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+fifth notes commit
+COMMIT
+
+from refs/notes/test^0
+deleteall
+
+INPUT_END
+
+cat >expect <<EXPECT_END
+    fourth commit
+    third commit
+    second commit
+    first commit
+EXPECT_END
+
+test_expect_success 'verify that deleteall also removes notes' '
+
+	git fast-import <input &&
+	GIT_NOTES_REF=refs/notes/test git log | grep "^    " > actual &&
+	test_cmp expect actual
+
+'
+
+test_tick
+cat >input <<INPUT_END
+commit refs/notes/test
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+sixth notes commit
+COMMIT
+
+from refs/notes/test^0
+M 644 inline $commit1
+data <<EOF
+third note for first commit
+EOF
+
+M 644 inline $commit3
+data <<EOF
+third note for third commit
+EOF
+
+N inline $commit1
+data <<EOF
+fourth note for first commit
+EOF
+
+N inline $commit3
+data <<EOF
+fourth note for third commit
+EOF
+
+INPUT_END
+
+cat >expect <<EXPECT_END
+    fourth commit
+    third commit
+    fourth note for third commit
+    second commit
+    first commit
+    fourth note for first commit
+EXPECT_END
+
+test_expect_success 'verify that N commands override M commands' '
+
+	git fast-import <input &&
+	GIT_NOTES_REF=refs/notes/test git log | grep "^    " > actual &&
+	test_cmp expect actual
+
+'
+
+# Write fast-import commands to create the given number of commits
+fast_import_commits () {
+	my_ref=$1
+	my_num_commits=$2
+	my_append_to_file=$3
+	my_i=0
+	while test $my_i -lt $my_num_commits
+	do
+		my_i=$(($my_i + 1))
+		test_tick
+		cat >>"$my_append_to_file" <<INPUT_END
+commit $my_ref
+mark :$my_i
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+commit #$my_i
+COMMIT
+
+M 644 inline file
+data <<EOF
+file contents in commit #$my_i
+EOF
+
+INPUT_END
+	done
+}
+
+# Write fast-import commands to create the given number of notes annotating
+# the commits created by fast_import_commits()
+fast_import_notes () {
+	my_notes_ref=$1
+	my_num_commits=$2
+	my_append_to_file=$3
+	my_note_append=$4
+	test_tick
+	cat >>"$my_append_to_file" <<INPUT_END
+commit $my_notes_ref
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+committing $my_num_commits notes
+COMMIT
+
+INPUT_END
+
+	my_i=0
+	while test $my_i -lt $my_num_commits
+	do
+		my_i=$(($my_i + 1))
+		cat >>"$my_append_to_file" <<INPUT_END
+N inline :$my_i
+data <<EOF
+note for commit #$my_i$my_note_append
+EOF
+
+INPUT_END
+	done
+}
+
+
+rm input expect
+num_commits=400
+# Create lots of commits
+fast_import_commits "refs/heads/many_commits" $num_commits input
+# Create one note per above commit
+fast_import_notes "refs/notes/many_notes" $num_commits input
+# Finally create the expected output from all these notes and commits
+i=$num_commits
+while test $i -gt 0
+do
+	cat >>expect <<EXPECT_END
+    commit #$i
+    note for commit #$i
+EXPECT_END
+	i=$(($i - 1))
+done
+
+test_expect_success 'add lots of commits and notes' '
+
+	git fast-import <input &&
+	GIT_NOTES_REF=refs/notes/many_notes git log refs/heads/many_commits |
+	    grep "^    " > actual &&
+	test_cmp expect actual
+
+'
+
+test_expect_success 'verify that lots of notes trigger a fanout scheme' '
+
+	# None of the entries in the top-level notes tree should be a full SHA1
+	git cat-file -p refs/notes/many_notes: |
+	while read mode type sha1 path
+	do
+		path_len=$(expr length "$path") &&
+		if test $path_len -ge 40
+		then
+			return 1
+		fi
+	done
+
+'
+
+commit1=$(git rev-parse "refs/heads/many_commits")
+commit2=$(git rev-parse "$commit1^")
+commit3=$(git rev-parse "$commit2^")
+commit4=$(git rev-parse "$commit3^")
+
+test_tick
+cat >input <<INPUT_END
+commit refs/notes/many_notes
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+replacing many notes with few notes
+COMMIT
+
+from refs/notes/many_notes^0
+deleteall
+N inline $commit1
+data <<EOF
+note for commit #$num_commits
+EOF
+
+N inline $commit2
+data <<EOF
+note for commit #$(($num_commits - 1))
+EOF
+
+N inline $commit3
+data <<EOF
+note for commit #$(($num_commits - 2))
+EOF
+
+N inline $commit4
+data <<EOF
+note for commit #$(($num_commits - 3))
+EOF
+
+INPUT_END
+
+i=$num_commits
+rm expect
+while test $i -gt 0
+do
+	cat >>expect <<EXPECT_END
+    commit #$i
+EXPECT_END
+	if test $i -gt $(($num_commits - 4))
+	then
+		cat >>expect <<EXPECT_END
+    note for commit #$i
+EXPECT_END
+	fi
+	i=$(($i - 1))
+done
+
+test_expect_success 'remove lots of notes' '
+
+	git fast-import <input &&
+	GIT_NOTES_REF=refs/notes/many_notes git log refs/heads/many_commits |
+	    grep "^    " > actual &&
+	test_cmp expect actual
+
+'
+
+test_expect_success 'verify that removing notes trigger fanout consolidation' '
+
+	# All entries in the top-level notes tree should be a full SHA1
+	git cat-file -p refs/notes/many_notes: |
+	while read mode type sha1 path
+	do
+		path_len=$(expr length "$path") &&
+		if test $path_len -ne 40
+		then
+			return 1
+		fi
+	done
+
+'
+
+rm input expect
+num_notes_refs=10
+num_commits=16
+some_commits=8
+# Create commits
+fast_import_commits "refs/heads/more_commits" $num_commits input
+# Create one note per above commit per notes ref
+i=0
+while test $i -lt $num_notes_refs
+do
+	i=$(($i + 1))
+	fast_import_notes "refs/notes/more_notes_$i" $num_commits input
+done
+# Trigger branch reloading in git-fast-import by repeating the note creation
+i=0
+while test $i -lt $num_notes_refs
+do
+	i=$(($i + 1))
+	fast_import_notes "refs/notes/more_notes_$i" $some_commits input " (2)"
+done
+# Finally create the expected output from the notes in refs/notes/more_notes_1
+i=$num_commits
+while test $i -gt 0
+do
+	note_data="note for commit #$i"
+	if test $i -le $some_commits
+	then
+		note_data="$note_data (2)"
+	fi
+	cat >>expect <<EXPECT_END
+    commit #$i
+    $note_data
+EXPECT_END
+	i=$(($i - 1))
+done
+
+test_expect_success "add notes to $num_commits commits in each of $num_notes_refs refs" '
+
+	git fast-import --active-branches=5 <input &&
+	GIT_NOTES_REF=refs/notes/more_notes_1 git log refs/heads/more_commits |
+	    grep "^    " > actual &&
+	test_cmp expect actual
+
+'
+
+test_done
-- 
1.6.5.3.433.g11067

^ permalink raw reply related

* [RFC/PATCHv9 06/11] Notes API: init_notes(): Initialize the notes tree from the given notes ref
From: Johan Herland @ 2009-12-02  2:09 UTC (permalink / raw)
  To: git; +Cc: gitster, johan, spearce
In-Reply-To: <1259719783-4674-1-git-send-email-johan@herland.net>

Created by a simple refactoring of initialize_notes().

Also add a new 'flags' parameter, which is a bitwise combination of notes
initialization flags. For now, there is only one flag - NOTES_INIT_EMPTY -
which indicates that the notes tree should not auto-load the contents of
the given (or default) notes ref, but rather should leave the notes tree
initialized to an empty state. This will become useful in the future when
manipulating the notes tree through the notes API.

Signed-off-by: Johan Herland <johan@herland.net>
---
 notes.c |   27 ++++++++++++++++-----------
 notes.h |   20 ++++++++++++++++++++
 2 files changed, 36 insertions(+), 11 deletions(-)

diff --git a/notes.c b/notes.c
index dddca31..23e62dd 100644
--- a/notes.c
+++ b/notes.c
@@ -339,13 +339,25 @@ static void load_subtree(struct leaf_node *subtree, struct int_node *node,
 	free(buf);
 }
 
-static void initialize_notes(const char *notes_ref_name)
+void init_notes(const char *notes_ref, int flags)
 {
 	unsigned char sha1[20], object_sha1[20];
 	unsigned mode;
 	struct leaf_node root_tree;
 
-	if (!notes_ref_name || read_ref(notes_ref_name, object_sha1) ||
+	assert(!initialized);
+	initialized = 1;
+
+	if (!notes_ref) {
+		const char *env = getenv(GIT_NOTES_REF_ENVIRONMENT);
+		if (env)
+			notes_ref = getenv(GIT_NOTES_REF_ENVIRONMENT);
+		else
+			notes_ref = GIT_NOTES_DEFAULT_REF;
+	}
+
+	if (flags & NOTES_INIT_EMPTY || !notes_ref ||
+	    read_ref(notes_ref, object_sha1) ||
 	    get_tree_entry(object_sha1, "", sha1, &mode))
 		return;
 
@@ -378,15 +390,8 @@ void format_note(const unsigned char *object_sha1, struct strbuf *sb,
 	unsigned long linelen, msglen;
 	enum object_type type;
 
-	if (!initialized) {
-		const char *env = getenv(GIT_NOTES_REF_ENVIRONMENT);
-		if (env)
-			notes_ref_name = getenv(GIT_NOTES_REF_ENVIRONMENT);
-		else if (!notes_ref_name)
-			notes_ref_name = GIT_NOTES_DEFAULT_REF;
-		initialize_notes(notes_ref_name);
-		initialized = 1;
-	}
+	if (!initialized)
+		init_notes(NULL, 0);
 
 	sha1 = lookup_notes(object_sha1);
 	if (!sha1)
diff --git a/notes.h b/notes.h
index d745ed1..6b52799 100644
--- a/notes.h
+++ b/notes.h
@@ -1,6 +1,26 @@
 #ifndef NOTES_H
 #define NOTES_H
 
+/*
+ * Flags controlling behaviour of notes tree initialization
+ *
+ * Default behaviour is to initialize the notes tree from the tree object
+ * specified by the given (or default) notes ref.
+ */
+#define NOTES_INIT_EMPTY 1
+
+/*
+ * Initialize internal notes tree structure with the notes tree at the given
+ * ref. If given ref is NULL, the value of the $GIT_NOTES_REF environment
+ * variable is used, and if that is missing, the default notes ref is used
+ * ("refs/notes/commits").
+ *
+ * If you need to re-intialize the internal notes tree structure (e.g. loading
+ * from a different notes ref), please first de-initialize the current notes
+ * tree by calling free_notes().
+ */
+void init_notes(const char *notes_ref, int flags);
+
 /* Free (and de-initialize) the internal notes tree structure */
 void free_notes(void);
 
-- 
1.6.5.3.433.g11067

^ permalink raw reply related

* [RFC/PATCHv9 08/11] Notes API: get_note(): Return the note annotating the given object
From: Johan Herland @ 2009-12-02  2:09 UTC (permalink / raw)
  To: git; +Cc: gitster, johan, spearce
In-Reply-To: <1259719783-4674-1-git-send-email-johan@herland.net>

Created by a simple cleanup and rename of lookup_notes().

Signed-off-by: Johan Herland <johan@herland.net>
---
 notes.c |   15 ++++++++-------
 notes.h |    3 +++
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/notes.c b/notes.c
index 3c8a6e0..6a36ff9 100644
--- a/notes.c
+++ b/notes.c
@@ -377,12 +377,13 @@ void add_note(const unsigned char *object_sha1, const unsigned char *note_sha1)
 	note_tree_insert(&root_node, 0, l, PTR_TYPE_NOTE);
 }
 
-static unsigned char *lookup_notes(const unsigned char *object_sha1)
+const unsigned char *get_note(const unsigned char *object_sha1)
 {
-	struct leaf_node *found = note_tree_find(&root_node, 0, object_sha1);
-	if (found)
-		return found->val_sha1;
-	return NULL;
+	struct leaf_node *found;
+
+	assert(initialized);
+	found = note_tree_find(&root_node, 0, object_sha1);
+	return found ? found->val_sha1 : NULL;
 }
 
 void free_notes(void)
@@ -396,7 +397,7 @@ void format_note(const unsigned char *object_sha1, struct strbuf *sb,
 		const char *output_encoding, int flags)
 {
 	static const char utf8[] = "utf-8";
-	unsigned char *sha1;
+	const unsigned char *sha1;
 	char *msg, *msg_p;
 	unsigned long linelen, msglen;
 	enum object_type type;
@@ -404,7 +405,7 @@ void format_note(const unsigned char *object_sha1, struct strbuf *sb,
 	if (!initialized)
 		init_notes(NULL, 0);
 
-	sha1 = lookup_notes(object_sha1);
+	sha1 = get_note(object_sha1);
 	if (!sha1)
 		return;
 
diff --git a/notes.h b/notes.h
index 5f22852..21a8930 100644
--- a/notes.h
+++ b/notes.h
@@ -25,6 +25,9 @@ void init_notes(const char *notes_ref, int flags);
 void add_note(const unsigned char *object_sha1,
 		const unsigned char *note_sha1);
 
+/* Get the note object SHA1 containing the note data for the given object */
+const unsigned char *get_note(const unsigned char *object_sha1);
+
 /* Free (and de-initialize) the internal notes tree structure */
 void free_notes(void);
 
-- 
1.6.5.3.433.g11067

^ 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