Git development
 help / color / mirror / Atom feed
* Re: [PATCH] Move try_merge_command and checkout_fast_forward to libgit.a
From: Jeff King @ 2012-10-26 12:02 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy; +Cc: git
In-Reply-To: <CACsJy8D1_c9NP_nwvp6uJyKikG+_jqnm1hVEs_t_wnsYTLZ4Yg@mail.gmail.com>

On Thu, Oct 25, 2012 at 07:50:26PM +0700, Nguyen Thai Ngoc Duy wrote:

> On Thu, Oct 25, 2012 at 4:45 PM, Jeff King <peff@peff.net> wrote:
> > On Tue, Oct 23, 2012 at 09:24:51AM +0700, Nguyen Thai Ngoc Duy wrote:
> >
> >> These functions are called in sequencer.c, which is part of
> >> libgit.a. This makes libgit.a potentially require builtin/merge.c for
> >> external git commands.
> >>
> >> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> >> ---
> >>  I made some unrelated changes in sequencer.c and this problem shown
> >>  up. merge-recursive.c is probably not the best place for these
> >>  functions. I just don't want to create merge.c for them.
> >
> > I'm fine with this conceptually, but merge-recursive really is the wrong
> > place. I'd much rather see a new merge.c to collect merge-related helper
> > functions that are not strategy-specific.
> 
> OK. I checked around for similar issues and found these used by
> libgit.a but stay in builtin/ instead:

Yeah, we have traditionally been kind of lazy about the distinction,
because it doesn't really matter for our build system (i.e., libgit.a is
not _really_ a library, but just a convenience in the build process). So
one option is just not caring about these. I'm also fine with fixing.

> estimate_bisect_steps: bisect.c and builtin/rev-list.c
> print_commit_list: bisect.c and builtin/rev-list.c
> 
>  -> move them to bisect.c? another candidate is revision.c.

I'd probably say bisect.c for the first, and commit.c for the latter
(that is where commit_list functions are defined, and it is really about
that).

> fetch_pack: transport.c and builtin/fetch-pack.c
> send_pack: transport.c and builtin/send-pack.c
> 
>  -> move them to transport.c? or new files fetch-pack.c and
> send-pack.c? I haven't check how many functions they may pull
> together.

I think I'd rather have fetch-pack.c and send-pack.c than putting them
into transport.c (which is getting kind of bloated already).

> setup_diff_pager: diff-no-index.c and builtin/diff.c
> 
>  -> to diff-lib.c?

I'm not sure (to be honest, I am not even sure of the intended
difference between diff.c and diff-lib.c). That function is really not
for general diffing, but for diff-like commands. Probably it would be OK
in diff.c.

-Peff

^ permalink raw reply

* Re: [PATCH] Use character class for sed expression instead of \s
From: Jeff King @ 2012-10-26 12:38 UTC (permalink / raw)
  To: Ben Walton; +Cc: tboegi, bosch, brian, git
In-Reply-To: <1351180699-24695-1-git-send-email-bdwalton@gmail.com>

On Thu, Oct 25, 2012 at 04:58:19PM +0100, Ben Walton wrote:

> Sed on Mac OS X doesn't handle \s in a sed expressions so use a more
> portable character set expression instead.
> 
> Signed-off-by: Ben Walton <bdwalton@gmail.com>

Thanks, I think this simple solution is the best.

-Peff

^ permalink raw reply

* Re: [PATCHv2] git-pull: Avoid merge-base on detached head
From: Jeff King @ 2012-10-26 12:48 UTC (permalink / raw)
  To: Phil Hord; +Cc: git, phil.hord, Junio C Hamano
In-Reply-To: <508989FD.7090101@cisco.com>

On Thu, Oct 25, 2012 at 02:50:37PM -0400, Phil Hord wrote:

> >> git pull --rebase does some clever tricks to find the base
> >> for $upstream , but it forgets that we may not have any
> >> branch at all.  When this happens, git merge-base reports its
> >> "usage" help in the middle of an otherwise successful
> >> rebase operation, because git-merge is called with one too
> >> few parameters.
> >>
> >> Since we do not need the merge-base trick in the case of a
> >> detached HEAD, detect this condition and bypass the clever
> >> trick and the usage noise.
> > Makes sense. I notice there is no test with your patch. It sounds from
> > the description like the operation still may succeed, but we just get
> > cruft on stderr?
> >
> 
> Yes, that's correct.

OK. I'd rather not worry about a test then, as it would necessarily be
flaky with respect to what git-pull might generate on its stderr. I've
queued your patch in 'next'.

Thanks.

-Peff

^ permalink raw reply

* Re: Subtree in Git
From: Herman van Rink @ 2012-10-26 13:10 UTC (permalink / raw)
  To: dag; +Cc: Junio C Hamano, greened, Hilco Wijbenga, Git Users
In-Reply-To: <nnga9vefu1v.fsf@transit.us.cray.com>

On 10/22/2012 04:41 PM, dag@cray.com wrote:
> Herman van Rink <rink@initfour.nl> writes:
>
>> On 10/21/2012 08:32 AM, Junio C Hamano wrote:
>>> Herman van Rink <rink@initfour.nl> writes:
>>>
>>>> Junio, Could you please consider merging the single commit from my
>>>> subtree-updates branch? https://github.com/helmo/git/tree/subtree-updates
>>> In general, in areas like contrib/ where there is a volunteer area
>>> maintainer, unless the change something ultra-urgent (e.g. serious
>>> security fix) and the area maintainer is unavailable, I'm really
>>> reluctant to bypass and take a single patch that adds many things
>>> that are independent from each other.
>> Who do you see as volunteer area maintainer for contrib/subtree?
>> My best guess would be Dave. And he already indicated earlier in the
>> thread to be ok with the combined patch as long as you are ok with it.
> Let's be clear.  Junio owns the project so what he says goes, no
> question.  I provided some review feedback which I thought would help
> the patches get in more easily.  We really shouldn't be adding multiple
> features in one patch.  This is easily separated into multiple patches.
>
> Then there is the issue of testcases.  We should NOT have git-subtree go
> back to the pre-merge _ad_hoc_ test environment.  We should use what the
> usptream project uses.  That will make mainlining this much easier in
> the future.
>
> If Junio is ok with overriding my decisions here, that's fine.  But I
> really don't understand why you are so hesitant to rework the patches
> when it should be realtively easy.  Certainly easier than convincing me
> they are in good shape currently.  :)

If it's so easy to rework these patches then please do so yourself.
It's been ages since I've worked on this so I would also have to
re-discover everything.

And yes it's ugly, but so is the code that you've merged in.
Which you haven't changed a single line in after merging. So it's still
the version from Avery which hasn't been maintained for the last two years.

This ball of wax is still a big improvement in my opinion.

Feel free to scrape the github forks for improvements on your own,
cleanup the code style and create fitting test cases.
But if you intend on taking in changes only when presented to you on a
silver patter, then we're better of with subtree removed from the
contrib tree.

