Git development
 help / color / mirror / Atom feed
* Re: [PATCH 2/3] Make has_commit non-static
From: Junio C Hamano @ 2009-01-26 20:06 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Jake Goulding, git
In-Reply-To: <alpine.DEB.1.00.0901261637300.25749@intel-tinevez-2-302>

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

> On Mon, 26 Jan 2009, Jake Goulding wrote:
>
>> Moving has_commit from branch to a common location in preparation for 
>> using it in tag. Renaming it to commit_has_any_in_commit_list to be more 
>> unique.
>
> I feel like bike-shedding for a change, and I'd also like to prove that 
> not all Germans like long names:
>
> 	is_ancestor_of_any()
>
> Hmm?

Is it ancestor or descendant?  The latter makes the name longer, though
;-)

^ permalink raw reply

* Re: Using the "What's cooking" scripts outside of Git
From: Junio C Hamano @ 2009-01-26 20:15 UTC (permalink / raw)
  To: R. Tyler Ballance; +Cc: git
In-Reply-To: <1232997904.22352.4041.camel@starfruit>

"R. Tyler Ballance" <tyler@slide.com> writes:

> I was toying around with the idea of generating a "What's cooking" email
> similar to Junio's for our internal development, and I'm wondering if
> anybody has any experience using the scripts outside of the standard Git
> repository.
>
> Our repo is laid out slightly different, there's no "next" branch but
> there are a number of "in development" branches that get folded down
> into master.

UTSL, it's all open source ;-)

It is generated by the toolchain I check out in Meta/ directory from the
'todo' branch of git.git.  Of interest would be

 - git-topic.perl -- this lists the topics with their doneness.  It
   unfortunately heavily depends on my workflow to have one stable
   ('next') and one wilder ('pu') test integration branches, and
   convention to name the topics as ??/?*.

 - WC -- this produces and helps me maintain "What's cooking".  It calls
   git-topic.perl to generate the list, and optionally calls UWC to merge
   the new one with the previous issue.

 - UWC -- this reads the last issue of "What's cooking", and uses the
   output from git-topic.perl to update it.  It primarily is needed
   because the order topics are output from git-topic.perl is different
   from how I want to have them listed in "What's cooking", and reads the
   last issue to classify each topics into the same category as they
   appeared in it.

 - WI -- this gives "What's in" and should be straightforward.

^ permalink raw reply

* Re: backwards compatibility, was Re: [PATCH v1 1/3] Introduce config  variable "diff.primer"
From: Jay Soffian @ 2009-01-26 20:32 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, Johannes Schindelin, Keith Cascio, git
In-Reply-To: <7vd4e96dh7.fsf@gitster.siamese.dyndns.org>

On Mon, Jan 26, 2009 at 3:04 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Where do all of these nonsense come from?  We are not adding any mechanism
> for scripts to say they need plumbing context.  By calling plumbing they
> are already asking for stable plumbing behaviour.

The suggestion was wrt to commands which are not strictly plumbing.

j.

^ permalink raw reply

* Re: backwards compatibility, was Re: [PATCH v1 1/3] Introduce config variable "diff.primer"
From: Jeff King @ 2009-01-26 20:35 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jay Soffian, Johannes Schindelin, Keith Cascio, git
In-Reply-To: <7vd4e96dh7.fsf@gitster.siamese.dyndns.org>

On Mon, Jan 26, 2009 at 12:04:20PM -0800, Junio C Hamano wrote:

> > Well, this issue seems to come up every so often, so the idea would be:
> >
> > - We're adding a mechanism for scripts to communicate that they need
> > plumbing context
> > - Start using it in your scripts when calling git if you rely on a
> > stable interface
> > - In the next major release, git may introduce changes to commands
> > which are not clearly plumbing if you haven't adopted the mechanism
> 
> Where do all of these nonsense come from?  We are not adding any mechanism
> for scripts to say they need plumbing context.  By calling plumbing they
> are already asking for stable plumbing behaviour.

I think this is my fault a little for mentioning "blurred plumbing and
porcelain" and not explaining further. This really has nothing to do
with actual plumbing commands. Scripts should use diff-tree and not
diff, and that has always and probably will always be the case.

But what about something like "git grep"? Is it plumbing or porcelain?
The functionality isn't exposed in any other way, so I can imagine that
some scripts are using it. But it's sad to think that we could never
have config that might change its behavior, because it is used directly
by users all the time.  I think the same applies for "git archive".
There may be others.

So something like Jay's proposal could future-proof those commands
better by allowing scripts to say "BTW, I am a script. Turn off your new
features." And there are two classes of alternatives:

  - as you described, instead of making scripts turn _off_ features,
    make them turn them _on_, effectively declaring these commands as
    plumbing. This is obviously much nicer because it Just Works with
    current scripts. But it means that these mixed porcelain/plumbing
    commands suffer in their porcelain capacity; we can never add a
    config option that might change the behavior without the user
    specifying "it's ok to use this feature" at each invocation.

  - we can provide support _now_ for splitting the functionality into
    porcelain and plumbing, scripts can adapt over time to using the
    plumbing version, and then eventually we can declare it safe to make
    changes to the porcelain. And that is more or less what Jay's
    proposal is doing.

    However, I don't think a command-line option is the best way to say
    "I am a script". It's too easy to type "git grep" in a script and
    "git grep -J" (either because you are clueless about "-J", or
    because you simply forget). Other signal methods include:

      - just making two different commands to expose the same
        functionality, one plumbing and one porcelain. This is what has
        evolved in other areas, such as diff (though note that there
        _isn't_ exactly a "git diff" plumbing command -- there is the
        plumbing that "git diff is based on). I'm not sure what the
        plumbing name for "git grep" would be.

      - set an environment variable like GIT_STRICT. It's easy to set
        once at the top of your script, and it trickles down
        automatically as we call other git commands and scripts.
        We could even set it in git-sh-setup, though that of course
        covers only shell scripts, and not other callers.

> The scripts can, if they want to, use newer options updated versions of
> the plumbing commands offer, by passing them when they want to.
> 
> And the trigger to do so is up to the scripts.  They can get new options
> from the end user, or they can peek into user's configuration variables
> similar to the diff.primer mentioned earlier in the discussion.

Right, I think that is absolutely the right thing for commands which are
clearly plumbing.

> One way could be a new option --screw-me-with=name that can be given to a
> plumbing command and tells it pretend as if the command line options
> specified by the configuration variable of the given name were given
> (e.g. a script runs "git diff-files --screw-me-with=diff.primer").
> 
> The important point is that it has to be opt _IN_.

Our precedent so far has been to just add a new command line option that
enables the feature (e.g., --ext-diff and --textconv). Functionally it
is no different than --screw-me-with=diff.*.textconv. :)

-Peff

^ permalink raw reply

