* Re: [PATCH 0/6] Gitweb caching changes v2
From: Jakub Narebski @ 2009-12-11 15:51 UTC (permalink / raw)
To: John 'Warthog9' Hawley; +Cc: git, John 'Warthog9' Hawley
In-Reply-To: <1260488743-25855-1-git-send-email-warthog9@kernel.org>
"John 'Warthog9' Hawley" <warthog9@kernel.org> writes:
> Evening everyone,
>
> This is the latest incarnation of gitweb w/ caching. This is
> finally at the point where it should probably start either being
> considered for inclusion or mainline, or I need to accept that this
> will never get in and more perminantely fork (as is the case with
> Fedora where this is going in as gitweb-caching as a parrallel rpm
> package).
>
> That said this brings the base up to mainline (again), it updates a
> number of elements in the caching engine, and this is a much cleaner
> break-out of the tree vs. what I am currently developing against.
>
> New things known to work:
> - Better breakout
> - You can actually disable the cache now
>
> - John 'Warthog9' Hawley
>
> John 'Warthog9' Hawley (6):
> GITWEB - Load Checking
> GITWEB - Missmatching git w/ gitweb
> GITWEB - Add git:// link to summary pages
> GITWEB - Makefile changes
> GITWEB - File based caching layer
This patch didn't made it to git mailing list. I suspect that you ran
afoul vger anti-SPAM filter.
Does this "File based caching layer" have anything common with GSoC
2008 project, available at git://repo.or.cz/git/gitweb-caching.git ?
> GITWEB - Separate defaults from main file
--
Jakub Narebski
Poland
ShadeHawk on #git
^ permalink raw reply
* Re: [PATCH 6/6] GITWEB - Separate defaults from main file
From: J.H. @ 2009-12-11 15:58 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git, John 'Warthog9' Hawley
In-Reply-To: <m3ljh9cy3b.fsf@localhost.localdomain>
>> This is also a not-so-subtle start of trying to break up gitweb into
>> separate files for easier maintainability, having everything in a
>> single file is just a mess and makes the whole thing more complicated
>> than it needs to be. This is a bit of a baby step towards breaking it
>> up for easier maintenance.
>
> The question is if easier maintenance and development by spliting
> gitweb for developers offsets ease of install for users.
This would just get dropped into the same location that gitweb.cgi
exists in, there is no real difference in installation, and thus I can't
see this as an issue for users.
>
>> Signed-off-by: John 'Warthog9' Hawley <warthog9@eaglescrag.net>
>
> Signoff mismatch.
>
>> ---
>> .gitignore | 1 +
>> Makefile | 15 +-
>> gitweb/Makefile | 2 +-
>> gitweb/gitweb.perl | 515 +++++--------------------------------------
>> gitweb/gitweb_defaults.perl | 468 +++++++++++++++++++++++++++++++++++++++
>> 5 files changed, 537 insertions(+), 464 deletions(-)
>> create mode 100644 gitweb/gitweb_defaults.perl
>>
>>
>> diff --git a/.gitignore b/.gitignore
>> index ac02a58..5e48102 100644
>> --- a/.gitignore
>> +++ b/.gitignore
>> @@ -151,6 +151,7 @@
>> /git-core-*/?*
>> /gitk-git/gitk-wish
>> /gitweb/gitweb.cgi
>> +/gitweb/gitweb_defaults.pl
>
> Hmmm... gitweb/gitweb_defaults.perl as source file, and
> gitweb/gitweb_defaults.pl as generated file? Wouldn't it be better to
> go with the convention used elsewhere in gitweb and use
> gitweb/gitweb_defaults.perl.in or gitweb/gitweb_defaults.pl.in as
> source file?
I think you got confused, the committed file is .perl the generated file
is .pl.
>> + #$(QUIET_GEN)$(RM) $@ $@+ &&
>
> What this line is about?
Cruft, thought I had deleted and excluded it, won't be there in next
version.
>
>> $(QUIET_GEN)$(RM) $@ $@+ && \
>> sed -e '1s|#!.*perl|#!$(PERL_PATH_SQ)|' \
>> -e 's|++GIT_VERSION++|$(GIT_VERSION)|g' \
>> @@ -1539,7 +1541,7 @@ endif
>> -e 's|++GITWEB_JS++|$(GITWEB_JS)|g' \
>> -e 's|++GITWEB_SITE_HEADER++|$(GITWEB_SITE_HEADER)|g' \
>> -e 's|++GITWEB_SITE_FOOTER++|$(GITWEB_SITE_FOOTER)|g' \
>> - $(patsubst %.cgi,%.perl,$@) >$@+ && \
>> + $(patsubst %.cgi,%.perl,$(patsubst %.pl, %.perl, $@)) >$@+ && \
>
> Why the slightly inconsistent style ("%.cgi,%perl" vs "%.pl, %perl")?
Considering that the defaults is more of an include vs. a cgi it
probably shouldn't share the standard expected executable suffix, thus I
used .pl. Could just as easily change it to .pm, or something else but
I think it would make the most sense to leave things we are expecting
the webserver to directly execute as .cgi, and includes as a different
suffix.
> Also wouldn't all replacements be in the new gitweb_defaults file, so
> there would be no need then to do replacements for gitweb.cgi?
Not all replacements are done in one or the other, and since it's
basically a NOP to perform the full set of replacements on both files
that seemed the easiest way to ensure they were done in both places.
> Oh, I see there is at least one that stayed in gitweb.perl: $version
>
<snip>
>> +# Define and than setup our configuration
>> +#
>> +our(
>> + $VERSION,
>> + $path_info,
>> + $GIT,
>> + $projectroot,
>> + $project_maxdepth,
>> + $home_link,
>> + $home_link_str,
>> + $site_name,
>> + $site_header,
>> + $home_text,
>> + $site_footer,
>> + @stylesheets,
>> + $stylesheet,
>> + $logo,
>> + $favicon,
>> + $javascript,
>> + $logo_url,
>> + $logo_label,
>> + $projects_list,
>> + $projects_list_description_width,
>> + $default_projects_order,
>> + $export_ok,
>> + $export_auth_hook,
>> + $strict_export,
>> + @git_base_url_list,
>> + $default_blob_plain_mimetype,
>> + $default_text_plain_charset,
>> + $mimetypes_file,
>> + $missmatch_git,
>> + $gitlinkurl,
>> + $maxload,
>> + $cache_enable,
>> + $minCacheTime,
>> + $maxCacheTime,
>> + $cachedir,
>> + $backgroundCache,
>> + $nocachedata,
>> + $nocachedatabin,
>> + $fullhashpath,
>> + $fullhashbinpath,
>> + $export_auth_hook,
>> + %known_snapshot_format_aliases,
>> + %known_snapshot_formats,
>> + $path_info,
>> + $fallback_encoding,
>> + %avatar_size,
>> + $project_maxdepth,
>> + $headerRefresh,
>> + $base_url,
>> + $projects_list_description_width,
>> + $default_projects_order,
>> + $prevent_xss,
>> + @diff_opts,
>> + %feature
>> );
>
> Why this block is required? Why not have variables defined (using
> "our") in gitweb_defaults file?
Wanted to make sure things were properly defined, if in an unexpected
state, should a user have gitweb.cgi in place but not the defaults.
>
> [cut deletion]
>
>> +do 'gitweb_defaults.pl';
>>
>> sub gitweb_get_feature {
>> my ($name) = @_;
>> diff --git a/gitweb/gitweb_defaults.perl b/gitweb/gitweb_defaults.perl
>> new file mode 100644
>> index 0000000..ede0daf
>> --- /dev/null
>> +++ b/gitweb/gitweb_defaults.perl
>> @@ -0,0 +1,468 @@
>> +# gitweb - simple web interface to track changes in git repositories
>> +#
>> +# (C) 2005-2006, Kay Sievers <kay.sievers@vrfy.org>
>> +# (C) 2005, Christian Gierke
>> +#
>> +# This program is licensed under the GPLv2
>> +
>> +# Base URL for relative URLs in gitweb ($logo, $favicon, ...),
>> +# needed and used only for URLs with nonempty PATH_INFO
>> +$base_url = $my_url;
>
> Why not "our $base_url = $my_url;"?
same reason as the other 'our' includes above, though why this ended up
as a separate patch vs. the rest of the file I don't know.
- John 'Warthog9' Hawley
^ permalink raw reply
* Re: [PATCH 4/6] GITWEB - Makefile changes
From: J.H. @ 2009-12-11 16:22 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git
In-Reply-To: <m3pr6ld1p2.fsf@localhost.localdomain>
<snip>
> IMPORTANT!
>
> A note about this change: I think it would be better to move creating
> gitweb.cgi (and optionally gitweb.min.js) to gitweb/Makefile, and make
> main Makefile call gitweb/Makefile, and not vice versa like in your
> solution.
>
> If it is possible.
It's quite possible, and I'm fine with doing that. If no one has any
objections I can re-work those with the understanding that the build
process for gitweb shift to the gitweb/ directory instead of the main
Makefile.
>
>> diff --git a/Makefile b/Makefile
>> index 4a1e5bc..8db9d01 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -1509,6 +1509,8 @@ $(patsubst %.perl,%,$(SCRIPT_PERL)): % : %.perl
>> chmod +x $@+ && \
>> mv $@+ $@
>>
>> +.PHONY: gitweb
>
> Why it is here, and not with the .PHONY block at line 1924 of
> Makefile? It would be nice to have comment supporting this choice in
> email with this patch (or in commit message).
There are 6 other instances of .PHONY in the makefile, having the .PHONY
localized seemed to make it the most obvious since it was right next to
the actual target.
>
>> +gitweb: gitweb/gitweb.cgi
>> ifdef JSMIN
>> OTHER_PROGRAMS += gitweb/gitweb.cgi gitweb/gitweb.min.js
>> gitweb/gitweb.cgi: gitweb/gitweb.perl gitweb/gitweb.min.js
>> @@ -1537,7 +1539,7 @@ endif
>> -e 's|++GITWEB_JS++|$(GITWEB_JS)|g' \
>> -e 's|++GITWEB_SITE_HEADER++|$(GITWEB_SITE_HEADER)|g' \
>> -e 's|++GITWEB_SITE_FOOTER++|$(GITWEB_SITE_FOOTER)|g' \
>> - $< >$@+ && \
>> + $(patsubst %.cgi,%.perl,$@) >$@+ && \
>
> Why this change?
Preparation for a later change. The change could happen all at the same
time if it makes more logical sense.
>
>> chmod +x $@+ && \
>> mv $@+ $@
>>
>> diff --git a/gitweb/Makefile b/gitweb/Makefile
>> new file mode 100644
>> index 0000000..8d318b3
>> --- /dev/null
>> +++ b/gitweb/Makefile
>> @@ -0,0 +1,14 @@
>> +SHELL = /bin/bash
>
> Why is this needed?
>
>> +
>> +FILES = gitweb.cgi
>> +
>> +.PHONY: $(FILES)
>
> Why .PHONY? $(FILES) are created.
From this makefile I wanted to explicitly call up to the main makefile
no matter what, the main makefile doesn't consider the targets .PHONY
and it has all the dependencies that it would expect.
>> +
>> +all: $(FILES)
>> +
>> +$(FILES):
>> + $(MAKE) $(MFLAGS) -C ../ -f Makefile gitweb/$@
>> +
>> +clean:
>> + rm -rf $(FILES)
>> +
^ permalink raw reply
* Re: [PATCH 4/6] GITWEB - Makefile changes
From: Jakub Narebski @ 2009-12-11 16:41 UTC (permalink / raw)
To: J.H.; +Cc: git
In-Reply-To: <4B2271B4.2010301@kernel.org>
On Fri, 11 Dec 2009, J.H. wrote:
>> IMPORTANT!
>>
>> A note about this change: I think it would be better to move creating
>> gitweb.cgi (and optionally gitweb.min.js) to gitweb/Makefile, and make
>> main Makefile call gitweb/Makefile, and not vice versa like in your
>> solution.
>>
>> If it is possible.
>
> It's quite possible, and I'm fine with doing that. If no one has any
> objections I can re-work those with the understanding that the build
> process for gitweb shift to the gitweb/ directory instead of the main
> Makefile.
In my opinion it would be better solution because it would reduce size
of main (master) Makefile, and not be much larger than this solution.
git-gui/, Documentation/, perl/ all have their own makefiles, which do
the work, and are called from main (master) Makefile.
>>> diff --git a/Makefile b/Makefile
>>> index 4a1e5bc..8db9d01 100644
>>> --- a/Makefile
>>> +++ b/Makefile
>>> @@ -1509,6 +1509,8 @@ $(patsubst %.perl,%,$(SCRIPT_PERL)): % : %.perl
>>> chmod +x $@+ && \
>>> mv $@+ $@
>>>
>>> +.PHONY: gitweb
>>
>> Why it is here, and not with the .PHONY block at line 1924 of
>> Makefile? It would be nice to have comment supporting this choice in
>> email with this patch (or in commit message).
>
> There are 6 other instances of .PHONY in the makefile, having the .PHONY
> localized seemed to make it the most obvious since it was right next to
> the actual target.
I was thinking here about this large block of .PHONY declarations,
the one which is not inside conditional.
>>> +gitweb: gitweb/gitweb.cgi
>>> ifdef JSMIN
>>> OTHER_PROGRAMS += gitweb/gitweb.cgi gitweb/gitweb.min.js
>>> gitweb/gitweb.cgi: gitweb/gitweb.perl gitweb/gitweb.min.js
>>> @@ -1537,7 +1539,7 @@ endif
>>> -e 's|++GITWEB_JS++|$(GITWEB_JS)|g' \
>>> -e 's|++GITWEB_SITE_HEADER++|$(GITWEB_SITE_HEADER)|g' \
>>> -e 's|++GITWEB_SITE_FOOTER++|$(GITWEB_SITE_FOOTER)|g' \
>>> - $< >$@+ && \
>>> + $(patsubst %.cgi,%.perl,$@) >$@+ && \
>>
>> Why this change?
>
> Preparation for a later change. The change could happen all at the same
> time if it makes more logical sense.
Please at least describe this change in commit message.
But I think it could be moved to other patch, or put in separate patch.
This change has nothing to do with easier gitweb generation.
>>> chmod +x $@+ && \
>>> mv $@+ $@
>>>
>>> diff --git a/gitweb/Makefile b/gitweb/Makefile
>>> new file mode 100644
>>> index 0000000..8d318b3
>>> --- /dev/null
>>> +++ b/gitweb/Makefile
>>> @@ -0,0 +1,14 @@
>>> +SHELL = /bin/bash
>>
>> Why is this needed?
Why do you need to define SHELL?
>>> +
>>> +FILES = gitweb.cgi
>>> +
>>> +.PHONY: $(FILES)
>>
>> Why .PHONY? $(FILES) are created.
>
> From this makefile I wanted to explicitly call up to the main makefile
> no matter what, the main makefile doesn't consider the targets .PHONY
> and it has all the dependencies that it would expect.
What is the reason of this phony .PHONY? If gitweb.cgi is newer than
gitweb.perl (and other sources), then without .PHONY it wouldn't be
regenerated. With .PHONY it would call master Makefile... which would
notice that gitweb.cgi is newer than gitweb.perl and do not regenerate.
So what this .PHONY does is unnecessary call make on master Makefile...
But I guess this issue would be moot if it was the other way around,
i.e. master Makefile calling gitweb/Makefile.
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [PATCH 0/6] Gitweb caching changes v2
From: Jakub Narebski @ 2009-12-11 18:01 UTC (permalink / raw)
To: J.H., git
In-Reply-To: <4B226D56.7000004@kernel.org>
On Fri, 11 Dec 2009, J.H. (John 'Warthog9' Hawley) wrote:
> Jakub Narebski wrote:
>> "John 'Warthog9' Hawley" <warthog9@kernel.org> writes:
>>> John 'Warthog9' Hawley (6):
>>> GITWEB - Load Checking
>>> GITWEB - Missmatching git w/ gitweb
>>> GITWEB - Add git:// link to summary pages
>>> GITWEB - Makefile changes
>>> GITWEB - File based caching layer
>>
>> This patch didn't made it to git mailing list. I suspect that you ran
>> afoul vger anti-SPAM filter.
>>
>> Does this "File based caching layer" have anything common with GSoC
>> 2008 project, available at git://repo.or.cz/git/gitweb-caching.git ?
>
> Yeah, it does seem that way (like I said eaten by a grue), it
> *currently* has nothing to do with Lea's GSoC code but it is still my
> intention, long term, to integrate the two.
>
> The patch, in all it's glory can be viewed at:
> http://git.kernel.org/?p=git/warthog9/gitweb.git;a=commitdiff;h=42641b1e3bfae14d5cc2e0150355e89cb87951db
>
> It is anything but a small patch to gitweb, the patch is 117K and
> comprises 3539 lines (including git header commit information). There's
> not any real good way to break it up as it's a bit of an all or nothing
> patch.
First, why do you reinvent the wheel instead of using one of existing
caching interfaces like CHI or Cache::Cache (perhaps creating a custom
backend or middle layer which incorporates required features, like being
load-aware)? This way changing from file-based cache to e.g. mmap based
one or to memcached would be very simple. And you would avoid pitfals
in doing your own cache management. perl-Cache-Cache should be available
package in extras repositories.
If module is no available this would simply mean no caching, like in many
(or not so many) other cases with optional features in gitweb.
Second, if you can't use CGI::Cache directly, you can always steal the
idea from it, then the change to gitweb itself would be minimal:
"Internally, the CGI::Cache module ties the output file descriptor
(usually STDOUT) to an internal variable to which all output is saved."
P.S. I'll postpone critique of the patch itself for now. The above issues
are much more important.
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [PATCH 0/6] Gitweb caching changes v2
From: J.H. @ 2009-12-11 18:26 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git
In-Reply-To: <200912111901.35781.jnareb@gmail.com>
Jakub Narebski wrote:
> On Fri, 11 Dec 2009, J.H. (John 'Warthog9' Hawley) wrote:
>> Jakub Narebski wrote:
>>> "John 'Warthog9' Hawley" <warthog9@kernel.org> writes:
>
>>>> John 'Warthog9' Hawley (6):
>>>> GITWEB - Load Checking
>>>> GITWEB - Missmatching git w/ gitweb
>>>> GITWEB - Add git:// link to summary pages
>>>> GITWEB - Makefile changes
>>>> GITWEB - File based caching layer
>>> This patch didn't made it to git mailing list. I suspect that you ran
>>> afoul vger anti-SPAM filter.
>>>
>>> Does this "File based caching layer" have anything common with GSoC
>>> 2008 project, available at git://repo.or.cz/git/gitweb-caching.git ?
>> Yeah, it does seem that way (like I said eaten by a grue), it
>> *currently* has nothing to do with Lea's GSoC code but it is still my
>> intention, long term, to integrate the two.
>>
>> The patch, in all it's glory can be viewed at:
>> http://git.kernel.org/?p=git/warthog9/gitweb.git;a=commitdiff;h=42641b1e3bfae14d5cc2e0150355e89cb87951db
>>
>> It is anything but a small patch to gitweb, the patch is 117K and
>> comprises 3539 lines (including git header commit information). There's
>> not any real good way to break it up as it's a bit of an all or nothing
>> patch.
>
> First, why do you reinvent the wheel instead of using one of existing
> caching interfaces like CHI or Cache::Cache (perhaps creating a custom
> backend or middle layer which incorporates required features, like being
> load-aware)?
Well for starters this isn't exactly a reinvention of the wheel, and
this isn't something "new" per-se. This code has been actively running
on git.kernel.org for something like 3 - 4 years so there's something to
be said for the devil we know and understand. As well using the other
caching strategies involves adding dramatically more complex
interactions with caching layer. The caching layer is actually quite
specific to how git + gitweb works and solves more than just "caching"
on the surface. Specifically it solves the stampeding herd problem
which would have to be solved either way even if I didn't implement my
own caching, and since I had to do that caching was barely a step beyond
that to implement.
> This way changing from file-based cache to e.g. mmap based
> one or to memcached would be very simple.
True but these are *VERY* different caching strategies than the one I've
got here, yes it's using files as a backend but it's doing so with
specific goals in mind. As I've said I plan to integrate Lea's
memcached based caching into this in the future and that has different
advantages and disadvantages.
At the end of the day the "normal" caching engines aren't as efficient
as mine and there is the case the very high performance sites are going
to have to investigate a number of different solutions to see what works
best for them. Mine is also *dramatically* simpler to setup as well,
turn it on, point it at a directory and your done.
> And you would avoid pitfals
> in doing your own cache management. perl-Cache-Cache should be available
> package in extras repositories.
There's pitfalls if I do it myself, or I use one of the other "common"
perl modules. I did it this way years ago, I've maintained it and it
works pretty well. I won't admit that it's the smartest caching engine
on the planet, far from it, but it has evolved specifically for gitweb
and that itself saves me a lot of pitfalls from cache engine + gitweb
integration.
> If module is no available this would simply mean no caching, like in many
> (or not so many) other cases with optional features in gitweb.
Yes, but as can be seen from how you enable various other caching
engines the setup of those is non-trivial, this is and either way
caching *HAS* to be explicitly turned on by the admin/user since they
are going to have to do *some* configuration, or at least be aware that
their webapp is going to chew up some sort of resource.
> Second, if you can't use CGI::Cache directly, you can always steal the
> idea from it, then the change to gitweb itself would be minimal:
>
> "Internally, the CGI::Cache module ties the output file descriptor
> (usually STDOUT) to an internal variable to which all output is saved."
I thought about that 3 years ago, and decided it wasn't a good option
for gitweb. Why? There's too many assumptions throughout the code that
when you do a print it will go immediately out. Things like error
messages and such. Breaking out the prints into prints (which will do
what is expected) and passing around the output in the $output variables
makes it a lot simpler easier to differentiate about how / what your
looking at and a *LOT* easier to debug.
> P.S. I'll postpone critique of the patch itself for now. The above issues
> are much more important.
That's fine. The issues your raising aren't new though, and stem back
to before I created gitweb-caching, got rehashed with Lea's patches and
not surprisingly are back on the table now. Like I said above, there is
no one caching strategy that's perfect in all cases here and that's
again why I eventually plan to merge Lea's changes (which uses
memcached) in as well, I'm just trying to get code that I'm getting
considerable demand for, that's proven, upstream.
- John 'Warthog9' Hawley
^ permalink raw reply
* FEATURE REQUEST: Announce branch name with merge comamnd
From: Jari Aalto @ 2009-12-11 18:55 UTC (permalink / raw)
To: git
Doing octopus merge results:
$ git merge ...
Trying simple merge with c87c49b1e413e5dc378d7e6b16951761a1e82f6d
Trying simple merge with b650c8c8809ef493ad4128fc941ed6b520c82f28
Trying simple merge with 047f83d6c8a08c4016004780e94257d9e487b7e6
Simple merge did not work, trying automatic merge.
Auto-merging example.jwmrc
Trying simple merge with 16860b016e198acd0814492092e2ad5ea88fb219
Simple merge did not work, trying automatic merge.
Auto-merging example.jwmrc
Trying simple merge with 9a397ff24a381ce49dd093c4f51c06c4c62f3ce7
Simple merge did not work, trying automatic merge.
...
SUGGESTION
Please announce the branch name being merged so that the listing is
easier to follow (possibly only with --verbose, -v option). Add
"Branch: <name>" just before the merge is attempted. somethiglike this
Branch: bug--manpage-fix-hyphen
Trying simple merge with c87c49b1e413e5dc378d7e6b16951761a1e82f6d
Branch: bug--manpage-fix-TH
Trying simple merge with b650c8c8809ef493ad4128fc941ed6b520c82f28
Branch: bug-manpage-change-binary-name
Trying simple merge with 047f83d6c8a08c4016004780e94257d9e487b7e6
Branch: feature--example.jwmrc-change-browser
Simple merge did not work, trying automatic merge.
Auto-merging example.jwmrc
Branch: feature--example.jwmrc-change-clock
Trying simple merge with 16860b016e198acd0814492092e2ad5ea88fb219
Simple merge did not work, trying automatic merge.
Auto-merging example.jwmrc
Branch: feature--example.jwmrc-change-font
Trying simple merge with 9a397ff24a381ce49dd093c4f51c06c4c62f3ce7
Simple merge did not work, trying automatic merge.
...
^ permalink raw reply
* Re: Re: Re: [RFC PATCH v2 0/2] git-gui: (un)stage a range of changes at once
From: Heiko Voigt @ 2009-12-11 18:57 UTC (permalink / raw)
To: Jeff Epler; +Cc: Shawn O. Pearce, git
In-Reply-To: <20091208003836.GB22330@unpythonic.net>
On Mon, Dec 07, 2009 at 06:38:36PM -0600, Jeff Epler wrote:
> On Mon, Dec 07, 2009 at 01:54:35PM +0100, Heiko Voigt wrote:
> > Jeff could you clarify or provide an example?
>
> If I recall correctly, the problem with the v2 patch was when the change
> was like
> @@ -13,8 +13,8 @@ set appvers {@@GITGUI_VERSION@@}
> set copyright [encoding convertfrom utf-8 {
> Copyright © 2006, 2007 Shawn Pearce, et. al.
>
> -This program is free software; you can redistribute it and/or modify
> -it under the terms of the GNU General Public License as published by
> +blah blah
> +blah blah
> the Free Software Foundation; either version 2 of the License, or
> (at your option) any later version.
> and the 'blah blah' lines were both staged in the same operation.
Thanks Jeff that was the missing piece. I was able to reproduce the
behavior and I can confirm it is gone with the new series. I was not yet
able to read through all of the code.
cheers Heiko
^ permalink raw reply
* Re: How to selectively recreate merge state?
From: Junio C Hamano @ 2009-12-11 19:24 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Michael J Gruber, Johannes Sixt, Jay Soffian, git
In-Reply-To: <200912111500.51982.jnareb@gmail.com>
Jakub Narebski <jnareb@gmail.com> writes:
> I have thought that if there exist stage #0 in index, git simply _ignores_
> higher stages, so git-add simply adds stage #0 and does not delete higher
> stages.
Then you thought wrong ;-).
Leaving resolved cruft in the main index (aka active_cache[]) will make
all the normal operation codepath unnecessarily complex. They rely on "if
I see stage #0, there is no higher stages for the same path". And extra
checks will slow things down.
But that does not necessarily mean the index is a wrong place to save away
pre-resolution information on resolved paths (read on).
Before suggesting a possible next move, there are a few things we should
notice while reading ec16779 (Add git-unresolve <paths>..., 2006-04-19):
- This was done about only one year after git was born. You should not
take it granted that the workflow it wanted to support makes sense.
Considering that using "git add" to mark the resolution is to declare
that you are _finished_ with that path, using it for other purposes
(e.g. leaving a note that says "I've looked at and have one possible
resolution in the file in the work tree, but I haven't verified the
result yet", which is what the commit talks about) is simply an
(ab|mis)use of the index. Lossage of higher stage information by this
misuse is user's problem, and there is this thing called pen & pencil
the user can use for taking notes if s/he does not want to lose the
original conflict information from the index.
- Even if we for a moment consider that the workflow made some sense, the
particular implementation is not suitable anymore for today's git.
Again, this was done only one year after git was born, and back then
"pull/merge" were the only things that left conflicts in every day
operations by end users, and not many people didn't expect git to merge
across renames. It was sufficient to read the path the end user asked
for from HEAD and MERGE_HEAD and pretend we "unresolved" in such a
simpler world.
But "merge" is not the primary thing that gives you conflicts anymore.
"rebase", "cherry-pick", "stash apply" are much more widely used by
ordinary users these days than back then, and reading from MERGE_HEAD
wouldn't do any good for recreating what these operations did. Even
with "merge", stages #2 and #3 can come from a totally different path
when using recursive and subtree strategies, so reading from
HEAD/MERGE_HEAD is not as useful as it used to be.
In fact, considering that there are many ways conflicts can be left in the
index and there are only two ways that they are resolved in the index by
the user (and both eventually uses a single function to do so), it would
make perfect sense to do the following:
- Define a new index extension section to record "unresolve"
information.
- Every time add_index_entry_with_check() in read-cache.c records a stage
0 entry while dropping higher stage entries for the same path, record
these higher stage entries to the "unresolve" section.
- An "update-index --unresolve" will use the information from this
"unresolve" extension to recreate the unmerged state.
- "rerere forget" that we earlier talked about in a separate thread will
use exactly the same mechanism to get back the unmerged state to
recompute the conflict identifier (this is why J6t is addded to the Cc:
list).
- "checkout --conflict" _might_ want to also consider unresolving the
path first using this information, if it finds the path user asked to
re-checkout with conflict markers has already been resolved.
It is important to think through to decide when we purge the "unresolve"
section.
If you run "read-tree", "checkout" to switch branches, or "reset" (any
option other than "--soft" which does not even touch the index), it is a
good sign that the information in the "unresolve" extension section is no
longer needed, so you can drop the section in these operations.
Optionally, write_index() could notice if there is no unmerged entries and
the cache_tree is fully valid---that is an indication that a tree object
has been written out of the now resolved index, and may (or may not) imply
that the "unresolve" information is no longer needed. But I haven't
thought this last one through. You could wish to unresolve even after you
committed your merge (you _could_ wish for anything after all), but I do
not yet know if granting that wish makes much sense.
There may be other cases we _must_ drop "unresolve".
^ permalink raw reply
* Re: FEATURE REQUEST: Announce branch name with merge comamnd
From: Junio C Hamano @ 2009-12-11 19:33 UTC (permalink / raw)
To: Jari Aalto; +Cc: git
In-Reply-To: <87zl5p1gsp.fsf@jondo.cante.net>
Patches welcome.
I didn't think people did Octopus these days. It feels so 2005 ;-)
^ permalink raw reply
* Re: How to selectively recreate merge state?
From: Paolo Bonzini @ 2009-12-11 20:38 UTC (permalink / raw)
To: git
In-Reply-To: <200912111220.40844.jnareb@gmail.com>
On 12/11/2009 12:20 PM, Jakub Narebski wrote:
>> > 2 and 3 are easy (cheap) to recreate from HEAD and MERGE_HEAD, 1 is not.
>> > I guess that's why --unresolve doesn't even attempt to do anything with 1.
> But then "git update-index --unresolve<file>" is next to useless.
Only "next to". It can still be useful if you added a file before
editing it, so you left in the conflict markers.
Paolo
^ permalink raw reply
* Re: How to selectively recreate merge state?
From: Junio C Hamano @ 2009-12-11 21:14 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: git, Jakub Narebski
In-Reply-To: <hfuakf$fnd$1@ger.gmane.org>
Paolo Bonzini <bonzini@gnu.org> writes:
> On 12/11/2009 12:20 PM, Jakub Narebski wrote:
>>> > 2 and 3 are easy (cheap) to recreate from HEAD and MERGE_HEAD, 1 is not.
>>> > I guess that's why --unresolve doesn't even attempt to do anything with 1.
>> But then "git update-index --unresolve<file>" is next to useless.
>
> Only "next to". It can still be useful if you added a file before
> editing it, so you left in the conflict markers.
To be fair, these need to be judged within their context, and then get
updated to today's reality.
"diff --cc" was merely a relatively new curiosity that allows a different
view into a conflicted merge (it was still cooking in 'next'). The
primary ways to inspect a conflict were "diff --theirs" and "diff --ours";
repopulating stages #2 and #3 was sufficient for them.
^ permalink raw reply
* Re: How to selectively recreate merge state?
From: Jay Soffian @ 2009-12-11 22:18 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jakub Narebski, Michael J Gruber, Johannes Sixt, git
In-Reply-To: <7v3a3h48lz.fsf@alter.siamese.dyndns.org>
On Fri, Dec 11, 2009 at 2:24 PM, Junio C Hamano <gitster@pobox.com> wrote:
> - This was done about only one year after git was born. You should not
> take it granted that the workflow it wanted to support makes sense.
>
> Considering that using "git add" to mark the resolution is to declare
> that you are _finished_ with that path, using it for other purposes
> (e.g. leaving a note that says "I've looked at and have one possible
> resolution in the file in the work tree, but I haven't verified the
> result yet", which is what the commit talks about) is simply an
> (ab|mis)use of the index. Lossage of higher stage information by this
> misuse is user's problem, and there is this thing called pen & pencil
> the user can use for taking notes if s/he does not want to lose the
> original conflict information from the index.
Just a little more data. What happened in my case was that I was using
a visual merge tool and accidentally saved instead of canceled, so git
mergetool automagically added my results. I had resolved about 15
files, and made a mistake with only one, so I was sad when I couldn't
determine how to unresolve that one file (at which point I saved off
the other 14 resolutions, reset, re-did the merge).
My intuition led me to try "git reset <path>" since that's how one
normally unstages additions to the index. But of course that didn't
work, where "of course" only makes sense if you know how the index is
used during a merge.
> In fact, considering that there are many ways conflicts can be left in the
> index and there are only two ways that they are resolved in the index by
> the user (and both eventually uses a single function to do so), it would
> make perfect sense to do the following:
>
> [...excellent list of suggestions elided...]
Also, I think we could improve the output of "git status" during merge
resolution, both before and after conflicts have been resolved in a
file. Immediately after a conflict, the conflicted files are shown as
"unmerged":
$ git status
foo: needs merge
# On branch master
# Changed but not updated:
# (use "git add <file>..." to update what will be committed)
# (use "git checkout -- <file>..." to discard changes in working directory)
#
# unmerged: foo
#
no changes added to commit (use "git add" and/or "git commit -a")
"unmerged" is good. But the instruction to use "git checkout --
<file>" to discard changes is wrong in this context:
$ git checkout -- foo
error: path 'foo' is unmerged
Then, after resolving foo and adding it:
$ git status
# On branch master
# Changes to be committed:
# (use "git reset HEAD <file>..." to unstage)
#
# modified: foo
#
Well, yes, I can use git reset, but that just keeps my side of the merge.
So I think with your suggested changes to the index, we can do better
with the status output during a merge.
j.
^ permalink raw reply
* Re: [PATCH 6/6] GITWEB - Separate defaults from main file
From: Jakub Narebski @ 2009-12-11 22:53 UTC (permalink / raw)
To: J.H.; +Cc: git, John 'Warthog9' Hawley
In-Reply-To: <4B226C0F.2070407@kernel.org>
On Fri, 11 Dec 2009, J.H. wrote:
>>> This is also a not-so-subtle start of trying to break up gitweb into
>>> separate files for easier maintainability, having everything in a
>>> single file is just a mess and makes the whole thing more complicated
>>> than it needs to be. This is a bit of a baby step towards breaking it
>>> up for easier maintenance.
>>
>> The question is if easier maintenance and development by spliting
>> gitweb for developers offsets ease of install for users.
>
> This would just get dropped into the same location that gitweb.cgi
> exists in, there is no real difference in installation, and thus I can't
> see this as an issue for users.
To be more exact you have to know that you have to drop _generated files_,
which means (for this version of patch) gitweb.cgi and gitweb_defaults.pl
(or whatever the generated file with config variables would be named).
ATTENTION!
Your changes would make stop working all gitweb tests. Currently they
do some magic with generated gitweb config file "$(pwd)/gitweb_config.perl"
set via GITWEB_CONFIG configuration variable to be able to run
_unprocessed_ gitweb/gitweb.perl (without any substitutions). This
allow to run tests on "live" version of gitweb.
After your changes it would be no longer possible, at least not if we
want to be sure that we test the same version of gitweb as gitweb_defaults.
It would probably mean that we need to move to testing built version,
i.e. gitweb.cgi, not gitweb.perl
>>> ---
>>> .gitignore | 1 +
>>> Makefile | 15 +-
>>> gitweb/Makefile | 2 +-
>>> gitweb/gitweb.perl | 515 +++++--------------------------------------
>>> gitweb/gitweb_defaults.perl | 468 +++++++++++++++++++++++++++++++++++++++
>>> 5 files changed, 537 insertions(+), 464 deletions(-)
>>> create mode 100644 gitweb/gitweb_defaults.perl
>>>
>>>
>>> diff --git a/.gitignore b/.gitignore
>>> index ac02a58..5e48102 100644
>>> --- a/.gitignore
>>> +++ b/.gitignore
>>> @@ -151,6 +151,7 @@
>>> /git-core-*/?*
>>> /gitk-git/gitk-wish
>>> /gitweb/gitweb.cgi
>>> +/gitweb/gitweb_defaults.pl
>>
>> Hmmm... gitweb/gitweb_defaults.perl as source file, and
>> gitweb/gitweb_defaults.pl as generated file? Wouldn't it be better to
>> go with the convention used elsewhere in gitweb and use
>> gitweb/gitweb_defaults.perl.in or gitweb/gitweb_defaults.pl.in as
>> source file?
>
> I think you got confused, the committed file is .perl the generated file
> is .pl.
Maybe I wasn't entirely clean. I meant that the committed source file
should perhaps have *.in extension to denote that it is to be processed
via variable substitution, so it would be
committed file: gitweb/gitweb_defaults.pl.in
generated file: gitweb/gitweb_defaults.pl
or whatever name (*.pm?) we agree on.
>>> $(QUIET_GEN)$(RM) $@ $@+ && \
>>> sed -e '1s|#!.*perl|#!$(PERL_PATH_SQ)|' \
>>> -e 's|++GIT_VERSION++|$(GIT_VERSION)|g' \
>>> @@ -1539,7 +1541,7 @@ endif
>>> -e 's|++GITWEB_JS++|$(GITWEB_JS)|g' \
>>> -e 's|++GITWEB_SITE_HEADER++|$(GITWEB_SITE_HEADER)|g' \
>>> -e 's|++GITWEB_SITE_FOOTER++|$(GITWEB_SITE_FOOTER)|g' \
>>> - $(patsubst %.cgi,%.perl,$@) >$@+ && \
>>> + $(patsubst %.cgi,%.perl,$(patsubst %.pl, %.perl, $@)) >$@+ && \
>>
>> Why the slightly inconsistent style ("%.cgi,%perl" vs "%.pl, %perl")?
>
> Considering that the defaults is more of an include vs. a cgi it
> probably shouldn't share the standard expected executable suffix, thus I
> used .pl. Could just as easily change it to .pm, or something else but
> I think it would make the most sense to leave things we are expecting
> the webserver to directly execute as .cgi, and includes as a different
> suffix.
I was not asking about that, but about
+ $(patsubst %.cgi,%.perl,$(patsubst %.pl, %.perl, $@)) >$@+ && \
vs
+ $(patsubst %.cgi,%.perl,$(patsubst %.pl,%.perl,$@)) >$@+ && \
But after thinking about it a bit, and consulting make documentation
(in particular definition of $@ variable) this rule shouldn't work at all.
`$@'
The file name of the target of the rule. If the target is an
archive member, then `$@' is the name of the archive file. In a
pattern rule that has multiple targets (*note Introduction to
Pattern Rules: Pattern Intro.), `$@' is the name of whichever
target caused the rule's commands to be run.
What we need is to run pattern substitution for _two_ files, perhaps
using the for loop.
Also I think the order of substitutions would be better to be reversed:
$(patsubst %.pl,%.perl,$(patsubst %.cgi,%.perl,$FILE)) >$FILE+
This way the gitweb_defaults file can even have *.perl extension
>> Also wouldn't all replacements be in the new gitweb_defaults file, so
>> there would be no need then to do replacements for gitweb.cgi?
>
> Not all replacements are done in one or the other, and since it's
> basically a NOP to perform the full set of replacements on both files
> that seemed the easiest way to ensure they were done in both places.
>
>> Oh, I see there is at least one that stayed in gitweb.perl: $version
>>
>
> <snip>
O.K.
But Makefile would be (slightly) simpler if replacements were needed only
for single file of two.
>>> +# Define and than setup our configuration
>>> +#
>>> +our(
>>> + $VERSION,
>>> + $path_info,
>>> + $GIT,
[...]
>>> + $prevent_xss,
>>> + @diff_opts,
>>> + %feature
>>> );
>>
>> Why this block is required? Why not have variables defined (using
>> "our") in gitweb_defaults file?
>
> Wanted to make sure things were properly defined, if in an unexpected
> state, should a user have gitweb.cgi in place but not the defaults.
In my opinion it actually *makes worse*. I am not sure if gitweb would
work if the variables are undefined, and you would lose 'undeclared
variable' warning.
>>> diff --git a/gitweb/gitweb_defaults.perl b/gitweb/gitweb_defaults.perl
>>> new file mode 100644
>>> index 0000000..ede0daf
>>> --- /dev/null
>>> +++ b/gitweb/gitweb_defaults.perl
>>> @@ -0,0 +1,468 @@
>>> +# gitweb - simple web interface to track changes in git repositories
>>> +#
>>> +# (C) 2005-2006, Kay Sievers <kay.sievers@vrfy.org>
>>> +# (C) 2005, Christian Gierke
>>> +#
>>> +# This program is licensed under the GPLv2
This header should probably be modified, at least stating what the file
is for.
>>> +
>>> +# Base URL for relative URLs in gitweb ($logo, $favicon, ...),
>>> +# needed and used only for URLs with nonempty PATH_INFO
>>> +$base_url = $my_url;
>>
>> Why not "our $base_url = $my_url;"?
>
> same reason as the other 'our' includes above,
See comment above about pre-declaring variables actually making it
worse wrt checking.
> though why this ended up
> as a separate patch vs. the rest of the file I don't know.
Errr... what you are talking about here?
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: How to selectively recreate merge state?
From: Junio C Hamano @ 2009-12-11 23:46 UTC (permalink / raw)
To: Jay Soffian; +Cc: Jakub Narebski, Michael J Gruber, Johannes Sixt, git
In-Reply-To: <76718490912111418i6b59056eq69671979613749f7@mail.gmail.com>
Jay Soffian <jaysoffian@gmail.com> writes:
> Also, I think we could improve the output of "git status" during merge
> resolution, both before and after conflicts have been resolved in a
> file.
I think you are talking about something that is largely unrelated, even
though they would be a pair of good issues to discuss. The solution to
them does not have much to do with what we have been discussing so far in
this thread, and actually should be much simpler, which is a good news
;-).
> $ git status
> foo: needs merge
> # On branch master
> # Changed but not updated:
> # (use "git add <file>..." to update what will be committed)
> # (use "git checkout -- <file>..." to discard changes in working directory)
> #
> # unmerged: foo
> #
> no changes added to commit (use "git add" and/or "git commit -a")
>
> "unmerged" is good. But the instruction to use "git checkout --
> <file>" to discard changes is wrong in this context:
You should be able to change this without any "unresolve" index extension
added to the index. Just notice an unmerged entry in the index and reword
the message accordingly.
More importantly, note that "git status" lists "unmerged" entries in a
separate section in its output in 1.6.6 (and has been so on 'master' for
some time) and your problem report needs to be adjusted for a more recent
reality. Here is what you would get:
$ git status
# On branch pu
# Changes to be committed:
# (use "git reset HEAD <file>..." to unstage)
#
# modified: builtin-send-pack.c
# modified: remote.c
# modified: remote.h
# modified: transport.c
#
# Unmerged paths:
# (use "git reset HEAD <file>..." to unstage)
# (use "git add <file>..." to mark resolution)
#
# both modified: transport-helper.c
#
One problem we can see is that 'use "git reset HEAD <file>..." to unstage'
is an invalid advice if we are in the middle of a merge, but is perfectly
valid if this were during "rebase", "am -3", "cherry-pick" and "revert".
The solution to this issue is exactly the same as the next one.
> $ git status
> # On branch master
> # Changes to be committed:
> # (use "git reset HEAD <file>..." to unstage)
> #
> # modified: foo
> #
>
> Well, yes, I can use git reset, but that just keeps my side of the merge.
If the conflict was coming from "rebase", "cherry-pick", etc., there is
nothing but one side, as there is no merge going on, and what "git reset"
does is exactly what the message tells you---to unstage.
I think "git status" should notice that the next commit you would make
from this state will be a merge commit, and remove these "reset HEAD"
lines. Once you "git add" to resolve, it makes _no_ sense to reset to
HEAD, if you are concluding a merge. Until "update-index --unresolve" is
revived as a modern version (and I suspect that a more logical Porcelain
interface would be a new option "reset --unmerge <paths>..."), we should
simply drop "reset HEAD" advice when we are in a merge.
Note that the "unresolve" index extension will not help you at all in
order for you to decide if you are going to make a merge commit. You
should instead ask "does .git/MERGE_HEAD exist?", and it is something you
should be able to implement directly on top of upcoming 1.6.6 release.
^ permalink raw reply
* [PATCH 2/3] octopus: reenable fast-forward merges
From: Stephen Boyd @ 2009-12-12 0:38 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <87zl5p1gsp.fsf@jondo.cante.net>
The fast-forward logic is never being triggered because $common and
$MRC are never equivalent. $common is initialized to a commit id by
merge-base and MRC is initialized to HEAD. Fix this by initializing
$MRC to the commit id for HEAD so that its possible for $MRC and
$common to be equal.
Signed-off-by: Stephen Boyd <bebarino@gmail.com>
---
Found this while making tests up for part 1 of this series.
git-merge-octopus.sh | 2 +-
t/t7602-merge-octopus-many.sh | 18 ++++++++++++++++++
2 files changed, 19 insertions(+), 1 deletions(-)
diff --git a/git-merge-octopus.sh b/git-merge-octopus.sh
index 1c8ee0a..99b6f8a 100755
--- a/git-merge-octopus.sh
+++ b/git-merge-octopus.sh
@@ -44,7 +44,7 @@ esac
# MRC is the current "merge reference commit"
# MRT is the current "merge result tree"
-MRC=$head MSG= PARENT="-p $head"
+MRC=$(git rev-parse --verify -q $head) MSG= PARENT="-p $head"
MRT=$(git write-tree)
CNT=1 ;# counting our head
NON_FF_MERGE=0
diff --git a/t/t7602-merge-octopus-many.sh b/t/t7602-merge-octopus-many.sh
index 7377033..2746169 100755
--- a/t/t7602-merge-octopus-many.sh
+++ b/t/t7602-merge-octopus-many.sh
@@ -82,4 +82,22 @@ test_expect_success 'merge up-to-date output uses pretty names' '
git merge c4 c5 >actual &&
test_cmp actual expected
'
+
+cat >expected <<\EOF
+Fast-forwarding to: c1
+Trying simple merge with c2
+Merge made by octopus.
+ c1.c | 1 +
+ c2.c | 1 +
+ 2 files changed, 2 insertions(+), 0 deletions(-)
+ create mode 100644 c1.c
+ create mode 100644 c2.c
+EOF
+
+test_expect_success 'merge fast-forward output uses pretty names' '
+ git reset --hard c0 &&
+ git merge c1 c2 >actual &&
+ test_cmp actual expected
+'
+
test_done
--
1.6.6.rc1.45.g9aadbb
^ permalink raw reply related
* [PATCH 1/3] octopus: make merge process simpler to follow
From: Stephen Boyd @ 2009-12-12 0:38 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jari Aalto
In-Reply-To: <87zl5p1gsp.fsf@jondo.cante.net>
Its not very easy to understand what heads are being merged given
the current output of an octopus merge. Fix this by replacing the
sha1 with the (usually) better description in GITHEAD_<SHA1>.
Suggested-by: Jari Aalto <jari.aalto@cante.net>
Signed-off-by: Stephen Boyd <bebarino@gmail.com>
---
Maybe this will work? At least it will replace the sha1 with
whatever is given on the command line.
git-merge-octopus.sh | 9 +++++----
t/t7602-merge-octopus-many.sh | 33 +++++++++++++++++++++++++++++++++
2 files changed, 38 insertions(+), 4 deletions(-)
diff --git a/git-merge-octopus.sh b/git-merge-octopus.sh
index 825c52c..1c8ee0a 100755
--- a/git-merge-octopus.sh
+++ b/git-merge-octopus.sh
@@ -61,12 +61,13 @@ do
exit 2
esac
+ pretty_name="$(eval echo \$GITHEAD_$SHA1)"
common=$(git merge-base --all $SHA1 $MRC) ||
- die "Unable to find common commit with $SHA1"
+ die "Unable to find common commit with $pretty_name"
case "$LF$common$LF" in
*"$LF$SHA1$LF"*)
- echo "Already up-to-date with $SHA1"
+ echo "Already up-to-date with $pretty_name"
continue
;;
esac
@@ -81,7 +82,7 @@ do
# tree as the intermediate result of the merge.
# We still need to count this as part of the parent set.
- echo "Fast-forwarding to: $SHA1"
+ echo "Fast-forwarding to: $pretty_name"
git read-tree -u -m $head $SHA1 || exit
MRC=$SHA1 MRT=$(git write-tree)
continue
@@ -89,7 +90,7 @@ do
NON_FF_MERGE=1
- echo "Trying simple merge with $SHA1"
+ echo "Trying simple merge with $pretty_name"
git read-tree -u -m --aggressive $common $MRT $SHA1 || exit 2
next=$(git write-tree 2>/dev/null)
if test $? -ne 0
diff --git a/t/t7602-merge-octopus-many.sh b/t/t7602-merge-octopus-many.sh
index 01e5415..7377033 100755
--- a/t/t7602-merge-octopus-many.sh
+++ b/t/t7602-merge-octopus-many.sh
@@ -49,4 +49,37 @@ test_expect_success 'merge c1 with c2, c3, c4, ... c29' '
done
'
+cat >expected <<\EOF
+Trying simple merge with c2
+Trying simple merge with c3
+Trying simple merge with c4
+Merge made by octopus.
+ c2.c | 1 +
+ c3.c | 1 +
+ c4.c | 1 +
+ 3 files changed, 3 insertions(+), 0 deletions(-)
+ create mode 100644 c2.c
+ create mode 100644 c3.c
+ create mode 100644 c4.c
+EOF
+
+test_expect_success 'merge output uses pretty names' '
+ git reset --hard c1 &&
+ git merge c2 c3 c4 >actual &&
+ test_cmp actual expected
+'
+
+cat >expected <<\EOF
+Already up-to-date with c4
+Trying simple merge with c5
+Merge made by octopus.
+ c5.c | 1 +
+ 1 files changed, 1 insertions(+), 0 deletions(-)
+ create mode 100644 c5.c
+EOF
+
+test_expect_success 'merge up-to-date output uses pretty names' '
+ git merge c4 c5 >actual &&
+ test_cmp actual expected
+'
test_done
--
1.6.6.rc1.45.g9aadbb
^ permalink raw reply related
* [PATCH 3/3] octopus: remove dead code
From: Stephen Boyd @ 2009-12-12 0:38 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <87zl5p1gsp.fsf@jondo.cante.net>
MSG, PARENT, and CNT are never used, just assigned to.
Signed-off-by: Stephen Boyd <bebarino@gmail.com>
---
I don't know if this is wanted. Looks like maybe they're used
as simple debug aides?
git-merge-octopus.sh | 6 +-----
1 files changed, 1 insertions(+), 5 deletions(-)
diff --git a/git-merge-octopus.sh b/git-merge-octopus.sh
index 99b6f8a..3cb0b31 100755
--- a/git-merge-octopus.sh
+++ b/git-merge-octopus.sh
@@ -44,9 +44,8 @@ esac
# MRC is the current "merge reference commit"
# MRT is the current "merge result tree"
-MRC=$(git rev-parse --verify -q $head) MSG= PARENT="-p $head"
+MRC=$(git rev-parse --verify -q $head)
MRT=$(git write-tree)
-CNT=1 ;# counting our head
NON_FF_MERGE=0
OCTOPUS_FAILURE=0
for SHA1 in $remotes
@@ -72,9 +71,6 @@ do
;;
esac
- CNT=`expr $CNT + 1`
- PARENT="$PARENT -p $SHA1"
-
if test "$common,$NON_FF_MERGE" = "$MRC,0"
then
# The first head being merged was a fast-forward.
--
1.6.6.rc1.45.g9aadbb
^ permalink raw reply related
* Re: [PATCH 3/3] octopus: remove dead code
From: Junio C Hamano @ 2009-12-12 1:01 UTC (permalink / raw)
To: Stephen Boyd; +Cc: git, Junio C Hamano
In-Reply-To: <1260578339-30750-3-git-send-email-bebarino@gmail.com>
Stephen Boyd <bebarino@gmail.com> writes:
> MSG, PARENT, and CNT are never used, just assigned to.
>
> Signed-off-by: Stephen Boyd <bebarino@gmail.com>
> ---
>
> I don't know if this is wanted. Looks like maybe they're used
> as simple debug aides?
No, thanks for spotting. It is the right thing to do to just get rid of
them.
IIRC, they are remnants of ancient logic of the script, even before
91063bb (Multi-backend merge driver., 2005-09-08) that added it to the
official git history, and were used to update MRC and to come up with the
end result (experimental versions of the script used to run "commit-tree"
itself).
^ permalink raw reply
* Re: FEATURE REQUEST: Announce branch name with merge comamnd
From: Alex Riesen @ 2009-12-12 1:35 UTC (permalink / raw)
To: Jari Aalto; +Cc: git
In-Reply-To: <87zl5p1gsp.fsf@jondo.cante.net>
On Fri, Dec 11, 2009 at 19:55, Jari Aalto <jari.aalto@cante.net> wrote:
> Please announce the branch name being merged so that the listing is
> easier to follow (possibly only with --verbose, -v option). Add
> "Branch: <name>" just before the merge is attempted. somethiglike this
>
> Branch: bug--manpage-fix-hyphen
> Trying simple merge with c87c49b1e413e5dc378d7e6b16951761a1e82f6d
It is not exactly "easier" to follow in your case. It is more
text and there is no immediately visible cue that the two
lines are related. You have to give the observer this information.
Put reference name and SHA-1 on the same line?
^ permalink raw reply
* Re: [PATCH 0/6] Gitweb caching changes v2
From: Jakub Narebski @ 2009-12-12 1:37 UTC (permalink / raw)
To: J.H.; +Cc: git
In-Reply-To: <4B228ED3.3030901@kernel.org>
On Fri, 11 Dec 2009, J.H. wrote:
> Jakub Narebski wrote:
>> On Fri, 11 Dec 2009, J.H. (John 'Warthog9' Hawley) wrote:
>>> Jakub Narebski wrote:
>>>> "John 'Warthog9' Hawley" <warthog9@kernel.org> writes:
>>>>>
>>>>> GITWEB - File based caching layer
>>>>>
>>>> This patch didn't made it to git mailing list. I suspect that you ran
>>>> afoul vger anti-SPAM filter.
>>>
>>> Yeah, it does seem that way (like I said eaten by a grue),
It _might_ be caused by the fact that you used attachement. But it might
not; you can always use vger-taboo.perl script to check.
>>>> Does this "File based caching layer" have anything common with GSoC
>>>> 2008 project, available at git://repo.or.cz/git/gitweb-caching.git ?
>>>
>>> It *currently* has nothing to do with Lea's GSoC code but it is still my
>>> intention, in long term, to integrate the two.
The question would be then whether it makes sense to have two caches at
different levels in the stack (see also discussion below about Lea
approach).
>>> The patch, in all it's glory can be viewed at:
>>> http://git.kernel.org/?p=git/warthog9/gitweb.git;a=commitdiff;h=42641b1e3bfae14d5cc2e0150355e89cb87951db
>>>
>>> It is anything but a small patch to gitweb, the patch is 117K and
>>> comprises 3539 lines (including git header commit information). There's
>>> not any real good way to break it up as it's a bit of an all or nothing
>>> patch.
>>
>> First, why do you reinvent the wheel instead of using one of existing
>> caching interfaces like CHI or Cache::Cache (perhaps creating a custom
>> backend or middle layer which incorporates required features, like being
>> load-aware)?
>
> Well for starters this isn't exactly a reinvention of the wheel, and
> this isn't something "new" per-se. This code has been actively running
> on git.kernel.org for something like 3 - 4 years so there's something to
> be said for the devil we know and understand.
Well, if it is not reinventing the wheel, then it is yak shaving (yet
another ...) ;-)
The fact that the code was used and tested at one single installation
doesn't mean that it doesn't have warts that could be avoided by using
ready solution (at least for parts of it).
> As well using the other
> caching strategies involves adding dramatically more complex
> interactions with caching layer.
I am hoping that if it was done right, by using CHI or Cache::Cache
caching interface, then choosing alternate caching engine, or adding
extra level of cache would be simple and decoupled from issues specific
to web app or gitweb in particular.
> The caching layer is actually quite
> specific to how git + gitweb works and solves more than just "caching"
> on the surface.
The idea is for gitweb/cache.pm (or gitweb/Gitweb-Cache.pm, or
gitweb/Gitweb/Cache.pm) is to encapsulate issues specific to gitweb,
like generating cache key, or printing "Generating...", etc.
Perhaps also the idea of filling cache in background (but see discussion
below about capturing STDOUT) could be put there.
> Specifically it solves the stampeding herd problem
> which would have to be solved either way even if I didn't implement my
> own caching, and since I had to do that caching was barely a step beyond
> that to implement.
CHI tries to solve cache miss stampedes via expires_variance mechanism.
There is Cache::Adaptive (and its subclass Cache::Adaptive::ByLoad)
which does adaptive lifetime control (it accepts any Cache::Cache
compatible cache, so I think it also accepts CHI compatible cache).
Those problems _were_ solved.
>> This way changing from file-based cache to e.g. mmap based
>> one or to memcached would be very simple.
>
> True but these are *VERY* different caching strategies than the one I've
> got here, yes it's using files as a backend but it's doing so with
> specific goals in mind. As I've said I plan to integrate Lea's
> memcached based caching into this in the future and that has different
> advantages and disadvantages.
Errr... besides using Cache::Cache compatible cache (see!!!), for example
Cache::Memcached, Lea Wiemann's gitweb caching did caching at entirely
different level than original kernel.org's gitweb.
The stack for gitweb looks somewhat like this:
git commands output open my $fd, '-|, git_cmd(), ...
|
v
parsed output data parse_ls_tree_line, parse_commit, ...
|
v
generated HTML etc. print ...
:
V
caching optional
reverse proxy Varnish, Squid
If I understand correctly Lea Wiemann code cache git command output.
The fork of gitweb used at repo.or.cz does caching of parsed data at
least for most intensive projects list page. This patch was about caching
generated output. HTTP caching requires that gitweb can respond to
If-Modified-Since (and generate Last-Modified) and If-None-Match (and
generate ETag) in a time that is much faster than generating full response.
There are advantages and disadvantages for each method of caching; also
the balance might depend on the view used. For example 'snapshot' view
is best cached via output caching with file-based cache, while for
'blame_incremental' view straight caching of output doesn't make much
sense while caching command output should give good behaviour.
> At the end of the day the "normal" caching engines aren't as efficient
> as mine and there is the case the very high performance sites are going
> to have to investigate a number of different solutions to see what works
> best for them. Mine is also *dramatically* simpler to setup as well,
> turn it on, point it at a directory and you're done.
Do you have any benchmarks?
>> And you would avoid pitfals
>> in doing your own cache management. perl-Cache-Cache should be available
>> package in extras repositories.
>
> There's pitfalls if I do it myself, or I use one of the other "common"
> perl modules. I did it this way years ago, I've maintained it and it
> works pretty well. I won't admit that it's the smartest caching engine
> on the planet, far from it, but it has evolved specifically for gitweb
> and that itself saves me a lot of pitfalls from cache engine + gitweb
> integration.
If I remember correctly the solution presented here has serious
disadvantage of not having any cache expire policy, and not being
size-aware.
>> If module is no available this would simply mean no caching, like in many
>> (or not so many) other cases with optional features in gitweb.
>
> Yes, but as can be seen from how you enable various other caching
> engines the setup of those is non-trivial, this is and either way
> caching *HAS* to be explicitly turned on by the admin/user since they
> are going to have to do *some* configuration, or at least be aware that
> their webapp is going to chew up some sort of resource.
I wonder if there is any data that describes when one should enable
caching, and when one can do without it, e.g. depending on the number
and total size of repositories presented via gitweb.
IMHO cache storage is orthogonal to expire policy, which in turn is
orthogonal on cache use in gitweb. And those parts should be kept separate
(and tested independently), even if we decide on homegrown caching
solution.
>> Second, if you can't use CGI::Cache directly, you can always steal the
>> idea from it, then the change to gitweb itself would be minimal:
>>
>> "Internally, the CGI::Cache module ties the output file descriptor
>> (usually STDOUT) to an internal variable to which all output is saved."
>
> I thought about that 3 years ago, and decided it wasn't a good option
> for gitweb. Why? There's too many assumptions throughout the code that
> when you do a print it will go immediately out. Things like error
> messages and such. Breaking out the prints into prints (which will do
> what is expected) and passing around the output in the $output variables
> makes it a lot simpler easier to differentiate about how / what your
> looking at and a *LOT* easier to debug.
Note that in quite a few places we print directly to output, streaming
the response, for performance (to reduce latency). If all data must be
first gathered in $output variable (increasing memory pressure in the
case of large files for 'blob_plain', large snapshots, large patches in
'patch' and 'patches' views) then we must wait for it to finish, and not
get data as soon as it is available.
Besides instead of just capturing STDOUT in tied variable (STDERR goes
to web server log courtesy of CGI.pm) we can tee it, i.e. capture it
to $output variable as it is streamed to web browser. See Capture::Tiny
(although I am not sure how it would interact with CGI.pm logging) and
e.g. PerlIO::tee mechanism from PerlIO::Util.
Going the route of CGI::Cache would mean minimal changes to gitweb...
and no diference in performance if caching is turned off (see streaming).
>> P.S. I'll postpone critique of the patch itself for now. The above issues
>> are much more important.
>
> That's fine. The issues your raising aren't new though, and stem back
> to before I created gitweb-caching, got rehashed with Lea's patches and
> not surprisingly are back on the table now. Like I said above, there is
> no one caching strategy that's perfect in all cases here and that's
> again why I eventually plan to merge Lea's changes (which uses
> memcached) in as well, I'm just trying to get code that I'm getting
> considerable demand for, that's proven, upstream.
Well, there are two solutions. One is first to decide on proper solution
for gitweb caching. Another is to have _some_ caching and then improve it.
So below there are a few initial comments about gitweb/cache.pm code:
* gitweb/cache.pm should be, I think, a proper module (require'd or use'd)
* you do not follow coding style used elsewhere in gitweb, e.g. spaces
around {} and (), for example it is
}elsif( $cache_enable == 1 ){
and should be
} elsif ($cache_enable == 1) {
* flags that are boolean are compared to 0 and 1
* cache key should be generated from both PATH_INFO and QUERY_STRING
in generic case (unless you turn off $path_info as default, and turn off
support for path_info URLs); see %input_params hash or href(-replay=>1)
* gitweb till now does not include any variable data in error info
* duplicated code (e.g. fork / cacheUpdate + cacheDisplay / cacheUpdate...)
* inconsistent naming style: cache_fetch but cacheDisplay.
* old style open using globs instead of local filehandles:
open(cacheFile, '<', "$fullhashpath");
and should be
open(my $cache_fh, '<', $fullhashpath);
* busy wait 'do { sleep 2; open ... } while (...)' instead of non-blocking
wait like select / IO::Select.
That's all from skimming gitweb-ml-v2:gitweb/cache.pm
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [PATCH v4 2/6] reset: use "unpack_trees()" directly instead of "git read-tree"
From: Christian Couder @ 2009-12-12 4:19 UTC (permalink / raw)
To: Stephen Boyd
Cc: Junio C Hamano, git, Linus Torvalds, Johannes Schindelin,
Stephan Beyer, Daniel Barkalow, Jakub Narebski, Paolo Bonzini,
Johannes Sixt
In-Reply-To: <1260261919.1554.11.camel@swboyd-laptop>
On mardi 08 décembre 2009, Stephen Boyd wrote:
> On Tue, 2009-12-08 at 08:56 +0100, Christian Couder wrote:
> > +static int parse_and_init_tree_desc(const unsigned char *sha1,
> > + struct tree_desc *desc)
> > +{
> > + struct tree *tree = parse_tree_indirect(sha1);
> > + if (!tree)
> > + return 1;
> > + init_tree_desc(desc, tree->buffer, tree->size);
> > + return 0;
> > +}
> > +
>
> Is there a reason why you use this function instead of
> fill_tree_descriptor()?
I didn't know about fill_tree_descriptor(). I will use it in the next
series.
Thanks,
Christian.
^ permalink raw reply
* [PATCH v5 0/7] "git reset --merge" related improvements
From: Christian Couder @ 2009-12-12 4:32 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Linus Torvalds, Johannes Schindelin, Stephan Beyer,
Daniel Barkalow, Jakub Narebski, Paolo Bonzini, Johannes Sixt,
Stephen Boyd
Another reroll with the following changes:
- the name of the new option is now "--keep" instead of "--keep-local-changes",
- the fill_tree_descriptor() function is used instead of adding a new
parse_and_init_tree_desc() function (thanks to Stephen Boyd),
- patch 1/7 was added to only accept soft reset when not in a working tree;
this makes the test suite pass,
- the commit message of patch 4/7 that adds the --keep option has been
improved; it talks more about the use case of this new option.
Christian Couder (6):
reset: do not accept a mixed reset in a .git dir
reset: add a few tests for "git reset --merge"
reset: add option "--keep" to "git reset"
reset: add test cases for "--keep" option
Documentation: reset: describe new "--keep" option
Documentation: reset: add some tables to describe the different
options
Stephan Beyer (1):
reset: use "unpack_trees()" directly instead of "git read-tree"
Documentation/git-reset.txt | 79 +++++++++++++++++++++-
builtin-reset.c | 74 +++++++++++++++-----
t/t7103-reset-bare.sh | 4 +-
t/t7110-reset-merge.sh | 156 +++++++++++++++++++++++++++++++++++++++++++
4 files changed, 292 insertions(+), 21 deletions(-)
create mode 100755 t/t7110-reset-merge.sh
^ permalink raw reply
* [PATCH v5 1/7] reset: do not accept a mixed reset in a .git dir
From: Christian Couder @ 2009-12-12 4:32 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Linus Torvalds, Johannes Schindelin, Stephan Beyer,
Daniel Barkalow, Jakub Narebski, Paolo Bonzini, Johannes Sixt,
Stephen Boyd
In-Reply-To: <20091212042042.3930.54783.chriscool@tuxfamily.org>
It is strange and fragile that a mixed reset is disallowed in a bare
repo but is allowed in a .git dir. So this patch simplifies things
by only allowing soft resets when not in a working tree.
This patch is also needed to speed up "git reset" by using
unpack_tree() directly (instead of execing "git read-tree"). A
following patch will do just that.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin-reset.c | 3 +--
t/t7103-reset-bare.sh | 4 ++--
2 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/builtin-reset.c b/builtin-reset.c
index 11d1c6e..14bdb03 100644
--- a/builtin-reset.c
+++ b/builtin-reset.c
@@ -286,8 +286,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
if (reset_type == NONE)
reset_type = MIXED; /* by default */
- if ((reset_type == HARD || reset_type == MERGE)
- && !is_inside_work_tree())
+ if (reset_type != SOFT && !is_inside_work_tree())
die("%s reset requires a work tree",
reset_type_names[reset_type]);
diff --git a/t/t7103-reset-bare.sh b/t/t7103-reset-bare.sh
index 68041df..13344b2 100755
--- a/t/t7103-reset-bare.sh
+++ b/t/t7103-reset-bare.sh
@@ -21,8 +21,8 @@ test_expect_success 'merge reset requires a worktree' '
test_must_fail git reset --merge)
'
-test_expect_success 'mixed reset is ok' '
- (cd .git && git reset)
+test_expect_success 'mixed reset requires a worktree' '
+ (cd .git && test_must_fail git reset)
'
test_expect_success 'soft reset is ok' '
--
1.6.6.rc1.8.gd33ec
^ permalink raw reply related
* [PATCH v5 4/7] reset: add option "--keep" to "git reset"
From: Christian Couder @ 2009-12-12 4:32 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Linus Torvalds, Johannes Schindelin, Stephan Beyer,
Daniel Barkalow, Jakub Narebski, Paolo Bonzini, Johannes Sixt,
Stephen Boyd
In-Reply-To: <20091212042042.3930.54783.chriscool@tuxfamily.org>
The purpose of this new option is to discard some of the
last commits but to keep current changes in the work tree.
The use case is when you work on something and commit
that work. And then you work on something else that touches
other files, but you don't commit it yet. Then you realize
that what you commited when you worked on the first thing
is not good or belongs to another branch.
So you want to get rid of the previous commits (at least in
the current branch) but you want to make sure that you keep
the changes you have in the work tree. And you are pretty
sure that your changes are independent from what you
previously commited, so you don't want the reset to succeed
if the previous commits changed a file that you also
changed in your work tree.
The "--keep" option will do what you want.
The table below shows what happens when running
"git reset --option target" to reset the HEAD to another
commit (as a special case "target" could be the same as
HEAD) in the cases where "--merge" and "--keep" behave
differently.
working index HEAD target working index HEAD
----------------------------------------------------
A B C D --keep (disallowed)
--merge (disallowed)
A B C C --keep A C C
--merge (disallowed)
B B C D --keep (disallowed)
--merge C C C
B B C C --keep B C C
--merge C C C
In this table, A, B and C are some different states of
a file. For example the last line of the table means
that if a file is in state B in the working tree and
the index, and in a different state C in HEAD and in
the target, then "git reset --merge target" will put
the file in state C in the working tree, in the index
and in HEAD.
So as can be seen in the table, "--merge" can discard
changes in the working tree, while "--keep" does not.
So if one wants to avoid using "git stash" before and
after using "git reset" to save current changes, it is
better to use "--keep" rather than "--merge".
The following table shows what happens on unmerged entries:
working index HEAD target working index HEAD
----------------------------------------------------
X U A B --keep X B B
--merge X B B
X U A A --keep X A A
--merge (disallowed)
In this table X can be any state and U means an unmerged
entry.
A following patch will add some test cases for
"--keep", where the differences between "--merge" and
"--keep" can also be seen.
The "--keep" option is implemented by doing a 2 way merge
between HEAD and the reset target, and if this succeeds
by doing a mixed reset to the target.
The code comes from the sequencer GSoC project, where
such an option was developed by Stephan Beyer:
git://repo.or.cz/git/sbeyer.git
(at commit 5a78908b70ceb5a4ea9fd4b82f07ceba1f019079)
But in the sequencer project the "reset" flag was set
in the "struct unpack_trees_options" passed to
"unpack_trees()". With this flag the changes in the
working tree were discarded if the file was different
between HEAD and the reset target.
Mentored-by: Daniel Barkalow <barkalow@iabervon.org>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Stephan Beyer <s-beyer@gmx.net>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin-reset.c | 30 +++++++++++++++++++++++++-----
1 files changed, 25 insertions(+), 5 deletions(-)
diff --git a/builtin-reset.c b/builtin-reset.c
index ac38aaa..859e0d1 100644
--- a/builtin-reset.c
+++ b/builtin-reset.c
@@ -22,13 +22,15 @@
#include "cache-tree.h"
static const char * const git_reset_usage[] = {
- "git reset [--mixed | --soft | --hard | --merge] [-q] [<commit>]",
+ "git reset [--mixed | --soft | --hard | --merge | --keep] [-q] [<commit>]",
"git reset [--mixed] <commit> [--] <paths>...",
NULL
};
-enum reset_type { MIXED, SOFT, HARD, MERGE, NONE };
-static const char *reset_type_names[] = { "mixed", "soft", "hard", "merge", NULL };
+enum reset_type { MIXED, SOFT, HARD, MERGE, KEEP, NONE };
+static const char *reset_type_names[] = {
+ "mixed", "soft", "hard", "merge", "keep", NULL
+};
static char *args_to_str(const char **argv)
{
@@ -71,6 +73,7 @@ static int reset_index_file(const unsigned char *sha1, int reset_type, int quiet
if (!quiet)
opts.verbose_update = 1;
switch (reset_type) {
+ case KEEP:
case MERGE:
opts.update = 1;
break;
@@ -85,6 +88,16 @@ static int reset_index_file(const unsigned char *sha1, int reset_type, int quiet
read_cache_unmerged();
+ if (reset_type == KEEP) {
+ unsigned char head_sha1[20];
+ if (get_sha1("HEAD", head_sha1))
+ return error("You do not have a valid HEAD.");
+ if (!fill_tree_descriptor(desc, head_sha1))
+ return error("Failed to find tree of HEAD.");
+ nr++;
+ opts.fn = twoway_merge;
+ }
+
if (!fill_tree_descriptor(desc + nr - 1, sha1))
return error("Failed to find tree of %s.", sha1_to_hex(sha1));
if (unpack_trees(nr, desc, &opts))
@@ -228,6 +241,9 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
"reset HEAD, index and working tree", HARD),
OPT_SET_INT(0, "merge", &reset_type,
"reset HEAD, index and working tree", MERGE),
+ OPT_SET_INT(0, "keep", &reset_type,
+ "reset HEAD but keep local changes",
+ KEEP),
OPT_BOOLEAN('q', NULL, &quiet,
"disable showing new HEAD in hard reset and progress message"),
OPT_BOOLEAN('p', "patch", &patch_mode, "select hunks interactively"),
@@ -315,9 +331,13 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
if (reset_type == SOFT) {
if (is_merge() || read_cache() < 0 || unmerged_cache())
die("Cannot do a soft reset in the middle of a merge.");
+ } else {
+ int err = reset_index_file(sha1, reset_type, quiet);
+ if (reset_type == KEEP)
+ err = err || reset_index_file(sha1, MIXED, quiet);
+ if (err)
+ die("Could not reset index file to revision '%s'.", rev);
}
- else if (reset_index_file(sha1, reset_type, quiet))
- die("Could not reset index file to revision '%s'.", rev);
/* Any resets update HEAD to the head being switched to,
* saving the previous head in ORIG_HEAD before. */
--
1.6.6.rc1.8.gd33ec
^ 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