What would a random user have to do to get a patch in? I've found a
number of subtree related mails on the git-user list go completely
unanswerd.
Amongst them a patch from James Nylen wich seems very reasonable.

-- 

Met vriendelijke groet / Regards,

Herman van Rink
Initfour websolutions

^ permalink raw reply

* Re: [PATCH] submodule status: properly pass options with --recursive
From: Jeff King @ 2012-10-26 13:15 UTC (permalink / raw)
  To: Jens Lehmann; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <5089BB2D.90400@web.de>

On Fri, Oct 26, 2012 at 12:20:29AM +0200, Jens Lehmann wrote:

> When renaming orig_args to orig_flags in 98dbe63d (submodule: only
> preserve flags across recursive status/update invocations) the call site
> of the recursive cmd_status was forgotten. At that place orig_args is
> still passed into the recursion, which is always empty now. This clears
> all options when recursing, as that variable is never set.
> 
> Fix that by renaming orig_args to orig_flags there too and add a test to
> catch that bug.

Thanks. I back-ported your patch on top of 98dbe63d so it can go to
'maint'. I'm curious, though: why didn't the test right before (which
checks recursion for --cached) catch this?

-Peff

^ permalink raw reply

* Re: [PATCH] Doc format-patch: clarify --notes use case
From: Jeff King @ 2012-10-26 13:26 UTC (permalink / raw)
  To: Philip Oakley; +Cc: GitList, Junio C Hamano, Eric Blake
In-Reply-To: <7427D8D31835443F96156C24D3EB6A5A@PhilipOakley>

On Thu, Oct 25, 2012 at 11:48:04PM +0100, Philip Oakley wrote:

> >+the commit that does not belong to the commit log message proper,
> >+and include it with the patch submission. While one can simply write
> >+these explanations after `format-patch` has run but before sending,
> >+keeping them as git notes allows them to be maintained between
> >versions
> >+of the patch series (but see the discussion of the `notes.rewrite.*`
> >+configuration in linkgit:git-notes[1] to use this workflow).
> 
> Should the notes.rewriteRef also be mentioned (not part of .*), which
> is at the end of the notes configuration and that it must be set may
> be missed by readers? Perhaps "the 'notes.rewrite' configurations",
> dropping the .* and making it plural?

Yeah, I think just saying "notes.rewrite" is better. The notes.rewrite.*
entry does point to rewriteRef, but there is no point in giving the user
a trail of "see also" breadcrumbs when we can just point them to the
area as a whole.

I updated it to this:

diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index 066dc8b..259dce4 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -196,13 +196,12 @@ The expected use case of this is to write supporting explanation for
 	after the three-dash line.
 +
 The expected use case of this is to write supporting explanation for
-the commit that does not belong to the commit log message proper
-when (or after) you create the commit, and include it in your patch
-submission.  But if you can plan ahead and write it down, there may
-not be a good reason not to write it in your commit message, and if
-you can't, you can always edit the output of format-patch before
-sending it out, so the practical value of this option is somewhat
-dubious, unless your workflow is broken.
+the commit that does not belong to the commit log message proper,
+and include it with the patch submission. While one can simply write
+these explanations after `format-patch` has run but before sending,
+keeping them as git notes allows them to be maintained between versions
+of the patch series (but see the discussion of the `notes.rewrite`
+configuration options in linkgit:git-notes[1] to use this workflow).
 
 --[no]-signature=<signature>::
 	Add a signature to each message produced. Per RFC 3676 the signature
-- 
1.8.0.3.g3456896

^ permalink raw reply related

* Re: [PATCH] Document git-svn fetch --log-window-size parameter
From: Jeff King @ 2012-10-26 13:32 UTC (permalink / raw)
  To: Eric Wong; +Cc: Gunnlaugur Þór Briem, git
In-Reply-To: <20121026094602.GA7887@dcvr.yhbt.net>

On Fri, Oct 26, 2012 at 09:46:02AM +0000, Eric Wong wrote:

> > Thanks. Your description makes sense to me, but I do not have enough
> > git-svn knowledge to know if it covers all intended uses of the flag.
> > Eric?
> > 
> > > +--log-window-size=<n>;;
> > > +    Fetch <n> log entries per request when scanning Subversion history.
> > > +    The default is 100. For very large Subversion repositories, larger
> > > +    values may be needed for 'clone'/'fetch' to complete in reasonable
> > > +    time. But overly large values may lead to request timeouts.
> 
> Overly large values also lead to excessive memory usage.  I may have
> only had 256M in my dev machine at the time I added this parameter:

That's probably worth mentioning. Gunnlaugur, any objection to me
amending your commit with:

diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
index 64756c9..8b0d3ad 100644
--- a/Documentation/git-svn.txt
+++ b/Documentation/git-svn.txt
@@ -150,7 +150,8 @@ Skip "branches" and "tags" of first level directories;;
     Fetch <n> log entries per request when scanning Subversion history.
     The default is 100. For very large Subversion repositories, larger
     values may be needed for 'clone'/'fetch' to complete in reasonable
-    time. But overly large values may lead to request timeouts.
+    time. But overly large values may lead to higher memory usage and
+    request timeouts.
 
 'clone'::
 	Runs 'init' and 'fetch'.  It will automatically create a

?

-Peff

^ permalink raw reply related

* [PATCH] replace: parse revision argument for -d
From: Michael J Gruber @ 2012-10-26 13:33 UTC (permalink / raw)
  To: git

'git replace' parses the revision arguments when it creates replacements
(so that a sha1 can be abbreviated, e.g.) but not when deleting
replacements.

This sucks.

Make it parse the argument to 'replace -d' in the same way.

Just in case someone lost the replacement object before deleting the
replacement, take the argument literally if it can not be resolved to a
full sha1.

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
 builtin/replace.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/builtin/replace.c b/builtin/replace.c
index e3aaf70..80e2039 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -46,24 +46,29 @@ typedef int (*each_replace_name_fn)(const char *name, const char *ref,
 
 static int for_each_replace_name(const char **argv, each_replace_name_fn fn)
 {
-	const char **p;
+	const char **p, *q;
 	char ref[PATH_MAX];
 	int had_error = 0;
 	unsigned char sha1[20];
 
 	for (p = argv; *p; p++) {
-		if (snprintf(ref, sizeof(ref), "refs/replace/%s", *p)
+		q = *p;
+		if (get_sha1(q, sha1))
+			warning("Failed to resolve '%s' as a valid ref; taking it literally.", q);
+		else
+			q = sha1_to_hex(sha1);
+		if (snprintf(ref, sizeof(ref), "refs/replace/%s", q)
 					>= sizeof(ref)) {
-			error("replace ref name too long: %.*s...", 50, *p);
+			error("replace ref name too long: %.*s...", 50, q);
 			had_error = 1;
 			continue;
 		}
 		if (read_ref(ref, sha1)) {
-			error("replace ref '%s' not found.", *p);
+			error("replace ref '%s' not found.", q);
 			had_error = 1;
 			continue;
 		}
-		if (fn(*p, ref, sha1))
+		if (fn(q, ref, sha1))
 			had_error = 1;
 	}
 	return had_error;
-- 
1.8.0.370.g8cbad08

^ permalink raw reply related

* Re: git push tags
From: Drew Northup @ 2012-10-26 13:37 UTC (permalink / raw)
  To: Angelo Borsotti; +Cc: git
In-Reply-To: <CAB9Jk9ApdbN+4Yk-6+u4Nje9+tbESSGGVj95iof1jnfG4Ga7GA@mail.gmail.com>

On Fri, Oct 26, 2012 at 2:42 AM, Angelo Borsotti
<angelo.borsotti@gmail.com> wrote:
> Hi Drew,
>
------Adding for clarity: On Thurs, Oct 25, 2012 at 17:16 EDT, Drew
Northup wrote:
>>
>> Changing the tag in the local repository is a tag modification
>> operation. Pushing that change to a remote repository DOES NOT execute
>> "git tag...." in the remote. Plain and simple the two are different
>> operations.
>>
>
> They are different for what concerns the implementation. They are not
> necessarily so for what concerns their semantics, and the most
> straightforward is to apply to the remote repository the changes done
> on the local one -- the changes that can legally done on it -- and
> changing a tag is not one allowed (unless forced).

So you want the rules of "git tag" to be forced upon "git push." I
heard (read) that already. By what means do you intend to enforce
that? Please go look at the code of "git push" (builtin/push.c) before
coming back with an answer.

> Obviously, the semantics of git-push is different, and then needs to
> be described clearly.
> Note that some (probably most) of the operations that are disallowed
> on the local repo are also disallowed by git-push, like, e.g. deleting
> the current branch. But the user cannot tell what is disallowed and
> what not if the man page does not state it.

If you think something is inadequately documented then do please
submit a patch proposing the changes you would like to see. Even if it
isn't quite right the first time you can get help from interested
people on the list getting it accepted. If nothing else you'll get
some constructive discussion going.

(As for deleting the current branch, you can't really do that on a
proper bare remote anyway as there is no such thing as a "current
branch" in that context.)

>> So here we come to the core argument. Is sounds to me like you want
>> changes to remote tags to work differently from push updates to ALL
>> other references. The required change, if I'm not mistaken, would be
>> for tags to not permit fast-forward updates while all other references
>> would be pushed normally. From my brief and un-enlightened look at the
>> push code I can't see that being as easy as it sounds.
>>
>
> No, I was hoping that git-push refused to change tags at all, unless
> forced (e.g. prefixing them with +), as it is on a local repository.

I'm not sure how you get to negate my statement and then restate it
and think that you are saying something different. Please go look at
the the source code. Tags are just a class of refs that we set aside
for human consumption while also providing them with meaning that the
computer can understand.

What are refs then? Refs are REFERENCES to specific objects (at this
point I know only of commits being _referenced_ that way) by their sha
hashes (literally, that's what the file contains) in the repository.
That's it. Git push/pull doesn't handle them any differently may they
be heads or tags. (Remote heads, aka tracking branches, are references
to objects in the repository that do not exist in a local branch. It
doesn't look like we push those, but I could be wrong.) What Git does
do is allow you the flexibility of pushing refs selectively (with some
caveats). Once tags are loaded into the queue of references to be
pushed they are handled like any other kind of reference.

If you think there would be a benefit to making the reference pushing
code "type aware" then do please look into the code and submit a
patch. I am not going to waste any more time arguing about how it
"should be" if you cannot go to the trouble of reading the code and
coming up with a solution that implements what you would like to see
happen. Frankly, if all you can come up with is pseudocode you still
will at least have a chance of working with the people here on this
list to to write up something that implements the behavior you would
like to see. The first step to that is understanding how it actually
works now.

-- 
-Drew Northup
--------------------------------------------------------------
"As opposed to vegetable or mineral error?"
-John Pescatore, SANS NewsBites Vol. 12 Num. 59

^ permalink raw reply

* Re: [PATCH] git-submodule add: Record branch name in .gitmodules
From: Jeff King @ 2012-10-26 13:37 UTC (permalink / raw)
  To: W. Trevor King; +Cc: Phil Hord, git
In-Reply-To: <20121025011227.GH801@odin.tremily.us>

On Wed, Oct 24, 2012 at 09:12:27PM -0400, W. Trevor King wrote:

> On Wed, Oct 24, 2012 at 02:12:18PM -0400, Phil Hord wrote:
> > +                                       VAR_NAME=$(printf '%s'
> > "$VAR_NAME" | tr A-Z a-z | sed -e 's/^[^a-z]/_/' -e 's/[^a-z0-9]/_/g')
> 
> Is there a reason why you use printf instead of echo?

Sadly, echo is not entirely portable for arbitrary content. We have run
into problems with backslashes being interpreted differently (bash
requires "echo -e" to interpret them, dash will always do it, and POSIX
leaves it vague).

> Also, this sort of name cleaning should probably live in a new
> function:
> 
>   clean_environment_variable()
> 
> or some such.  Is there a git-utility-functions.sh library hiding
> somewhere in the source? ;)

git-sh-setup?

-Peff

^ permalink raw reply

* Re: [PATCH] Document git-svn fetch --log-window-size parameter
From: Gunnlaugur Thor Briem @ 2012-10-26 13:45 UTC (permalink / raw)
  To: Jeff King; +Cc: Eric Wong, git
In-Reply-To: <20121026133250.GI1455@sigill.intra.peff.net>

On Fri, Oct 26, 2012 at 1:32 PM, Jeff King <peff@peff.net> wrote:
> That's probably worth mentioning. Gunnlaugur, any objection to me
> amending your commit with:
>
> diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
> index 64756c9..8b0d3ad 100644
> --- a/Documentation/git-svn.txt
> +++ b/Documentation/git-svn.txt
> @@ -150,7 +150,8 @@ Skip "branches" and "tags" of first level directories;;
>      Fetch <n> log entries per request when scanning Subversion history.
>      The default is 100. For very large Subversion repositories, larger
>      values may be needed for 'clone'/'fetch' to complete in reasonable
> -    time. But overly large values may lead to request timeouts.
> +    time. But overly large values may lead to higher memory usage and
> +    request timeouts.
>
>  'clone'::
>         Runs 'init' and 'fetch'.  It will automatically create a
>
> ?

No objection, that sounds fine to me.

-Gulli

^ permalink raw reply

* Re: Subtree in Git
From: David Michael Barr @ 2012-10-26 13:58 UTC (permalink / raw)
  To: Herman van Rink; +Cc: dag, Junio C Hamano, greened, Hilco Wijbenga, Git Users
In-Reply-To: <508A8BD3.9020901@initfour.nl>

On Saturday, 27 October 2012 at 12:10 AM, Herman van Rink wrote:
> On 10/22/2012 04:41 PM, dag@cray.com (mailto:dag@cray.com) wrote:
> > Herman van Rink <rink@initfour.nl (mailto:rink@initfour.nl)> writes:
> > 
> > > On 10/21/2012 08:32 AM, Junio C Hamano wrote:
> > > > Herman van Rink <rink@initfour.nl (mailto:rink@initfour.nl)> writes:
> > > > 
> > > > > Junio, Could you please consider merging the single commit from my
> > > > > subtree-updates branch? https://github.com/helmo/git/tree/subtree-updates
> > > > 
> > > > 
> > > > In general, in areas like contrib/ where there is a volunteer area
> > > > maintainer, unless the change something ultra-urgent (e.g. serious
> > > > security fix) and the area maintainer is unavailable, I'm really
> > > > reluctant to bypass and take a single patch that adds many things
> > > > that are independent from each other.
> > > 
> > > 
> > > Who do you see as volunteer area maintainer for contrib/subtree?
> > > My best guess would be Dave. And he already indicated earlier in the
> > > thread to be ok with the combined patch as long as you are ok with it.
> > 
> > 
> > Let's be clear. Junio owns the project so what he says goes, no
> > question. I provided some review feedback which I thought would help
> > the patches get in more easily. We really shouldn't be adding multiple
> > features in one patch. This is easily separated into multiple patches.
> > 
> > Then there is the issue of testcases. We should NOT have git-subtree go
> > back to the pre-merge _ad_hoc_ test environment. We should use what the
> > usptream project uses. That will make mainlining this much easier in
> > the future.
> > 
> > If Junio is ok with overriding my decisions here, that's fine. But I
> > really don't understand why you are so hesitant to rework the patches
> > when it should be realtively easy. Certainly easier than convincing me
> > they are in good shape currently. :)
> 
> 
> 
> If it's so easy to rework these patches then please do so yourself.
> It's been ages since I've worked on this so I would also have to
> re-discover everything.

>From a quick survey, it appears there are no more than 55 patches
squashed into the submitted patch.
As I have an interest in git-subtree for maintaining the out-of-tree
version of vcs-svn/ and a desire to improve my rebase-fu, I am tempted
to make some sense of the organic growth that happened on GitHub.
It doesn't appear that anyone else is willing to do this, so I doubt
there will be any duplication of effort.



--
David Michael Barr

^ permalink raw reply

* Re: git push tags
From: Chris Rorvick @ 2012-10-26 13:59 UTC (permalink / raw)
  To: Drew Northup; +Cc: Angelo Borsotti, git
In-Reply-To: <CAM9Z-n=+YeF3BaF-5UB5f2GBHYw7WDZEFDVT72RGSsDBaH4PJA@mail.gmail.com>

On Fri, Oct 26, 2012 at 8:37 AM, Drew Northup <n1xim.email@gmail.com> wrote:
> (As for deleting the current branch, you can't really do that on a
> proper bare remote anyway as there is no such thing as a "current
> branch" in that context.)

Really?  When I clone a bare repository I see a HEAD, and Git doesn't
want me to delete it with a push from a remote repo.  So is this not a
"proper" bare repository?

^ permalink raw reply

* Re: [PATCH v2] git-submodule add: Add -r/--record option.
From: Jeff King @ 2012-10-26 14:00 UTC (permalink / raw)
  To: W. Trevor King; +Cc: Git, Nahor, Phil Hord
In-Reply-To: <20121025221431.GH13647@odin.tremily.us>

On Thu, Oct 25, 2012 at 06:14:31PM -0400, W. Trevor King wrote:

> Should I rebase this so it lands cleanly atop 38ae92e4 in next?
> 
>   commit 38ae92e4d027063b9b87e51a9bf12809d10066f6
>   Author: W. Trevor King <wking@tremily.us>
>   Date:   Tue Oct 23 17:00:21 2012 -0400
> 
>     git-submodule: wrap branch option with "<>" in usage strings.

In general, it is not a good idea to base your patches on things in
next, because it means your topic is held hostage to the one in next,
which may or may not graduate to master. We can always do a merge later
(and in this case, it is really just a one-line conflict).

-Peff

^ permalink raw reply

* git submodule summary doesn't return an error when passed a wrong commit/rev
From: Francis Moreau @ 2012-10-26 14:03 UTC (permalink / raw)
  To: git

Hi,

it seems to me that when passed an unknown rev or a wrong commit/sha1,
git-submodule-summary should at least exit with an error status. Even better
would be a error output.

Test was done with git version 1.7.10.4 from debian wheezy.

Thanks
-- 
Francis

^ permalink raw reply

* Can't understand the behaviour of git-diff --submodule
From: Francis Moreau @ 2012-10-26 14:07 UTC (permalink / raw)
  To: git

Hi,

I'm trying to use the --submodule switch with git-diff but doesnt
understand the following behaviour:

$ git diff 2c9a257718d1803de720f95766ff256d33accad5 HEAD
diff --git a/configs b/configs
index 16c6a89..ce12289 160000
--- a/configs
+++ b/configs
@@ -1 +1 @@
-Subproject commit 16c6a89f245f0eed7fb0bce8e027c59fcf1d543e
+Subproject commit ce12289c5bfca7b2c423d9f1871c13ad1ba1dc32

but adding the --submodule option gives:

$ git diff --submodule=log 2c9a257718d1803de720f95766ff256d33accad5 HEAD
Submodule configs 16c6a89...ce12289 (commits not present)

Could anybody enlight me ?
-- 
Francis

^ permalink raw reply related

* Re: git push tags
From: Drew Northup @ 2012-10-26 14:13 UTC (permalink / raw)
  To: Chris Rorvick; +Cc: Angelo Borsotti, git
In-Reply-To: <CAEUsAPZC80aSwnS0sBTt9nEu0HgyPp0BD2TWjSBb7tS7=i=VhQ@mail.gmail.com>

On Fri, Oct 26, 2012 at 9:59 AM, Chris Rorvick <chris@rorvick.com> wrote:
> On Fri, Oct 26, 2012 at 8:37 AM, Drew Northup <n1xim.email@gmail.com> wrote:
>> (As for deleting the current branch, you can't really do that on a
>> proper bare remote anyway as there is no such thing as a "current
>> branch" in that context.)
>
> Really?  When I clone a bare repository I see a HEAD, and Git doesn't
> want me to delete it with a push from a remote repo.  So is this not a
> "proper" bare repository?

Chris,
If there's no working directory in the remote repository is there a
"current branch" in the _remote_ repository? (I am not talking about
the tracking branch here. I also presume that attempting to delete the
one and only branch is somewhat nonsensical.)

In any case, this is a different mental model than the one Angelo
seems (to me) to be arguing.

-- 
-Drew Northup
--------------------------------------------------------------
"As opposed to vegetable or mineral error?"
-John Pescatore, SANS NewsBites Vol. 12 Num. 59

^ permalink raw reply

* Re: git push tags
From: Chris Rorvick @ 2012-10-26 14:23 UTC (permalink / raw)
  To: Drew Northup; +Cc: Angelo Borsotti, git
In-Reply-To: <CAM9Z-nnrFxhX7XxBqe4qfuqZigpD12CrN2W6d=L996rJ_ecWFQ@mail.gmail.com>

On Fri, Oct 26, 2012 at 9:13 AM, Drew Northup <n1xim.email@gmail.com> wrote:
> On Fri, Oct 26, 2012 at 9:59 AM, Chris Rorvick <chris@rorvick.com> wrote:
>> On Fri, Oct 26, 2012 at 8:37 AM, Drew Northup <n1xim.email@gmail.com> wrote:
>>> (As for deleting the current branch, you can't really do that on a
>>> proper bare remote anyway as there is no such thing as a "current
>>> branch" in that context.)
>>
>> Really?  When I clone a bare repository I see a HEAD, and Git doesn't
>> want me to delete it with a push from a remote repo.  So is this not a
>> "proper" bare repository?
>
> Chris,
> If there's no working directory in the remote repository is there a
> "current branch" in the _remote_ repository?

Yes, I believe it is the default branch when the repository is cloned.

^ permalink raw reply

* Re: git push tags
From: Angelo Borsotti @ 2012-10-26 15:23 UTC (permalink / raw)
  To: Drew Northup; +Cc: git
In-Reply-To: <CAM9Z-n=+YeF3BaF-5UB5f2GBHYw7WDZEFDVT72RGSsDBaH4PJA@mail.gmail.com>

Hi Drew,

git is an open source, community project, which means that it benefits
from all the contributions of many people, and they are not restricted
to patches.
If the only one suggestions that were taken into account were patches
sent by people that had the time to study the sources and propose
changes, then we would miss the opportunity of taking many good things
that the community generates, like new ideas for example.

By the way, I already browsed the code, but have seen that there is
not only push.c to understand, but a dozen or more of other sources.
This is why I am not yet able to propose patches.
But I am using git all day, and often come to unexpected or
undocumented behaviors, and want to share my findings in case they
could serve to improve git. This means to me to spend time in
formulating them the best I can to make others understand what I have
found, and that is a contribution too.

Here is a proposed change to the git-push manpage:

- section: "DESCRIPTION", first paragraph ("Updates remote .."), add at the end:

   "Remote references (branches and tags) that do not exist are
created. The ones that exist are updated except when + is not
specified and they are not fast-forward updates."

-Angelo

^ permalink raw reply

* Re: [PATCH] replace: parse revision argument for -d
From: Christian Couder @ 2012-10-26 15:25 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: git
In-Reply-To: <807340e40adb1fc97cd97161fe1fabd292bc79c3.1351258394.git.git@drmicha.warpmail.net>

Hi,

On Fri, Oct 26, 2012 at 3:33 PM, Michael J Gruber
<git@drmicha.warpmail.net> wrote:
> 'git replace' parses the revision arguments when it creates replacements
> (so that a sha1 can be abbreviated, e.g.) but not when deleting
> replacements.
>
> This sucks.
>
> Make it parse the argument to 'replace -d' in the same way.

Nit: there could be more than one argument to 'replace -d', so perhaps
"each argument" is better.

> Just in case someone lost the replacement object before deleting the
> replacement, take the argument literally if it can not be resolved to a

Here too.

> full sha1.
>
> Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
> ---
>  builtin/replace.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/builtin/replace.c b/builtin/replace.c
> index e3aaf70..80e2039 100644
> --- a/builtin/replace.c
> +++ b/builtin/replace.c
> @@ -46,24 +46,29 @@ typedef int (*each_replace_name_fn)(const char *name, const char *ref,
>
>  static int for_each_replace_name(const char **argv, each_replace_name_fn fn)
>  {
> -       const char **p;
> +       const char **p, *q;
>         char ref[PATH_MAX];
>         int had_error = 0;
>         unsigned char sha1[20];
>
>         for (p = argv; *p; p++) {
> -               if (snprintf(ref, sizeof(ref), "refs/replace/%s", *p)
> +               q = *p;
> +               if (get_sha1(q, sha1))
> +                       warning("Failed to resolve '%s' as a valid ref; taking it literally.", q);
> +               else
> +                       q = sha1_to_hex(sha1);
> +               if (snprintf(ref, sizeof(ref), "refs/replace/%s", q)
>                                         >= sizeof(ref)) {
> -                       error("replace ref name too long: %.*s...", 50, *p);
> +                       error("replace ref name too long: %.*s...", 50, q);
>                         had_error = 1;
>                         continue;
>                 }
>                 if (read_ref(ref, sha1)) {
> -                       error("replace ref '%s' not found.", *p);
> +                       error("replace ref '%s' not found.", q);
>                         had_error = 1;
>                         continue;
>                 }
> -               if (fn(*p, ref, sha1))
> +               if (fn(q, ref, sha1))
>                         had_error = 1;
>         }
>         return had_error;

Looks good to me.

Thanks,
Christian.

^ permalink raw reply

* Re: [PATCH] Fix git p4 sync errors
From: Christian Couder @ 2012-10-26 15:44 UTC (permalink / raw)
  To: Matt Arsenault; +Cc: Junio C Hamano, git, Pete Wyckoff, Luke Diamand
In-Reply-To: <F0F92B0A-D37F-40D4-A0DF-43EEDA2818B9@gmail.com>

Hi,

On Thu, Oct 25, 2012 at 4:41 AM, Matt Arsenault <arsenm2@gmail.com> wrote:
>
> On Oct 21, 2012, at 12:06 , Junio C Hamano <gitster@pobox.com> wrote:
>>
>> - Why is it a bug not to pass "-s"?  How does the bug happen?
>
> I encountered this one time after using it for months. One day I couldn't git p4 rebase
> with the key error.  I searched for the error and found some version of git-p4 that fixed
> a similar error by adding the -s to describe. Adding the -s fixed the error and
> everything seemed to be working correctly.

The perforce documentation says the following about this flag:

"The -s flag omits the diffs of files that were updated."

So if the diffs are not used, there is no downside to use -s.
Maybe the patch should just state this.

Best,
Christian.

^ permalink raw reply

* [PATCH 0/7] Move code from builtin to libgit.a
From: Nguyễn Thái Ngọc Duy @ 2012-10-26 15:53 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Nguyễn Thái Ngọc Duy
In-Reply-To: <20121026120253.GA1455@sigill.intra.peff.net>

On Fri, Oct 26, 2012 at 7:02 PM, Jeff King <peff@peff.net> wrote:
>> OK. I checked around for similar issues and found these used by
>> libgit.a but stay in builtin/ instead:
>
> Yeah, we have traditionally been kind of lazy about the distinction,
> because it doesn't really matter for our build system (i.e., libgit.a is
> not _really_ a library, but just a convenience in the build process). So
> one option is just not caring about these.

Yeah. We can fix it up when we encounter new undefined reference
errors (e.g. a new test program that links to libgit.a). Anyway it
does not look too difficult to do. Take it or drop it. Your call.

> I'm also fine with fixing.
>
>> estimate_bisect_steps: bisect.c and builtin/rev-list.c
>> print_commit_list: bisect.c and builtin/rev-list.c
>>
>>  -> move them to bisect.c? another candidate is revision.c.
>
> I'd probably say bisect.c for the first, and commit.c for the latter
> (that is where commit_list functions are defined, and it is really about
> that).

Makes sense.

>> setup_diff_pager: diff-no-index.c and builtin/diff.c
>>
>>  -> to diff-lib.c?
>
> I'm not sure (to be honest, I am not even sure of the intended
> difference between diff.c and diff-lib.c). That function is really not
> for general diffing, but for diff-like commands. Probably it would be OK
> in diff.c.

Ditto. check_pager_config() is also moved from git.c to pager.c.

>> fetch_pack: transport.c and builtin/fetch-pack.c
>> send_pack: transport.c and builtin/send-pack.c
>>
>>  -> move them to transport.c? or new files fetch-pack.c and
>> send-pack.c? I haven't check how many functions they may pull
>> together.
>
> I think I'd rather have fetch-pack.c and send-pack.c than putting them
> into transport.c (which is getting kind of bloated already).

These two are big code moves, basically all {fetch,send}-pack code.
While I think this is good cleanup (I get surprise every time I look
at transport.c and it leads to buitin/fetch-pack.c), this is probably
not a good idea if we have on flight topics, which may cause lots of
conflicts for you and Junio. Fortunately we don't at the moment.

Nguyễn Thái Ngọc Duy (7):
  Move try_merge_command and checkout_fast_forward to libgit.a
  Move estimate_bisect_steps to libgit.a
  Move print_commit_list to libgit.a
  Move setup_diff_pager to libgit.a
  send-pack: move core code to libgit.a
  fetch-pack: remove global (static) configuration variable "args"
  fetch-pack: move core code to libgit.a

 Makefile             |   3 +
 bisect.c             |  38 ++
 bisect.h             |   4 -
 builtin.h            |   4 -
 builtin/diff.c       |  16 -
 builtin/fetch-pack.c | 951 +--------------------------------------------------
 builtin/merge.c      | 106 +-----
 builtin/rev-list.c   |  49 ---
 builtin/send-pack.c  | 333 ------------------
 cache.h              |  12 +-
 commit.c             |  10 +
 commit.h             |   4 +
 diff.c               |  16 +
 diff.h               |   1 +
 fetch-pack.c         | 951 +++++++++++++++++++++++++++++++++++++++++++++++++++
 git.c                |  33 --
 merge-recursive.h    |   5 -
 merge.c              | 112 ++++++
 pager.c              |  34 ++
 send-pack.c          | 344 +++++++++++++++++++
 sequencer.c          |   2 +-
 transport.h          |   5 +
 22 files changed, 1535 insertions(+), 1498 deletions(-)
 create mode 100644 fetch-pack.c
 create mode 100644 merge.c
 create mode 100644 send-pack.c

-- 
1.8.0.rc2.23.g1fb49df

^ permalink raw reply

* [PATCH 1/7] Move try_merge_command and checkout_fast_forward to libgit.a
From: Nguyễn Thái Ngọc Duy @ 2012-10-26 15:53 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Nguyễn Thái Ngọc Duy
In-Reply-To: <1351266835-22610-1-git-send-email-pclouds@gmail.com>

These functions are called in sequencer.c, which is part of
libgit.a. This makes libgit.a potentially require builtin/merge.c for
external git commands.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Makefile          |   1 +
 builtin/merge.c   | 106 +--------------------------------------------------
 cache.h           |  11 +++++-
 merge-recursive.h |   5 ---
 merge.c           | 112 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 sequencer.c       |   2 +-
 6 files changed, 125 insertions(+), 112 deletions(-)
 create mode 100644 merge.c

diff --git a/Makefile b/Makefile
index f69979e..2c333dc 100644
--- a/Makefile
+++ b/Makefile
@@ -762,6 +762,7 @@ LIB_OBJS += lockfile.o
 LIB_OBJS += log-tree.o
 LIB_OBJS += mailmap.o
 LIB_OBJS += match-trees.o
+LIB_OBJS += merge.o
 LIB_OBJS += merge-file.o
 LIB_OBJS += merge-recursive.o
 LIB_OBJS += mergesort.o
diff --git a/builtin/merge.c b/builtin/merge.c
index 0ec8f0d..a96e8ea 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -628,59 +628,6 @@ static void write_tree_trivial(unsigned char *sha1)
 		die(_("git write-tree failed to write a tree"));
 }
 
-static const char *merge_argument(struct commit *commit)
-{
-	if (commit)
-		return sha1_to_hex(commit->object.sha1);
-	else
-		return EMPTY_TREE_SHA1_HEX;
-}
-
-int try_merge_command(const char *strategy, size_t xopts_nr,
-		      const char **xopts, struct commit_list *common,
-		      const char *head_arg, struct commit_list *remotes)
-{
-	const char **args;
-	int i = 0, x = 0, ret;
-	struct commit_list *j;
-	struct strbuf buf = STRBUF_INIT;
-
-	args = xmalloc((4 + xopts_nr + commit_list_count(common) +
-			commit_list_count(remotes)) * sizeof(char *));
-	strbuf_addf(&buf, "merge-%s", strategy);
-	args[i++] = buf.buf;
-	for (x = 0; x < xopts_nr; x++) {
-		char *s = xmalloc(strlen(xopts[x])+2+1);
-		strcpy(s, "--");
-		strcpy(s+2, xopts[x]);
-		args[i++] = s;
-	}
-	for (j = common; j; j = j->next)
-		args[i++] = xstrdup(merge_argument(j->item));
-	args[i++] = "--";
-	args[i++] = head_arg;
-	for (j = remotes; j; j = j->next)
-		args[i++] = xstrdup(merge_argument(j->item));
-	args[i] = NULL;
-	ret = run_command_v_opt(args, RUN_GIT_CMD);
-	strbuf_release(&buf);
-	i = 1;
-	for (x = 0; x < xopts_nr; x++)
-		free((void *)args[i++]);
-	for (j = common; j; j = j->next)
-		free((void *)args[i++]);
-	i += 2;
-	for (j = remotes; j; j = j->next)
-		free((void *)args[i++]);
-	free(args);
-	discard_cache();
-	if (read_cache() < 0)
-		die(_("failed to read the cache"));
-	resolve_undo_clear();
-
-	return ret;
-}
-
 static int try_merge_strategy(const char *strategy, struct commit_list *common,
 			      struct commit_list *remoteheads,
 			      struct commit *head, const char *head_arg)
@@ -762,56 +709,6 @@ static int count_unmerged_entries(void)
 	return ret;
 }
 
-int checkout_fast_forward(const unsigned char *head, const unsigned char *remote)
-{
-	struct tree *trees[MAX_UNPACK_TREES];
-	struct unpack_trees_options opts;
-	struct tree_desc t[MAX_UNPACK_TREES];
-	int i, fd, nr_trees = 0;
-	struct dir_struct dir;
-	struct lock_file *lock_file = xcalloc(1, sizeof(struct lock_file));
-
-	refresh_cache(REFRESH_QUIET);
-
-	fd = hold_locked_index(lock_file, 1);
-
-	memset(&trees, 0, sizeof(trees));
-	memset(&opts, 0, sizeof(opts));
-	memset(&t, 0, sizeof(t));
-	if (overwrite_ignore) {
-		memset(&dir, 0, sizeof(dir));
-		dir.flags |= DIR_SHOW_IGNORED;
-		setup_standard_excludes(&dir);
-		opts.dir = &dir;
-	}
-
-	opts.head_idx = 1;
-	opts.src_index = &the_index;
-	opts.dst_index = &the_index;
-	opts.update = 1;
-	opts.verbose_update = 1;
-	opts.merge = 1;
-	opts.fn = twoway_merge;
-	setup_unpack_trees_porcelain(&opts, "merge");
-
-	trees[nr_trees] = parse_tree_indirect(head);
-	if (!trees[nr_trees++])
-		return -1;
-	trees[nr_trees] = parse_tree_indirect(remote);
-	if (!trees[nr_trees++])
-		return -1;
-	for (i = 0; i < nr_trees; i++) {
-		parse_tree(trees[i]);
-		init_tree_desc(t+i, trees[i]->buffer, trees[i]->size);
-	}
-	if (unpack_trees(nr_trees, t, &opts))
-		return -1;
-	if (write_cache(fd, active_cache, active_nr) ||
-		commit_locked_index(lock_file))
-		die(_("unable to write new index file"));
-	return 0;
-}
-
 static void split_merge_strategies(const char *string, struct strategy **list,
 				   int *nr, int *alloc)
 {
@@ -1424,7 +1321,8 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		}
 
 		if (checkout_fast_forward(head_commit->object.sha1,
-					  commit->object.sha1)) {
+					  commit->object.sha1,
+					  overwrite_ignore)) {
 			ret = 1;
 			goto done;
 		}
diff --git a/cache.h b/cache.h
index a58df84..2dc4dec 100644
--- a/cache.h
+++ b/cache.h
@@ -1265,8 +1265,15 @@ struct startup_info {
 };
 extern struct startup_info *startup_info;
 
-/* builtin/merge.c */
-int checkout_fast_forward(const unsigned char *from, const unsigned char *to);
+/* merge.c */
+struct commit_list;
+int try_merge_command(const char *strategy, size_t xopts_nr,
+		const char **xopts, struct commit_list *common,
+		const char *head_arg, struct commit_list *remotes);
+int checkout_fast_forward(const unsigned char *from,
+			  const unsigned char *to,
+			  int overwrite_ignore);
+
 
 int sane_execvp(const char *file, char *const argv[]);
 
diff --git a/merge-recursive.h b/merge-recursive.h
index 58f3435..9e090a3 100644
--- a/merge-recursive.h
+++ b/merge-recursive.h
@@ -59,9 +59,4 @@ struct tree *write_tree_from_memory(struct merge_options *o);
 
 int parse_merge_opt(struct merge_options *out, const char *s);
 
-/* builtin/merge.c */
-int try_merge_command(const char *strategy, size_t xopts_nr,
-		const char **xopts, struct commit_list *common,
-		const char *head_arg, struct commit_list *remotes);
-
 #endif
diff --git a/merge.c b/merge.c
new file mode 100644
index 0000000..70f1000
--- /dev/null
+++ b/merge.c
@@ -0,0 +1,112 @@
+#include "cache.h"
+#include "commit.h"
+#include "run-command.h"
+#include "resolve-undo.h"
+#include "tree-walk.h"
+#include "unpack-trees.h"
+#include "dir.h"
+
+static const char *merge_argument(struct commit *commit)
+{
+	if (commit)
+		return sha1_to_hex(commit->object.sha1);
+	else
+		return EMPTY_TREE_SHA1_HEX;
+}
+
+int try_merge_command(const char *strategy, size_t xopts_nr,
+		      const char **xopts, struct commit_list *common,
+		      const char *head_arg, struct commit_list *remotes)
+{
+	const char **args;
+	int i = 0, x = 0, ret;
+	struct commit_list *j;
+	struct strbuf buf = STRBUF_INIT;
+
+	args = xmalloc((4 + xopts_nr + commit_list_count(common) +
+			commit_list_count(remotes)) * sizeof(char *));
+	strbuf_addf(&buf, "merge-%s", strategy);
+	args[i++] = buf.buf;
+	for (x = 0; x < xopts_nr; x++) {
+		char *s = xmalloc(strlen(xopts[x])+2+1);
+		strcpy(s, "--");
+		strcpy(s+2, xopts[x]);
+		args[i++] = s;
+	}
+	for (j = common; j; j = j->next)
+		args[i++] = xstrdup(merge_argument(j->item));
+	args[i++] = "--";
+	args[i++] = head_arg;
+	for (j = remotes; j; j = j->next)
+		args[i++] = xstrdup(merge_argument(j->item));
+	args[i] = NULL;
+	ret = run_command_v_opt(args, RUN_GIT_CMD);
+	strbuf_release(&buf);
+	i = 1;
+	for (x = 0; x < xopts_nr; x++)
+		free((void *)args[i++]);
+	for (j = common; j; j = j->next)
+		free((void *)args[i++]);
+	i += 2;
+	for (j = remotes; j; j = j->next)
+		free((void *)args[i++]);
+	free(args);
+	discard_cache();
+	if (read_cache() < 0)
+		die(_("failed to read the cache"));
+	resolve_undo_clear();
+
+	return ret;
+}
+
+int checkout_fast_forward(const unsigned char *head,
+			  const unsigned char *remote,
+			  int overwrite_ignore)
+{
+	struct tree *trees[MAX_UNPACK_TREES];
+	struct unpack_trees_options opts;
+	struct tree_desc t[MAX_UNPACK_TREES];
+	int i, fd, nr_trees = 0;
+	struct dir_struct dir;
+	struct lock_file *lock_file = xcalloc(1, sizeof(struct lock_file));
+
+	refresh_cache(REFRESH_QUIET);
+
+	fd = hold_locked_index(lock_file, 1);
+
+	memset(&trees, 0, sizeof(trees));
+	memset(&opts, 0, sizeof(opts));
+	memset(&t, 0, sizeof(t));
+	if (overwrite_ignore) {
+		memset(&dir, 0, sizeof(dir));
+		dir.flags |= DIR_SHOW_IGNORED;
+		setup_standard_excludes(&dir);
+		opts.dir = &dir;
+	}
+
+	opts.head_idx = 1;
+	opts.src_index = &the_index;
+	opts.dst_index = &the_index;
+	opts.update = 1;
+	opts.verbose_update = 1;
+	opts.merge = 1;
+	opts.fn = twoway_merge;
+	setup_unpack_trees_porcelain(&opts, "merge");
+
+	trees[nr_trees] = parse_tree_indirect(head);
+	if (!trees[nr_trees++])
+		return -1;
+	trees[nr_trees] = parse_tree_indirect(remote);
+	if (!trees[nr_trees++])
+		return -1;
+	for (i = 0; i < nr_trees; i++) {
+		parse_tree(trees[i]);
+		init_tree_desc(t+i, trees[i]->buffer, trees[i]->size);
+	}
+	if (unpack_trees(nr_trees, t, &opts))
+		return -1;
+	if (write_cache(fd, active_cache, active_nr) ||
+		commit_locked_index(lock_file))
+		die(_("unable to write new index file"));
+	return 0;
+}
diff --git a/sequencer.c b/sequencer.c
index e3723d2..be0cb8b 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -191,7 +191,7 @@ static int fast_forward_to(const unsigned char *to, const unsigned char *from)
 	struct ref_lock *ref_lock;
 
 	read_cache();
-	if (checkout_fast_forward(from, to))
+	if (checkout_fast_forward(from, to, 1))
 		exit(1); /* the callee should have complained already */
 	ref_lock = lock_any_ref_for_update("HEAD", from, 0);
 	return write_ref_sha1(ref_lock, to, "cherry-pick");
-- 
1.8.0.rc2.23.g1fb49df

^ permalink raw reply related

* [PATCH 2/7] Move estimate_bisect_steps to libgit.a
From: Nguyễn Thái Ngọc Duy @ 2012-10-26 15:53 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Nguyễn Thái Ngọc Duy
In-Reply-To: <1351266835-22610-1-git-send-email-pclouds@gmail.com>

This function is used by bisect.c, part of libgit.a while
estimate_bisect_steps stays in builtin/rev-list.c. Move it to bisect.a
so we won't have undefine reference if a standalone program that uses
libgit.a happens to pull it in.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 bisect.c           | 38 ++++++++++++++++++++++++++++++++++++++
 builtin/rev-list.c | 39 ---------------------------------------
 2 files changed, 38 insertions(+), 39 deletions(-)

diff --git a/bisect.c b/bisect.c
index 1aad49b..bd1b7b5 100644
--- a/bisect.c
+++ b/bisect.c
@@ -956,3 +956,41 @@ int bisect_next_all(const char *prefix, int no_checkout)
 	return bisect_checkout(bisect_rev_hex, no_checkout);
 }
 
+static inline int log2i(int n)
+{
+	int log2 = 0;
+
+	for (; n > 1; n >>= 1)
+		log2++;
+
+	return log2;
+}
+
+static inline int exp2i(int n)
+{
+	return 1 << n;
+}
+
+/*
+ * Estimate the number of bisect steps left (after the current step)
+ *
+ * For any x between 0 included and 2^n excluded, the probability for
+ * n - 1 steps left looks like:
+ *
+ * P(2^n + x) == (2^n - x) / (2^n + x)
+ *
+ * and P(2^n + x) < 0.5 means 2^n < 3x
+ */
+int estimate_bisect_steps(int all)
+{
+	int n, x, e;
+
+	if (all < 3)
+		return 0;
+
+	n = log2i(all);
+	e = exp2i(n);
+	x = all - e;
+
+	return (e < 3 * x) ? n : n - 1;
+}
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index ff5a383..a0ec84e 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -201,45 +201,6 @@ static void show_edge(struct commit *commit)
 	printf("-%s\n", sha1_to_hex(commit->object.sha1));
 }
 
-static inline int log2i(int n)
-{
-	int log2 = 0;
-
-	for (; n > 1; n >>= 1)
-		log2++;
-
-	return log2;
-}
-
-static inline int exp2i(int n)
-{
-	return 1 << n;
-}
-
-/*
- * Estimate the number of bisect steps left (after the current step)
- *
- * For any x between 0 included and 2^n excluded, the probability for
- * n - 1 steps left looks like:
- *
- * P(2^n + x) == (2^n - x) / (2^n + x)
- *
- * and P(2^n + x) < 0.5 means 2^n < 3x
- */
-int estimate_bisect_steps(int all)
-{
-	int n, x, e;
-
-	if (all < 3)
-		return 0;
-
-	n = log2i(all);
-	e = exp2i(n);
-	x = all - e;
-
-	return (e < 3 * x) ? n : n - 1;
-}
-
 void print_commit_list(struct commit_list *list,
 		       const char *format_cur,
 		       const char *format_last)
-- 
1.8.0.rc2.23.g1fb49df

^ permalink raw reply related

* [PATCH 3/7] Move print_commit_list to libgit.a
From: Nguyễn Thái Ngọc Duy @ 2012-10-26 15:53 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Nguyễn Thái Ngọc Duy
In-Reply-To: <1351266835-22610-1-git-send-email-pclouds@gmail.com>

This is used by bisect.c, part of libgit.a while it stays in
builtin/rev-list.c. Move it to commit.c so that we won't get undefined
reference if a program that uses libgit.a happens to pull it in.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 bisect.h           |  4 ----
 builtin/rev-list.c | 10 ----------
 commit.c           | 10 ++++++++++
 commit.h           |  4 ++++
 4 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/bisect.h b/bisect.h
index ec3c3ff..2a6c831 100644
--- a/bisect.h
+++ b/bisect.h
@@ -11,10 +11,6 @@ extern struct commit_list *filter_skipped(struct commit_list *list,
 					  int *count,
 					  int *skipped_first);
 
-extern void print_commit_list(struct commit_list *list,
-			      const char *format_cur,
-			      const char *format_last);
-
 #define BISECT_SHOW_ALL		(1<<0)
 #define REV_LIST_QUIET		(1<<1)
 
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index a0ec84e..67701be 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -201,16 +201,6 @@ static void show_edge(struct commit *commit)
 	printf("-%s\n", sha1_to_hex(commit->object.sha1));
 }
 
-void print_commit_list(struct commit_list *list,
-		       const char *format_cur,
-		       const char *format_last)
-{
-	for ( ; list; list = list->next) {
-		const char *format = list->next ? format_cur : format_last;
-		printf(format, sha1_to_hex(list->item->object.sha1));
-	}
-}
-
 static void print_var_str(const char *var, const char *val)
 {
 	printf("%s='%s'\n", var, val);
diff --git a/commit.c b/commit.c
index 213bc98..e8eb0ae 100644
--- a/commit.c
+++ b/commit.c
@@ -1347,3 +1347,13 @@ struct commit_list **commit_list_append(struct commit *commit,
 	new->next = NULL;
 	return &new->next;
 }
+
+void print_commit_list(struct commit_list *list,
+		       const char *format_cur,
+		       const char *format_last)
+{
+	for ( ; list; list = list->next) {
+		const char *format = list->next ? format_cur : format_last;
+		printf(format, sha1_to_hex(list->item->object.sha1));
+	}
+}
diff --git a/commit.h b/commit.h
index 9f21313..c4cd046 100644
--- a/commit.h
+++ b/commit.h
@@ -222,4 +222,8 @@ struct commit *get_merge_parent(const char *name);
 
 extern int parse_signed_commit(const unsigned char *sha1,
 			       struct strbuf *message, struct strbuf *signature);
+extern void print_commit_list(struct commit_list *list,
+			      const char *format_cur,
+			      const char *format_last);
+
 #endif /* COMMIT_H */
-- 
1.8.0.rc2.23.g1fb49df

^ 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