Git development
 help / color / mirror / Atom feed
* Re: [PATCH 5/8] Add a config option for remotes to specify a foreign vcs
From: Jakub Narebski @ 2009-08-12  9:33 UTC (permalink / raw)
  To: Jeff King
  Cc: Johannes Schindelin, Bert Wesarg, Junio C Hamano, Daniel Barkalow,
	git, Brian Gernhardt
In-Reply-To: <20090812074521.GD15152@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

>   1. Is there some other syntax that _doesn't_ have this breakage
>      but that similarly helps the "vast majority of Git users".

Well, proposed possible syntax was:

1. <vcs>:<repository location>

   e.g.:

     svn:http://svn.example.com/project

   but

     host:path/to/repo

2. <vcs>::<repository location>

   e.g.

     svn::http://svn.example.com/project

3. <vcs>+<repository location>

   e.g.

     svn+http://svn.example.com/project

   but

     http+svn://svn.example.com/project
     svn+path/to/repo

-- 
Jakub Narebski
Poland
ShadeHawk on #git

^ permalink raw reply

* Re: Problems with filters and git status - reproduction steps
From: Peter Krefting @ 2009-08-12  9:25 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: Johannes Sixt, Git List
In-Reply-To: <4A828368.5010206@drmicha.warpmail.net>

Michael J Gruber:

> I get "Changed but not updated" already here!

With git 1.5.6.5, I do get that. With git 1.6.4, I seem to only get it after 
the last step.

I have 1.5.6.5 on the server with the master repo, and am running 1.6.4 on 
the client (although when I ran the recipe through a shell script, I got the 
unclean status earlier, which made me think it ran an earlier version I have 
installed in another directory).

> Do you really want the date in the checked-in version of the file?

Yes. This way, the checked out copy that makes my web server can do its job 
without requiring any of the filters being installed. And the files in 
history are marked as necessary.

> I would assume otherwise. Then your clean filter should really be the 
> smudge filter, and you would need a clean filter to go with it (remove the 
> date and restore the keyword).

The problem with that approach is that the "smudge" filter does not have 
access to the file name, and so can not look up the last change date of the 
file it is re-writing. And I want the last time the file was *changed*, not 
the time it was checked out.

Also, I imported my entire CVS history with keywords expanded to allow for 
this (and "checked out" the Git-generated tree over my CVS check-out to not 
have Git update all the time-stamps).

-- 
\\// Peter - http://www.softwolves.pp.se/

^ permalink raw reply

* Re: [RFCv3 2/4] Add Python support library for CVS remote helper
From: Johan Herland @ 2009-08-12  9:08 UTC (permalink / raw)
  To: David Aguilar; +Cc: git, barkalow, gitster, Johannes.Schindelin
In-Reply-To: <20090812021017.GB62301@gmail.com>

First, thank you very much for the review. It is very helpful, and I really 
appreciate it.

On Wednesday 12 August 2009, David Aguilar wrote:
> On Wed, Aug 12, 2009 at 02:13:49AM +0200, Johan Herland wrote:
> > This patch introduces a Python package called "git_remote_cvs"
> > containing the building blocks of the CVS remote helper. The CVS remote
> > helper itself is NOT part of this patch.
>
> Interesting...
>
> > diff --git a/git_remote_cvs/changeset.py b/git_remote_cvs/changeset.py
> > new file mode 100644
> > index 0000000..27c4129
> > --- /dev/null
> > +++ b/git_remote_cvs/changeset.py
> > @@ -0,0 +1,114 @@
> > +#!/usr/bin/env python
> > +
> > +"""Functionality for collecting individual CVS revisions into
> > "changesets" +
> > +A changeset is a collection of CvsRev objects that belong together in
> > the same +"commit". This is a somewhat artificial construct on top of
> > CVS, which only +stores changes at the per-file level. Normally, CVS
> > users create several CVS +revisions simultaneously by applying the "cvs
> > commit" command to several files +with related changes. This module
> > tries to reconstruct this notion of related +revisions.
> > +"""
> > +
> > +from util import *
>
> Importing * is frowned upon in Python.
>
> It's much easier to see where things are coming from if you
> 'import util' and use the namespaced util.foo() way of accessing
> the functions.

I'd rather do "from util import X Y Z", as the util stuff is used all over 
the place.

> Furthermore, you're going to want to use absolute imports.
> Anyone can create 'util.py' and blindly importing 'util' is
> asking for trouble.
>
> Instead use:
> from git_remote_cvs import util

I thought the python import rules specified that the current package was 
consulted first, and therefore the 'util' package would always come from the 
current package. However, I must confess that I don't know these rules very 
well, so I'll take your word for it and use absolute imports instead.

> > +class Changeset (object):
> > +	"""Encapsulate a single changeset/commit"""
>
> I think it reads better as Changeset(object)
> (drop the spaces before the parens).
>
> That applies to the rest of this patch as well.

Ok. Will change.

> This also had me wondering about the following:
> 	git uses tabs for indentation
>
> BUT, the python convention is to use 4-space indents ala PEP-8
> http://www.python.org/dev/peps/pep-0008/

Interesting. I have (obviously) never looked at PEP 8... :)

> It might be appealing to when-in-Rome (Rome being Python) here
> and do things the python way when we code in Python.
>
> Consistency with pep8 is good if we expect to get python hackers
> to contribute to git_remote_cvs.

I see your point, but I believe that since git_remote_cvs is not an 
independent project (but very much coupled to git), its allegiance is with 
Git, and it should therefore follow the Git coding style. In other words, I 
claim exception (2) in PEP 8

> > +
> > +	__slots__ = ('revs', 'date', 'author', 'message')
>
> __slots__ is pretty esoteric in Python-land.
>
> But, if your justification is to minimize memory usage, then
> yes, this is a good thing to do.

Yes, I only use __slots__ for classes that potentially have a large number 
of instances.

> > +	def __init__ (self, date, author, message):
> > +		self.revs    = {}      # dict: path -> CvsRev object
> > +		self.date    = date    # CvsDate object
> > +		self.author  = author
> > +		self.message = message # Lines of commit message
>
> pep8 and other parts of the git codebase recommend against
> lining up the equals signs like that.  Ya, sorry for the nits
> being that they're purely stylistic.

I can't find a good rationale for this rule in PEP8 (other than Guido's 
personal style), and I personally find the above much more readable 
(otherwise I wouldn't go through the trouble of lining them all up...). Can 
I claim exception (1) (readability)?

> > +		if len(msg) > 25: msg = msg[:22] + "..." # Max 25 chars long
> > +		return "<Changeset @(%s) by %s (%s) updating %i files>" % (
> > +			self.date, self.author, msg, len(self.revs))
>
> Similar to the git coding style, this might be better written:
>
> ...
> if len(msg) > 25:
>     msg = msg[:22] + '...' # Max 25 chars long
> ...
>
> (aka avoid single-line ifs)
>
> There's a few other instances of this in the patch as well.

Ok. Will try to eliminate single-line ifs.

> > diff --git a/git_remote_cvs/cvs.py b/git_remote_cvs/cvs.py
> > new file mode 100644
> > index 0000000..cc2e13f
> > --- /dev/null
> > +++ b/git_remote_cvs/cvs.py
> > @@ -0,0 +1,884 @@
> > [...]
> > +
> > +	def enumerate (self):
> > +		"""Return a list of integer components in this CVS number"""
> > +		return list(self.l)
>
> enumerate has special meaning in Python.
>
> items = (1, 2, 3, 4)
> for idx, item in enumerate(items):
>     print idx, item
>
>
> I'm not sure if this would cause confusion...

Good point, I should probably rename this method.

> > [...]
> > +		else: # revision number
> > +			assert self.l[-1] > 0
>
> asserts go away when running with PYTHONOPTIMIZE.
>
> If this is really an error then we should we raise an exception
> instead?

I use asserts to verify pre/post-conditions and other invariants. I believe 
that if this assert fails, it is indicative of something horribly wrong with 
the code itself. However, I now see that one can also trigger this case with 
bad input (e.g. CvsNum("1.2.3.0").parent()). I will keep the assert here, 
but will also add some input verification to the CvsNum class.

> > +	@classmethod
> > +	def test (cls):
> > +		assert cls("1.2.4") == cls("1.2.0.4")
>
> Hmm.. Does it make more sense to use the unittest module?
>
> e.g. self.assertEqual(foo, bar)

Probably. I'm not familiar with 'unittest', but will take a look.

> > diff --git a/git_remote_cvs/cvs_revision_map.py
> > b/git_remote_cvs/cvs_revision_map.py new file mode 100644
> > index 0000000..7d7810f
> > --- /dev/null
> > +++ b/git_remote_cvs/cvs_revision_map.py
> > @@ -0,0 +1,362 @@
> > +#!/usr/bin/env python
> > +
> > +"""Functionality for mapping CVS revisions to associated
> > metainformation""" +
> > +from util import *
> > +from cvs  import CvsNum, CvsDate
> > +from git  import GitFICommit, GitFastImport, GitObjectFetcher
>
> We definitely need absolute imports here.
>
> 'import git' could find the git-python project's git module.

Ok. Will fix.

> Nonetheless, interesting stuff.

Thanks for the review!


Have fun! :)

...Johan

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

^ permalink raw reply

* Re: Problems with filters and git status - reproduction steps
From: Michael J Gruber @ 2009-08-12  8:55 UTC (permalink / raw)
  To: Peter Krefting; +Cc: Johannes Sixt, Git List
In-Reply-To: <alpine.DEB.2.00.0908120856110.30907@ds9.cixit.se>

Peter Krefting venit, vidit, dixit 12.08.2009 10:36:
> ORIGINREPO=git://git.debian.org/users/peterk/gitfilterproblem.git
> DESTINATIONREPO=gitfilterproblem-testrepo
> 
> # Set up repository
> echo -- Cloning
> git clone ${ORIGINREPO} ${DESTINATIONREPO}
> cd ${DESTINATIONREPO}
> 
> # Status should be clean
> echo -- After cloning, status should be clean
> git status
> 
> # Set up filter
> echo -- Set up filter, status should be clean
> ./reposetup.sh 
> git status

