* 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: 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: 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: 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: 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: 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
* 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: [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
* 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 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 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 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 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: Jakub Narebski @ 2009-12-11 15:46 UTC (permalink / raw)
To: John 'Warthog9' Hawley; +Cc: git, John 'Warthog9' Hawley
In-Reply-To: <1260488743-25855-7-git-send-email-warthog9@kernel.org>
"John 'Warthog9' Hawley" <warthog9@kernel.org> writes:
> This is an attempt to break out the default values & associated
> documentation from the main gitweb file so that it's easier to
> browse / read and understand without the associated code involved.
>
> This helps by making defaults self contained with their documentation
> making it easier for someone to read through things and find what
> they want
>
> 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.
> 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?
> /test-chmtime
> /test-ctype
> /test-date
> diff --git a/Makefile b/Makefile
> index 8db9d01..2c5f139 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1510,14 +1510,16 @@ $(patsubst %.perl,%,$(SCRIPT_PERL)): % : %.perl
> mv $@+ $@
>
> .PHONY: gitweb
> -gitweb: gitweb/gitweb.cgi
> +gitweb: gitweb/gitweb.cgi gitweb/gitweb_defaults.pl
> ifdef JSMIN
> -OTHER_PROGRAMS += gitweb/gitweb.cgi gitweb/gitweb.min.js
> -gitweb/gitweb.cgi: gitweb/gitweb.perl gitweb/gitweb.min.js
> +OTHER_PROGRAMS += gitweb/gitweb.cgi gitweb/gitweb.min.js gitweb/gitweb_defaults.pl
> +gitweb/gitweb.cgi gitweb/gitweb_defaults.pl: gitweb/gitweb.perl gitweb/gitweb.min.js gitweb/gitweb_defaults.perl
> else
> -OTHER_PROGRAMS += gitweb/gitweb.cgi
> -gitweb/gitweb.cgi: gitweb/gitweb.perl
> +OTHER_PROGRAMS += gitweb/gitweb.cgi gitweb/gitweb_defaults.pl
> +gitweb/gitweb.cgi: gitweb/gitweb_defaults.pl
> +gitweb/gitweb.cgi gitweb/gitweb_defaults.pl: gitweb/gitweb.perl gitweb/gitweb_defaults.perl
> endif
> + #$(QUIET_GEN)$(RM) $@ $@+ &&
What this line is about?
> $(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")?
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?
Oh, I see there is at least one that stayed in gitweb.perl: $version
> chmod +x $@+ && \
> mv $@+ $@
>
> @@ -1913,6 +1915,7 @@ clean:
> $(MAKE) -C Documentation/ clean
> ifndef NO_PERL
> $(RM) gitweb/gitweb.cgi
> + $(RM) gitweb/gitweb_defaults.pl
> $(MAKE) -C perl clean
> endif
> $(MAKE) -C templates/ clean
> diff --git a/gitweb/Makefile b/gitweb/Makefile
> index 8d318b3..2bd421a 100644
> --- a/gitweb/Makefile
> +++ b/gitweb/Makefile
> @@ -1,6 +1,6 @@
> SHELL = /bin/bash
>
> -FILES = gitweb.cgi
> +FILES = gitweb.cgi gitweb_defaults.pl
>
> .PHONY: $(FILES)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 3b44371..fd41539 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -36,466 +36,67 @@ our $version = "++GIT_VERSION++";
> our $my_url = $cgi->url();
> our $my_uri = $cgi->url(-absolute => 1);
>
[cut deletion]
> +# 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?
[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;"?
[cut]
> +1;
--
Jakub Narebski
Poland
ShadeHawk on #git
^ permalink raw reply
* Re: How to selectively recreate merge state?
From: Junio C Hamano @ 2009-12-11 15:35 UTC (permalink / raw)
To: Michael J Gruber; +Cc: Jakub Narebski, Jay Soffian, git
In-Reply-To: <4B225DC0.4020603@drmicha.warpmail.net>
Michael J Gruber <git@drmicha.warpmail.net> writes:
>> The documentation of "git update-index --unresolve" lacks this info,
>> and it doesn't tell one what it is for (see commit message for commit
>> ec16779 (Add git-unresolve <paths>..., 2006-04-19)).
>
> Oh yes, one should always read the classics ;) [Really nice commit
> message, that is.]
Thanks.
This is exactly why I often give _explanation_ of the current behaviour
based on history, without defending it is good nor claiming it is bad.
Knowing how things came about gives us better perspective to decide where
to go next.
^ permalink raw reply
* Re: How to selectively recreate merge state?
From: Michael J Gruber @ 2009-12-11 14:57 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Junio C Hamano, Jay Soffian, git
In-Reply-To: <200912111500.51982.jnareb@gmail.com>
Jakub Narebski venit, vidit, dixit 11.12.2009 15:00:
> Dnia piątek 11. grudnia 2009 13:33, Michael J Gruber napisał:
>> Jakub Narebski venit, vidit, dixit 11.12.2009 12:20:
>>> Dnia piątek 11. grudnia 2009 11:44, Michael J Gruber napisał:
>>>> Jakub Narebski venit, vidit, dixit 11.12.2009 02:33:
>>>>> Dnia piątek 11. grudnia 2009 02:11, Junio C Hamano napisał:
>>>>>> Jakub Narebski <jnareb@gmail.com> writes:
>>>>>>
>>>>>>> --unresolve::
>>>>>>> Restores the 'unmerged' or 'needs updating' state of a
>>>>>>> file during a merge if it was cleared by accident.
>>>>>>>
>>>>>>> Unless "git add foo" not only adds current contents of foo at stage 0,
>>>>>>> but also removes higher stages from index...
>>>>>>
>>>>>> By definition, adding anything at stage #0 is to remove higher stages.
>>>>>
>>>>> Hmmm... let's test it:
>>>>>
>>>>> $ git merge side-branch
>>>>> Auto-merging foo
>>>>> CONFLICT (content): Merge conflict in foo
>>>>> Automatic merge failed; fix conflicts and then commit the result.
>>>>> $ git ls-files --stage
>>>>> 100644 257cc5642cb1a054f08cc83f2d943e56fd3ebe99 1 foo
>>>>> 100644 3bd1f0e29744a1f32b08d5650e62e2e62afb177c 2 foo
>>>>> 100644 469a41eda5c8b45503a3bfc32ad6b5decc658132 3 foo
>>>>> $ <edit foo>
>>>>> $ git add foo
>>>>> $ git ls-files --stage
>>>>> 100644 a1b58d38ffa61e8e99b7cb95cdf540aedf2a96b3 0 foo
>>>
>>> I thought that "git add foo" only adds current contents of foo in stage 0,
>>> and does not delete other stages.
>>>
>>> Unless "git add foo" does more than "git update-index foo" does here.
>>
>> Quoting Junio:
>>
>> By definition, adding anything at stage #0 is to remove higher stages.
>>
>> Could one leave 1 alone but still mark the conflict resolved?
>
> 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.
>
> But I see that "git update-index --unresolve" (and its predecessor
> "git-unresolve") simply recreate stages #2 and #3.
>
>
> The documentation of "git update-index --unresolve" lacks this info,
> and it doesn't tell one what it is for (see commit message for commit
> ec16779 (Add git-unresolve <paths>..., 2006-04-19)).
>
Oh yes, one should always read the classics ;) [Really nice commit
message, that is.]
Michael
^ permalink raw reply
* Re: [PATCH 4/6] GITWEB - Makefile changes
From: Jakub Narebski @ 2009-12-11 14:28 UTC (permalink / raw)
To: John 'Warthog9' Hawley; +Cc: git, John 'Warthog9' Hawley
In-Reply-To: <1260488743-25855-5-git-send-email-warthog9@kernel.org>
"John 'Warthog9' Hawley" <warthog9@kernel.org> writes:
Below are _proposed_ changes to make commit message easier to read, in
my opinion. But they are not _necessary_ changes.
> This adjust the makefiles so that you can do such things as
Add "gitweb" target to main Makefile so you would be able to simply
use
>
> make gitweb
>
> from the top level make tree,
instead of requiring to spell it in full
make gitweb/gitweb.cgi
> or if your in the gitweb directory
> itself typing
Add Makefile in gitweb subdirectory so one can simply run
>
> make
when in gitweb subdirectory,
>
> will call back up to the main Makefile and build gitweb
>
> Signed-off-by: John 'Warthog9' Hawley <warthog9@eaglescrag.net>
Signoff mismatch.
> ---
> Makefile | 4 +++-
> gitweb/Makefile | 14 ++++++++++++++
> 2 files changed, 17 insertions(+), 1 deletions(-)
> create mode 100644 gitweb/Makefile
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.
> 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).
> +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?
> 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.
> +
> +all: $(FILES)
> +
> +$(FILES):
> + $(MAKE) $(MFLAGS) -C ../ -f Makefile gitweb/$@
> +
> +clean:
> + rm -rf $(FILES)
> +
--
Jakub Narebski
Poland
ShadeHawk on #git
^ permalink raw reply
* Re: [PATCH 1/6] GITWEB - Load Checking
From: Mihamina Rakotomandimby @ 2009-12-11 13:53 UTC (permalink / raw)
To: git; +Cc: John 'Warthog9' Hawley
In-Reply-To: <1260488743-25855-2-git-send-email-warthog9@kernel.org>
> "John 'Warthog9' Hawley" <warthog9@kernel.org> :
> + open($load, '<', '/proc/loadavg') or return 0;
What about systems not having /proc/loadavg
--
Architecte Informatique chez Blueline/Gulfsat:
Administration Systeme, Recherche & Developpement
+261 34 29 155 34 / +261 33 11 207 36
^ permalink raw reply
* Re: How to selectively recreate merge state?
From: Jakub Narebski @ 2009-12-11 14:00 UTC (permalink / raw)
To: Michael J Gruber; +Cc: Junio C Hamano, Jay Soffian, git
In-Reply-To: <4B223C1E.6010403@drmicha.warpmail.net>
Dnia piątek 11. grudnia 2009 13:33, Michael J Gruber napisał:
> Jakub Narebski venit, vidit, dixit 11.12.2009 12:20:
>> Dnia piątek 11. grudnia 2009 11:44, Michael J Gruber napisał:
>>> Jakub Narebski venit, vidit, dixit 11.12.2009 02:33:
>>>> Dnia piątek 11. grudnia 2009 02:11, Junio C Hamano napisał:
>>>>> Jakub Narebski <jnareb@gmail.com> writes:
>>>>>
>>>>>> --unresolve::
>>>>>> Restores the 'unmerged' or 'needs updating' state of a
>>>>>> file during a merge if it was cleared by accident.
>>>>>>
>>>>>> Unless "git add foo" not only adds current contents of foo at stage 0,
>>>>>> but also removes higher stages from index...
>>>>>
>>>>> By definition, adding anything at stage #0 is to remove higher stages.
>>>>
>>>> Hmmm... let's test it:
>>>>
>>>> $ git merge side-branch
>>>> Auto-merging foo
>>>> CONFLICT (content): Merge conflict in foo
>>>> Automatic merge failed; fix conflicts and then commit the result.
>>>> $ git ls-files --stage
>>>> 100644 257cc5642cb1a054f08cc83f2d943e56fd3ebe99 1 foo
>>>> 100644 3bd1f0e29744a1f32b08d5650e62e2e62afb177c 2 foo
>>>> 100644 469a41eda5c8b45503a3bfc32ad6b5decc658132 3 foo
>>>> $ <edit foo>
>>>> $ git add foo
>>>> $ git ls-files --stage
>>>> 100644 a1b58d38ffa61e8e99b7cb95cdf540aedf2a96b3 0 foo
>>
>> I thought that "git add foo" only adds current contents of foo in stage 0,
>> and does not delete other stages.
>>
>> Unless "git add foo" does more than "git update-index foo" does here.
>
> Quoting Junio:
>
> By definition, adding anything at stage #0 is to remove higher stages.
>
> Could one leave 1 alone but still mark the conflict resolved?
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.
But I see that "git update-index --unresolve" (and its predecessor
"git-unresolve") simply recreate stages #2 and #3.
The documentation of "git update-index --unresolve" lacks this info,
and it doesn't tell one what it is for (see commit message for commit
ec16779 (Add git-unresolve <paths>..., 2006-04-19)).
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [PATCH 3/6] GITWEB - Add git:// link to summary pages
From: Jakub Narebski @ 2009-12-11 13:44 UTC (permalink / raw)
To: John 'Warthog9' Hawley; +Cc: git, John 'Warthog9' Hawley
In-Reply-To: <1260488743-25855-4-git-send-email-warthog9@kernel.org>
"John 'Warthog9' Hawley" <warthog9@kernel.org> writes:
> This adds a git:// link to the summary pages should a common
> $gitlinkurl be defined (default is nothing defined, thus nothing
> shown)
>
> This does make the assumption that the git trees share a common
> path, and nothing to date is known to actually make use of the link
The problem I had and have with this patch is the duplication of data:
$gitlinkurl contains subset of information in @git_base_url_list,
which in turn is filled from GITWEB_BASE_URL build config variable.
I can understand that for performance reason you don't want to check
$projectroot/$project/cloneurl nor gitweb.url config variable for
each and every displayed project; if the link to repository (for git)
cannot be derived from project path (repository path), then simply
do not dosplay it.
>
> Signed-off-by: John 'Warthog9' Hawley <warthog9@eaglescrag.net>
> ---
> gitweb/gitweb.perl | 8 ++++++++
> 1 files changed, 8 insertions(+), 0 deletions(-)
>
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index d84f4c0..7ad096c 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -224,6 +224,10 @@ our %avatar_size = (
> # This is here to allow for missmatch git & gitweb versions
> our $missmatch_git = '';
>
> +#This is here to deal with an extra link on the summary pages - if it's left blank
> +# this link will not be shwon. If it's set, this will be prepended to the repo and used
s/shwon/shown/
I'd say that 'Full URL is "$gitlinkurl/$project"' instead of last
sentence in above comment.
Please watch for excessive line lengths.
> +our $gitlinkurl = '';
Why not
our $gitlinkurl_base = "++GITWEB_BASE_URL++";
of course changing the name everywhere.
> +
> # Used to set the maximum load that we will still respond to gitweb queries.
> # if we exceed this than we do the processing to figure out if there's a mirror
> # and redirect to it, or to just return 503 server busy
> @@ -4454,6 +4458,10 @@ sub git_project_list_body {
> $cgi->a({-href => href(project=>$pr->{'path'}, action=>"log")}, "log") . " | " .
> $cgi->a({-href => href(project=>$pr->{'path'}, action=>"tree")}, "tree") .
> ($pr->{'forks'} ? " | " . $cgi->a({-href => href(project=>$pr->{'path'}, action=>"forks")}, "forks") : '') .
> + if( $gitlinkurl ne '' ){
> + print " | ". $cgi->a({-href => "git://$gitlinkurl/".esc_html($pr->{'path'})}, "git");
> + }
> + print "".
Does it even pass tests?
$cgi->a({-href => href(project=>$pr->{'path'}, action=>"log")}, "log") . " | " .
$cgi->a({-href => href(project=>$pr->{'path'}, action=>"tree")}, "tree") .
($pr->{'forks'} ? " | " . $cgi->a({-href => href(project=>$pr->{'path'}, action=>"forks")}, "forks") : '') .
+ ($gitlinkurl_base ?
+ " | " . $cgi->a({-href=>"$gitlinkurl_base/$pr->{'path'}", "git") : '') .
"</td>\n" .
"</tr>\n";
}
Changes made:
* Instead of using separate if conditional statement and print
statement (note that you forgot to change '.' to ';' to end
statement) use ternary conditional operator "?:"
* Make $gitlinkurl_base include "git://" protocol specifier
* Do not create "git" link if $gitlinkurl_base is false, which means
undef, empty string '' and 0 (but 0 is not very likely to be base
for "git" link).
* Do not use esc_html on fragment of URL. The CGI.pm should escape
attributes itself. If it was HTTP link, one should perhaps esc_url
on whole link, but esc_html is for escaping HTML.
--
Jakub Narebski
Poland
ShadeHawk on #git
^ permalink raw reply
* Re: How to selectively recreate merge state?
From: Thomas Rast @ 2009-12-11 12:51 UTC (permalink / raw)
To: Björn Steinbrink
Cc: Michael J Gruber, Jakub Narebski, Junio C Hamano, Jay Soffian,
git
In-Reply-To: <20091211110944.GB19232@atjola.homenet>
Björn Steinbrink wrote:
> On 2009.12.11 11:44:25 +0100, Michael J Gruber wrote:
> > Jakub Narebski venit, vidit, dixit 11.12.2009 02:33:
> > > $ echo -e "100644 257cc5642cb1a054f08cc83f2d943e56fd3ebe99 1\tfoo" |
> > > git update-index --index-info
> >
> > Yeah, if we knew that sha1...
>
> Hm, isn't that "$(git merge-base HEAD MERGE_HEAD):foo"?
Not if the merge-base isn't unique and you're using the recursive
strategy, IIUC.
--
Thomas Rast
trast@{inf,student}.ethz.ch
^ permalink raw reply
* Re: [PATCH 3/6] GITWEB - Add git:// link to summary pages
From: Johannes Schindelin @ 2009-12-11 12:52 UTC (permalink / raw)
To: John 'Warthog9' Hawley; +Cc: git, John 'Warthog9' Hawley
In-Reply-To: <1260488743-25855-4-git-send-email-warthog9@kernel.org>
Hi,
On Thu, 10 Dec 2009, John 'Warthog9' Hawley wrote:
> This adds a git:// link to the summary pages should a common $gitlinkurl
> be defined (default is nothing defined, thus nothing shown)
Nice.
I forgot to mention in my comments to 2/6 that you seem to wrap after more
than 80 characters. However, I have no idea what the suggested line width
is for gitweb.
Again, this could be done by having the variable defined as undef.
Maybe it would be even nicer if the administrator could specify the
protocol, e.g. when they do not want/cannot allow git:// but only http://
access to the repositories?
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 2/6] GITWEB - Missmatching git w/ gitweb
From: Johannes Schindelin @ 2009-12-11 12:49 UTC (permalink / raw)
To: John 'Warthog9' Hawley; +Cc: git, John 'Warthog9' Hawley
In-Reply-To: <1260488743-25855-3-git-send-email-warthog9@kernel.org>
Hi,
On Thu, 10 Dec 2009, John 'Warthog9' Hawley wrote:
> This adds $missmatch_git so that gitweb can run with a miss-matched
> git install.
I'm not a native English speaker and all, but I thought it was spelt
'mismatch', i.e. with only one 's'. Maybe even name it
'allow_different_git_version' or 'no_strict_git_version'.
A few comments on the patch: the style of the if() statement disagrees
with the other ones; please use the same style.
Also, as with 1/6, turning off the feature might be better done by setting
it to undef.
Finally, would it not be nicer if the warning really was only a warning,
i.e. that the script would try to continue after giving the users a pretty
warning header?
Ciao,
Dscho
^ permalink raw reply
* Re: How to selectively recreate merge state?
From: Michael J Gruber @ 2009-12-11 12:33 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Junio C Hamano, Jay Soffian, git
In-Reply-To: <200912111220.40844.jnareb@gmail.com>
Jakub Narebski venit, vidit, dixit 11.12.2009 12:20:
> Dnia piątek 11. grudnia 2009 11:44, Michael J Gruber napisał:
>> Jakub Narebski venit, vidit, dixit 11.12.2009 02:33:
>>> Dnia piątek 11. grudnia 2009 02:11, Junio C Hamano napisał:
>>>> Jakub Narebski <jnareb@gmail.com> writes:
>>>>
>>>>> --unresolve::
>>>>> Restores the 'unmerged' or 'needs updating' state of a
>>>>> file during a merge if it was cleared by accident.
>>>>>
>>>>> Unless "git add foo" not only adds current contents of foo at stage 0,
>>>>> but also removes higher stages from index...
>>>>
>>>> By definition, adding anything at stage #0 is to remove higher stages.
>>>
>>> Hmmm... let's test it:
>>>
>>> $ git merge side-branch
>>> Auto-merging foo
>>> CONFLICT (content): Merge conflict in foo
>>> Automatic merge failed; fix conflicts and then commit the result.
>>> $ git ls-files --stage
>>> 100644 257cc5642cb1a054f08cc83f2d943e56fd3ebe99 1 foo
>>> 100644 3bd1f0e29744a1f32b08d5650e62e2e62afb177c 2 foo
>>> 100644 469a41eda5c8b45503a3bfc32ad6b5decc658132 3 foo
>>> $ <edit foo>
>>> $ git add foo
>>> $ git ls-files --stage
>>> 100644 a1b58d38ffa61e8e99b7cb95cdf540aedf2a96b3 0 foo
>
> I thought that "git add foo" only adds current contents of foo in stage 0,
> and does not delete other stages.
>
> Unless "git add foo" does more than "git update-index foo" does here.
Quoting Junio:
By definition, adding anything at stage #0 is to remove higher stages.
Could one leave 1 alone but still mark the conflict resolved?
>>> Now let's test '--unresolve' option of git-update-index:
>>>
>>> $ git update-index --unresolve foo
>>> $ git ls-files --stage foo
>>> 100644 3bd1f0e29744a1f32b08d5650e62e2e62afb177c 2 foo
>>> 100644 469a41eda5c8b45503a3bfc32ad6b5decc658132 3 foo
>>>
>>> WTF? What happened to stage 1 (ancestor)?
>>
>> 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.
Well, I'm not defending current behaviour, just describing its
implementation.
>
>>>
>>> $ git checkout --conflict=merge foo
>>> error: path 'foo' does not have all three versions
>>>
>>> Let's recover it by hand:
>>>
>>> $ echo -e "100644 257cc5642cb1a054f08cc83f2d943e56fd3ebe99 1\tfoo" |
>>> git update-index --index-info
>>> $ git ls-files --stage foo
>>> 100644 257cc5642cb1a054f08cc83f2d943e56fd3ebe99 1 foo
>>> 100644 3bd1f0e29744a1f32b08d5650e62e2e62afb177c 2 foo
>>> 100644 469a41eda5c8b45503a3bfc32ad6b5decc658132 3 foo
>>> $ git checkout --conflict=merge foo
>>
>> Yeah, if we knew that sha1...
>
> Isn't it:
>
> $ git ls-tree $(git merge-base HEAD MERGE_HEAD) -- foo
>
> or
>
> $ git rev-parse "$(git merge-base HEAD MERGE_HEAD):foo"
Yes, sure. That's why I wrote "cheap": --unresolve simply reads HEAD and
MERGE_HEAD. Resetting 1 requires (re)calculation of the merge base.
Michael
^ permalink raw reply
* git fetch with GIT_SSH fails with "The remote end hung up unexpectedly"
From: Gert Palok @ 2009-12-11 11:57 UTC (permalink / raw)
To: git
At my work I have to use an intermediate gateway to ssh to the outer
world. I have set up private-public keys to allow easy connection:
ssh gateway ssh user@outside
Now, I want to fetch from outside repo, so I created a GIT_SSH wrapper:
#! /bin/bash
LOG="/path/to/git-ssh-wrapper.
log"
HOST="$1"
COMMAND="$2"
echo host: $HOST >>"$LOG"
echo command: $COMMAND >>"$LOG"
echo exec: ssh gateway ssh "$HOST" $COMMAND >>"$LOG"
ssh gateway ssh "$HOST" $COMMAND
And ran:
$ GIT_SSH="/path/to/git-ssh-wrapper" git clone ssh://user@outside/path/to/repo
Initialized empty Git repository in /path/to/local-repo/.git/
warning: You appear to have cloned an empty repository.
$ fatal: The remote end hung up unexpectedly
And again just to be sure:
$ GIT_SSH="/path/to/git-ssh-wrapper" git fetch origin
$ fatal: The remote end hung up unexpectedly
fatal: The remote end hung up unexpectedly
Now, the log says:
host: user@outside
command: git-upload-pack '/path/to/repo'
exec: ssh gateway ssh user@outside git-upload-pack '/path/to/repo'
When I ran the command from shell I got:
$ "$GIT_SSH" user@outside "git-upload-pack '/path/to/repo'"
0000
And the connection was kept open (waiting for input, got protocol
error after entering something)
Local environment: Windows Vista 32-bit, cygwin 1.7
Local git version (installed by cygwin): 1.6.4.2
Remote git version: 1.6.4.4
What might be the cause(s)? Have there been compatibility breaking
protocol changes between those version?
--
Gert Palok
^ permalink raw reply
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