* Re: [PATCH 2/3] Make has_commit non-static
From: Johannes Schindelin @ 2009-01-26 20:38 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jake Goulding, git
In-Reply-To: <7v63k16dd0.fsf@gitster.siamese.dyndns.org>

Hi,

On Mon, 26 Jan 2009, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > On Mon, 26 Jan 2009, Jake Goulding wrote:
> >
> >> Moving has_commit from branch to a common location in preparation for 
> >> using it in tag. Renaming it to commit_has_any_in_commit_list to be 
> >> more unique.
> >
> > I feel like bike-shedding for a change, and I'd also like to prove 
> > that not all Germans like long names:
> >
> > 	is_ancestor_of_any()
> >
> > Hmm?
> 
> Is it ancestor or descendant?  The latter makes the name longer, though 
> ;-)

I did not read the patch and missed that it means the opposite, sorry...

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] Allow format-patch to create patches for merges
From: Jeff King @ 2009-01-26 20:45 UTC (permalink / raw)
  To: Nathan W. Panike; +Cc: Johannes Schindelin, git
In-Reply-To: <d77df1110901260827j2200fe41oe1b84c387d88aba@mail.gmail.com>

On Mon, Jan 26, 2009 at 10:27:18AM -0600, Nathan W. Panike wrote:

> I think I have an unusual workflow where my patch makes sense,
> although it probably does not for the vast majority of git users.  I
> regularly use 3 machines: S, L, and H.  I keep my work synchronized by
> using git.  Normally, I fetch from S to L or to H, depending on which
> machine I am working on at the moment.  I also push from L or H to S.
> I sporadically lose connectivity to S, so I have a hook in the repo on
> S to send a backup email to me on mail server M, which has a more
> reliable connection.  This email also serves as a  reminder when I

Have you considered sending a bundle instead of a patch in the backup
email? That is the more exact equivalent of a push (i.e., it preserves
your actual commits, sha1 and all).

-Peff

^ permalink raw reply

* Re: [PATCH] rebase -i: correctly remember --root flag across --continue
From: Junio C Hamano @ 2009-01-26 20:47 UTC (permalink / raw)
  To: Thomas Rast; +Cc: git, Johannes Schindelin
In-Reply-To: <1232960722-17480-1-git-send-email-trast@student.ethz.ch>

Thomas Rast <trast@student.ethz.ch> writes:

> From: Junio C Hamano <gitster@pobox.com>
>
> d911d14 (rebase -i: learn to rebase root commit, 2009-01-02) tried to
> remember the --root flag across a merge conflict in a broken way.
> Introduce a flag file $DOTEST/rebase-root to fix and clarify.
>
> While at it, also make sure $UPSTREAM is always initialized to guard
> against existing values in the environment.
>
> [tr: added tests]
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> Signed-off-by: Thomas Rast <trast@student.ethz.ch>
> ---
>
> Junio C Hamano wrote:
>> Since you never use the value stored in "$DOTEST/upstream" for anything
>> else anyway, how about doing something like this instead?  It would make
>> the meaning of the file used as a state variable much clearer.
>
> Yes, thanks, a patch precisely "like this" is in fact the right fix.
>
> I came up with some tests that try a conflicted --root rebase of each
> flavour, to guard against the problem in the future.  I wasn't
> entirely sure how to shape this into a patch, but here's a version
> that forges patch message and sign-off in your name.
>
> Dscho, with that confusion cleared, you can add my Ack to your 1/2
> (unchanged, though I'm afraid you'll get a textual conflict).

Ok, so I'll queue this to 'master' as a bugfix.

Thanks.

^ permalink raw reply

* Re: Common ancestor in merge diffs?
From: Linus Torvalds @ 2009-01-26 20:54 UTC (permalink / raw)
  To: Daniel Barkalow; +Cc: git
In-Reply-To: <alpine.LNX.1.00.0901261412110.19665@iabervon.org>



On Mon, 26 Jan 2009, Daniel Barkalow wrote:
> 
> That is really nice, and quite handy. I ended up getting approximately 
> that effect with blame and show, but gitk is much easier. For some reason, 
> I never think of the graphical tools.

You can do the same thing on the command-line with just doing

	git log --merge [filename]

but when doing conflict resolution you often do care (deeply) about the 
way the commits hang together, so the graphical tools really do end up 
giving you fundamentally interesting information that is hard to show in 
any readable way with just plain text.

[ Yeah, there's "--graph", but it really loses out on the whole 
  readability thing ]

> Is there an easy way of focusing on the changes that end up in a 
> particular conflict? Half of the work was finding the right commit and 
> finding the right part of the diff.

This is where the command line tools really help. What I do is to just 
fire up that "gitk" and 90% of the time the thing is clear from that. But 
_if_ it then is complex, and has a ton of changes to the same file that 
really aren't that interesting, doing a 

	git log --merge -S'interesting-part-goes-here' [filename]

in another window tends to be a good idea. That way you get the commits 
that just touch that string, and you can do the whole 'goto-SHA1' in the 
gitk window to see them there.

Sometimes, "git blame -C " is also a good tool to have around. You seem to 
have found it already. One usage case is

	git blame -C $(git merge-base HEAD MERGE_HEAD).. filename

which then high-lights the lines changed from the merge-base to the HEAD 
(change it to "..MERGE_HEAD" to see what changed to MERGE_HEAD), but quite 
frankly, I tend to use "git blame" more as a tool _after_ the merge, when 
I noticed that I merged things wrong and go back and try to figure out 
what caused the problem.

		Linus

^ permalink raw reply

* Re: Translations [of Documentation] in Git release?
From: Junio C Hamano @ 2009-01-26 20:38 UTC (permalink / raw)
  To: Dill; +Cc: Jakub Narebski, Peter Krefting, Git Mailing List, yasuaki_n
In-Reply-To: <60646ee10901261158w65b539dida26d2bd3bae6903@mail.gmail.com>

Dill <sarpulhu@gmail.com> writes:

> On 1/26/09, Jakub Narebski <jnareb@gmail.com> wrote:
>> On Mon, 26 Jan 2009, Peter Krefting wrote:
>>> Jakub Narebski wrote:
>>>
>>> > With GUI translations we just use gettext conventions. I don't know
>>> > any such convention for docs:
>>>
>>> There is a lot of documentation being translated using PO files. po4a -
>>> http://po4a.alioth.debian.org/ - is a nice starting point for that.
>>
>> I'm not sure if XLIFF wouldn't be better format to use to translate
>> _documents_.  Gettext was meant to translate, I think, not very long
>> messages in programs.
>>
>> Also I am not sure how much support this idea has. True, in last Git
>> User's Survey[1] 63% to 76% wanted (parts of) Documentation... but that
>> was out of 325 people who answered this question, with 3236 responses
>> to survey in total, so numbers are more like 6% - 8%.
>>
>> [1] http://git.or.cz/gitwiki/GitSurvey2008
>> [2] http://translate.sourceforge.net/wiki/
>> ...
> I was thinking of handling it like the Linux kernel documentation...?

