Git development
 help / color / mirror / Atom feed
* Re: [PATCH] RFC Allow case insensitive search flag with git-grep for fixed-strings
From: Jeff King @ 2009-11-06 10:13 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Brian Collins, git
In-Reply-To: <7vbpjg0y8k.fsf@alter.siamese.dyndns.org>

On Fri, Nov 06, 2009 at 02:00:11AM -0800, Junio C Hamano wrote:

> > I don't see a reason not to allow this combination if our grep
> > implementation supports it. My only reservation would be that we
> > sometimes call out to an external grep, and non-GNU grep might barf on
> > this. But I think that is OK, as the user should get a sane error from
> > the external grep.
> 
> I think that is OK but not for the reason you stated.  The user should
> never get such an error message.
> 
> The reason why I think this is OK is because the builder can choose to use
> NO_EXTERNAL_GREP if the external grep does not allow this combination.

Yes, I think that would be a sane thing to do (and I suspect anyone
using non-GNU grep is probably already doing it). But what I meant more
was that the _transition_ should be fine. If we start shipping with this
patch but people haven't updated their build configuration, it is not
going to break horribly; it should just produce an error, which is what
it is doing now.

>     # Define NO_EXTERNAL_GREP if you don't want "git grep" to ever call
>     # your external grep (e.g., if your system lacks grep, if its grep
>     # does not support necessary features, or spawning external process is
>     # slower than built-in grep git has).  To be usable, your grep must
>     # support -C<n> (show n lines of context), -e <pattern> (primarily
>     # used to quote a pattern that begins with a dash), and use of -F and
>     # -i at the same time.  Otherwise define this.
> 
> But I didn't try hard to find out what _else_ we are depending on.

It is not really _us_ depending on it. It is "things the user wants to
do that _we_ support, but that their grep might not." So I don't think
there is much point in enumerating features. If their system grep
doesn't handle options that they want to use, then it won't work for
them. If they don't use them, then they will be fine.

Though "-e" might be the exception, as I think we might use it
unconditionally. But something like "-F -i" really depends on whether
the user wants to use it.

So I am fine with the text above, but I wouldn't worry too hard about
trying to come up with an exhaustive feature list.

-Peff

^ permalink raw reply

* Timur Sufiev: Re: [PATCH I18N filenames v2 3/3] Provide compatibility with MinGW
From: Timur Sufiev @ 2009-11-06 10:00 UTC (permalink / raw)
  To: git

[-- Attachment #1: forwarded message --]
[-- Type: message/rfc822, Size: 2205 bytes --]

From: Timur Sufiev <tsufiev@gmail.com>
To: Peter Krefting <peter@softwolves.pp.se>
Subject: Re: [PATCH I18N filenames v2 3/3] Provide compatibility with MinGW
Date: Tue, 03 Nov 2009 19:53:44 +0300

Hello, Peter
> Hi!
> 
> Instead of calling the open_i18n() which converts from UTF-8 to a local 
> 8-bit character set, this should probably call a version that converts from 
> UTF-8 to UTF-16 and uses _wopen().
> 
> Same thing for fopen_i18n() and _wfopen().
> 
> I created a small RFC patch for that that changed parts of the system 
> earlier this year - http://kerneltrap.org/mailarchive/git/2009/3/2/5350814
> 
> I did not address readdir() and friends, I'm not sure if they are available 
> in UTF-16 form or if they need to be rewritten using findfirst()/findnext().
> 
> -- 
> \\// Peter - http://www.softwolves.pp.se/

I've decided to stick to local 8-bit encoding for now having considered
the following issues:

1. Many git front-ends, e.g. TortoiseGit, use 8-bit set, not UTF-16:
they call git plumbing commands and pass filenames to command line (in
local 8-bit encoding). So, using [UTF-8] <-> [UTF-16] approach, I had to
deal with 3 different encodings: UTF-8, UTF-16 and local 8-bit one
(CP1251 in my case). Moreover, Windows itself uses both UTF-16 and
CP1251, so one had to deal with reencoding between them (if he plans to
support UTF-16). Too much confusion.
 
2. UTF-16 is a proper solution for Windows, but my patch is useful for
other OSes with locales different from UTF-8 (e.g. Linux with KOI8-R
locale).

Still there is a possibility that one day we'll stumble upon some UTF-8
symbol which cannot not be correctly mapped into 8-bit encoding. UTF-16
would be a remedy in this case, but what if don't have it (see 2)?
-- 
Timur Sufiev

[-- Attachment #2: Signature --]
[-- Type: text/plain, Size: 17 bytes --]

-- 
Timur Sufiev

^ permalink raw reply

* Re: [PATCH] RFC Allow case insensitive search flag with git-grep for fixed-strings
From: Junio C Hamano @ 2009-11-06 10:00 UTC (permalink / raw)
  To: Jeff King; +Cc: Brian Collins, git
In-Reply-To: <20091106084855.GA20964@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

>> git-grep currently throws an error when you combine the -F and -i
>> flags. This isn't in line with how GNU grep handles it. This patch
>> allows the simultaneous use of those flags.
>
> I don't see a reason not to allow this combination if our grep
> implementation supports it. My only reservation would be that we
> sometimes call out to an external grep, and non-GNU grep might barf on
> this. But I think that is OK, as the user should get a sane error from
> the external grep.

I think that is OK but not for the reason you stated.  The user should
never get such an error message.

The reason why I think this is OK is because the builder can choose to use
NO_EXTERNAL_GREP if the external grep does not allow this combination.

We need to update comments on the Makefile if we are going to take this
patch.  Currently the description suggests three possible reasons you
might want to choose NO_EXTERNAL_GREP.  "lacks grep" is obvious, "slower"
is not about correctness, but "is broken" is way underspecified, and this
patch adds one more reason to label your "grep" as broken.

Currently, SunOS and IRIX/IRIX64 are the only ones that specify
NO_EXTERNAL_GREP, and I suspect both are defined due to "is broken", but
we do not tell the builder in what way they are considered broken, iow,
what features we expect from the platform "grep", so somebody coming up
with a new port would be at loss.

I suspect 01ae841 (SunOS grep does not understand -C<n> nor -e, 2009-07-23)
would be a good starting point.  Something like this...

    # Define NO_EXTERNAL_GREP if you don't want "git grep" to ever call
    # your external grep (e.g., if your system lacks grep, if its grep
    # does not support necessary features, or spawning external process is
    # slower than built-in grep git has).  To be usable, your grep must
    # support -C<n> (show n lines of context), -e <pattern> (primarily
    # used to quote a pattern that begins with a dash), and use of -F and
    # -i at the same time.  Otherwise define this.

But I didn't try hard to find out what _else_ we are depending on.

> Tests? They help prove to us that your feature works, and also prevent
> us from accidentally breaking your feature in the future.

Yeah, thanks.  The latter is the primary purpose of test.

^ permalink raw reply

* Re: Allowing push --dry-run through fetch url
From: Mike Hommey @ 2009-11-06  9:53 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vfx8s0yy1.fsf@alter.siamese.dyndns.org>

On Fri, Nov 06, 2009 at 01:44:54AM -0800, Junio C Hamano wrote:
<snip>
> But doesn't "branch -v" give the necessary information for that purpose
> and even a bit more?  Couldn't "git fetch && git branch -v" be a better
> solution for your real problem "X"?

Usually, when I run git push --dry-run, it's to check that what follows
(usually the refspec part) does what I want it to do, such as not pushing
tags I didn't intend to push(*), and stuff like that.

Mike

(*) the fact that all tags from all remotes are being mixed doesn't help,
here, and without dry-running, I end up pushing tags from one remote onto
another.

^ permalink raw reply

* Re: Problem signing a tag
From: Michael J Gruber @ 2009-11-06  9:54 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Joshua J. Kugler, Alex Riesen, git
In-Reply-To: <7vy6mk91ig.fsf@alter.siamese.dyndns.org>

Junio C Hamano venit, vidit, dixit 05.11.2009 21:09:
> Michael J Gruber <git@drmicha.warpmail.net> writes:
> 
>> Dig dig dig... gpg exits with 2 in a lot of cases, one would need to
>> parse fd-error to find out more. But it also looks as if gpg exits
>> normally with a good passphrase. So I tried, and at least with gpg 1.4.9
>> and git 1.6.5.2 I can sign tags with "use-agent" and without a running
>> agent: I get asked for the passphrase (after reporting the agent MIA),
>> and everything's fine.
>>
>> My gpg returns 0 in this case; it returns 2 only if I don't enter the
>> passphrase. So, this seems to depend on the version of gpg. Or on
>> entering the correct passphrase ;)
> 
> If the problematic gpg that gives 2 is older than yours, the situation
> looks to me that "exiting 2 when failed to contact agent but got a good
> passphrase some other way and successfully signed" was diagnosed as a bug
> and then fixed in gpg.  If that is the case can we find out which version
> that fix is in, and add an entry to FAQ to help next person who will be
> hit by this when using "tag -s"?

Both of us seem to be using gpg 1.4.9, which is weird. I even checked
Fedora's srpm, they don't apply any patches for this. For the record,
I'm doing

unset GPG_AGENT_INFO
echo a |gpg -bsa

with "use-agent" and a default key signing specified in my gpg conf.
This returns "0" if I enter the correct passphrase (after being warned
about the missing agent) and "2" if I enter a wrong one repeatedly.
Joshua, your reports seem to confirm that you get 2 in both cases from
your gpg 1.4.9, right?

Michael

^ permalink raw reply

* Re: Allowing push --dry-run through fetch url
From: Junio C Hamano @ 2009-11-06  9:44 UTC (permalink / raw)
  To: Mike Hommey; +Cc: git
In-Reply-To: <20091106073707.GA14881@glandium.org>

Mike Hommey <mh@glandium.org> writes:

> The typical use of both at the same time is to put an authenticated
> value for pushurl (ssh://, for example) and an anonymous one (git://,
> for example) for url.
>
> What has been annoying me lately is that git push --dry-run asks me
> for the ssh key or password. I know I could be using an ssh-agent, but
> that's not the point.

I actually sense a possible XY problem here.

What are you trying to achieve with "git push --dry-run", especially when
you would instead be doing an equivalent of "git fetch" (or "ls-remote")
but not storing what you learned from that session with a change like what
you are imagining?

The answer to the above question is the real reason "X".

It could be that what you are interested in is if you are ahead of the
other side.  In other words, you would want to know if some branches
result in non-fast-forward error, causing you to re-fetch and re-merge (or
rebase).

And "push --dry-run" that fails would give you that information, if it
worked for you without authenticating.  And that could be your "Y".

But doesn't "branch -v" give the necessary information for that purpose
and even a bit more?  Couldn't "git fetch && git branch -v" be a better
solution for your real problem "X"?

It is a better solution _if_ the real problem you are trying to solve is
what I suspected above because:

 (1) If you will end up fetching to make you ahead of them again, doing
     "push --dry-run" to learn fast-forwardness first would still require
     you to fetch from there anyway.  With "git fetch && git branch -v",
     you have already fetched when you learned that you are not ahead;

 (2) When you learn from "git fetch && git branch -v" that you were indeed
     ahead, you can push.  In such a case, because you were ahead, the
     fetch wouldn't have slurped a lot of data from the other end anyway,
     so there is no real overhead for doing so.

 (3) In either case, "branch -v" output would give you not just "is it
     fast-forward?" but also "if not, by how much they have diverged" (in
     addition to the commit message for the tip).

But this may not be an XY problem.  I am just curious.

^ permalink raw reply

* Re: [PATCH] RFC Allow case insensitive search flag with git-grep for  fixed-strings
From: Brian Collins @ 2009-11-06  9:22 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20091106084855.GA20964@coredump.intra.peff.net>

2009/11/6 Jeff King <peff@peff.net>:
> You're in the right place (though judging from the response, nobody
> seemed to find your patch all that interesting...).

Heh, yeah it is a bit of a boring edge case but a TextMate plugin I am writing
requires this functionality.

> Tests? They help prove to us that your feature works, and also prevent
> us from accidentally breaking your feature in the future.

Ah yes that is what I was forgetting. Please see the amended patch including
test.
Thanks for your help


---
 builtin-grep.c  |    8 +++++---
 grep.c          |   16 ++++++++++++----
 grep.h          |    2 ++
 t/t7002-grep.sh |    9 +++++++++
 4 files changed, 28 insertions(+), 7 deletions(-)

diff --git a/builtin-grep.c b/builtin-grep.c
index 761799d..c73f05b 100644
--- a/builtin-grep.c
+++ b/builtin-grep.c
@@ -367,7 +367,7 @@ static int external_grep(struct grep_opt *opt,
const char **paths, int cached)
 		push_arg("-h");
 	if (opt->regflags & REG_EXTENDED)
 		push_arg("-E");
-	if (opt->regflags & REG_ICASE)
+	if (opt->caseless)
 		push_arg("-i");
 	if (opt->binary == GREP_BINARY_NOMATCH)
 		push_arg("-I");
@@ -706,8 +706,8 @@ int cmd_grep(int argc, const char **argv, const
char *prefix)
 		OPT_GROUP(""),
 		OPT_BOOLEAN('v', "invert-match", &opt.invert,
 			"show non-matching lines"),
-		OPT_BIT('i', "ignore-case", &opt.regflags,
-			"case insensitive matching", REG_ICASE),
+		OPT_BOOLEAN('i', "ignore-case", &opt.caseless,
+			"case insensitive matching"),
 		OPT_BOOLEAN('w', "word-regexp", &opt.word_regexp,
 			"match patterns only at word boundaries"),
 		OPT_SET_INT('a', "text", &opt.binary,
@@ -830,6 +830,8 @@ int cmd_grep(int argc, const char **argv, const
char *prefix)
 		external_grep_allowed = 0;
 	if (!opt.pattern_list)
 		die("no pattern given.");
+	if (!opt.fixed && opt.caseless)
+		opt.regflags |= REG_ICASE;
 	if ((opt.regflags != REG_NEWLINE) && opt.fixed)
 		die("cannot mix --fixed-strings and regexp");
 	compile_grep_patterns(&opt);
diff --git a/grep.c b/grep.c
index 5d162da..d8f14be 100644
--- a/grep.c
+++ b/grep.c
@@ -41,7 +41,8 @@ static void compile_regexp(struct grep_pat *p,
struct grep_opt *opt)
 	int err;

 	p->word_regexp = opt->word_regexp;
-
+	p->caseless = opt->caseless;
+	
 	if (opt->fixed || is_fixed(p->pattern))
 		p->fixed = 1;
 	if (opt->regflags & REG_ICASE)
@@ -262,9 +263,16 @@ static void show_name(struct grep_opt *opt, const
char *name)
 	printf("%s%c", name, opt->null_following_name ? '\0' : '\n');
 }

-static int fixmatch(const char *pattern, char *line, regmatch_t *match)
+
+static int fixmatch(const char *pattern, char *line, int caseless,
regmatch_t *match)
 {
-	char *hit = strstr(line, pattern);
+	char *hit;
+	if (caseless) {
+		hit = strcasestr(line, pattern);
+	} else {
+		hit = strstr(line, pattern);
+	}
+	
 	if (!hit) {
 		match->rm_so = match->rm_eo = -1;
 		return REG_NOMATCH;
@@ -326,7 +334,7 @@ static int match_one_pattern(struct grep_pat *p,
char *bol, char *eol,

  again:
 	if (p->fixed)
-		hit = !fixmatch(p->pattern, bol, pmatch);
+		hit = !fixmatch(p->pattern, bol, p->caseless, pmatch);
 	else
 		hit = !regexec(&p->regexp, bol, 1, pmatch, eflags);

diff --git a/grep.h b/grep.h
index f6eecc6..24b7d44 100644
--- a/grep.h
+++ b/grep.h
@@ -32,6 +32,7 @@ struct grep_pat {
 	enum grep_header_field field;
 	regex_t regexp;
 	unsigned fixed:1;
+	unsigned caseless:1;
 	unsigned word_regexp:1;
 };

@@ -64,6 +65,7 @@ struct grep_opt {
 	regex_t regexp;
 	int linenum;
 	int invert;
+	int caseless;
 	int status_only;
 	int name_only;
 	int unmatch_name_only;
diff --git a/t/t7002-grep.sh b/t/t7002-grep.sh
index ae56a36..87b47dd 100755
--- a/t/t7002-grep.sh
+++ b/t/t7002-grep.sh
@@ -345,4 +345,13 @@ test_expect_success 'grep from a subdirectory to
search wider area (2)' '
 	)
 '

+cat >expected <<EOF
+hello.c:	return 0;
+EOF
+
+test_expect_success 'grep -Fi' '
+	git grep -Fi rEtUrN >actual &&
+	test_cmp expected actual
+'
+
 test_done
-- 
1.6.4.4

^ permalink raw reply related

* Re: Allowing push --dry-run through fetch url
From: Mike Hommey @ 2009-11-06  9:21 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v4op82fh0.fsf@alter.siamese.dyndns.org>

On Fri, Nov 06, 2009 at 01:02:35AM -0800, Junio C Hamano wrote:
> > Maybe I'm missing something, but the only missing thing I can see at
> > first thought is whether the server is going to reject non fast-forward
> > updates. Are there any others ?
> 
> Hooks, for a starter.

AFAICS, they are not run when using --dry-run.

Mike

^ permalink raw reply

* Re: Allowing push --dry-run through fetch url
From: Junio C Hamano @ 2009-11-06  9:02 UTC (permalink / raw)
  To: Mike Hommey; +Cc: git
In-Reply-To: <20091106085917.GA497@glandium.org>

Mike Hommey <mh@glandium.org> writes:

> On Fri, Nov 06, 2009 at 12:49:49AM -0800, Junio C Hamano wrote:
>> The daemon sitting on the other end to serve "git://" URL won't understand
>> an attempt to push into.  What goes over the wire in the fetch protocol
>> does not give your updated "git push" enough information to guess what
>> would happen if you pushed.
>
> Maybe I'm missing something, but the only missing thing I can see at
> first thought is whether the server is going to reject non fast-forward
> updates. Are there any others ?

Hooks, for a starter.

^ permalink raw reply

* Re: Allowing push --dry-run through fetch url
From: Mike Hommey @ 2009-11-06  8:59 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vtyx82g2a.fsf@alter.siamese.dyndns.org>

On Fri, Nov 06, 2009 at 12:49:49AM -0800, Junio C Hamano wrote:
> The daemon sitting on the other end to serve "git://" URL won't understand
> an attempt to push into.  What goes over the wire in the fetch protocol
> does not give your updated "git push" enough information to guess what
> would happen if you pushed.

Maybe I'm missing something, but the only missing thing I can see at
first thought is whether the server is going to reject non fast-forward
updates. Are there any others ?

Mike

^ permalink raw reply

* Re: Allowing push --dry-run through fetch url
From: Junio C Hamano @ 2009-11-06  8:49 UTC (permalink / raw)
  To: Mike Hommey; +Cc: git
In-Reply-To: <20091106073707.GA14881@glandium.org>

Mike Hommey <mh@glandium.org> writes:

> So, before I dive in, what would you think about such a feature?

The daemon sitting on the other end to serve "git://" URL won't understand
an attempt to push into.  What goes over the wire in the fetch protocol
does not give your updated "git push" enough information to guess what
would happen if you pushed.

Of course, it is open source and everything can be modified.  You _could_
change the daemon whose primary purpose has been to serve fetch requests
to also support dry-run only push to make what you outlined work.

But isn't it adding unnecessary complexity for no real reason?  It would
be a tough sell, I would imagine.

^ permalink raw reply

* Re: [PATCH] RFC Allow case insensitive search flag with git-grep for fixed-strings
From: Jeff King @ 2009-11-06  8:48 UTC (permalink / raw)
  To: Brian Collins; +Cc: git
In-Reply-To: <B7C4E16C-B15D-4A7B-873A-B6BD0FDAD8C8@gmail.com>

On Thu, Oct 29, 2009 at 06:21:59PM -0700, Brian Collins wrote:

> You will have to excuse me, this is my first patch and I don't know
> if this is the right place to post this. Apologies in advance if I'm
> in the wrong place.

You're in the right place (though judging from the response, nobody
seemed to find your patch all that interesting...).

> git-grep currently throws an error when you combine the -F and -i
> flags. This isn't in line with how GNU grep handles it. This patch
> allows the simultaneous use of those flags.

I don't see a reason not to allow this combination if our grep
implementation supports it. My only reservation would be that we
sometimes call out to an external grep, and non-GNU grep might barf on
this. But I think that is OK, as the user should get a sane error from
the external grep.

>  builtin-grep.c |    8 +++++---
>  grep.c         |   16 ++++++++++++----
>  grep.h         |    2 ++

Tests? They help prove to us that your feature works, and also prevent
us from accidentally breaking your feature in the future.

-Peff

^ permalink raw reply

* Re: [PATCH] gitk: disable checkout of remote branch
From: Sverre Rabbelier @ 2009-11-06  8:30 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, Tim Mazid, git
In-Reply-To: <7v8wek7a6z.fsf@alter.siamese.dyndns.org>

Heya,

On Fri, Nov 6, 2009 at 01:45, Junio C Hamano <gitster@pobox.com> wrote:
> Jeff King <peff@peff.net> writes:
>> Sorry, yes, I just saw Sverre's comment and misread the original
>> proposal.  Checking out "$remote/$branch" will still detach the HEAD,
>> and I don't think anybody has a previous proposal to change that.
>
> Heh, I think both of us forgot that we decided it is safe enough not to
> wait for 1.7.0 already, because the situation this kicks in has always
> resulted in an error.  We have it in master since e3de372 (Merge branch
> 'jc/checkout-auto-track', 2009-10-30).

Sorry for causing so much confusion, I misremembered what the patch
was about ("git checkout <branch>" vs "git checkout
<remote>/<branch>"). Apologies.

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* Re: [PATCH] MSVC: Windows-native implementation for subset of Pthreads API
From: Johannes Sixt @ 2009-11-06  8:25 UTC (permalink / raw)
  To: Andrzej K. Haczewski
  Cc: git, Nicolas Pitre, kusmabite, Johannes Schindelin, Paolo Bonzini
In-Reply-To: <1257495059-12394-1-git-send-email-ahaczewski@gmail.com>

Andrzej K. Haczewski schrieb:
> One more round of that patch with Nicolas' comments considered, and
> disclamer about the implementation added.
> 
> Johannes, can you replace previous commit with that patch?

Thanks; the result is in

  git://repo.or.cz/git/mingw/j6t.git pthreads-for-windows

-- Hannes

^ permalink raw reply

* Re: [QGIT PATCH/RFC]
From: Abdelrazak Younes @ 2009-11-06  8:16 UTC (permalink / raw)
  To: Marco Costalba; +Cc: git
In-Reply-To: <e5bfff550911051225s13c6e39dh355dc3ab1c0623f@mail.gmail.com>

Marco Costalba wrote:
> On Thu, Nov 5, 2009 at 11:37, Abdelrazak Younes <younes@lyx.org> wrote:
>   
>> I recompiled qgit with the Qt version and I didn't notice any performance
>> problem with a big repo (Qt).
>>
>>     
>
> In git we don't need to compute hashes of sha strings because they are
> already hashed !
>   

Now you tell it, it's obvious indeed :-)

Thanks,
Abdel.

^ permalink raw reply

* Re: [QGIT PATCH/RFC]
From: Abdelrazak Younes @ 2009-11-06  8:15 UTC (permalink / raw)
  To: Marco Costalba; +Cc: git
In-Reply-To: <e5bfff550911051227g257312b2idf729b9451d1bb@mail.gmail.com>

Marco Costalba wrote:
> On Thu, Nov 5, 2009 at 21:25, Marco Costalba <mcostalba@gmail.com> wrote:
>   
>> When I tested I _found_ a speed difference, but now I don't remember
>> of how much. Be sure you have warm cache when doing the test (press
>> F5) for few times to be sure all is in RAM.
>>
>>     
>
> Sorry, I forgot.  Be sure also our custom hashing function is actually
> called in your binary and the compiler didn't linked the Qt one
> instead :-)
>   

Yes I made sure of that (see my other mail).

Abdel.

^ permalink raw reply

* [PATCH] MSVC: Windows-native implementation for subset of Pthreads API
From: Andrzej K. Haczewski @ 2009-11-06  8:10 UTC (permalink / raw)
  To: git
  Cc: Andrzej K. Haczewski, Nicolas Pitre, kusmabite,
	Johannes Schindelin, Johannes Sixt, Paolo Bonzini
In-Reply-To: <1257283802-29726-1-git-send-email-ahaczewski@gmail.com>

This patch implements native to Windows subset of pthreads API used by Git.
It allows to remove Pthreads for Win32 dependency for MSVC, msysgit and
Cygwin.

The patch modifies Makefile only for MSVC (that's the environment I'm
capable of testing on), so it requires further corrections to compile
with MinGW or Cygwin.

Signed-off-by: Andrzej K. Haczewski <ahaczewski@gmail.com>
---
One more round of that patch with Nicolas' comments considered, and
disclamer about the implementation added.

Johannes, can you replace previous commit with that patch?

 Makefile               |    7 ++-
 builtin-pack-objects.c |   31 +++++++++++--
 compat/mingw.c         |    2 +-
 compat/mingw.h         |    5 ++
 compat/win32/pthread.c |  120 ++++++++++++++++++++++++++++++++++++++++++++++++
 compat/win32/pthread.h |   68 +++++++++++++++++++++++++++
 6 files changed, 225 insertions(+), 8 deletions(-)
 create mode 100644 compat/win32/pthread.c
 create mode 100644 compat/win32/pthread.h

diff --git a/Makefile b/Makefile
index bc039ac..30089a8 100644
--- a/Makefile
+++ b/Makefile
@@ -453,6 +453,7 @@ LIB_H += commit.h
 LIB_H += compat/bswap.h
 LIB_H += compat/cygwin.h
 LIB_H += compat/mingw.h
+LIB_H += compat/win32/pthread.h
 LIB_H += csum-file.h
 LIB_H += decorate.h
 LIB_H += delta.h
@@ -971,15 +972,15 @@ ifdef MSVC
 	OBJECT_CREATION_USES_RENAMES = UnfortunatelyNeedsTo
 	NO_REGEX = YesPlease
 	NO_CURL = YesPlease
-	NO_PTHREADS = YesPlease
+	THREADED_DELTA_SEARCH = YesPlease
 	BLK_SHA1 = YesPlease
 
 	CC = compat/vcbuild/scripts/clink.pl
 	AR = compat/vcbuild/scripts/lib.pl
 	CFLAGS =
 	BASIC_CFLAGS = -nologo -I. -I../zlib -Icompat/vcbuild -Icompat/vcbuild/include -DWIN32 -D_CONSOLE -DHAVE_STRING_H -D_CRT_SECURE_NO_WARNINGS -D_CRT_NONSTDC_NO_DEPRECATE
-	COMPAT_OBJS = compat/msvc.o compat/fnmatch/fnmatch.o compat/winansi.o
-	COMPAT_CFLAGS = -D__USE_MINGW_ACCESS -DNOGDI -DHAVE_STRING_H -DHAVE_ALLOCA_H -Icompat -Icompat/fnmatch -Icompat/regex -Icompat/fnmatch -DSTRIP_EXTENSION=\".exe\"
+	COMPAT_OBJS = compat/msvc.o compat/fnmatch/fnmatch.o compat/winansi.o compat/win32/pthread.o
+	COMPAT_CFLAGS = -D__USE_MINGW_ACCESS -DNOGDI -DHAVE_STRING_H -DHAVE_ALLOCA_H -Icompat -Icompat/fnmatch -Icompat/regex -Icompat/fnmatch -Icompat/win32 -DSTRIP_EXTENSION=\".exe\"
 	BASIC_LDFLAGS = -IGNORE:4217 -IGNORE:4049 -NOLOGO -SUBSYSTEM:CONSOLE -NODEFAULTLIB:MSVCRT.lib
 	EXTLIBS = advapi32.lib shell32.lib wininet.lib ws2_32.lib
 	lib =
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 02f9246..00594fd 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -1255,15 +1255,15 @@ static int delta_cacheable(unsigned long src_size, unsigned long trg_size,
 
 #ifdef THREADED_DELTA_SEARCH
 
-static pthread_mutex_t read_mutex = PTHREAD_MUTEX_INITIALIZER;
+static pthread_mutex_t read_mutex;
 #define read_lock()		pthread_mutex_lock(&read_mutex)
 #define read_unlock()		pthread_mutex_unlock(&read_mutex)
 
-static pthread_mutex_t cache_mutex = PTHREAD_MUTEX_INITIALIZER;
+static pthread_mutex_t cache_mutex;
 #define cache_lock()		pthread_mutex_lock(&cache_mutex)
 #define cache_unlock()		pthread_mutex_unlock(&cache_mutex)
 
-static pthread_mutex_t progress_mutex = PTHREAD_MUTEX_INITIALIZER;
+static pthread_mutex_t progress_mutex;
 #define progress_lock()		pthread_mutex_lock(&progress_mutex)
 #define progress_unlock()	pthread_mutex_unlock(&progress_mutex)
 
@@ -1590,7 +1590,26 @@ struct thread_params {
 	unsigned *processed;
 };
 
-static pthread_cond_t progress_cond = PTHREAD_COND_INITIALIZER;
+static pthread_cond_t progress_cond;
+
+/*
+ * Mutex and conditional variable can't be statically-initialized on Windows.
+ */
+static void init_threaded_search()
+{
+	pthread_mutex_init(&read_mutex, NULL);
+	pthread_mutex_init(&cache_mutex, NULL);
+	pthread_mutex_init(&progress_mutex, NULL);
+	pthread_cond_init(&progress_cond, NULL);
+}
+
+static void cleanup_threaded_search()
+{
+	pthread_cond_destroy(&progress_cond);
+	pthread_mutex_destroy(&read_mutex);
+	pthread_mutex_destroy(&cache_mutex);
+	pthread_mutex_destroy(&progress_mutex);
+}
 
 static void *threaded_find_deltas(void *arg)
 {
@@ -1629,8 +1648,11 @@ static void ll_find_deltas(struct object_entry **list, unsigned list_size,
 	struct thread_params *p;
 	int i, ret, active_threads = 0;
 
+	init_threaded_search();
+
 	if (delta_search_threads <= 1) {
 		find_deltas(list, &list_size, window, depth, processed);
+		cleanup_threaded_search();
 		return;
 	}
 	if (progress > pack_to_stdout)
@@ -1745,6 +1767,7 @@ static void ll_find_deltas(struct object_entry **list, unsigned list_size,
 			active_threads--;
 		}
 	}
+	cleanup_threaded_search();
 	free(p);
 }
 
diff --git a/compat/mingw.c b/compat/mingw.c
index 6b5b5b2..f2e9f02 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -5,7 +5,7 @@
 
 #include <shellapi.h>
 
-static int err_win_to_posix(DWORD winerr)
+int err_win_to_posix(DWORD winerr)
 {
 	int error = ENOSYS;
 	switch(winerr) {
diff --git a/compat/mingw.h b/compat/mingw.h
index 6907345..7e25fb5 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -294,3 +294,8 @@ struct mingw_dirent
 #define readdir(x) mingw_readdir(x)
 struct dirent *mingw_readdir(DIR *dir);
 #endif // !NO_MINGW_REPLACE_READDIR
+
+/*
+ * Used by Pthread API implementation for Windows
+ */
+extern int err_win_to_posix(DWORD winerr);
diff --git a/compat/win32/pthread.c b/compat/win32/pthread.c
new file mode 100644
index 0000000..652d7b4
--- /dev/null
+++ b/compat/win32/pthread.c
@@ -0,0 +1,120 @@
+/*
+ * Copyright (C) 2009 Andrzej K. Haczewski <ahaczewski@gmail.com>
+ *
+ * DISCLAMER: The implementation is Git-specific, it is subset of original
+ * Pthreads API, without lots of other features that Git doesn't use.
+ * Git also makes sure that the passed arguments are valid, so there's
+ * no need for double-checking.
+ */
+
+#include "../../git-compat-util.h"
+#include "pthread.h"
+
+#include <errno.h>
+#include <limits.h>
+
+static unsigned __stdcall win32_start_routine(void *arg)
+{
+	pthread_t *thread = arg;
+	thread->arg = thread->start_routine(thread->arg);
+	return 0;
+}
+
+int pthread_create(pthread_t *thread, const void *unused,
+		   void *(*start_routine)(void*), void *arg)
+{
+	thread->arg = arg;
+	thread->start_routine = start_routine;
+	thread->handle = (HANDLE)
+		_beginthreadex(NULL, 0, win32_start_routine, thread, 0, NULL);
+
+	if (!thread->handle)
+		return errno;
+	else
+		return 0;
+}
+
+int win32_pthread_join(pthread_t *thread, void **value_ptr)
+{
+	DWORD result = WaitForSingleObject(thread->handle, INFINITE);
+	switch (result) {
+		case WAIT_OBJECT_0:
+			if (value_ptr)
+				*value_ptr = thread->arg;
+			return 0;
+		case WAIT_ABANDONED:
+			return EINVAL;
+		default:
+			return err_win_to_posix(GetLastError());
+	}
+}
+
+int pthread_cond_init(pthread_cond_t *cond, const void *unused)
+{
+	cond->waiters = 0;
+
+	InitializeCriticalSection(&cond->waiters_lock);
+
+	cond->sema = CreateSemaphore(NULL, 0, LONG_MAX, NULL);
+	if (!cond->sema)
+		die("CreateSemaphore() failed");
+	return 0;
+}
+
+int pthread_cond_destroy(pthread_cond_t *cond)
+{
+	CloseHandle(cond->sema);
+	cond->sema = NULL;
+
+	DeleteCriticalSection(&cond->waiters_lock);
+
+	return 0;
+}
+
+int pthread_cond_wait(pthread_cond_t *cond, CRITICAL_SECTION *mutex)
+{
+	/* serialize access to waiters count */
+	EnterCriticalSection(&cond->waiters_lock);
+	++cond->waiters;
+	LeaveCriticalSection(&cond->waiters_lock);
+
+	/*
+	 * Unlock external mutex and wait for signal.
+	 * NOTE: we've held mutex locked long enough to increment
+	 * waiters count above, so there's no problem with
+	 * leaving mutex unlocked before we wait on semaphore.
+	 */
+	LeaveCriticalSection(mutex);
+
+	/* let's wait - ignore return value */
+	WaitForSingleObject(cond->sema, INFINITE);
+
+	/* we're done waiting, so make sure we decrease waiters count */
+	EnterCriticalSection(&cond->waiters_lock);
+	--cond->waiters;
+	LeaveCriticalSection(&cond->waiters_lock);
+
+	/* lock external mutex again */
+	EnterCriticalSection(mutex);
+
+	return 0;
+}
+
+int pthread_cond_signal(pthread_cond_t *cond)
+{
+	int have_waiters;
+
+	/* serialize access to waiters count */
+	EnterCriticalSection(&cond->waiters_lock);
+	have_waiters = cond->waiters > 0;
+	LeaveCriticalSection(&cond->waiters_lock);
+
+	/*
+	 * Signal only when there are waiters
+	 */
+	if (have_waiters)
+		return ReleaseSemaphore(cond->sema, 1, NULL) ?
+			0 : err_win_to_posix(GetLastError());
+	else
+		return 0;
+}
diff --git a/compat/win32/pthread.h b/compat/win32/pthread.h
new file mode 100644
index 0000000..63293ad
--- /dev/null
+++ b/compat/win32/pthread.h
@@ -0,0 +1,68 @@
+/*
+ * Header used to adapt pthread-based POSIX code to Windows API threads.
+ *
+ * Copyright (C) 2009 Andrzej K. Haczewski <ahaczewski@gmail.com>
+ */
+
+#ifndef PTHREAD_H
+#define PTHREAD_H
+
+#ifndef WIN32_LEAN_AND_MEAN
+#define WIN32_LEAN_AND_MEAN
+#endif
+
+#include <windows.h>
+
+/*
+ * Defines that adapt Windows API threads to pthreads API
+ */
+#define pthread_mutex_t CRITICAL_SECTION
+
+#define pthread_mutex_init(a,b) InitializeCriticalSection((a))
+#define pthread_mutex_destroy(a) DeleteCriticalSection((a))
+#define pthread_mutex_lock EnterCriticalSection
+#define pthread_mutex_unlock LeaveCriticalSection
+
+/*
+ * Implement simple condition variable for Windows threads, based on ACE
+ * implementation.
+ *
+ * See original implementation: http://bit.ly/1vkDjo
+ * ACE homepage: http://www.cse.wustl.edu/~schmidt/ACE.html
+ * See also: http://www.cse.wustl.edu/~schmidt/win32-cv-1.html
+ */
+typedef struct {
+	LONG waiters;
+	CRITICAL_SECTION waiters_lock;
+	HANDLE sema;
+} pthread_cond_t;
+
+extern int pthread_cond_init(pthread_cond_t *cond, const void *unused);
+
+extern int pthread_cond_destroy(pthread_cond_t *cond);
+
+extern int pthread_cond_wait(pthread_cond_t *cond, CRITICAL_SECTION *mutex);
+
+extern int pthread_cond_signal(pthread_cond_t *cond);
+
+/*
+ * Simple thread creation implementation using pthread API
+ */
+typedef struct {
+	HANDLE handle;
+	void *(*start_routine)(void*);
+	void *arg;
+} pthread_t;
+
+extern int pthread_create(pthread_t *thread, const void *unused,
+			  void *(*start_routine)(void*), void *arg);
+
+/*
+ * To avoid the need of copying a struct, we use small macro wrapper to pass
+ * pointer to win32_pthread_join instead.
+ */
+#define pthread_join(a, b) win32_pthread_join(&(a), (b))
+
+extern int win32_pthread_join(pthread_t *thread, void **value_ptr);
+
+#endif /* PTHREAD_H */
-- 
1.6.5.2

^ permalink raw reply related

* Allowing push --dry-run through fetch url
From: Mike Hommey @ 2009-11-06  7:37 UTC (permalink / raw)
  To: git

Hi,

I am currently considering, when I'll get some time, to dive in the git
push code to allow push --dry-run without the rights to push.

We currently have two remote configuration items for urls:
remote.<name>.url and remote.<name>.pushurl. The latter is used when
pushing and the former when pulling/fetching.

The typical use of both at the same time is to put an authenticated
value for pushurl (ssh://, for example) and an anonymous one (git://,
for example) for url.

What has been annoying me lately is that git push --dry-run asks me
for the ssh key or password. I know I could be using an ssh-agent, but
that's not the point.

It would be interesting, to me at least, that git push --dry-run can do
its job through the git:// url instead of the ssh:// one. But for now,
all that does is telling me:
fatal: The remote end hung up unexpectedly
(probably because the git server doesn't allow pushes at all)

So, before I dive in, what would you think about such a feature?

There is one thing that bothers me, though, it's that --dry-run would,
in the end, not really be a dry-run anymore, and, for example, could
not be used to validate that the ssh setup is good without actually
pushing.

Cheers,

Mike

^ permalink raw reply

* Re: suggestions for local configs?
From: Johannes Sixt @ 2009-11-06  7:21 UTC (permalink / raw)
  To: Tim Rupp; +Cc: git
In-Reply-To: <4AF393EE.4030205@gmail.com>

Tim Rupp schrieb:
> I have a piece of software that has default and local configuration
> files. The default files ship with the tarball. The local files are
> copied over from the default folder during installation and can be
> modified for a particular install.

This is how *I* would organize it, but it is a bit overengineered if the
changes needed in the configuration are only minimal:

  ---o--o--X1--o--o--X2--o          master (aka upstream)
      \     \             \
       R-----M1------------M3       rename
        \     \             \
         Y-----M2------------M4     production

We need two branches in addition to master, which tracks upstream version.

1. The first branch *renames* the example configuration to the effective
configuration:

   git mv foo.conf.example foo.conf
   # commit R
   git commit -m "Use the example configuration"

It is important that *no change* is made to the configuration file, and it
furthermore assumes that the production configuration (foo.conf) does not
exist [*].

The point of this commit is that git can determine that the file was
renamed with 100% similarity.

2. The second branch is actually where the customization happens, this is
commit Y:

   git checkout production
   edit foo.conf
   # commit Y
   git commit -m "configuration changes needed in production"


Assume, upstream made changes to the configuration (commit X1). These
changes are merged in the following way:

1.  git checkout rename && git merge master

This creates M1. git detects that on our side the file was renamed, and
merges the upstream changes into the renamed file. Since we didn't modify
the file in R, there are no conflicts.

2.  git checkout production && git merge rename

This creates M2. This is a regular merge that integrates upstream changes
with the local changes made in Y, possibly with conflicts.

You repeat the procedure when upstream makes more changes to the example
configuration in X2. This time, git merge again detects that the file was
renamed with 100% similarity, and things work just like with M1 and M2.

[*] If there is a configuration file in addition to the example
configuration, another branch is needed that removes it.

-- Hannes

^ permalink raw reply

* Re: [PATCH] pack-objects: move thread autodetection closer to relevant code
From: Junio C Hamano @ 2009-11-06  7:20 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Junio C Hamano, Andrzej K. Haczewski, git, Johannes Sixt
In-Reply-To: <alpine.LFD.2.00.0911041623570.10340@xanadu.home>

Nicolas Pitre <nico@fluxnic.net> writes:

> Let's keep thread stuff close together if possible.  And in this case,
> this even reduces the #ifdef noise, and allows for skipping the
> autodetection altogether if delta search is not needed (like with a pure
> clone).

Nice.  The ll_find_delta() function itself will disappear and becomes the
single-threaded find_deltas() when THREADED_DELTA_SEARCH is not defined,
and the variable in question is used only inside the function anyway, so
this works beautifully.

Very nice.

^ permalink raw reply

* Re: [PATCH/RFC 0/3] teach fetch --prune
From: Junio C Hamano @ 2009-11-06  7:05 UTC (permalink / raw)
  To: Jay Soffian; +Cc: git
In-Reply-To: <1257484241-27219-1-git-send-email-jaysoffian@gmail.com>

Jay Soffian <jaysoffian@gmail.com> writes:

> This is just a start so I can get some feedback. Some things still missing:
>
> 1) "git remote prune <remote>" calls warn_dangling_symref(), but 
> "git fetch --prune" does not. I ran out of time tonight to refactor 
> warn_dangling_symref() to do something more intelligent than just spew to
> stdout (which doesn't get along with fetch, which spews to stderr...).
>
> 2) Perhaps "git remote update --prune" should be refactored to call 
> "git fetch --prune". If so, then fetch should gain a "--prune-only" option
> so that "git remote prune" can just call "got fetch --prune-only".
>
> 3) Perhaps add a config option for users who wish to prune by default.
>
> Thoughts, comments, flames?

I usually refrain from talking about multi-year long term plans, because I
do not have one, but in a longer term (across 1.7.0 boundary and beyond),
the general direction would be:

 - "fetch" will eventually prune by default; I expect we will have a
   configuration option "fetch.autoprune = yes" to allow early adopters to
   let it prune until 1.7.0, and in 1.7.0 we will change the default for
   the configuration variable to "yes", i.e. we prune unless the user
   explicitly declines with "fetch.autoprune = no".

 - In general, "remote" should go back to its roots of being the
   management interface to [remote "nick"] configuration section.  We
   should start planning to remove extra features that were piled on top
   of the original "remote definition management tool".  "update/prune"
   should have been the duty of "fetch" in the first place, but they were
   added to "remote" primarily because it was easier to do so ("remote"
   used to be script but "fetch" was already written in "C").

   This means two things:

   * "remote prune" that only prunes without updating remaining tracking
     will be largely become unnecessary [*1*], once we have a way to tell
     "fetch" to prune at the same time.  As soon as "fetch --prune"
     becomes available, "remote prune" should become an alias to it.  And
     "remote prune" itself should eventually be removed.

   * "remote update" that runs "fetch" for multiple remotes should be
     deprecated and eventually removed.  "remote update" is a band-aid
     that exists only because "fetch" started as a strange chimera between
     plumbing and Porcelain, and we did not want to add too much features
     to it.  "fetch" itself should learn to do the "from multiple places"
     part as a full-fledged Porcelain.

Of course, a removal of a subcommand ("remote update" and "remote prune")
will have to happen way after 1.7.0 boundary, but the above should be the
longer term direction.

Don't worry about keeping the "only prune without updating" misfeature. If
omission of it simplifies what you are trying to do, it is Ok if "git
prune" becomes a synonym to "git update --prune" aka "git fetch --prune"
and starts updating the tracking refs.

[Footnote]

*1* "remote prune" is a band-aid that exists only because "fetch"
currently has no-way to prune at the same time.

One could argue that a user may want to prune _only_ stale refs without
getting the state of remaining refs updated to his tracking refs.  That
certainly is _possible_ with "remote prune", but being possible and being
sensible are two different things.  

What is the reason the user does not want to update a tracking ref that
corresponds to a branch that remains at the remote, when he runs "prune
only" version if it existed?  It cannot be because the branch may have
been rewound and you may lose the only remaining history---the user is
actively asking to prune and is willing to lose the history of stale
branches.

In other words, once we have "fetch --prune" (either default or
optional---the important part is that there is a way to cause fetch to do
this), there is no sane reason to have a separate command that only prunes
without updating the remaining tracking refs.

^ permalink raw reply

* Re: [PATCH v2 09/13] Honour the refspec when updating refs after  import
From: Junio C Hamano @ 2009-11-06  6:05 UTC (permalink / raw)
  To: Daniel Barkalow
  Cc: Sverre Rabbelier, Git List, Johannes Schindelin, Johan Herland,
	Junio C Hamano
In-Reply-To: <alpine.LNX.2.00.0911051929340.14365@iabervon.org>

Daniel Barkalow <barkalow@iabervon.org> writes:

> On Fri, 6 Nov 2009, Sverre Rabbelier wrote:
>
>> > I don't know why Junio squashed your
>> > changes into my patch, particularly when I disagreed with those changes.
>> 
>> Junio didn't squash anything, it's just that pu still contains v4 of
>> the series, in which I had squashed my changes in.
>
> Oh, okay. You probably shouldn't squash un-acked changes into other 
> people's patches when taking over their series.

Thanks for having this exchange.  You are right, and I should have more
explicitly explained what I was doing when I replaced two series (yours
and Johan's cvs series) with Sverre's consolidated re-roll.

^ permalink raw reply

* [PATCH/RFC 3/3] builtin-fetch: add --prune option
From: Jay Soffian @ 2009-11-06  5:10 UTC (permalink / raw)
  To: git; +Cc: Jay Soffian
In-Reply-To: <1257484241-27219-3-git-send-email-jaysoffian@gmail.com>

Teach fetch --prune, as an alternative to git remote prune.
---
 builtin-fetch.c |   22 +++++++++++++++++++++-
 1 files changed, 21 insertions(+), 1 deletions(-)

diff --git a/builtin-fetch.c b/builtin-fetch.c
index 985b36b..e8a5b9b 100644
--- a/builtin-fetch.c
+++ b/builtin-fetch.c
@@ -23,7 +23,7 @@ enum {
 	TAGS_SET = 2
 };
 
-static int append, dry_run, force, keep, update_head_ok, verbosity;
+static int append, dry_run, force, keep, prune, update_head_ok, verbosity;
 static int tags = TAGS_DEFAULT;
 static const char *depth;
 static const char *upload_pack;
@@ -42,6 +42,8 @@ static struct option builtin_fetch_options[] = {
 		    "fetch all tags and associated objects", TAGS_SET),
 	OPT_SET_INT('n', NULL, &tags,
 		    "do not fetch all tags (--no-tags)", TAGS_UNSET),
+	OPT_BOOLEAN('p', "prune", &prune,
+		    "prune tracking branches no longer on remote"),
 	OPT_BOOLEAN(0, "dry-run", &dry_run,
 		    "dry run"),
 	OPT_BOOLEAN('k', "keep", &keep, "keep downloaded pack"),
@@ -489,6 +491,22 @@ static int fetch_refs(struct transport *transport, struct ref *ref_map)
 	return ret;
 }
 
+static int prune_refs(struct transport *transport, struct ref *ref_map)
+{
+	int result = 0;
+	struct ref *ref, *stale_refs = get_stale_heads(transport->remote, ref_map);
+	for (ref = stale_refs; ref; ref = ref->next) {
+		if (!dry_run)
+			result |= delete_ref(ref->name, NULL, 0);
+		if (verbosity >= 0)
+			fprintf(stderr, " x %-*s %-*s -> %s\n",
+				SUMMARY_WIDTH, "[deleted]",
+				REFCOL_WIDTH, "(none)", prettify_refname(ref->name));
+	}
+	free_refs(stale_refs);
+	return 0;
+}
+
 static int add_existing(const char *refname, const unsigned char *sha1,
 			int flag, void *cbdata)
 {
@@ -613,6 +631,8 @@ static int do_fetch(struct transport *transport,
 		free_refs(ref_map);
 		return 1;
 	}
+	if (prune)
+		prune_refs(transport, ref_map);
 	free_refs(ref_map);
 
 	/* if neither --no-tags nor --tags was specified, do automated tag
-- 
1.6.4.2

^ permalink raw reply related

* [PATCH/RFC 2/3] builtin-fetch: add --dry-run option
From: Jay Soffian @ 2009-11-06  5:10 UTC (permalink / raw)
  To: git; +Cc: Jay Soffian
In-Reply-To: <1257484241-27219-2-git-send-email-jaysoffian@gmail.com>

Teach fetch --dry-run. Unfortunately OPT__DRY_RUN() cannot be used as fetch
already uses "-n" for something else.
---
 builtin-fetch.c |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/builtin-fetch.c b/builtin-fetch.c
index cb48c57..985b36b 100644
--- a/builtin-fetch.c
+++ b/builtin-fetch.c
@@ -23,7 +23,7 @@ enum {
 	TAGS_SET = 2
 };
 
-static int append, force, keep, update_head_ok, verbosity;
+static int append, dry_run, force, keep, update_head_ok, verbosity;
 static int tags = TAGS_DEFAULT;
 static const char *depth;
 static const char *upload_pack;
@@ -42,6 +42,8 @@ static struct option builtin_fetch_options[] = {
 		    "fetch all tags and associated objects", TAGS_SET),
 	OPT_SET_INT('n', NULL, &tags,
 		    "do not fetch all tags (--no-tags)", TAGS_UNSET),
+	OPT_BOOLEAN(0, "dry-run", &dry_run,
+		    "dry run"),
 	OPT_BOOLEAN('k', "keep", &keep, "keep downloaded pack"),
 	OPT_BOOLEAN('u', "update-head-ok", &update_head_ok,
 		    "allow updating of HEAD ref"),
@@ -178,6 +180,8 @@ static int s_update_ref(const char *action,
 	char *rla = getenv("GIT_REFLOG_ACTION");
 	static struct ref_lock *lock;
 
+	if (dry_run)
+		return 0;
 	if (!rla)
 		rla = default_rla.buf;
 	snprintf(msg, sizeof(msg), "%s: %s", rla, action);
@@ -303,7 +307,7 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
 	char note[1024];
 	const char *what, *kind;
 	struct ref *rm;
-	char *url, *filename = git_path("FETCH_HEAD");
+	char *url, *filename = dry_run ? "/dev/null" : git_path("FETCH_HEAD");
 
 	fp = fopen(filename, "a");
 	if (!fp)
@@ -586,7 +590,7 @@ static int do_fetch(struct transport *transport,
 		die("Don't know how to fetch from %s", transport->url);
 
 	/* if not appending, truncate FETCH_HEAD */
-	if (!append) {
+	if (!append && !dry_run) {
 		char *filename = git_path("FETCH_HEAD");
 		FILE *fp = fopen(filename, "w");
 		if (!fp)
-- 
1.6.4.2

^ permalink raw reply related

* [PATCH/RFC 1/3] remote: refactor some logic into get_stale_heads()
From: Jay Soffian @ 2009-11-06  5:10 UTC (permalink / raw)
  To: git; +Cc: Jay Soffian
In-Reply-To: <1257484241-27219-1-git-send-email-jaysoffian@gmail.com>

Move the logic in builtin-remote.c which determines which local heads are stale
to remote.c so it can be used by other builtins.
---
 builtin-remote.c |   32 ++++++++------------------------
 remote.c         |   40 ++++++++++++++++++++++++++++++++++++++++
 remote.h         |    3 +++
 3 files changed, 51 insertions(+), 24 deletions(-)

diff --git a/builtin-remote.c b/builtin-remote.c
index 0777dd7..b48267b 100644
--- a/builtin-remote.c
+++ b/builtin-remote.c
@@ -227,32 +227,10 @@ struct ref_states {
 	int queried;
 };
 
-static int handle_one_branch(const char *refname,
-	const unsigned char *sha1, int flags, void *cb_data)
-{
-	struct ref_states *states = cb_data;
-	struct refspec refspec;
-
-	memset(&refspec, 0, sizeof(refspec));
-	refspec.dst = (char *)refname;
-	if (!remote_find_tracking(states->remote, &refspec)) {
-		struct string_list_item *item;
-		const char *name = abbrev_branch(refspec.src);
-		/* symbolic refs pointing nowhere were handled already */
-		if ((flags & REF_ISSYMREF) ||
-		    string_list_has_string(&states->tracked, name) ||
-		    string_list_has_string(&states->new, name))
-			return 0;
-		item = string_list_append(name, &states->stale);
-		item->util = xstrdup(refname);
-	}
-	return 0;
-}
-
 static int get_ref_states(const struct ref *remote_refs, struct ref_states *states)
 {
 	struct ref *fetch_map = NULL, **tail = &fetch_map;
-	struct ref *ref;
+	struct ref *ref, *stale_refs;
 	int i;
 
 	for (i = 0; i < states->remote->fetch_refspec_nr; i++)
@@ -268,11 +246,17 @@ static int get_ref_states(const struct ref *remote_refs, struct ref_states *stat
 		else
 			string_list_append(abbrev_branch(ref->name), &states->tracked);
 	}
+	stale_refs = get_stale_heads(states->remote, fetch_map);
+	for (ref = stale_refs; ref; ref = ref->next) {
+		struct string_list_item *item =
+			string_list_append(abbrev_branch(ref->name), &states->stale);
+		item->util = xstrdup(ref->name);
+	}
+	free_refs(stale_refs);
 	free_refs(fetch_map);
 
 	sort_string_list(&states->new);
 	sort_string_list(&states->tracked);
-	for_each_ref(handle_one_branch, states);
 	sort_string_list(&states->stale);
 
 	return 0;
diff --git a/remote.c b/remote.c
index 73d33f2..ee48b49 100644
--- a/remote.c
+++ b/remote.c
@@ -6,6 +6,7 @@
 #include "revision.h"
 #include "dir.h"
 #include "tag.h"
+#include "string-list.h"
 
 static struct refspec s_tag_refspec = {
 	0,
@@ -1586,3 +1587,42 @@ struct ref *guess_remote_head(const struct ref *head,
 
 	return list;
 }
+
+struct stale_heads_info {
+	struct remote *remote;
+	struct string_list *ref_names;
+	struct ref **stale_refs_tail;
+};
+
+static int get_stale_heads_cb(const char *refname,
+	const unsigned char *sha1, int flags, void *cb_data)
+{
+	struct stale_heads_info *info = cb_data;
+	struct refspec refspec;
+	memset(&refspec, 0, sizeof(refspec));
+	refspec.dst = (char *)refname;
+	if (!remote_find_tracking(info->remote, &refspec)) {
+		if (!((flags & REF_ISSYMREF) ||
+		    string_list_has_string(info->ref_names, refspec.src))) {
+			struct ref *ref = make_linked_ref(refname, &info->stale_refs_tail);
+			hashcpy(ref->new_sha1, sha1);
+		}
+	}
+	return 0;
+}
+
+struct ref *get_stale_heads(struct remote *remote, struct ref *fetch_map)
+{
+	struct ref *ref, *stale_refs = NULL;
+	struct string_list ref_names = { NULL, 0, 0, 0 };
+	struct stale_heads_info info;
+	info.remote = remote;
+	info.ref_names = &ref_names;
+	info.stale_refs_tail = &stale_refs;
+	for (ref = fetch_map; ref; ref = ref->next)
+		string_list_append(ref->name, &ref_names);
+	sort_string_list(&ref_names);
+	for_each_ref(get_stale_heads_cb, &info);
+	string_list_clear(&ref_names, 0);
+	return stale_refs;
+}
diff --git a/remote.h b/remote.h
index 5db8420..d0aba81 100644
--- a/remote.h
+++ b/remote.h
@@ -154,4 +154,7 @@ struct ref *guess_remote_head(const struct ref *head,
 			      const struct ref *refs,
 			      int all);
 
+/* Return refs which no longer exist on remote */
+struct ref *get_stale_heads(struct remote *remote, struct ref *fetch_map);
+
 #endif
-- 
1.6.4.2

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox