* Re: Git Books
From: nadim khemir @ 2008-12-06 14:38 UTC (permalink / raw)
To: git list
In-Reply-To: <m34p1hihx4.fsf@localhost.localdomain>
On Saturday 06 December 2008 13.54.06 Jakub Narebski wrote:
> "Scott Chacon" <schacon@gmail.com> writes:
> > I have been talked into helping write a real, paper-based book on Git
> > ...
>
> What I really would like to see in a paper book is _diagrams_, in the
> form of simple graphs (and not UML-like diagrams, of flow-control like
> diagrams).
I was thinking about buying the pdf below. The little I can see looks like
there are a bunch of diagrams in it.
http://peepcode.com/products/git-internals-pdf
> You can find them in various slides for presentations
> (among others Junio's talks), and sometimes in blog posts[1], but
> usually only as ASCII-diagrams[2] in git documentation. (And the
> examples in"The Git Comminity Book" I've seen so far are a bit too
> complicated).
I like doing my ASCII-diagrams with Asciio, unsurprizingly.
********
* HEAD *
********
|
v
.-----. .--------.
| tag | | branch |
'-----' '--------'
| |
v v
.......... .......... ..........
. commit .<--------------. commit .<----. commit .
.......... .......... ..........
| | |
v v v
.------. .------. .------.
| tree |--------.--------| tree | | tree |
'------' | '------' '------'
| v | |
v .------. v v
.------. | blob | .------. .------.
| tree |--. '------' .--| tree | | blob |
'------' | | '------' '------'
'-----.-----' |
| v
v .------.
.------. | tree |
| blob | '------'
'------' |
v
.------.
| blob |
'------'
cheers, Nadim.
^ permalink raw reply
* Re: [RFCv3 1/2] gitweb: add patch view
From: Giuseppe Bilotta @ 2008-12-06 13:46 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git, Petr Baudis, Junio C Hamano
In-Reply-To: <200812061409.02730.jnareb@gmail.com>
On Sat, Dec 6, 2008 at 2:09 PM, Jakub Narebski <jnareb@gmail.com> wrote:
> On Sat, Dec 6, 2008 at 13:34, Giuseppe Bilotta wrote:
>> On Sat, Dec 6, 2008 at 1:34 AM, Jakub Narebski <jnareb@gmail.com> wrote:
>>> On Wed, 3 Dec 2008, Giuseppe Bilotta wrote:
>>>
>>>> The manually-built email format in commitdiff_plain output is not
>>>> appropriate for feeding git-am, because of two limitations:
>>>> * when a range of commits is specified, commitdiff_plain publishes a
>>>> single patch with the message from the first commit, instead of a
>>>> patchset,
>>>
>>> This is because 'commitdiff_plain' wasn't _meant_ as patch series view,
>>> to be fed to git-am. Actually it is a bit cross between "git show"
>>> result with '--pretty=email' format, and "git diff" between two commits,
>>> to be fed to git-apply or GNU patch.
>>>
>>> Nevertheless the above reasoning doesn't need to be put in a commit
>>> message. But it explains why new 'patch' / 'patchset' view is needed:
>>> because there was no equivalent.
>>
>> I'll remove it.
>
> Errr... sorry, I haven't made myself clear. I meant here that *my*
> comment should not be added; your explanation about adding 'patch'
> view should IMHO stay, perhaps reworked a bit: commitdiff is not
> about generating patch series.
Oops, ok, I'll just rewrite it better 8-)
>>>> * in either case, the patch summary is replicated both as email subject
>>>> and as first line of the email itself, resulting in a doubled summary
>>>> if the output is fed to git-am.
>>>
>>> This is independent issue which is I think worth correcting anyway,
>>> unless we decide to scrap 'commitdiff_plain' view altogether.
>>> But I think we would want some text/plain patch view to be applied
>>> by GNU patch (for example in RPM .spec file).
>>
>> I don't think we should scrap commitdiff either, but the subject
>> replication is not really an issue if the view is not fed to git am.
>
> Well, there is it.
Considering the comments on the second patch too, I've been thinking
that it might worth it to merge commitdiff_plain and patch view for
single commits. Some changes for multi-commits commitdiff_plain can be
considered too, but as I mentioned this is a major changes and should
be dealt with on its own.
>>>> + # The maximum number of patches in a patchset generated in patch
>>>> + # view. Set this to 0 or undef to disable patch view, or to a
>>>> + # negative number to remove any limit.
>>>> + 'patches' => {
>>>> + 'override' => 1,
>>>> + 'default' => [16]},
>>>> );
>>>
>>> You need to set "'sub' => \&feature_patches" for feature to be
>>> override-able at all. Also features are usually not overridable
>>> by default, which reduces load a tiny bit (by _possibly_ not reading
>>> config, although that shouldn't matter much now with reading whole
>>> commit using single call to git-config, and not one call per variable).
>>
>> I think I'll make the feature non-overridable. I'll also make it
>> default to disabled, although I'm not particularly happy with the
>> choice.
>
> I think that having 'patches' feature to be able to override is a good
> idea, as limit on number of patches might depend on _repository_, for
> example being lower for repository with large commits (large in sense
> of diff size).
>
> Default with override unset seems to be precedence... as to having it
> disabled by default: having feature which require configuration enabled
> by default serves as an example of configuration; on the other hand the
> example might be in comments, like for 'actions' %feature.
I've added the feature_patches sub. Also, considering that there are
now basically 3 votes against 1 for enabling the feature by default,
I'll keep it enabled.
>>> And I think the default might be set larger: 'log' view generates
>>> as big if not bigger load, and it is split into 100 commits long
>>> pages.
>>
>> Hm, I would say the load of patch view is much higher than the load of
>> log view, both in terms of bandwidth and in terms of load on the
>> server, because of the diffs.
>
> Ah, I forgot about that. Limit to 16-25 seems to be reasonable, then.
I'll go with 16 for the time being, I think it's large enough to
accomodate most patchsets.
--
Giuseppe "Oblomov" Bilotta
^ permalink raw reply
* Re: Git Books
From: Dilip M @ 2008-12-06 13:39 UTC (permalink / raw)
To: Christian MICHON; +Cc: Scott Chacon, git list
In-Reply-To: <46d6db660812060427h3c97efd9ie4aa604133aa3583@mail.gmail.com>
On Sat, Dec 6, 2008 at 5:57 PM, <christian.michon@gmail.com> wrote:
> workflows, workflows, workflows...
>
> gitconfig/aliases best practices
>
> tips and tricks in branches (ex: parentless...) and in setting up git servers
Work flows, setting up the GIT server, publishing codes,....would be
_really_ helpful to great extent....and help a lot in deploying GIT
The work flow something like...http://source.android.com/submit-patches/workflow
--
Dilip
^ permalink raw reply
* Re: [RFCv3 2/2] gitweb: links to patch action in commitdiff and shortlog view
From: Giuseppe Bilotta @ 2008-12-06 13:25 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git, Petr Baudis, Junio C Hamano
In-Reply-To: <200812060153.52947.jnareb@gmail.com>
On Sat, Dec 6, 2008 at 1:53 AM, Jakub Narebski <jnareb@gmail.com> wrote:
> On Wed, 3 Dec 2008, Giuseppe Bilotta wrote:
>
>> In shortlog view, a link to the patchset is only offered when the number
>> of commits shown is less than the allowed maximum number of patches.
>>
>> Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
>> + if (gitweb_check_feature('patches')) {
>> + $formats_nav .= " | " .
>> + $cgi->a({-href => href(action=>"patch", -replay=>1)},
>> + "patch");
>> + }
>>
>> if (!defined $parent) {
>> $parent = "--root";
>> @@ -5415,6 +5420,11 @@ sub git_commitdiff {
>> $formats_nav =
>> $cgi->a({-href => href(action=>"commitdiff_plain", -replay=>1)},
>> "raw");
>> + if ($patch_max) {
>> + $formats_nav .= " | " .
>> + $cgi->a({-href => href(action=>"patch", -replay=>1)},
>> + "patch");
>> + }
>>
>> if (defined $hash_parent &&
>> $hash_parent ne '-c' && $hash_parent ne '--cc') {
>
> In the above two hunks 'patch' view functions as "git show --pretty=email"
> text/plain equivalent, but this duplicates a bit 'commitdiff_plain'
> functionality. Well, 'commitdiff_plain' has currently some errors,
> but...
All things considered, for single commit view there is (modulo bugs)
no factual difference between plain diff and patch view.
Although we could merge them, I'm thinking that the plain diff view
has somewhat too much information in it. For backwards compatibility
it's probably not wise to change it, but we should consider it for the
next major version, honestly.
>> @@ -5949,6 +5959,14 @@ sub git_shortlog {
>> $cgi->a({-href => href(-replay=>1, page=>$page+1),
>> -accesskey => "n", -title => "Alt-n"}, "next");
>> }
>> + my $patch_max = gitweb_check_feature('patches');
>> + if ($patch_max) {
>> + if ($patch_max < 0 || @commitlist <= $patch_max) {
>> + $paging_nav .= " ⋅ " .
>> + $cgi->a({-href => href(action=>"patch", -replay=>1)},
>> + @commitlist > 1 ? "patchset" : "patch");
>> + }
>> + }
>
> Here 'patch' view functions as "git format-patch", able to be downloaded
> and fed to git-am. Perhaps the action should also be named 'patches'
> here?; it could lead to the same function.
I had half an idea to do so. 'patches' or 'patchset'?
> By the way, there is subtle bug in above link. If shortlog view is less
> than $patch_max commits long, but it is because the history for a given
> branch (or starting from given commit) is so short, and not because
> there is cutoff $hash_parent set, the 'patchset' view wouldn't display
> plain text equivalent view, but only patch for top commit.
Ah, good point.
Hm, not easy to solve. One way could be to add the hash_parent
manually. Or we could make the 'patches' view different from the
'patch' view in the way it handles refspecs without ranges. I'm
leaning towards the latter. What's your opinion?
> I assume that the link is only for 'shortlog' view, and not also for
> 'log' and 'history' views because 'shortlog' is the only log-like view
> which support $hash_parent?
The actual reason is that I never use log nor history view, but since
they don't support hash_parent because of this (I was the one who sent
the patch to support hash_parent in shortlog view) you could
paralogistically say that's the reason ;-)
I'm not sure about history view, but for log view I'm considering
addiong also a 'patch' link next to each commit. I'll think about it.
--
Giuseppe "Oblomov" Bilotta
^ permalink raw reply
* Re: [RFCv3 1/2] gitweb: add patch view
From: Petr Baudis @ 2008-12-06 13:10 UTC (permalink / raw)
To: Giuseppe Bilotta; +Cc: Junio C Hamano, Jakub Narebski, git
In-Reply-To: <cb7bb73a0812060501s6a3faea7l74e81c2a591ba2a7@mail.gmail.com>
On Sat, Dec 06, 2008 at 02:01:09PM +0100, Giuseppe Bilotta wrote:
> On Sat, Dec 6, 2008 at 1:46 AM, Junio C Hamano <gitster@pobox.com> wrote:
> > Some things, like sitewide default css changes, cannot be made disabled
> > by default. But a new feature can easily be kept disabled by default not
> > to cause needless surprises.
>
> In the eternal war between making feature easily available and not
> disturbing the user too much between upgrades, I'm more on the 'making
> available' field, especially when the features are not particularly
> invasive (e.g., the patch view only adds one single link, on the
> navigation bar, and only in some views).
>
> It should also be noted that if the sysadmin deploys a new software
> version without telling its users, there's an obvious communication
> problem between the sysadmin and its users, but that's not something
> the tool developers should try to work around. Otherwise we'd still be
> offering the dash version of the commands by default.
I feel exactly the same.
Acked-by: Petr Baudis <pasky@suse.cz>
Same goes for the patches, I haven't had time to examine them in detail
but the general shape looks fine.
^ permalink raw reply
* Re: [RFCv3 1/2] gitweb: add patch view
From: Jakub Narebski @ 2008-12-06 13:09 UTC (permalink / raw)
To: Giuseppe Bilotta; +Cc: git, Petr Baudis, Junio C Hamano
In-Reply-To: <cb7bb73a0812060434r2b48fd1ev359020fb1e0d1603@mail.gmail.com>
On Sat, Dec 6, 2008 at 13:34, Giuseppe Bilotta wrote:
> On Sat, Dec 6, 2008 at 1:34 AM, Jakub Narebski <jnareb@gmail.com> wrote:
>> On Wed, 3 Dec 2008, Giuseppe Bilotta wrote:
>>
>>> The manually-built email format in commitdiff_plain output is not
>>> appropriate for feeding git-am, because of two limitations:
>>> * when a range of commits is specified, commitdiff_plain publishes a
>>> single patch with the message from the first commit, instead of a
>>> patchset,
>>
>> This is because 'commitdiff_plain' wasn't _meant_ as patch series view,
>> to be fed to git-am. Actually it is a bit cross between "git show"
>> result with '--pretty=email' format, and "git diff" between two commits,
>> to be fed to git-apply or GNU patch.
>>
>> Nevertheless the above reasoning doesn't need to be put in a commit
>> message. But it explains why new 'patch' / 'patchset' view is needed:
>> because there was no equivalent.
>
> I'll remove it.
Errr... sorry, I haven't made myself clear. I meant here that *my*
comment should not be added; your explanation about adding 'patch'
view should IMHO stay, perhaps reworked a bit: commitdiff is not
about generating patch series.
>>> * in either case, the patch summary is replicated both as email subject
>>> and as first line of the email itself, resulting in a doubled summary
>>> if the output is fed to git-am.
>>
>> This is independent issue which is I think worth correcting anyway,
>> unless we decide to scrap 'commitdiff_plain' view altogether.
>> But I think we would want some text/plain patch view to be applied
>> by GNU patch (for example in RPM .spec file).
>
> I don't think we should scrap commitdiff either, but the subject
> replication is not really an issue if the view is not fed to git am.
Well, there is it.
>>> + # The maximum number of patches in a patchset generated in patch
>>> + # view. Set this to 0 or undef to disable patch view, or to a
>>> + # negative number to remove any limit.
>>> + 'patches' => {
>>> + 'override' => 1,
>>> + 'default' => [16]},
>>> );
>>
>> You need to set "'sub' => \&feature_patches" for feature to be
>> override-able at all. Also features are usually not overridable
>> by default, which reduces load a tiny bit (by _possibly_ not reading
>> config, although that shouldn't matter much now with reading whole
>> commit using single call to git-config, and not one call per variable).
>
> I think I'll make the feature non-overridable. I'll also make it
> default to disabled, although I'm not particularly happy with the
> choice.
I think that having 'patches' feature to be able to override is a good
idea, as limit on number of patches might depend on _repository_, for
example being lower for repository with large commits (large in sense
of diff size).
Default with override unset seems to be precedence... as to having it
disabled by default: having feature which require configuration enabled
by default serves as an example of configuration; on the other hand the
example might be in comments, like for 'actions' %feature.
>> And I think the default might be set larger: 'log' view generates
>> as big if not bigger load, and it is split into 100 commits long
>> pages.
>
> Hm, I would say the load of patch view is much higher than the load of
> log view, both in terms of bandwidth and in terms of load on the
> server, because of the diffs.
Ah, I forgot about that. Limit to 16-25 seems to be reasonable, then.
>>> @@ -5483,7 +5498,12 @@ sub git_commitdiff {
>>> open $fd, "-|", git_cmd(), "diff-tree", '-r', @diff_opts,
>>> '-p', $hash_parent_param, $hash, "--"
>>> or die_error(500, "Open git-diff-tree failed");
>>> -
>>> + } elsif ($format eq 'patch') {
>>> + open $fd, "-|", git_cmd(), "format-patch", '--encoding=utf8',
>>> + '--stdout', $patch_max > 0 ? "-$patch_max" : (),
>>> + $hash_parent ? ('-n', "$hash_parent..$hash") :
>>> + ('--root', '-1', $hash)
>>
>> This bit makes 'patch' view behave bit differently than git-format-patch,
>> which I think is a good idea: namely it shows single patch if there is
>> no cutoff. This should be mentioned in commit message, and perhaps
>> even in a comment in code.
>>
>> Beside, if you show only single patch because $hash_parent is not set,
>> you don't need to examine $patch_max nor set limit, because you use '-1'.
>> Currently if $patch_max > 0 and !$hash_parent, you pass limit for the
>> number of patches twice. This I think is harmless but...
>
> I've reworked the code a bit, making the commit spec an array computed
> before passing it to the command line. The code is cleaner but
> obviously longer 8-)
Cleaner is good.
> The double limit worked properly, btw.
Nice to know. But should we rely on corner cases handling?
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [RFCv3 1/2] gitweb: add patch view
From: Giuseppe Bilotta @ 2008-12-06 13:01 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jakub Narebski, git, Petr Baudis
In-Reply-To: <7vabbaxh8y.fsf@gitster.siamese.dyndns.org>
On Sat, Dec 6, 2008 at 1:46 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
>
>>> + # The maximum number of patches in a patchset generated in patch
>>> + # view. Set this to 0 or undef to disable patch view, or to a
>>> + # negative number to remove any limit.
>>> + 'patches' => {
>>> + 'override' => 1,
>>> + 'default' => [16]},
>>> );
>>
>> You need to set "'sub' => \&feature_patches" for feature to be
>> override-able at all. Also features are usually not overridable
>> by default, which reduces load a tiny bit (by _possibly_ not reading
>> config, although that shouldn't matter much now with reading whole
>> commit using single call to git-config, and not one call per variable).
>> And I think the default might be set larger: 'log' view generates
>> as big if not bigger load, and it is split into 100 commits long
>> pages.
>
> I do not think defaulting to 'no' for overridability nor defaulting a new
> feature to 'disabled' have much to do with the load, but they are more
> about the principle of least surprise. Somebody who runs gitweb in the
> playpen he was given on the server shouldn't be getting a phone call from
> his users late at night complaining that the page his gitweb serves look
> different and has one extra link per each line, only because the sysadmin
> of the server decided to update git to 1.6.1 without telling him.
>
> Once a new version capable of serving a new feature is introduced, he can
> plan, announce and deploy by switching the feature on in his gitweb
> configuration file.
>
> Some things, like sitewide default css changes, cannot be made disabled
> by default. But a new feature can easily be kept disabled by default not
> to cause needless surprises.
In the eternal war between making feature easily available and not
disturbing the user too much between upgrades, I'm more on the 'making
available' field, especially when the features are not particularly
invasive (e.g., the patch view only adds one single link, on the
navigation bar, and only in some views).
It should also be noted that if the sysadmin deploys a new software
version without telling its users, there's an obvious communication
problem between the sysadmin and its users, but that's not something
the tool developers should try to work around. Otherwise we'd still be
offering the dash version of the commands by default.
(Plus, weren't you the one suggesting that the remote heads feature
should be enabled by default? And that's an even more invasive change,
if you ask me.)
--
Giuseppe "Oblomov" Bilotta
^ permalink raw reply
* Re: Git Books
From: Paolo Ciarrocchi @ 2008-12-06 12:56 UTC (permalink / raw)
To: Scott Chacon; +Cc: git list
In-Reply-To: <d411cc4a0812060358ub640ea3kd04072c5640eef68@mail.gmail.com>
On 12/6/08, Scott Chacon <schacon@gmail.com> wrote:
> Hey all,
>
> I have been talked into helping write a real, paper-based book on Git
> for a publisher big enough that you may even see it in your local
> Borders or whatnot. (And, it appears that Junio has been as well:
> http://gitster.livejournal.com/21616.html)
>
> So, since I'm near the beginning of this process, I was wondering if
> the group had any feedback as to what might be super helpful to
> include. I mean, I have a pretty good layout and all, but if you
> wanted to point me to some threads that tend to crop up in the mailing
> list and IRC channel from relative newcomers that I might be able to
> nip in the bud, I would like to. I'm addressing the stuff that _I_
> hear a lot, and I'm scanning the IRC logs and list for topics, but I
> figured many of you must answer the same questions all the time, too.
Please spend lot of words about fetch and pull. These are the most
hard to ùnderstand commands for people that are used to cvs and svn.
Btw, i would love to see a git in a nutshell book in my native
language (italian).
I'm willing to help writing and translating.
Ciao,
--
Paolo
http://paolo.ciarrocchi.googlepages.com/
http://mypage.vodafone.it/
^ permalink raw reply
* Re: Git Books
From: Jakub Narebski @ 2008-12-06 12:54 UTC (permalink / raw)
To: Scott Chacon; +Cc: git list
In-Reply-To: <d411cc4a0812060358ub640ea3kd04072c5640eef68@mail.gmail.com>
"Scott Chacon" <schacon@gmail.com> writes:
> I have been talked into helping write a real, paper-based book on Git
> for a publisher big enough that you may even see it in your local
> Borders or whatnot. (And, it appears that Junio has been as well:
> http://gitster.livejournal.com/21616.html)
>
> So, since I'm near the beginning of this process, I was wondering if
> the group had any feedback as to what might be super helpful to
> include. I mean, I have a pretty good layout and all, but if you
> wanted to point me to some threads that tend to crop up in the mailing
> list and IRC channel from relative newcomers that I might be able to
> nip in the bud, I would like to. I'm addressing the stuff that _I_
> hear a lot, and I'm scanning the IRC logs and list for topics, but I
> figured many of you must answer the same questions all the time, too.
What I really would like to see in a paper book is _diagrams_, in the
form of simple graphs (and not UML-like diagrams, of flow-control like
diagrams). You can find them in various slides for presentations
(among others Junio's talks), and sometimes in blog posts[1], but
usually only as ASCII-diagrams[2] in git documentation. (And the
examples in"The Git Comminity Book" I've seen so far are a bit too
complicated).
For example explaining git object model, explaining refs: local
branches, remote-tracking branches and tags, explaining pulling and
pushing, explaining merging and 3-way merge algorithm are difficult to
do without diagrams; diagrams make it much easier to understand.
Others have emphasized workflows enough...
Footnotes:
==========
[1] http://www.gnome.org/~federico/news-2008-11.html#pushing-and-pulling-with-git-1
[2] This is understandable, as while AsciiDoc format makes it quite
good on promise to be easy to edit for non-tech users, AFAIK there
is no such format for diagrams and pictures. PIC and Asymptote
nonwithstanding.
--
Jakub Narebski
Poland
ShadeHawk on #git
^ permalink raw reply
* Re: [RFCv3 1/2] gitweb: add patch view
From: Giuseppe Bilotta @ 2008-12-06 12:34 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git, Petr Baudis, Junio C Hamano
In-Reply-To: <200812060134.22959.jnareb@gmail.com>
On Sat, Dec 6, 2008 at 1:34 AM, Jakub Narebski <jnareb@gmail.com> wrote:
> On Wed, 3 Dec 2008, Giuseppe Bilotta wrote:
>
>> The manually-built email format in commitdiff_plain output is not
>> appropriate for feeding git-am, because of two limitations:
>> * when a range of commits is specified, commitdiff_plain publishes a
>> single patch with the message from the first commit, instead of a
>> patchset,
>
> This is because 'commitdiff_plain' wasn't _meant_ as patch series view,
> to be fed to git-am. Actually it is a bit cross between "git show"
> result with '--pretty=email' format, and "git diff" between two commits,
> to be fed to git-apply or GNU patch.
>
> Nevertheless the above reasoning doesn't need to be put in a commit
> message. But it explains why new 'patch' / 'patchset' view is needed:
> because there was no equivalent.
I'll remove it.
>> * in either case, the patch summary is replicated both as email subject
>> and as first line of the email itself, resulting in a doubled summary
>> if the output is fed to git-am.
>
> This is independent issue which is I think worth correcting anyway,
> unless we decide to scrap 'commitdiff_plain' view altogether.
> But I think we would want some text/plain patch view to be applied
> by GNU patch (for example in RPM .spec file).
I don't think we should scrap commitdiff either, but the subject
replication is not really an issue if the view is not fed to git am.
>> + # The maximum number of patches in a patchset generated in patch
>> + # view. Set this to 0 or undef to disable patch view, or to a
>> + # negative number to remove any limit.
>> + 'patches' => {
>> + 'override' => 1,
>> + 'default' => [16]},
>> );
>
> You need to set "'sub' => \&feature_patches" for feature to be
> override-able at all. Also features are usually not overridable
> by default, which reduces load a tiny bit (by _possibly_ not reading
> config, although that shouldn't matter much now with reading whole
> commit using single call to git-config, and not one call per variable).
I think I'll make the feature non-overridable. I'll also make it
default to disabled, although I'm not particularly happy with the
choice.
> And I think the default might be set larger: 'log' view generates
> as big if not bigger load, and it is split into 100 commits long
> pages.
Hm, I would say the load of patch view is much higher than the load of
log view, both in terms of bandwidth and in terms of load on the
server, because of the diffs.
>> sub git_commitdiff {
>> my $format = shift || 'html';
>> +
>> + my $patch_max = gitweb_check_feature('patches');
>
> Wouldn't it be better to name this variable $max_patchset_size, or
> something like that? I'm not saying that this name is bad, but I'm
> wondering if it could be better...
max_patchset_size sounds much worse than patch_max to me, which is why
I went for the latter 8-)
>> if ($format eq 'html') {
>> $formats_nav =
>> $cgi->a({-href => href(action=>"commitdiff_plain", -replay=>1)},
>> @@ -5483,7 +5498,12 @@ sub git_commitdiff {
>> open $fd, "-|", git_cmd(), "diff-tree", '-r', @diff_opts,
>> '-p', $hash_parent_param, $hash, "--"
>> or die_error(500, "Open git-diff-tree failed");
>> -
>> + } elsif ($format eq 'patch') {
>> + open $fd, "-|", git_cmd(), "format-patch", '--encoding=utf8',
>> + '--stdout', $patch_max > 0 ? "-$patch_max" : (),
>
>> + $hash_parent ? ('-n', "$hash_parent..$hash") :
>> + ('--root', '-1', $hash)
>
> This bit makes 'patch' view behave bit differently than git-format-patch,
> which I think is a good idea: namely it shows single patch if there is
> no cutoff. This should be mentioned in commit message, and perhaps
> even in a comment in code.
>
> Beside, if you show only single patch because $hash_parent is not set,
> you don't need to examine $patch_max nor set limit, because you use '-1'.
> Currently if $patch_max > 0 and !$hash_parent, you pass limit for the
> number of patches twice. This I think is harmless but...
I've reworked the code a bit, making the commit spec an array computed
before passing it to the command line. The code is cleaner but
obviously longer 8-)
The double limit worked properly, btw.
>> + # TODO add X-Git-Tag/X-Git-Url headers in a sensible way
>
> Sensible way would mean modifying git-format-patch to be able to add
> extra headers via command option, just like it is now possible via
> config variable format.headers, I think. Because there are no surefire
> markers of where one patch ends and another begins: commit message is
> free text, and can contain diff... although if it contains diff
> separator '---' then git-am would have problem with patch; or at least
> even assuming sane commit messages it is not easy.
>
> Also I think that only X-Git-Url makes sense, and it is per whole
> patchset (whole 'patch' view output) and not for each individual
> patch.
I've stripped this commet for the time being. I'm not sure even
X-Git-Url makes sense, and the fact that it should only attached to
the first email makes it an oddball.
--
Giuseppe "Oblomov" Bilotta
^ permalink raw reply
* Re: Git Books
From: Christian MICHON @ 2008-12-06 12:27 UTC (permalink / raw)
To: Scott Chacon; +Cc: git list
In-Reply-To: <d411cc4a0812060358ub640ea3kd04072c5640eef68@mail.gmail.com>
On Sat, Dec 6, 2008 at 12:58 PM, Scott Chacon <schacon@gmail.com> wrote:
> Hey all,
>
> I have been talked into helping write a real, paper-based book on Git
> for a publisher big enough that you may even see it in your local
> Borders or whatnot. (And, it appears that Junio has been as well:
> http://gitster.livejournal.com/21616.html)
>
> So, since I'm near the beginning of this process, I was wondering if
> the group had any feedback as to what might be super helpful to
> include. I mean, I have a pretty good layout and all, but if you
> wanted to point me to some threads that tend to crop up in the mailing
> list and IRC channel from relative newcomers that I might be able to
> nip in the bud, I would like to. I'm addressing the stuff that _I_
> hear a lot, and I'm scanning the IRC logs and list for topics, but I
> figured many of you must answer the same questions all the time, too.
>
> Thanks,
> Scott
> --
workflows, workflows, workflows...
gitconfig/aliases best practices
tips and tricks in branches (ex: parentless...) and in setting up git servers
my 3 cents :)
--
Christian
--
http://detaolb.sourceforge.net/, a linux distribution for Qemu with Git inside !
^ permalink raw reply
* How to clone git repository with git-svn meta-data included?
From: Grzegorz Kossakowski @ 2008-12-06 12:15 UTC (permalink / raw)
To: git
Hello,
Some folks at Apache are experimenting with Git and we are currently seeking for the git-svn
integration that fits our needs and infrastructure.
After some evaluation we decided that our setup could be described using following points:
a) our svn repository remains our main, official server where every committer is obligated to push
their changes to at some time.
b) we set up clone of svn repository using git-svn. One of our members, Jukka Zitting, maintains
such a service here[1]. Such repositories should be usable both for our committers (that have rights
to push to svn) and our contributors that want to contribute random patches
c) we want carefully track who committed/contributed what
Basically, a) implies b) and point b) looks little bit problematic right now.
Jukka has set up his hosting using method described in his e-mail[2] which basically makes use of
git svn. The major problem is that if one clones Jukka's repository then git svn information is not
being cloned so committers have no means to push their changes to main, svn server.
I've tried to play a little bit around with this issue and I tried to copy information from .git
directory found on Jukka's server. This made me able to push my changes but git svn insisted on
rebasing my repository using commits found in svn which is wrong. Basically we want such a setup
that uses git repository (Jukka's clone) for pulling changes and local git svn for pushing changes.
Git svn should never try to rebase local repository because this will lead to two different trees on
two different machines so we won't be able to exchange and merge changesets.
Is it possible with Git right now?
Another point (c) which seems to be brought a couple of times but never a definitive answer has been
given, AFAIK. Let's imagine we have committer C and two contributors A and B.
A and B start to work on some feature and C agreed to help A and B and once their work is finished
to merge their changes into his repository and eventually push them to main, svn repository. Now A
and B work on implementation and from time to time their merge changes from each other. Once they
are finished A asks C to merge their work into C's repository. Everything is fine provided we can
trust both A and B.
What if A was not fair and has rewritten a few commits coming from B so they contain malicious code?
How we can detect something like that and how C be sure that what he merges is really work
attributed by correct names?
Thanks for your answers.
[1] http://jukka.zitting.name/git/
[2] http://markmail.org/message/fzzy7nepk7olx5fl
--
Best regards,
Grzegorz Kossakowski
^ permalink raw reply
* Re: Git Books
From: Thomas Adam @ 2008-12-06 12:09 UTC (permalink / raw)
To: Scott Chacon; +Cc: git list
In-Reply-To: <d411cc4a0812060358ub640ea3kd04072c5640eef68@mail.gmail.com>
2008/12/6 Scott Chacon <schacon@gmail.com>:
> So, since I'm near the beginning of this process, I was wondering if
> the group had any feedback as to what might be super helpful to
> include. I mean, I have a pretty good layout and all, but if you
> wanted to point me to some threads that tend to crop up in the mailing
> list and IRC channel from relative newcomers that I might be able to
> nip in the bud, I would like to. I'm addressing the stuff that _I_
> hear a lot, and I'm scanning the IRC logs and list for topics, but I
> figured many of you must answer the same questions all the time, too.
Perhaps you're able to share this layout? Whilst I am not about to go
trawling through the archives, one thing I do know that comes up a
lot, isn't so much how to use git to mimick coming from another SCM,
but more workflows -- that's the big stumbling point for people using
Git. I know I'd like to see that aspect in a Git book heavily
addressed.
-- Thomas Adam
^ permalink raw reply
* Git Books
From: Scott Chacon @ 2008-12-06 11:58 UTC (permalink / raw)
To: git list
Hey all,
I have been talked into helping write a real, paper-based book on Git
for a publisher big enough that you may even see it in your local
Borders or whatnot. (And, it appears that Junio has been as well:
http://gitster.livejournal.com/21616.html)
So, since I'm near the beginning of this process, I was wondering if
the group had any feedback as to what might be super helpful to
include. I mean, I have a pretty good layout and all, but if you
wanted to point me to some threads that tend to crop up in the mailing
list and IRC channel from relative newcomers that I might be able to
nip in the bud, I would like to. I'm addressing the stuff that _I_
hear a lot, and I'm scanning the IRC logs and list for topics, but I
figured many of you must answer the same questions all the time, too.
Thanks,
Scott
^ permalink raw reply
* Re: gitk: 'show origin of this line' barfs if gitk run in subdir
From: Paul Mackerras @ 2008-12-06 10:56 UTC (permalink / raw)
To: Mark Burton; +Cc: Git Mailing List, Shawn O. Pearce
In-Reply-To: <20081203222258.6db9b34e@crow>
Mark Burton writes:
> I think the same problem exists with the 'run git gui blame on this
> line' feature. It doesn't work when gitk is started in a subdir of the
> working tree but it does work when gitk is started in the top-level
> directory.
I have checked in a fix into my gitk.git repository at
git://git.kernel.org/pub/scm/gitk/gitk.git. However, there seems to
be a bug in git gui blame; it gives a "list index out of range" error
sometimes.
Shawn, to see an example of the error, get a current kernel tree and
do:
$ cd arch/powerpc
$ git gui blame --line=1183 2c5e76158fcea6e3b9536a74efa7b5e2e846d374 \
../../net/sunrpc/svcsock.c
Paul.
^ permalink raw reply
* Re: Casual observers (was Re: Git weekly news: 2008-49)
From: Boyd Stephen Smith Jr. @ 2008-12-06 6:52 UTC (permalink / raw)
To: git list; +Cc: Jean-Rene David
In-Reply-To: <20081206035307.GE24463@lefuneste.homelinux.org>
[-- Attachment #1: Type: text/plain, Size: 643 bytes --]
On Friday 2008 December 05 21:53, Jean-Rene David wrote:
> I, for one, don't feel comfortable asking beginner
> question here (perhaps wrongly).
Bah, I'll answer them.
I don't think my question that I joined the list to get an answer for was a
beginner question (maybe; I'm rarely a good judge of my skills), but (since
there's no other list) I feel justified asking and answering questions here.
--
Boyd Stephen Smith Jr. ,= ,-_-. =.
bss03@volumehost.net ((_/)o o(\_))
ICQ: 514984 YM/AIM: DaTwinkDaddy `-'(. .)`-'
http://iguanasuicide.org/ \_/
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: Casual observers (was Re: Git weekly news: 2008-49)
From: Edward Z. Yang @ 2008-12-06 5:53 UTC (permalink / raw)
To: git
In-Reply-To: <20081206035307.GE24463@lefuneste.homelinux.org>
Jean-Rene David wrote:
> I, for one, don't feel comfortable asking beginner
> question here (perhaps wrongly).
The #git channel on freenode.net is a great place to ask questions like
that; there's almost always someone around, and you get instant feedback
too. :-)
Cheers,
Edward
^ permalink raw reply
* Casual observers (was Re: Git weekly news: 2008-49)
From: Jean-Rene David @ 2008-12-06 3:53 UTC (permalink / raw)
To: git list
In-Reply-To: <7v3ah2z6jh.fsf@gitster.siamese.dyndns.org>
* Junio C Hamano [2008.12.05 16:00]:
> "Git Traffic" was great because it attempted to
> directly address the issue that the traffic on
> the mailing list was simply too high (and still
> is) to follow for casual observers. It did so
> by giving a comprehensive summary of what
> important topics were discussed
> [...]
While I agree having a summary of the discussions
that took place on the list is great (and I would
read it), what I would really like is a
'git-users' list.
The traffic here is very developper-oriented and
there isn't much room for casual, beginner-level
discussion on the use of git. A lot of projects
work that way and I think it's a really useful
segregation. And it's really not a lot of work to
setup...
I, for one, don't feel comfortable asking beginner
question here (perhaps wrongly).
My two cents.
--
JR
^ permalink raw reply
* Re: [PATCH 5/6] builtin-commit.c: further refactor branch switching
From: Linus Torvalds @ 2008-12-06 2:08 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <1228528455-3089-6-git-send-email-gitster@pobox.com>
The whole series looks good, I think.
But one thing I reacted to is exemplified best by this one:
On Fri, 5 Dec 2008, Junio C Hamano wrote:
>
> +int reset_index_and_worktree(int force, int merge, int quiet, int *wt_error,
> + struct commit *old_commit, const char *old_label,
> + struct commit *new_commit, const char *new_label)
This takes three flags (force, merge, quiet) as three parameters.
And in the series, 3/6 and 4/6 had other cases where the same flags (just
not _all_ of them) were passed around to other functions. In 4/6, you had
"switch_trees()" taking "int merge, int quiet", and in 3/6 you had
"reset_tree()" taking "int quiet" and also have a "int worktree" flag.
I think it would be really nice to have a unified namespace for these
flags. There are lots of commonalities between "checkout" and "reset" (and
certainly differences too), wouldn't it be nice to have something like
#define CO_FORCE 0x001
#define CO_MERGE 0x002
#define CO_QUIET 0x004
#define CO_WORKTREE 0x008
and just pass in a single "unsigned int checkout_flags" as an argument?
Having eight parameters is obscene.
Linus
^ permalink raw reply
* Re: [PATCH] t9129: Prevent test failure if no UTF-8 locale
From: Junio C Hamano @ 2008-12-06 2:05 UTC (permalink / raw)
To: applehq; +Cc: git
In-Reply-To: <20081206013152.GA6129@cumin>
applehq <theappleman@gmail.com> writes:
> Commit 16fc08e2d86dad152194829d21bc55b2ef0c8fb1 introduced a
> test that failed if the en_US.UTF-8 locale was not installed.
>
> Make the test find a UTF-8 locale, and expect failure.
NAK on the latter one.
test_expect_failure does not mean "This might, and it is Ok to, fail", as
you seem to think. It means "This should succeed if our software is not
buggy, but there is a known breakage to cause it to fail, so this test is
marked as such until the bug is fixed."
Skipping the test on a platform that lacks necessary locale is fine, but
if you run the test, they should expect success. Otherwise you cannot
tell if it is a platform issue (i.e. not having any UTF-8 locale) or
a bug in the software (i.e. git-svn not working as expected).
> Signed-off-by: applehq <theappleman@gmail.com>
Sign off with an unreal name, hmm..., what good would that give us...?
> @@ -15,8 +15,9 @@ compare_git_head_with () {
> }
>
> compare_svn_head_with () {
> - LC_ALL=en_US.UTF-8 svn log --limit 1 `git svn info --url` | \
> - sed -e 1,3d -e "/^-\{1,\}\$/d" >current &&
> + LC_ALL=`locale -a | grep -i utf | head -1` \
> + svn log --limit 1 `git svn info --url` | \
> + sed -e 1,3d -e "/^-\{1,\}\$/d" >current &&
I think what this part tries to do is good, in that we do not care if it
is en_US UTF-8 or any other UTF-8. But cramming that logic all in one
pipeline (and an inefficient one at that) makes it harder to read.
Do something like this upfront:
a_utf8_locale=$(locale -a | sed -ne '/[Uu][Tt][Ff]-*8/{
p
q
'})
and if that variable is empty then skip the test that relies on having a
UTF-8 locale on the platform. Then the function can say:
LC_ALL="$a_utf8_locale" svn log --limit 1 ...
^ permalink raw reply
* [PATCH 6/6] builtin-reset.c: use reset_index_and_worktree()
From: Junio C Hamano @ 2008-12-06 1:54 UTC (permalink / raw)
To: git
In-Reply-To: <1228528455-3089-6-git-send-email-gitster@pobox.com>
This makes "git reset --merge" to use the same underlying mechanism "git
checkout" uses to update the index and the work tree.
It is possible to make it use the 3-way merge fallback "git checkout -m"
allows, but this commit does not go there yet.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin-checkout.c | 1 +
builtin-reset.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++-
reset.h | 11 ++++++++
3 files changed, 76 insertions(+), 2 deletions(-)
create mode 100644 reset.h
diff --git a/builtin-checkout.c b/builtin-checkout.c
index a08941a..d196521 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -16,6 +16,7 @@
#include "blob.h"
#include "xdiff-interface.h"
#include "ll-merge.h"
+#include "reset.h"
static const char * const checkout_usage[] = {
"git checkout [options] <branch>",
diff --git a/builtin-reset.c b/builtin-reset.c
index c0cb915..481a1cc 100644
--- a/builtin-reset.c
+++ b/builtin-reset.c
@@ -18,6 +18,7 @@
#include "tree.h"
#include "branch.h"
#include "parse-options.h"
+#include "reset.h"
static const char * const git_reset_usage[] = {
"git reset [--mixed | --soft | --hard | --merge] [-q] [<commit>]",
@@ -52,7 +53,7 @@ static inline int is_merge(void)
return !access(git_path("MERGE_HEAD"), F_OK);
}
-static int reset_index_file(const unsigned char *sha1, int reset_type, int quiet)
+static int reset_index_file_via_read_tree(const unsigned char *sha1, int reset_type, int quiet)
{
int i = 0;
const char *args[6];
@@ -77,6 +78,67 @@ static int reset_index_file(const unsigned char *sha1, int reset_type, int quiet
return run_command_v_opt(args, RUN_GIT_CMD);
}
+static int reset_index_file(struct commit *new, int reset_type, int quiet)
+{
+ /*
+ * SOFT reset won't even touch index nor work tree so
+ * this function is not called.
+ * MIXED updates the index only (should have been called
+ * --cached), and we let "git read-tree" to do so.
+ * HARD and MERGE corresponds to "checkout -f" and "checkout [-m]"
+ */
+ int merge, wt_error, ret;
+ struct commit *old;
+ unsigned char head_sha1[20];
+ unsigned char *new_sha1 = new->object.sha1;
+ struct lock_file *lock_file;
+ int newfd;
+
+ /*
+ * We play lazy and let read-tree complain if HEAD is not
+ * readable. Also on hard reset, HEAD does not have to be
+ * readable.
+ */
+ if (reset_type == MIXED ||
+ reset_type == HARD ||
+ get_sha1("HEAD", head_sha1) ||
+ !(old = lookup_commit_reference_gently(head_sha1, 1)))
+ return reset_index_file_via_read_tree(new_sha1, reset_type,
+ quiet);
+
+ lock_file = xcalloc(1, sizeof(struct lock_file));
+ newfd = hold_locked_index(lock_file, 1);
+ if (read_cache() < 0) {
+ rollback_lock_file(lock_file);
+ return reset_index_file_via_read_tree(new_sha1, reset_type,
+ quiet);
+ }
+
+ /*
+ * If we want "checkout -m" behaviour of falling back to
+ * the 3-way content level merges, we could use
+ *
+ * merge = (reset_type == MERGE);
+ *
+ * here.
+ */
+ merge = 0;
+
+ wt_error = 0;
+ ret = reset_index_and_worktree(0, merge, quiet, &wt_error,
+ old, "local",
+ new, "reset");
+ if (ret || wt_error) {
+ rollback_lock_file(lock_file);
+ return -1;
+ }
+
+ if (write_cache(newfd, active_cache, active_nr) ||
+ commit_locked_index(lock_file))
+ return error("unable to write new index file");
+ return 0;
+}
+
static void print_new_head_line(struct commit *commit)
{
const char *hex, *body;
@@ -276,7 +338,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
if (is_merge() || read_cache() < 0 || unmerged_cache())
die("Cannot do a soft reset in the middle of a merge.");
}
- else if (reset_index_file(sha1, reset_type, quiet))
+ else if (reset_index_file(commit, reset_type, quiet))
die("Could not reset index file to revision '%s'.", rev);
/* Any resets update HEAD to the head being switched to,
diff --git a/reset.h b/reset.h
new file mode 100644
index 0000000..9c42838
--- /dev/null
+++ b/reset.h
@@ -0,0 +1,11 @@
+#ifndef RESET_H
+#define RESET_H
+
+extern int reset_index_and_worktree(int force, int merge, int quiet,
+ int *wt_error,
+ struct commit *old_commit,
+ const char *old_label,
+ struct commit *new_commit,
+ const char *new_label);
+
+#endif
--
1.6.1.rc1.72.ga5ce6
^ permalink raw reply related
* [PATCH 4/6] builtin-checkout.c: refactor merge_working_tree()
From: Junio C Hamano @ 2008-12-06 1:54 UTC (permalink / raw)
To: git
In-Reply-To: <1228528455-3089-4-git-send-email-gitster@pobox.com>
The logic to bring the index and the working tree from one commit to
another is a nontrivial amount of code in this function. Separate it out
into its own function, so that other callers can call it.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin-checkout.c | 172 +++++++++++++++++++++++++++-------------------------
1 files changed, 89 insertions(+), 83 deletions(-)
diff --git a/builtin-checkout.c b/builtin-checkout.c
index d88fce2..9c45c49 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -352,6 +352,87 @@ static int reset_tree(struct tree *tree, int quiet, int worktree, int *wt_error)
}
}
+static int switch_trees(int merge, int quiet, int *wt_error,
+ struct commit *old_commit, const char *old_label,
+ struct commit *new_commit, const char *new_label)
+{
+ int ret;
+ struct tree_desc trees[2];
+ struct tree *tree;
+ struct unpack_trees_options topts;
+
+ memset(&topts, 0, sizeof(topts));
+ topts.head_idx = -1;
+ topts.src_index = &the_index;
+ topts.dst_index = &the_index;
+
+ topts.msgs.not_uptodate_file = "You have local changes to '%s'; cannot switch branches.";
+
+ refresh_cache(REFRESH_QUIET);
+
+ if (unmerged_cache()) {
+ error("you need to resolve your current index first");
+ return 1;
+ }
+
+ /* 2-way merge to the new branch */
+ topts.initial_checkout = is_cache_unborn();
+ topts.update = 1;
+ topts.merge = 1;
+ topts.gently = merge;
+ topts.verbose_update = !quiet;
+ topts.fn = twoway_merge;
+ topts.dir = xcalloc(1, sizeof(*topts.dir));
+ topts.dir->show_ignored = 1;
+ topts.dir->exclude_per_dir = ".gitignore";
+ tree = parse_tree_indirect(old_commit->object.sha1);
+ init_tree_desc(&trees[0], tree->buffer, tree->size);
+ tree = parse_tree_indirect(new_commit->object.sha1);
+ init_tree_desc(&trees[1], tree->buffer, tree->size);
+
+ ret = unpack_trees(2, trees, &topts);
+ if (ret == -1) {
+ /*
+ * Unpack couldn't do a trivial merge; either give up
+ * or do a real merge, depending on whether the merge
+ * flag was used.
+ */
+ struct tree *result;
+ struct tree *work;
+ struct merge_options o;
+ if (!merge)
+ return 1;
+ parse_commit(old_commit);
+
+ /* Do more real merge */
+
+ /*
+ * We update the index fully, then write the tree from
+ * the index, then merge the new branch with the
+ * current tree, with the old branch as the base. Then
+ * we reset the index (but not the working tree) to
+ * the new branch, leaving the working tree as the
+ * merged version, but skipping unmerged entries in
+ * the index.
+ */
+
+ add_files_to_cache(NULL, NULL, 0);
+ init_merge_options(&o);
+ o.verbosity = 0;
+ work = write_tree_from_memory(&o);
+
+ ret = reset_tree(new_commit->tree, quiet, 1, wt_error);
+ if (ret)
+ return ret;
+ o.branch1 = new_label;
+ o.branch2 = old_label;
+ merge_trees(&o, new_commit->tree, work,
+ old_commit->tree, &result);
+ ret = reset_tree(new_commit->tree, quiet, 0, wt_error);
+ }
+ return ret;
+}
+
struct branch_info {
const char *name; /* The short name used */
const char *path; /* The full name of a real branch */
@@ -376,91 +457,16 @@ static int merge_working_tree(struct checkout_opts *opts,
if (read_cache() < 0)
return error("corrupt index file");
- if (opts->force) {
+ if (opts->force)
ret = reset_tree(new->commit->tree, opts->quiet, 1,
&opts->writeout_error);
- if (ret)
- return ret;
- } else {
- struct tree_desc trees[2];
- struct tree *tree;
- struct unpack_trees_options topts;
-
- memset(&topts, 0, sizeof(topts));
- topts.head_idx = -1;
- topts.src_index = &the_index;
- topts.dst_index = &the_index;
-
- topts.msgs.not_uptodate_file = "You have local changes to '%s'; cannot switch branches.";
-
- refresh_cache(REFRESH_QUIET);
-
- if (unmerged_cache()) {
- error("you need to resolve your current index first");
- return 1;
- }
-
- /* 2-way merge to the new branch */
- topts.initial_checkout = is_cache_unborn();
- topts.update = 1;
- topts.merge = 1;
- topts.gently = opts->merge;
- topts.verbose_update = !opts->quiet;
- topts.fn = twoway_merge;
- topts.dir = xcalloc(1, sizeof(*topts.dir));
- topts.dir->show_ignored = 1;
- topts.dir->exclude_per_dir = ".gitignore";
- tree = parse_tree_indirect(old->commit->object.sha1);
- init_tree_desc(&trees[0], tree->buffer, tree->size);
- tree = parse_tree_indirect(new->commit->object.sha1);
- init_tree_desc(&trees[1], tree->buffer, tree->size);
-
- ret = unpack_trees(2, trees, &topts);
- if (ret == -1) {
- /*
- * Unpack couldn't do a trivial merge; either
- * give up or do a real merge, depending on
- * whether the merge flag was used.
- */
- struct tree *result;
- struct tree *work;
- struct merge_options o;
- if (!opts->merge)
- return 1;
- parse_commit(old->commit);
-
- /* Do more real merge */
-
- /*
- * We update the index fully, then write the
- * tree from the index, then merge the new
- * branch with the current tree, with the old
- * branch as the base. Then we reset the index
- * (but not the working tree) to the new
- * branch, leaving the working tree as the
- * merged version, but skipping unmerged
- * entries in the index.
- */
-
- add_files_to_cache(NULL, NULL, 0);
- init_merge_options(&o);
- o.verbosity = 0;
- work = write_tree_from_memory(&o);
-
- ret = reset_tree(new->commit->tree, opts->quiet, 1,
- &opts->writeout_error);
- if (ret)
- return ret;
- o.branch1 = new->name;
- o.branch2 = "local";
- merge_trees(&o, new->commit->tree, work,
- old->commit->tree, &result);
- ret = reset_tree(new->commit->tree, opts->quiet, 0,
- &opts->writeout_error);
- if (ret)
- return ret;
- }
- }
+ else
+ ret = switch_trees(opts->merge, opts->quiet,
+ &opts->writeout_error,
+ old->commit, "local",
+ new->commit, new->name);
+ if (ret)
+ return ret;
if (write_cache(newfd, active_cache, active_nr) ||
commit_locked_index(lock_file))
--
1.6.1.rc1.72.ga5ce6
^ permalink raw reply related
* [PATCH 5/6] builtin-commit.c: further refactor branch switching
From: Junio C Hamano @ 2008-12-06 1:54 UTC (permalink / raw)
To: git
In-Reply-To: <1228528455-3089-5-git-send-email-gitster@pobox.com>
This adds reset_index_and_worktree() to further factor out the logic for
switching the index and the work tree state from one commit to another to
shrink merge_working_tree() function.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin-checkout.c | 24 ++++++++++++++++--------
1 files changed, 16 insertions(+), 8 deletions(-)
diff --git a/builtin-checkout.c b/builtin-checkout.c
index 9c45c49..a08941a 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -433,6 +433,18 @@ static int switch_trees(int merge, int quiet, int *wt_error,
return ret;
}
+int reset_index_and_worktree(int force, int merge, int quiet, int *wt_error,
+ struct commit *old_commit, const char *old_label,
+ struct commit *new_commit, const char *new_label)
+{
+ if (force)
+ return reset_tree(new_commit->tree, quiet, 1, wt_error);
+ else
+ return switch_trees(merge, quiet, wt_error,
+ old_commit, old_label,
+ new_commit, new_label);
+}
+
struct branch_info {
const char *name; /* The short name used */
const char *path; /* The full name of a real branch */
@@ -457,14 +469,10 @@ static int merge_working_tree(struct checkout_opts *opts,
if (read_cache() < 0)
return error("corrupt index file");
- if (opts->force)
- ret = reset_tree(new->commit->tree, opts->quiet, 1,
- &opts->writeout_error);
- else
- ret = switch_trees(opts->merge, opts->quiet,
- &opts->writeout_error,
- old->commit, "local",
- new->commit, new->name);
+ ret = reset_index_and_worktree(opts->force, opts->merge, opts->quiet,
+ &opts->writeout_error,
+ old->commit, "local",
+ new->commit, new->name);
if (ret)
return ret;
--
1.6.1.rc1.72.ga5ce6
^ permalink raw reply related
* [PATCH 2/6] read-cache.c: typofix in comment
From: Junio C Hamano @ 2008-12-06 1:54 UTC (permalink / raw)
To: git
In-Reply-To: <1228528455-3089-2-git-send-email-gitster@pobox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
read-cache.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/read-cache.c b/read-cache.c
index 22a8143..58d3f88 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1486,7 +1486,7 @@ int write_index(const struct index_state *istate, int newfd)
/*
* Read the index file that is potentially unmerged into given
- * index_state, dropping any unmerged entries. Returns true is
+ * index_state, dropping any unmerged entries. Returns true if
* the index is unmerged. Callers who want to refuse to work
* from an unmerged state can call this and check its return value,
* instead of calling read_cache().
--
1.6.1.rc1.72.ga5ce6
^ permalink raw reply related
* [PATCH 1/6] builtin-checkout.c: check error return from read_cache()
From: Junio C Hamano @ 2008-12-06 1:54 UTC (permalink / raw)
To: git
In-Reply-To: <1228528455-3089-1-git-send-email-gitster@pobox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin-checkout.c | 7 +++++--
1 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/builtin-checkout.c b/builtin-checkout.c
index 7f3bd7b..c2c0561 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -228,7 +228,8 @@ static int checkout_paths(struct tree *source_tree, const char **pathspec,
struct lock_file *lock_file = xcalloc(1, sizeof(struct lock_file));
newfd = hold_locked_index(lock_file, 1);
- read_cache();
+ if (read_cache() < 0)
+ return error("corrupt index file");
if (source_tree)
read_tree_some(source_tree, pathspec);
@@ -371,7 +372,9 @@ static int merge_working_tree(struct checkout_opts *opts,
int ret;
struct lock_file *lock_file = xcalloc(1, sizeof(struct lock_file));
int newfd = hold_locked_index(lock_file, 1);
- read_cache();
+
+ if (read_cache() < 0)
+ return error("corrupt index file");
if (opts->force) {
ret = reset_tree(new->commit->tree, opts, 1);
--
1.6.1.rc1.72.ga5ce6
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox