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