By this, I understand you mean the model that lets the authors of the
original English documentation be unaware of the presense of translations,
and resulting translated files are placed in Documentation/??_??/ (where
"??_??" are ja_JP, zh_CN, etc.) subdirectory.

The approach obviously risks the translations to go stale very easily, but
gives a nice separation of reponsibility and does not slow down the way
the original documents are updated.  I would actually prefer a directory
structure "Documentation/translated/??_??/" so that people who are not
involved in the translation do not have to see anything below _one_
directory (i.e. "translated").

If you step in as the Documentation translation coordinator to maintain
such a tree structure that I can have as a submodule (or subtree merge) to
git.git tree, you could talk me into updating my tree from time to time
from your tree, but at that point we might actually want to have such a
translation project as a separate and unrelated project.

By the way,

    http://github.com/yasuaki/git-manual-jp.git/

has some Japanese translations (no, I am not involved in this any way, and
I do not know about its current status).

If you look at files in Documentation/ (not Documentation.ja) in that
repository, you can see how they tried to make it easier to update the
translation to match the original documentation set when the original gets
updated.  I do not know how well the approach works in practice, though.

^ permalink raw reply

* Re: git 1.6.1 on AIX 5.3
From: Jeff King @ 2009-01-26 21:00 UTC (permalink / raw)
  To: Perry Smith; +Cc: Mike Ralphson, git
In-Reply-To: <A8D76E61-4442-4640-BD0C-84085375E6F1@gmail.com>

[cc-ing Mike Ralphson, our local AIX expert]

On Mon, Jan 26, 2009 at 02:02:15PM -0600, Perry Smith wrote:

> I tried building git 1.6.1 on AIX 5.3 as an "out of tree" build and it  
> does not seem to be set up to do out of tree builds.  If that is not  
> true, please let me know.
>
> The install process wants to call install with a -d option.  AIX has two 
> install programs but they are pretty old -- neither takes a -d option.
>
> Is there a GNU install program I can get?  I've not been able to locate 
> one.

It's in GNU coreutils:

  http://www.gnu.org/software/coreutils/

I don't know what Mike uses to install on AIX; you can see his config
setup here:

  http://repo.or.cz/w/git/gitbuild.git?a=tree;f=mr/aix;hb=platform

but I don't see any override of install.

> Last -- just so I know for future reference, is this list a 'text only  
> email' list?

If you mean "no html", then yes, it is absolutely text only.

-Peff

^ permalink raw reply

* Re: [PATCH] rebase -i: correctly remember --root flag across --continue
From: Junio C Hamano @ 2009-01-26 21:05 UTC (permalink / raw)
  To: Thomas Rast; +Cc: git, Johannes Schindelin
In-Reply-To: <1232960722-17480-1-git-send-email-trast@student.ethz.ch>

Thomas Rast <trast@student.ethz.ch> writes:

> +test_expect_success 'rebase -i --root with conflict (first part)' '
> +	git checkout -b conflict2 other &&
> +	GIT_EDITOR=: test_must_fail git rebase -i --root --onto master &&
> +	git ls-files -u | grep "B$"
> +'

Maybe I am misrecalling things but didn't we have reports from people on
some platforms that single-shot exporting of the environment like this one
does not work for them?

> +test_expect_success 'fix the conflict' '
> +	echo 3 > B &&
> +	git add B
> +'
> +
> +test_expect_success 'rebase -i --root with conflict (second part)' '
> +	git rebase --continue &&
> +	git log --pretty=tformat:"%s" > conflict2 &&
> +	test_cmp expect-conflict conflict2
> +'
> +
> +sed 's/#/ /g' > expect-conflict-p <<'EOF'
> +*   Merge branch 'third' into other
> +|\##
> +| * 6
> +* |   Merge branch 'side' into other
> +|\ \##
> +| * | 5
> +* | | 4
> +|/ /##
> +* | 3
> +|/##
> +* conflict
> +* 2
> +* 1
> +EOF

I do not like this very much.  Future improvements of the graph drawing
algorithm (one obvious "flaw" you are exposing by the above is that it has
trailing whitespaces that can be trimmed, and somebody else may be
inclined to fix) would break the expectation this test vector has.

Do you have to compare the topology this way, or are there other more
reliable ways?

^ permalink raw reply

* Re: [PATCH] rebase -i: correctly remember --root flag across --continue
From: Jeff King @ 2009-01-26 21:09 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Thomas Rast, git, Johannes Schindelin
In-Reply-To: <7veiyp4w2m.fsf@gitster.siamese.dyndns.org>

On Mon, Jan 26, 2009 at 01:05:37PM -0800, Junio C Hamano wrote:

> > +test_expect_success 'rebase -i --root with conflict (first part)' '
> > +	git checkout -b conflict2 other &&
> > +	GIT_EDITOR=: test_must_fail git rebase -i --root --onto master &&
> > +	git ls-files -u | grep "B$"
> > +'
> 
> Maybe I am misrecalling things but didn't we have reports from people on
> some platforms that single-shot exporting of the environment like this one
> does not work for them?

I don't think you are misrecalling. The problem is with one-shot
variables and functional calls. See 09b78bc1.

-Peff

^ permalink raw reply

* Re: rerere: how to remove an erroneous resolution?
From: Jeff King @ 2009-01-26 21:11 UTC (permalink / raw)
  To: SZEDER Gábor; +Cc: git
In-Reply-To: <20090126151326.GA2618@neumann>

On Mon, Jan 26, 2009 at 04:13:26PM +0100, SZEDER Gábor wrote:

> On Mon, Jan 26, 2009 at 03:42:39PM +0100, SZEDER Gábor wrote:
> > some time ago I mistakenly resolved a merge conflict incorrectly.  Of
> > course, rerere noted the erroneous conflict resolution, and whenever
> > the same merge conflict occurs rerere offers me that erroneous
> > conflict resolution, even though I correct it each time.
> > 
> > So, the question is how could I make rerere forget that particular
> > merge conflict resolution?  Is it at all possible?
> 
> Ok, I should have investigated a little longer before sending that
> email.

Hmm. I have never actually used rerere in a real setting, so I am
somewhat clueless. But from your description, it sounds like it would
make sense for it to update the resolution to the latest one used. If
you changed it, it was probably to correct it; you would have no need to
make a change to break an already-working resolution.

Then you would not have had to go mucking about in .git.

-Peff

^ permalink raw reply

* Re: [PATCH] rebase -i: correctly remember --root flag across --continue
From: Jeff King @ 2009-01-26 21:12 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Thomas Rast, git, Johannes Schindelin
In-Reply-To: <20090126210942.GH27604@coredump.intra.peff.net>

On Mon, Jan 26, 2009 at 04:09:42PM -0500, Jeff King wrote:

> > Maybe I am misrecalling things but didn't we have reports from people on
> > some platforms that single-shot exporting of the environment like this one
> > does not work for them?
> 
> I don't think you are misrecalling. The problem is with one-shot
> variables and functional calls. See 09b78bc1.

Err, _function_ calls. Need sleep badly.

-Peff

^ permalink raw reply

* [PATCH/RFC v1 0/6] git checkout: more cleanups, optimisation, less lstat() calls
From: Kjetil Barvik @ 2009-01-26 21:17 UTC (permalink / raw)
  To: git; +Cc: Kjetil Barvik

Here is 6 small patches which should further improve the time it take
to do 'git checkout'.  After the 6 patches, the numbers from the 'git
checkout -q my-v2.6.27' test, should now look like this:

TOTAL        136752 100.000% OK:124567 NOT: 12185   8.641698 sec   63 usec/call
lstat64       55502  40.586% OK: 49122 NOT:  6380   2.463762 sec   44 usec/call
    strings   55502 tot  30163 uniq   1.840 /uniq   2.463762 sec   44 usec/call
    files     47122 tot  23813 uniq   1.979 /uniq   2.070486 sec   44 usec/call
    dirs       2000 tot   1436 uniq   1.393 /uniq   0.087281 sec   44 usec/call
    errors     6380 tot   5187 uniq   1.230 /uniq   0.305995 sec   48 usec/call
                  4   0.007% OK:     3 NOT:     1  "arch/sh/boards"
                  3   0.005% OK:     3 NOT:     0  ".git/HEAD"
                  3   0.005% OK:     3 NOT:     0  ".git/refs/heads/my-v2.6.27"
                  3   0.005% OK:     3 NOT:     0  ".gitignore"
                  3   0.005% OK:     3 NOT:     0  ".mailmap"
                  3   0.005% OK:     3 NOT:     0  "CREDITS"
                  3   0.005% OK:     3 NOT:     0  "Documentation"
                  3   0.005% OK:     3 NOT:     0  "Documentation/00-INDEX"
                  3   0.005% OK:     3 NOT:     0  "Documentation/ABI/testing/sysfs-block"
                  3   0.005% OK:     3 NOT:     0  "Documentation/ABI/testing/sysfs-firmware-acpi"
                  3   0.005% OK:     3 NOT:     0  "Documentation/CodingStyle"
                  3   0.005% OK:     3 NOT:     0  "Documentation/DMA-API.txt"
                  3   0.005% OK:     3 NOT:     0  "Documentation/DMA-mapping.txt"
                  3   0.005% OK:     3 NOT:     0  "Documentation/DocBook/Makefile"
                  3   0.005% OK:     3 NOT:     0  "Documentation/DocBook/gadget.tmpl"
                  3   0.005% OK:     3 NOT:     0  "Documentation/DocBook/kernel-api.tmpl"
                  3   0.005% OK:     3 NOT:     0  "Documentation/DocBook/kernel-locking.tmpl"
                  3   0.005% OK:     3 NOT:     0  "Documentation/DocBook/procfs-guide.tmpl"
                  3   0.005% OK:     3 NOT:     0  "Documentation/DocBook/procfs_example.c"
                  3   0.005% OK:     3 NOT:     0  "Documentation/DocBook/rapidio.tmpl"
<snipp>
fstat64       14403  10.532% OK: 14403 NOT:     0   0.179938 sec   12 usec/call

So, since last time, and because of patch 4/6, almost 14 400 of the
lstat() calls has now become fstat() calls, and it seems we have saved
(* 14403 (- 44 12)) = 460 896 microseconds system time because of
this.  I am planing to do a more complete and long-running
/usr/bin/time test, to get real numbers.

With both patch-series, the count of lstat() calls for this particular
test have dropped from 120 954 to 55 502, which is a total reduction
of 65 452 calls or 54%.

Please note that patch 6/6 is only to be a debug patch, to catch a
possible ping-pong situation inside the lstat_cache(), so I think that
it should at _most_ be merged to the pu branch, such that people who
wish to test, can 'git cherry-pick' the patch from there.


Kjetil Barvik (6):
  symlinks.c: small cleanup and optimisation
  remove some memcpy() and strchr() calls inside create_directories()
  cleanup of write_entry() in entry.c
  use fstat() instead of lstat() when we have an opened file
  combine-diff.c: remove a call to fstat() inside show_patch_diff()
  lstat_cache(): print a warning if doing ping-pong between cache types

 combine-diff.c |    5 +-
 entry.c        |  144 ++++++++++++++++++++++++++++++--------------------------
 symlinks.c     |   48 ++++++++++++++-----
 3 files changed, 115 insertions(+), 82 deletions(-)

^ permalink raw reply

* [PATCH/RFC v1 1/6] symlinks.c: small cleanup and optimisation
From: Kjetil Barvik @ 2009-01-26 21:17 UTC (permalink / raw)
  To: git; +Cc: Kjetil Barvik
In-Reply-To: <1233004637-15112-1-git-send-email-barvik@broadpark.no>

Simplify the if-else test in longest_match_lstat_cache() such that we
only have one simple if test.  Instead of testing for 'i == cache.len'
or 'i == len', we transform this to a common test for 'i == max_len'.

And to further optimise we use 'i >= max_len' instead of 'i ==
max_len', the reason is that it is now the exact opposite of one part
inside the while-loop termination expression 'i < max_len && name[i]
== cache.path[i]', and then the compiler can hopefully/probably reuse
a test-result from it.

We do similar transformations inside the lstat_cache() and the
invalidate_lstat_cache() functions.

The result is a little smaller text-size as shown below:

 ~/git/git $ size symlinks_??_*
   text	   data	    bss	    dec	    hex	filename
   1282       0	   4116	   5398	   1516	symlinks_O2_after.o
   1378       0	   4116	   5494	   1576	symlinks_O2_before.o
    896       0	   4116	   5012	   1394	symlinks_Os_after.o
    902       0	   4116	   5018	   139a	symlinks_Os_before.o

'O2' means that the file is compiled with 'gcc -O2', and similar 'Os'
means that the file is compiled with 'gcc -Os' (gcc 4.3.2).

Signed-off-by: Kjetil Barvik <barvik@broadpark.no>
---
 symlinks.c |   25 +++++++++++++------------
 1 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/symlinks.c b/symlinks.c
index f262b7c..81f490c 100644
--- a/symlinks.c
+++ b/symlinks.c
@@ -25,15 +25,16 @@ static inline int longest_match_lstat_cache(int len, const char *name,
 		}
 		i++;
 	}
