* Re: Convert 'git blame' to parse_options()
From: Junio C Hamano @ 2008-06-23 6:35 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Git Mailing List, Pierre Habouzit
In-Reply-To: <alpine.LFD.1.10.0806222207220.2926@woody.linux-foundation.org>
Linus Torvalds <torvalds@linux-foundation.org> writes:
> That builtin-blame option parsing is really ugly,...
Yeah, but wasn't it because it needed to be compatible with both annotate
syntax and rev-list style "range" notation at the same time?
> +static int blame_bottomtop_callback(const struct option *option, const char *arg, int unset)
> +{
> + const char **bottomtop = option->value;
> + if (!arg)
> + return -1;
> + if (*bottomtop)
> + die("More than one '-L n,m' option given");
> + *bottomtop = arg;
> + return 0;
> +}
Hmmmm. I actually wanted to eventually allow more than one -L so that we
can blame two functions inside a file, for example. Would this make it
even harder, I have to wonder...
^ permalink raw reply
* Re: [JGIT RFC PATCH] Add a stdio prompt for SSH connection information.
From: Shawn O. Pearce @ 2008-06-23 6:34 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: Marek Zawirski, git
In-Reply-To: <200806230823.20534.robin.rosenberg.lists@dewire.com>
Robin Rosenberg <robin.rosenberg.lists@dewire.com> wrote:
>
> > > I'm also a little unsure about how to invoke the promptKeyboardInteractive method.
> >
> > I think you implemented this method correctly. Its a confusing API,
> > but it does seem to make sense.
>
> No idea on how to make JSch inoke it?
Might require a server that has a token cards for authentication.
Server can ask for a user password and also give you a code to
enter into the card, which gives you the response to enter back in.
I have never worked with such a configuration so I have never seen
that sort of multi-input prompt come up in any SSH client before.
--
Shawn.
^ permalink raw reply
* Re: [JGIT RFC PATCH] Add a stdio prompt for SSH connection information.
From: Robin Rosenberg @ 2008-06-23 6:23 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Marek Zawirski, git
In-Reply-To: <20080622231355.GH11793@spearce.org>
måndagen den 23 juni 2008 01.13.55 skrev Shawn O. Pearce:
> I think that services on Windows have no console, and also can't
> talk to the desktop UI, so its both headless and without a console.
On Windows a service in Java throws up dialogs in void. Your programs will just hang. You have to tell awt that you are headless or your services may just hang. There is an option on services that makes it show windows on the console, but it's not the default and seems to have no effect on the headless property. As for the console() returning null or not I haven't tried it yet.
> > I'm also a little unsure about how to invoke the promptKeyboardInteractive method.
>
> I think you implemented this method correctly. Its a confusing API,
> but it does seem to make sense.
No idea on how to make JSch inoke it?
-- robin
^ permalink raw reply
* Re: Shrink the git binary a bit by avoiding unnecessary inline functions
From: Christian MICHON @ 2008-06-23 5:26 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <alpine.LFD.1.10.0806222150180.2926@woody.linux-foundation.org>
On Mon, Jun 23, 2008 at 6:54 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> gcc version 4.3.0 20080428 (Red Hat 4.3.0-8) (GCC)
>
> Also notice that this is x86-64, maybe you're 32-bit?
it is an AMD 64, but the ubuntu and detaolb are indeed 32 bits distros.
>
> You also have much smaller size difference (616606 -> 591554 is just over
> 16kB), but if my version of gcc bloats things up more, then I'm not
> surprised that the difference is bigger too.
>
I guess I should compare results between compiler versions too...
Thanks for the explainations.
--
Christian
--
http://detaolb.sourceforge.net/, a linux distribution for Qemu with Git inside !
^ permalink raw reply
* Convert 'git blame' to parse_options()
From: Linus Torvalds @ 2008-06-23 5:15 UTC (permalink / raw)
To: Git Mailing List, Junio C Hamano, Pierre Habouzit
Why? I did some more size statistics, and 'cmd_blame()' was the biggest
function in all of git. Admittedly my script for finding sizes is probably
broken, but it does look very messy.
Just cleaning up the option parsing actually cuts down the size a bit, and
while parse_options() isn't really perfect for what cmd_blame() wants,
it's manageable.
I think I'll want to add a PARSE_OPT_IGNORE_UNRECOGNIZED flag, and also
make it not write the resulting array into argv[0..] but back into
argv[1..] (so that you can use parse_options() as a _filter_ for options
parsing and make it easier to do partial conversions), but in the meantime
this mostly works.
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
---
That builtin-blame option parsing is really ugly, trying to then move
unrecognized options to be parsed by a secondary
setup_revisions(unk, argv, &revs, NULL);
which parses the rest. This is _really_ not what parse_options() was
written with in mind, and to give strictly the same behaviour as before
you really do need some kind of PARSE_OPT_IGNORE_UNRECOGNIZED flag, but I
think this is already a good enough halfway point.
Hmm?
Btw, making the "struct options" array be 'static' avoids having to build
it up at run-time on the stack, which is why it's usually a good idea. Of
course, it means that the option variables themselves have to be static,
which has its own set of downsides, so it's a balancing act.
builtin-blame.c | 177 +++++++++++++++++++++++++++----------------------------
1 files changed, 86 insertions(+), 91 deletions(-)
diff --git a/builtin-blame.c b/builtin-blame.c
index b451f6c..f04bd93 100644
--- a/builtin-blame.c
+++ b/builtin-blame.c
@@ -18,24 +18,11 @@
#include "cache-tree.h"
#include "path-list.h"
#include "mailmap.h"
+#include "parse-options.h"
-static char blame_usage[] =
-"git-blame [-c] [-b] [-l] [--root] [-t] [-f] [-n] [-s] [-p] [-w] [-L n,m] [-S <revs-file>] [-M] [-C] [-C] [--contents <filename>] [--incremental] [commit] [--] file\n"
-" -c Use the same output mode as git-annotate (Default: off)\n"
-" -b Show blank SHA-1 for boundary commits (Default: off)\n"
-" -l Show long commit SHA1 (Default: off)\n"
-" --root Do not treat root commits as boundaries (Default: off)\n"
-" -t Show raw timestamp (Default: off)\n"
-" -f, --show-name Show original filename (Default: auto)\n"
-" -n, --show-number Show original linenumber (Default: off)\n"
-" -s Suppress author name and timestamp (Default: off)\n"
-" -p, --porcelain Show in a format designed for machine consumption\n"
-" -w Ignore whitespace differences\n"
-" -L n,m Process only line range n,m, counting from 1\n"
-" -M, -C Find line movements within and across files\n"
-" --incremental Show blame entries as we find them, incrementally\n"
-" --contents file Use <file>'s contents as the final image\n"
-" -S revs-file Use revisions from revs-file instead of calling git-rev-list\n";
+static char blame_usage[] = "git-blame [-c] [-b] [-l] [--root] [-t] [-f] [-n] [-s] [-p] [-w] [-L n,m] [-S <revs-file>] [-M] [-C] [-C] [--contents <filename>] [--incremental] [commit] [--] file";
+
+static const char *blame_opt_usage[] = { blame_usage, NULL };
static int longest_file;
static int longest_author;
@@ -2122,6 +2109,50 @@ static struct commit *fake_working_tree_commit(const char *path, const char *con
return commit;
}
+static int blame_copy_callback(const struct option *option, const char *arg, int unset)
+{
+ int *opt = option->value;
+
+ /*
+ * -C enables copy from removed files;
+ * -C -C enables copy from existing files, but only
+ * when blaming a new file;
+ * -C -C -C enables copy from existing files for
+ * everybody
+ */
+ if (*opt & PICKAXE_BLAME_COPY_HARDER)
+ *opt |= PICKAXE_BLAME_COPY_HARDEST;
+ if (*opt & PICKAXE_BLAME_COPY)
+ *opt |= PICKAXE_BLAME_COPY_HARDER;
+ *opt |= PICKAXE_BLAME_COPY | PICKAXE_BLAME_MOVE;
+
+ if (arg)
+ blame_copy_score = parse_score(arg);
+ return 0;
+}
+
+static int blame_move_callback(const struct option *option, const char *arg, int unset)
+{
+ int *opt = option->value;
+
+ *opt |= PICKAXE_BLAME_MOVE;
+
+ if (arg)
+ blame_move_score = parse_score(arg);
+ return 0;
+}
+
+static int blame_bottomtop_callback(const struct option *option, const char *arg, int unset)
+{
+ const char **bottomtop = option->value;
+ if (!arg)
+ return -1;
+ if (*bottomtop)
+ die("More than one '-L n,m' option given");
+ *bottomtop = arg;
+ return 0;
+}
+
int cmd_blame(int argc, const char **argv, const char *prefix)
{
struct rev_info revs;
@@ -2129,94 +2160,58 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
struct scoreboard sb;
struct origin *o;
struct blame_entry *ent;
- int i, seen_dashdash, unk, opt;
+ int i, seen_dashdash, unk;
long bottom, top, lno;
- int output_option = 0;
- int show_stats = 0;
- const char *revs_file = NULL;
const char *final_commit_name = NULL;
enum object_type type;
- const char *bottomtop = NULL;
- const char *contents_from = NULL;
+
+ static const char *bottomtop = NULL;
+ static int output_option = 0, opt = 0;
+ static int show_stats = 0;
+ static const char *revs_file = NULL;
+ static const char *contents_from = NULL;
+ static const struct option options[] = {
+ OPT_BOOLEAN(0, "incremental", &incremental, "Show blame entries as we find them, incrementally"),
+ OPT_BOOLEAN('b', NULL, &blank_boundary, "Show blank SHA-1 for boundary commits (Default: off)"),
+ OPT_BOOLEAN(0, "root", &show_root, "Do not treat root commits as boundaries (Default: off)"),
+ OPT_BOOLEAN(0, "show-stats", &show_stats, "Show work cost statistics"),
+ OPT_BIT(0, "score-debug", &output_option, "Show output score for blame entries", OUTPUT_SHOW_SCORE),
+ OPT_BIT('f', "show-name", &output_option, "Show original filename (Default: auto)", OUTPUT_SHOW_NAME),
+ OPT_BIT('n', "show-number", &output_option, "Show original linenumber (Default: off)", OUTPUT_SHOW_NUMBER),
+ OPT_BIT('p', "porcelain", &output_option, "Show in a format designed for machine consumption", OUTPUT_PORCELAIN),
+ OPT_BIT('c', NULL, &output_option, "Use the same output mode as git-annotate (Default: off)", OUTPUT_ANNOTATE_COMPAT),
+ OPT_BIT('t', NULL, &output_option, "Show raw timestamp (Default: off)", OUTPUT_RAW_TIMESTAMP),
+ OPT_BIT('l', NULL, &output_option, "Show long commit SHA1 (Default: off)", OUTPUT_LONG_OBJECT_NAME),
+ OPT_BIT('s', NULL, &output_option, "Suppress author name and timestamp (Default: off)", OUTPUT_NO_AUTHOR),
+ OPT_BIT('w', NULL, &xdl_opts, "Ignore whitespace differences", XDF_IGNORE_WHITESPACE),
+ OPT_STRING('S', NULL, &revs_file, "file", "Use revisions from <file> instead of calling git-rev-list"),
+ OPT_STRING(0, "contents", &contents_from, "file", "Use <file>'s contents as the final image"),
+ { OPTION_CALLBACK, 'C', NULL, &opt, "score", "Find line copies within and across files", PARSE_OPT_OPTARG, blame_copy_callback },
+ { OPTION_CALLBACK, 'M', NULL, &opt, "score", "Find line movements within and across files", PARSE_OPT_OPTARG, blame_move_callback },
+ OPT_CALLBACK('L', NULL, &bottomtop, "n,m", "Process only line range n,m, counting from 1", blame_bottomtop_callback),
+ OPT_END()
+ };
cmd_is_annotate = !strcmp(argv[0], "annotate");
+ argc = parse_options(argc, argv, options, blame_opt_usage,
+ PARSE_OPT_STOP_AT_NON_OPTION | PARSE_OPT_KEEP_DASHDASH);
+
+ /*
+ * parse_options() offsets things by one - undo for now to make
+ * old code happy.
+ */
+ argc++;
+ argv--;
+
git_config(git_blame_config, NULL);
save_commit_buffer = 0;
- opt = 0;
seen_dashdash = 0;
for (unk = i = 1; i < argc; i++) {
const char *arg = argv[i];
if (*arg != '-')
break;
- else if (!strcmp("-b", arg))
- blank_boundary = 1;
- else if (!strcmp("--root", arg))
- show_root = 1;
- else if (!strcmp(arg, "--show-stats"))
- show_stats = 1;
- else if (!strcmp("-c", arg))
- output_option |= OUTPUT_ANNOTATE_COMPAT;
- else if (!strcmp("-t", arg))
- output_option |= OUTPUT_RAW_TIMESTAMP;
- else if (!strcmp("-l", arg))
- output_option |= OUTPUT_LONG_OBJECT_NAME;
- else if (!strcmp("-s", arg))
- output_option |= OUTPUT_NO_AUTHOR;
- else if (!strcmp("-w", arg))
- xdl_opts |= XDF_IGNORE_WHITESPACE;
- else if (!strcmp("-S", arg) && ++i < argc)
- revs_file = argv[i];
- else if (!prefixcmp(arg, "-M")) {
- opt |= PICKAXE_BLAME_MOVE;
- blame_move_score = parse_score(arg+2);
- }
- else if (!prefixcmp(arg, "-C")) {
- /*
- * -C enables copy from removed files;
- * -C -C enables copy from existing files, but only
- * when blaming a new file;
- * -C -C -C enables copy from existing files for
- * everybody
- */
- if (opt & PICKAXE_BLAME_COPY_HARDER)
- opt |= PICKAXE_BLAME_COPY_HARDEST;
- if (opt & PICKAXE_BLAME_COPY)
- opt |= PICKAXE_BLAME_COPY_HARDER;
- opt |= PICKAXE_BLAME_COPY | PICKAXE_BLAME_MOVE;
- blame_copy_score = parse_score(arg+2);
- }
- else if (!prefixcmp(arg, "-L")) {
- if (!arg[2]) {
- if (++i >= argc)
- usage(blame_usage);
- arg = argv[i];
- }
- else
- arg += 2;
- if (bottomtop)
- die("More than one '-L n,m' option given");
- bottomtop = arg;
- }
- else if (!strcmp("--contents", arg)) {
- if (++i >= argc)
- usage(blame_usage);
- contents_from = argv[i];
- }
- else if (!strcmp("--incremental", arg))
- incremental = 1;
- else if (!strcmp("--score-debug", arg))
- output_option |= OUTPUT_SHOW_SCORE;
- else if (!strcmp("-f", arg) ||
- !strcmp("--show-name", arg))
- output_option |= OUTPUT_SHOW_NAME;
- else if (!strcmp("-n", arg) ||
- !strcmp("--show-number", arg))
- output_option |= OUTPUT_SHOW_NUMBER;
- else if (!strcmp("-p", arg) ||
- !strcmp("--porcelain", arg))
- output_option |= OUTPUT_PORCELAIN;
else if (!strcmp("--", arg)) {
seen_dashdash = 1;
i++;
^ permalink raw reply related
* Re: Shrink the git binary a bit by avoiding unnecessary inline functions
From: Linus Torvalds @ 2008-06-23 4:54 UTC (permalink / raw)
To: Christian MICHON; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <46d6db660806221801j7207e6b0sdf91c2243fad5349@mail.gmail.com>
On Mon, 23 Jun 2008, Christian MICHON wrote:
>
> There's more than 80k difference with your figures (I have not even
> applied your patch yet).
> May I ask which version of gcc you're using ? The numbers you provided
> were really with -Os ?
gcc version 4.3.0 20080428 (Red Hat 4.3.0-8) (GCC)
Also notice that this is x86-64, maybe you're 32-bit?
You also have much smaller size difference (616606 -> 591554 is just over
16kB), but if my version of gcc bloats things up more, then I'm not
surprised that the difference is bigger too.
Linus
^ permalink raw reply
* Re: git blame for a commit
From: Ian Hilt @ 2008-06-23 3:40 UTC (permalink / raw)
To: Mircea Bardac; +Cc: git
In-Reply-To: <485ED2E5.3070906@mircea.bardac.net>
On Sun, 22 Jun 2008 at 11:32pm +0100, Mircea Bardac wrote:
> Hi everyone,
>
> Is there any straightforward way of doing git blame for all the files that got
> changed in a commit. Problems are renames, deletes and copies.
Sounds like you want to track files rather than content. Git tracks the
latter.
> I was also thinking of git diff with a huge number of context lines, but this
> one feels a bit hacking. "git diff" is also missing author info, so "git
> blame" is a bit more desirable.
>
> Has anyone ever done this before?
>
> Many thanks.
Try "git diff --name-only commit1..commit2" or "git log --name-only
commit1..commit2". This will give you only the names of the files in
which content was altered. Other than that, I think you should re-read
(hint) git-blame(1). It's focus is on content, not files as a whole.
--
Ian Hilt
Ian.Hilt (at) gmx.com
GnuPG key: 0x4AFC1EE3
^ permalink raw reply
* Re: [jgit PATCH] Paper bag fix quoting for SSH transport commands
From: Robin Rosenberg @ 2008-06-23 2:30 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git, Marek Zawirski
In-Reply-To: <20080622221545.GF11793@spearce.org>
måndagen den 23 juni 2008 00.15.45 skrev Shawn O. Pearce:
> Robin Rosenberg <robin.rosenberg.lists@dewire.com> wrote:
> > As for pushing and signing. One way is for you (Shawn) and me is
> > to sign-off and push each other's patches. I pushed this one.
>
> Given that repo.or.cz doesn't show reflogs, I take it this is only
> a way to make sure at least someone has reviewed the patch before
> it goes into the main tree, since we both have write access? I
> can live with that.
We'll try that (unless we need pushing desperately).
-- robin
^ permalink raw reply
* Re: [PATCH v6] gitweb: add test suite with Test::WWW::Mechanize::CGI
From: Junio C Hamano @ 2008-06-23 2:30 UTC (permalink / raw)
To: Lea Wiemann; +Cc: git, Jakub Narebski
In-Reply-To: <1214183688-8544-1-git-send-email-LeWiemann@gmail.com>
Lea Wiemann <lewiemann@gmail.com> writes:
> Changed since v5: honor $ENV{GITPERL} for gitweb.cgi execution, not
> just test execution:
>
> diff --git a/t/t9503/test.pl b/t/t9503/test.pl
> index 28894c5..947e2e8 100755
> --- a/t/t9503/test.pl
> +++ b/t/t9503/test.pl
> @@ -54,5 +54,5 @@ my $gitweb = abs_path(File::Spec->catfile('..', '..', 'gitweb', 'gitweb.cgi'));
> my $cgi = sub {
> # Use exec, not the shell, to support blanks in the path.
> - my $status = system $gitweb $gitweb;
> + my $status = system { $ENV{GITPERL} } $ENV{GITPERL}, $gitweb;
> my $value = $status >> 8;
My Perl must be getting rusty. I had to look up this tricky syntax in
perlfunc.pod, under "=item exec", with examples like:
$shell = '/bin/csh';
exec $shell '-sh'; # pretend it's a login shell
or, more directly,
exec {'/bin/csh'} '-sh'; # pretend it's a login shell
> t/t9503-gitweb-Mechanize.sh | 132 ++++++++++++++++
> t/t9503/test.pl | 354 +++++++++++++++++++++++++++++++++++++++++++
> t/test-lib.sh | 2 +
> 3 files changed, 488 insertions(+), 0 deletions(-)
> create mode 100755 t/t9503-gitweb-Mechanize.sh
> create mode 100755 t/t9503/test.pl
>
> diff --git a/t/t9503-gitweb-Mechanize.sh b/t/t9503-gitweb-Mechanize.sh
> new file mode 100755
> index 0000000..3fe6d8b
> --- /dev/null
> +++ b/t/t9503-gitweb-Mechanize.sh
> @@ -0,0 +1,132 @@
> +#!/bin/sh
> +#
> +# Copyright (c) 2008 Jakub Narebski
> +# Copyright (c) 2008 Lea Wiemann
If you mean by this that originally Jakub started and then Lea continued
extending the work, probably the order of Sign-off should match that order
to express the patch flow trail better.
> +# check if test can be run
> +"$GITPERL" -MEncode -e 'decode_utf8("", Encode::FB_CROAK)' >/dev/null 2>&1 || {
> + test_expect_success \
> + 'skipping gitweb tests, perl version is too old' :
> + test_done
> + exit
> +}
> +
> +"$GITPERL" -MTest::WWW::Mechanize::CGI -e '' >/dev/null 2>&1 || {
> + test_expect_success \
> + 'skipping gitweb tests, Test::WWW::Mechanize::CGI not found' :
> + test_done
> + exit
> +}
Nice touch.
^ permalink raw reply
* Re: core.autocrlf and merge conflict output
From: Edward Z. Yang @ 2008-06-23 2:23 UTC (permalink / raw)
To: git; +Cc: Johannes.Schindelin
In-Reply-To: <alpine.DEB.1.00.0806230259140.6439@racer>
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Johannes Schindelin wrote:
> Funny. And I thought 1.5.6 contained that patch... Because 1.5.6 was
> merged by Steffen recently, and announced on both the msysgit list as well
> as this one.
And I'm wrong once again. My git checkout was on a detached head and
wasn't updating (apparently, I'm no good at reading error messages).
Switching back to master, pulling and then re-compiling fixed it.
While 99% of this is my fault, there might be a bug in netinstall.
However, I'm going to take that one to the msysgit list.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
iD8DBQFIXwk/qTO+fYacSNoRAkIhAJ9LU6jH5ThQbu0tSjmkbp7ynEyh5wCdHvbJ
pFVPllbiXjfLrgEYuY+uVlM=
=ZxbH
-----END PGP SIGNATURE-----
^ permalink raw reply
* Re: core.autocrlf and merge conflict output
From: Johannes Schindelin @ 2008-06-23 2:00 UTC (permalink / raw)
To: Edward Z. Yang; +Cc: git
In-Reply-To: <g3mvbh$mmj$1@ger.gmane.org>
Hi,
On Sun, 22 Jun 2008, Edward Z. Yang wrote:
> Junio C Hamano wrote:
> > Is this an old news before 249c61a (merge-recursive: respect
> > core.autocrlf when writing out the result, 2008-06-09) happened?
>
> Apparently, yes. :o) Thus are the perils of working off a "downstream"
> branch. I'll go bug the git/mingw/4msysgit.git maintainers to get this
> change merged in. Thanks!
Funny. And I thought 1.5.6 contained that patch... Because 1.5.6 was
merged by Steffen recently, and announced on both the msysgit list as well
as this one.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH/RFC] git.el: Commands for committing patches
From: Edward Z. Yang @ 2008-06-23 1:59 UTC (permalink / raw)
To: git
In-Reply-To: <m2prq9ujpa.fsf@nschum.de>
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Nikolaj Schumacher wrote:
[blank]
As per Documentation/SubmittingPatches, we'd appreciate it if you
submitted the patch inline. Thanks!
- From SubmittingPatches:
> People on the git mailing list need to be able to read and
> comment on the changes you are submitting. It is important for
> a developer to be able to "quote" your changes, using standard
> e-mail tools, so that they may comment on specific portions of
> your code. For this reason, all patches should be submitted
> "inline". WARNING: Be wary of your MUAs word-wrap
> corrupting your patch. Do not cut-n-paste your patch; you can
> lose tabs that way if you are not careful.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
iD8DBQFIXwOIqTO+fYacSNoRAt+kAJ9AF8D560SV9liqQ1XTPM0g1i56PgCfVP19
J5FXtNyDj/CEYxolhO0rKSk=
=RNRN
-----END PGP SIGNATURE-----
^ permalink raw reply
* Re: core.autocrlf and merge conflict output
From: Junio C Hamano @ 2008-06-23 1:54 UTC (permalink / raw)
To: Edward Z. Yang; +Cc: git
In-Reply-To: <g3mvbh$mmj$1@ger.gmane.org>
"Edward Z. Yang" <edwardzyang@thewritingpot.com> writes:
> Junio C Hamano wrote:
>> Is this an old news before 249c61a (merge-recursive: respect core.autocrlf
>> when writing out the result, 2008-06-09) happened?
>
> Apparently, yes. :o) Thus are the perils of working off a "downstream"
> branch. I'll go bug the git/mingw/4msysgit.git maintainers to get this
> change merged in. Thanks!
Did cherry-picking that particular commit locally fixes the issue for you?
^ permalink raw reply
* Re: core.autocrlf and merge conflict output
From: Edward Z. Yang @ 2008-06-23 1:47 UTC (permalink / raw)
To: git
In-Reply-To: <7vy74xj7ww.fsf@gitster.siamese.dyndns.org>
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Junio C Hamano wrote:
> Is this an old news before 249c61a (merge-recursive: respect core.autocrlf
> when writing out the result, 2008-06-09) happened?
Apparently, yes. :o) Thus are the perils of working off a "downstream"
branch. I'll go bug the git/mingw/4msysgit.git maintainers to get this
change merged in. Thanks!
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
iD8DBQFIXwC4qTO+fYacSNoRAi8RAJ4/ZF0C0XB9QSPgCnQYae/33YSRUgCff/y5
5Dd9vy/pQQsNJbyQ+xfc4lY=
=fyGn
-----END PGP SIGNATURE-----
^ permalink raw reply
* Re: [PATCH 1/2] parse_options: Add flag to prevent errors for further processing
From: Junio C Hamano @ 2008-06-23 1:45 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: Jeff King, Shawn Bohrer, git
In-Reply-To: <20080622170733.GA16252@artemis.madism.org>
Pierre Habouzit <madcoder@debian.org> writes:
> On Wed, Jun 18, 2008 at 05:13:02AM +0000, Junio C Hamano wrote:
>> Jeff King <peff@peff.net> writes:
>>
>> > I think the only right way to accomplish this is to convert the revision
>> > and diff parameters into a parseopt-understandable format.
>>
>> Not necessarily. You could structure individual option parsers like how
>> diff option parsers are done. You iterate over argv[], feed diff option
>> parser the current index into argv[] and ask if it is an option diff
>> understands, have diff eat the option (and possibly its parameter) to
>> advance the index, or allow diff option to say "I do not understand this",
>> and then handle it yourself or hand it to other parsers.
>
> If you do that, you need to relocate pars option structures,...
> ... Note that "recursing"
> is not really trivial, because with flags aggregation and stuff like
> that, things that look like an option can also be a value in the context
> of an other option parser.
Note that I was just saying "not necessarily" in response to "the only
right way" to point out it is not the _only_ way.
Parse-options has been done in a tablish way and it would involve cost to
modify it in a way I outlined (even if such a rewrite would make chaining
different set of option parsers easier, as each parser needs to handle
only what it knows about and handling aggregation and stuff would become
trivial). I do not know if it is worth the cost, and I am not married to
the option parser structure that diff and revision part uses.
^ permalink raw reply
* [PATCH v6] gitweb: add test suite with Test::WWW::Mechanize::CGI
From: Lea Wiemann @ 2008-06-23 1:14 UTC (permalink / raw)
To: git; +Cc: Lea Wiemann, Jakub Narebski
In-Reply-To: <1214181904-30265-1-git-send-email-LeWiemann@gmail.com>
This test uses Test::WWW::Mechanize::CGI to check gitweb's output. It
also uses HTML::Lint, XML::Parser, and Archive::Tar (if present, each)
to validate the HTML/XML/tgz output, and checks all links on the
tested pages if --long-tests is given.
Also add a GITPERL environment variable that allows running Perl-based
tests with different perl binaries (and thus under different
versions).
Signed-off-by: Lea Wiemann <LeWiemann@gmail.com>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Changed since v5: honor $ENV{GITPERL} for gitweb.cgi execution, not
just test execution:
diff --git a/t/t9503/test.pl b/t/t9503/test.pl
index 28894c5..947e2e8 100755
--- a/t/t9503/test.pl
+++ b/t/t9503/test.pl
@@ -54,5 +54,5 @@ my $gitweb = abs_path(File::Spec->catfile('..', '..', 'gitweb', 'gitweb.cgi'));
my $cgi = sub {
# Use exec, not the shell, to support blanks in the path.
- my $status = system $gitweb $gitweb;
+ my $status = system { $ENV{GITPERL} } $ENV{GITPERL}, $gitweb;
my $value = $status >> 8;
t/t9503-gitweb-Mechanize.sh | 132 ++++++++++++++++
t/t9503/test.pl | 354 +++++++++++++++++++++++++++++++++++++++++++
t/test-lib.sh | 2 +
3 files changed, 488 insertions(+), 0 deletions(-)
create mode 100755 t/t9503-gitweb-Mechanize.sh
create mode 100755 t/t9503/test.pl
diff --git a/t/t9503-gitweb-Mechanize.sh b/t/t9503-gitweb-Mechanize.sh
new file mode 100755
index 0000000..3fe6d8b
--- /dev/null
+++ b/t/t9503-gitweb-Mechanize.sh
@@ -0,0 +1,132 @@
+#!/bin/sh
+#
+# Copyright (c) 2008 Jakub Narebski
+# Copyright (c) 2008 Lea Wiemann
+#
+
+# This test supports the --long-tests option.
+
+# This test only runs on Perl 5.8 and later versions, since
+# Test::WWW::Mechanize::CGI requires Perl 5.8.
+
+test_description='gitweb tests (using WWW::Mechanize)
+
+This test uses Test::WWW::Mechanize::CGI to test gitweb.'
+
+# helper functions
+
+safe_chmod () {
+ chmod "$1" "$2" &&
+ if [ "$(git config --get core.filemode)" = false ]
+ then
+ git update-index --chmod="$1" "$2"
+ fi
+}
+
+. ./test-lib.sh
+
+# check if test can be run
+"$GITPERL" -MEncode -e 'decode_utf8("", Encode::FB_CROAK)' >/dev/null 2>&1 || {
+ test_expect_success \
+ 'skipping gitweb tests, perl version is too old' :
+ test_done
+ exit
+}
+
+"$GITPERL" -MTest::WWW::Mechanize::CGI -e '' >/dev/null 2>&1 || {
+ test_expect_success \
+ 'skipping gitweb tests, Test::WWW::Mechanize::CGI not found' :
+ test_done
+ exit
+}
+
+# set up test repository
+test_expect_success 'set up test repository' '
+
+ echo "Not an empty file." > file &&
+ git add file &&
+ test_tick && git commit -a -m "Initial commit." &&
+ git branch b &&
+
+ echo "New file" > new_file &&
+ git add new_file &&
+ test_tick && git commit -a -m "File added." &&
+
+ safe_chmod +x new_file &&
+ test_tick && git commit -a -m "Mode changed." &&
+
+ git mv new_file renamed_file &&
+ test_tick && git commit -a -m "File renamed." &&
+
+ rm renamed_file &&
+ ln -s file renamed_file &&
+ test_tick && git commit -a -m "File to symlink." &&
+ git tag with-symlink &&
+
+ git rm renamed_file &&
+ rm -f renamed_file &&
+ test_tick && git commit -a -m "File removed." &&
+
+ cp file file2 &&
+ git add file2 &&
+ test_tick && git commit -a -m "File copied." &&
+
+ echo "New line" >> file2 &&
+ safe_chmod +x file2 &&
+ test_tick && git commit -a -m "Mode change and modification." &&
+
+ git checkout b &&
+ echo "Branch" >> b &&
+ git add b &&
+ test_tick && git commit -a -m "On branch" &&
+ git checkout master &&
+ test_tick && git pull . b
+'
+
+# set up empty repository
+# TODO!
+
+# set up repositories for gitweb
+# TODO!
+
+# set up gitweb configuration
+safe_pwd="$("$GITPERL" -MPOSIX=getcwd -e 'print quotemeta(getcwd)')"
+cat >gitweb_config.perl <<EOF
+# gitweb configuration for tests
+
+our \$version = "current";
+our \$GIT = "$GIT_EXEC_PATH/git";
+our \$projectroot = "$safe_pwd";
+our \$project_maxdepth = 8;
+our \$home_link_str = "projects";
+our \$site_name = "[localhost]";
+our \$site_header = "";
+our \$site_footer = "";
+our \$home_text = "indextext.html";
+our @stylesheets = ("file:///$safe_pwd/../../gitweb/gitweb.css");
+our \$logo = "file:///$safe_pwd/../../gitweb/git-logo.png";
+our \$favicon = "file:///$safe_pwd/../../gitweb/git-favicon.png";
+our \$projects_list = "";
+our \$export_ok = "";
+our \$strict_export = "";
+our %feature;
+\$feature{'blame'}{'default'} = [1];
+
+1;
+__END__
+EOF
+
+cat >.git/description <<EOF
+$0 test repository
+EOF
+
+GITWEB_CONFIG="$(pwd)/gitweb_config.perl"
+export GITWEB_CONFIG
+
+# run tests
+
+test_external \
+ 'test gitweb output' \
+ "$GITPERL" ../t9503/test.pl
+
+test_done
diff --git a/t/t9503/test.pl b/t/t9503/test.pl
new file mode 100755
index 0000000..947e2e8
--- /dev/null
+++ b/t/t9503/test.pl
@@ -0,0 +1,354 @@
+#!/usr/bin/perl
+use lib (split(/:/, $ENV{GITPERLLIB}));
+
+# This test supports the --long-tests option.
+
+use warnings;
+use strict;
+
+use Cwd qw(abs_path);
+use File::Spec;
+use File::Temp;
+
+# We don't count properly when skipping, so no_plan is necessary.
+use Test::More qw(no_plan);
+use Test::WWW::Mechanize::CGI;
+
+our $long_tests = $ENV{GIT_TEST_LONG};
+
+eval { require Archive::Tar; };
+my $archive_tar_installed = !$@
+ or diag('Archive::Tar is not installed; no tests for valid snapshots');
+
+eval { require HTML::Lint; };
+my $html_lint_installed = !$@
+ or diag('HTML::Lint is not installed; no HTML validation tests');
+
+eval { require XML::Parser; };
+my $xml_parser_installed = !$@
+ or diag('XML::Parser is not installed; no tests for well-formed XML');
+
+my @revisions = split /\s/, `git-rev-list --first-parent HEAD`;
+chomp(my @heads = map { (split('/', $_))[2] } `git-for-each-ref --sort=-committerdate refs/heads`);
+chomp(my @tags = map { (split('/', $_))[2] } `git-for-each-ref --sort=-committerdate refs/tags`);
+# files and directories in HEAD root:
+chomp(my @files = map { (split("\t", $_))[1] } `git-ls-tree HEAD`);
+
+sub rev_parse {
+ my $name = shift;
+ chomp(my $hash = `git rev-parse $name 2> /dev/null`);
+ $hash or undef;
+}
+
+sub get_type {
+ my $name = shift;
+ chomp(my $type = `git cat-file -t $name 2> /dev/null`);
+ $type or undef;
+}
+
+my $gitweb = abs_path(File::Spec->catfile('..', '..', 'gitweb', 'gitweb.cgi'));
+
+# Thus subroutine was copied (and modified to work with blanks in the
+# application path) from WWW::Mechanize::CGI 0.3, which is licensed
+# 'under the same terms as perl itself' and thus GPL compatible.
+my $cgi = sub {
+ # Use exec, not the shell, to support blanks in the path.
+ my $status = system { $ENV{GITPERL} } $ENV{GITPERL}, $gitweb;
+ my $value = $status >> 8;
+
+ croak( qq/Failed to execute application '$gitweb'. Reason: '$!'/ )
+ if ( $status == -1 );
+ croak( qq/Application '$gitweb' exited with value: $value/ )
+ if ( $value > 0 );
+};
+
+my $mech = new Test::WWW::Mechanize::CGI;
+$mech->cgi($cgi);
+# On some systems(?) it's necessary to have %ENV here, otherwise the
+# CGI process won't get *any* of the current environment variables
+# (not even PATH, etc.)
+$mech->env(%ENV,
+ GITWEB_CONFIG => $ENV{'GITWEB_CONFIG'},
+ SCRIPT_FILENAME => $gitweb,
+ $mech->env);
+
+# import config, predeclaring config variables
+our $site_name;
+require_ok($ENV{'GITWEB_CONFIG'})
+ or diag('Could not load gitweb config; some tests would fail');
+
+# Perform non-recursive checks on the current page, but do not check
+# the status code.
+my %verified_uris;
+sub _verify_page {
+ my ($uri, $fragment) = split '#', $mech->uri;
+ if (!$verified_uris{$uri}) {
+ $verified_uris{$uri} = 1;
+
+ # Internal errors yield 200, but cause gitweb.cgi to
+ # exit with non-zero exit code, which Mechanize::CGI
+ # translates to 500, so we don't really need to check
+ # for "Software error" here, provided that the test
+ # cases always check the status code.
+ #$mech->content_lacks('<h1>Software error:</h1>') or return 0;
+
+ # Validating is fast, so we can do it even without
+ # $long_tests.
+ $mech->html_lint_ok('[auto] validate HTML') or return 0
+ if $html_lint_installed && $mech->is_html;
+ my $content_type = $mech->response->header('Content-Type')
+ or die "$uri does not have a Content-Type header";
+ if ($xml_parser_installed && $content_type =~ /xml/) {
+ eval { XML::Parser->new->parse($mech->content); };
+ ok(!$@, "[auto] check for XML well-formedness ($uri)") or diag($@);
+ }
+ if ($archive_tar_installed && $uri =~ /sf=tgz/) {
+ my $snapshot_file = File::Temp->new;
+ print $snapshot_file $mech->content;
+ close $snapshot_file;
+ my $t = Archive::Tar->new;
+ $t->read($snapshot_file->filename, 1);
+ ok($t->get_files, "[auto] valid tgz snapshot ($uri)");
+ }
+ # WebService::Validator::Feed::W3C would be nice to
+ # use, but it doesn't support direct input (as opposed
+ # to URIs) long enough for our feeds.
+ }
+ $mech->content_like(qr/(name|id)="$fragment"/,
+ "[auto] fragment #$fragment exists ($uri)")
+ if $fragment;
+ return 1;
+}
+
+# Verify and spider the current page, the latter only if --long-tests
+# (-l) is given. Do not check the status code of the current page.
+my %spidered_uris; # pages whose links have been checked
+my %status_checked_uris; # verified pages whose status is known to be 2xx
+sub check_page {
+ _verify_page or return 0;
+ my $orig_url = $mech->uri;
+ if ($long_tests && !$spidered_uris{$mech->uri} ) {
+ $spidered_uris{$mech->uri} = 1;
+ for my $url (map { $_->url_abs } $mech->followable_links) {
+ if (!$status_checked_uris{$url}) {
+ $status_checked_uris{$url} = 1;
+ $mech->get_ok($url, "[auto] check link ($url)")
+ or diag("broken link to $url on $orig_url");
+ _verify_page;
+ $mech->back;
+ }
+ }
+ }
+ return 1
+}
+
+my $baseurl = "http://localhost";
+my ($params, $url, $pagedesc, $status);
+
+# test_page ( <params>, <page_description>, <expected_status> )
+# Example:
+# if (test_page('?p=.git;a=summary', 'repository summary')) {
+# $mech->...;
+# $mech->...;
+# }
+#
+# Test that the page can be opened, call _verify_page on it, and
+# return true if there was no test failure. Also set the global
+# variables $params, $pagedesc, and $url for use in the if block.
+# Optionally pass a third parameter $status to test the HTTP status
+# code of the page (useful for error pages). You can also pass a full
+# URL instead of just parameters as the first parameter.
+sub test_page {
+ ($params, $pagedesc, $status) = @_;
+ $pagedesc = $pagedesc ? " -- $pagedesc" : '';
+ if($params =~ /^$baseurl/) {
+ $url = "$params";
+ } else {
+ $url = "$baseurl$params";
+ }
+ if ($status) {
+ $mech->get($url);
+ } else {
+ $mech->get_ok($url, "get $url$pagedesc") or return 0;
+ }
+ check_page or return 0;
+ if ($status) {
+ return is($mech->status, $status, "getting $url$pagedesc -- yields $status");
+ } else {
+ return 1;
+ }
+}
+
+# follow_link ( \%parms, $pagedesc )
+# Example: follow_link( { text => 'commit' }, 'first commit link')
+# Like test_page, but does not support status code testing.
+sub follow_link {
+ (my $parms, $pagedesc) = @_;
+ $mech->follow_link_ok($parms, "follow link: $pagedesc") or return 0;
+ $url = $mech->uri;
+ return check_page;
+}
+
+if (test_page '', 'project list (implicit)') {
+ $mech->title_like(qr!$site_name!,
+ "title contains $site_name");
+ $mech->content_contains('./t9503-gitweb-Mechanize.sh test repository',
+ 'lists test repository (by description)');
+}
+
+# Test repository summary: implicit, implicit with pathinfo, explicit.
+for my $sumparams ('?p=.git', '/.git', '?p=.git;a=summary') {
+ if (test_page $sumparams, 'repository summary') {
+ $mech->title_like(qr!$site_name.*\.git/summary!,
+ "title contains $site_name and \".git/summary\"");
+ }
+}
+
+# Search form (on summary page).
+$mech->get_ok('?p=.git', 'get repository summary');
+if ($mech->submit_form_ok( { form_number => 1,
+ fields => { 's' => 'Initial' }
+ }, "submit search form (default)")) {
+ check_page;
+ $mech->content_contains('Initial commit',
+ 'content contains searched commit');
+}
+
+test_page('?p=non-existent.git', 'non-existent project', 404);
+test_page('?p=.git;a=commit;h=non-existent', 'non-existent commit', 404);
+
+
+# Summary view
+
+# Check short log. To do: Extract into separate test_short_log
+# function since the short log occurs on several pages.
+$mech->get_ok('?p=.git', 'get repository summary');
+for my $revision (@revisions[0..2]) {
+ for my $link_text qw( commit commitdiff tree snapshot ) {
+ ok($mech->find_link(url_abs_regex => qr/h=$revision/, text => $link_text), "$link_text link for $revision");
+ }
+}
+# Check that branches and tags are highlighted in green and yellow in
+# the shortlog. We assume here that we are on master, so it should be
+# at the top.
+$mech->content_like(qr{<span [^>]*class="head"[^>]*>master</span>},
+ 'master branch is highlighted in shortlog');
+$mech->content_like(qr{<span [^>]*class="tag"[^>]*>$tags[0]</span>},
+ "$tags[0] (most recent tag) is highlighted in shortlog");
+
+# Check heads. (This should be extracted as well.)
+for my $head (@heads) {
+ for my $link_text qw( shortlog log tree ) {
+ ok($mech->find_link(url_abs_regex => qr{h=refs/heads/$head}, text => $link_text), "$link_text link for head '$head'");
+ }
+}
+
+# Check tags (assume we only have tags referring to commits).
+for my $tag (@tags) {
+ my $commit = rev_parse("$tag^{commit}");
+ ok($mech->find_link(url_abs_regex => qr{h=refs/tags/$tag}, text => 'shortlog'),
+ "shortlog link for tag '$tag'");
+ ok($mech->find_link(url_abs_regex => qr{h=refs/tags/$tag}, text => 'log'),
+ "log link for tag '$tag'");
+ ok($mech->find_link(url_abs_regex => qr{h=$commit}, text => 'commit'),
+ "commit link for tag '$tag'");
+ # To do: Test tag link for tag objects.
+ # Why don't we have tree + snapshot links?
+}
+
+
+# RSS/Atom/OPML view. Simply retrieve and check.
+{
+ # Broken link in Atom/RSS view -- cannot spider:
+ # http://mid.gmane.org/485EB333.5070108@gmail.com
+ local $long_tests = 0;
+ test_page('?p=.git;a=atom', 'Atom feed');
+ test_page('?p=.git;a=rss', 'RSS feed');
+}
+test_page('?a=opml', 'OPML outline');
+
+
+# Commit view
+if (test_page('?p=.git;a=commit;h=master')) {
+ ok($mech->find_link(url_abs_regex => qr/a=tree/),
+ "tree link on commit page ($url)");
+ $mech->content_like(qr/A U Thor/, "author mentioned on commit page ($url)");
+}
+
+
+# Commitdiff view
+if ($mech->get_ok('?p=.git', 'get repository summary') &&
+ follow_link( { text_regex => qr/file added/i }, 'commit with added file') &&
+ follow_link( { text => 'commitdiff' }, 'commitdiff')) {
+ $mech->content_like(qr/new file with mode/, "commitdiff has diffstat ($url)");
+ $mech->content_like(qr/new file mode/, "commitdiff has diff ($url)");
+}
+
+
+# Tree view
+if ($mech->get_ok('?p=.git', 'get repository summary') &&
+ follow_link( { text => 'tree' }, 'follow first tree link on page')) {
+ for my $file (@files) {
+ my $file_hash = rev_parse("HEAD:$file");
+ ok($mech->find_link(text => $file), "'$file' listed (and linked) in tree view ($url)");
+ if (get_type("HEAD:$file") eq 'blob') {
+ for my $link_text qw( blob blame history raw ) {
+ my $link = $mech->find_link(url_abs_regex => qr/[^a-z]f=$file(;|$)/,
+ text => $link_text);
+ ok($link, "'$file' file has $link_text link in tree view ($url)");
+ }
+ } else {
+ # Subtree -- to do: write tests. (Need to set
+ # up a subtree in t9503-gitweb-Mechanize.sh.)
+ }
+ }
+}
+
+
+# Blame view
+{
+ # Broken link in blame view -- cannot spider:
+ # http://mid.gmane.org/485EC621.7090101@gmail.com
+ local $long_tests = 0;
+ if ($mech->get_ok('?p=.git', 'get repository summary') &&
+ follow_link( { text => 'tree' }, 'follow first tree link on page')) {
+ for my $blame_link ($mech->find_all_links(text => 'blame')) {
+ test_page($blame_link->url, "follow blame link from tree view");
+ $mech->content_like(qr/A U Thor/,
+ "author mentioned on blame page");
+ }
+ }
+}
+
+
+# History view
+if ($mech->get_ok('?p=.git', 'get repository summary') &&
+ follow_link( { text => 'tree' }, 'follow first tree link on page')) {
+ for my $history_link ($mech->find_all_links(text => 'history')) {
+ test_page($history_link->url, "follow history link from tree view");
+ # To do: Expand.
+ }
+}
+
+
+# Blob view
+if ($mech->get_ok('?p=.git', 'get repository summary') &&
+ follow_link( { text => 'tree' }, 'follow first tree link on page')) {
+ for my $blob_link ($mech->find_all_links(text => 'blob')) {
+ test_page($blob_link->url, "follow blob link from tree view");
+ # To do: Expand beyond standard tests.
+ }
+}
+
+
+# Raw view
+if ($mech->get_ok('?p=.git', 'get repository summary') &&
+ follow_link( { text => 'tree' }, 'follow first tree link on page')) {
+ for my $raw_link ($mech->find_all_links(text => 'raw')) {
+ test_page($raw_link->url, "follow raw link from tree view");
+ }
+}
+
+
+1;
+__END__
diff --git a/t/test-lib.sh b/t/test-lib.sh
index a9fc621..504c0bb 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -472,6 +472,8 @@ export PATH GIT_EXEC_PATH GIT_TEMPLATE_DIR GIT_CONFIG_NOSYSTEM GIT_CONFIG_NOGLOB
GITPERLLIB=$(pwd)/../perl/blib/lib:$(pwd)/../perl/blib/arch/auto/Git
export GITPERLLIB
+GITPERL=${GITPERL:-perl}
+export GITPERL
test -d ../templates/blt || {
error "You haven't built things yet, have you?"
}
--
1.5.6.86.g5139f.dirty
^ permalink raw reply related
* Re: Shrink the git binary a bit by avoiding unnecessary inline functions
From: Christian MICHON @ 2008-06-23 1:01 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <46d6db660806221432m6a6e2f3egf78faa1510f8d643@mail.gmail.com>
On Sun, Jun 22, 2008 at 9:32 PM, Christian MICHON
<christian.michon@gmail.com> wrote:
> using gcc-3.4.6 and uclibc-0.9.29 (not exactly everyone's
> configuration of course...)
> I get different numbers with CFLAGS=-Os and NO_CURL, NO_ICONV on plain
> git-1.5.6:
>
> sh-3.2# ls -lh git
> -rwxr-xr-x 3 root root 699.7k Jun 22 23:26 git
>
> sh-3.2# size git
> text data bss dec hex filename
> 616544 10960 272272 899776 dbac0 git
>
> after I use your patch, it goes to:
>
> sh-3.2# ls -lh git
> -rwxr-xr-x 1 root root 652.6k Jun 22 23:30 git
>
> sh-3.2# size git
> text data bss dec hex filename
> 568124 10960 272272 851356 cfd9c git
>
> So your patch obviously works here too but I get quite smaller figures too.
>
> curl and iconv are not available on my distro detaolb, maybe it's a
> big difference too...
>
> Could your figures come from recent gcc/glibc versions ?
>
Linus,
I've put my hands on a usb stick with ubuntu (gcc-4.2.3 and
glibc-2.7), containing all packages needed for a pristine compilation
of git-1.5.6.
I only changed CFLAGS to -Os, no debug info compiled.
ubuntu@ubuntu:~/Desktop/git-1.5.6$ ls -lh git
-rwxr-xr-x 1 ubuntu ubuntu 718K 2008-06-23 00:47 git
ubuntu@ubuntu:~/Desktop/git-1.5.6$ size git
text data bss dec hex filename
616606 9876 270812 897294 db10e git
There's more than 80k difference with your figures (I have not even
applied your patch yet).
May I ask which version of gcc you're using ? The numbers you provided
were really with -Os ?
Once your patch is applied, these are the final figures:
ubuntu@ubuntu:~/Desktop/git-1.5.6$ ls -lh git
-rwxr-xr-x 1 ubuntu ubuntu 692K 2008-06-23 00:59 git
ubuntu@ubuntu:~/Desktop/git-1.5.6$ size git
text data bss dec hex filename
591554 9876 270812 872242 d4f32 git
--
Christian
--
http://detaolb.sourceforge.net/, a linux distribution for Qemu with Git inside !
^ permalink raw reply
* Re: core.autocrlf and merge conflict output
From: Junio C Hamano @ 2008-06-23 0:45 UTC (permalink / raw)
To: Edward Z. Yang; +Cc: git
In-Reply-To: <g3miho$se5$1@ger.gmane.org>
"Edward Z. Yang" <edwardzyang@thewritingpot.com> writes:
> Apparently, the conflict information Git writes to the working copy
> during merge doesn't respect core.autocrlf,...
Is this an old news before 249c61a (merge-recursive: respect core.autocrlf
when writing out the result, 2008-06-09) happened?
^ permalink raw reply
* [PATCH v5] gitweb: add test suite with Test::WWW::Mechanize::CGI
From: Lea Wiemann @ 2008-06-23 0:45 UTC (permalink / raw)
To: git; +Cc: Lea Wiemann, Jakub Narebski
In-Reply-To: <1214000325-26389-1-git-send-email-LeWiemann@gmail.com>
This test uses Test::WWW::Mechanize::CGI to check gitweb's output. It
also uses HTML::Lint, XML::Parser, and Archive::Tar (if present, each)
to validate the HTML/XML/tgz output, and checks all links on the
tested pages if --long-tests is given.
Also add a GITPERL environment variable that allows running Perl-based
tests with different perl binaries (and thus under different
versions).
Signed-off-by: Lea Wiemann <LeWiemann@gmail.com>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Windows users: Please apply this patch and run the test, I'd love to
know if it works on Windows!
This patch applies against the *next* branch (without any other
patches, finally).
Note that in order to actually run the test you need the
Test::WWW::Mechanize::CGI Perl module.
Changes since v4:
- Add a $GITPERL variable so you can choose the perl binary and run
the test suite with different Perl versions. Note that this affects
test-lib.sh.
- Add a workaroud for WWW::Mechanize::CGI so that the test suite also
works if the path to the Git tree contains blanks (which could
happen on Windows, for instance).
- Integrated all of Jakub's "[RFC/PATCH (WIP)] gitweb: Check that RSS,
Atom and OPML output is well formed XML", which uses XML::Parser.
- Also added snapshot validation using Archive::Tar.
- Spidering (with --long-tests) is now much more efficient, in that it
doesn't check any page more than once.
- Added checks to ensure that fragments have corresponding name/id
attributes in the page.
- Added plenty more actual tests: check more elements on the summary
page, and also test the commit, commitdiff, tree, blame, history,
blob, and blob_plain views. Some of these tests don't do anything
more than downloading a few pages with the given action (without
checking any of the content), but this already helps a lot since it
checks that gitweb doesn't die, that the HTML is valid, and (if
--long-tests is given) that all links work.
I've tested the test suite with Perl 5.8, Perl 5.10, and with and
without the optional modules (HTML::Lint, XML::Parser, Archive::Tar).
I'd like some feedback on this (especially from Jakub) -- if other
people OK the patch, I think it's ready for inclusion.
Apologies for any tyops and last-minute glitches, I'm getting kinda
tired... :)
-- Lea
t/t9503-gitweb-Mechanize.sh | 132 ++++++++++++++++
t/t9503/test.pl | 354 +++++++++++++++++++++++++++++++++++++++++++
t/test-lib.sh | 2 +
3 files changed, 488 insertions(+), 0 deletions(-)
create mode 100755 t/t9503-gitweb-Mechanize.sh
create mode 100755 t/t9503/test.pl
diff --git a/t/t9503-gitweb-Mechanize.sh b/t/t9503-gitweb-Mechanize.sh
new file mode 100755
index 0000000..3fe6d8b
--- /dev/null
+++ b/t/t9503-gitweb-Mechanize.sh
@@ -0,0 +1,132 @@
+#!/bin/sh
+#
+# Copyright (c) 2008 Jakub Narebski
+# Copyright (c) 2008 Lea Wiemann
+#
+
+# This test supports the --long-tests option.
+
+# This test only runs on Perl 5.8 and later versions, since
+# Test::WWW::Mechanize::CGI requires Perl 5.8.
+
+test_description='gitweb tests (using WWW::Mechanize)
+
+This test uses Test::WWW::Mechanize::CGI to test gitweb.'
+
+# helper functions
+
+safe_chmod () {
+ chmod "$1" "$2" &&
+ if [ "$(git config --get core.filemode)" = false ]
+ then
+ git update-index --chmod="$1" "$2"
+ fi
+}
+
+. ./test-lib.sh
+
+# check if test can be run
+"$GITPERL" -MEncode -e 'decode_utf8("", Encode::FB_CROAK)' >/dev/null 2>&1 || {
+ test_expect_success \
+ 'skipping gitweb tests, perl version is too old' :
+ test_done
+ exit
+}
+
+"$GITPERL" -MTest::WWW::Mechanize::CGI -e '' >/dev/null 2>&1 || {
+ test_expect_success \
+ 'skipping gitweb tests, Test::WWW::Mechanize::CGI not found' :
+ test_done
+ exit
+}
+
+# set up test repository
+test_expect_success 'set up test repository' '
+
+ echo "Not an empty file." > file &&
+ git add file &&
+ test_tick && git commit -a -m "Initial commit." &&
+ git branch b &&
+
+ echo "New file" > new_file &&
+ git add new_file &&
+ test_tick && git commit -a -m "File added." &&
+
+ safe_chmod +x new_file &&
+ test_tick && git commit -a -m "Mode changed." &&
+
+ git mv new_file renamed_file &&
+ test_tick && git commit -a -m "File renamed." &&
+
+ rm renamed_file &&
+ ln -s file renamed_file &&
+ test_tick && git commit -a -m "File to symlink." &&
+ git tag with-symlink &&
+
+ git rm renamed_file &&
+ rm -f renamed_file &&
+ test_tick && git commit -a -m "File removed." &&
+
+ cp file file2 &&
+ git add file2 &&
+ test_tick && git commit -a -m "File copied." &&
+
+ echo "New line" >> file2 &&
+ safe_chmod +x file2 &&
+ test_tick && git commit -a -m "Mode change and modification." &&
+
+ git checkout b &&
+ echo "Branch" >> b &&
+ git add b &&
+ test_tick && git commit -a -m "On branch" &&
+ git checkout master &&
+ test_tick && git pull . b
+'
+
+# set up empty repository
+# TODO!
+
+# set up repositories for gitweb
+# TODO!
+
+# set up gitweb configuration
+safe_pwd="$("$GITPERL" -MPOSIX=getcwd -e 'print quotemeta(getcwd)')"
+cat >gitweb_config.perl <<EOF
+# gitweb configuration for tests
+
+our \$version = "current";
+our \$GIT = "$GIT_EXEC_PATH/git";
+our \$projectroot = "$safe_pwd";
+our \$project_maxdepth = 8;
+our \$home_link_str = "projects";
+our \$site_name = "[localhost]";
+our \$site_header = "";
+our \$site_footer = "";
+our \$home_text = "indextext.html";
+our @stylesheets = ("file:///$safe_pwd/../../gitweb/gitweb.css");
+our \$logo = "file:///$safe_pwd/../../gitweb/git-logo.png";
+our \$favicon = "file:///$safe_pwd/../../gitweb/git-favicon.png";
+our \$projects_list = "";
+our \$export_ok = "";
+our \$strict_export = "";
+our %feature;
+\$feature{'blame'}{'default'} = [1];
+
+1;
+__END__
+EOF
+
+cat >.git/description <<EOF
+$0 test repository
+EOF
+
+GITWEB_CONFIG="$(pwd)/gitweb_config.perl"
+export GITWEB_CONFIG
+
+# run tests
+
+test_external \
+ 'test gitweb output' \
+ "$GITPERL" ../t9503/test.pl
+
+test_done
diff --git a/t/t9503/test.pl b/t/t9503/test.pl
new file mode 100755
index 0000000..28894c5
--- /dev/null
+++ b/t/t9503/test.pl
@@ -0,0 +1,354 @@
+#!/usr/bin/perl
+use lib (split(/:/, $ENV{GITPERLLIB}));
+
+# This test supports the --long-tests option.
+
+use warnings;
+use strict;
+
+use Cwd qw(abs_path);
+use File::Spec;
+use File::Temp;
+
+# We don't count properly when skipping, so no_plan is necessary.
+use Test::More qw(no_plan);
+use Test::WWW::Mechanize::CGI;
+
+our $long_tests = $ENV{GIT_TEST_LONG};
+
+eval { require Archive::Tar; };
+my $archive_tar_installed = !$@
+ or diag('Archive::Tar is not installed; no tests for valid snapshots');
+
+eval { require HTML::Lint; };
+my $html_lint_installed = !$@
+ or diag('HTML::Lint is not installed; no HTML validation tests');
+
+eval { require XML::Parser; };
+my $xml_parser_installed = !$@
+ or diag('XML::Parser is not installed; no tests for well-formed XML');
+
+my @revisions = split /\s/, `git-rev-list --first-parent HEAD`;
+chomp(my @heads = map { (split('/', $_))[2] } `git-for-each-ref --sort=-committerdate refs/heads`);
+chomp(my @tags = map { (split('/', $_))[2] } `git-for-each-ref --sort=-committerdate refs/tags`);
+# files and directories in HEAD root:
+chomp(my @files = map { (split("\t", $_))[1] } `git-ls-tree HEAD`);
+
+sub rev_parse {
+ my $name = shift;
+ chomp(my $hash = `git rev-parse $name 2> /dev/null`);
+ $hash or undef;
+}
+
+sub get_type {
+ my $name = shift;
+ chomp(my $type = `git cat-file -t $name 2> /dev/null`);
+ $type or undef;
+}
+
+my $gitweb = abs_path(File::Spec->catfile('..', '..', 'gitweb', 'gitweb.cgi'));
+
+# Thus subroutine was copied (and modified to work with blanks in the
+# application path) from WWW::Mechanize::CGI 0.3, which is licensed
+# 'under the same terms as perl itself' and thus GPL compatible.
+my $cgi = sub {
+ # Use exec, not the shell, to support blanks in the path.
+ my $status = system $gitweb $gitweb;
+ my $value = $status >> 8;
+
+ croak( qq/Failed to execute application '$gitweb'. Reason: '$!'/ )
+ if ( $status == -1 );
+ croak( qq/Application '$gitweb' exited with value: $value/ )
+ if ( $value > 0 );
+};
+
+my $mech = new Test::WWW::Mechanize::CGI;
+$mech->cgi($cgi);
+# On some systems(?) it's necessary to have %ENV here, otherwise the
+# CGI process won't get *any* of the current environment variables
+# (not even PATH, etc.)
+$mech->env(%ENV,
+ GITWEB_CONFIG => $ENV{'GITWEB_CONFIG'},
+ SCRIPT_FILENAME => $gitweb,
+ $mech->env);
+
+# import config, predeclaring config variables
+our $site_name;
+require_ok($ENV{'GITWEB_CONFIG'})
+ or diag('Could not load gitweb config; some tests would fail');
+
+# Perform non-recursive checks on the current page, but do not check
+# the status code.
+my %verified_uris;
+sub _verify_page {
+ my ($uri, $fragment) = split '#', $mech->uri;
+ if (!$verified_uris{$uri}) {
+ $verified_uris{$uri} = 1;
+
+ # Internal errors yield 200, but cause gitweb.cgi to
+ # exit with non-zero exit code, which Mechanize::CGI
+ # translates to 500, so we don't really need to check
+ # for "Software error" here, provided that the test
+ # cases always check the status code.
+ #$mech->content_lacks('<h1>Software error:</h1>') or return 0;
+
+ # Validating is fast, so we can do it even without
+ # $long_tests.
+ $mech->html_lint_ok('[auto] validate HTML') or return 0
+ if $html_lint_installed && $mech->is_html;
+ my $content_type = $mech->response->header('Content-Type')
+ or die "$uri does not have a Content-Type header";
+ if ($xml_parser_installed && $content_type =~ /xml/) {
+ eval { XML::Parser->new->parse($mech->content); };
+ ok(!$@, "[auto] check for XML well-formedness ($uri)") or diag($@);
+ }
+ if ($archive_tar_installed && $uri =~ /sf=tgz/) {
+ my $snapshot_file = File::Temp->new;
+ print $snapshot_file $mech->content;
+ close $snapshot_file;
+ my $t = Archive::Tar->new;
+ $t->read($snapshot_file->filename, 1);
+ ok($t->get_files, "[auto] valid tgz snapshot ($uri)");
+ }
+ # WebService::Validator::Feed::W3C would be nice to
+ # use, but it doesn't support direct input (as opposed
+ # to URIs) long enough for our feeds.
+ }
+ $mech->content_like(qr/(name|id)="$fragment"/,
+ "[auto] fragment #$fragment exists ($uri)")
+ if $fragment;
+ return 1;
+}
+
+# Verify and spider the current page, the latter only if --long-tests
+# (-l) is given. Do not check the status code of the current page.
+my %spidered_uris; # pages whose links have been checked
+my %status_checked_uris; # verified pages whose status is known to be 2xx
+sub check_page {
+ _verify_page or return 0;
+ my $orig_url = $mech->uri;
+ if ($long_tests && !$spidered_uris{$mech->uri} ) {
+ $spidered_uris{$mech->uri} = 1;
+ for my $url (map { $_->url_abs } $mech->followable_links) {
+ if (!$status_checked_uris{$url}) {
+ $status_checked_uris{$url} = 1;
+ $mech->get_ok($url, "[auto] check link ($url)")
+ or diag("broken link to $url on $orig_url");
+ _verify_page;
+ $mech->back;
+ }
+ }
+ }
+ return 1
+}
+
+my $baseurl = "http://localhost";
+my ($params, $url, $pagedesc, $status);
+
+# test_page ( <params>, <page_description>, <expected_status> )
+# Example:
+# if (test_page('?p=.git;a=summary', 'repository summary')) {
+# $mech->...;
+# $mech->...;
+# }
+#
+# Test that the page can be opened, call _verify_page on it, and
+# return true if there was no test failure. Also set the global
+# variables $params, $pagedesc, and $url for use in the if block.
+# Optionally pass a third parameter $status to test the HTTP status
+# code of the page (useful for error pages). You can also pass a full
+# URL instead of just parameters as the first parameter.
+sub test_page {
+ ($params, $pagedesc, $status) = @_;
+ $pagedesc = $pagedesc ? " -- $pagedesc" : '';
+ if($params =~ /^$baseurl/) {
+ $url = "$params";
+ } else {
+ $url = "$baseurl$params";
+ }
+ if ($status) {
+ $mech->get($url);
+ } else {
+ $mech->get_ok($url, "get $url$pagedesc") or return 0;
+ }
+ check_page or return 0;
+ if ($status) {
+ return is($mech->status, $status, "getting $url$pagedesc -- yields $status");
+ } else {
+ return 1;
+ }
+}
+
+# follow_link ( \%parms, $pagedesc )
+# Example: follow_link( { text => 'commit' }, 'first commit link')
+# Like test_page, but does not support status code testing.
+sub follow_link {
+ (my $parms, $pagedesc) = @_;
+ $mech->follow_link_ok($parms, "follow link: $pagedesc") or return 0;
+ $url = $mech->uri;
+ return check_page;
+}
+
+if (test_page '', 'project list (implicit)') {
+ $mech->title_like(qr!$site_name!,
+ "title contains $site_name");
+ $mech->content_contains('./t9503-gitweb-Mechanize.sh test repository',
+ 'lists test repository (by description)');
+}
+
+# Test repository summary: implicit, implicit with pathinfo, explicit.
+for my $sumparams ('?p=.git', '/.git', '?p=.git;a=summary') {
+ if (test_page $sumparams, 'repository summary') {
+ $mech->title_like(qr!$site_name.*\.git/summary!,
+ "title contains $site_name and \".git/summary\"");
+ }
+}
+
+# Search form (on summary page).
+$mech->get_ok('?p=.git', 'get repository summary');
+if ($mech->submit_form_ok( { form_number => 1,
+ fields => { 's' => 'Initial' }
+ }, "submit search form (default)")) {
+ check_page;
+ $mech->content_contains('Initial commit',
+ 'content contains searched commit');
+}
+
+test_page('?p=non-existent.git', 'non-existent project', 404);
+test_page('?p=.git;a=commit;h=non-existent', 'non-existent commit', 404);
+
+
+# Summary view
+
+# Check short log. To do: Extract into separate test_short_log
+# function since the short log occurs on several pages.
+$mech->get_ok('?p=.git', 'get repository summary');
+for my $revision (@revisions[0..2]) {
+ for my $link_text qw( commit commitdiff tree snapshot ) {
+ ok($mech->find_link(url_abs_regex => qr/h=$revision/, text => $link_text), "$link_text link for $revision");
+ }
+}
+# Check that branches and tags are highlighted in green and yellow in
+# the shortlog. We assume here that we are on master, so it should be
+# at the top.
+$mech->content_like(qr{<span [^>]*class="head"[^>]*>master</span>},
+ 'master branch is highlighted in shortlog');
+$mech->content_like(qr{<span [^>]*class="tag"[^>]*>$tags[0]</span>},
+ "$tags[0] (most recent tag) is highlighted in shortlog");
+
+# Check heads. (This should be extracted as well.)
+for my $head (@heads) {
+ for my $link_text qw( shortlog log tree ) {
+ ok($mech->find_link(url_abs_regex => qr{h=refs/heads/$head}, text => $link_text), "$link_text link for head '$head'");
+ }
+}
+
+# Check tags (assume we only have tags referring to commits).
+for my $tag (@tags) {
+ my $commit = rev_parse("$tag^{commit}");
+ ok($mech->find_link(url_abs_regex => qr{h=refs/tags/$tag}, text => 'shortlog'),
+ "shortlog link for tag '$tag'");
+ ok($mech->find_link(url_abs_regex => qr{h=refs/tags/$tag}, text => 'log'),
+ "log link for tag '$tag'");
+ ok($mech->find_link(url_abs_regex => qr{h=$commit}, text => 'commit'),
+ "commit link for tag '$tag'");
+ # To do: Test tag link for tag objects.
+ # Why don't we have tree + snapshot links?
+}
+
+
+# RSS/Atom/OPML view. Simply retrieve and check.
+{
+ # Broken link in Atom/RSS view -- cannot spider:
+ # http://mid.gmane.org/485EB333.5070108@gmail.com
+ local $long_tests = 0;
+ test_page('?p=.git;a=atom', 'Atom feed');
+ test_page('?p=.git;a=rss', 'RSS feed');
+}
+test_page('?a=opml', 'OPML outline');
+
+
+# Commit view
+if (test_page('?p=.git;a=commit;h=master')) {
+ ok($mech->find_link(url_abs_regex => qr/a=tree/),
+ "tree link on commit page ($url)");
+ $mech->content_like(qr/A U Thor/, "author mentioned on commit page ($url)");
+}
+
+
+# Commitdiff view
+if ($mech->get_ok('?p=.git', 'get repository summary') &&
+ follow_link( { text_regex => qr/file added/i }, 'commit with added file') &&
+ follow_link( { text => 'commitdiff' }, 'commitdiff')) {
+ $mech->content_like(qr/new file with mode/, "commitdiff has diffstat ($url)");
+ $mech->content_like(qr/new file mode/, "commitdiff has diff ($url)");
+}
+
+
+# Tree view
+if ($mech->get_ok('?p=.git', 'get repository summary') &&
+ follow_link( { text => 'tree' }, 'follow first tree link on page')) {
+ for my $file (@files) {
+ my $file_hash = rev_parse("HEAD:$file");
+ ok($mech->find_link(text => $file), "'$file' listed (and linked) in tree view ($url)");
+ if (get_type("HEAD:$file") eq 'blob') {
+ for my $link_text qw( blob blame history raw ) {
+ my $link = $mech->find_link(url_abs_regex => qr/[^a-z]f=$file(;|$)/,
+ text => $link_text);
+ ok($link, "'$file' file has $link_text link in tree view ($url)");
+ }
+ } else {
+ # Subtree -- to do: write tests. (Need to set
+ # up a subtree in t9503-gitweb-Mechanize.sh.)
+ }
+ }
+}
+
+
+# Blame view
+{
+ # Broken link in blame view -- cannot spider:
+ # http://mid.gmane.org/485EC621.7090101@gmail.com
+ local $long_tests = 0;
+ if ($mech->get_ok('?p=.git', 'get repository summary') &&
+ follow_link( { text => 'tree' }, 'follow first tree link on page')) {
+ for my $blame_link ($mech->find_all_links(text => 'blame')) {
+ test_page($blame_link->url, "follow blame link from tree view");
+ $mech->content_like(qr/A U Thor/,
+ "author mentioned on blame page");
+ }
+ }
+}
+
+
+# History view
+if ($mech->get_ok('?p=.git', 'get repository summary') &&
+ follow_link( { text => 'tree' }, 'follow first tree link on page')) {
+ for my $history_link ($mech->find_all_links(text => 'history')) {
+ test_page($history_link->url, "follow history link from tree view");
+ # To do: Expand.
+ }
+}
+
+
+# Blob view
+if ($mech->get_ok('?p=.git', 'get repository summary') &&
+ follow_link( { text => 'tree' }, 'follow first tree link on page')) {
+ for my $blob_link ($mech->find_all_links(text => 'blob')) {
+ test_page($blob_link->url, "follow blob link from tree view");
+ # To do: Expand beyond standard tests.
+ }
+}
+
+
+# Raw view
+if ($mech->get_ok('?p=.git', 'get repository summary') &&
+ follow_link( { text => 'tree' }, 'follow first tree link on page')) {
+ for my $raw_link ($mech->find_all_links(text => 'raw')) {
+ test_page($raw_link->url, "follow raw link from tree view");
+ }
+}
+
+
+1;
+__END__
diff --git a/t/test-lib.sh b/t/test-lib.sh
index a9fc621..504c0bb 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -472,6 +472,8 @@ export PATH GIT_EXEC_PATH GIT_TEMPLATE_DIR GIT_CONFIG_NOSYSTEM GIT_CONFIG_NOGLOB
GITPERLLIB=$(pwd)/../perl/blib/lib:$(pwd)/../perl/blib/arch/auto/Git
export GITPERLLIB
+GITPERL=${GITPERL:-perl}
+export GITPERL
test -d ../templates/blt || {
error "You haven't built things yet, have you?"
}
--
1.5.6.86.g5139f.dirty
^ permalink raw reply related
* Re: [PATCH 1/2] Create a fnmatch-style pattern TreeFilter
From: Shawn O. Pearce @ 2008-06-23 0:27 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: git, Marek Zawirski, Florian Koeberle
In-Reply-To: <1214177145-18963-1-git-send-email-robin.rosenberg@dewire.com>
Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
> +
> +package org.spearce.jgit.revwalk.filter;
This should be treewalk.filter, its a filter for tree entries.
> + * This class implements a TreeeFilter that uses the wildcard style pattern
fyi, minooooor typo on TreeFilter.
> +public class WildCardTreeFilter extends TreeFilter {
...
> + @Override
> + public boolean include(TreeWalk walker) throws MissingObjectException,
> + IncorrectObjectTypeException, IOException {
> + matcher.reset();
> + matcher.append(walker.getPathString());
> + if (matcher.isMatch())
> + return true;
> + return false;
> + }
Hmm.
It isn't as efficient as it could be. Obtaining the string of
the path is somewhat costly as we have to convert from the byte[]
to a char[] and then wrap that into a String, just to check the
pattern again. Its more accurate to convert to the string, but
it is a lot slower.
But I'm wondering about what happens when the TreeWalk is considering
a subtree path name. Does the matcher still match the pattern? By
that I mean lets say the pattern was:
src/xdiff/*.c
In this case the include method is first called with the path "src";
if it returns false the TreeWalk won't recurse into the subtree and
thus you'll never get to consider "src/xdiff" or "src/xdiff/foo.c".
--
Shawn.
^ permalink raw reply
* [PATCH/RFC] git.el: Commands for committing patches
From: Nikolaj Schumacher @ 2008-06-22 23:35 UTC (permalink / raw)
To: git; +Cc: Alexandre Julliard
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: patch --]
[-- Type: text/x-patch, Size: 11542 bytes --]
>From 8fb3c0b6c431bbe7c145882811b1419add34527b Mon Sep 17 00:00:00 2001
From: Nikolaj Schumacher <git@nschum.de>
Date: Mon, 23 Jun 2008 00:24:36 +0200
Subject: [PATCH] git.el: Added command for committing patches.
This adds commands for committing patches from files, buffers and email
buffers.
In order to minimize code duplication, git-start-log-edit and
git-prepare-log-buffer have been extracted from git-commit-file.
Signed-off-by: Nikolaj Schumacher <git@nschum.de>
---
contrib/emacs/git.el | 192 +++++++++++++++++++++++++++++++++++++++++++-------
1 files changed, 167 insertions(+), 25 deletions(-)
diff --git a/contrib/emacs/git.el b/contrib/emacs/git.el
index 4fa853f..c6df5aa 100644
--- a/contrib/emacs/git.el
+++ b/contrib/emacs/git.el
@@ -189,6 +189,13 @@ if there is already one that displays the same directory."
process-environment)))
(apply #'call-process "git" nil buffer nil args)))
+(defun git-call-process-env-on-region (buffer env beg end &rest args)
+ "Wrapper for `call-process-region' that sets environment strings."
+ (let ((process-environment (append (git-get-env-strings env)
+ process-environment)))
+ (buffer-string)
+ (apply #'call-process-region beg end "git" nil buffer nil args)))
+
(defun git-call-process-display-error (&rest args)
"Wrapper for call-process that displays error messages."
(let* ((dir default-directory)
@@ -315,6 +322,34 @@ and returns the process output as a string, or nil if the git failed."
"\"")
name))
+(defun git-parse-email-address (email-address)
+ "Split the EMAIL-ADDRESS string into a cons of address and name."
+ (if (string-match "\\`\"?\\([^\n<,\"]+\\)\"?[ \t]<\\([^ @]+@[^ \n\t]+\\)>\\'"
+ email-address)
+ (cons (match-string-no-properties 2 email-address)
+ (match-string-no-properties 1 email-address))
+ (when (string-match
+ "\\`\\([^ \t\n@]+@[^ \t\n,]+\\)\\([ \t](\\([^)]*\\))\\)?"
+ email-address)
+ (cons (match-string-no-properties 1 email-address)
+ (match-string-no-properties 3 email-address)))))
+
+(defun git-find-patch (&optional start)
+ "Find the patch in the current buffer."
+ (save-excursion
+ (goto-char (or start (point-min)))
+ (when (re-search-forward "^\\(---$\\|diff -\\|Index: \\)" nil t)
+ (match-beginning 0))))
+
+(defun git-fetch-header (header limit)
+ (save-excursion
+ (goto-char limit)
+ (let ((case-fold-search t))
+ (when (re-search-backward (concat "^" (regexp-quote header)
+ "[ \t]*:[ \t]*")
+ nil t)
+ (buffer-substring-no-properties (match-end 0) (point-at-eol))))))
+
(defun git-success-message (text files)
"Print a success message after having handled FILES."
(let ((n (length files)))
@@ -891,6 +926,58 @@ Return the list of files that haven't been handled."
(message "No files to commit.")))
(delete-file index-file))))))
+(defun git-apply-patch-to-index (index-file patch &optional beg end)
+ "Run git-apply on a patch."
+ (with-temp-buffer
+ (let ((env (and index-file `(("GIT_INDEX_FILE" . ,index-file))))
+ (temp-buffer (current-buffer))
+ res)
+ (if (stringp patch)
+ (if (file-exists-p patch)
+ (setq res (git-call-process-env temp-buffer env "apply"
+ "--cached"
+ (expand-file-name patch)))
+ (error "Patch file disappeared"))
+ (if (buffer-live-p patch)
+ (setq res (with-current-buffer patch
+ (git-call-process-env-on-region
+ temp-buffer env (or beg (point-min))
+ (or end (point-max)) "apply" "--cached" "-")))
+ (error "Patch buffer disappeared")))
+ (unless (= 0 res)
+ (error "Applying patch failed:\n%s" (buffer-string))))))
+
+(defun git-do-commit-patch (patch &optional beg end)
+ "Actually commit the patch using the current buffer as log message."
+ (interactive)
+ (let ((buffer (current-buffer))
+ (index-file (make-temp-file "gitidx")))
+ (with-current-buffer log-edit-parent-buffer
+ (unwind-protect
+ (let (head parent head-tree)
+ (unless (git-empty-db-p)
+ (setq head (git-rev-parse "HEAD")
+ head-tree (git-rev-parse "HEAD^{tree}")))
+ (message "Running git commit...")
+ (git-read-tree head-tree index-file)
+ ;; Update both the default index and the temporary one.
+ (git-apply-patch-to-index index-file patch beg end)
+ (git-apply-patch-to-index nil patch beg end)
+ (let* ((tree (git-write-tree index-file))
+ (commit (git-commit-tree buffer tree head)))
+ (when commit
+ (condition-case nil (delete-file ".git/MERGE_HEAD") (error nil)
+ (condition-case nil (delete-file ".git/MERGE_MSG") (error nil))
+ (with-current-buffer buffer (erase-buffer))
+ (git-call-process-env nil nil "rerere")
+ (git-call-process-env nil nil "gc" "--auto")
+ (git-refresh-status)
+ (git-refresh-ewoc-hf git-status)
+ (message "Committed %s." commit)
+ (git-run-hook "post-commit" nil)))
+ t)
+ (delete-file index-file)
+ nil)))))
;;;; Interactive functions
;;;; ------------------------------------------------------------
@@ -1263,36 +1350,41 @@ Return the list of files that haven't been handled."
(when sign-off (git-append-sign-off committer-name committer-email)))
buffer))
+(defun git-start-log-edit (buffer action)
+ (if (boundp 'log-edit-diff-function)
+ (log-edit action nil '((log-edit-listfun . git-log-edit-files)
+ (log-edit-diff-function . git-log-edit-diff)) buffer)
+ (log-edit action nil 'git-log-edit-files buffer))
+ (setq font-lock-keywords (font-lock-compile-keywords git-log-edit-font-lock-keywords))
+ (setq buffer-file-coding-system (git-get-commits-coding-system))
+ (re-search-forward (regexp-quote (concat git-log-msg-separator "\n")) nil t))
+
+(defun git-prepare-log-buffer ()
+ (let ((buffer (get-buffer-create "*git-commit*"))
+ author-name author-email subject date)
+ (when (eq 0 (buffer-size buffer))
+ (when (file-readable-p ".dotest/info")
+ (with-temp-buffer
+ (insert-file-contents ".dotest/info")
+ (goto-char (point-min))
+ (when (re-search-forward "^Author: \\(.*\\)\nEmail: \\(.*\\)$" nil t)
+ (setq author-name (match-string 1))
+ (setq author-email (match-string 2)))
+ (goto-char (point-min))
+ (when (re-search-forward "^Subject: \\(.*\\)$" nil t)
+ (setq subject (match-string 1)))
+ (goto-char (point-min))
+ (when (re-search-forward "^Date: \\(.*\\)$" nil t)
+ (setq date (match-string 1)))))
+ (git-setup-log-buffer buffer author-name author-email subject date))
+ buffer))
+
(defun git-commit-file ()
"Commit the marked file(s), asking for a commit message."
(interactive)
(unless git-status (error "Not in git-status buffer."))
(when (git-run-pre-commit-hook)
- (let ((buffer (get-buffer-create "*git-commit*"))
- (coding-system (git-get-commits-coding-system))
- author-name author-email subject date)
- (when (eq 0 (buffer-size buffer))
- (when (file-readable-p ".dotest/info")
- (with-temp-buffer
- (insert-file-contents ".dotest/info")
- (goto-char (point-min))
- (when (re-search-forward "^Author: \\(.*\\)\nEmail: \\(.*\\)$" nil t)
- (setq author-name (match-string 1))
- (setq author-email (match-string 2)))
- (goto-char (point-min))
- (when (re-search-forward "^Subject: \\(.*\\)$" nil t)
- (setq subject (match-string 1)))
- (goto-char (point-min))
- (when (re-search-forward "^Date: \\(.*\\)$" nil t)
- (setq date (match-string 1)))))
- (git-setup-log-buffer buffer author-name author-email subject date))
- (if (boundp 'log-edit-diff-function)
- (log-edit 'git-do-commit nil '((log-edit-listfun . git-log-edit-files)
- (log-edit-diff-function . git-log-edit-diff)) buffer)
- (log-edit 'git-do-commit nil 'git-log-edit-files buffer))
- (setq font-lock-keywords (font-lock-compile-keywords git-log-edit-font-lock-keywords))
- (setq buffer-file-coding-system coding-system)
- (re-search-forward (regexp-quote (concat git-log-msg-separator "\n")) nil t))))
+ (git-start-log-edit (git-prepare-log-buffer) 'git-do-commit)))
(defun git-setup-commit-buffer (commit)
"Setup the commit buffer with the contents of COMMIT."
@@ -1340,6 +1432,56 @@ amended version of it."
(git-setup-commit-buffer commit)
(git-commit-file))))
+(defun git-commit-patch (patch)
+ "Commit the patch in file PATCH, asking for a commit message."
+ (interactive "fPatch file: ")
+ (unless git-status (error "Not in git-status buffer."))
+ (when (git-run-pre-commit-hook)
+ (git-start-log-edit (git-prepare-log-buffer)
+ `(lambda ()
+ (interactive)
+ (git-do-commit-patch ,patch)))))
+
+(defun git-commit-patch-buffer (patch)
+ "Commit the patch in buffer PATCH, asking for a commit message."
+ (interactive "bPatch buffer")
+ (when (stringp patch)
+ (setq patch (get-buffer patch)))
+ (unless git-status (error "Not in git-status buffer."))
+ (when (git-run-pre-commit-hook)
+ (git-start-log-edit (git-prepare-log-buffer)
+ `(lambda ()
+ (interactive)
+ (git-do-commit-patch ,patch)))))
+
+(defun git-commit-email-patch (email)
+ "Commit the patch in the email in buffer EMAIL."
+ (interactive "bEmail buffer")
+ (require 'message)
+ (when (stringp email)
+ (setq email (get-buffer email)))
+ (let ((buffer (get-buffer-create "*git-commit-patch*"))
+ author-name author-email email-subject subject date
+ body-start patch-start)
+ (with-current-buffer email
+ (setq body-start (save-excursion (message-goto-body))
+ patch-start (git-find-patch body-start))
+ (let ((address (git-parse-email-address (message-fetch-field "From")))
+ (email-subject (git-fetch-header "Subject" patch-start)))
+ (setq author-name (car address)
+ author-email (cdr address)
+ subject (when (string-match "\\`\\[PATCH[^]]*\\]\s *"
+ email-subject)
+ (substring email-subject (match-end 0)))
+ date (message-fetch-field "Date")
+ msg (buffer-substring body-start patch-start))))
+ (when (git-run-pre-commit-hook)
+ (git-setup-log-buffer buffer author-name author-email subject date msg)
+ (git-start-log-edit buffer
+ `(lambda ()
+ (interactive)
+ (git-do-commit-patch ,email ,patch-start))))))
+
(defun git-find-file ()
"Visit the current file in its own buffer."
(interactive)
--
1.5.5.3
^ permalink raw reply related
* [PATCH 2/2] LsTree: Enable pattern matching in LsTree
From: Robin Rosenberg @ 2008-06-22 23:25 UTC (permalink / raw)
To: git; +Cc: Shawn O. Pearce, Marek Zawirski, Florian Koeberle,
Robin Rosenberg
In-Reply-To: <1214177145-18963-1-git-send-email-robin.rosenberg@dewire.com>
Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
.../src/org/spearce/jgit/pgm/LsTree.java | 19 +++++++++++++------
1 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/org.spearce.jgit/src/org/spearce/jgit/pgm/LsTree.java b/org.spearce.jgit/src/org/spearce/jgit/pgm/LsTree.java
index c242bd7..2cda485 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/pgm/LsTree.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/pgm/LsTree.java
@@ -41,10 +41,13 @@ import java.io.File;
import org.spearce.jgit.lib.Constants;
import org.spearce.jgit.lib.FileMode;
+import org.spearce.jgit.revwalk.filter.WildCardTreeFilter;
import org.spearce.jgit.treewalk.FileTreeIterator;
import org.spearce.jgit.treewalk.TreeWalk;
+import org.spearce.jgit.treewalk.filter.TreeFilter;
class LsTree extends TextBuiltin {
+
@Override
void execute(final String[] args) throws Exception {
final TreeWalk walk = new TreeWalk(db);
@@ -60,17 +63,21 @@ class LsTree extends TextBuiltin {
break;
}
- if (argi == args.length)
- throw die("usage: [-r] treename");
- else if (argi + 1 < args.length)
- throw die("too many arguments");
-
- final String n = args[argi];
+ final String n = args[argi++];
if (is_WorkDir(n))
walk.addTree(new FileTreeIterator(new File(n)));
else
walk.addTree(resolve(n));
+ if (argi == args.length - 1) {
+ TreeFilter filter = WildCardTreeFilter.create(args[argi++]);
+ walk.setFilter(filter);
+ }
+ if (argi + 1 == args.length)
+ throw die("usage: [-r] treename [pattern]");
+ else if (argi + 1 < args.length)
+ throw die("too many arguments");
+
while (walk.next()) {
final FileMode mode = walk.getFileMode(1);
if (mode == FileMode.TREE)
--
1.5.5.1.178.g1f811
^ permalink raw reply related
* [PATCH 1/2] Create a fnmatch-style pattern TreeFilter
From: Robin Rosenberg @ 2008-06-22 23:25 UTC (permalink / raw)
To: git; +Cc: Shawn O. Pearce, Marek Zawirski, Florian Koeberle,
Robin Rosenberg
This uses Florian's pattern matcher to perform the matching.
Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
.../jgit/revwalk/filter/WildCardTreeFilter.java | 101 ++++++++++++++++++++
1 files changed, 101 insertions(+), 0 deletions(-)
create mode 100644 org.spearce.jgit/src/org/spearce/jgit/revwalk/filter/WildCardTreeFilter.java
diff --git a/org.spearce.jgit/src/org/spearce/jgit/revwalk/filter/WildCardTreeFilter.java b/org.spearce.jgit/src/org/spearce/jgit/revwalk/filter/WildCardTreeFilter.java
new file mode 100644
index 0000000..fd75458
--- /dev/null
+++ b/org.spearce.jgit/src/org/spearce/jgit/revwalk/filter/WildCardTreeFilter.java
@@ -0,0 +1,101 @@
+/*
+ * Copyright (C) 2008, Robin Rosenberg <robin.rosenberg@dewire.com>
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following
+ * disclaimer in the documentation and/or other materials provided
+ * with the distribution.
+ *
+ * - Neither the name of the Git Development Community nor the
+ * names of its contributors may be used to endorse or promote
+ * products derived from this software without specific prior
+ * written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+package org.spearce.jgit.revwalk.filter;
+
+import java.io.IOException;
+
+import org.spearce.jgit.errors.IncorrectObjectTypeException;
+import org.spearce.jgit.errors.InvalidPatternException;
+import org.spearce.jgit.errors.MissingObjectException;
+import org.spearce.jgit.fnmatch.FileNameMatcher;
+import org.spearce.jgit.treewalk.TreeWalk;
+import org.spearce.jgit.treewalk.filter.TreeFilter;
+
+/**
+ * This class implements a TreeeFilter that uses the wildcard style pattern
+ * matching like of Posix fnmatch function.
+ */
+public class WildCardTreeFilter extends TreeFilter {
+
+ private final FileNameMatcher matcher;
+
+ private final String pattern;
+
+ protected WildCardTreeFilter(final String pattern) {
+ try {
+ this.pattern = pattern;
+ matcher = new FileNameMatcher(pattern, null);
+ } catch (InvalidPatternException e) {
+ throw new IllegalArgumentException(e);
+ }
+ }
+
+ @Override
+ public TreeFilter clone() {
+ return new WildCardTreeFilter(pattern);
+ }
+
+ @Override
+ public boolean include(TreeWalk walker) throws MissingObjectException,
+ IncorrectObjectTypeException, IOException {
+ matcher.reset();
+ matcher.append(walker.getPathString());
+ if (matcher.isMatch())
+ return true;
+ return false;
+ }
+
+ @Override
+ public boolean shouldBeRecursive() {
+ return true;
+ }
+
+ /**
+ * Construct a WildCardmatcher like POSIX fnmatch.
+ *
+ * @param pattern
+ * A POSIX wildcard pattern
+ * @return a {@link TreeFilter} that matches pattern
+ * @throws IllegalArgumentException
+ * if the pattern is malformed
+ */
+ public static TreeFilter create(final String pattern) {
+ return new WildCardTreeFilter(pattern);
+ }
+
+}
--
1.5.5.1.178.g1f811
^ permalink raw reply related
* Re: [JGIT RFC PATCH] Add a stdio prompt for SSH connection information.
From: Shawn O. Pearce @ 2008-06-22 23:13 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: Marek Zawirski, git
In-Reply-To: <200806222306.25434.robin.rosenberg.lists@dewire.com>
Robin Rosenberg <robin.rosenberg.lists@dewire.com> wrote:
> Starting a command line utility like jgit and getting a graphical prompt is almost
> an insult. The problem here is that Java 5, which we support, does not have a
> portable way of disabling echoing of characters. Java 6 (and anythung newr)
> does. There are several solutions involving non-portable tricks. Should we
> support an insecure practice of echoing passwords, or as I do here, only support
> it if one is using Java 6. A downside of supporting it at all is that one needs a
> JavaSE 6 compiler to build the thing.
We really shouldn't support insecure entry of the password. But I'm
also not ready to give up on Java 5 support either. I wonder if
we shouldn't hide the console reading/writing into a class in our
util package and use reflection to access it, like we do for the
executable flag of java.io.File.
> btw, does anyone know if console() yields null when runnings as a Windows
> service? I tentatively assume that it does without explicily setting the headless
> property.
I think that services on Windows have no console, and also can't
talk to the desktop UI, so its both headless and without a console.
> I'm also a little unsure about how to invoke the promptKeyboardInteractive method.
I think you implemented this method correctly. Its a confusing API,
but it does seem to make sense.
--
Shawn.
^ permalink raw reply
* Re: [PATCH] Take care of errors reported from the server when upload command is started
From: Shawn O. Pearce @ 2008-06-22 23:01 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: git, Marek Zawirski
In-Reply-To: <1214156797-29186-1-git-send-email-robin.rosenberg@dewire.com>
Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
> @@ -44,4 +46,31 @@ class EclipseSshSessionFactory extends SshSessionFactory {
> + @Override
> + public OutputStream getErrorStream() {
...
> + Activator.logError(s, new Throwable());
I'm not sure what value the Throwable gives us here; it may be some
call stack deep within JSch, isn't it? Is it useful to include?
We may also want the log records in Eclipse to say which remote the
message came from, which means passing in the URIish as a parameter.
> @@ -77,6 +78,7 @@ class TransportGitSsh extends PackTransport {
> }
>
> final SshSessionFactory sch;
> + OutputStream errStream;
Could we avoid putting the error stream as an instance member
of the transport by instead using channel.getErrStream() in the
exception case below?
> @@ -179,7 +181,8 @@ class TransportGitSsh extends PackTransport {
> cmd.append(' ');
> sqAlways(cmd, path);
> channel.setCommand(cmd.toString());
> - channel.setErrStream(System.err);
> + errStream = SshSessionFactory.getInstance().getErrorStream();
Use sch rather than SshSessionFactory.getInstance(). We store it in the
transport so that once the transport instance is created it always goes
to the same SshSessionFactory for anything it needs, even if the caller
has changed the global SshSessionFactory away on us.
> @@ -198,7 +201,12 @@ class TransportGitSsh extends PackTransport {
> try {
> session = openSession();
> channel = exec(session, getOptionUploadPack());
> - init(channel.getInputStream(), channel.getOutputStream());
> +
> + if (channel.isConnected())
> + init(channel.getInputStream(), channel.getOutputStream());
> + else
> + throw new TransportException(errStream.toString());
I think you can say channel.getErrStream() here and not need the
instance member.
--
Shawn.
^ 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