I get "Changed but not updated" already here!

Also, what's your git version? There have been some fix-ups recently
regarding the interaction between filters/textconv and assumptions of
the code about dirtiness of the worktree.

> 
> # Create tracking branch
> echo -- Create tracking branch for changed file, status should be clean
> git branch --track changed-text-files origin/changed-text-files 
> git status
> 
> # Merge the branch
> echo -- Merge the changes from the branch, now status gets unclean
> git merge changed-text-files
> echo
> git status
> git diff

Do you really want the date in the checked-in version of the file? I
would assume otherwise. Then your clean filter should really be the
smudge filter, and you would need a clean filter to go with it (remove
the date and restore the keyword).

Michael

^ permalink raw reply

* Re: How do gmail users try out patches from this list?
From: Mike Ralphson @ 2009-08-12  8:43 UTC (permalink / raw)
  To: skillzero; +Cc: Wesley J. Landaker, Nicolas Sebrecht, git, Michael J Gruber
In-Reply-To: <4A827BF3.8080208@drmicha.warpmail.net>

2009/8/11  <skillzero@gmail.com>:
> Sorry if this is dumb question, but I didn't see any good info in my searches.
>
> How do gmail users normally apply patches that come through the list?
> Do you just manually copy and paste the email to patch files and use
> git apply? Do you use a tool to export to mbox files and use git am?
>
> I've been just doing it manually via copy and paste, but it's kinda tedious.

Yep, show original, copy and paste and git apply. Personally I would
prefer to fetch changes using, oh, I don't know, some kind of dvcs
tool... 8-) It means I don't tend to build and test many patch series
until they get merged.

It's a pity there's a patchwork server for many kernel.org projects,
but not for the git mailing list 8-(

http://patchwork.kernel.org/

http://patchwork.kernel.org/help/pwclient/

http://gitster.livejournal.com/18696.html

^ permalink raw reply

* Problems with filters and git status - reproduction steps
From: Peter Krefting @ 2009-08-12  8:36 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Git List
In-Reply-To: <alpine.DEB.2.00.0908120751500.30907@ds9.cixit.se>

Peter Krefting:

> However, as I have set the "ident" attribute, Git wants to expand it itself 
> and check in files with "$Id$". When I do a reset, it seems it records the 
> entry as clean against a version stored with just "$Id$", but the record in 
> history has an expanded "$Id$", and the entry is thus never deemed clean.

Actually, that is not the case. It seems to be the filter that causes the 
problems, all by itself. I cannot seem to reproduce this *reliably*. I tried 
setting up a minimal repository and a reproduction recipe, but I get 
different behaviour when I perform the steps manually, and when I run it 
from a shell script.

Here is the reproduction recipe:

#!/bin/bash
# Reproduction recipe for $Date$ dirty issue

ORIGINREPO=git://git.debian.org/users/peterk/gitfilterproblem.git
DESTINATIONREPO=gitfilterproblem-testrepo

# Set up repository
echo -- Cloning
git clone ${ORIGINREPO} ${DESTINATIONREPO}
cd ${DESTINATIONREPO}

# Status should be clean
echo -- After cloning, status should be clean
git status

# Set up filter
echo -- Set up filter, status should be clean
./reposetup.sh 
git status

# Create tracking branch
echo -- Create tracking branch for changed file, status should be clean
git branch --track changed-text-files origin/changed-text-files 
git status

# Merge the branch
echo -- Merge the changes from the branch, now status gets unclean
git merge changed-text-files
echo
git status
git diff

-- 
\\// Peter - http://www.softwolves.pp.se/

^ permalink raw reply

* Re: How do gmail users try out patches from this list?
From: Michael J Gruber @ 2009-08-12  8:23 UTC (permalink / raw)
  To: Wesley J. Landaker; +Cc: Nicolas Sebrecht, skillzero, git
In-Reply-To: <200908111917.19267.wjl@icecavern.net>

Wesley J. Landaker venit, vidit, dixit 12.08.2009 03:17:
> On Tuesday 11 August 2009 16:14:08 Nicolas Sebrecht wrote:
>> The 11/08/09, skillzero@gmail.com wrote:
>>> Sorry if this is dumb question, but I didn't see any good info in my
>>> searches.
>>>
>>> How do gmail users normally apply patches that come through the list?
>>
>> It doesn't rely on your address mail provider but on your local email
>> workflow/MUA.
> 
> I'm not in this situation, but my guess is that a lot of people use gmail 
> primarily through the web interface (e.g. because of corporate firewalls or 
> some other reason). Maybe someone in that situation should make an new "git 
> imap-am" command? Kind of the reverse to imap-send. Just a thought. =)

Well, if they can't do imap (because of a firewall) git can't do imap...

I guess for them (webmail users) it would be better if we attached
patches, but we don't do that here. In any case, our list is mirrored on
gmane, and you can use the interface there. For example, you get the
first message in this thread using the gmane id or the message id like this:

http://article.gmane.org/gmane.comp.version-control.git/125591
http://mid.gmane.org/2729632a0908111343v73fa475fqb6353dcf2f718101@mail.gmail.com

If you add /raw to those URLs you get the original message so that you
can happily wget/curl/browse and save away.

Michael

^ permalink raw reply

* Re: fatal: bad revision 'HEAD'
From: Jeff King @ 2009-08-12  7:58 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Joel Mahoney, Johannes Schindelin, git
In-Reply-To: <7v7hx98otz.fsf@alter.siamese.dyndns.org>

On Wed, Aug 12, 2009 at 12:37:44AM -0700, Junio C Hamano wrote:

> But just like we twisted the definition of merge to mean "merging
> something into nothing yields that something", we could twist the
> definition of rebase to mean "rebasing nothing on top of something result
> in that something".  It sort of makes sense in a twisted way.

I dunno. It doesn't seem all that twisted to me.

But like many of the "branch to be born" and "initial commit" edge cases
we have dealt with, it is not so much about somebody intentionally
triggering this as it is about doing something sane when some script
_does_ trigger it. And I think the sane thing is obvious and easy to do
here, so why not?

>  * Is "rev-parse -q --verify" a safe test to guarantee that HEAD is
>    unborn?  Shouldn't we be checking with "symbolic-ref" or something?

I'm not sure. The test in git-checkout, for example, seems to basically
just be looking up HEAD as a commit. If it doesn't work, then the branch
is to-be-born (see switch_branches in builtin-checkout.c).

Which is more or less what's happening here (except we don't check that
the type is a commit).

With symbolic-ref, I guess we could find out what the ref is, and check
to see if _that_ exists. But I can't think of a situation where that
would be meaningfully different than just resolving HEAD. Obviously
detached HEADs come to mind, but wouldn't you then by definition not be
a branch-to-be-born, which is what this rev-parse test would tell you?

>  * In such an "unborn branch" case, by definition, a non-empty index won't
>    be based on whatever we are pulling down from the remote.  So how about
>    doing something like the following instead?
> 
> 	if on unborn branch
> 	then
> 		if test -f "$GIT_DIR/index"
>                 then
> 			die "refusing to update; you have a non-empty index"
> 		fi
> 	else
> 		... existing tests against HEAD ...
> 	fi

Yeah, I think that is a better idea. Do you want to tweak the patch, or
should I re-submit?

-Peff

^ permalink raw reply

* Re: [RFC/PATCH 5/6] Let transport_helper_init() decide if a remote helper program can be used
From: Jeff King @ 2009-08-12  7:46 UTC (permalink / raw)
  To: Daniel Barkalow; +Cc: Johan Herland, git, gitster, benji, Johannes.Schindelin
In-Reply-To: <alpine.LNX.2.00.0908111915100.27553@iabervon.org>

On Tue, Aug 11, 2009 at 07:28:19PM -0400, Daniel Barkalow wrote:

> of 'Could not find (...) "git remote-master.kernel.org" (...)'? That 
> would be certain to upset some people. I think we can assume that people's 
> scheme parts of their URLs that are actually URLs don't contain '.', and 
> that people with:
> 
> 	url = master:something
> 
> will append their domains if the warning gets annoying.

Keep in mind that these URLs should be usable from the command-line,
too. So it is not just appending the domain in the config, but appending
it every time you want to do a one-off pull.

-Peff

^ permalink raw reply

* Re: [PATCH 5/8] Add a config option for remotes to specify a foreign vcs
From: Jeff King @ 2009-08-12  7:45 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Bert Wesarg, Junio C Hamano, Daniel Barkalow, git,
	Brian Gernhardt
In-Reply-To: <alpine.DEB.1.00.0908120128120.8306@pacific.mpi-cbg.de>

On Wed, Aug 12, 2009 at 01:53:38AM +0200, Johannes Schindelin wrote:

> > It is not actually that unreasonable. I have remotes which point to:
> > 
> >   vcs:git/foo.git
> 
> That is still not "svn".

No, but you snipped the part where I explain how that leads me to
believe "svn" is plausible. Remember that you and I are just a
representative sample of a much larger userbase.

There is also a related question: should the the meaning of the URL rely
purely on _syntax_, or must we understand the _semantics_ of the
individual tokens? That is, given $X:$Y, does that syntactically mean
that $X _must_ be a remote helper, or must I understand what helpers git
knows about to know what it is?

I tend to think purely syntactic systems are more robust and easier to
understand. The downside is that it's less DWIM, which can often mean
more typing.

> If _I_ were to judge whether to make it convenient for computer-savvy 
> people like you who would have _no_ problem diagnosing the problem (_if_ 
> they have the problem, having edited .ssh/config themselves!), who would 
> curse briefly, and then go on fixing the problem, or in the alternative 
> make it convenient for people who do not know their way around .ssh/config 
> as well as you (and who happen to make up the _vast_ majority of Git users 
> by now [*1*]), and who would really prefer to have an easy way to clone 
> "foreign" repositories, I have _no_ problem deciding which way to go.
> 
> So I'm a bastard.  Big news.  But I'm a pragmatic one.

You didn't quote the part of my email about how ssh:// sucks. It is not
just about having my config break, figuring it out, and fixing it. You
are losing a useful construct that I might be using on the command line.

That being said, I am not 100% opposed to the proposal. I just think it
is worth considering this breakage as a downside, and considering

  1. Is there some other syntax that _doesn't_ have this breakage
     but that similarly helps the "vast majority of Git users".

  2. Should such a breakage follow a deprecation schedule, and if so,
     what schedule?

-Peff

^ permalink raw reply

* Re: [EGIT PATCH] Provide a more JavaBeans-style 'getName' accessor for the id Signed-off-by: Alex Blewitt <alex.blewitt@gmail.com>
From: Alex Blewitt @ 2009-08-11 12:53 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: robin.rosenberg@dewire.com, git@vger.kernel.org
In-Reply-To: <20090810205907.GY1033@spearce.org>

On 10 Aug 2009, at 21:59, "Shawn O." <spearce@spearce.org> wrote:

> Alex Blewitt <alex.blewitt@gmail.com> wrote:
>> That patch was originally mailed on the 11th May. Has it taken  
>> until now
>> to notice the problem, or was the other method added in the last  
>> month or
>> so? If I'm to blame, I apologise but didn't note any compile time  
>> issues
>> at the time.
>
> Arrgh, you are right, I lost this patch in my inbox, and in the
> interm we applied new features to RevTag which added getName there. .
>
>>> ./org/spearce/jgit/revwalk/RevTag.java:206: getName() in
>>> org.spearce.jgit.revwalk.RevTag cannot override getName() in
>>> org.spearce.jgit.lib.AnyObjectId; overridden method is final
>
> I can't apply this patch because getName() on RevTag is already
> defined with a different meaning.  :-(

That sounds dangerous. We now have a .name() and a .getName() with  
different semantics. Can we not change the RevTag method name to  
something else so that we dont have an inconsistency?
>

Alex 

^ permalink raw reply

* Re: fatal: bad revision 'HEAD'
From: Junio C Hamano @ 2009-08-12  7:37 UTC (permalink / raw)
  To: Jeff King; +Cc: Joel Mahoney, Johannes Schindelin, git
In-Reply-To: <20090812032740.GA26089@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> I was able to replicate your problem, but only if I set
> branch.master.rebase to "true" in my user-wide git config (i.e.,
> ~/.gitconfig). It looks like "git pull" is not capable of handling a
> rebase when you have no commits yet.

It does sound sick to store such a setting in $HOME/.gitconfig file, when
the variable is clearly per-repository.  As you wrote, you can easily
trigger it with an explicit --rebase, but again, it is insane to ask
"rebase" when you clearly do not have anything.

But just like we twisted the definition of merge to mean "merging
something into nothing yields that something", we could twist the
definition of rebase to mean "rebasing nothing on top of something result
in that something".  It sort of makes sense in a twisted way.

> diff --git a/git-pull.sh b/git-pull.sh
> index 0f24182..427b5c6 100755
> --- a/git-pull.sh
> +++ b/git-pull.sh
> @@ -119,9 +119,15 @@ error_on_no_merge_candidates () {
>  }
>  
>  test true = "$rebase" && {
> +	if git rev-parse -q --verify HEAD >/dev/null; then
> +		parent_tree=HEAD
> +	else # empty tree
> +		parent_tree=4b825dc642cb6eb9a060e54bf8d69288fbee4904
> +	fi
> +
>  	git update-index --ignore-submodules --refresh &&
>  	git diff-files --ignore-submodules --quiet &&
> -	git diff-index --ignore-submodules --cached --quiet HEAD -- ||
> +	git diff-index --ignore-submodules --cached --quiet $parent_tree -- ||
>  	die "refusing to pull with rebase: your working tree is not up-to-date"

Two comments.

 * Is "rev-parse -q --verify" a safe test to guarantee that HEAD is
   unborn?  Shouldn't we be checking with "symbolic-ref" or something?

 * In such an "unborn branch" case, by definition, a non-empty index won't
   be based on whatever we are pulling down from the remote.  So how about
   doing something like the following instead?

	if on unborn branch
	then
		if test -f "$GIT_DIR/index"
                then
			die "refusing to update; you have a non-empty index"
		fi
	else
		... existing tests against HEAD ...
	fi

^ permalink raw reply

* Re: [RFC PATCH v3 8/8] --sparse for porcelains
From: Johannes Sixt @ 2009-08-12  7:31 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy
  Cc: git, Johannes Schindelin, Junio C Hamano
In-Reply-To: <1250005446-12047-9-git-send-email-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy schrieb:
> This series is useless until now because no one would use read-tree to
> checkout. At least with this, you can really use/test the series.
> Porcelain design was originally "if you have .git/info/sparse,
> porcelains will use it, if you don't like that, remove
> .git/info/sparse" while plumblings have an option to
> enable/disable this feature.
> 
> And I still like that behavior. How about we enable sparse checkout
> by default for porcelains and make a config option to disable it?

I would enable sparse checkout by default even for plumbing. Whether the
checkout area is sparse should always be governed by .git/info/sparse.
This way, existing scripts and aliases should automatically work in sparse
worktrees.

BTW, the name .git/info/sparse is perhaps a bit too technical in the sense
that only git developers know that this feature runs under the name
"sparse checkout". Perhaps it should be named

   .git/info/indexonly
   .git/info/nocheckout

or so.

-- Hannes

^ permalink raw reply

* Re: [msysGit] Re: Using VC build git
From: Marius Storm-Olsen @ 2009-08-12  7:13 UTC (permalink / raw)
  To: Frank Li; +Cc: Johannes Schindelin, git, msysGit
In-Reply-To: <1976ea660908101826q26faa37ao7920d5cf9d4f53fd@mail.gmail.com>

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

[Please do *not* do top-posting! Both git and msysgit mailing lists 
use bottom-posting]

Frank Li said the following on 11.08.2009 03:26:
> Thank you take care my patch.
> I can fix all problems.

Good! Many people want to see git build with MSVC, if only to use a 
compiler better at optimizing code on Windows. (And the debugger, of 
course)


> This patch is base on v1.6.4 release.  My working branch is vc_build
> at git://repo.or.cz/tgit.git.

Ok. Dscho wondered why it wasn't a proper fork of the main git.git 
repo, so it *should* really have been
     git://repo.or.cz/git/tgit.git
                      ^^^^ <-- Notice the fork relationship?

It would save some valuable space on the server, show relations, make 
it easier to find, etc.


> That is actually prototype to approve VC can build git.
> The code style is not big problem. I will fix it.

You will experience that for the git community coding style is very 
important (for good reason), so expect many rounds of rewriting your 
patches, until they all shine like diamonds.

> VC build will reuse many msysgit works because msysgit really do many
> work at windows porting.

Sure, but I think it rarely will involve your patches changing the 
code in MinGW at all. Try your outmost to keep your patches separated 
from anything else. If in doubt, please ask us on the msysgit mailing 
list, and we will guide you.


> I think the below is most important problem.
> 
> 1.  Where are vcbuild directory put, is it okay under contrib ?
> 2.  How to handle external library, such as zlib? Can use submodule?

For point 2: If we cannot compile we don't accept it, is the basic 
rule. This means that if you think the people building git with MSVC 
cannot install some dependencies themselves, you need to provide the 
full sources yourself, and make sure that the code compiles without 
changes. So, in this case if would mean to include the sources for 
zlib, and setup the vcproj to compile this code into the executable, 
instead of linking with a precompiled lib.

In this case, however, I think you should rely on the developer 
providing this library themselves, and not add it to the git project. 
The zlib project is ~750KB of code itself..

--
.marius


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 187 bytes --]

^ permalink raw reply

* Meta data available to filters?
From: Peter Krefting @ 2009-08-12  6:59 UTC (permalink / raw)
  To: Git List

Hi!

If I set up a filter, is there a way from my filter script to know the name 
or identity of the file that I am processing?

I get the data on standard in and output the changed data on standard out, 
but is there any way to get access to meta data?


I find the documentation on filters a bit terse. Once I figure out how they 
actually work, I may be inclined to send a documentation patch. A minimum 
would be to document their existence on the git-config manual page (with 
reference to gitattributes), it took me some time to find it :-)

-- 
\\// Peter - http://www.softwolves.pp.se/

^ permalink raw reply

* Re: Implementing $Date$ substitution - problem with git status
From: Peter Krefting @ 2009-08-12  6:54 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Git List
In-Reply-To: <200908092252.58363.j6t@kdbg.org>

Johannes Sixt:

>> Doing "git reset --hard" or "git checkout master filename" does not 
>> help, the file is still believed to be modified by git.

> Now, that's an entirely different problem, and I think that there is a 
> bug. I have observed this as well with my own clean filter sometimes, but 
> not always. I haven't found a recipe that reliably exhibits the problem.

After som examination, it seems to be caused by the way I imported the CVS 
history: I kept all the $ keywords expanded in history (so that if I check 
out an old version from Git, it looks like it did in CVS). This means that 
still in the latest revision of several files, I have "$Id$" lines checked 
in in CVS format.

However, as I have set the "ident" attribute, Git wants to expand it itself 
and check in files with "$Id$". When I do a reset, it seems it records the 
entry as clean against a version stored with just "$Id$", but the record in 
history has an expanded "$Id$", and the entry is thus never deemed clean.

I can probably work around this by removing the "$Id$" attributes, or by 
removing the "ident" rule.

-- 
\\// Peter - http://www.softwolves.pp.se/

^ permalink raw reply

* Re: [RFC PATCH v3 8/8] --sparse for porcelains
From: Junio C Hamano @ 2009-08-12  6:33 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git, Johannes Schindelin
In-Reply-To: <1250005446-12047-9-git-send-email-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> @@ -594,6 +596,8 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
>  		OPT_BOOLEAN('m', "merge", &opts.merge, "merge"),
>  		OPT_STRING(0, "conflict", &conflict_style, "style",
>  			   "conflict style (merge or diff3)"),
> +		OPT_SET_INT(0, "sparse", &opts.apply_sparse,
> +			    "apply sparse checkout filter", 1),

Shouldn't this be BOOLEAN not INT, i.e. "--[no-]sparse"?  That way, you
could enable it by simply the presense of $GIT_DIR/info/sparse.

It could also require core.sparseworktree configuration set to true if we
are really paranoid, but without the actual sparse specification file
flipping that configuration to true would not be useful anyway, so in
practice, giving --sparse-work-tree option to these Porcelain commands
would be no-op, but --no-sparse-work-tree option would be useful to
ignore $GIT_DIR/info/sparse and populate the work tree fully.

Or am I missing something?

^ permalink raw reply

* Re: unpack a single object
From: tarmigan @ 2009-08-12  5:34 UTC (permalink / raw)
  To: Bryan Donlan; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <3e8340490908111349j323812dbmf2211e4cf454b8f1@mail.gmail.com>

On Tue, Aug 11, 2009 at 1:49 PM, Bryan Donlan<bdonlan@gmail.com> wrote:
> On Tue, Aug 11, 2009 at 4:48 PM, Bryan Donlan<bdonlan@gmail.com> wrote:
>> On Tue, Aug 11, 2009 at 4:15 PM, tarmigan<tarmigan+lists@gmail.com> wrote:
>>> I would rather not copy the whole good repo back to the one that ran
>>> out of space because it's multiple gigs.  My plan is to just explode
>>> the bad pack on of the corrupted repo, explode good pack and then,
>>> copy the corrupted object back.  So my question is how do I tell which
>>> pack contains that object?  (I would rather not explode all the packs
>>> because of the repo size.)  Is there a way to extract a single object
>>> from a pack without exploding the whole pack?
>>
>> You should be able to just extract the single object in question:
>>
>> goodrepo$ git cat-file commit 3d4c2b0225e7605a7e2a38ffc44dcb888589f4ce
>>  > ~/commit.dat
>> goodrepo$ cd ~/badrepo
>> badrepo$ git read-file -t commit ~/commit.dat
>> (should output 3d4c2b0225e7605a7e2a38ffc44dcb888589f4ce)
>>
>> At this point your repo should be repaired.
>>
>
> Err, that should read git hash-object, not git read-file.
>

Thanks, and thanks to Junio who also responded with something similar
off-list.  I was vaguely familiar with cat-file and hash-object before
but didn't realize how they could be used to do what I wanted, so
thanks for the lesson.

As Junio suspected, I after the next fsck, I also had problems with
the tree associated with that commit and also the commits behind that
one.  Each fsck takes around 12 minutes, so replacing one commit at a
time seemed not very productive.  So now I am doing another svn clone
from scratch and letting it run for a few hours (it was a straight
mirror, so nothing is lost).  It seems somewhat wasteful, but I guess
better a few hours of cpu time than a few hours of my time.

Thanks,
Tarmigan

^ permalink raw reply

* [PATCH 11/13] sequencer: add "do_commit()" and related functions
From: Christian Couder @ 2009-08-12  5:15 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Johannes Schindelin, Stephan Beyer, Daniel Barkalow,
	Jakub Narebski
In-Reply-To: <20090812051116.18155.70541.chriscool@tuxfamily.org>

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

This patch adds some code that comes from the sequencer GSoC project:

git://repo.or.cz/git/sbeyer.git

(commit e7b8dab0c2a73ade92017a52bb1405ea1534ef20)

It adds "struct commit_info", the "next_commit" static variable and
the following functions:

        - do_commit()
        - set_author_info()
        - set_message_source()
        - set_pick_subject()
        - write_commit_summary_into()

This makes it possible to prepare and perform a commit.

Compared to the sequencer project, the only change is that "mark"
related (3 lines long) code has been removed from do_commit().

    Mentored-by: Daniel Barkalow <barkalow@iabervon.org>
    Mentored-by: Christian Couder <chriscool@tuxfamily.org>
    Signed-off-by: Stephan Beyer <s-beyer@gmx.net>
    Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin-sequencer--helper.c |  214 +++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 214 insertions(+), 0 deletions(-)

diff --git a/builtin-sequencer--helper.c b/builtin-sequencer--helper.c
index 71a7fef..61a8f2e 100644
--- a/builtin-sequencer--helper.c
+++ b/builtin-sequencer--helper.c
@@ -5,26 +5,69 @@
 #include "refs.h"
 #include "diff.h"
 #include "unpack-trees.h"
+#include "string-list.h"
+#include "pick.h"
+#include "rerere.h"
+#include "dir.h"
+#include "cache-tree.h"
+#include "utf8.h"
 
 #define SEQ_DIR "rebase-merge"
 
 #define PATCH_FILE	git_path(SEQ_DIR "/patch")
+#define MERGE_MSG	git_path("MERGE_MSG")
+#define SQUASH_MSG	git_path("SQUASH_MSG")
+
+/**********************************************************************
+ * Data structures
+ */
+
+struct user_info {
+	const char *name;
+	const char *mail;
+	const char *time; /* "<timestamp> <timezone>" */
+};
+
+struct commit_info {
+	struct user_info author; /* author info */
+	struct user_info committer; /* not used, but for easy extendability */
+	const char *encoding; /* encoding */
+	char *subject; /* basically the first line of the summary */
+	struct strbuf summary; /* the commit message */
+	char *source; /* source of the commit message, either
+		       * "message", "merge", "squash" or a commit SHA1 */
+	char *patch; /* a patch */
+	struct string_list parents; /* list of parents' hex'ed sha1 ids */
+};
+
+/**********************************************************************
+ * Global variables
+ */
 
 static char *reflog;
 
+static int squash_count = 0;
+
 static int allow_dirty = 0, verbosity = 1, advice = 1;
 
 static unsigned char head_sha1[20];
 
+static struct commit_info next_commit;
+
 static const char * const git_sequencer_helper_usage[] = {
 	"git sequencer--helper --make-patch <commit>",
 	"git sequencer--helper --reset-hard <commit> <reflog-msg> "
 		"<verbosity> [<allow-dirty>]",
 	"git sequencer--helper --fast-forward <commit> <reflog-msg> "
 		"<verbosity> [<allow-dirty>]",
+	"git sequencer--helper --cherry-pick <commit> [<do-not-commit>]",
 	NULL
 };
 
+/**********************************************************************
+ * Sequencer functions
+ */
+
 static int parse_and_init_tree_desc(const unsigned char *sha1,
 				    struct tree_desc *desc)
 {
@@ -162,6 +205,157 @@ static void make_patch(struct commit *commit)
 	free(args);
 }
 
+/* Commit current index with information next_commit onto parent_sha1. */
+static int do_commit(unsigned char *parent_sha1)
+{
+	int failed;
+	unsigned char tree_sha1[20];
+	unsigned char commit_sha1[20];
+	struct strbuf sbuf;
+	const char *reencoded = NULL;
+
+	if (squash_count) {
+		squash_count = 0;
+		if (file_exists(SQUASH_MSG))
+			unlink(SQUASH_MSG);
+	}
+
+	if (!index_differs_from("HEAD", 0) &&
+	    !next_commit.parents.nr)
+		return error("No changes! Do you really want an empty commit?");
+
+	if (!next_commit.author.name || !next_commit.author.mail)
+		return error("Internal error: Author information not set properly.");
+
+	if (write_cache_as_tree(tree_sha1, 0, NULL))
+		return 1;
+
+	if (!next_commit.encoding)
+		next_commit.encoding = xstrdup("utf-8");
+	if (!git_commit_encoding)
+		git_commit_encoding = "utf-8";
+
+	strbuf_init(&sbuf, 8192); /* should avoid reallocs for the headers */
+	strbuf_addf(&sbuf, "tree %s\n", sha1_to_hex(tree_sha1));
+	if (parent_sha1)
+		strbuf_addf(&sbuf, "parent %s\n", sha1_to_hex(parent_sha1));
+	if (next_commit.parents.nr) {
+		int i;
+		for (i = 0; i < next_commit.parents.nr; ++i)
+			strbuf_addf(&sbuf, "parent %s\n",
+					next_commit.parents.items[i].string);
+	}
+	if (!next_commit.author.time) {
+		char time[50];
+		datestamp(time, sizeof(time));
+		next_commit.author.time = xstrdup(time);
+	}
+
+	stripspace(&next_commit.summary, 1);
+
+	/* if encodings differ, reencode whole buffer */
+	if (strcasecmp(git_commit_encoding, next_commit.encoding)) {
+		if ((reencoded = reencode_string(next_commit.author.name,
+				git_commit_encoding, next_commit.encoding))) {
+			free((void *)next_commit.author.name);
+			next_commit.author.name = reencoded;
+		}
+		if ((reencoded = reencode_string(next_commit.summary.buf,
+				git_commit_encoding, next_commit.encoding))) {
+			strbuf_reset(&next_commit.summary);
+			strbuf_addstr(&next_commit.summary, reencoded);
+		}
+	}
+	strbuf_addf(&sbuf, "author %s <%s> %s\n", next_commit.author.name,
+			next_commit.author.mail, next_commit.author.time);
+	strbuf_addf(&sbuf, "committer %s\n", git_committer_info(0));
+	if (!is_encoding_utf8(git_commit_encoding))
+		strbuf_addf(&sbuf, "encoding %s\n", git_commit_encoding);
+	strbuf_addch(&sbuf, '\n');
+	strbuf_addbuf(&sbuf, &next_commit.summary);
+	if (sbuf.buf[sbuf.len-1] != '\n')
+		strbuf_addch(&sbuf, '\n');
+
+	failed = write_sha1_file(sbuf.buf, sbuf.len, commit_type, commit_sha1);
+	strbuf_release(&sbuf);
+	if (failed)
+		return 1;
+
+	if (verbosity > 1)
+		printf("Created %scommit %s\n",
+			parent_sha1 || next_commit.parents.nr ? "" : "initial ",
+			sha1_to_hex(commit_sha1));
+
+	if (update_ref(reflog, "HEAD", commit_sha1, NULL, 0, 0))
+		return error("Could not update HEAD to %s.",
+						sha1_to_hex(commit_sha1));
+
+	return 0;
+}
+
+/*
+ * Fill next_commit.author according to ident.
+ * Ident may have one of the following forms:
+ * 	"name <e-mail> timestamp timezone\n..."
+ * 	"name <e-mail> timestamp timezone"
+ * 	"name <e-mail>"
+ */
+static void set_author_info(const char *ident)
+{
+	const char *tmp1 = strstr(ident, " <");
+	const char *tmp2;
+	char *data;
+	if (!tmp1)
+		return;
+	tmp2 = strstr(tmp1+2, ">");
+	if (!tmp2)
+		return;
+	if (tmp2[1] != 0 && tmp2[1] != ' ')
+		return;
+
+	data = xmalloc(strlen(ident)); /* a trivial upper bound */
+
+	snprintf(data, tmp1-ident+1, "%s", ident);
+	next_commit.author.name = xstrdup(data);
+	snprintf(data, tmp2-tmp1-1, "%s", tmp1+2);
+	next_commit.author.mail = xstrdup(data);
+
+	if (tmp2[1] == 0) {
+		free(data);
+		return;
+	}
+
+	tmp1 = strpbrk(tmp2+2, "\r\n");
+	if (!tmp1)
+		tmp1 = tmp2 + strlen(tmp2);
+
+	snprintf(data, tmp1-tmp2-1, "%s", tmp2+2);
+	next_commit.author.time = xstrdup(data);
+	free(data);
+}
+
+static void set_message_source(const char *source)
+{
+	if (next_commit.source)
+		free(next_commit.source);
+	next_commit.source = xstrdup(source);
+}
+
+/* Set subject, an information for the case of conflict */
+static void set_pick_subject(const char *hex, struct commit *commit)
+{
+	const char *tmp = strstr(commit->buffer, "\n\n");
+	if (tmp) {
+		const char *eol;
+		int len = strlen(hex);
+		tmp += 2;
+		eol = strchrnul(tmp, '\n');
+		next_commit.subject = xmalloc(eol - tmp + len + 5);
+		snprintf(next_commit.subject, eol - tmp + len + 5, "%s... %s",
+								hex, tmp);
+	}
+}
+
 /* Return a commit object of "arg" */
 static struct commit *get_commit(const char *arg)
 {
@@ -198,6 +392,26 @@ static int set_verbosity(int verbose)
 	return 0;
 }
 
+static int write_commit_summary_into(const char *filename)
+{
+	struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
+	int fd = hold_lock_file_for_update(lock, filename, 0);
+	if (fd < 0)
+		return error("Unable to create '%s.lock': %s", filename,
+							strerror(errno));
+	if (write_in_full(fd, next_commit.summary.buf,
+			      next_commit.summary.len) < 0)
+		return error("Could not write to %s: %s",
+						filename, strerror(errno));
+	if (commit_lock_file(lock) < 0)
+		return error("Error wrapping up %s", filename);
+	return 0;
+}
+
+/**********************************************************************
+ * Builtin sequencer helper functions
+ */
+
 /* v should be "" or "t" or "\d" */
 static int parse_verbosity(const char *v)
 {
-- 
1.6.4.271.ge010d

^ permalink raw reply related

* [PATCH 06/13] pick: simplify "error(...)" followed by "return -1"
From: Christian Couder @ 2009-08-12  5:15 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Johannes Schindelin, Stephan Beyer, Daniel Barkalow,
	Jakub Narebski
In-Reply-To: <20090812051116.18155.70541.chriscool@tuxfamily.org>

into "return error(...)", as suggested by Junio.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 pick.c |   62 ++++++++++++++++++++++++--------------------------------------
 1 files changed, 24 insertions(+), 38 deletions(-)

diff --git a/pick.c b/pick.c
index 77c7169..6fea39c 100644
--- a/pick.c
+++ b/pick.c
@@ -93,24 +93,18 @@ int pick(struct commit *pick_commit, int mainline, int flags,
 	 * that represents the "current" state for merge-recursive
 	 * to work on.
 	 */
-	if (write_cache_as_tree(head, 0, NULL)) {
-		error("Your index file is unmerged.");
-		return -1;
-	}
+	if (write_cache_as_tree(head, 0, NULL))
+		return error("Your index file is unmerged.");
 	discard_cache();
 
 	index_fd = hold_locked_index(&index_lock, 0);
-	if (index_fd < 0) {
-		error("Unable to create locked index: %s",
-						strerror(errno));
-		return -1;
-	}
+	if (index_fd < 0)
+		return error("Unable to create locked index: %s",
+			     strerror(errno));
 
 	if (!commit->parents) {
-		if (flags & PICK_REVERSE) {
-			error("Cannot revert a root commit");
-			return -1;
-		}
+		if (flags & PICK_REVERSE)
+			return error("Cannot revert a root commit");
 		parent = NULL;
 	}
 	else if (commit->parents->next) {
@@ -118,40 +112,32 @@ int pick(struct commit *pick_commit, int mainline, int flags,
 		int cnt;
 		struct commit_list *p;
 
-		if (!mainline) {
-			error("Commit %s is a merge but no mainline was given.",
-			    sha1_to_hex(commit->object.sha1));
-			return -1;
-		}
+		if (!mainline)
+			return error("Commit %s is a merge but no mainline was given.",
+				     sha1_to_hex(commit->object.sha1));
 
 		for (cnt = 1, p = commit->parents;
 		     cnt != mainline && p;
 		     cnt++)
 			p = p->next;
-		if (cnt != mainline || !p) {
-			error("Commit %s does not have parent %d",
-			    sha1_to_hex(commit->object.sha1), mainline);
-			return -1;
-		}
+		if (cnt != mainline || !p)
+			return error("Commit %s does not have parent %d",
+				     sha1_to_hex(commit->object.sha1),
+				     mainline);
 		parent = p->item;
-	} else if (0 < mainline) {
-		error("Mainline was specified but commit %s is not a merge.",
-		    sha1_to_hex(commit->object.sha1));
-		return -1;
-	} else
+	} else if (0 < mainline)
+		return error("Mainline was specified but commit %s is not a merge.",
+			     sha1_to_hex(commit->object.sha1));
+	else
 		parent = commit->parents->item;
 
-	if (!(message = commit->buffer)) {
-		error("Cannot get commit message for %s",
-				sha1_to_hex(commit->object.sha1));
-		return -1;
-	}
+	if (!(message = commit->buffer))
+		return error("Cannot get commit message for %s",
+			     sha1_to_hex(commit->object.sha1));
 
-	if (parent && parse_commit(parent) < 0) {
-		error("Cannot parse parent commit %s",
-		      sha1_to_hex(parent->object.sha1));
-		return -1;
-	}
+	if (parent && parse_commit(parent) < 0)
+		return error("Cannot parse parent commit %s",
+			     sha1_to_hex(parent->object.sha1));
 
 	oneline = get_oneline(message);
 
-- 
1.6.4.271.ge010d

^ permalink raw reply related

* [PATCH 05/13] revert: libify pick
From: Christian Couder @ 2009-08-12  5:15 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Johannes Schindelin, Stephan Beyer, Daniel Barkalow,
	Jakub Narebski
In-Reply-To: <20090812051116.18155.70541.chriscool@tuxfamily.org>

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

This commit is made of code from the sequencer GSoC project:

git://repo.or.cz/git/sbeyer.git

(commit e7b8dab0c2a73ade92017a52bb1405ea1534ef20)

The goal of this commit is to abstract out pick functionnality
into a new pick() function made of code from "builtin-revert.c".

The new pick() function is in a new "pick.c" file with an
associated "pick.h".

"pick.h" and "pick.c" are strictly the same as on the sequencer repo,
but a few changes were needed on "builtin-revert.c" to keep it up to
date with changes on git.git.

Mentored-by: Daniel Barkalow <barkalow@iabervon.org>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Stephan Beyer <s-beyer@gmx.net>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 Makefile         |    2 +
 builtin-revert.c |  272 +++++++++---------------------------------------------
 pick.c           |  229 +++++++++++++++++++++++++++++++++++++++++++++
 pick.h           |   13 +++
 4 files changed, 288 insertions(+), 228 deletions(-)
 create mode 100644 pick.c
 create mode 100644 pick.h

diff --git a/Makefile b/Makefile
index 189aee5..03ddc59 100644
--- a/Makefile
+++ b/Makefile
@@ -444,6 +444,7 @@ LIB_H += pack-refs.h
 LIB_H += pack-revindex.h
 LIB_H += parse-options.h
 LIB_H += patch-ids.h
+LIB_H += pick.h
 LIB_H += pkt-line.h
 LIB_H += progress.h
 LIB_H += quote.h
@@ -532,6 +533,7 @@ LIB_OBJS += parse-options.o
 LIB_OBJS += patch-delta.o
 LIB_OBJS += patch-ids.o
 LIB_OBJS += path.o
+LIB_OBJS += pick.o
 LIB_OBJS += pkt-line.o
 LIB_OBJS += preload-index.o
 LIB_OBJS += pretty.o
diff --git a/builtin-revert.c b/builtin-revert.c
index 151aa6a..6dd29a3 100644
--- a/builtin-revert.c
+++ b/builtin-revert.c
@@ -1,18 +1,14 @@
 #include "cache.h"
 #include "builtin.h"
-#include "object.h"
 #include "commit.h"
 #include "tag.h"
-#include "wt-status.h"
-#include "run-command.h"
 #include "exec_cmd.h"
 #include "utf8.h"
 #include "parse-options.h"
-#include "cache-tree.h"
 #include "diff.h"
 #include "revision.h"
 #include "rerere.h"
-#include "merge-recursive.h"
+#include "pick.h"
 
 /*
  * This implements the builtins revert and cherry-pick.
@@ -35,25 +31,23 @@ static const char * const cherry_pick_usage[] = {
 	NULL
 };
 
-static int edit, no_replay, no_commit, mainline, signoff;
-static enum { REVERT, CHERRY_PICK } action;
+static int edit, no_commit, mainline, signoff;
+static int flags;
 static struct commit *commit;
 
-static const char *me;
-
 #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
 
 static void parse_args(int argc, const char **argv)
 {
 	const char * const * usage_str =
-		action == REVERT ?  revert_usage : cherry_pick_usage;
+		flags & PICK_REVERSE ? revert_usage : cherry_pick_usage;
 	unsigned char sha1[20];
 	const char *arg;
 	int noop;
 	struct option options[] = {
 		OPT_BOOLEAN('n', "no-commit", &no_commit, "don't automatically commit"),
 		OPT_BOOLEAN('e', "edit", &edit, "edit the commit message"),
-		OPT_BOOLEAN('x', NULL, &no_replay, "append commit name when cherry-picking"),
+		OPT_BIT('x', NULL, &flags, "append commit name when cherry-picking", PICK_ADD_NOTE),
 		OPT_BOOLEAN('r', NULL, &noop, "no-op (backward compatibility)"),
 		OPT_BOOLEAN('s', "signoff", &signoff, "add Signed-off-by:"),
 		OPT_INTEGER('m', "mainline", &mainline, "parent number"),
@@ -77,42 +71,12 @@ static void parse_args(int argc, const char **argv)
 		die ("'%s' does not point to a commit", arg);
 }
 
-static char *get_oneline(const char *message)
-{
-	char *result;
-	const char *p = message, *abbrev, *eol;
-	int abbrev_len, oneline_len;
-
-	if (!p)
-		die ("Could not read commit message of %s",
-				sha1_to_hex(commit->object.sha1));
-	while (*p && (*p != '\n' || p[1] != '\n'))
-		p++;
-
-	if (*p) {
-		p += 2;
-		for (eol = p + 1; *eol && *eol != '\n'; eol++)
-			; /* do nothing */
-	} else
-		eol = p;
-	abbrev = find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV);
-	abbrev_len = strlen(abbrev);
-	oneline_len = eol - p;
-	result = xmalloc(abbrev_len + 5 + oneline_len);
-	memcpy(result, abbrev, abbrev_len);
-	memcpy(result + abbrev_len, "... ", 4);
-	memcpy(result + abbrev_len + 4, p, oneline_len);
-	result[abbrev_len + 4 + oneline_len] = '\0';
-	return result;
-}
-
 static char *get_encoding(const char *message)
 {
 	const char *p = message, *eol;
 
 	if (!p)
-		die ("Could not read commit message of %s",
-				sha1_to_hex(commit->object.sha1));
+		return NULL;
 	while (*p && *p != '\n') {
 		for (eol = p + 1; *eol && *eol != '\n'; eol++)
 			; /* do nothing */
@@ -128,30 +92,6 @@ static char *get_encoding(const char *message)
 	return NULL;
 }
 
-static struct lock_file msg_file;
-static int msg_fd;
-
-static void add_to_msg(const char *string)
-{
-	int len = strlen(string);
-	if (write_in_full(msg_fd, string, len) < 0)
-		die_errno ("Could not write to MERGE_MSG");
-}
-
-static void add_message_to_msg(const char *message)
-{
-	const char *p = message;
-	while (*p && (*p != '\n' || p[1] != '\n'))
-		p++;
-
-	if (!*p)
-		add_to_msg(sha1_to_hex(commit->object.sha1));
-
-	p += 2;
-	add_to_msg(p);
-	return;
-}
-
 static void set_author_ident_env(const char *message)
 {
 	const char *p = message;
@@ -214,7 +154,7 @@ static char *help_msg(const unsigned char *sha1)
 	       "mark the corrected paths with 'git add <paths>' "
 	       "or 'git rm <paths>' and commit the result.");
 
-	if (action == CHERRY_PICK) {
+	if (!(flags & PICK_REVERSE)) {
 		sprintf(helpbuf + strlen(helpbuf),
 			"\nWhen commiting, use the option "
 			"'-c %s' to retain authorship and message.",
@@ -223,187 +163,68 @@ static char *help_msg(const unsigned char *sha1)
 	return helpbuf;
 }
 
-static struct tree *empty_tree(void)
+static void write_message(struct strbuf *msgbuf, const char *filename)
 {
-	struct tree *tree = xcalloc(1, sizeof(struct tree));
-
-	tree->object.parsed = 1;
-	tree->object.type = OBJ_TREE;
-	pretend_sha1_file(NULL, 0, OBJ_TREE, tree->object.sha1);
-	return tree;
+	struct lock_file msg_file;
+	int msg_fd;
+	msg_fd = hold_lock_file_for_update(&msg_file, filename,
+					   LOCK_DIE_ON_ERROR);
+	if (write_in_full(msg_fd, msgbuf->buf, msgbuf->len) < 0)
+		die_errno("Could not write to %s.", filename);
+	strbuf_release(msgbuf);
+	if (commit_lock_file(&msg_file) < 0)
+		die("Error wrapping up %s", filename);
 }
 
 static int revert_or_cherry_pick(int argc, const char **argv)
 {
-	unsigned char head[20];
-	struct commit *base, *next, *parent;
-	int i, index_fd, clean;
-	char *oneline, *reencoded_message = NULL;
-	const char *message, *encoding;
-	char *defmsg = git_pathdup("MERGE_MSG");
-	struct merge_options o;
-	struct tree *result, *next_tree, *base_tree, *head_tree;
-	static struct lock_file index_lock;
+	const char *me;
+	struct strbuf msgbuf;
+	char *reencoded_message = NULL;
+	const char *encoding;
+	int failed;
 
 	git_config(git_default_config, NULL);
-	me = action == REVERT ? "revert" : "cherry-pick";
+	me = flags & PICK_REVERSE ? "revert" : "cherry-pick";
 	setenv(GIT_REFLOG_ACTION, me, 0);
 	parse_args(argc, argv);
 
-	/* this is copied from the shell script, but it's never triggered... */
-	if (action == REVERT && !no_replay)
-		die("revert is incompatible with replay");
-
 	if (read_cache() < 0)
 		die("git %s: failed to read the index", me);
-	if (no_commit) {
-		/*
-		 * We do not intend to commit immediately.  We just want to
-		 * merge the differences in, so let's compute the tree
-		 * that represents the "current" state for merge-recursive
-		 * to work on.
-		 */
-		if (write_cache_as_tree(head, 0, NULL))
-			die ("Your index file is unmerged.");
-	} else {
-		if (get_sha1("HEAD", head))
-			die ("You do not have a valid HEAD");
-		if (index_differs_from("HEAD", 0))
-			die ("Dirty index: cannot %s", me);
-	}
-	discard_cache();
-
-	index_fd = hold_locked_index(&index_lock, 1);
+	if (!no_commit && index_differs_from("HEAD", 0))
+		die ("Dirty index: cannot %s", me);
 
-	if (!commit->parents) {
-		if (action == REVERT)
-			die ("Cannot revert a root commit");
-		parent = NULL;
-	}
-	else if (commit->parents->next) {
-		/* Reverting or cherry-picking a merge commit */
-		int cnt;
-		struct commit_list *p;
-
-		if (!mainline)
-			die("Commit %s is a merge but no -m option was given.",
-			    sha1_to_hex(commit->object.sha1));
-
-		for (cnt = 1, p = commit->parents;
-		     cnt != mainline && p;
-		     cnt++)
-			p = p->next;
-		if (cnt != mainline || !p)
-			die("Commit %s does not have parent %d",
-			    sha1_to_hex(commit->object.sha1), mainline);
-		parent = p->item;
-	} else if (0 < mainline)
-		die("Mainline was specified but commit %s is not a merge.",
-		    sha1_to_hex(commit->object.sha1));
-	else
-		parent = commit->parents->item;
-
-	if (!(message = commit->buffer))
-		die ("Cannot get commit message for %s",
+	if (!commit->buffer)
+		return error("Cannot get commit message for %s",
 				sha1_to_hex(commit->object.sha1));
-
-	if (parent && parse_commit(parent) < 0)
-		die("%s: cannot parse parent commit %s",
-		    me, sha1_to_hex(parent->object.sha1));
-
-	/*
-	 * "commit" is an existing commit.  We would want to apply
-	 * the difference it introduces since its first parent "prev"
-	 * on top of the current HEAD if we are cherry-pick.  Or the
-	 * reverse of it if we are revert.
-	 */
-
-	msg_fd = hold_lock_file_for_update(&msg_file, defmsg,
-					   LOCK_DIE_ON_ERROR);
-
-	encoding = get_encoding(message);
+	encoding = get_encoding(commit->buffer);
 	if (!encoding)
 		encoding = "UTF-8";
 	if (!git_commit_encoding)
 		git_commit_encoding = "UTF-8";
-	if ((reencoded_message = reencode_string(message,
+	if ((reencoded_message = reencode_string(commit->buffer,
 					git_commit_encoding, encoding)))
-		message = reencoded_message;
-
-	oneline = get_oneline(message);
-
-	if (action == REVERT) {
-		char *oneline_body = strchr(oneline, ' ');
+		commit->buffer = reencoded_message;
 
-		base = commit;
-		next = parent;
-		add_to_msg("Revert \"");
-		add_to_msg(oneline_body + 1);
-		add_to_msg("\"\n\nThis reverts commit ");
-		add_to_msg(sha1_to_hex(commit->object.sha1));
-
-		if (commit->parents->next) {
-			add_to_msg(", reversing\nchanges made to ");
-			add_to_msg(sha1_to_hex(parent->object.sha1));
-		}
-		add_to_msg(".\n");
-	} else {
-		base = parent;
-		next = commit;
-		set_author_ident_env(message);
-		add_message_to_msg(message);
-		if (no_replay) {
-			add_to_msg("(cherry picked from commit ");
-			add_to_msg(sha1_to_hex(commit->object.sha1));
-			add_to_msg(")\n");
-		}
-	}
-
-	read_cache();
-	init_merge_options(&o);
-	o.branch1 = "HEAD";
-	o.branch2 = oneline;
-
-	head_tree = parse_tree_indirect(head);
-	next_tree = next ? next->tree : empty_tree();
-	base_tree = base ? base->tree : empty_tree();
-
-	clean = merge_trees(&o,
-			    head_tree,
-			    next_tree, base_tree, &result);
-
-	if (active_cache_changed &&
-	    (write_cache(index_fd, active_cache, active_nr) ||
-	     commit_locked_index(&index_lock)))
-		die("%s: Unable to write new index file", me);
-	rollback_lock_file(&index_lock);
-
-	if (!clean) {
-		add_to_msg("\nConflicts:\n\n");
-		for (i = 0; i < active_nr;) {
-			struct cache_entry *ce = active_cache[i++];
-			if (ce_stage(ce)) {
-				add_to_msg("\t");
-				add_to_msg(ce->name);
-				add_to_msg("\n");
-				while (i < active_nr && !strcmp(ce->name,
-						active_cache[i]->name))
-					i++;
-			}
-		}
-		if (commit_lock_file(&msg_file) < 0)
-			die ("Error wrapping up %s", defmsg);
+	failed = pick(commit, mainline, flags, &msgbuf);
+	if (failed < 0) {
+		exit(1);
+	} else if (failed > 0) {
 		fprintf(stderr, "Automatic %s failed.%s\n",
 			me, help_msg(commit->object.sha1));
+		write_message(&msgbuf, git_path("MERGE_MSG"));
 		rerere();
 		exit(1);
 	}
-	if (commit_lock_file(&msg_file) < 0)
-		die ("Error wrapping up %s", defmsg);
+	if (!(flags & PICK_REVERSE))
+		set_author_ident_env(commit->buffer);
+	free(reencoded_message);
+
 	fprintf(stderr, "Finished one %s.\n", me);
 
+	write_message(&msgbuf, git_path("MERGE_MSG"));
+
 	/*
-	 *
 	 * If we are cherry-pick, and if the merge did not result in
 	 * hand-editing, we will hit this commit and inherit the original
 	 * author date and name.
@@ -421,14 +242,11 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 			args[i++] = "-s";
 		if (!edit) {
 			args[i++] = "-F";
-			args[i++] = defmsg;
+			args[i++] = git_path("MERGE_MSG");
 		}
 		args[i] = NULL;
 		return execv_git_cmd(args);
 	}
-	free(reencoded_message);
-	free(defmsg);
-
 	return 0;
 }
 
@@ -436,14 +254,12 @@ int cmd_revert(int argc, const char **argv, const char *prefix)
 {
 	if (isatty(0))
 		edit = 1;
-	no_replay = 1;
-	action = REVERT;
+	flags = PICK_REVERSE | PICK_ADD_NOTE;
 	return revert_or_cherry_pick(argc, argv);
 }
 
 int cmd_cherry_pick(int argc, const char **argv, const char *prefix)
 {
-	no_replay = 0;
-	action = CHERRY_PICK;
+	flags = 0;
 	return revert_or_cherry_pick(argc, argv);
 }
diff --git a/pick.c b/pick.c
new file mode 100644
index 0000000..77c7169
--- /dev/null
+++ b/pick.c
@@ -0,0 +1,229 @@
+#include "cache.h"
+#include "commit.h"
+#include "run-command.h"
+#include "cache-tree.h"
+#include "pick.h"
+#include "merge-recursive.h"
+
+static struct commit *commit;
+
+static char *get_oneline(const char *message)
+{
+	char *result;
+	const char *p = message, *abbrev, *eol;
+	int abbrev_len, oneline_len;
+
+	if (!p)
+		return NULL;
+	while (*p && (*p != '\n' || p[1] != '\n'))
+		p++;
+
+	if (*p) {
+		p += 2;
+		for (eol = p + 1; *eol && *eol != '\n'; eol++)
+			; /* do nothing */
+	} else
+		eol = p;
+	abbrev = find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV);
+	abbrev_len = strlen(abbrev);
+	oneline_len = eol - p;
+	result = xmalloc(abbrev_len + 5 + oneline_len);
+	memcpy(result, abbrev, abbrev_len);
+	memcpy(result + abbrev_len, "... ", 4);
+	memcpy(result + abbrev_len + 4, p, oneline_len);
+	result[abbrev_len + 4 + oneline_len] = '\0';
+	return result;
+}
+
+static void add_message_to_msg(struct strbuf *msg, const char *message)
+{
+	const char *p = message;
+	while (*p && (*p != '\n' || p[1] != '\n'))
+		p++;
+
+	if (!*p)
+		strbuf_addstr(msg, sha1_to_hex(commit->object.sha1));
+
+	p += 2;
+	strbuf_addstr(msg, p);
+	return;
+}
+
+static struct tree *empty_tree(void)
+{
+	struct tree *tree = xcalloc(1, sizeof(struct tree));
+
+	tree->object.parsed = 1;
+	tree->object.type = OBJ_TREE;
+	pretend_sha1_file(NULL, 0, OBJ_TREE, tree->object.sha1);
+	return tree;
+}
+
+/*
+ * Pick changes introduced by pick_commit into current working tree
+ * and index.
+ *
+ * Return 0 on success.
+ * Return negative value on error before picking,
+ * and a positive value after picking,
+ * and return 1 if and only if a conflict occurs but no other error.
+ */
+int pick(struct commit *pick_commit, int mainline, int flags,
+						struct strbuf *msg)
+{
+	unsigned char head[20];
+	struct commit *base, *next, *parent;
+	int i, index_fd, clean;
+	int ret = 0;
+	char *oneline;
+	const char *message;
+	struct merge_options o;
+	struct tree *result, *next_tree, *base_tree, *head_tree;
+	static struct lock_file index_lock;
+
+	strbuf_init(msg, 0);
+	commit = pick_commit;
+
+	if (flags & PICK_REVERSE) /* REVERSE implies ADD_NOTE */
+		flags |= PICK_ADD_NOTE;
+
+	/*
+	 * We do not intend to commit immediately.  We just want to
+	 * merge the differences in, so let's compute the tree
+	 * that represents the "current" state for merge-recursive
+	 * to work on.
+	 */
+	if (write_cache_as_tree(head, 0, NULL)) {
+		error("Your index file is unmerged.");
+		return -1;
+	}
+	discard_cache();
+
+	index_fd = hold_locked_index(&index_lock, 0);
+	if (index_fd < 0) {
+		error("Unable to create locked index: %s",
+						strerror(errno));
+		return -1;
+	}
+
+	if (!commit->parents) {
+		if (flags & PICK_REVERSE) {
+			error("Cannot revert a root commit");
+			return -1;
+		}
+		parent = NULL;
+	}
+	else if (commit->parents->next) {
+		/* Reverting or cherry-picking a merge commit */
+		int cnt;
+		struct commit_list *p;
+
+		if (!mainline) {
+			error("Commit %s is a merge but no mainline was given.",
+			    sha1_to_hex(commit->object.sha1));
+			return -1;
+		}
+
+		for (cnt = 1, p = commit->parents;
+		     cnt != mainline && p;
+		     cnt++)
+			p = p->next;
+		if (cnt != mainline || !p) {
+			error("Commit %s does not have parent %d",
+			    sha1_to_hex(commit->object.sha1), mainline);
+			return -1;
+		}
+		parent = p->item;
+	} else if (0 < mainline) {
+		error("Mainline was specified but commit %s is not a merge.",
+		    sha1_to_hex(commit->object.sha1));
+		return -1;
+	} else
+		parent = commit->parents->item;
+
+	if (!(message = commit->buffer)) {
+		error("Cannot get commit message for %s",
+				sha1_to_hex(commit->object.sha1));
+		return -1;
+	}
+
+	if (parent && parse_commit(parent) < 0) {
+		error("Cannot parse parent commit %s",
+		      sha1_to_hex(parent->object.sha1));
+		return -1;
+	}
+
+	oneline = get_oneline(message);
+
+	if (flags & PICK_REVERSE) {
+		char *oneline_body = strchr(oneline, ' ');
+
+		base = commit;
+		next = parent;
+		strbuf_addstr(msg, "Revert \"");
+		strbuf_addstr(msg, oneline_body + 1);
+		strbuf_addstr(msg, "\"\n\nThis reverts commit ");
+		strbuf_addstr(msg, sha1_to_hex(commit->object.sha1));
+
+		if (commit->parents->next) {
+			strbuf_addstr(msg, ", reversing\nchanges made to ");
+			strbuf_addstr(msg, sha1_to_hex(parent->object.sha1));
+		}
+		strbuf_addstr(msg, ".\n");
+	} else {
+		base = parent;
+		next = commit;
+		add_message_to_msg(msg, message);
+		if (flags & PICK_ADD_NOTE) {
+			strbuf_addstr(msg, "(cherry picked from commit ");
+			strbuf_addstr(msg, sha1_to_hex(commit->object.sha1));
+			strbuf_addstr(msg, ")\n");
+		}
+	}
+
+	read_cache();
+	init_merge_options(&o);
+	o.branch1 = "HEAD";
+	o.branch2 = oneline;
+
+	head_tree = parse_tree_indirect(head);
+	next_tree = next ? next->tree : empty_tree();
+	base_tree = base ? base->tree : empty_tree();
+
+	clean = merge_trees(&o,
+			    head_tree,
+			    next_tree, base_tree, &result);
+
+	if (active_cache_changed &&
+	    (write_cache(index_fd, active_cache, active_nr) ||
+	     commit_locked_index(&index_lock))) {
+		error("Unable to write new index file");
+		return 2;
+	}
+	rollback_lock_file(&index_lock);
+
+	if (!clean) {
+		strbuf_addstr(msg, "\nConflicts:\n\n");
+		for (i = 0; i < active_nr;) {
+			struct cache_entry *ce = active_cache[i++];
+			if (ce_stage(ce)) {
+				strbuf_addstr(msg, "\t");
+				strbuf_addstr(msg, ce->name);
+				strbuf_addstr(msg, "\n");
+				while (i < active_nr && !strcmp(ce->name,
+						active_cache[i]->name))
+					i++;
+			}
+		}
+		ret = 1;
+	}
+	free(oneline);
+
+	discard_cache();
+	if (read_cache() < 0) {
+		error("Cannot read the index");
+		return 2;
+	}
+
+	return ret;
+}
diff --git a/pick.h b/pick.h
new file mode 100644
index 0000000..7eb0d3a
--- /dev/null
+++ b/pick.h
@@ -0,0 +1,13 @@
+#ifndef PICK_H
+#define PICK_H
+
+#include "commit.h"
+
+/* Pick flags: */
+#define PICK_REVERSE   1 /* pick the reverse changes ("revert") */
+#define PICK_ADD_NOTE  2 /* add note about original commit (unless conflict) */
+/* We don't need a PICK_QUIET. This is done by
+ *	setenv("GIT_MERGE_VERBOSITY", "0", 1); */
+extern int pick(struct commit *pick_commit, int mainline, int flags, struct strbuf *msg);
+
+#endif
-- 
1.6.4.271.ge010d

^ permalink raw reply related

* [PATCH 09/13] pick: simplify bogus comment about commiting immediately
From: Christian Couder @ 2009-08-12  5:15 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Johannes Schindelin, Stephan Beyer, Daniel Barkalow,
	Jakub Narebski
In-Reply-To: <20090812051116.18155.70541.chriscool@tuxfamily.org>

as suggested by Junio.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 pick.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/pick.c b/pick.c
index a6b1d6f..058b877 100644
--- a/pick.c
+++ b/pick.c
@@ -85,10 +85,8 @@ int pick_commit(struct commit *pick_commit, int mainline, int flags,
 	commit = pick_commit;
 
 	/*
-	 * We do not intend to commit immediately.  We just want to
-	 * merge the differences in, so let's compute the tree
-	 * that represents the "current" state for merge-recursive
-	 * to work on.
+	 * Let's compute the tree that represents the "current" state
+	 * for merge-recursive to work on.
 	 */
 	if (write_cache_as_tree(head, 0, NULL))
 		return error("Your index file is unmerged.");
-- 
1.6.4.271.ge010d

^ permalink raw reply related

* [PATCH 03/13] sequencer: let "git sequencer--helper" callers set "allow_dirty"
From: Christian Couder @ 2009-08-12  5:15 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Johannes Schindelin, Stephan Beyer, Daniel Barkalow,
	Jakub Narebski
In-Reply-To: <20090812051116.18155.70541.chriscool@tuxfamily.org>

This flag can be set when using --reset-hard or --fast-forward, and
in this case changes in the work tree will be kept.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin-sequencer--helper.c |   11 ++++++++---
 1 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/builtin-sequencer--helper.c b/builtin-sequencer--helper.c
index bd72f65..71a7fef 100644
--- a/builtin-sequencer--helper.c
+++ b/builtin-sequencer--helper.c
@@ -18,8 +18,10 @@ static unsigned char head_sha1[20];
 
 static const char * const git_sequencer_helper_usage[] = {
 	"git sequencer--helper --make-patch <commit>",
-	"git sequencer--helper --reset-hard <commit> <reflog-msg> <verbosity>",
-	"git sequencer--helper --fast-forward <commit> <reflog-msg> <verbosity>",
+	"git sequencer--helper --reset-hard <commit> <reflog-msg> "
+		"<verbosity> [<allow-dirty>]",
+	"git sequencer--helper --fast-forward <commit> <reflog-msg> "
+		"<verbosity> [<allow-dirty>]",
 	NULL
 };
 
@@ -247,7 +249,7 @@ int cmd_sequencer__helper(int argc, const char **argv, const char *prefix)
 		unsigned char sha1[20];
 		char *commit = ff_commit ? ff_commit : reset_commit;
 
-		if (argc != 2)
+		if (argc != 2 && argc != 3)
 			usage_with_options(git_sequencer_helper_usage,
 					   options);
 
@@ -263,6 +265,9 @@ int cmd_sequencer__helper(int argc, const char **argv, const char *prefix)
 			return 1;
 		}
 
+		if (argc == 3 && *argv[2] && strcmp(argv[2], "0"))
+			allow_dirty = 1;
+
 		if (ff_commit)
 			return do_fast_forward(sha1);
 		else
-- 
1.6.4.271.ge010d

^ permalink raw reply related

* [PATCH 04/13] rebase -i: use "git sequencer--helper --fast-forward"
From: Christian Couder @ 2009-08-12  5:15 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Johannes Schindelin, Stephan Beyer, Daniel Barkalow,
	Jakub Narebski
In-Reply-To: <20090812051116.18155.70541.chriscool@tuxfamily.org>

when fast forwarding.

Note that in the first hunk of this patch, there was this line:

test "a$1" = a-n && output git reset --soft $current_sha1

but the test always failed.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 git-rebase--interactive.sh |   11 +++--------
 1 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 0041994..7651fd6 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -154,11 +154,8 @@ pick_one () {
 		die "Could not get the parent of $sha1"
 	current_sha1=$(git rev-parse --verify HEAD)
 	if test "$no_ff$current_sha1" = "$parent_sha1"; then
-		git sequencer--helper --reset-hard $sha1 \
+		git sequencer--helper --fast-forward $sha1 \
 			"$GIT_REFLOG_ACTION" "$VERBOSE"
-		test "a$1" = a-n && output git reset --soft $current_sha1
-		sha1=$(git rev-parse --short $sha1)
-		output warn Fast forward to $sha1
 	else
 		output git cherry-pick "$@"
 	fi
@@ -238,10 +235,8 @@ pick_one_preserving_merges () {
 	done
 	case $fast_forward in
 	t)
-		output warn "Fast forward to $sha1"
-		git sequencer--helper --reset-hard $sha1 \
-			"$GIT_REFLOG_ACTION" "$VERBOSE" ||
-			die "Cannot fast forward to $sha1"
+		git sequencer--helper --fast-forward $sha1 \
+			"$GIT_REFLOG_ACTION" "$VERBOSE" || exit
 		;;
 	f)
 		first_parent=$(expr "$new_parents" : ' \([^ ]*\)')
-- 
1.6.4.271.ge010d

^ permalink raw reply related

* [PATCH 12/13] sequencer: add "--cherry-pick" option to "git sequencer--helper"
From: Christian Couder @ 2009-08-12  5:15 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Johannes Schindelin, Stephan Beyer, Daniel Barkalow,
	Jakub Narebski
In-Reply-To: <20090812051116.18155.70541.chriscool@tuxfamily.org>

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

This patch adds some code that comes from the sequencer GSoC project:

git://repo.or.cz/git/sbeyer.git

(commit e7b8dab0c2a73ade92017a52bb1405ea1534ef20)

Most of the code is taken from the insn_pick_act() function.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin-sequencer--helper.c |   52 ++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 51 insertions(+), 1 deletions(-)

diff --git a/builtin-sequencer--helper.c b/builtin-sequencer--helper.c
index 61a8f2e..0f2255a 100644
--- a/builtin-sequencer--helper.c
+++ b/builtin-sequencer--helper.c
@@ -288,7 +288,7 @@ static int do_commit(unsigned char *parent_sha1)
 
 	if (update_ref(reflog, "HEAD", commit_sha1, NULL, 0, 0))
 		return error("Could not update HEAD to %s.",
-						sha1_to_hex(commit_sha1));
+			     sha1_to_hex(commit_sha1));
 
 	return 0;
 }
@@ -436,6 +436,7 @@ int cmd_sequencer__helper(int argc, const char **argv, const char *prefix)
 	char *patch_commit = NULL;
 	char *reset_commit = NULL;
 	char *ff_commit = NULL;
+	char *cp_commit = NULL;
 	struct option options[] = {
 		OPT_STRING(0, "make-patch", &patch_commit, "commit",
 			   "create a patch from commit"),
@@ -443,6 +444,8 @@ int cmd_sequencer__helper(int argc, const char **argv, const char *prefix)
 			   "reset to commit"),
 		OPT_STRING(0, "fast-forward", &ff_commit, "commit",
 			   "fast forward to commit"),
+		OPT_STRING(0, "cherry-pick", &cp_commit, "commit",
+			   "cherry pick commit"),
 		OPT_END()
 	};
 
@@ -488,5 +491,52 @@ int cmd_sequencer__helper(int argc, const char **argv, const char *prefix)
 			return reset_almost_hard(sha1);
 	}
 
+	if (cp_commit) {
+		struct commit *commit;
+		int failed;
+		const char *author;
+		int no_commit = 0;
+
+		if (argc != 0 && argc != 1)
+			usage_with_options(git_sequencer_helper_usage,
+					   options);
+
+		if (argc == 1 && *argv[0] && strcmp(argv[0], "0"))
+			no_commit = 1;
+
+		if (get_sha1("HEAD", head_sha1))
+			return error("You do not have a valid HEAD.");
+
+		commit = get_commit(cp_commit);
+		if (!commit)
+			return 1;
+
+		set_pick_subject(cp_commit, commit);
+
+		failed = pick_commit(commit, 0, 0, &next_commit.summary);
+
+		set_message_source(sha1_to_hex(commit->object.sha1));
+		author = strstr(commit->buffer, "\nauthor ");
+		if (author)
+			set_author_info(author + 8);
+
+		/* We do not want extra Conflicts: lines on cherry-pick,
+		   so just take the old commit message. */
+		if (failed) {
+			strbuf_setlen(&next_commit.summary, 0);
+			strbuf_addstr(&next_commit.summary,
+				      strstr(commit->buffer, "\n\n") + 2);
+			rerere();
+			make_patch(commit);
+			write_commit_summary_into(MERGE_MSG);
+			return error(pick_help_msg(commit->object.sha1, 0));
+		}
+
+		if (!no_commit && do_commit(head_sha1))
+			return error("Could not commit.");
+
+		return 0;
+	}
+
 	usage_with_options(git_sequencer_helper_usage, options);
 }
-- 
1.6.4.271.ge010d

^ 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