* Re: [PATCH] honour GIT_ASKPASS for querying username in git-svn
From: Sven Strickroth @ 2011-12-27 14:39 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git, Jeff King
In-Reply-To: <m3d3baf5kd.fsf@localhost.localdomain>
Am 27.12.2011 15:33 schrieb Jakub Narebski:
>> +sub prompt {
>> + my ($self, $prompt) = _maybe_self(@_);
>> + if (exists $ENV{'GIT_ASKPASS'}) {
>> + return _prompt($ENV{'GIT_ASKPASS'}, $prompt);
>> + } elsif (exists $ENV{'SSH_ASKPASS'}) {
>> + return _prompt($ENV{'SSH_ASKPASS'}, $prompt);
>> + } else {
>> + return undef;
>> + }
>> +}
>
> ...and provide some kind of fallback even if neither of GIT_ASKPASS
> nor SSH_ASKPASS are set (perhaps assuming that some Perl packages from
> CPAN are installed).
If neither of GIT_ASKPASS nor SSH_ASKPASS are set the caller has to
handle the request. This has to be done this way, because of lots of
different needs (username, password (no echo) and so on).
>> +sub _prompt {
>> + my ($self, $askpass, $prompt) = _maybe_self(@_);
>> + my $ret;
>> + open(PH, "-|", $askpass, $prompt);
>> + $ret = <PH>;
>> + $ret =~ s/[\012\015]//g; # strip \n\r
>> + close(PH);
>> + return $ret;
>> +}
>
> Please, use modern Perl, in particula use lexical filehandles instead
> of typeglobs (which are global variables), i.e.
I used the same style as I found in Git.pm (see lines I removed in patch 2).
--
Best regards,
Sven Strickroth
ClamAV, a GPL anti-virus toolkit http://www.clamav.net
PGP key id F5A9D4C4 @ any key-server
^ permalink raw reply
* Re: [PATCH] honour GIT_ASKPASS for querying username in git-svn
From: Jakub Narebski @ 2011-12-27 14:33 UTC (permalink / raw)
To: Sven Strickroth; +Cc: git, Jeff King
In-Reply-To: <4EF907F1.1030801@tu-clausthal.de>
Sven Strickroth <sven.strickroth@tu-clausthal.de> writes:
> +=item prompt ( PROMPT)
> +
> +Checks if GIT_ASKPASS or SSH_ASKPASS is set, and if yes
> +use it and return answer from user.
> +
> +=cut
I think it would be good idea to describe what this function is for...
> +sub prompt {
> + my ($self, $prompt) = _maybe_self(@_);
> + if (exists $ENV{'GIT_ASKPASS'}) {
> + return _prompt($ENV{'GIT_ASKPASS'}, $prompt);
> + } elsif (exists $ENV{'SSH_ASKPASS'}) {
> + return _prompt($ENV{'SSH_ASKPASS'}, $prompt);
> + } else {
> + return undef;
> + }
> +}
...and provide some kind of fallback even if neither of GIT_ASKPASS
nor SSH_ASKPASS are set (perhaps assuming that some Perl packages from
CPAN are installed).
> +sub _prompt {
> + my ($self, $askpass, $prompt) = _maybe_self(@_);
> + my $ret;
> + open(PH, "-|", $askpass, $prompt);
> + $ret = <PH>;
> + $ret =~ s/[\012\015]//g; # strip \n\r
> + close(PH);
> + return $ret;
> +}
Please, use modern Perl, in particula use lexical filehandles instead
of typeglobs (which are global variables), i.e.
+ open my $fh, "-|", $askpass, $prompt
+ or die "...";
+ $ret = <$fh>;
+ chomp($ret);
+ close($fh)
+ or die "...";
> --
> 1.7.7.1.msysgit.0
>
> From ef4c6557d1b0e33440d13c64742d44b2a22143f3 Mon Sep 17 00:00:00 2001
> From: Sven Strickroth <email@cs-ware.de>
> Date: Tue, 27 Dec 2011 00:34:09 +0100
> Subject: [PATCH 2/4] switch to central prompt method
>
> Signed-off-by: Sven Strickroth <email@cs-ware.de>
Please send those patches as a separate emails, not concatenated in a
single email (perhaps even with cover letter).
See Documentation/SubmittingPatches
[...]
--
Jakub Narebski
^ permalink raw reply
* [PATCH] gc --auto: warn garbage collection happens soon
From: Nguyễn Thái Ngọc Duy @ 2011-12-27 13:45 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
This gives users a chance to run gc explicitly elsewhere if they do not
want gc to run suddenly in current terminal.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
v2 of a patch posted a few months ago. The warning limits are in
percentage and configurable. I could have set the default limits to
100% (i.e. no warnings) to keep current behavior. However I think
warning is better.
May need rewording inn config.txt, I'm not sure I state it clearly.
Documentation/config.txt | 12 ++++++++++++
Documentation/git-gc.txt | 4 ++++
builtin/gc.c | 41 +++++++++++++++++++++++++++++++++++++++--
3 files changed, 55 insertions(+), 2 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 5a841da..c263496 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -965,12 +965,24 @@ gc.auto::
light-weight garbage collection from time to time. The
default value is 6700. Setting this to 0 disables it.
+gc.autowarn::
+ The percentage of loose objects specified in `gc.auto`. If the
+ number of loose objects exceeds this limit, `git gc --auto`
+ will warn users garbage collection will happen soon. Default
+ value is 90. Setting this to 100 disables it.
+
gc.autopacklimit::
When there are more than this many packs that are not
marked with `*.keep` file in the repository, `git gc
--auto` consolidates them into one larger pack. The
default value is 50. Setting this to 0 disables it.
+gc.autopackwarn::
+ The percentage of packs specified in `gc.autopacklimit`. If
+ the number of packs exceeds this limit, `git gc --auto` will
+ warn users garbage collection will happen soon. Default value
+ is 90. Setting this to 100 disables it.
+
gc.packrefs::
Running `git pack-refs` in a repository renders it
unclonable by Git versions prior to 1.5.1.2 over dumb
diff --git a/Documentation/git-gc.txt b/Documentation/git-gc.txt
index 815afcb..937b3d6 100644
--- a/Documentation/git-gc.txt
+++ b/Documentation/git-gc.txt
@@ -59,6 +59,10 @@ then existing packs (except those marked with a `.keep` file)
are consolidated into a single pack by using the `-A` option of
'git repack'. Setting `gc.autopacklimit` to 0 disables
automatic consolidation of packs.
++
+`git gc --auto` will warn users when the number of loose objects or
+packs is close to the limits. See `gc.autowarn` and `gc.autopackwarn`
+for details.
--prune=<date>::
Prune loose objects older than date (default is 2 weeks ago,
diff --git a/builtin/gc.c b/builtin/gc.c
index 0498094..f3fa46d 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -28,6 +28,10 @@ static int gc_auto_threshold = 6700;
static int gc_auto_pack_limit = 50;
static const char *prune_expire = "2.weeks.ago";
+/* numbers are in percent, to be converted to absolute later */
+static int gc_warn_auto_threshold = 90;
+static int gc_warn_auto_pack_limit = 90;
+
#define MAX_ADD 10
static const char *argv_pack_refs[] = {"pack-refs", "--all", "--prune", NULL};
static const char *argv_reflog[] = {"reflog", "expire", "--all", NULL};
@@ -52,10 +56,26 @@ static int gc_config(const char *var, const char *value, void *cb)
gc_auto_threshold = git_config_int(var, value);
return 0;
}
+ if (!strcmp(var, "gc.autowarn")) {
+ int percent = percent = git_config_int(var, value);
+ if (percent <= 0 || percent > 100)
+ die(_("gc.autowarn %d%% does not make sense"),
+ percent);
+ gc_warn_auto_threshold = percent;
+ return 0;
+ }
if (!strcmp(var, "gc.autopacklimit")) {
gc_auto_pack_limit = git_config_int(var, value);
return 0;
}
+ if (!strcmp(var, "gc.autopackwarn")) {
+ int percent = percent = git_config_int(var, value);
+ if (percent <= 0 || percent > 100)
+ die(_("gc.autopackwarn %d%% does not make sense"),
+ percent);
+ gc_warn_auto_pack_limit = percent;
+ return 0;
+ }
if (!strcmp(var, "gc.pruneexpire")) {
if (value && strcmp(value, "now")) {
unsigned long now = approxidate("now");
@@ -118,7 +138,15 @@ static int too_many_loose_objects(void)
}
}
closedir(dir);
- return needed;
+ if (needed)
+ return 1;
+
+ auto_threshold = (gc_warn_auto_threshold + 255) / 256;
+ if (num_loose >= auto_threshold)
+ warning(_("Too many loose objects (current approx. %d, limit %d).\n"
+ "\"git gc\" will soon run automatically"),
+ num_loose * 256, gc_auto_threshold);
+ return 0;
}
static int too_many_packs(void)
@@ -141,7 +169,14 @@ static int too_many_packs(void)
*/
cnt++;
}
- return gc_auto_pack_limit <= cnt;
+ if (gc_auto_pack_limit <= cnt)
+ return 1;
+
+ if (gc_warn_auto_pack_limit <= cnt)
+ warning(_("Too many packs (current %d, limit %d)\n"
+ "\"git gc\" will soon run automatically."),
+ cnt, gc_auto_pack_limit);
+ return 0;
}
static int need_to_gc(void)
@@ -193,6 +228,8 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
usage_with_options(builtin_gc_usage, builtin_gc_options);
git_config(gc_config, NULL);
+ gc_warn_auto_threshold = 0.01 * gc_auto_threshold * gc_warn_auto_threshold;
+ gc_warn_auto_pack_limit = 0.01 * gc_auto_pack_limit * gc_auto_pack_limit;
if (pack_refs < 0)
pack_refs = !is_bare_repository();
--
1.7.8.36.g69ee2
^ permalink raw reply related
* Re: Git beginner - Need help understanding
From: chirin @ 2011-12-27 7:02 UTC (permalink / raw)
To: git
In-Reply-To: <CA++fsGHPKhzfd7-KohOZ4WpYatx_-EW0bjq46zwswbu8TomHCg@mail.gmail.com>
From what I understand, your scenario is exactly what I expect. Which is why
when I asked around my colleagues, no one was able to explain why I'm having
this issue.
As per your scenario:
# A changes hello.txt
# Going into B (who has not done anything to hello.txt)
git pull --> merge conflict on hello.txt
git commit
git pull --> OK
Would going through Gerrit have anything to do with it? I've compared git
config -l with the others but they are the same (aside from
remote.origin.url which has our own gerrit userid).
Dov Grobgeld wrote
>
> The best way of understanding and also of asking questions, is if you
> can reproduce the steps of exactly what you want and don't understand
> by a sequence of commands like so:
>
> # First create a bare repository
> mkdir R
> cd R
> git init --bare .
>
> # Clone it into A
> git clone R A
>
> # Clone it into B
> git clone R B
>
> # Now start doing changes for A and B, pulling and pushing into R
> cd A
> echo "Change #1" > hello.txt
> git add hello.txt
> git commit -m 'Commit #1'
> git push origin master
>
> # Get into B
> cd ../B
> git pull
> echo "Change #2" >> hello.txt
> git commit -a -m 'Commit #2'
> git push
>
> # Get into A and pull the changes done by B
> cd ../A
> git pull
>
> In this sequence, which fulfills the scenario that you described,
> there are no conflicts. So I suggest that you try to change the
> command sequence to illustrate what you don't understand and ask
> again.
>
> Regards,
> Dov
>
--
View this message in context: http://git.661346.n2.nabble.com/Git-beginner-Need-help-understanding-tp7129186p7129429.html
Sent from the git mailing list archive at Nabble.com.
^ permalink raw reply
* Re: [PATCH] add post-fetch hook
From: Junio C Hamano @ 2011-12-27 6:37 UTC (permalink / raw)
To: Joey Hess; +Cc: git
In-Reply-To: <20111226155152.GA29582@gnu.kitenet.net>
Joey Hess <joey@kitenet.net> writes:
> .... And other code in git uses an async feeder similarly,
> see for example convert.c's apply_filter(). So I think this is ok..?
Yeah, I didn't look at your patch (sorry) but if it uses async like the
filtering codepath does, it should be perfectly fine (please forget about
the select(2) based kludge I alluded to; the async interface is the right
thing to use here).
Thanks.
^ permalink raw reply
* Re: Git beginner - Need help understanding
From: Dov Grobgeld @ 2011-12-27 6:34 UTC (permalink / raw)
To: chirin; +Cc: git
In-Reply-To: <CA++fsGGEv=jS4YNEUCxTwZ3pZc7HbbmoPbDH+MamrqamxrsADA@mail.gmail.com>
The best way of understanding and also of asking questions, is if you
can reproduce the steps of exactly what you want and don't understand
by a sequence of commands like so:
# First create a bare repository
mkdir R
cd R
git init --bare .
# Clone it into A
git clone R A
# Clone it into B
git clone R B
# Now start doing changes for A and B, pulling and pushing into R
cd A
echo "Change #1" > hello.txt
git add hello.txt
git commit -m 'Commit #1'
git push origin master
# Get into B
cd ../B
git pull
echo "Change #2" >> hello.txt
git commit -a -m 'Commit #2'
git push
# Get into A and pull the changes done by B
cd ../A
git pull
In this sequence, which fulfills the scenario that you described,
there are no conflicts. So I suggest that you try to change the
command sequence to illustrate what you don't understand and ask
again.
Regards,
Dov
On Tue, Dec 27, 2011 at 05:12, chirin <takonatto@gmail.com> wrote:
>
> I'm just beginning to use Git in my workplace, and (rather shamefully) have
> never heard of Git until now. While I pore over the stacks of documentations
> for beginners, could someone help me understand this issue I've been having?
>
> Every time a colleague updates a file, I would not be able to pull due to
> merge conflicts - even though I have never made any changes to the
> repository. I'd try and then again, but it would still give the same merge
> conflict. The only way I could pull currently is to do a git commit (even
> without changes), and then git pull.
>
> If this was SVN, I could simply resolve this with Update. What am I missing,
> and what should I be looking for?
>
> Thanks in advance..
>
> --
> View this message in context: http://git.661346.n2.nabble.com/Git-beginner-Need-help-understanding-tp7129186p7129186.html
> Sent from the git mailing list archive at Nabble.com.
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* I can never finish a push
From: Bill Zaumen @ 2011-12-27 4:18 UTC (permalink / raw)
To: mresnick, git
Aside from the other comments, you said, "someone else on-site
pushes and adds new commits before mine can finish." If I
interpreted that correctly, the "someone else" should also be
using a link as slow as yours. Why isn't he having the same
problem you are having?
If you can track that person down (his name should show up in
the new commits you pulled), compare your config files to see
what is different.
Bill
^ permalink raw reply
* Git beginner - Need help understanding
From: chirin @ 2011-12-27 3:12 UTC (permalink / raw)
To: git
I'm just beginning to use Git in my workplace, and (rather shamefully) have
never heard of Git until now. While I pore over the stacks of documentations
for beginners, could someone help me understand this issue I've been having?
Every time a colleague updates a file, I would not be able to pull due to
merge conflicts - even though I have never made any changes to the
repository. I'd try and then again, but it would still give the same merge
conflict. The only way I could pull currently is to do a git commit (even
without changes), and then git pull.
If this was SVN, I could simply resolve this with Update. What am I missing,
and what should I be looking for?
Thanks in advance..
--
View this message in context: http://git.661346.n2.nabble.com/Git-beginner-Need-help-understanding-tp7129186p7129186.html
Sent from the git mailing list archive at Nabble.com.
^ permalink raw reply
* [PATCH] Fix an incorrect reference to --set-all.
From: Jelmer Vernooij @ 2011-12-27 2:03 UTC (permalink / raw)
To: git
Signed-off-by: Jelmer Vernooij <jelmer@samba.org>
---
builtin/config.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/builtin/config.c b/builtin/config.c
index 0315ad7..d35c06a 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -444,7 +444,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)
ret = git_config_set(argv[0], value);
if (ret == CONFIG_NOTHING_SET)
error("cannot overwrite multiple values with a single value\n"
- " Use a regexp, --add or --set-all to change %s.", argv[0]);
+ " Use a regexp, --add or --replace-all to change %s.", argv[0]);
return ret;
}
else if (actions == ACTION_SET_ALL) {
--
1.7.7.3
^ permalink raw reply related
* Re: [PATCH] honour GIT_ASKPASS for querying username in git-svn
From: Sven Strickroth @ 2011-12-26 23:49 UTC (permalink / raw)
To: git; +Cc: Jeff King, gitster
In-Reply-To: <20111130064401.GC5317@sigill.intra.peff.net>
Hi,
Am 30.11.2011 07:44 schrieb Jeff King:
> That aside, I think this is an improvement over the current code.
> 1. Regular git will also respect SSH_ASKPASS
> 2. Regular git will ignore an askpass variable that is set but empty.
> Perhaps git-svn should be refactored to have a reusable "prompt"
> function that respects askpass and tries to behave like C git? It could
> even go into the Git perl module.
I honoured all your ideas. Hopefully the patches can be applied now. The new patches
follow (you can also pull from git://github.com/csware/git.git askpass-prompt):
>From b760546c59d1b9982296c19f8eaea6dc225b5a4f Mon Sep 17 00:00:00 2001
From: Sven Strickroth <email@cs-ware.de>
Date: Tue, 27 Dec 2011 00:33:46 +0100
Subject: [PATCH 1/4] add central method for prompting a user using
GIT_ASKPASS or SSH_ASKPASS
Signed-off-by: Sven Strickroth <email@cs-ware.de>
---
perl/Git.pm | 31 ++++++++++++++++++++++++++++++-
1 files changed, 30 insertions(+), 1 deletions(-)
diff --git a/perl/Git.pm b/perl/Git.pm
index f7ce511..8176d47 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -58,7 +58,7 @@ require Exporter;
command_output_pipe command_input_pipe command_close_pipe
command_bidi_pipe command_close_bidi_pipe
version exec_path html_path hash_object git_cmd_try
- remote_refs
+ remote_refs prompt
temp_acquire temp_release temp_reset temp_path);
@@ -512,6 +512,35 @@ C<git --html-path>). Useful mostly only internally.
sub html_path { command_oneline('--html-path') }
+=item prompt ( PROMPT)
+
+Checks if GIT_ASKPASS or SSH_ASKPASS is set, and if yes
+use it and return answer from user.
+
+=cut
+
+sub prompt {
+ my ($self, $prompt) = _maybe_self(@_);
+ if (exists $ENV{'GIT_ASKPASS'}) {
+ return _prompt($ENV{'GIT_ASKPASS'}, $prompt);
+ } elsif (exists $ENV{'SSH_ASKPASS'}) {
+ return _prompt($ENV{'SSH_ASKPASS'}, $prompt);
+ } else {
+ return undef;
+ }
+}
+
+sub _prompt {
+ my ($self, $askpass, $prompt) = _maybe_self(@_);
+ my $ret;
+ open(PH, "-|", $askpass, $prompt);
+ $ret = <PH>;
+ $ret =~ s/[\012\015]//g; # strip \n\r
+ close(PH);
+ return $ret;
+}
+
+
=item repo_path ()
Return path to the git repository. Must be called on a repository instance.
--
1.7.7.1.msysgit.0
>From ef4c6557d1b0e33440d13c64742d44b2a22143f3 Mon Sep 17 00:00:00 2001
From: Sven Strickroth <email@cs-ware.de>
Date: Tue, 27 Dec 2011 00:34:09 +0100
Subject: [PATCH 2/4] switch to central prompt method
Signed-off-by: Sven Strickroth <email@cs-ware.de>
---
git-svn.perl | 9 ++-------
1 files changed, 2 insertions(+), 7 deletions(-)
diff --git a/git-svn.perl b/git-svn.perl
index eeb83d3..4fd4eca 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -4415,13 +4415,8 @@ sub username {
sub _read_password {
my ($prompt, $realm) = @_;
- my $password = '';
- if (exists $ENV{GIT_ASKPASS}) {
- open(PH, "-|", $ENV{GIT_ASKPASS}, $prompt);
- $password = <PH>;
- $password =~ s/[\012\015]//; # \n\r
- close(PH);
- } else {
+ my $password = Git->prompt($prompt);;
+ if (!defined $password) {
print STDERR $prompt;
STDERR->flush;
require Term::ReadKey;
--
1.7.7.1.msysgit.0
>From d58f41d7b9b8e690c9839f6f7539774da88aa3a4 Mon Sep 17 00:00:00 2001
From: Sven Strickroth <email@cs-ware.de>
Date: Tue, 27 Dec 2011 00:37:43 +0100
Subject: [PATCH 3/4] honour *_ASKPASS for querying username and for querying
further actions on unknown certificates
git-svn reads usernames (and answers for certificate errors) from an interactive terminal.
This behavior cause GUIs to hang waiting for git-svn to complete (http://code.google.com/p/tortoisegit/issues/detail?id=967).
Also see commit 56a853b62c0ae7ebaad0a7a0a704f5ef561eb795.
Signed-off-by: Sven Strickroth <email@cs-ware.de>
---
git-svn.perl | 13 ++++++++++---
1 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/git-svn.perl b/git-svn.perl
index 4fd4eca..b85a7de 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -4357,11 +4357,15 @@ sub ssl_server_trust {
issuer_dname fingerprint);
my $choice;
prompt:
- print STDERR $may_save ?
+ my $options = $may_save ?
"(R)eject, accept (t)emporarily or accept (p)ermanently? " :
"(R)eject or accept (t)emporarily? ";
- STDERR->flush;
- $choice = lc(substr(<STDIN> || 'R', 0, 1));
+ $choice = Git->prompt("Certificate unknown. " . $options);
+ if (!defined $choice) {
+ print STDERR $options;
+ STDERR->flush;
+ $choice = lc(substr(<STDIN> || 'R', 0, 1));
+ }
if ($choice =~ /^t$/i) {
$cred->may_save(undef);
} elsif ($choice =~ /^r$/i) {
@@ -4404,6 +4408,9 @@ sub username {
if (defined $_username) {
$username = $_username;
} else {
+ $username = Git->prompt("Username");
+ }
+ if (!defined $username) {
print STDERR "Username: ";
STDERR->flush;
chomp($username = <STDIN>);
--
1.7.7.1.msysgit.0
>From 2c1dbdae8024f28d17abfbdc7e45865a1277151a Mon Sep 17 00:00:00 2001
From: Sven Strickroth <email@cs-ware.de>
Date: Tue, 27 Dec 2011 00:42:07 +0100
Subject: [PATCH 4/4] ignore empty *_ASKPASS variables
Signed-off-by: Sven Strickroth <email@cs-ware.de>
---
perl/Git.pm | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/perl/Git.pm b/perl/Git.pm
index 8176d47..fade617 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -532,6 +532,9 @@ sub prompt {
sub _prompt {
my ($self, $askpass, $prompt) = _maybe_self(@_);
+ unless ($askpass) {
+ return undef;
+ }
my $ret;
open(PH, "-|", $askpass, $prompt);
$ret = <PH>;
--
1.7.7.1.msysgit.0
--
Best regards,
Sven Strickroth
ClamAV, a GPL anti-virus toolkit http://www.clamav.net
PGP key id F5A9D4C4 @ any key-server
^ permalink raw reply related
* [PATCH] write first for-merge ref to FETCH_HEAD first
From: Joey Hess @ 2011-12-26 16:16 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vd3bb929n.fsf@alter.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 6263 bytes --]
The FETCH_HEAD refname is supposed to refer to the ref that was fetched
and should be merged. However all fetched refs are written to
.git/FETCH_HEAD in an arbitrary order, and resolve_ref_unsafe simply
takes the first ref as the FETCH_HEAD, which is often the wrong one,
when other branches were also fetched.
The solution is to write the for-merge ref(s) to FETCH_HEAD first.
Then, unless --append is used, the FETCH_HEAD refname behaves as intended.
If the user uses --append, they presumably are doing so in order to
preserve the old FETCH_HEAD.
Also included a fix to documentation that assumes FETCH_HEAD contains
only a single ref.
---
Documentation/git-read-tree.txt | 2 +-
builtin/fetch.c | 158 +++++++++++++++++++++------------------
2 files changed, 85 insertions(+), 75 deletions(-)
diff --git a/Documentation/git-read-tree.txt b/Documentation/git-read-tree.txt
index 5375549..2d3ff23 100644
--- a/Documentation/git-read-tree.txt
+++ b/Documentation/git-read-tree.txt
@@ -342,7 +342,7 @@ since you pulled from him:
----------------
$ git fetch git://.... linus
-$ LT=`cat .git/FETCH_HEAD`
+$ LT=`git rev-parse FETCH_HEAD`
----------------
Your work tree is still based on your HEAD ($JC), but you have
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 33ad3aa..db565cd 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -377,6 +377,7 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
const char *what, *kind;
struct ref *rm;
char *url, *filename = dry_run ? "/dev/null" : git_path("FETCH_HEAD");
+ signed int want_merge;
fp = fopen(filename, "a");
if (!fp)
@@ -393,84 +394,93 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
goto abort;
}
- for (rm = ref_map; rm; rm = rm->next) {
- struct ref *ref = NULL;
-
- if (rm->peer_ref) {
- ref = xcalloc(1, sizeof(*ref) + strlen(rm->peer_ref->name) + 1);
- strcpy(ref->name, rm->peer_ref->name);
- hashcpy(ref->old_sha1, rm->peer_ref->old_sha1);
- hashcpy(ref->new_sha1, rm->old_sha1);
- ref->force = rm->peer_ref->force;
- }
+ /* Two passes are made over the ref_map, to write merge refs
+ * to FETCH_HEAD first. This allows using FETCH_HEAD as a refname
+ * to refer to the ref to be merged. */
+ for (want_merge = 1; want_merge >= 0 ; want_merge--) {
+ for (rm = ref_map; rm; rm = rm->next) {
+ struct ref *ref = NULL;
+
+ commit = lookup_commit_reference_gently(rm->old_sha1, 1);
+ if (!commit)
+ rm->merge = 0;
+
+ if (rm->merge != want_merge)
+ continue;
+
+ if (rm->peer_ref) {
+ ref = xcalloc(1, sizeof(*ref) + strlen(rm->peer_ref->name) + 1);
+ strcpy(ref->name, rm->peer_ref->name);
+ hashcpy(ref->old_sha1, rm->peer_ref->old_sha1);
+ hashcpy(ref->new_sha1, rm->old_sha1);
+ ref->force = rm->peer_ref->force;
+ }
- commit = lookup_commit_reference_gently(rm->old_sha1, 1);
- if (!commit)
- rm->merge = 0;
- if (!strcmp(rm->name, "HEAD")) {
- kind = "";
- what = "";
- }
- else if (!prefixcmp(rm->name, "refs/heads/")) {
- kind = "branch";
- what = rm->name + 11;
- }
- else if (!prefixcmp(rm->name, "refs/tags/")) {
- kind = "tag";
- what = rm->name + 10;
- }
- else if (!prefixcmp(rm->name, "refs/remotes/")) {
- kind = "remote-tracking branch";
- what = rm->name + 13;
- }
- else {
- kind = "";
- what = rm->name;
- }
+ if (!strcmp(rm->name, "HEAD")) {
+ kind = "";
+ what = "";
+ }
+ else if (!prefixcmp(rm->name, "refs/heads/")) {
+ kind = "branch";
+ what = rm->name + 11;
+ }
+ else if (!prefixcmp(rm->name, "refs/tags/")) {
+ kind = "tag";
+ what = rm->name + 10;
+ }
+ else if (!prefixcmp(rm->name, "refs/remotes/")) {
+ kind = "remote-tracking branch";
+ what = rm->name + 13;
+ }
+ else {
+ kind = "";
+ what = rm->name;
+ }
- url_len = strlen(url);
- for (i = url_len - 1; url[i] == '/' && 0 <= i; i--)
- ;
- url_len = i + 1;
- if (4 < i && !strncmp(".git", url + i - 3, 4))
- url_len = i - 3;
-
- strbuf_reset(¬e);
- if (*what) {
- if (*kind)
- strbuf_addf(¬e, "%s ", kind);
- strbuf_addf(¬e, "'%s' of ", what);
- }
- fprintf(fp, "%s\t%s\t%s",
- sha1_to_hex(rm->old_sha1),
- rm->merge ? "" : "not-for-merge",
- note.buf);
- for (i = 0; i < url_len; ++i)
- if ('\n' == url[i])
- fputs("\\n", fp);
- else
- fputc(url[i], fp);
- fputc('\n', fp);
-
- strbuf_reset(¬e);
- if (ref) {
- rc |= update_local_ref(ref, what, ¬e);
- free(ref);
- } else
- strbuf_addf(¬e, "* %-*s %-*s -> FETCH_HEAD",
- TRANSPORT_SUMMARY_WIDTH,
- *kind ? kind : "branch",
- REFCOL_WIDTH,
- *what ? what : "HEAD");
- if (note.len) {
- if (verbosity >= 0 && !shown_url) {
- fprintf(stderr, _("From %.*s\n"),
- url_len, url);
- shown_url = 1;
+ url_len = strlen(url);
+ for (i = url_len - 1; url[i] == '/' && 0 <= i; i--)
+ ;
+ url_len = i + 1;
+ if (4 < i && !strncmp(".git", url + i - 3, 4))
+ url_len = i - 3;
+
+ strbuf_reset(¬e);
+ if (*what) {
+ if (*kind)
+ strbuf_addf(¬e, "%s ", kind);
+ strbuf_addf(¬e, "'%s' of ", what);
+ }
+ fprintf(fp, "%s\t%s\t%s",
+ sha1_to_hex(rm->old_sha1),
+ rm->merge ? "" : "not-for-merge",
+ note.buf);
+ for (i = 0; i < url_len; ++i)
+ if ('\n' == url[i])
+ fputs("\\n", fp);
+ else
+ fputc(url[i], fp);
+ fputc('\n', fp);
+
+ strbuf_reset(¬e);
+ if (ref) {
+ rc |= update_local_ref(ref, what, ¬e);
+ free(ref);
+ } else
+ strbuf_addf(¬e, "* %-*s %-*s -> FETCH_HEAD",
+ TRANSPORT_SUMMARY_WIDTH,
+ *kind ? kind : "branch",
+ REFCOL_WIDTH,
+ *what ? what : "HEAD");
+ if (note.len) {
+ if (verbosity >= 0 && !shown_url) {
+ fprintf(stderr, _("From %.*s\n"),
+ url_len, url);
+ shown_url = 1;
+ }
+ if (verbosity >= 0)
+ fprintf(stderr, " %s\n", note.buf);
}
- if (verbosity >= 0)
- fprintf(stderr, " %s\n", note.buf);
}
}
--
1.7.7.3
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 828 bytes --]
^ permalink raw reply related
* Re: [PATCH] add post-fetch hook
From: Joey Hess @ 2011-12-26 15:51 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vlipz930t.fsf@alter.siamese.dyndns.org>
[-- Attachment #1.1: Type: text/plain, Size: 4024 bytes --]
Junio C Hamano wrote:
> Thanks. I do have a few comments.
>
> This hook is no longer a "post" fetch hook. The mechanism lets the object
> transfer phase does its work and then rewrites/tweaks the result before
> fetch completes. To an outside observer, what the hook does is an integral
> part of what "fetch" does, and not something that happens _after_ fetch
> completes. I am bad at naming things, but something along the lines of
> "tweak-fetch" that makes it clear that what happens in the hook is still
> part of the fetching may be a more appropriate name, methinks.
I'd be happy to call it tweak-fetch.
> I very much on purpose said that the hook "must read everything from its
> standard input, *and* *then* return..." in my response. Your "Demo" hook
> emits output as it reads its input with sed, but your main process invokes
> the hook, drains everything with write_in_full() before starting to read a
> single line, so I suspect that your hook will deadlock when its output
> pipe buffer fills up without being read by the main process. Of course,
> for this deadlock to actually happen, you need to be fetching quite a lot
> of refs.
Doesn't having the hook be fed by the async process/thread avoid this
problem? That's exactly why I did that, being familiar with the problem
from stupid perl scripts like this one:
#!/usr/bin/perl
use FileHandle;
use IPC::Open2;
$pid = open2(*Reader, *Writer, "cat");
print Writer "stuff $_\n" for 1..100000; # blocks forever
close Writer;
print "got: $_" while <Reader>;
I've tested feeding large quantities of data through my demo hook
(just feed it all the lines repeated 100 thousand times) and have
not seen it block. And other code in git uses an async feeder similarly,
see for example convert.c's apply_filter(). So I think this is ok..?
> We seem to already have too many hook drivers, each of which hand-roll
> similar logic using run-command API. At some point, we would want to have
> a single "run_hook" helper function that takes:
This was also my feeling as I looked around the code.
> By formalizing the hook driver API that way, any hook driver that drives a
> tricky hook that may need a select(2) loop to avoid a deadlock in a way
> similar to your patch do would not have to worry about the issue, as the
> run_hook() helper would take care of it by reading from the hook's output
> pipe and drain the pipe by calling the "consumer" callback before calling
> the "generator" callback and feed more input to the hook to cause a
> deadlock.
Actually, run_hook would need to either have a select loop, or an async
feeder like I've used. The method you describe is itself prone to deadlock!
Consider a hook that *does* consume all its input before outputting anything.
The first call to the "consumer" callback would block.
> - run a set of hooks on the same triggering condition. You may want to
> have two "post-receive" hooks, one to feed an e-mail based notification
> system and another to drive an autobuilder
Something I've always wanted, in fact..
> I have been wondering when would be the good time to refactor the hook
> driver API. We can add your patch, after polishing it enough to make it
> ready for inclusion, independent of the hook API refactoring. But that
> would mean that it would require more work when refactoring the hook API,
> as we would have one more hand-rolled hook caller that is based on
> run_command().
On the other hand, I have uses for this hook that are blocked on it
getting into git, so would rather not see it hung up in a general
refactoring. If it helps, I'd be happy to help with a hook refactoring
later.
The latest version of my patch (attached) already lifts reading from the
hook out into a helper function, so should need not many changes in such
a refactoring -- most of my run_tweak_fetch_hook would simply go away
then. I've also cleaned it up a lot and and pretty happy with it now.
--
see shy jo
[-- Attachment #1.2: 0001-preparations-for-tweak-fetch-hook.patch --]
[-- Type: text/x-diff, Size: 4094 bytes --]
From a06b5ef9908f56692a07fb37f5794c4122f10491 Mon Sep 17 00:00:00 2001
From: Joey Hess <joey@kitenet.net>
Date: Mon, 26 Dec 2011 10:44:55 -0400
Subject: [PATCH 1/2] preparations for tweak-fetch hook
No behavior changes yet, only some groundwork for the next
change.
The refs_result structure combines a status code with a ref map,
which can be NULL even on success. This will be needed when
there's a tweak-fetch hook, because it can filter out all refs,
while still succeeding.
fetch_refs returns a refs_result, so that it can modify the ref_map.
---
builtin/fetch.c | 54 +++++++++++++++++++++++++++++++++++-------------------
1 files changed, 35 insertions(+), 19 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 33ad3aa..70b9f89 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -29,6 +29,11 @@ enum {
TAGS_SET = 2
};
+struct refs_result {
+ struct ref *new_refs;
+ int status;
+};
+
static int all, append, dry_run, force, keep, multiple, prune, update_head_ok, verbosity;
static int progress, recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
static int tags = TAGS_DEFAULT;
@@ -89,6 +94,15 @@ static struct option builtin_fetch_options[] = {
OPT_END()
};
+static int add_existing(const char *refname, const unsigned char *sha1,
+ int flag, void *cbdata)
+{
+ struct string_list *list = (struct string_list *)cbdata;
+ struct string_list_item *item = string_list_insert(list, refname);
+ item->util = (void *)sha1;
+ return 0;
+}
+
static void unlock_pack(void)
{
if (transport)
@@ -507,17 +521,24 @@ static int quickfetch(struct ref *ref_map)
return check_everything_connected(iterate_ref_map, 1, &rm);
}
-static int fetch_refs(struct transport *transport, struct ref *ref_map)
+static struct refs_result fetch_refs(struct transport *transport,
+ struct ref *ref_map)
{
- int ret = quickfetch(ref_map);
- if (ret)
- ret = transport_fetch_refs(transport, ref_map);
- if (!ret)
- ret |= store_updated_refs(transport->url,
+ struct refs_result res;
+ res.status = quickfetch(ref_map);
+ if (res.status)
+ res.status = transport_fetch_refs(transport, ref_map);
+ if (!res.status) {
+ res.new_refs = ref_map
+ res.status |= store_updated_refs(transport->url,
transport->remote->name,
- ref_map);
+ res.new_refs);
+ }
+ else {
+ res.new_refs = ref_map;
+ }
transport_unlock_pack(transport);
- return ret;
+ return res;
}
static int prune_refs(struct refspec *refs, int ref_count, struct ref *ref_map)
@@ -542,15 +563,6 @@ static int prune_refs(struct refspec *refs, int ref_count, struct ref *ref_map)
return result;
}
-static int add_existing(const char *refname, const unsigned char *sha1,
- int flag, void *cbdata)
-{
- struct string_list *list = (struct string_list *)cbdata;
- struct string_list_item *item = string_list_insert(list, refname);
- item->util = (void *)sha1;
- return 0;
-}
-
static int will_fetch(struct ref **head, const unsigned char *sha1)
{
struct ref *rm = *head;
@@ -673,6 +685,7 @@ static int do_fetch(struct transport *transport,
struct string_list_item *peer_item = NULL;
struct ref *ref_map;
struct ref *rm;
+ struct refs_result res;
int autotags = (transport->remote->fetch_tags == 1);
for_each_ref(add_existing, &existing_refs);
@@ -710,7 +723,9 @@ static int do_fetch(struct transport *transport,
if (tags == TAGS_DEFAULT && autotags)
transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, "1");
- if (fetch_refs(transport, ref_map)) {
+ res = fetch_refs(transport, ref_map);
+ ref_map = res.new_refs;
+ if (res.status) {
free_refs(ref_map);
return 1;
}
@@ -750,7 +765,8 @@ static int do_fetch(struct transport *transport,
if (ref_map) {
transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, NULL);
transport_set_option(transport, TRANS_OPT_DEPTH, "0");
- fetch_refs(transport, ref_map);
+ res = fetch_refs(transport, ref_map);
+ ref_map = res.new_refs;
}
free_refs(ref_map);
}
--
1.7.7.3
[-- Attachment #1.3: 0002-add-tweak-fetch-hook.patch --]
[-- Type: text/x-diff, Size: 7599 bytes --]
From 073b0921bb5988628e7af423924c410f522f403a Mon Sep 17 00:00:00 2001
From: Joey Hess <joey@kitenet.net>
Date: Mon, 26 Dec 2011 10:53:27 -0400
Subject: [PATCH 2/2] add tweak-fetch hook
The tweak-fetch hook is fed lines on stdin for all refs that were fetched,
and outputs on stdout possibly modified lines. Its output is parsed and
used when git fetch updates the remote tracking refs, records the entries
in FETCH_HEAD, and produces its report.
---
Documentation/githooks.txt | 29 +++++++
builtin/fetch.c | 191 +++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 219 insertions(+), 1 deletions(-)
diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index 28edefa..be2624c 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -162,6 +162,35 @@ This hook can be used to perform repository validity checks, auto-display
differences from the previous HEAD if different, or set working dir metadata
properties.
+tweak-fetch
+~~~~~~~~~~
+
+This hook is invoked by 'git fetch' (commonly called by 'git pull'), after
+refs have been fetched from the remote repository. It is not executed, if
+nothing was fetched.
+
+The output of the hook is used to update the remote-tracking branches, and
+`.git/FETCH_HEAD`, in preparation for for a later merge operation done by
+'git merge'.
+
+It takes no arguments, but is fed a line of the following format on
+its standard input for each ref that was fetched.
+
+ <sha1> SP not-for-merge|merge SP <remote-refname> SP <local-refname> LF
+
+Where the "not-for-merge" flag indicates the ref is not to be merged into the
+current branch, and the "merge" flag indicates that 'git merge' should
+later merge it. The `<remote-refname>` is the remote's name for the ref
+that was pulled, and `<local-refname>` is a name of a remote-tracking branch,
+like "refs/remotes/origin/master", or can be empty if the fetched ref is not
+being stored in a local refname.
+
+The hook must consume all of its standard input, and output back lines
+of the same format. It can modify its input as desired, including
+adding or removing lines, updating the sha1 (i.e. re-point the
+remote-tracking branch), changing the merge flag, and changing the
+`<local-refname>` (i.e. use different remote-tracking branch).
+
post-merge
~~~~~~~~~~
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 70b9f89..5434b6f 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -103,6 +103,194 @@ static int add_existing(const char *refname, const unsigned char *sha1,
return 0;
}
+static const char tweak_fetch_hook[] = "tweak-fetch";
+
+int feed_tweak_fetch_hook (int in, int out, void *data)
+{
+ struct ref *ref;
+ struct strbuf buf = STRBUF_INIT;
+ int ret;
+
+ for (ref = data; ref; ref = ref->next) {
+ strbuf_addstr(&buf, sha1_to_hex(ref->old_sha1));
+ strbuf_addch(&buf, ' ');
+ strbuf_addstr(&buf, ref->merge ? "merge" : "not-for-merge");
+ strbuf_addch(&buf, ' ');
+ if (ref->name)
+ strbuf_addstr(&buf, ref->name);
+ strbuf_addch(&buf, ' ');
+ if (ref->peer_ref && ref->peer_ref->name)
+ strbuf_addstr(&buf, ref->peer_ref->name);
+ strbuf_addch(&buf, '\n');
+ }
+
+ ret = write_in_full(out, buf.buf, buf.len) != buf.len;
+ if (ret)
+ warning("%s hook failed to consume all its input",
+ tweak_fetch_hook);
+ close(out);
+ strbuf_release(&buf);
+ return ret;
+}
+
+struct ref *parse_tweak_fetch_hook_line (char *l,
+ struct string_list *existing_refs)
+{
+ struct ref *ref = NULL, *peer_ref = NULL;
+ struct string_list_item *peer_item = NULL;
+ char *words[4];
+ int i, word=0;
+ char *problem;
+
+ for (i=0; l[i]; i++) {
+ if (isspace(l[i])) {
+ l[i]='\0';
+ words[word]=l;
+ l+=i+1;
+ i=0;
+ word++;
+ if (word > 3) {
+ problem="too many words";
+ goto unparsable;
+ }
+ }
+ }
+ if (word < 3) {
+ problem="not enough words";
+ goto unparsable;
+ }
+
+ ref = alloc_ref(words[2]);
+ peer_ref = ref->peer_ref = alloc_ref(l);
+ ref->peer_ref->force=1;
+
+ if (get_sha1_hex(words[0], ref->old_sha1)) {
+ problem="bad sha1";
+ goto unparsable;
+ }
+
+ if (strcmp(words[1], "merge") == 0) {
+ ref->merge=1;
+ }
+ else if (strcmp(words[1], "not-for-merge") != 0) {
+ problem="bad merge flag";
+ goto unparsable;
+ }
+
+ peer_item = string_list_lookup(existing_refs, peer_ref->name);
+ if (peer_item)
+ hashcpy(peer_ref->old_sha1, peer_item->util);
+
+ return ref;
+
+ unparsable:
+ warning("%s hook output a wrongly formed line: %s",
+ tweak_fetch_hook, problem);
+ free(ref);
+ free(peer_ref);
+ return NULL;
+}
+
+struct refs_result read_tweak_fetch_hook (int in) {
+ struct refs_result res;
+ FILE *f;
+ struct strbuf buf;
+ struct string_list existing_refs = STRING_LIST_INIT_NODUP;
+ struct ref *ref, *prevref=NULL;
+
+ res.status = 0;
+ res.new_refs = NULL;
+
+ f = fdopen(in, "r");
+ if (f == NULL) {
+ res.status = 1;
+ return res;
+ }
+
+ strbuf_init(&buf, 128);
+ for_each_ref(add_existing, &existing_refs);
+
+ while (strbuf_getline(&buf, f, '\n') != EOF) {
+ char *l = strbuf_detach(&buf, NULL);
+ ref = parse_tweak_fetch_hook_line(l, &existing_refs);
+ if (ref) {
+ if (prevref) {
+ prevref->next=ref;
+ prevref=ref;
+ }
+ else {
+ res.new_refs = prevref = ref;
+ }
+ }
+ else {
+ res.status = 1;
+ }
+ free(l);
+ }
+
+ string_list_clear(&existing_refs, 0);
+ strbuf_release(&buf);
+ fclose(f);
+ return res;
+}
+
+/* The hook is fed lines of the form:
+ * <sha1> SP <not-for-merge|merge> SP <remote-refname> SP <local-refname> LF
+ * And should output rewritten lines of the same form.
+ */
+struct ref *run_tweak_fetch_hook (struct ref *fetched_refs)
+{
+ struct child_process hook;
+ const char *argv[2];
+ struct async async;
+ struct refs_result res;
+
+ if (! fetched_refs)
+ return fetched_refs;
+
+ argv[0] = git_path("hooks/%s", tweak_fetch_hook);
+ if (access(argv[0], X_OK) < 0)
+ return fetched_refs;
+ argv[1] = NULL;
+
+ memset(&hook, 0, sizeof(hook));
+ hook.argv = argv;
+ hook.in = -1;
+ hook.out = -1;
+ if (start_command(&hook))
+ return fetched_refs;
+
+ /* Use an async writer to feed the hook process.
+ * This allows the hook to read and write a line at
+ * a time without blocking. */
+ memset(&async, 0, sizeof(async));
+ async.proc = feed_tweak_fetch_hook;
+ async.data = fetched_refs;
+ async.out = hook.in;
+ if (start_async(&async)) {
+ close(hook.in);
+ close(hook.out);
+ finish_command(&hook);
+ return fetched_refs;
+ }
+ res = read_tweak_fetch_hook(hook.out);
+ res.status |= finish_async(&async);
+ res.status |= finish_command(&hook);
+
+ if (res.status) {
+ warning("%s hook failed, ignoring its output", tweak_fetch_hook);
+ free(res.new_refs);
+ return fetched_refs;
+ }
+ else {
+ /* The new_refs are returned, to be used in place of
+ * fetched_refs, so it is not needed anymore and can
+ * be freed here. */
+ free_refs(fetched_refs);
+ return res.new_refs;
+ }
+}
+
static void unlock_pack(void)
{
if (transport)
@@ -529,7 +717,8 @@ static struct refs_result fetch_refs(struct transport *transport,
if (res.status)
res.status = transport_fetch_refs(transport, ref_map);
if (!res.status) {
- res.new_refs = ref_map
+ res.new_refs = run_tweak_fetch_hook(ref_map);
+
res.status |= store_updated_refs(transport->url,
transport->remote->name,
res.new_refs);
--
1.7.7.3
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 828 bytes --]
^ permalink raw reply related
* [PATCH 3/3] index-pack: eliminate unlimited recursion in get_delta_base()
From: Nguyễn Thái Ngọc Duy @ 2011-12-26 12:04 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Shawn O. Pearce,
Nguyễn Thái Ngọc Duy
In-Reply-To: <1324901080-23215-1-git-send-email-pclouds@gmail.com>
Revert the order of delta applying so that by the time a delta is
applied, its base is either non-delta or already inflated.
get_delta_base() is still recursive, but because base's data is always
ready, the inner get_delta_base() call never has any chance to call
itself again.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
builtin/index-pack.c | 30 +++++++++++++++++++++---------
1 files changed, 21 insertions(+), 9 deletions(-)
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index d311c05..e8801bb 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -519,10 +519,25 @@ static void *get_base_data(struct base_data *c)
{
if (!c->data) {
struct object_entry *obj = c->obj;
+ struct base_data **delta = NULL;
+ int delta_nr = 0, delta_alloc = 0;
- if (is_delta_type(obj->type)) {
- void *base = get_base_data(c->base);
- void *raw = get_data_from_pack(obj);
+ for (; is_delta_type(c->obj->type); c = c->base) {
+ ALLOC_GROW(delta, delta_nr + 1, delta_alloc);
+ delta[delta_nr++] = c;
+ }
+ if (!delta_nr) {
+ c->data = get_data_from_pack(obj);
+ c->size = obj->size;
+ base_cache_used += c->size;
+ prune_base_data(c);
+ }
+ for (; delta_nr > 0; delta_nr--) {
+ void *base, *raw;
+ c = delta[delta_nr - 1];
+ obj = c->obj;
+ base = get_base_data(c->base);
+ raw = get_data_from_pack(obj);
c->data = patch_delta(
base, c->base->size,
raw, obj->size,
@@ -530,13 +545,10 @@ static void *get_base_data(struct base_data *c)
free(raw);
if (!c->data)
bad_object(obj->idx.offset, "failed to apply delta");
- } else {
- c->data = get_data_from_pack(obj);
- c->size = obj->size;
+ base_cache_used += c->size;
+ prune_base_data(c);
}
-
- base_cache_used += c->size;
- prune_base_data(c);
+ free(delta);
}
return c->data;
}
--
1.7.8.36.g69ee2
^ permalink raw reply related
* [PATCH 2/3] index-pack: eliminate recursion in find_unresolved_deltas
From: Nguyễn Thái Ngọc Duy @ 2011-12-26 12:04 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Shawn O. Pearce,
Nguyễn Thái Ngọc Duy
In-Reply-To: <1324901080-23215-1-git-send-email-pclouds@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
builtin/index-pack.c | 112 +++++++++++++++++++++++++++++++------------------
1 files changed, 71 insertions(+), 41 deletions(-)
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 98025da..d311c05 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -34,6 +34,8 @@ struct base_data {
struct object_entry *obj;
void *data;
unsigned long size;
+ int ref_first, ref_last;
+ int ofs_first, ofs_last;
};
/*
@@ -221,6 +223,15 @@ static NORETURN void bad_object(unsigned long offset, const char *format, ...)
die("pack has bad object at offset %lu: %s", offset, buf);
}
+static struct base_data *alloc_base_data()
+{
+ struct base_data *base = xmalloc(sizeof(struct base_data));
+ memset(base, 0, sizeof(*base));
+ base->ref_last = -1;
+ base->ofs_last = -1;
+ return base;
+}
+
static void free_base_data(struct base_data *c)
{
if (c->data) {
@@ -553,58 +564,77 @@ static void resolve_delta(struct object_entry *delta_obj,
nr_resolved_deltas++;
}
-static void find_unresolved_deltas(struct base_data *base,
- struct base_data *prev_base)
+static struct base_data *find_unresolved_deltas_1(struct base_data *base,
+ struct base_data *prev_base)
{
- int i, ref_first, ref_last, ofs_first, ofs_last;
-
- /*
- * This is a recursive function. Those brackets should help reducing
- * stack usage by limiting the scope of the delta_base union.
- */
- {
+ if (base->ref_last == -1 && base->ofs_last == -1) {
union delta_base base_spec;
hashcpy(base_spec.sha1, base->obj->idx.sha1);
find_delta_children(&base_spec,
- &ref_first, &ref_last, OBJ_REF_DELTA);
+ &base->ref_first, &base->ref_last, OBJ_REF_DELTA);
memset(&base_spec, 0, sizeof(base_spec));
base_spec.offset = base->obj->idx.offset;
find_delta_children(&base_spec,
- &ofs_first, &ofs_last, OBJ_OFS_DELTA);
- }
+ &base->ofs_first, &base->ofs_last, OBJ_OFS_DELTA);
- if (ref_last == -1 && ofs_last == -1) {
- free(base->data);
- return;
- }
+ if (base->ref_last == -1 && base->ofs_last == -1) {
+ free(base->data);
+ return NULL;
+ }
- link_base_data(prev_base, base);
+ link_base_data(prev_base, base);
+ }
- for (i = ref_first; i <= ref_last; i++) {
- struct object_entry *child = objects + deltas[i].obj_no;
- struct base_data result;
+ if (base->ref_first <= base->ref_last) {
+ struct object_entry *child = objects + deltas[base->ref_first].obj_no;
+ struct base_data *result = alloc_base_data();
assert(child->real_type == OBJ_REF_DELTA);
- resolve_delta(child, base, &result);
- if (i == ref_last && ofs_last == -1)
+ resolve_delta(child, base, result);
+ if (base->ref_first == base->ref_last && base->ofs_last == -1)
free_base_data(base);
- find_unresolved_deltas(&result, base);
+
+ base->ref_first++;
+ return result;
}
- for (i = ofs_first; i <= ofs_last; i++) {
- struct object_entry *child = objects + deltas[i].obj_no;
- struct base_data result;
+ if (base->ofs_first <= base->ofs_last) {
+ struct object_entry *child = objects + deltas[base->ofs_first].obj_no;
+ struct base_data *result = alloc_base_data();
assert(child->real_type == OBJ_OFS_DELTA);
- resolve_delta(child, base, &result);
- if (i == ofs_last)
+ resolve_delta(child, base, result);
+ if (base->ofs_first == base->ofs_last)
free_base_data(base);
- find_unresolved_deltas(&result, base);
+
+ base->ofs_first++;
+ return result;
}
unlink_base_data(base);
+ return NULL;
+}
+
+static void find_unresolved_deltas(struct base_data *base)
+{
+ struct base_data *new_base, *prev_base = NULL;
+ for (;;) {
+ new_base = find_unresolved_deltas_1(base, prev_base);
+
+ if (new_base) {
+ prev_base = base;
+ base = new_base;
+ }
+ else {
+ free(base);
+ base = prev_base;
+ if (!base)
+ return;
+ prev_base = base->base;
+ }
+ }
}
static int compare_delta_entry(const void *a, const void *b)
@@ -684,13 +714,13 @@ static void parse_pack_objects(unsigned char *sha1)
progress = start_progress("Resolving deltas", nr_deltas);
for (i = 0; i < nr_objects; i++) {
struct object_entry *obj = &objects[i];
- struct base_data base_obj;
+ struct base_data *base_obj = alloc_base_data();
if (is_delta_type(obj->type))
continue;
- base_obj.obj = obj;
- base_obj.data = NULL;
- find_unresolved_deltas(&base_obj, NULL);
+ base_obj->obj = obj;
+ base_obj->data = NULL;
+ find_unresolved_deltas(base_obj);
display_progress(progress, nr_resolved_deltas);
}
}
@@ -783,20 +813,20 @@ static void fix_unresolved_deltas(struct sha1file *f, int nr_unresolved)
for (i = 0; i < n; i++) {
struct delta_entry *d = sorted_by_pos[i];
enum object_type type;
- struct base_data base_obj;
+ struct base_data *base_obj = alloc_base_data();
if (objects[d->obj_no].real_type != OBJ_REF_DELTA)
continue;
- base_obj.data = read_sha1_file(d->base.sha1, &type, &base_obj.size);
- if (!base_obj.data)
+ base_obj->data = read_sha1_file(d->base.sha1, &type, &base_obj->size);
+ if (!base_obj->data)
continue;
- if (check_sha1_signature(d->base.sha1, base_obj.data,
- base_obj.size, typename(type)))
+ if (check_sha1_signature(d->base.sha1, base_obj->data,
+ base_obj->size, typename(type)))
die("local object %s is corrupt", sha1_to_hex(d->base.sha1));
- base_obj.obj = append_obj_to_pack(f, d->base.sha1,
- base_obj.data, base_obj.size, type);
- find_unresolved_deltas(&base_obj, NULL);
+ base_obj->obj = append_obj_to_pack(f, d->base.sha1,
+ base_obj->data, base_obj->size, type);
+ find_unresolved_deltas(base_obj);
display_progress(progress, nr_resolved_deltas);
}
free(sorted_by_pos);
--
1.7.8.36.g69ee2
^ permalink raw reply related
* [PATCH 1/3] Eliminate recursion in setting/clearing marks in commit list
From: Nguyễn Thái Ngọc Duy @ 2011-12-26 12:04 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Shawn O. Pearce, Nguyen Thai Ngoc Duy
From: Nguyen Thai Ngoc Duy <pclouds@gmail.com>
Recursion in a DAG is generally a bad idea because it could be very
deep. Be defensive and avoid recursion in mark_parents_uninteresting()
and clear_commit_marks().
mark_parents_uninteresting() learns a trick from clear_commit_marks()
to avoid malloc() in (dorminant) single-parent case.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
commit.c | 13 +++++++++++--
revision.c | 45 +++++++++++++++++++++++++++++----------------
2 files changed, 40 insertions(+), 18 deletions(-)
diff --git a/commit.c b/commit.c
index 73b7e00..628066c 100644
--- a/commit.c
+++ b/commit.c
@@ -421,7 +421,8 @@ struct commit *pop_most_recent_commit(struct commit_list **list,
return ret;
}
-void clear_commit_marks(struct commit *commit, unsigned int mark)
+static void clear_commit_marks_1(struct commit_list **plist,
+ struct commit *commit, unsigned int mark)
{
while (commit) {
struct commit_list *parents;
@@ -436,12 +437,20 @@ void clear_commit_marks(struct commit *commit, unsigned int mark)
return;
while ((parents = parents->next))
- clear_commit_marks(parents->item, mark);
+ commit_list_insert(parents->item, plist);
commit = commit->parents->item;
}
}
+void clear_commit_marks(struct commit *commit, unsigned int mark)
+{
+ struct commit_list *list = NULL;
+ commit_list_insert(commit, &list);
+ while (list)
+ clear_commit_marks_1(&list, pop_commit(&list), mark);
+}
+
void clear_commit_marks_for_object_array(struct object_array *a, unsigned mark)
{
struct object *object;
diff --git a/revision.c b/revision.c
index 8764dde..7cc72fc 100644
--- a/revision.c
+++ b/revision.c
@@ -139,11 +139,32 @@ void mark_tree_uninteresting(struct tree *tree)
void mark_parents_uninteresting(struct commit *commit)
{
- struct commit_list *parents = commit->parents;
+ struct commit_list *parents = NULL, *l;
+
+ for (l = commit->parents; l; l = l->next)
+ commit_list_insert(l->item, &parents);
while (parents) {
struct commit *commit = parents->item;
- if (!(commit->object.flags & UNINTERESTING)) {
+ l = parents;
+ parents = parents->next;
+ free(l);
+
+ while (commit) {
+ /*
+ * A missing commit is ok iff its parent is marked
+ * uninteresting.
+ *
+ * We just mark such a thing parsed, so that when
+ * it is popped next time around, we won't be trying
+ * to parse it and get an error.
+ */
+ if (!has_sha1_file(commit->object.sha1))
+ commit->object.parsed = 1;
+
+ if (commit->object.flags & UNINTERESTING)
+ break;
+
commit->object.flags |= UNINTERESTING;
/*
@@ -154,21 +175,13 @@ void mark_parents_uninteresting(struct commit *commit)
* wasn't uninteresting), in which case we need
* to mark its parents recursively too..
*/
- if (commit->parents)
- mark_parents_uninteresting(commit);
- }
+ if (!commit->parents)
+ break;
- /*
- * A missing commit is ok iff its parent is marked
- * uninteresting.
- *
- * We just mark such a thing parsed, so that when
- * it is popped next time around, we won't be trying
- * to parse it and get an error.
- */
- if (!has_sha1_file(commit->object.sha1))
- commit->object.parsed = 1;
- parents = parents->next;
+ for (l = commit->parents->next; l; l = l->next)
+ commit_list_insert(l->item, &parents);
+ commit = commit->parents->item;
+ }
}
}
--
1.7.8.36.g69ee2
^ permalink raw reply related
* Re: FETCH_HEAD documentation vs reality
From: Junio C Hamano @ 2011-12-26 8:16 UTC (permalink / raw)
To: Joey Hess; +Cc: git
In-Reply-To: <20111225173901.GA668@gnu.kitenet.net>
Joey Hess <joey@kitenet.net> writes:
> Or yet another way to fix it would be to make git fetch always write the
> intended FETCH_HEAD first into .git/FETCH_HEAD. (When not in --append mode.)
> This seems like perhaps the best fix,...
Sounds good, if you mean "the first one that is marked as for-merge" by
"intended FETCH_HEAD".
Thanks. Please make it so.
^ permalink raw reply
* Re: [PATCH] add post-fetch hook
From: Junio C Hamano @ 2011-12-26 8:09 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Joey Hess, git
In-Reply-To: <m3liq0fwkz.fsf@localhost.localdomain>
Jakub Narebski <jnareb@gmail.com> writes:
> One thing that needs to be specified is what should happen if the hook
> changes "the refname at the remote" part...
Hmm, good eyes.
The mechanism is to allow the hook to rewrite what happened during the
fetch. If we decided refs/heads/master on the remote that points at the
commit $X should update refs/remotes/origin/master, we tell that to the
hook, and the hook reads it. The hook may tell us to pretend that we
fetched refs/heads/next on the remote that points at the commit $Y should
update refs/remotes/origin/pu. In FETCH_HEAD we leave where the commit was
fetched from and hook will affect that information (which is used in the
resulting merge commit log message), but otherwise I do not think anything
unexpected would happen, as tracking refs do not record where the stuff
came from (perhaps they are in reflog? I didn't check).
^ permalink raw reply
* Re: [PATCH] add post-fetch hook
From: Junio C Hamano @ 2011-12-26 7:59 UTC (permalink / raw)
To: Joey Hess; +Cc: git
In-Reply-To: <20111226023154.GA3243@gnu.kitenet.net>
Joey Hess <joey@kitenet.net> writes:
> The post-fetch hook is fed lines on stdin for all refs that were fetched, and
> outputs on stdout possibly modified lines. Its output is parsed and used
> when git fetch updates the remote tracking refs, records the entries in
> FETCH_HEAD, and produces its report.
>
> ---
>
> Not quite ready to sign off on this yet, but it does work.
> Comments and code review appreciated.
Thanks. I do have a few comments.
This hook is no longer a "post" fetch hook. The mechanism lets the object
transfer phase does its work and then rewrites/tweaks the result before
fetch completes. To an outside observer, what the hook does is an integral
part of what "fetch" does, and not something that happens _after_ fetch
completes. I am bad at naming things, but something along the lines of
"tweak-fetch" that makes it clear that what happens in the hook is still
part of the fetching may be a more appropriate name, methinks.
I very much on purpose said that the hook "must read everything from its
standard input, *and* *then* return..." in my response. Your "Demo" hook
emits output as it reads its input with sed, but your main process invokes
the hook, drains everything with write_in_full() before starting to read a
single line, so I suspect that your hook will deadlock when its output
pipe buffer fills up without being read by the main process. Of course,
for this deadlock to actually happen, you need to be fetching quite a lot
of refs.
To make the life of hook writers easier, however, it would be good to
support a hook written in the style of your "Demo" hook, instead of
requiring it to read everything before emiting any output. I think you
could solve this by having a select(2) loop to avoid the deadlock on our
end (lets call the code that spawns a hook with run_command() and
interacts with it a "hook driver" in the rest of this message), but before
going in that direction, I would like to see us stepping back and bit and
think about the way hooks are called in the current code.
We seem to already have too many hook drivers, each of which hand-roll
similar logic using run-command API. At some point, we would want to have
a single "run_hook" helper function that takes:
- the name of a hook;
- the command line arguments to be fed;
- a callback "generator" function to feed the standard input stream of
the hook process;
- a callback "consumer" function to receive the standard output stream of
the hook process; and
- set of environment tweaks while running the hook (e.g. run the hook
while setting GIT_INDEX_FILE to a temporary file).
and hides away the complexity from hook drivers. The command line
arguments, input and output callback functions are all optional depending
on the API between the hook driver and the hook (e.g. the "post-update"
hook takes arguments from its command line but does not interact with the
standard I/O stream, while the "post-receive" hook takes its input from
the standard input stream). Tweaking of the environment is also optional;
not many hooks need it.
By formalizing the hook driver API that way, any hook driver that drives a
tricky hook that may need a select(2) loop to avoid a deadlock in a way
similar to your patch do would not have to worry about the issue, as the
run_hook() helper would take care of it by reading from the hook's output
pipe and drain the pipe by calling the "consumer" callback before calling
the "generator" callback and feed more input to the hook to cause a
deadlock. We also could in the future do many other things if and when we
wanted to that the current code structure makes difficult. A few examples
that readily come to my mind are:
- relocate where the hook scripts live, by limiting the hook driver API
to take just the "name" of the hook. The current hook callsites know
that the hooks live in git_path("hooks/$name") and call run_command()
on their own, but the run_hook() helper could redirect it away to
somewhere else, e.g. "/etc/git-core/hooks/$name".
- run a set of hooks on the same triggering condition. You may want to
have two "post-receive" hooks, one to feed an e-mail based notification
system and another to drive an autobuilder, for example. For this to
work, the "generator" and "consumer" callbacks need to have a way for
us to tell "beginning of a new session" and "end of a new session", so
that they can produce/consume the same set of values more than once.
- perhaps ignore SIGPIPE if the hook chooses not to read any information
the hook driver provides with it.
I have been wondering when would be the good time to refactor the hook
driver API. We can add your patch, after polishing it enough to make it
ready for inclusion, independent of the hook API refactoring. But that
would mean that it would require more work when refactoring the hook API,
as we would have one more hand-rolled hook caller that is based on
run_command().
^ permalink raw reply
* [PATCH] add post-fetch hook
From: Joey Hess @ 2011-12-26 2:31 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vsjk99exw.fsf@alter.siamese.dyndns.org>
The post-fetch hook is fed lines on stdin for all refs that were fetched, and
outputs on stdout possibly modified lines. Its output is parsed and used
when git fetch updates the remote tracking refs, records the entries in
FETCH_HEAD, and produces its report.
---
Not quite ready to sign off on this yet, but it does work.
Comments and code review appreciated.
Demo:
joey@gnu:~/tmp/demo>cat .git/hooks/post-fetch
#!/bin/sh
# Rename branches, and block all tags.
sed 's!foo!bar!g' | grep -v refs/tags/
joey@gnu:~/tmp/demo>chmod +x .git/hooks/post-fetch
joey@gnu:~/tmp/demo>git pull
From /home/joey/tmp/a
* [new branch] bar -> origin/bar
Already up-to-date.
joey@gnu:~/tmp/demo>chmod -x .git/hooks/post-fetch
joey@gnu:~/tmp/demo>git pull
From /home/joey/tmp/a
* [new branch] foo -> origin/foo
* [new tag] v1.0 -> v1.0
Already up-to-date.
joey@gnu:~/tmp/demo>chmod +x .git/hooks/post-fetch
joey@gnu:~/tmp/demo>git remote update --prune
Fetching origin
x [deleted] (none) -> origin/foo
Documentation/githooks.txt | 29 ++++++
builtin/fetch.c | 228 ++++++++++++++++++++++++++++++++++++++++----
2 files changed, 238 insertions(+), 19 deletions(-)
diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index 28edefa..9c2b6bf 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -162,6 +162,35 @@ This hook can be used to perform repository validity checks, auto-display
differences from the previous HEAD if different, or set working dir metadata
properties.
+post-fetch
+~~~~~~~~~~
+
+This hook is invoked by 'git fetch' (commonly called by 'git pull'), after
+refs have been fetched from the remote repository. It is not executed, if
+nothing was fetched.
+
+It takes no arguments, but is fed a line of the following format on
+its standard input for each ref that was fetched.
+
+ <sha1> SP not-for-merge|merge SP <remote-refname> SP <local-refname> LF
+
+Where the "not-for-merge" flag indicates the ref is not to be merged into the
+current branch, and the "merge" flag indicates that 'git merge' should
+later merge it. The `<remote-refname>` is the remote's name for the ref
+that was pulled, and `<local-refname>` is a name of a remote-tracking branch,
+like "refs/remotes/origin/master", or can be empty if the fetched ref is not
+being stored in a local refname.
+
+The hook must consume all of its standard input, and output back lines
+of the same format. It can modify its input as desired, including
+adding or removing lines, updating the sha1 (i.e. re-point the
+remote-tracking branch), changing the merge flag, and changing the
+`<local-refname>` (i.e. use different remote-tracking branch).
+
+The output of the hook is used to update the remote-tracking branches, and
+`.git/FETCH_HEAD`, in preparation for for a later merge operation done by
+'git merge'.
+
post-merge
~~~~~~~~~~
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 33ad3aa..aa401b2 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -89,6 +89,188 @@ static struct option builtin_fetch_options[] = {
OPT_END()
};
+static int add_existing(const char *refname, const unsigned char *sha1,
+ int flag, void *cbdata)
+{
+ struct string_list *list = (struct string_list *)cbdata;
+ struct string_list_item *item = string_list_insert(list, refname);
+ item->util = (void *)sha1;
+ return 0;
+}
+
+static const char post_fetch_hook[] = "post-fetch";
+struct ref *post_fetch_hook_refs;
+
+int feed_post_fetch_hook (int in, int out, void *data)
+{
+ struct ref *ref;
+ struct strbuf buf = STRBUF_INIT;
+ int ret;
+
+ for (ref = post_fetch_hook_refs; ref; ref = ref->next) {
+ strbuf_addstr(&buf, sha1_to_hex(ref->old_sha1));
+ strbuf_addch(&buf, ' ');
+ strbuf_addstr(&buf, ref->merge ? "merge" : "not-for-merge");
+ strbuf_addch(&buf, ' ');
+ if (ref->name)
+ strbuf_addstr(&buf, ref->name);
+ strbuf_addch(&buf, ' ');
+ if (ref->peer_ref && ref->peer_ref->name)
+ strbuf_addstr(&buf, ref->peer_ref->name);
+ strbuf_addch(&buf, '\n');
+ }
+
+ ret = write_in_full(out, buf.buf, buf.len) != buf.len;
+ if (ret)
+ warning("%s hook failed to consume all its input",
+ post_fetch_hook);
+ close(out);
+ strbuf_release(&buf);
+ return ret;
+}
+
+struct ref *parse_post_fetch_hook_line (char *l,
+ struct string_list *existing_refs)
+{
+ struct ref *ref = NULL, *peer_ref = NULL;
+ struct string_list_item *peer_item = NULL;
+ char *words[4];
+ int i, word=0;
+ char *problem;
+
+ for (i=0; l[i]; i++) {
+ if (isspace(l[i])) {
+ l[i]='\0';
+ words[word]=l;
+ l+=i+1;
+ i=0;
+ word++;
+ if (word > 3) {
+ problem="too many words";
+ goto unparsable;
+ }
+ }
+ }
+ if (word < 3) {
+ problem="not enough words";
+ goto unparsable;
+ }
+
+ ref = alloc_ref(words[2]);
+ peer_ref = ref->peer_ref = alloc_ref(l);
+ ref->peer_ref->force=1;
+
+ if (get_sha1_hex(words[0], ref->old_sha1)) {
+ problem="bad sha1";
+ goto unparsable;
+ }
+
+ if (strcmp(words[1], "merge") == 0) {
+ ref->merge=1;
+ }
+ else if (strcmp(words[1], "not-for-merge") != 0) {
+ problem="bad merge flag";
+ goto unparsable;
+ }
+
+ peer_item = string_list_lookup(existing_refs, peer_ref->name);
+ if (peer_item)
+ hashcpy(peer_ref->old_sha1, peer_item->util);
+
+ return ref;
+
+ unparsable:
+ warning("%s hook output a wrongly formed line: %s",
+ post_fetch_hook, problem);
+ free(ref);
+ free(peer_ref);
+ return NULL;
+}
+
+/* The hook is fed lines of the form:
+ * <sha1> SP <not-for-merge|merge> SP <remote-refname> SP <local-refname> LF
+ * And should output rewritten lines of the same form.
+ */
+struct ref *run_post_fetch_hook (struct ref *fetched_refs)
+{
+ struct ref *new_refs = NULL;
+ struct string_list existing_refs = STRING_LIST_INIT_NODUP;
+ struct child_process hook;
+ struct async async;
+ const char *argv[2];
+ FILE *f;
+ struct strbuf buf;
+ struct ref *ref, *prevref=NULL;
+ int ok = 1;
+
+ if (! fetched_refs)
+ return fetched_refs;
+
+ argv[0] = git_path("hooks/%s", post_fetch_hook);
+ if (access(argv[0], X_OK) < 0)
+ return fetched_refs;
+ argv[1] = NULL;
+
+ memset(&hook, 0, sizeof(hook));
+ hook.argv = argv;
+ hook.in = -1;
+ hook.out = -1;
+ if (start_command(&hook) != 0)
+ return fetched_refs;
+
+ /* Use an async writer to feed the hook process.
+ * This allows the hook to read and write a line at
+ * a time without blocking. */
+ memset(&async, 0, sizeof(async));
+ async.proc = feed_post_fetch_hook;
+ post_fetch_hook_refs = fetched_refs;
+ async.out = hook.in;
+ if (start_async(&async))
+ goto failed_hook;
+
+ for_each_ref(add_existing, &existing_refs);
+
+ f = fdopen(hook.out, "r");
+ if (f == NULL)
+ goto failed_hook;
+ strbuf_init(&buf, 128);
+ while (strbuf_getline(&buf, f, '\n') != EOF) {
+ char *l = strbuf_detach(&buf, NULL);
+ ref = parse_post_fetch_hook_line(l, &existing_refs);
+ if (ref) {
+ if (prevref) {
+ prevref->next=ref;
+ prevref=ref;
+ }
+ else {
+ new_refs = prevref = ref;
+ }
+ }
+ else {
+ ok = 0; /* ignore the other output */
+ }
+ free(l);
+ }
+ strbuf_release(&buf);
+ fclose(f);
+
+ if (finish_async(&async))
+ goto failed_hook;
+ finish_command(&hook);
+
+ if (! ok)
+ return fetched_refs;
+ /* The new_refs are returned, to be used in place of fetched_refs,
+ * so it is not needed anymore and can be freed here. */
+ free_refs(fetched_refs);
+ return new_refs;
+
+failed_hook:
+ close(hook.out);
+ finish_command(&hook);
+ return fetched_refs;
+}
+
static void unlock_pack(void)
{
if (transport)
@@ -507,17 +689,30 @@ static int quickfetch(struct ref *ref_map)
return check_everything_connected(iterate_ref_map, 1, &rm);
}
-static int fetch_refs(struct transport *transport, struct ref *ref_map)
+struct fetch_refs_result {
+ struct ref *new_refs;
+ int status;
+};
+
+static struct fetch_refs_result fetch_refs(struct transport *transport,
+ struct ref *ref_map)
{
- int ret = quickfetch(ref_map);
- if (ret)
- ret = transport_fetch_refs(transport, ref_map);
- if (!ret)
- ret |= store_updated_refs(transport->url,
+ struct fetch_refs_result res;
+ res.status = quickfetch(ref_map);
+ if (res.status)
+ res.status = transport_fetch_refs(transport, ref_map);
+ if (!res.status) {
+ res.new_refs = run_post_fetch_hook(ref_map);
+
+ res.status |= store_updated_refs(transport->url,
transport->remote->name,
- ref_map);
+ res.new_refs);
+ }
+ else {
+ res.new_refs = ref_map;
+ }
transport_unlock_pack(transport);
- return ret;
+ return res;
}
static int prune_refs(struct refspec *refs, int ref_count, struct ref *ref_map)
@@ -542,15 +737,6 @@ static int prune_refs(struct refspec *refs, int ref_count, struct ref *ref_map)
return result;
}
-static int add_existing(const char *refname, const unsigned char *sha1,
- int flag, void *cbdata)
-{
- struct string_list *list = (struct string_list *)cbdata;
- struct string_list_item *item = string_list_insert(list, refname);
- item->util = (void *)sha1;
- return 0;
-}
-
static int will_fetch(struct ref **head, const unsigned char *sha1)
{
struct ref *rm = *head;
@@ -673,6 +859,7 @@ static int do_fetch(struct transport *transport,
struct string_list_item *peer_item = NULL;
struct ref *ref_map;
struct ref *rm;
+ struct fetch_refs_result res;
int autotags = (transport->remote->fetch_tags == 1);
for_each_ref(add_existing, &existing_refs);
@@ -710,7 +897,9 @@ static int do_fetch(struct transport *transport,
if (tags == TAGS_DEFAULT && autotags)
transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, "1");
- if (fetch_refs(transport, ref_map)) {
+ res = fetch_refs(transport, ref_map);
+ ref_map = res.new_refs;
+ if (res.status) {
free_refs(ref_map);
return 1;
}
@@ -750,7 +939,8 @@ static int do_fetch(struct transport *transport,
if (ref_map) {
transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, NULL);
transport_set_option(transport, TRANS_OPT_DEPTH, "0");
- fetch_refs(transport, ref_map);
+ res = fetch_refs(transport, ref_map);
+ ref_map = res.new_refs;
}
free_refs(ref_map);
}
--
1.7.7.3
^ permalink raw reply related
* Re: [RFC PATCH] Allow cloning branches selectively
From: Carlos Martín Nieto @ 2011-12-25 20:37 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vmxajaswj.fsf@alter.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 3455 bytes --]
On Fri, Dec 23, 2011 at 01:18:36PM -0800, Junio C Hamano wrote:
> Carlos Martín Nieto <cmn@elego.de> writes:
>
> > Sometimes it's useful to clone only a subset of branches from a remote
> > we're cloning. Teach clone the --fetch option to select which branches
> > should get fetched.
>
> This is just a knee-jerk reaction without reading the patch text (which I
> won't be doing over the holiday weekend anyway), but is the workflow of
> the primarly intended audience to clone "a subset of branches" or "a
> single branch"?
>
> I have a slight suspicion that this started out as "I often want to create
> a clone to _track_ a single branch, but because I am mucking with the code
> related to cloning anyway, I might as well allow more than one to be
> fetched, even though I do not have any need for that, somebody might find
> it useful". And that is why it is important to answer the first question.
The main usefulness is indeed to track single branches. Limiting it to
one looked to me to be an arbitrary limitation and I don't like
those. Tracking between two and all branches is probably quite a niche
however, so I'll go with allowing one -t/--track option and if enough
people want to extend it, we'll see what we do then.
>
> If the primary motivation is for a single branch, I suspect supporting
> only a single branch and advertising the feature as "tracking only one
> branch" might make it much easier to understand to the end users.
>
> Upon "git clone --track cn/single-clone $there x.git", you would do
> something like:
>
> it=cn/single-clone &&
> git init x.git &&
> cd x.git &&
>
> # configure "git fetch origin" to only get branch $it
> git config remote.origin.url "$there" &&
> git config remote.origin.fetch refs/heads/$it:refs/remotes/origin/$it
>
> # declare that the primary branch at origin is $it as far as we are concerned
> git symbolic-ref -m clone refs/remotes/origin/HEAD refs/remotes/origin/$it &&
As Jakub pointed out, remote already has this option, so you can
replace these calls with
git remote add origin $there -t $it -m $it
>
> # Git aware prompt reminds us that this repository is to track branch $it
> git symbolic-ref -m clone HEAD refs/heads/$it &&
>
> # And Go!
> git fetch origin &&
> git reset --hard remotes/origin/$it &&
> git config branch.$it.remote origin &&
> git config branch.$it.merge $it
A lazier man (or one who doesn't work with internals every day) might do
git checkout -t origin/$it
which the documentation claims would do the same thing. So it boils
down to doing three commands and it feels like it'd be much easier to
just write a small script than to modify the clone code. Putting it in
C will hopefully make the code a bit cleaner, if I use the code that
already exists in remote.
>
> Of course you _could_ support more than one pretty easily, but the point
> is that it is unclear how you explain to the end user what the feature
> does and what it is for in easily understoodd terms, once you start doing
> so. It will no longer be "this new clone is to track that branch", but
> something else, and I do not know what that something else is.
Looking at the explanation for the -t (--track) option in git-remote,
it's certainly not very friendly unless you already know exactly what
a -t option would do if you were to implement it.
cmn
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply
* Re: Gitk: shortcut to jump to the current HEAD (yellow spot)?
From: Dirk Süsserott @ 2011-12-25 19:33 UTC (permalink / raw)
To: Martin von Zweigbergk; +Cc: Pat Thoyts, Git Mailing List
In-Reply-To: <CAOeW2eGCKxYW1TT-HPoSCO0_PsQPX5C-bcHLUy73MTd7=CsqRA@mail.gmail.com>
Am 24.12.2011 05:22 schrieb Martin von Zweigbergk:
> 2011/12/23 Dirk Süsserott <newsletter@dirk.my1.cc>:
>>
>> That's because gitk behaves odd (at least to me) when not run from the
>> top-level directory. E.g. the "touching paths" box won't find files in
>> the top dir if you don't prefix them with a slash.
>
> This should be fixed in c332f44 (gitk: Fix file highlight when run in
> subdirectory, 2011-04-04), which is in the current master and thus, I
> believe, to be released in Git 1.7.9.
>
> Martin
Ahh, cool. I wouldn't have noticed because I'm so used to my "cd $TOP &&
gitk". I thought it was by intention because it just behaves like "git
log": When run from subdirs it doesn't know about topdir files: Assume
README.txt is in the topdir and current dir is some subdir:
$ git log -- README.txt # fails
$ git log -- ../README.txt # works
My alias (or function) was just a helper to avoid remembering where I
started gitk from.
Cheers,
Dirk
BTW, Merry X-Mas to you and all others on the list :-)
^ permalink raw reply
* Re: [RFC PATCH] Allow cloning branches selectively
From: Carlos Martín Nieto @ 2011-12-25 19:00 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: git
In-Reply-To: <CACsJy8ANZvkY+na-tM95prHEfXD+N2OT8+3NLeccycGL1BmbCg@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 596 bytes --]
On Sat, Dec 24, 2011 at 11:31:51AM +0700, Nguyen Thai Ngoc Duy wrote:
> On Sat, Dec 24, 2011 at 3:13 AM, Carlos Martín Nieto <cmn@elego.de> wrote:
> > Sometimes it's useful to clone only a subset of branches from a remote
> > we're cloning. Teach clone the --fetch option to select which branches
> > should get fetched.
>
> What about tags? Are all tags fetched or only ones part of the
> selected branches?
I haven't really touched that part of the code, so I think it'll fetch
every tag, as that's what clone does by default. Certainly something
that should get fixed.
cmn
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply
* Re: [PATCH] add post-fetch hook
From: Joey Hess @ 2011-12-25 18:06 UTC (permalink / raw)
To: git
In-Reply-To: <m3liq0fwkz.fsf@localhost.localdomain>
[-- Attachment #1: Type: text/plain, Size: 952 bytes --]
Jakub Narebski wrote:
> This is a very nice idea, IMHO, both because it makes it simple to
> implement no-op (example) hook by just using "cat", and beause it
> makes possible to stack such hooks (e.g. one from git-annex with the
> one from pristine-tar etc.).
Indeed.
> One thing that needs to be specified is what should happen if the hook
> changes "the refname at the remote" part...
I'm not sure what the use case is for including the remote's refname is yet.
Changing it would allow changing what's written into FETCH_HEAD though.
For example:
-2ce0edcd786b790fed580e7df56291619834d276 not-for-merge branch 'maint' of git://git.kernel.org/pub/scm/git/git
+2ce0edcd786b790fed580e7df56291619834d276 not-for-merge branch 'jc-maint' of git://git.kernel.org/pub/scm/git/git
And that would in turn be used by a few things that consume that
information. Whether that's useful, I don't know.
--
see shy jo
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 828 bytes --]
^ permalink raw reply
* Re: [PATCH] add post-fetch hook
From: Joey Hess @ 2011-12-25 17:54 UTC (permalink / raw)
To: git
In-Reply-To: <7vsjk99exw.fsf@alter.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 2987 bytes --]
Junio C Hamano wrote:
> So if we were to allow the hook to lie what commits were fetched and store
> something different from what we fetched in the remote tracking refs, I
> think the correct place to do so would be in store_updated_refs(),
> immediately before we call check_everything_connected().
>
> - Feed the contents of the ref_map to the hook. For each entry, the hook
> would get (at least):
> . the object name;
> . the refname at the remote;
> . the refname at the local (which could be empty when we are not
> copying it to any of our local ref); and
> . if the entry is to be used for merge.
>
> - The hook must read _everything_ from its standard input, and then
> return the
> re-written result in the same format as its input. The hook could
> . update the object name (i.e. re-point the remote tracking ref);
> . update the local refname (i.e. use different remote tracking ref);
> . change "merge" flag between true/false; and/or
> . add or remove entries
>
> - You read from the hook and replace the ref_map list that is fed to
> check_everything_connected(). This ref_map list is what is used in the
> next for() loop that calls update_local_ref() to update the remote
> tracking ref, records the entry in FETCH_HEAD, and produces the report.
>
> This way, the hook cannot screw up, as what it tells us will consistently
> be written by us to where it should go.
This is a good plan, the only problem I see with it is that
store_updated_refs is potentially called twice in a fetch, when the
automated tag following is done. I don't see that as a large problem,
perhaps it could even be optimised away.
The format of .git/FETCH_HEAD does not seem appropriate for this hook
to use (it's not documented, and it doesn't quite have all the necessary
fields, particularly missing the local refname). Instead, how about this,
for the hook's input/output format?
<sha1> SP <not-for-merge|merge> SP <remote-refname> SP <local-refname> LF
Example:
5d6dfc7cb140a6eb90138334fab2245b69bc8bc4 merge refs/heads/master refs/remotes/origin/master
f95247ea15bc62a2dab0f6ae3cd247267a0639b8 not-for-merge refs/heads/pu refs/remotes/origin/pu
2ce0edcd786b790fed580e7df56291619834d276 not-for-merge refs/tags/v1.7.8.1 refs/tags/v1.7.8.1
Allowing the hook to change the merge flag does open up some other
interesting uses of the hook. I can now think of three use cases for it:
1. Only accepting tags that meet some criteria, such as being signed
by a trusted signature.
2. Causing branches that would not normally be merged to get merged.
For example, a hook could set the merge flag on a branch when it was
pulled from a remote other than branch.master.remote. This could be useful
when using git without a single central origin and with a number of
repositories that are always wanted to be kept merged.
3. My git annex merge case.
--
see shy jo
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 828 bytes --]
^ permalink raw reply
* Re: [RFC PATCH] Allow cloning branches selectively
From: Carlos Martín Nieto @ 2011-12-25 17:43 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git
In-Reply-To: <m3hb0ofwem.fsf@localhost.localdomain>
[-- Attachment #1: Type: text/plain, Size: 838 bytes --]
On Sun, Dec 25, 2011 at 08:28:39AM -0800, Jakub Narebski wrote:
> Carlos Martín Nieto <cmn@elego.de> writes:
>
> > Sometimes it's useful to clone only a subset of branches from a remote
> > we're cloning. Teach clone the --fetch option to select which branches
> > should get fetched.
> >
> > Each --fetch sets up a fetch refspec for that branch. Previously this
> > was only possible by initializing a repo and manually setting up the
> > config.
>
> I haven't read the code, but I guess it should be possible to share
> code with "git remote add", which allows to select which branches to
> track, and which branch is to be 'main' on remote.
>
> git remote add [-t <branch>] [-m <master>] [-f] ...
Ah, very nice. I didn't know about those options. Hopefully I can use
it. Thanks for pointing it out.
cmn
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ 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