-	/* Is the cached path string a substring of 'name'? */
-	if (i == cache.len && cache.len < len && name[cache.len] == '/') {
-		match_len_prev = match_len;
-		match_len = cache.len;
-	/* Is 'name' a substring of the cached path string? */
-	} else if ((i == len && len < cache.len && cache.path[len] == '/') ||
-		   (i == len && len == cache.len)) {
+	/*
+	 * Is the cached path string a substring of 'name', is 'name'
+	 * a substring of the cached path string, or is 'name' and the
+	 * cached path string the exact same string?
+	 */
+	if (i >= max_len && ((i < len && name[i] == '/') ||
+			     (i < cache.len && cache.path[i] == '/') ||
+			     (len == cache.len))) {
 		match_len_prev = match_len;
-		match_len = len;
+		match_len = i;
 	}
 	*previous_slash = match_len_prev;
 	return match_len;
@@ -91,7 +92,7 @@ static int lstat_cache(int len, const char *name,
 		match_len = last_slash =
 			longest_match_lstat_cache(len, name, &previous_slash);
 		match_flags = cache.flags & track_flags & (FL_NOENT|FL_SYMLINK);
-		if (match_flags && match_len == cache.len)
+		if (match_flags && match_len >= cache.len)
 			return match_flags;
 		/*
 		 * If we now have match_len > 0, we would know that
@@ -102,7 +103,7 @@ static int lstat_cache(int len, const char *name,
 		 * we can return immediately.
 		 */
 		match_flags = track_flags & FL_DIR;
-		if (match_flags && len == match_len)
+		if (match_flags && match_len >= len)
 			return match_flags;
 	}
 
@@ -153,7 +154,7 @@ static int lstat_cache(int len, const char *name,
 		cache.path[last_slash] = '\0';
 		cache.len = last_slash;
 		cache.flags = save_flags;
-	} else if (track_flags & FL_DIR &&
+	} else if ((track_flags & FL_DIR) &&
 		   last_slash_dir > 0 && last_slash_dir <= PATH_MAX) {
 		/*
 		 * We have a separate test for the directory case,
@@ -184,7 +185,7 @@ void invalidate_lstat_cache(int len, const char *name)
 	int match_len, previous_slash;
 
 	match_len = longest_match_lstat_cache(len, name, &previous_slash);
-	if (len == match_len) {
+	if (match_len >= len) {
 		if ((cache.track_flags & FL_DIR) && previous_slash > 0) {
 			cache.path[previous_slash] = '\0';
 			cache.len = previous_slash;
-- 
1.6.1.349.g99fa5

^ permalink raw reply related

* [PATCH/RFC v1 2/6] remove some memcpy() and strchr() calls inside create_directories()
From: Kjetil Barvik @ 2009-01-26 21:17 UTC (permalink / raw)
  To: git; +Cc: Kjetil Barvik
In-Reply-To: <1233004637-15112-1-git-send-email-barvik@broadpark.no>

Remove the call to memcpy() and strchr() for each path component
tested, and instead add each path component as we go forward inside
the while-loop.

Impact: small optimisation

Signed-off-by: Kjetil Barvik <barvik@broadpark.no>
---

OK, maybe I instead should have tried to merge the function
create_directories() with the safe_create_leading_directories() and
*_const() functions?  What do pepople think?


 entry.c |   23 ++++++++++++++---------
 1 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/entry.c b/entry.c
index 05aa58d..c2404ea 100644
--- a/entry.c
+++ b/entry.c
@@ -2,15 +2,19 @@
 #include "blob.h"
 #include "dir.h"
 
-static void create_directories(const char *path, const struct checkout *state)
+static void
+create_directories(int path_len, const char *path, const struct checkout *state)
 {
-	int len = strlen(path);
-	char *buf = xmalloc(len + 1);
-	const char *slash = path;
-
-	while ((slash = strchr(slash+1, '/')) != NULL) {
-		len = slash - path;
-		memcpy(buf, path, len);
+	char *buf = xmalloc(path_len + 1);
+	int len = 0;
+
+	while (len < path_len) {
+		do {
+			buf[len] = path[len];
+			len++;
+		} while (len < path_len && path[len] != '/');
+		if (len >= path_len)
+			break;
 		buf[len] = 0;
 
 		/*
@@ -190,6 +194,7 @@ int checkout_entry(struct cache_entry *ce, const struct checkout *state, char *t
 
 	memcpy(path, state->base_dir, len);
 	strcpy(path + len, ce->name);
+	len += ce_namelen(ce);
 
 	if (!lstat(path, &st)) {
 		unsigned changed = ce_match_stat(ce, &st, CE_MATCH_IGNORE_VALID);
@@ -218,6 +223,6 @@ int checkout_entry(struct cache_entry *ce, const struct checkout *state, char *t
 			return error("unable to unlink old '%s' (%s)", path, strerror(errno));
 	} else if (state->not_new)
 		return 0;
-	create_directories(path, state);
+	create_directories(len, path, state);
 	return write_entry(ce, path, state, 0);
 }
-- 
1.6.1.349.g99fa5

^ permalink raw reply related

* [PATCH/RFC v1 3/6] cleanup of write_entry() in entry.c
From: Kjetil Barvik @ 2009-01-26 21:17 UTC (permalink / raw)
  To: git; +Cc: Kjetil Barvik
In-Reply-To: <1233004637-15112-1-git-send-email-barvik@broadpark.no>

The switch-cases for S_IFREG and S_IFLNK was so similar that it will
be better to do some cleanup and use the common parts of it.

Also fold the longest lines such that no line is longer then 80 chars
or so.

And the entry.c file should now be clean for 'gcc -Wextra' warnings.

Signed-off-by: Kjetil Barvik <barvik@broadpark.no>
---

If people do not like this approach I can be willing to drop it from
this patch-series, but then I get some source code duplication from
the next patch (4/6).


 entry.c |  114 +++++++++++++++++++++++++++++++-------------------------------
 1 files changed, 57 insertions(+), 57 deletions(-)

diff --git a/entry.c b/entry.c
index c2404ea..8543755 100644
--- a/entry.c
+++ b/entry.c
@@ -78,7 +78,7 @@ static int create_file(const char *path, unsigned int mode)
 	return open(path, O_WRONLY | O_CREAT | O_EXCL, mode);
 }
 
-static void *read_blob_entry(struct cache_entry *ce, const char *path, unsigned long *size)
+static void *read_blob_entry(struct cache_entry *ce, unsigned long *size)
 {
 	enum object_type type;
 	void *new = read_sha1_file(ce->sha1, &type, size);
@@ -91,88 +91,86 @@ static void *read_blob_entry(struct cache_entry *ce, const char *path, unsigned
 	return NULL;
 }
 
-static int write_entry(struct cache_entry *ce, char *path, const struct checkout *state, int to_tempfile)
+static int
+write_entry(struct cache_entry *ce, char *path, const struct checkout *state,
+	    int to_tempfile)
 {
-	int fd;
-	long wrote;
-
-	switch (ce->ce_mode & S_IFMT) {
-		char *new;
-		struct strbuf buf;
-		unsigned long size;
-
+	unsigned int ce_mode_s_ifmt = ce->ce_mode & S_IFMT;
+	int fd, ret;
+	char *new;
+	struct strbuf buf = STRBUF_INIT;
+	unsigned long size;
+	size_t wrote, newsize = 0;
+
+	switch (ce_mode_s_ifmt) {
 	case S_IFREG:
-		new = read_blob_entry(ce, path, &size);
+	case S_IFLNK:
+		new = read_blob_entry(ce, &size);
 		if (!new)
-			return error("git checkout-index: unable to read sha1 file of %s (%s)",
-				path, sha1_to_hex(ce->sha1));
+			return error("git checkout-index: "\
+				     "unable to read sha1 file of %s (%s)",
+				     path, sha1_to_hex(ce->sha1));
+
+		if (ce_mode_s_ifmt == S_IFLNK && has_symlinks && !to_tempfile) {
+			ret = symlink(new, path);
+			free(new);
+			if (ret)
+				return error("git checkout-index: "\
+					     "unable to create symlink %s (%s)",
+					     path, strerror(errno));
+			break;
+		}
 
 		/*
 		 * Convert from git internal format to working tree format
 		 */
-		strbuf_init(&buf, 0);
-		if (convert_to_working_tree(ce->name, new, size, &buf)) {
-			size_t newsize = 0;
+		if (ce_mode_s_ifmt == S_IFREG &&
+		    convert_to_working_tree(ce->name, new, size, &buf)) {
 			free(new);
 			new = strbuf_detach(&buf, &newsize);
 			size = newsize;
 		}
 
 		if (to_tempfile) {
-			strcpy(path, ".merge_file_XXXXXX");
+			if (ce_mode_s_ifmt == S_IFREG)
+				strcpy(path, ".merge_file_XXXXXX");
+			else
+				strcpy(path, ".merge_link_XXXXXX");
 			fd = mkstemp(path);
-		} else
-			fd = create_file(path, ce->ce_mode);
+		} else {
+			if (ce_mode_s_ifmt == S_IFREG)
+				fd = create_file(path, ce->ce_mode);
+			else
+				fd = create_file(path, 0666);
+		}
 		if (fd < 0) {
 			free(new);
-			return error("git checkout-index: unable to create file %s (%s)",
-				path, strerror(errno));
+			return error("git checkout-index: "\
+				     "unable to create file %s (%s)",
+				     path, strerror(errno));
 		}
 
 		wrote = write_in_full(fd, new, size);
 		close(fd);
 		free(new);
 		if (wrote != size)
-			return error("git checkout-index: unable to write file %s", path);
-		break;
-	case S_IFLNK:
-		new = read_blob_entry(ce, path, &size);
-		if (!new)
-			return error("git checkout-index: unable to read sha1 file of %s (%s)",
-				path, sha1_to_hex(ce->sha1));
-		if (to_tempfile || !has_symlinks) {
-			if (to_tempfile) {
-				strcpy(path, ".merge_link_XXXXXX");
-				fd = mkstemp(path);
-			} else
-				fd = create_file(path, 0666);
-			if (fd < 0) {
-				free(new);
-				return error("git checkout-index: unable to create "
-						 "file %s (%s)", path, strerror(errno));
-			}
-			wrote = write_in_full(fd, new, size);
-			close(fd);
-			free(new);
-			if (wrote != size)
-				return error("git checkout-index: unable to write file %s",
-					path);
-		} else {
-			wrote = symlink(new, path);
-			free(new);
-			if (wrote)
-				return error("git checkout-index: unable to create "
-						 "symlink %s (%s)", path, strerror(errno));
-		}
+			return error("git checkout-index: "\
+				     "unable to write file %s", path);
+
 		break;
 	case S_IFGITLINK:
 		if (to_tempfile)
-			return error("git checkout-index: cannot create temporary subproject %s", path);
+			return error("git checkout-index: "\
+				     "cannot create temporary subproject %s",
+				     path);
 		if (mkdir(path, 0777) < 0)
-			return error("git checkout-index: cannot create subproject directory %s", path);
+			return error("git checkout-index: "\
+				     "cannot create subproject directory %s (%s)",
+				     path, strerror(errno));
 		break;
 	default:
-		return error("git checkout-index: unknown file mode for %s", path);
+		return error("git checkout-index: "\
+			     "unknown file mode for %s", path);
 	}
 
 	if (state->refresh_cache) {
@@ -202,7 +200,8 @@ int checkout_entry(struct cache_entry *ce, const struct checkout *state, char *t
 			return 0;
 		if (!state->force) {
 			if (!state->quiet)
-				fprintf(stderr, "git-checkout-index: %s already exists\n", path);
+				fprintf(stderr, "git-checkout-index: "\
+					"%s already exists\n", path);
 			return -1;
 		}
 
@@ -220,7 +219,8 @@ int checkout_entry(struct cache_entry *ce, const struct checkout *state, char *t
 				return error("%s is a directory", path);
 			remove_subtree(path);
 		} else if (unlink(path))
-			return error("unable to unlink old '%s' (%s)", path, strerror(errno));
+			return error("unable to unlink old '%s' (%s)",
+				     path, strerror(errno));
 	} else if (state->not_new)
 		return 0;
 	create_directories(len, path, state);
-- 
1.6.1.349.g99fa5

^ permalink raw reply related

* [PATCH/RFC v1 4/6] use fstat() instead of lstat() when we have an opened file
From: Kjetil Barvik @ 2009-01-26 21:17 UTC (permalink / raw)
  To: git; +Cc: Kjetil Barvik
In-Reply-To: <1233004637-15112-1-git-send-email-barvik@broadpark.no>

Currently inside write_entry() we do an lstat(path, &st) call on a
file which have just been opened inside the exact same function.  It
should be better to call fstat(fd, &st) on the file while it is
opened, and it should be at least as fast as the lstat() method.

Signed-off-by: Kjetil Barvik <barvik@broadpark.no>
---
 entry.c |   11 ++++++++---
 1 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/entry.c b/entry.c
index 8543755..c932ae8 100644
--- a/entry.c
+++ b/entry.c
@@ -96,11 +96,12 @@ write_entry(struct cache_entry *ce, char *path, const struct checkout *state,
 	    int to_tempfile)
 {
 	unsigned int ce_mode_s_ifmt = ce->ce_mode & S_IFMT;
-	int fd, ret;
+	int fd, ret, fstat_done = 0;
 	char *new;
 	struct strbuf buf = STRBUF_INIT;
 	unsigned long size;
 	size_t wrote, newsize = 0;
+	struct stat st;
 
 	switch (ce_mode_s_ifmt) {
 	case S_IFREG:
@@ -151,6 +152,10 @@ write_entry(struct cache_entry *ce, char *path, const struct checkout *state,
 		}
 
 		wrote = write_in_full(fd, new, size);
+		if (state->refresh_cache) {
+			fstat(fd, &st);
+			fstat_done = 1;
+		}
 		close(fd);
 		free(new);
 		if (wrote != size)
@@ -174,8 +179,8 @@ write_entry(struct cache_entry *ce, char *path, const struct checkout *state,
 	}
 
 	if (state->refresh_cache) {
-		struct stat st;
-		lstat(ce->name, &st);
+		if (!fstat_done)
+			lstat(ce->name, &st);
 		fill_stat_cache_info(ce, &st);
 	}
 	return 0;
-- 
1.6.1.349.g99fa5

^ permalink raw reply related

* [PATCH/RFC v1 5/6] combine-diff.c: remove a call to fstat() inside show_patch_diff()
From: Kjetil Barvik @ 2009-01-26 21:17 UTC (permalink / raw)
  To: git; +Cc: Kjetil Barvik
In-Reply-To: <1233004637-15112-1-git-send-email-barvik@broadpark.no>

Currently inside show_patch_diff() we have and fstat() call after an
ok lstat() call.  Since we before the call to fstat() have already
test for the link case with S_ISLNK() the fstat() can be removed.

Signed-off-by: Kjetil Barvik <barvik@broadpark.no>
---
 combine-diff.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/combine-diff.c b/combine-diff.c
index bccc018..ab4df31 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -713,9 +713,8 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
 			result_size = buf.len;
 			result = strbuf_detach(&buf, NULL);
 			elem->mode = canon_mode(st.st_mode);
-		}
-		else if (0 <= (fd = open(elem->path, O_RDONLY)) &&
-			 !fstat(fd, &st)) {
+
+		} else if (0 <= (fd = open(elem->path, O_RDONLY))) {
 			size_t len = xsize_t(st.st_size);
 			ssize_t done;
 			int is_file, i;
-- 
1.6.1.349.g99fa5

^ permalink raw reply related

* [PATCH/RFC v1 6/6] lstat_cache(): print a warning if doing ping-pong between cache types
From: Kjetil Barvik @ 2009-01-26 21:18 UTC (permalink / raw)
  To: git; +Cc: Kjetil Barvik

This is a debug patch which is only to be used while the lstat_cache()
is in the test stage, and should be removed/reverted before the final
relase.

I think it should be useful to catch these warnings, as I it could be
an indication of that the cache would not be very effective if it is
doing ping-pong by switching between different cache types too many
times.

Also, if someone is experimenting with the lstat_cache(), this patch
will maybe be useful while debugging.

If someone is able to trigger the warning, then send a mail to the GIT
mailing list, containing the first 15 lines of the warning, and a
short description of the GIT commands to trigger the warnings.

This patch is against the 'next' branch.  I hope someone is willing to
use this patch for a while, to be able to catch possible ping-pong's.

Signed-off-by: Kjetil Barvik <barvik@broadpark.no>
---

Hmmm, it seems that 'git send-email' always fails for this mail, with
the following error message:

   Died at /usr/local/libexec/git-core/git-send-email line 882.

So I send this as a separate command from 'git send-email', sorry
abouth that.


 symlinks.c |   23 +++++++++++++++++++++++
 1 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/symlinks.c b/symlinks.c
index 81f490c..1c07ae0 100644
--- a/symlinks.c
+++ b/symlinks.c
@@ -49,6 +49,11 @@ static inline void reset_lstat_cache(int track_flags, int prefix_len_stat_func)
 	cache.prefix_len_stat_func = prefix_len_stat_func;
 }
 
+#define SWITCHES_BEFORE_WARNING 10
+static unsigned int cache_switches = 0, number_of_warnings = 0;
+static unsigned int current_cache_func = 0, last_cache_func = 0;
+static unsigned int total_calls = 0;
+
 #define FL_DIR      (1 << 0)
 #define FL_NOENT    (1 << 1)
 #define FL_SYMLINK  (1 << 2)
@@ -75,6 +80,7 @@ static int lstat_cache(int len, const char *name,
 	int match_flags, ret_flags, save_flags, max_len, ret;
 	struct stat st;
 
+	total_calls++;
 	if (cache.track_flags != track_flags ||
 	    cache.prefix_len_stat_func != prefix_len_stat_func) {
 		/*
@@ -84,6 +90,17 @@ static int lstat_cache(int len, const char *name,
 		 */
 		reset_lstat_cache(track_flags, prefix_len_stat_func);
 		match_len = last_slash = 0;
+		cache_switches++;
+		if (cache_switches > SWITCHES_BEFORE_WARNING) {
+			if (number_of_warnings < 10 || number_of_warnings % 1000 == 0)
+				printf("warning from %s:%d cache_switches:%u > %u "\
+				       "(current:%u last:%u total:%u)\n",
+				       __FILE__, __LINE__,
+				       cache_switches, SWITCHES_BEFORE_WARNING,
+				       current_cache_func, last_cache_func,
+				       total_calls);
+			number_of_warnings++;
+		}
 	} else {
 		/*
 		 * Check to see if we have a match from the cache for
@@ -211,6 +228,8 @@ void clear_lstat_cache(void)
  */
 int has_symlink_leading_path(int len, const char *name)
 {
+	last_cache_func = current_cache_func;
+	current_cache_func = 1;
 	return lstat_cache(len, name,
 			   FL_SYMLINK|FL_DIR, USE_ONLY_LSTAT) &
 		FL_SYMLINK;
@@ -222,6 +241,8 @@ int has_symlink_leading_path(int len, const char *name)
  */
 int has_symlink_or_noent_leading_path(int len, const char *name)
 {
+	last_cache_func = current_cache_func;
+	current_cache_func = 2;
 	return lstat_cache(len, name,
 			   FL_SYMLINK|FL_NOENT|FL_DIR, USE_ONLY_LSTAT) &
 		(FL_SYMLINK|FL_NOENT);
@@ -236,6 +257,8 @@ int has_symlink_or_noent_leading_path(int len, const char *name)
  */
 int has_dirs_only_path(int len, const char *name, int prefix_len)
 {
+	last_cache_func = current_cache_func;
+	current_cache_func = 3;
 	return lstat_cache(len, name,
 			   FL_DIR|FL_FULLPATH, prefix_len) &
 		FL_DIR;
-- 
1.6.1.349.g99fa5

^ permalink raw reply related

* Re: [PATCH] mergetool: respect autocrlf by using checkout-index
From: Junio C Hamano @ 2009-01-26 21:28 UTC (permalink / raw)
  To: Charles Bailey; +Cc: Hannu Koivisto, git, Theodore Tso
In-Reply-To: <20090126163114.GD32604@hashpling.org>

Charles Bailey <charles@hashpling.org> writes:

> I suspect that the LF endings in the file is due to the fact that in
> builtin-merge-file.c, the file is opened (fopen) in binary mode
> ("wb"), but xdl_merge terminates all lines with a raw '\n'.
>
> The obvious fix would be to change fopen in builtin-file-merge.c to
> use "w" instead, but this doesn't work in a number of scenarios. In
> particular, it is wrong for repositories on windows with core.autocrlf
> set to false, and would not fix non-windows repositories with
> core.autocrlf set to true.
>
> Currently, I've no idea as to what the solution should be.

"git file-merge" is designed to be a replacement for stock RCS merge, and
unfortunately it does not call convert_to_working_tree(), nor has any way
to know for which path it should take the attributes to apply to affect
what convert_to_working_tree() should do even if it were to call it.

I think we would need a new option to the command that says "pretend this
is about merging this path, and use the gitattributes specified for it
when writing out the result."

^ permalink raw reply

* Re: [PATCH] rebase -i: correctly remember --root flag across --continue
From: Thomas Rast @ 2009-01-26 21:28 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Johannes Schindelin
In-Reply-To: <7veiyp4w2m.fsf@gitster.siamese.dyndns.org>

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

Junio C Hamano wrote:
> Thomas Rast <trast@student.ethz.ch> writes:
> > +	GIT_EDITOR=: test_must_fail git rebase -i --root --onto master &&
> 
> Maybe I am misrecalling things but didn't we have reports from people on
> some platforms that single-shot exporting of the environment like this one
> does not work for them?

Then that deserves a fix, though I should point out that the original
patches (now on master) have the same code in them.

[So many quirks and so little time!]

> > +sed 's/#/ /g' > expect-conflict-p <<'EOF'
> > +*   Merge branch 'third' into other
> > +|\##
> > +| * 6
> > +* |   Merge branch 'side' into other
> > +|\ \##
> > +| * | 5
> > +* | | 4
> > +|/ /##
> > +* | 3
> > +|/##
> > +* conflict
> > +* 2
> > +* 1
> > +EOF
> 
> I do not like this very much.  Future improvements of the graph drawing
> algorithm (one obvious "flaw" you are exposing by the above is that it has
> trailing whitespaces that can be trimmed, and somebody else may be
> inclined to fix) would break the expectation this test vector has.
> 
> Do you have to compare the topology this way, or are there other more
> reliable ways?

It would certainly be possible to test the SHA1 of the resulting
branch tip, but t/README says I shouldn't.  Or we could spell it out
as a series of 'parent of X is Y' and 'message of Y is foo' tests,
which seems rather messy.

The above approach at least has the advantage that a test failure due
to format change can be diagnosed very quickly just from the diff that
is shown with -v.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply

* Re: [PATCH] Allow format-patch to create patches for merges
From: Nathan W. Panike @ 2009-01-26 21:46 UTC (permalink / raw)
  To: Jeff King; +Cc: Johannes Schindelin, git
In-Reply-To: <20090126204543.GF27604@coredump.intra.peff.net>

I have not used the bundle stuff, but yes, it seems to be a better fit
for what I am trying to do.

Thanks,

Nathan Panike

On Mon, Jan 26, 2009 at 2:45 PM, Jeff King <peff@peff.net> wrote:
> On Mon, Jan 26, 2009 at 10:27:18AM -0600, Nathan W. Panike wrote:
>
>> I think I have an unusual workflow where my patch makes sense,
>> although it probably does not for the vast majority of git users.  I
>> regularly use 3 machines: S, L, and H.  I keep my work synchronized by
>> using git.  Normally, I fetch from S to L or to H, depending on which
>> machine I am working on at the moment.  I also push from L or H to S.
>> I sporadically lose connectivity to S, so I have a hook in the repo on
>> S to send a backup email to me on mail server M, which has a more
>> reliable connection.  This email also serves as a  reminder when I
>
> Have you considered sending a bundle instead of a patch in the backup
> email? That is the more exact equivalent of a push (i.e., it preserves
> your actual commits, sha1 and all).
>
> -Peff
>

^ permalink raw reply

* Re: [PATCH] rebase -i: correctly remember --root flag across --continue
From: Junio C Hamano @ 2009-01-26 21:49 UTC (permalink / raw)
  To: Thomas Rast; +Cc: git, Johannes Schindelin
In-Reply-To: <7veiyp4w2m.fsf@gitster.siamese.dyndns.org>

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

> Thomas Rast <trast@student.ethz.ch> writes:
> ...
>> +sed 's/#/ /g' > expect-conflict-p <<'EOF'
>> +*   Merge branch 'third' into other
>> +|\##
>> +| * 6
>> +* |   Merge branch 'side' into other
>> +|\ \##
>> +| * | 5
>> +* | | 4
>> +|/ /##
>> +* | 3
>> +|/##
>> +* conflict
>> +* 2
>> +* 1
>> +EOF
>
> I do not like this very much.  Future improvements of the graph drawing
> algorithm (one obvious "flaw" you are exposing by the above is that it has
> trailing whitespaces that can be trimmed, and somebody else may be
> inclined to fix) would break the expectation this test vector has.
>
> Do you have to compare the topology this way, or are there other more
> reliable ways?

Perhaps something like this.

 t/t3412-rebase-root.sh |   36 +++++++++++++++++++++---------------
 1 files changed, 21 insertions(+), 15 deletions(-)

diff --git i/t/t3412-rebase-root.sh w/t/t3412-rebase-root.sh
index 29bb6d0..2408cf8 100755
--- i/t/t3412-rebase-root.sh
+++ w/t/t3412-rebase-root.sh
@@ -240,19 +240,24 @@ test_expect_success 'rebase -i --root with conflict (second part)' '
 '
 
 sed 's/#/ /g' > expect-conflict-p <<'EOF'
-*   Merge branch 'third' into other
-|\##
-| * 6
-* |   Merge branch 'side' into other
-|\ \##
-| * | 5
-* | | 4
-|/ /##
-* | 3
-|/##
-* conflict
-* 2
-* 1
+commit conflict3 conflict3~1 conflict3^2
+Merge branch 'third' into other
+commit conflict3^2 conflict3~4
+6
+commit conflict3~1 conflict3~2 conflict3~1^2
+Merge branch 'side' into other
+commit conflict3~1^2 conflict3~3
+5
+commit conflict3~2 conflict3~3
+4
+commit conflict3~3 conflict3~4
+3
+commit conflict3~4 conflict3~5
+conflict
+commit conflict3~5 conflict3~6
+2
+commit conflict3~6
+1
 EOF
 
 test_expect_success 'rebase -i -p --root with conflict (first part)' '
@@ -268,8 +273,9 @@ test_expect_success 'fix the conflict' '
 
 test_expect_success 'rebase -i -p --root with conflict (second part)' '
 	git rebase --continue &&
-	git log --graph --topo-order --pretty=tformat:"%s" > conflict3 &&
-	test_cmp expect-conflict-p conflict3
+	git rev-list --topo-order --parents --pretty="tformat:%s" HEAD |
+	git name-rev --stdin --name-only --refs=refs/heads/conflict3 >out &&
+	test_cmp expect-conflict-p out
 '
 
 test_done

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox