* Re: [RFC/PATCH] rebase: add -x option to record original commit name
From: Junio C Hamano @ 2010-02-06 17:47 UTC (permalink / raw)
To: tytso; +Cc: Jay Soffian, Eric Wong, git
In-Reply-To: <20100206133227.GE321@thunk.org>
tytso@mit.edu writes:
> I really don't think it's a contamination of the log when it's used to
> record a cherry pick of patch from the dev stream to an older
> maintainance branch,...
Ok, then I'll happily scrap my earlier comment and accept -x to rebase,
too.
Thanks for a feedback and injection of some sanity.
^ permalink raw reply
* Re: notes TODOs
From: Junio C Hamano @ 2010-02-06 18:01 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Giuseppe Bilotta, Johan Herland, git, Johannes Schindelin
In-Reply-To: <m3aavma9gg.fsf@localhost.localdomain>
Jakub Narebski <jnareb@gmail.com> writes:
> 'git notes' move [-f] <oldobject> <newobject>
I suspect "copy" to keep the old one than "move" would be a lot more
sensible, especially when you are talking about people (like me) who amend
often. They cannot get it right in their first try by definition ;-), and
their very original edition is sometimes easier to start from than their
second edition, when they are trying to come up with the final edition of
the commit. Using "move" to lose the notes from the old object will make
it harder to go back to the original and start amending from there.
^ permalink raw reply
* Re: [gitolite] repo config for delegated projects
From: Teemu Matilainen @ 2010-02-06 18:21 UTC (permalink / raw)
To: Sitaram Chamarty; +Cc: martin f krafft, git discussion list
In-Reply-To: <2e24e5b91002051650k3c7cf14ev8752d36b5616e9a4@mail.gmail.com>
On Sat, 06 Feb 2010, Sitaram Chamarty wrote:
> Now I just looked up hooks.showrev, and it's supposed to be any shell
> command. Clearly this means anyone who can set that gitconfig option
> now has shell capability, and it's game over.
But of course you need to have a hook that runs the command. And
setting hooks requires shell access.
Sorry for not thinking any problems with the config thing. I personally
don't use the delegation and on the other hand all our gitolite
administrators anyway have shell access to the server...
> Regardless of how I look at it, I can't think of a cure for this short
> of either:
> - putting all the allowed gitconfigs in the RC file, and not in the
> config (writing the RC file requires shell access, and we presume the
> "root of trust" person has enough smarts to know what to allow and
> what not to allow), and allowing repo admins to *refer* to them to use
> whichever they want
This sounds better solution for me.
> - someone coming up with a list of gitconfig's that are "safe", and
> specific values for those that are unsafe (like saying "if you use
> showrev, you can only use this command as the value", and forcing
> only those.
Might get too complicated. Anyway the person setting the hook script
should know what it does and which configuration keys it uses and how.
--
- Teemu
^ permalink raw reply
* [PATCH v2] cvsimport: new -R option: generate .git/cvs-revisions mapping
From: Aaron Crane @ 2010-02-06 18:26 UTC (permalink / raw)
To: git, Jeff King
This option causes the creation or updating of a file mapping CVS
(filename, revision number) pairs to Git commit IDs. This is expected
to be useful if you have CVS revision numbers stored in commit messages,
bug-tracking systems, email archives, and the like.
Signed-off-by: Aaron Crane <git@aaroncrane.co.uk>
---
See also the thread beginning at
http://thread.gmane.org/gmane.comp.version-control.git/138079
Thanks to Peff for his review at
http://article.gmane.org/gmane.comp.version-control.git/138544
I believe this revised patch takes account of all his comments. In
particular, compared to the previous version:
- Tests are included
- Now works with incremental import
- The file is always generated as .git/cvs-revisions, rather than
letting the user pick the name
Documentation/git-cvsimport.txt | 18 +++++++++++++++++-
git-cvsimport.perl | 21 +++++++++++++++++----
t/t9600-cvsimport.sh | 36 +++++++++++++++++++++++++++++++++---
3 files changed, 67 insertions(+), 8 deletions(-)
diff --git a/Documentation/git-cvsimport.txt b/Documentation/git-cvsimport.txt
index ddfcb3d..8bcd875 100644
--- a/Documentation/git-cvsimport.txt
+++ b/Documentation/git-cvsimport.txt
@@ -13,7 +13,7 @@ SYNOPSIS
[-A <author-conv-file>] [-p <options-for-cvsps>] [-P <file>]
[-C <git_repository>] [-z <fuzz>] [-i] [-k] [-u] [-s <subst>]
[-a] [-m] [-M <regex>] [-S <regex>] [-L <commitlimit>]
- [-r <remote>] [<CVS_module>]
+ [-r <remote>] [-R] [<CVS_module>]
DESCRIPTION
@@ -157,6 +157,22 @@ It is not recommended to use this feature if you intend to
export changes back to CVS again later with
'git cvsexportcommit'.
+-R::
+ Generate a `$GIT_DIR/cvs-revisions` file containing a mapping from CVS
+ revision numbers to newly-created Git commit IDs. The generated file
+ will contain one line for each (filename, revision) pair imported;
+ each line will look like
++
+---------
+src/widget.c 1.1 1d862f173cdc7325b6fa6d2ae1cfd61fd1b512b7
+---------
++
+The revision data is appended to the file if it already exists, for use when
+doing incremental imports.
++
+This option may be useful if you have CVS revision numbers stored in commit
+messages, bug-tracking systems, email archives, and the like.
+
-h::
Print a short usage message and exit.
diff --git a/git-cvsimport.perl b/git-cvsimport.perl
index 4853bf7..9e03eee 100755
--- a/git-cvsimport.perl
+++ b/git-cvsimport.perl
@@ -29,7 +29,7 @@ use IPC::Open2;
$SIG{'PIPE'}="IGNORE";
$ENV{'TZ'}="UTC";
-our ($opt_h,$opt_o,$opt_v,$opt_k,$opt_u,$opt_d,$opt_p,$opt_C,$opt_z,$opt_i,$opt_P,
$opt_s,$opt_m,@opt_M,$opt_A,$opt_S,$opt_L, $opt_a, $opt_r);
+our ($opt_h,$opt_o,$opt_v,$opt_k,$opt_u,$opt_d,$opt_p,$opt_C,$opt_z,$opt_i,$opt_P,
$opt_s,$opt_m,@opt_M,$opt_A,$opt_S,$opt_L, $opt_a, $opt_r, $opt_R);
my (%conv_author_name, %conv_author_email);
sub usage(;$) {
@@ -40,7 +40,7 @@ Usage: git cvsimport # fetch/update GIT from CVS
[-o branch-for-HEAD] [-h] [-v] [-d CVSROOT] [-A author-conv-file]
[-p opts-for-cvsps] [-P file] [-C GIT_repository] [-z fuzz] [-i] [-k]
[-u] [-s subst] [-a] [-m] [-M regex] [-S regex] [-L commitlimit]
- [-r remote] [CVS_module]
+ [-r remote] [-R] [CVS_module]
END
exit(1);
}
@@ -110,7 +110,7 @@ sub read_repo_config {
}
}
-my $opts = "haivmkuo:d:p:r:C:z:s:M:P:A:S:L:";
+my $opts = "haivmkuo:d:p:r:C:z:s:M:P:A:S:L:R";
read_repo_config($opts);
Getopt::Long::Configure( 'no_ignore_case', 'bundling' );
@@ -659,6 +659,11 @@ if ($opt_A) {
write_author_info("$git_dir/cvs-authors");
}
+# open .git/cvs-revisions, if requested
+open my $revision_map, '>>', "$git_dir/cvs-revisions"
+ or die "Can't open $git_dir/cvs-revisions for appending: $!\n"
+ if defined $opt_R;
+
#
# run cvsps into a file unless we are getting
@@ -742,7 +747,7 @@ sub write_tree () {
}
my ($patchset,$date,$author_name,$author_email,$branch,$ancestor,$tag,$logmsg);
-my (@old,@new,@skipped,%ignorebranch);
+my (@old,@new,@skipped,%ignorebranch,@commit_revisions);
# commits that cvsps cannot place anywhere...
$ignorebranch{'#CVSPS_NO_BRANCH'} = 1;
@@ -825,6 +830,11 @@ sub commit {
system('git' , 'update-ref', "$remote/$branch", $cid) == 0
or die "Cannot write branch $branch for update: $!\n";
+ if ($revision_map) {
+ print $revision_map "@$_ $cid\n" for @commit_revisions;
+ }
+ @commit_revisions = ();
+
if ($tag) {
my ($xtag) = $tag;
$xtag =~ s/\s+\*\*.*$//; # Remove stuff like ** INVALID ** and ** FUNKY **
@@ -959,6 +969,7 @@ while (<CVS>) {
push(@skipped, $fn);
next;
}
+ push @commit_revisions, [$fn, $rev];
print "Fetching $fn v $rev\n" if $opt_v;
my ($tmpname, $size) = $cvs->file($fn,$rev);
if ($size == -1) {
@@ -981,7 +992,9 @@ while (<CVS>) {
unlink($tmpname);
} elsif ($state == 9 and
/^\s+(.+?):\d+(?:\.\d+)+->(\d+(?:\.\d+)+)\(DEAD\)\s*$/) {
my $fn = $1;
+ my $rev = $2;
$fn =~ s#^/+##;
+ push @commit_revisions, [$fn, $rev];
push(@old,$fn);
print "Delete $fn\n" if $opt_v;
} elsif ($state == 9 and /^\s*$/) {
diff --git a/t/t9600-cvsimport.sh b/t/t9600-cvsimport.sh
index 363345f..b572ce3 100755
--- a/t/t9600-cvsimport.sh
+++ b/t/t9600-cvsimport.sh
@@ -47,13 +47,20 @@ EOF
test_expect_success 'import a trivial module' '
- git cvsimport -a -z 0 -C module-git module &&
+ git cvsimport -a -R -z 0 -C module-git module &&
test_cmp module-cvs/o_fortuna module-git/o_fortuna
'
test_expect_success 'pack refs' 'cd module-git && git gc && cd ..'
+test_expect_success 'initial import has correct .git/cvs-revisions' '
+
+ (cd module-git &&
+ git log --format="o_fortuna 1.1 %H" -1) > expected &&
+ test_cmp expected module-git/.git/cvs-revisions
+'
+
test_expect_success 'update cvs module' '
cd module-cvs &&
@@ -86,13 +93,21 @@ EOF
test_expect_success 'update git module' '
cd module-git &&
- git cvsimport -a -z 0 module &&
+ git cvsimport -a -R -z 0 module &&
git merge origin &&
cd .. &&
test_cmp module-cvs/o_fortuna module-git/o_fortuna
'
+test_expect_success 'update has correct .git/cvs-revisions' '
+
+ (cd module-git &&
+ git log --format="o_fortuna 1.1 %H" -1 HEAD^ &&
+ git log --format="o_fortuna 1.2 %H" -1 HEAD) > expected &&
+ test_cmp expected module-git/.git/cvs-revisions
+'
+
test_expect_success 'update cvs module' '
cd module-cvs &&
@@ -107,13 +122,22 @@ test_expect_success 'cvsimport.module config works' '
cd module-git &&
git config cvsimport.module module &&
- git cvsimport -a -z0 &&
+ git cvsimport -a -R -z0 &&
git merge origin &&
cd .. &&
test_cmp module-cvs/tick module-git/tick
'
+test_expect_success 'second update has correct .git/cvs-revisions' '
+
+ (cd module-git &&
+ git log --format="o_fortuna 1.1 %H" -1 HEAD^^ &&
+ git log --format="o_fortuna 1.2 %H" -1 HEAD^
+ git log --format="tick 1.1 %H" -1 HEAD) > expected &&
+ test_cmp expected module-git/.git/cvs-revisions
+'
+
test_expect_success 'import from a CVS working tree' '
$CVS co -d import-from-wt module &&
@@ -126,6 +150,12 @@ test_expect_success 'import from a CVS working tree' '
'
+test_expect_success 'no .git/cvs-revisions created by default' '
+
+ ! test -e import-from-wt/.git/cvs-revisions
+
+'
+
test_expect_success 'test entire HEAD' 'test_cmp_branch_tree master'
test_done
--
1.6.6.1
^ permalink raw reply related
* Re: [PATCH] t9501: Skip testing load if we can't detect it
From: Junio C Hamano @ 2010-02-06 18:31 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Brian Gernhardt, Git List
In-Reply-To: <201002061505.13886.jnareb@gmail.com>
Jakub Narebski <jnareb@gmail.com> writes:
> or the test should use
>
> $maxload = -1;
>
> to artificially force 'load too high' situation.
One related thing that bugs me somewhat is that there is no corresponding
test to make sure that "too loaded" does not trigger when it shouldn't.
But it is harder to arrange (there is no safe $maxload value you can
hardcode in the test and guarantee the system will not loaded that high),
so you need to teach the get_loadavg() to lie under test only for the sake
of running the test, which is backwards---so I'd say that is not worth it.
Here is my attempt to reword the commit log message from Brian (the "-1"
one, squashed into a revert of the one from yesterday, the latter of which
I already pushed out). I just added the first paragraph to better justify
the reason why we are testing the codepath that would never be exercised
in real life on platforms that lack /proc/loadavg.
Subject: t9501: Re-fix max load test
Revert the previous attempt to skip this test on platforms where we
currently cannot determine the system load. We want to make sure that
the max-load-limit codepath produces results cleanly, when gitweb is
updated and becomes capable of reading the load average by some other
method.
The code to check for load returns 0 if it doesn't know how to find
load. It also checks to see if the current load is higher than the
max load. So to force the script to quit early by setting the maxload
variable negative which should work for systems where we can detect
load (which should be a positive number) and systems where we can't
(where detected load is 0)
Signed-off-by: Brian Gernhardt <brian@gernhardtsoftware.com>
^ permalink raw reply
* Re: [PATCH] grep: simple test for operation in a bare repository
From: Junio C Hamano @ 2010-02-06 18:47 UTC (permalink / raw)
To: René Scharfe; +Cc: Git Mailing List, Nguyen Thai Ngoc Duy, Junio C Hamano
In-Reply-To: <4B6D37D7.6010505@lsrfire.ath.cx>
René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
> This patch converts the setenv() calls in path.c and setup.c. After
> the call, git grep with a pager works again in bare repos.
>
> It leaves the setenv(GIT_DIR_ENVIRONMENT, ...) calls in git.c alone, as
> they respond to command line switches that emulate the effect of setting
> the environment variable directly.
>
> The remaining site in environment.c is in set_git_dir() and is left
> alone, too, of course. Finally, builtin-init-db.c is left changed
> because the repo is still being carefully constructed when the
> environment variable is set.
Thanks for a thorough analysis. The patch looks correct and I am very
tempted to put it into 1.7.0, though I'd stop at queuing it to 'next'.
^ permalink raw reply
* Re: [PATCH] add new options to git format-patch: --cover-subject and --cover-blurb
From: Jonathan Nieder @ 2010-02-06 19:13 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Larry D'Anna, Wesley J. Landaker, git
In-Reply-To: <7vtytvjhit.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
> Larry D'Anna <larry@elder-gods.org> writes:
>> 1) make your branch
>>
>> 2) git format-patch --cover-letter
>>
>> 3) edit the cover letter
>>
>> 3) review the series, and realize you need to fix something, fix it.
>
> Hmph, this begs a natural question: why didn't you review and realize that
> in step (1)?
One answer: writing a cover letter forces one to reflect a little.
Perhaps that is why the cover letter and review share step 3. ;-)
> It might be sufficient for format-patch to notice a 0000-cover file that
> is already there, read the subject and blurb part and carry that forward,
> instead of unconditionally writing "*** SUBJECT HERE ***" and stuff. That
> way, the user does not have to prepare a separate file before running
> format-patch.
FWIW I think this sounds sane and would be happy to see this feature.
Jonathan
> By scanning from the bottom of the existing 0000-cover file, skipping
> diffstat part (easy to spot with regexp) and then skip backwards a block
> of text whose lines are one of:
>
> (1) two space indented---that's one-line-per-commit;
>
> (2) empty line---separator; or
>
> (3) unindented line that ends with '(' number ')' ':'---the author.
>
> The remainder would be the BLURB. And you know it is much easier to find
> where the Subject: is ;-)
^ permalink raw reply
* imap.preformattedHTML and imap.sslverify
From: Junio C Hamano @ 2010-02-06 19:26 UTC (permalink / raw)
To: git; +Cc: Jeremy White, Robert Shearman
I hate to bring up a topic that is almost a year old, but has either of
these configuration variables ever worked?
The code does this while reading its configuration file:
static int git_imap_config(const char *key, const char *val, void *cb)
{
char imap_key[] = "imap.";
if (strncmp(key, imap_key, sizeof imap_key - 1))
return 0;
if (!val)
return config_error_nonbool(key);
...
else if (!strcmp("sslverify", key))
server.ssl_verify = git_config_bool(key, val);
else if (!strcmp("preformattedHTML", key))
server.use_html = git_config_bool(key, val);
Two issues:
- The body of the function is protected by "nonbool" written back in the
days when there was no boolean variables in imap.* namespace. Hence,
a user cannot write
[imap]
sslverify
and turn it on. The user needs to write
[imap]
sslverify = True
which is against the parsing rules for boolean variables.
- The config parser downcases the key before calling the parse callback
function, so !strcmp("preformattedHTML", key) will never trigger.
The fix is obvious (see below), but I am far more disturbed by the
apparent lack of testing. Especially, preformattedHTML one would have
never worked as setting the configuration is the only way to trigger this.
Could peole _test_ this patch and report, as I don't use this program at
all.
Thanks.
imap-send.c | 15 ++++++++-------
1 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/imap-send.c b/imap-send.c
index de8114b..ea769a9 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -1335,11 +1335,16 @@ static int git_imap_config(const char *key, const char *val, void *cb)
if (strncmp(key, imap_key, sizeof imap_key - 1))
return 0;
- if (!val)
- return config_error_nonbool(key);
-
key += sizeof imap_key - 1;
+ /* check booleans first, and barf on others */
+ if (!strcmp("sslverify", key))
+ server.ssl_verify = git_config_bool(key, val);
+ else if (!strcmp("preformattedhtml", key))
+ server.use_html = git_config_bool(key, val);
+ else if (!val)
+ return config_error_nonbool(key);
+
if (!strcmp("folder", key)) {
imap_folder = xstrdup(val);
} else if (!strcmp("host", key)) {
@@ -1360,10 +1365,6 @@ static int git_imap_config(const char *key, const char *val, void *cb)
server.port = git_config_int(key, val);
else if (!strcmp("tunnel", key))
server.tunnel = xstrdup(val);
- else if (!strcmp("sslverify", key))
- server.ssl_verify = git_config_bool(key, val);
- else if (!strcmp("preformattedHTML", key))
- server.use_html = git_config_bool(key, val);
return 0;
}
^ permalink raw reply related
* Re: [PATCH] t9501: Correctly force over max load everywhere
From: Jakub Narebski @ 2010-02-06 19:45 UTC (permalink / raw)
To: Brian Gernhardt; +Cc: Junio C Hamano, Git List
In-Reply-To: <1265467803-31210-1-git-send-email-brian@gernhardtsoftware.com>
Dnia sobota 6. lutego 2010 15:50, Brian Gernhardt napisał:
> The code to check for load returns 0 if it doesn't know how to find
> load. It also checks to see if the current load is higher than the
> max load. So to force the script to quit early by setting the maxload
> variable negative which should work for systems where we can detect
> load (which should be a positive number) and systems where we can't
> (where detected load is 0)
>
> Signed-off-by: Brian Gernhardt <brian@gernhardtsoftware.com>
Acked-by: Jakub Narebski <jnareb@gmail.com>
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [PATCH v2] cvsimport: new -R option: generate .git/cvs-revisions mapping
From: Aaron Crane @ 2010-02-06 17:15 UTC (permalink / raw)
To: git; +Cc: Jeff King, Junio C Hamano
This option causes the creation or updating of a file mapping CVS
(filename, revision number) pairs to Git commit IDs. This is expected
to be useful if you have CVS revision numbers stored in commit messages,
bug-tracking systems, email archives, and the like.
Signed-off-by: Aaron Crane <git@aaroncrane.co.uk>
---
Re-sent because of line wrapping; sorry for the noise.
See also the thread beginning at
http://thread.gmane.org/gmane.comp.version-control.git/138079
Thanks to Peff for his review at
http://article.gmane.org/gmane.comp.version-control.git/138544
I believe this revised patch takes account of all his comments. In
particular, compared to the previous version:
- Tests are included
- Now works with incremental import
- The file is always generated as .git/cvs-revisions, rather than
letting the user pick the name
Documentation/git-cvsimport.txt | 18 +++++++++++++++++-
git-cvsimport.perl | 21 +++++++++++++++++----
t/t9600-cvsimport.sh | 36 +++++++++++++++++++++++++++++++++---
3 files changed, 67 insertions(+), 8 deletions(-)
diff --git a/Documentation/git-cvsimport.txt b/Documentation/git-cvsimport.txt
index ddfcb3d..8bcd875 100644
--- a/Documentation/git-cvsimport.txt
+++ b/Documentation/git-cvsimport.txt
@@ -13,7 +13,7 @@ SYNOPSIS
[-A <author-conv-file>] [-p <options-for-cvsps>] [-P <file>]
[-C <git_repository>] [-z <fuzz>] [-i] [-k] [-u] [-s <subst>]
[-a] [-m] [-M <regex>] [-S <regex>] [-L <commitlimit>]
- [-r <remote>] [<CVS_module>]
+ [-r <remote>] [-R] [<CVS_module>]
DESCRIPTION
@@ -157,6 +157,22 @@ It is not recommended to use this feature if you intend to
export changes back to CVS again later with
'git cvsexportcommit'.
+-R::
+ Generate a `$GIT_DIR/cvs-revisions` file containing a mapping from CVS
+ revision numbers to newly-created Git commit IDs. The generated file
+ will contain one line for each (filename, revision) pair imported;
+ each line will look like
++
+---------
+src/widget.c 1.1 1d862f173cdc7325b6fa6d2ae1cfd61fd1b512b7
+---------
++
+The revision data is appended to the file if it already exists, for use when
+doing incremental imports.
++
+This option may be useful if you have CVS revision numbers stored in commit
+messages, bug-tracking systems, email archives, and the like.
+
-h::
Print a short usage message and exit.
diff --git a/git-cvsimport.perl b/git-cvsimport.perl
index 4853bf7..9e03eee 100755
--- a/git-cvsimport.perl
+++ b/git-cvsimport.perl
@@ -29,7 +29,7 @@ use IPC::Open2;
$SIG{'PIPE'}="IGNORE";
$ENV{'TZ'}="UTC";
-our ($opt_h,$opt_o,$opt_v,$opt_k,$opt_u,$opt_d,$opt_p,$opt_C,$opt_z,$opt_i,$opt_P, $opt_s,$opt_m,@opt_M,$opt_A,$opt_S,$opt_L, $opt_a, $opt_r);
+our ($opt_h,$opt_o,$opt_v,$opt_k,$opt_u,$opt_d,$opt_p,$opt_C,$opt_z,$opt_i,$opt_P, $opt_s,$opt_m,@opt_M,$opt_A,$opt_S,$opt_L, $opt_a, $opt_r, $opt_R);
my (%conv_author_name, %conv_author_email);
sub usage(;$) {
@@ -40,7 +40,7 @@ Usage: git cvsimport # fetch/update GIT from CVS
[-o branch-for-HEAD] [-h] [-v] [-d CVSROOT] [-A author-conv-file]
[-p opts-for-cvsps] [-P file] [-C GIT_repository] [-z fuzz] [-i] [-k]
[-u] [-s subst] [-a] [-m] [-M regex] [-S regex] [-L commitlimit]
- [-r remote] [CVS_module]
+ [-r remote] [-R] [CVS_module]
END
exit(1);
}
@@ -110,7 +110,7 @@ sub read_repo_config {
}
}
-my $opts = "haivmkuo:d:p:r:C:z:s:M:P:A:S:L:";
+my $opts = "haivmkuo:d:p:r:C:z:s:M:P:A:S:L:R";
read_repo_config($opts);
Getopt::Long::Configure( 'no_ignore_case', 'bundling' );
@@ -659,6 +659,11 @@ if ($opt_A) {
write_author_info("$git_dir/cvs-authors");
}
+# open .git/cvs-revisions, if requested
+open my $revision_map, '>>', "$git_dir/cvs-revisions"
+ or die "Can't open $git_dir/cvs-revisions for appending: $!\n"
+ if defined $opt_R;
+
#
# run cvsps into a file unless we are getting
@@ -742,7 +747,7 @@ sub write_tree () {
}
my ($patchset,$date,$author_name,$author_email,$branch,$ancestor,$tag,$logmsg);
-my (@old,@new,@skipped,%ignorebranch);
+my (@old,@new,@skipped,%ignorebranch,@commit_revisions);
# commits that cvsps cannot place anywhere...
$ignorebranch{'#CVSPS_NO_BRANCH'} = 1;
@@ -825,6 +830,11 @@ sub commit {
system('git' , 'update-ref', "$remote/$branch", $cid) == 0
or die "Cannot write branch $branch for update: $!\n";
+ if ($revision_map) {
+ print $revision_map "@$_ $cid\n" for @commit_revisions;
+ }
+ @commit_revisions = ();
+
if ($tag) {
my ($xtag) = $tag;
$xtag =~ s/\s+\*\*.*$//; # Remove stuff like ** INVALID ** and ** FUNKY **
@@ -959,6 +969,7 @@ while (<CVS>) {
push(@skipped, $fn);
next;
}
+ push @commit_revisions, [$fn, $rev];
print "Fetching $fn v $rev\n" if $opt_v;
my ($tmpname, $size) = $cvs->file($fn,$rev);
if ($size == -1) {
@@ -981,7 +992,9 @@ while (<CVS>) {
unlink($tmpname);
} elsif ($state == 9 and /^\s+(.+?):\d+(?:\.\d+)+->(\d+(?:\.\d+)+)\(DEAD\)\s*$/) {
my $fn = $1;
+ my $rev = $2;
$fn =~ s#^/+##;
+ push @commit_revisions, [$fn, $rev];
push(@old,$fn);
print "Delete $fn\n" if $opt_v;
} elsif ($state == 9 and /^\s*$/) {
diff --git a/t/t9600-cvsimport.sh b/t/t9600-cvsimport.sh
index 363345f..b572ce3 100755
--- a/t/t9600-cvsimport.sh
+++ b/t/t9600-cvsimport.sh
@@ -47,13 +47,20 @@ EOF
test_expect_success 'import a trivial module' '
- git cvsimport -a -z 0 -C module-git module &&
+ git cvsimport -a -R -z 0 -C module-git module &&
test_cmp module-cvs/o_fortuna module-git/o_fortuna
'
test_expect_success 'pack refs' 'cd module-git && git gc && cd ..'
+test_expect_success 'initial import has correct .git/cvs-revisions' '
+
+ (cd module-git &&
+ git log --format="o_fortuna 1.1 %H" -1) > expected &&
+ test_cmp expected module-git/.git/cvs-revisions
+'
+
test_expect_success 'update cvs module' '
cd module-cvs &&
@@ -86,13 +93,21 @@ EOF
test_expect_success 'update git module' '
cd module-git &&
- git cvsimport -a -z 0 module &&
+ git cvsimport -a -R -z 0 module &&
git merge origin &&
cd .. &&
test_cmp module-cvs/o_fortuna module-git/o_fortuna
'
+test_expect_success 'update has correct .git/cvs-revisions' '
+
+ (cd module-git &&
+ git log --format="o_fortuna 1.1 %H" -1 HEAD^ &&
+ git log --format="o_fortuna 1.2 %H" -1 HEAD) > expected &&
+ test_cmp expected module-git/.git/cvs-revisions
+'
+
test_expect_success 'update cvs module' '
cd module-cvs &&
@@ -107,13 +122,22 @@ test_expect_success 'cvsimport.module config works' '
cd module-git &&
git config cvsimport.module module &&
- git cvsimport -a -z0 &&
+ git cvsimport -a -R -z0 &&
git merge origin &&
cd .. &&
test_cmp module-cvs/tick module-git/tick
'
+test_expect_success 'second update has correct .git/cvs-revisions' '
+
+ (cd module-git &&
+ git log --format="o_fortuna 1.1 %H" -1 HEAD^^ &&
+ git log --format="o_fortuna 1.2 %H" -1 HEAD^
+ git log --format="tick 1.1 %H" -1 HEAD) > expected &&
+ test_cmp expected module-git/.git/cvs-revisions
+'
+
test_expect_success 'import from a CVS working tree' '
$CVS co -d import-from-wt module &&
@@ -126,6 +150,12 @@ test_expect_success 'import from a CVS working tree' '
'
+test_expect_success 'no .git/cvs-revisions created by default' '
+
+ ! test -e import-from-wt/.git/cvs-revisions
+
+'
+
test_expect_success 'test entire HEAD' 'test_cmp_branch_tree master'
test_done
--
1.6.6.1
^ permalink raw reply related
* Re: [PATCH] t9501: Skip testing load if we can't detect it
From: Brian Gernhardt @ 2010-02-06 20:09 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jakub Narebski, Git List
In-Reply-To: <7vwryq1aid.fsf@alter.siamese.dyndns.org>
On Feb 6, 2010, at 1:31 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Here is my attempt to reword the commit log message from Brian (the
> "-1"
> one, squashed into a revert of the one from yesterday, the latter of
> which
> I already pushed out). I just added the first paragraph to better
> justify
> the reason why we are testing the codepath that would never be
> exercised
> in real life on platforms that lack /proc/loadavg.
Looks good to me.
> Subject: t9501: Re-fix max load test
>
> Revert the previous attempt to skip this test on platforms where we
> currently cannot determine the system load. We want to make sure
> that
> the max-load-limit codepath produces results cleanly, when gitweb
> is
> updated and becomes capable of reading the load average by some
> other
> method.
>
> The code to check for load returns 0 if it doesn't know how to find
> load. It also checks to see if the current load is higher than the
> max load. So to force the script to quit early by setting the
> maxload
> variable negative which should work for systems where we can detect
> load (which should be a positive number) and systems where we can't
> (where detected load is 0)
>
> Signed-off-by: Brian Gernhardt <brian@gernhardtsoftware.com>
^ permalink raw reply
* Gmail and unwanted line-wrapping
From: Aaron Crane @ 2010-02-06 20:29 UTC (permalink / raw)
To: git
Documentation/SubmittingPatches says this about dealing with Gmail's
propensity for breaking your email:
> GMail does not appear to have any way to turn off line wrapping in the web
> interface, so this will mangle any emails that you send. You can however
> use any IMAP email client to connect to the google imap server, and forward
> the emails through that. Just make sure to disable line wrapping in that
> email client. Alternatively, use "git send-email" instead.
>
> Submitting properly formatted patches via Gmail is simple now that
> IMAP support is available. First, edit your ~/.gitconfig to specify your
> account settings:
<snip>
> Next, ensure that your Gmail settings are correct. In "Settings" the
> "Use Unicode (UTF-8) encoding for outgoing messages" should be checked.
>
> Once your commits are ready to send to the mailing list, run the following
> command to send the patch emails to your Gmail Drafts folder.
>
> $ git format-patch -M --stdout origin/master | git imap-send
>
> Go to your Gmail account, open the Drafts folder, find the patch email, fill
> in the To: and CC: fields and send away!
However, the advice beginning "Submitting properly formatted patches
via Gmail is simple now" doesn't match my experience. Following those
guidelines seemed to work, but my patch was line-wrapped anyway.
It seems I'm not the only person who's observed this:
http://article.gmane.org/gmane.comp.version-control.git/133020
Can anyone think of anything I might have done wrong here? If not,
I'm inclined to suggest dropping all of that advice. That's not
ideal, because it leaves Gmail users with no obvious way to submit
well-formatted patches to the list; but it's better than suggesting
something which apparently doesn't work.
--
Aaron Crane ** http://aaroncrane.co.uk/
^ permalink raw reply
* Re: Gmail and unwanted line-wrapping
From: Sverre Rabbelier @ 2010-02-06 20:47 UTC (permalink / raw)
To: Aaron Crane; +Cc: git
In-Reply-To: <bc341e101002061229t7a1525c2w2d5a8e221124b3c2@mail.gmail.com>
Heya,
On Sat, Feb 6, 2010 at 21:29, Aaron Crane <git@aaroncrane.co.uk> wrote:
> Can anyone think of anything I might have done wrong here? If not,
> I'm inclined to suggest dropping all of that advice. That's not
> ideal, because it leaves Gmail users with no obvious way to submit
> well-formatted patches to the list; but it's better than suggesting
> something which apparently doesn't work.
I'm using git send-email with this configuration:
[sendemail]
smtencryption = tls
smtpserver = smtp.gmai.com
smtpuser = srabbelier@gmail.com
smptserverport = 587
It'll ask you for your password when you're sending the emails. Like so:
$ git format-patch --no-color -C -M origin/master..topic-branch -o outgoing/
$ git send-email --compose outgoing/00*
Of course you should substitute specific things as appropriate, I
doubt you'll want to be sending your patches pretending to be me :P.
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Re: Gmail and unwanted line-wrapping
From: Jacob Helwig @ 2010-02-06 20:49 UTC (permalink / raw)
To: Aaron Crane; +Cc: git
In-Reply-To: <bc341e101002061229t7a1525c2w2d5a8e221124b3c2@mail.gmail.com>
On 20:29 Sat 06 Feb , Aaron Crane wrote:
> Documentation/SubmittingPatches says this about dealing with Gmail's
> propensity for breaking your email:
>
> > GMail does not appear to have any way to turn off line wrapping in the web
> > interface, so this will mangle any emails that you send. You can however
> > use any IMAP email client to connect to the google imap server, and forward
> > the emails through that. Just make sure to disable line wrapping in that
> > email client. Alternatively, use "git send-email" instead.
> >
> > Submitting properly formatted patches via Gmail is simple now that
> > IMAP support is available. First, edit your ~/.gitconfig to specify your
> > account settings:
> <snip>
> > Next, ensure that your Gmail settings are correct. In "Settings" the
> > "Use Unicode (UTF-8) encoding for outgoing messages" should be checked.
> >
> > Once your commits are ready to send to the mailing list, run the following
> > command to send the patch emails to your Gmail Drafts folder.
> >
> > $ git format-patch -M --stdout origin/master | git imap-send
> >
> > Go to your Gmail account, open the Drafts folder, find the patch email, fill
> > in the To: and CC: fields and send away!
>
> However, the advice beginning "Submitting properly formatted patches
> via Gmail is simple now" doesn't match my experience. Following those
> guidelines seemed to work, but my patch was line-wrapped anyway.
>
> It seems I'm not the only person who's observed this:
> http://article.gmane.org/gmane.comp.version-control.git/133020
>
> Can anyone think of anything I might have done wrong here? If not,
> I'm inclined to suggest dropping all of that advice. That's not
> ideal, because it leaves Gmail users with no obvious way to submit
> well-formatted patches to the list; but it's better than suggesting
> something which apparently doesn't work.
>
> --
> Aaron Crane ** http://aaroncrane.co.uk/
I don't use the web interface at all, when I want to send out patches
using GMail (we use Google Apps for Business at $day_job). I just use
"git send-email" and send the patch(es) out straight from the command
line. There's a nice summary online [0] of how to get this setup with
GMail.
I don't get the handy address-book of the web interface, but I don't
have to worry about line-wrapping.
[0] http://morefedora.blogspot.com/2009/02/configuring-git-send-email-to-use-gmail.html
--
Jacob Helwig
^ permalink raw reply
* Re: Gmail and unwanted line-wrapping
From: Sverre Rabbelier @ 2010-02-06 20:59 UTC (permalink / raw)
To: Jacob Helwig; +Cc: Aaron Crane, git
In-Reply-To: <20100206204917.GA30894@vfb-9.home>
Heya,
On Sat, Feb 6, 2010 at 21:49, Jacob Helwig <jacob.helwig@gmail.com> wrote:
> I don't get the handy address-book of the web interface, but I don't
> have to worry about line-wrapping.
I get both using the magic awesomeness that is copy/paste. (Figure out
who to mail in web interface, then copy/paste To: line to cmdline)
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Re: [PATCH 3/4] gitweb: show notes in log
From: Jakub Narebski @ 2010-02-06 21:47 UTC (permalink / raw)
To: Giuseppe Bilotta; +Cc: git, Johannes Schindelin, Johan Herland, Junio C Hamano
In-Reply-To: <cb7bb73a1002060514p33a4bcbdqb13d636cd6448998@mail.gmail.com>
On Sat, 6 Feb 2010, Giuseppe Bilotta wrote:
> 2010/2/6 Jakub Narebski <jnareb@gmail.com>:
> > On Thu, 4 Feb 2010, Giuseppe Bilotta wrote:
> >
> > > The notes are shown in full to the left of the log message.
> >
> > Thats all good if you have wide (high resolution) screen, and your
> > project follows common commit message conventions of keeping lines in
> > commit message no longer than at most 80 characters, and you don't need
> > to use large size fonts.
> >
> > What happens if screen size is too small to contain both commit message
> > and notes? Does it do the sensible thing of putting notes _below_
> > commit message in such situation? I do not know CSS+HTML enogh to
> > answer this question myself.
>
> The CSS forces the width of the notes div at 150px, which is the
> amount left to the left of the commit message. This means that notes
> will line-wrap, but they will not shift the text.
[...]
> > > + print "$notes\n";
> > > print "<div class=\"log_body\">\n";
> > > git_print_log($co{'comment'}, -final_empty_line=> 1);
> > > print "</div>\n";
> >
> > With respect to the question about what happens if the screen is not
> > wide enough, shouldn't notes be put in HTML source below body (commit
> > message)?
>
> As I mentioned, notes width is fixed at the amount of the whitespace
> to the left of the log, so this should not be an issue. Additionally,
> putting notes below makes it much harder to let them float to the left
> of the log body.
Perhaps the log body should be floated to the right, instead of notes
being floated to the left, so that when screen width is to narrow for
both commit message and notes, notes would be put below commit message.
A question how to create styles using HTML elements and CSS styling
to get side-by-side with one below other as fallback can be asked
on http://stackoverflow.com, or perhaps even better on http://doctype.com/
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [PATCH 4/4] gitweb: show notes in commit(diff) view
From: Jakub Narebski @ 2010-02-06 21:50 UTC (permalink / raw)
To: Giuseppe Bilotta; +Cc: git, Johannes Schindelin, Johan Herland, Junio C Hamano
In-Reply-To: <cb7bb73a1002060813l451ad669nf9ba2dc4643bbd58@mail.gmail.com>
On Sat, 6 Feb 2010, Giuseppe Bilotta wrote:
> On Sat, Feb 6, 2010 at 3:34 PM, Jakub Narebski <jnareb@gmail.com> wrote:
>> Giuseppe Bilotta wrote:
>>> 2010/2/6 Jakub Narebski <jnareb@gmail.com>:
>>>> On Thu, 4 Jan 2010, Giuseppe Bilotta wrote:
>>
>> [...]
>>>>> # return all refs matching refs/notes/<globspecs> where the globspecs
>>>>> # are taken from the notes feature content.
>>>>> sub get_note_refs {
>>>>> + local $/ = "";
>>>>> +
>>>>
>>>> Why it is needed here? Why you want to use empty lines as terminator
>>>> (which means reading whole paragraphs), while treating two or more
>>>> consecutive empty lines as a single empty line (according to
>>>> perlvar(1))?
>>>>
>>>> If you want to slurp whole file, this should be
>>>>
>>>> local $/;
>>>>
>>>> or more explicit
>>>>
>>>> local $/ = undef;
>>>
>>> Ah, sorry, for some reason I thought "" was the default.
>>
>> If you wanted to use default value, why set it at all?
>
> Ach, sorry, forgot to reply to the first part of the question. It's
> used in a context where $/ is locally set to \0, so it needs to be
> reset.
Oh, so it should be something like the following, then?
sub get_note_refs {
+ # reset to default value (can be called with $/ set to "\0")
+ local $/ = "\n"; # line by line
>
> --
> Giuseppe "Oblomov" Bilotta
>
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [PATCH 1/4] gitweb: notes feature
From: Jakub Narebski @ 2010-02-06 22:14 UTC (permalink / raw)
To: Giuseppe Bilotta; +Cc: git, Johannes Schindelin, Johan Herland, Junio C Hamano
In-Reply-To: <cb7bb73a1002060102n5544f825x3d01860b1228cee@mail.gmail.com>
On Sat, 6 Feb 2010, Giuseppe Bilotta wrote:
> 2010/2/6 Jakub Narebski <jnareb@gmail.com>:
>> On Thu, 4 Feb 2010, Giuseppe Bilotta wrote:
>>> + # Notes support. When this feature is enabled, the presence of notes
>>> + # for any commit is signaled, and the note content is made available
>>> + # in a way appropriate for the current view.
>>> + # Set this to '*' to enable all notes namespace, or to a shell-glob
>>> + # specification to enable specific namespaces only.
>>
>> It is not obvious from this description that you can provide _list_ of
>> notes namespaces (or list of shell-globs).
>
> I'm starting to think it might make sense to not have a list here, but
> rather a single value only. First of all, multiple refs can be
> indicated à la shell with {ref1,ref2,ref3}. Or, we can also use the
> intended command-line syntax ref1:ref2:ref3, which would help
> consistency. It also makes things easier for project overrides, as per
> your subsequent comment:
So it is to be single shell-glob / fnmatch (I think) compatible pattern,
isn't it?
[...]
>>> + my %notes = () ;
>>> + foreach my $note_ref (@note_refs) {
>>> + my $obj = "$note_ref:$co{'id'}";
>>> + if (open my $fd, '-|', git_cmd(), 'rev-parse',
>>> + '--verify', '-q', $obj) {
>>> + my $exists = <$fd>;
>>> + close $fd;
>>> + if (defined $exists) {
>>> + if (open $fd, '-|', git_cmd(), 'show', $obj) {
>>> + $notes{$note_ref} = scalar <$fd>;
>>> + close $fd;
>>> + }
>>> + }
>>> + }
>>> + }
>>
>> First, there are '--batch' and '--batch-check' options to git-cat-file.
>> With these I think you can get all notes with just single git command,
>> although using it is a bit complicated (requires open2 from IPC::Open2
>> for bidi communication).
>
> Hm. The IPC::Open2 doc makes it sound horribly scary, but still doable.
It would look something like the following (fragment of my WIP code):
use IPC::Open2 qw(open2);
use IO::Handle;
# ...
unless ($object_stdout) {
# Open bidi pipe the first time get_object is called.
# open2 raises an exception on error, no need to 'or die'.
$object_pid =
open2($object_stdout, $object_stdin,
git_cmd(), 'cat-file', '--batch');
}
$object_stdin->printflush("$object_id\n") # NB: \n required to avoid deadlocking
or die "get_object: cannot write to pipe: $!";
my ($sha1, $type, $size) =
split ' ', $object_stdout->getline()
or die "get_object: cannot read from pipe: $!";
die "'$object_id' not found in repository"
if $type eq 'missing';
$object_stdout->read(my $content, $size);
$object_stdout->getline(); # eat trailing newline
The above fragment of code is tested that it works. You would probably
need to replace dies with something less fatal...
>> Second, if not using 'git cat-file --batch', perhaps it would be easier
>> to read each $note_ref tree using 'git ls-tree'/'git ls-tree -r', and
>> parse its output to check for which commits/objects there are notes
>> available, and only then call 'git show' (or reuse connection to
>> 'git cat-file --batch').
>>
>> The second solution, with a bit more work, could work even in presence
>> of fan-out schemes for notes, I think.
>
> An interesting approach. Without fan-out, git ls-tree -r
> refs/notes/whatever [hash ...] gives us the blobs we're interested in.
> In case of fan-out schemes, the efficiency of this approach probably
> depends on the kind of fan-out we have, and would require some
> heavy-duty grepping. A git ls-notes plumbing with a similar syntax and
> output would be a nice thing to have.
No grepping, just pass '-r' option to 'git-ls-tree', and use
parse_ls_tree_line() to parse lines. Then if we have fanout scheme
we would get, I guess, something like the following:
100644 blob 23da787d... de/adbeef...
100644 blob bc10f25f... c5/31d986...
100644 blob c9656ece... 24/d93129...
Now you only need to s!/!!g on filename to get SHA1 of annotated object
(for which is the note).
The identifier of note itself would be either id from tree (e.g. 23da787d...
for note from first line), or note namespace composed with note "pathname",
(e.g. refs/notes/commits:de/adbeef... for note from first line).
Even if you use one git-show per note, it would need only git commands
to discover object to note mapping.
P.S. We still would want parse_commit_text to parse notes from default
namespace. parse_commit / parse_commits output contains notes from
default namespace, e.g.:
d6bbe7fd52058cdf0e48bec00701ae0f4861dcd3 94ac0c7b30a7dc43d926b0ffbe90892c1e19e5f6
tree b9ee8876df81b80b13c6b017be993fff8427cfaf
parent 94ac0c7b30a7dc43d926b0ffbe90892c1e19e5f6
author Jakub Narebski <jnareb@gmail.com> 1265309578 +0100
committer Jakub Narebski <jnareb@gmail.com> 1265309578 +0100
This is a commit message
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
Notes:
This is just a note for commit d6bbe7fd52058cdf0e48bec00701ae0f4861dcd3
to get commit message lines in $co{'comment'} (as array reference),
and notes in $co{'note'} (or $co{'notes'}).
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [PATCH 4/4] gitweb: show notes in commit(diff) view
From: Giuseppe Bilotta @ 2010-02-06 22:17 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git, Johannes Schindelin, Johan Herland, Junio C Hamano
In-Reply-To: <201002062250.43613.jnareb@gmail.com>
On Sat, Feb 6, 2010 at 10:50 PM, Jakub Narebski <jnareb@gmail.com> wrote:
>
> Oh, so it should be something like the following, then?
>
> sub get_note_refs {
> + # reset to default value (can be called with $/ set to "\0")
> + local $/ = "\n"; # line by line
Yes, it's probably worth to mention it in a comment.
--
Giuseppe "Oblomov" Bilotta
^ permalink raw reply
* Re: [msysGit] upload-pack timing issue on windows?
From: Johannes Sixt @ 2010-02-06 22:18 UTC (permalink / raw)
To: kusmabite; +Cc: msysgit, Git Mailing List
In-Reply-To: <40aa078e1002060401r1dec3c2ate3ddd4f5f5db1e0c@mail.gmail.com>
On Samstag, 6. Februar 2010, Erik Faye-Lund wrote:
> However, I have tracked down a bit of what goes on in the client.
> There's a call to read_in_full, called from pack-write.c, line 246
> that fails in the failure-case, but not in the success-case. This is
> where the client expects "pack\tSHA-1" or "keep\tSHA-1". There "fatal:
> early EOF"-messages seems to originate from index-pack.c, line 197.
> This is the first line of code in parse_pack_header(), it's also
> AFAICT the first call-site for any read(0, <...>) (though fill()).
This looks like upload-pack died without sending enough to fill a pack header.
Try merging this branch:
git://repo.or.cz/git/mingw/j6t.git async-in-thread
It contains your changes to start_async plus a refinement of die() when it is
called from the async procedure (it passes t5530, for example). It is also
converted to pthreads, and therefore also works on Unix. The new
implementation of start_async is more careful about the file handles, though
not so much on Windows.
If there's no change for you, then you could look into implementing
fcntl(F_GETFD/SETFD, FD_CLOEXEC), which are currently ignored, on top of this
branch, using Get/SetHandleInformation().
Background: On Unix, we need FD_CLOEXEC so that the fds that are meant for the
async thread do not remain open in an unrelated child process; on Windows, we
are just lucky and can get away without FD_CLOEXEC because our pipe()s are
non-inheritable and async only work with pipes. But once we pass other fds to
the async procedure, we need a working FD_CLOEXEC. Perhaps something in this
direction is related to your problem.
You could push out your current state of the git-daemon and a recipe to
reproduce the problem. Perhaps I find some time to look into it.
-- Hannes
^ permalink raw reply
* Re: [PATCH 1/4] gitweb: notes feature
From: Giuseppe Bilotta @ 2010-02-06 22:58 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git, Johannes Schindelin, Johan Herland, Junio C Hamano
In-Reply-To: <201002062314.00631.jnareb@gmail.com>
On Sat, Feb 6, 2010 at 11:14 PM, Jakub Narebski <jnareb@gmail.com> wrote:
> On Sat, 6 Feb 2010, Giuseppe Bilotta wrote:
>> 2010/2/6 Jakub Narebski <jnareb@gmail.com>:
>>> On Thu, 4 Feb 2010, Giuseppe Bilotta wrote:
>
>>>> + # Notes support. When this feature is enabled, the presence of notes
>>>> + # for any commit is signaled, and the note content is made available
>>>> + # in a way appropriate for the current view.
>>>> + # Set this to '*' to enable all notes namespace, or to a shell-glob
>>>> + # specification to enable specific namespaces only.
>>>
>>> It is not obvious from this description that you can provide _list_ of
>>> notes namespaces (or list of shell-globs).
>>
>> I'm starting to think it might make sense to not have a list here, but
>> rather a single value only. First of all, multiple refs can be
>> indicated à la shell with {ref1,ref2,ref3}. Or, we can also use the
>> intended command-line syntax ref1:ref2:ref3, which would help
>> consistency. It also makes things easier for project overrides, as per
>> your subsequent comment:
>
> So it is to be single shell-glob / fnmatch (I think) compatible pattern,
> isn't it?
Sort of. fnmatch doesn't do brace expansion, which is a pity IMO, but
that's just my personal preference. Colon-separated, fnmatched
components is probably the easiest thing to implement to have multiple
refs. I'll go with whatever is chosen for core.
>>> First, there are '--batch' and '--batch-check' options to git-cat-file.
>>> With these I think you can get all notes with just single git command,
>>> although using it is a bit complicated (requires open2 from IPC::Open2
>>> for bidi communication).
>>
>> Hm. The IPC::Open2 doc makes it sound horribly scary, but still doable.
>
> It would look something like the following (fragment of my WIP code):
>
> use IPC::Open2 qw(open2);
> use IO::Handle;
>
> # ...
>
> unless ($object_stdout) {
> # Open bidi pipe the first time get_object is called.
> # open2 raises an exception on error, no need to 'or die'.
> $object_pid =
> open2($object_stdout, $object_stdin,
> git_cmd(), 'cat-file', '--batch');
> }
> $object_stdin->printflush("$object_id\n") # NB: \n required to avoid deadlocking
> or die "get_object: cannot write to pipe: $!";
> my ($sha1, $type, $size) =
> split ' ', $object_stdout->getline()
> or die "get_object: cannot read from pipe: $!";
> die "'$object_id' not found in repository"
> if $type eq 'missing';
> $object_stdout->read(my $content, $size);
> $object_stdout->getline(); # eat trailing newline
>
> The above fragment of code is tested that it works. You would probably
> need to replace dies with something less fatal...
On the other hand, as mentioned by Junio, this approach is not
future-proof enough for any kind of fan-out schemes.
>>> Second, if not using 'git cat-file --batch', perhaps it would be easier
>>> to read each $note_ref tree using 'git ls-tree'/'git ls-tree -r', and
>>> parse its output to check for which commits/objects there are notes
>>> available, and only then call 'git show' (or reuse connection to
>>> 'git cat-file --batch').
>>>
>>> The second solution, with a bit more work, could work even in presence
>>> of fan-out schemes for notes, I think.
>>
>> An interesting approach. Without fan-out, git ls-tree -r
>> refs/notes/whatever [hash ...] gives us the blobs we're interested in.
>> In case of fan-out schemes, the efficiency of this approach probably
>> depends on the kind of fan-out we have, and would require some
>> heavy-duty grepping. A git ls-notes plumbing with a similar syntax and
>> output would be a nice thing to have.
>
> No grepping, just pass '-r' option to 'git-ls-tree', and use
> parse_ls_tree_line() to parse lines. Then if we have fanout scheme
> we would get, I guess, something like the following:
>
> 100644 blob 23da787d... de/adbeef...
> 100644 blob bc10f25f... c5/31d986...
> 100644 blob c9656ece... 24/d93129...
>
> Now you only need to s!/!!g on filename to get SHA1 of annotated object
> (for which is the note).
What worries me is that you're going to get fan-outs when there are
LOTS of notes, and that's precisely the kind of situation where you
_don't_ want to go through all the notes to pick the ones you're only
interested in.
If we have a guarantee that the fan-outs follow a 2/[2/...] scheme,
the open2 approach might still be the best way to go, by just trying
not only namespace:xxxxx...xxx but also namespace:xx/xxxxx etc.
Horrible, but could still be coalesced in a single call. It mgiht also
be optimized to stop at the first successfull hit in a namespace.
> P.S. We still would want parse_commit_text to parse notes from default
> namespace. parse_commit / parse_commits output contains notes from
> default namespace, e.g.:
>
> d6bbe7fd52058cdf0e48bec00701ae0f4861dcd3 94ac0c7b30a7dc43d926b0ffbe90892c1e19e5f6
> tree b9ee8876df81b80b13c6b017be993fff8427cfaf
> parent 94ac0c7b30a7dc43d926b0ffbe90892c1e19e5f6
> author Jakub Narebski <jnareb@gmail.com> 1265309578 +0100
> committer Jakub Narebski <jnareb@gmail.com> 1265309578 +0100
>
> This is a commit message
>
> Signed-off-by: Jakub Narebski <jnareb@gmail.com>
>
> Notes:
> This is just a note for commit d6bbe7fd52058cdf0e48bec00701ae0f4861dcd3
>
> to get commit message lines in $co{'comment'} (as array reference),
> and notes in $co{'note'} (or $co{'notes'}).
I'm not getting these in the repo I'm testing this. And I think this
is indeed the behavior of current git next
--
Giuseppe "Oblomov" Bilotta
^ permalink raw reply
* Re: [RFC PATCH 00/10] gitweb: Simple file based output caching
From: Jakub Narebski @ 2010-02-06 23:56 UTC (permalink / raw)
To: J.H.; +Cc: git, John 'Warthog9' Hawley, Petr Baudis
In-Reply-To: <4B6CBD05.6040604@eaglescrag.net>
On Sat, 6 Feb 2010, J.H. wrote:
> > Table of contents:
> > ~~~~~~~~~~~~~~~~~~
> > [RFC PATCH 01/10] gitweb: Print to explicit filehandle (preparing
> > for caching)
I am working on v2 of this series, where this patch is not necessary.
It uses *STDOUT->push_layer(scalar => \$data) and *STDOUT->pop_layer()
from PerlIO::Util if it is available, and manipulation of *STDOUT (which
means *STDOUT = $data_fh and not $out = $data_fh). But I must say that
doing capture of STDOUT (only; STDERR is not captured) without requiring
extra Perl modules (like recommended Capture::Tiny or e.g. IO::Capture),
and especially testing that it works correctly with capturing output
of cache_fetch is serious PITA.
This patch has the advantage that all operations are simpler. In
particular it is easy to have section which should be not captured,
or where capture should be turned off (slightly different).
It has the disadvantage that all future contributions must use
"print $out <something>" / "print {$out} <something>", and that
contributions from before this change would have to be carefully
updated. (Well, we could probably add the test that would check
that everything that needs to go to $out does, and everything that
shouldn't got to $out but to STDOUT doesn't.)
If I were to have such patch in new version of "gitweb output
caching" series, I would make the following changes:
* (optionally) use simpler 'print $out <sth>' instead of visually
distinct 'print {$out} <sth>', where from first glance one can
see that $out is filehandle and not something to be printed
* use short filehandle name: $out, or $oh, or $o/$O.
* split above patch in 2 to 4 patches:
- pure mechanical (scripted) change:
+ print <sth> -> print $out <sth>
+ printf(<sth>) -> printf($out <sth>)
+ binmode STDOUT -> binmode $out
The last with possible exception of very first binmode call.
- realign (purely whitespace change)
- wrap too long lines (newlines and whitespace), optional
- change $out to $bout/$bin ($binary_output_fh) where needed;
but see comment below (optional)
>
> This looks fine, I did some quick testing to verify that this would work
> - and it does.
I have only ran test, and didn't actually check that it works correctly.
This commit shouldn't change gitweb behaviour at all.
>
> The only caveat that needs to be aware is that if the layer is going to
> output binary data it needs to flip the whole stream to :raw before
> outputting (this is going to be more specific to the caching layer).
>
> One advantage to having the file handles separate is that it's easier to
> distinguish if the data is going to need to be binary data that will
> need to be flipped properly.
I don't think that it would be needed.
First, all mode changing operations, i.e. calls to binmode are changed
to act on $out rather than on STDOUT it means. It means that if we are
using 'in memory file' to capture output to scalar variable, then captured
data would be properly converted in variable. So it would be enough to
save this variable in :raw mode to file. If we are saving directly to
cache file, then of course saved data would go through layer and would
be converted properly. In any case in cache file we would have _already_
_converted_ data.
This means that regardless whether $out used ':utf8' (pseudo)layer,
or ':raw' (pseudo)layer, if we read from cache file in ':raw' (binary mode)
and print data from cache to original (true) STDOUT also in ':raw' mode,
we would print correctly formatted data.
>
> Also means you could cache the binary data differently than textual data.
>
> I.E. binary data gets saved to disk, but page data gets saved to memcached.
That's true, but on the other hand it would be easy to add some extra
command marking data as binary below binmode. Or we can examine IO
layers (using PerlIO::get_layers($out); the PerlIO module is in Perl
core) if there is 'utf8' layer or 'raw' (pseudo)layer.
>
> Just food for thought, I'm not sure which way makes more sense
> personally, though I would have a tendency to err on the side of
> flexibility and have both.
It might be good idea... but nevertheless I'd like to have short name
for binary filehandle, if we decode to keep it. What should it be?
$bout, $bin, $B, $bin_out, $out_bin, $bin_fh?
>
> > [RFC PATCH 02/10] gitweb: href(..., -path_info => 0|1)
>
> note: delaying additional comment till I've finished reading through the
> basics of the following patches.
This is to use later _full_ _normalized_ URI as cache key for given page.
IIRC in your original patch you ignored path_info; but on the other hand
git.kernel.org has path_info feature turned off...
>
> > [RFC PATCH 03/10] gitweb/cache.pm - Very simple file based caching
>
> Ok this is quite the departure from what I had, I'm unsure that it's the
> right way to go, but it obviously has merits (I.E. much simpler addition
> of any Cache::Cache or CHI compatible caching layer)
>
> This patch itself looks fine, and as it states it borrows heavily from
> my basic implementation - just wraps it differently. I might have some
> thoughts on extending this a bit to be a bit more flushed out from a
> basic standpoint.
>
> Need to dig through it some more, but I'm generally ok with it.
Note that the new implementation in (not send yet) new version of
"gitweb output caching" series is based more on newer and more modern
CHI unified interface rather than older Cache::Cache interface. It
is I think much cleaner and easier to read.
The major difference from your implementation is that in my version
the gitweb caching engine uses "save to temporary file + rename file
to final name" method to have atomic write to cache (atomic cache
filling). It should be more robust, but OTOH it introduces a bit of
performance penalty. With locking and single writer we could use
predictable temporary file name rather than using tempfile/mkstemp
or equivalent from File::Temp, or UUID based filename like CHI does
it.
Also, tests.
Current code (even the v2 version) lacks proper error detection, error
signalling and logging.
>
> > [RFC PATCH 04/10] gitweb/cache.pm - Stat-based cache expiration
>
> Looks fine to me, though the note about getting the errors should get
> moved to previous patch, as it says.
I wanted to get this series out faster, that is why it is not polished.
>
> Note: I'm going to stop here as the following are WIP and I want to play
> around with this particular direction on my own a little more before
> further comment. There's some ideas running around I want to try and
> get down in code first. Me moving on and trying these other ideas is
> not a reflection on the following patches, just some alternative
> thinking before I discuss some other ideas on the following patches.
Take a look at gitweb/cache-kernel-v2 branch (the new caching series).
Note however that it would be subject to rebasing / changes.
>
> Also I've been sitting on this e-mail in this state for almost a week
> while I've been playing with this and having to fight other fires and I
> know that Jakub has been looking for commentary on this.
Thank you very much for your commentary, in spite of your heavy load.
>
> > [RFC PATCH 05/10] gitweb: Use Cache::Cache compatibile (get, set)
> > output caching (WIP)
> > [RFC PATCH 06/10] gitweb/cache.pm - Adaptive cache expiration time (WIP)
> > [RFC PATCH 07/10] gitweb: Use CHI compatibile (compute method) caching (WIP)
> > [RFC PATCH 08/10] gitweb/cache.pm - Use locking to avoid 'stampeding herd'
> > problem (WIP)
> > [RFC PATCH 09/10] gitweb/cache.pm - Serve stale data when waiting for
> > filling cache (WIP)
> > [RFC PATCH 10/10] gitweb: Show appropriate "Generating..." page when
> > regenerating cache (WIP)
There is new version of this series in gitweb/cache-kernel-v2 in my
git/jnareb-git.git fork (clone) of git.git repository at repo.or.cz.
Now all commits have proper description (for first series one had to
read comment section in emails for commit description), and all features
are tested (at least on API level, and to some extent) -- full tests
do require having PerlIO::Util installed (I have done it following
local::lib and installing it from 'cpan' client), though.
Also all features are fully configurable, to even greater extent than
in original series by J.H. (this what what v1 was lacking). And there
is (see diffstat) section about caching in gitweb/README.
The following changes since commit d5f8a3d6f4d946c33459e00edf02819f89711777:
Junio C Hamano (1):
Merge branch 'master' into next
are available in the git repository at:
git://repo.or.cz/git/jnareb-git.git gitweb/cache-kernel-v2
You can view it via gitweb at:
http://repo.or.cz/w/git/jnareb-git.git/shortlog/refs/heads/gitweb/cache-kernel-v2
http://repo.or.cz/w/git/jnareb-git.git/log/refs/heads/gitweb/cache-kernel-v2
SHORTLOG (10):
gitweb: href(..., -path_info => 0|1)
gitweb/cache.pm - Very simple file based caching
gitweb/cache.pm - Stat-based cache expiration
gitweb: Use Cache::Cache compatibile (get, set) output caching
gitweb/cache.pm - Adaptive cache expiration time
gitweb: Use CHI compatibile (compute method) caching
gitweb/cache.pm - Use locking to avoid 'cache miss stampede' problem
gitweb/cache.pm - Serve stale data when waiting for filling cache
gitweb/cache.pm - Regenerate (refresh) cache in background
gitweb: Show appropriate "Generating..." page when regenerating cache
gitweb/README | 70 +++++
gitweb/cache.pm | 527 ++++++++++++++++++++++++++++++++
gitweb/gitweb.perl | 301 +++++++++++++++++-
t/gitweb-lib.sh | 2 +
t/t9500-gitweb-standalone-no-errors.sh | 19 ++
t/t9503-gitweb-caching.sh | 32 ++
t/t9503/test_cache_interface.pl | 380 +++++++++++++++++++++++
t/test-lib.sh | 3 +
8 files changed, 1319 insertions(+), 15 deletions(-)
create mode 100644 gitweb/cache.pm
create mode 100755 t/t9503-gitweb-caching.sh
create mode 100755 t/t9503/test_cache_interface.pl
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: Gmail and unwanted line-wrapping
From: David Aguilar @ 2010-02-07 0:24 UTC (permalink / raw)
To: Sverre Rabbelier; +Cc: Aaron Crane, git
In-Reply-To: <fabb9a1e1002061247k7a8fba5at4c687faac0dcabb8@mail.gmail.com>
On Sat, Feb 06, 2010 at 09:47:39PM +0100, Sverre Rabbelier wrote:
> Heya,
>
> On Sat, Feb 6, 2010 at 21:29, Aaron Crane <git@aaroncrane.co.uk> wrote:
> > Can anyone think of anything I might have done wrong here? If not,
> > I'm inclined to suggest dropping all of that advice. That's not
> > ideal, because it leaves Gmail users with no obvious way to submit
> > well-formatted patches to the list; but it's better than suggesting
> > something which apparently doesn't work.
>
> I'm using git send-email with this configuration:
> [sendemail]
> smtencryption = tls
> smtpserver = smtp.gmai.com
> smtpuser = srabbelier@gmail.com
> smptserverport = 587
>
> It'll ask you for your password when you're sending the emails. Like so:
>
> $ git format-patch --no-color -C -M origin/master..topic-branch -o outgoing/
> $ git send-email --compose outgoing/00*
>
> Of course you should substitute specific things as appropriate, I
> doubt you'll want to be sending your patches pretending to be me :P.
>
> --
> Cheers,
>
> Sverre Rabbelier
Great tip!
I've added it to the Git Wiki:
http://git.wiki.kernel.org/index.php/GitTips#Using_gmail_to_send_your_patches
Good stuff,
--
David
^ permalink raw reply
* Re: Gmail and unwanted line-wrapping
From: Jay Soffian @ 2010-02-07 0:50 UTC (permalink / raw)
To: David Aguilar; +Cc: Sverre Rabbelier, Aaron Crane, git
In-Reply-To: <20100207002413.GA14744@gmail.com>
> I've added it to the Git Wiki:
> http://git.wiki.kernel.org/index.php/GitTips#Using_gmail_to_send_your_patches
If you're on MacOS X and have MacPorts installed, then install both
the curl and msmtp packages, then use an .msmtprc as follows:
defaults
tls on
tls_trust_file /opt/local/share/curl/curl-ca-bundle.crt
syslog LOG_MAIL
# gmail
account gmail
host smtp.gmail.com
port 587
auth on
from jaysoffian@gmail.com
user jaysoffian@gmail.com
# Default account
account default : gmail
Adjusting for you account of course. And in your git config:
[sendemail]
smtpserver = /opt/local/bin/msmtp
Now why would you do this you ask instead of using git's built-in SMTP
support? Because I provided a patch to msmtp long ago that lets it
access your gmail password from Mac OS X's keychain. Note that it
looks for a password stored in the same format as Mail.app would store
it, see the msmtp man page for details.
FWIW, I think Gnome also has a keychain like thingy that msmtp can also query.
j.
^ permalink raw reply
* Re: Gmail and unwanted line-wrapping
From: Jay Soffian @ 2010-02-07 0:51 UTC (permalink / raw)
To: David Aguilar; +Cc: Sverre Rabbelier, Aaron Crane, git
In-Reply-To: <76718491002061650ge299426s22de5e00b26af108@mail.gmail.com>
On Sat, Feb 6, 2010 at 7:50 PM, Jay Soffian <jaysoffian@gmail.com> wrote:
> tls_trust_file /opt/local/share/curl/curl-ca-bundle.crt
Actually you need the curl-ca-bundle port.
j.
^ 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