Git development
 help / color / mirror / Atom feed
* Re: [PATCH] MSVC: Windows-native implementation for subset of  Pthreads API
From: Andrzej K. Haczewski @ 2009-11-04 21:16 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: git, Johannes Sixt
In-Reply-To: <alpine.LFD.2.00.0911041247250.10340@xanadu.home>

2009/11/4 Nicolas Pitre <nico@fluxnic.net>:
> On Wed, 4 Nov 2009, Andrzej K. Haczewski wrote:
>
>> +     NO_STATIC_PTHREADS_INIT = YesPlease
>
> Let's not go that route please.  If Windows can't get away without
> runtime initializations then let's use them on all platforms.  There is
> no gain in exploding the code path combinations here wrt testing
> coverage.
>

I don't like that approach either, but I was frighten of Junio being
anal about static inits ;).

Let's make it clear: has anyone have any objections that I add
explicit initialization of mutexes and condition variables for POSIX
also?

>> +static THREAD_RET_TYPE threaded_find_deltas(void *arg)
>
> Why can't you just cast the function pointer in your pthread_create
> wrapper instead?  No one cares about the returned value anyway.

Because of calling convention - I'd have to cast cdecl function as
stdcall function, which would change the function call clean up (in
cdecl caller is responsible for unwinding stack, stdcall callee; the
effect - double stack unwinding).

>> @@ -2327,6 +2354,8 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
>>  #ifdef THREADED_DELTA_SEARCH
>>       if (!delta_search_threads)      /* --threads=0 means autodetect */
>>               delta_search_threads = online_cpus();
>> +
>> +     init_threaded_delta_search();
>
> What about doing this at the beginning of ll_find_deltas() instead?
> And similarly for cleanup_threaded_delta_search(): call it right before
> leaving ll_find_deltas().  This way thread issues would remain more
> localized.  In fact I'd move the whole thing above in ll_find_deltas()
> as well (separately from this patch though).

Sounds sensible, but I'd wait for the NO_STATIC_PTHREADS_INIT verdict.

--
Andrzej

^ permalink raw reply

* Re: [PATCH] MSVC: port pthread code to native Windows threads
From: Nicolas Pitre @ 2009-11-04 21:17 UTC (permalink / raw)
  To: Daniel Barkalow; +Cc: Andrzej K. Haczewski, Johannes Sixt, git
In-Reply-To: <alpine.LNX.2.00.0911041406101.14365@iabervon.org>

On Wed, 4 Nov 2009, Daniel Barkalow wrote:

> On Wed, 4 Nov 2009, Andrzej K. Haczewski wrote:
> 
> > 2009/11/4 Johannes Sixt <j.sixt@viscovery.net>:
> > >
> > > You are right. But #ifdef THREADED_DELTA_SEARCH is about a "generic"
> > > property of the code and is already used elsewhere in the file, whereas
> > > #ifdef WIN32 would be new and is is about platform differences.
> > >
> > > Anyway, we would have to see what Junio says about the new function calls,
> > > because he's usually quite anal when it comes to added code vs. static
> > > initialization. ;)
> > 
> > I could do it with wrappers for pthread_mutex_lock and _unlock and
> > lazy init there plus lazy init cond var in cond_wait and _signal, that
> > way it could be done without any additional code in the first #ifdef.
> > But I don't see any simple solution for working around
> > deinitialization, that's why I'd leave non-static initialization. Let
> > me put some touchups and resubmit for another round.
> 
> Is it actually necessary to deinitialize? Since the variables are static 
> and therefore can't leak, and would presumably not need to be 
> reinitialized differently if they were used again, I think they should be 
> able to just stay. If Windows is unhappy about processes still having 
> locks initialized at exit, I suppose we could go through and destroy all 
> our mutexes and conds at cleanup time. Pthreads does have the appropriate 
> functions, and it would be correct to use them, although unnecessary.

Lazy initialization would probably turn up to be more expensive 
(checking a flag on each usage) than unconditionally initializing them 
once.  Remember that those are used at least once per object meaning a 
lot.

And I much prefer having runtime initialization for both Unix and 
Windows than having separate paths on each platform potentially hiding 
different bugs.  And given that on Unix you can statically initialize 
those, then a runtime initialization is certainly not going to be _that_ 
costly.

And while at it, we might just deinitialize them as soon as we're done 
with them saving resources (on Unix that might turn up to be a no op 
anyway) which is why I suggested doing this within ll_find_deltas() 
directly.


Nicolas

^ permalink raw reply

* Re: [PATCH v2 09/13] Honour the refspec when updating refs after import
From: Daniel Barkalow @ 2009-11-04 21:20 UTC (permalink / raw)
  To: Sverre Rabbelier; +Cc: Git List, Johannes Schindelin, Johan Herland
In-Reply-To: <1257364098-1685-10-git-send-email-srabbelier@gmail.com>

On Wed, 4 Nov 2009, Sverre Rabbelier wrote:

> This way the helper can write to 'refs/remotes/origin/*' instead of
> writing to 'refs/heads/*'.
> 
> Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
> ---
> 
>   Daniel, does this look good to you?
> 
>   Let's assume the following:
>   * list: only HEAD is a symref (to refs/heads/<branch>), all other
>           refs are '? refs/heads/<branch>'.
>   * import: the helper creates branches under refs/remotes/<alias>/*
>             (since the refspec code is not yet in that would allow
>              it to create them under refs/<vcs>/<alias>/*)
>
>   Now when updating the refs we do the following:
>   transport-helper.c:145 "read_ref(posn->name, posn->old_sha1);"
> 
>   The value of posn->name looks like "refs/heads/<branch>". So what
>   does this lookup do? It tries to lookup the new value of the ref
>   _where it will be created_, this fails of course, since the ref
>   currently only exists as "refs/remotes/origin/<branch>". So, we
>   should instead be doing a lookup using posn->peer_ref->name, and
>   not do a lookup at all if it posn->peer_ref is not set (which
>   should not occur as we are passed only wanted peer refs).

That's not true for "git pull <url> <branch>"; we do want the remote ref, 
but it doesn't have a local peer. I think going straight to the refspec 
command is the right answer.

	-Daniel
*This .sig left intentionally blank*

^ permalink raw reply

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

Heya,

On Wed, Nov 4, 2009 at 22:20, Daniel Barkalow <barkalow@iabervon.org> wrote:
> That's not true for "git pull <url> <branch>"; we do want the remote ref,
> but it doesn't have a local peer. I think going straight to the refspec
> command is the right answer.

Can you clarity what you mean with "the refspec command"?

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* Re: [RFC/PATCH 0/3] use '--bisect-refs' as bisect rev machinery option
From: Christian Couder @ 2009-11-04 21:26 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Linus Torvalds, git
In-Reply-To: <7veioegko3.fsf@alter.siamese.dyndns.org>

On Wednesday 04 November 2009, Junio C Hamano wrote:
> Linus Torvalds <torvalds@linux-foundation.org> writes:
> > On Wed, 4 Nov 2009, Linus Torvalds wrote:
> >> Yes, it is a behavioral change, but is it a bad one?
> >
> > .. and perhaps we could introduce --bisect-refs as the "old behavior"
> > of '--bisect' to git rev-list?
> >
> > I kind of suspect that it is unlikely that people are using 'git
> > rev-list --bisect' while _inside_ a bisection, but then wanting to
> > bisect someting that is outside the set of commits we're currently
> > actively bisecting.
> >
> > But maybe I'm wrong.
>
> Maybe I'm wrong too, but I do not think that is plausible that people are
> doing nested bisection that way.  It is probably a useful thing to do,
> but if somebody has thought of doing so we would have at least seen a
> request to add a way to tell "git-bisect" what names to use to record the
> good/bad set of commits under to make their implementation easier.  I
> haven't, and I take it an indication that it is very implausible that
> such scripts by people exist to be broken by this change.
>
> I was more worried about people who reinvented the wheel and are using
> their own git-bisect.sh derivative.  It probably was forked from the
> version that still used 'git rev-list --bisect", manually feeding good
> and bad set of commits to it from the command line.  But then what they
> are feeding would be the same as the new --bisect option implicitly gives
> them anyway, so there won't be a regression either.

I don't remember exactly when, but at one time some people talked about 
parallelizing bisection. The idea was that if it takes a long time to test 
one commit but you can test many commits at the same time (for example on 
different machines), then you can bisect faster by testing at the same time 
the current commit that git bisect checked out for you and for example the 
commit that git bisect would give you if the current commit is bad and the 
commit it would give you if the current commit is good.

So to do that you would use "git bisect start ..." and then you could use:

$ git rev-list --bisect HEAD --not $GOOD_COMMITS

to get the commit that you would have to test if the current commit is bad 
and:

$ git rev-list --bisect  $BAD --not $GOOD_COMMITS HEAD

to get the commit that you would have to test if the current commit is good.

Ok, perhaps nobody is doing that.

And yes I agree that it would be probably better to have --bisect be the 
name of the revision machinery option and --bisect-refs or perhaps another 
name like --bisect-compute be the name of the special option to "git 
rev-list".

But perhaps we can introduce --bisect-compute to do the same thing 
that --bisect currently does and deprecate --bisect with a warning and then 
a few versions later remove it and after a few more versions 
introduce --bisect to do the same as --bisect-refs.

Best regards,
Christian.

^ permalink raw reply

* Re: [PATCH v2 09/13] Honour the refspec when updating refs after  import
From: Daniel Barkalow @ 2009-11-04 21:30 UTC (permalink / raw)
  To: Sverre Rabbelier; +Cc: Git List, Johannes Schindelin, Johan Herland
In-Reply-To: <fabb9a1e0911041321i1ccec898r53ddafb9405c6331@mail.gmail.com>

On Wed, 4 Nov 2009, Sverre Rabbelier wrote:

> Heya,
> 
> On Wed, Nov 4, 2009 at 22:20, Daniel Barkalow <barkalow@iabervon.org> wrote:
> > That's not true for "git pull <url> <branch>"; we do want the remote ref,
> > but it doesn't have a local peer. I think going straight to the refspec
> > command is the right answer.
> 
> Can you clarity what you mean with "the refspec command"?

Whatever it is that lets the helper tell the transport code where in the 
helper's private namespace to look for refs. I'd been thinking the helper 
would advertize the "refspec" capability, and the transport code would 
call the "refspec" command in order to get the helper to report that; but 
then I actually only said that the helper reports refspec, and not 
proposed a name for the command.

	-Daniel
*This .sig left intentionally blank*

^ permalink raw reply

* [PATCH] pack-objects: move thread autodetection closer to relevant code
From: Nicolas Pitre @ 2009-11-04 21:32 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Andrzej K. Haczewski, git, Johannes Sixt
In-Reply-To: <16cee31f0911041316n20fc9f12s6595dadc813d8f46@mail.gmail.com>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1920 bytes --]

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).

Signed-off-by: Nicolas Pitre <nico@fluxnic.net>
---

> 2009/11/4 Nicolas Pitre <nico@fluxnic.net>:
> >> @@ -2327,6 +2354,8 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
> >>  #ifdef THREADED_DELTA_SEARCH
> >>       if (!delta_search_threads)      /* --threads=0 means autodetect */
> >>               delta_search_threads = online_cpus();
> >> +
> >> +     init_threaded_delta_search();
> >
> > What about doing this at the beginning of ll_find_deltas() instead?
> > And similarly for cleanup_threaded_delta_search(): call it right before
> > leaving ll_find_deltas().  This way thread issues would remain more
> > localized.  In fact I'd move the whole thing above in ll_find_deltas()
> > as well (separately from this patch though).

So here it is.

diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 02f9246..4c91e94 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -1629,6 +1629,8 @@ static void ll_find_deltas(struct object_entry **list, unsigned list_size,
 	struct thread_params *p;
 	int i, ret, active_threads = 0;
 
+	if (!delta_search_threads)	/* --threads=0 means autodetect */
+		delta_search_threads = online_cpus();
 	if (delta_search_threads <= 1) {
 		find_deltas(list, &list_size, window, depth, processed);
 		return;
@@ -2324,11 +2326,6 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
 	if (keep_unreachable && unpack_unreachable)
 		die("--keep-unreachable and --unpack-unreachable are incompatible.");
 
-#ifdef THREADED_DELTA_SEARCH
-	if (!delta_search_threads)	/* --threads=0 means autodetect */
-		delta_search_threads = online_cpus();
-#endif
-
 	prepare_packed_git();
 
 	if (progress)

^ permalink raw reply related

* Re: [PATCH v2 11/13] Allow helpers to request the path to the .git directory
From: Daniel Barkalow @ 2009-11-04 21:35 UTC (permalink / raw)
  To: Sverre Rabbelier; +Cc: Git List, Johannes Schindelin, Johan Herland
In-Reply-To: <1257364098-1685-12-git-send-email-srabbelier@gmail.com>

On Wed, 4 Nov 2009, Sverre Rabbelier wrote:

> The 'gitdir' capability is reported by the remote helper if it
> requires the location of the .git directory. The location of the .git
> directory can then be used by the helper to store status files even
> when the current directory is not a git repository (such as is the
> case when cloning).
> 
> The location of the .git dir is specified as an absolute path.

I thought we cd into the repository in the middle of clone somewhere, 
before running stuff. In any case, I think it would be good to have 
something like that, but I think maybe we should tell it where it can put 
its status files, rather than telling it where the .git dir is. It would 
also be nice if this is the absolute path that gfi will base a relative 
path for the "marks" options on when importing.

	-Daniel
*This .sig left intentionally blank*

^ permalink raw reply

* Re: [PATCH] MSVC: Windows-native implementation for subset of  Pthreads API
From: Erik Faye-Lund @ 2009-11-04 21:41 UTC (permalink / raw)
  To: Andrzej K. Haczewski; +Cc: Nicolas Pitre, git, Johannes Sixt
In-Reply-To: <16cee31f0911041316n20fc9f12s6595dadc813d8f46@mail.gmail.com>

On Wed, Nov 4, 2009 at 10:16 PM, Andrzej K. Haczewski
<ahaczewski@gmail.com> wrote:
>>> +static THREAD_RET_TYPE threaded_find_deltas(void *arg)
>>
>> Why can't you just cast the function pointer in your pthread_create
>> wrapper instead?  No one cares about the returned value anyway.
>
> Because of calling convention - I'd have to cast cdecl function as
> stdcall function, which would change the function call clean up (in
> cdecl caller is responsible for unwinding stack, stdcall callee; the
> effect - double stack unwinding).
>

Couldn't the windows version of pthread_create have a wrapper
function, that corrected the calling convention, much like the
function run_thread that start_async in run-command.c has?

-- 
Erik "kusma" Faye-Lund

^ permalink raw reply

* Re: BUG: git rebase -i -p silently looses commits
From: Greg Price @ 2009-11-04 21:46 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Constantine Plotnikov, git
In-Reply-To: <alpine.DEB.1.00.0911021832530.2479@felix-maschine>

On Mon, 2 Nov 2009, Johannes Schindelin wrote:
> Having said that, I worked for some time on fixing this issue, and I 
> actually run a version of rebase -i -p here that allows reordering 
> commits, but it is far from stable (and due to GSoC and day-job 
> obligations, I had no time to work on it in months).

I'm interested in this topic too.  Some weeks ago I took your
rebase-i-p branch from January and rebased it onto the latest release;
it's at
  git://repo.or.cz/git/price.git rebase-i-p
and now based on v1.6.5.2.  I fixed a few bugs and added a feature,
and it's the version I run day to day.

Constantine and others interested in reordering commits with -p,
you're welcome to pull and build this version and try it out.  It
mostly solves my problems, and maybe it will solve yours.  Be warned
it does have bugs, and also be warned that I may rewind and rebase
that branch.  I'd be glad to hear about bugs you see, though.

Dscho, do you have a TODO written somewhere of what work you're aware
the topic still needs?  I plan to continue spending a little time
working on it, and I have my own list but it'd be good to compare
it with yours.

Cheers,
Greg


PS - I'm open to suggestions on the workflow for how to develop a
topic branch like this.  Some rewinding seems necessary as half-baked
patches get finished, etc, but if Dscho or someone else finds time to
work on it too, then the rewinding gets in the way of pull/push
collaboration.


PPS - For those just scanning along, the shortlog so far:

Greg Price (6):
      rebase -i -p: honor -s
      rebase -i -p: get full message from original merge commit
      rebase -i -p: always merge --no-ff
      rebase -i: Add the "ref" command
      rebase -i -p: Preserve author information on merges.
      [broken] rebase -i: implement pause

Johannes Schindelin (19):
      Some of Dscho's tools
      debug settings in Makefile
      Make CFLAGS more strict
      rebase -i --root: simplify code
      rebase -i: make pick_one() safer
      rebase -i -p: add helper parse_commit() to find rewritten commits
      rebase -i: add the "goto" command
      rebase -i -p: add a helper to add mappings for rewritten commits
      rebase -i -p: Add the "merge" command
      rebase -i: make sure that the commands record the rewritten commits
      rebase -i: move the code to write the rebase script into generate_script()
      rebase -i: let generate_script output to stdout
      rebase -i -p: refactor the preparation for -p into its own function
      rebase -i -p: use patch-id directly to determine the dropped commits
      rebase tests' fake-editor.sh: allow debugging with DEBUG_EDIT
      rebase's fake-editor: prepare for "goto" and "merge" commands
      rebase -i -p: generate a script using "goto" and "merge"
      TODO
      Make some tests in t3412 a little bit stricter

^ permalink raw reply

* Re: [PATCH] MSVC: Windows-native implementation for subset of Pthreads API
From: Nicolas Pitre @ 2009-11-04 21:52 UTC (permalink / raw)
  To: Andrzej K. Haczewski; +Cc: git, Johannes Sixt
In-Reply-To: <16cee31f0911041316n20fc9f12s6595dadc813d8f46@mail.gmail.com>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 849 bytes --]

On Wed, 4 Nov 2009, Andrzej K. Haczewski wrote:

> 2009/11/4 Nicolas Pitre <nico@fluxnic.net>:
> > On Wed, 4 Nov 2009, Andrzej K. Haczewski wrote:
> >
> >> +     NO_STATIC_PTHREADS_INIT = YesPlease
> >
> > Let's not go that route please.  If Windows can't get away without
> > runtime initializations then let's use them on all platforms.  There is
> > no gain in exploding the code path combinations here wrt testing
> > coverage.
> >
> 
> I don't like that approach either, but I was frighten of Junio being
> anal about static inits ;).

I think the alternative is much worse than loosing those static inits.

> Let's make it clear: has anyone have any objections that I add
> explicit initialization of mutexes and condition variables for POSIX
> also?

Please do it and if anyone finds a problem with it then we'll start from 
there.


Nicolas

^ permalink raw reply

* Re: [RFC/PATCH 0/3] use '--bisect-refs' as bisect rev machinery option
From: Junio C Hamano @ 2009-11-04 21:52 UTC (permalink / raw)
  To: Christian Couder; +Cc: Linus Torvalds, git
In-Reply-To: <200911042226.25599.chriscool@tuxfamily.org>

Christian Couder <chriscool@tuxfamily.org> writes:

> So to do that you would use "git bisect start ..." and then you could use:
>
> $ git rev-list --bisect HEAD --not $GOOD_COMMITS
>
> to get the commit that you would have to test if the current commit is bad 
> and:
>
> $ git rev-list --bisect  $BAD --not $GOOD_COMMITS HEAD
>
> to get the commit that you would have to test if the current commit is good.

Even in that case, the problem is still about narrowing the set of the
current bisection graph.  If --bisect option implicitly grabs good and bad
defined in the refspace like Linus's patch does, it will give you the same
behaviour of the above two commands, no?

^ permalink raw reply

* Re: [PATCH] More precise description of 'git describe --abbrev'
From: Gisle Aas @ 2009-11-04 22:04 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vmy38im51.fsf@alter.siamese.dyndns.org>

[-- Attachment #1: Type: text/plain, Size: 603 bytes --]

Unfortunately there was still a grammar typo in that patch:

--- a/Documentation/git-describe.txt
+++ b/Documentation/git-describe.txt
@@ -120,7 +120,7 @@ closest tagname without any suffix:
        tags/v1.0.0

 Note that the suffix you get if you type these commands today may be
-longer than what Linus saw above when he ran this command, as your
+longer than what Linus saw above when he ran these commands, as your
 git repository may have new commits whose object names begin with
 975b that did not exist back then, and "-g975b" suffix alone may not
 be sufficient to disambiguate these commits.

[-- Attachment #2: 0001-Fix-documentation-grammar-typo.patch --]
[-- Type: application/octet-stream, Size: 1029 bytes --]

From 8910eed48f0c4c53af1b74d0016ef772bdce5689 Mon Sep 17 00:00:00 2001
From: Gisle Aas <gisle@aas.no>
Date: Wed, 4 Nov 2009 22:57:46 +0100
Subject: [PATCH] Fix documentation grammar typo

Introduced in commit 492cf3f72f9d8...

Signed-off-by: Gisle Aas <gisle@aas.no>
---
 Documentation/git-describe.txt |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-describe.txt b/Documentation/git-describe.txt
index e9dbca7..2f97916 100644
--- a/Documentation/git-describe.txt
+++ b/Documentation/git-describe.txt
@@ -120,7 +120,7 @@ closest tagname without any suffix:
 	tags/v1.0.0
 
 Note that the suffix you get if you type these commands today may be
-longer than what Linus saw above when he ran this command, as your
+longer than what Linus saw above when he ran these commands, as your
 git repository may have new commits whose object names begin with
 975b that did not exist back then, and "-g975b" suffix alone may not
 be sufficient to disambiguate these commits.
-- 
1.6.2.95.g934f7


^ permalink raw reply related

* Re: [PATCH v2 11/13] Allow helpers to request the path to the .git  directory
From: Sverre Rabbelier @ 2009-11-04 22:06 UTC (permalink / raw)
  To: Daniel Barkalow; +Cc: Git List, Johannes Schindelin, Johan Herland
In-Reply-To: <alpine.LNX.2.00.0911041621400.14365@iabervon.org>

Heya,

On Wed, Nov 4, 2009 at 22:35, Daniel Barkalow <barkalow@iabervon.org> wrote:
> I thought we cd into the repository in the middle of clone somewhere,
> before running stuff.

It does, but not soon enough, that is, not before I needed it in my helper.

> In any case, I think it would be good to have
> something like that, but I think maybe we should tell it where it can put
> its status files, rather than telling it where the .git dir is.

Yes, that would probably be a good idea, .git/info/remote-<vcs>/<alias> perhaps?

> It would
> also be nice if this is the absolute path that gfi will base a relative
> path for the "marks" options on when importing.

The marks option has been dumped in favor of 'feature import-marks='
and 'feature export-marks=' which will be in the reroll of
sr/gfi-options.

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* Re: [PATCH] MSVC: port pthread code to native Windows threads
From: Daniel Barkalow @ 2009-11-04 22:22 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Andrzej K. Haczewski, Johannes Sixt, git
In-Reply-To: <alpine.LFD.2.00.0911041607250.10340@xanadu.home>

On Wed, 4 Nov 2009, Nicolas Pitre wrote:

> On Wed, 4 Nov 2009, Daniel Barkalow wrote:
> 
> > On Wed, 4 Nov 2009, Andrzej K. Haczewski wrote:
> > 
> > > 2009/11/4 Johannes Sixt <j.sixt@viscovery.net>:
> > > >
> > > > You are right. But #ifdef THREADED_DELTA_SEARCH is about a "generic"
> > > > property of the code and is already used elsewhere in the file, whereas
> > > > #ifdef WIN32 would be new and is is about platform differences.
> > > >
> > > > Anyway, we would have to see what Junio says about the new function calls,
> > > > because he's usually quite anal when it comes to added code vs. static
> > > > initialization. ;)
> > > 
> > > I could do it with wrappers for pthread_mutex_lock and _unlock and
> > > lazy init there plus lazy init cond var in cond_wait and _signal, that
> > > way it could be done without any additional code in the first #ifdef.
> > > But I don't see any simple solution for working around
> > > deinitialization, that's why I'd leave non-static initialization. Let
> > > me put some touchups and resubmit for another round.
> > 
> > Is it actually necessary to deinitialize? Since the variables are static 
> > and therefore can't leak, and would presumably not need to be 
> > reinitialized differently if they were used again, I think they should be 
> > able to just stay. If Windows is unhappy about processes still having 
> > locks initialized at exit, I suppose we could go through and destroy all 
> > our mutexes and conds at cleanup time. Pthreads does have the appropriate 
> > functions, and it would be correct to use them, although unnecessary.
> 
> Lazy initialization would probably turn up to be more expensive 
> (checking a flag on each usage) than unconditionally initializing them 
> once.  Remember that those are used at least once per object meaning a 
> lot.

Meh, checking a flag on the same cache line as the lock you're about to 
take can't be a big incremental cost, especially if it's actually checking 
whether some sort of cookie is non-zero before doing something with it.

> And I much prefer having runtime initialization for both Unix and 
> Windows than having separate paths on each platform potentially hiding 
> different bugs.  And given that on Unix you can statically initialize 
> those, then a runtime initialization is certainly not going to be _that_ 
> costly.

Yeah, definitely best to have them match, whichever way we go.

I don't think it matters terribly much either way which we use, so long as 
its consistent. It'd be nice if the static initializers worked, just 
because people seem to write code with them, but we could just not do that 
in the future.

	-Daniel
*This .sig left intentionally blank*

^ permalink raw reply

* Re: [PATCH v2 11/13] Allow helpers to request the path to the .git directory
From: Daniel Barkalow @ 2009-11-04 22:27 UTC (permalink / raw)
  To: Sverre Rabbelier; +Cc: Git List, Johannes Schindelin, Johan Herland
In-Reply-To: <fabb9a1e0911041406tce0956ai2ad3fe6cfbdc546d@mail.gmail.com>

On Wed, 4 Nov 2009, Sverre Rabbelier wrote:

> Heya,
> 
> On Wed, Nov 4, 2009 at 22:35, Daniel Barkalow <barkalow@iabervon.org> wrote:
> > I thought we cd into the repository in the middle of clone somewhere,
> > before running stuff.
> 
> It does, but not soon enough, that is, not before I needed it in my helper.
> 
> > In any case, I think it would be good to have
> > something like that, but I think maybe we should tell it where it can put
> > its status files, rather than telling it where the .git dir is.
> 
> Yes, that would probably be a good idea, .git/info/remote-<vcs>/<alias> perhaps?

Seems reasonable to me, unless we make it .git/info/remote-<vcs>, and tell 
helper authors to deal with having different imports not conflict with 
each other.

> > It would
> > also be nice if this is the absolute path that gfi will base a relative
> > path for the "marks" options on when importing.
> 
> The marks option has been dumped in favor of 'feature import-marks='
> and 'feature export-marks=' which will be in the reroll of
> sr/gfi-options.

Right, but the values of those have to be paths, and I think they should 
be relative paths, relative to the helper's state directory.

	-Daniel
*This .sig left intentionally blank*

^ permalink raw reply

* Re: Common setting for interoperability repo across windows and unix?
From: Anthony W. Youngman @ 2009-11-04 21:45 UTC (permalink / raw)
  To: git
In-Reply-To: <32541b130911030733i734b9f6doc366934873bf7713@mail.gmail.com>

In message 
<32541b130911030733i734b9f6doc366934873bf7713@mail.gmail.com>, Avery 
Pennarun <apenwarr@gmail.com> writes
>You can pull from a Windows box by running git-daemon on that box from
>the command line.  (It's easier than it sounds.)

That presumes you're running Cygwin ...

git-daemon doesn't (currently) work on msysgit. Currently I run 
git-daemon on my linpus netbook and pull/push from windows.

Cheers,
Wol
-- 
Anthony W. Youngman - anthony@thewolery.demon.co.uk

^ permalink raw reply

* Re: [PATCH v2 11/13] Allow helpers to request the path to the .git  directory
From: Sverre Rabbelier @ 2009-11-04 22:42 UTC (permalink / raw)
  To: Daniel Barkalow; +Cc: Git List, Johannes Schindelin, Johan Herland
In-Reply-To: <alpine.LNX.2.00.0911041723010.14365@iabervon.org>

Heya,

On Wed, Nov 4, 2009 at 23:27, Daniel Barkalow <barkalow@iabervon.org> wrote:
>> Yes, that would probably be a good idea, .git/info/remote-<vcs>/<alias> perhaps?
>
> Seems reasonable to me, unless we make it .git/info/remote-<vcs>, and tell
> helper authors to deal with having different imports not conflict with
> each other.

Ah, yeah, the helper might want to store some global state as well, so
'.git/info/remote-<vcs>' is probably best.

>> The marks option has been dumped in favor of 'feature import-marks='
>> and 'feature export-marks=' which will be in the reroll of
>> sr/gfi-options.
>
> Right, but the values of those have to be paths, and I think they should
> be relative paths, relative to the helper's state directory.

Hmmm, I don't remember exactly what we decided the paths should be
releative, pretty sure it was somewhere in .git/ though.

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* Re: [PATCH] MSVC: Windows-native implementation for subset of    Pthreads API
From: Andrzej K. Haczewski @ 2009-11-04 22:50 UTC (permalink / raw)
  To: kusmabite; +Cc: Nicolas Pitre, git, Johannes Sixt
In-Reply-To: <40aa078e0911041341s1adbbf31t6961207ba9c7905b@mail.gmail.com>

Erik Faye-Lund pisze:
> Couldn't the windows version of pthread_create have a wrapper
> function, that corrected the calling convention, much like the
> function run_thread that start_async in run-command.c has?

Can't be done without allocations. I'd have to pass to that wrapping
thread function an address of original function *and* an original
argument, and there's no way to pack that as one void*.

--
Andrzej

^ permalink raw reply

* Re: Common setting for interoperability repo across windows and unix?
From: Avery Pennarun @ 2009-11-04 22:59 UTC (permalink / raw)
  To: Anthony W. Youngman; +Cc: git
In-Reply-To: <S0ax5AE0Xf8KFwjv@thewolery.demon.co.uk>

On Wed, Nov 4, 2009 at 4:45 PM, Anthony W. Youngman
<wol@thewolery.demon.co.uk> wrote:
> In message <32541b130911030733i734b9f6doc366934873bf7713@mail.gmail.com>,
> Avery Pennarun <apenwarr@gmail.com> writes
>> You can pull from a Windows box by running git-daemon on that box from
>> the command line.  (It's easier than it sounds.)
>
> That presumes you're running Cygwin ...
>
> git-daemon doesn't (currently) work on msysgit. Currently I run git-daemon
> on my linpus netbook and pull/push from windows.

I didn't know that.  Windows sockets are mercifully very nearly
compatible with Linux ones, so hopefully this wouldn't be too hard to
fix for someone that needs it.  (Not me; I prefer cygwin over msys in
all cases.)

Avery

^ permalink raw reply

* Re: Common setting for interoperability repo across windows and unix?
From: Shawn O. Pearce @ 2009-11-04 23:01 UTC (permalink / raw)
  To: Avery Pennarun; +Cc: Anthony W. Youngman, git
In-Reply-To: <32541b130911041459y7a2201a3r18601522187891f2@mail.gmail.com>

Avery Pennarun <apenwarr@gmail.com> wrote:
> >
> > git-daemon doesn't (currently) work on msysgit. Currently I run git-daemon
> > on my linpus netbook and pull/push from windows.
> 
> I didn't know that.  Windows sockets are mercifully very nearly
> compatible with Linux ones, so hopefully this wouldn't be too hard to
> fix for someone that needs it.  (Not me; I prefer cygwin over msys in
> all cases.)

git-daemon relies on fork, do work, then later exec.  Windows lacks
this concept.  Making it hard to port.

-- 
Shawn.

^ permalink raw reply

* Re: Common setting for interoperability repo across windows and unix?
From: Erik Faye-Lund @ 2009-11-04 23:10 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Avery Pennarun, Anthony W. Youngman, git
In-Reply-To: <20091104230122.GK10505@spearce.org>

On Thu, Nov 5, 2009 at 12:01 AM, Shawn O. Pearce <spearce@spearce.org> wrote:
> Avery Pennarun <apenwarr@gmail.com> wrote:
>> >
>> > git-daemon doesn't (currently) work on msysgit. Currently I run git-daemon
>> > on my linpus netbook and pull/push from windows.
>>
>> I didn't know that.  Windows sockets are mercifully very nearly
>> compatible with Linux ones, so hopefully this wouldn't be too hard to
>> fix for someone that needs it.  (Not me; I prefer cygwin over msys in
>> all cases.)
>
> git-daemon relies on fork, do work, then later exec.  Windows lacks
> this concept.  Making it hard to port.

Before anyone starts hacking: I've got a working git-daemon on
Windows, based on some old patches from Mike Pape, plus some extending
of the start_async-api. It's still not ready for submission, but I
think I've got the last important technical details nailed down now,
all that is needed is a fair share of clean-up. Hopefully I'll have it
ready for submission during the week-end some time.

-- 
Erik "kusma" Faye-Lund

^ permalink raw reply

* Re: [PATCH v2 11/13] Allow helpers to request the path to the .git directory
From: Daniel Barkalow @ 2009-11-04 23:17 UTC (permalink / raw)
  To: Sverre Rabbelier; +Cc: Git List, Johannes Schindelin, Johan Herland
In-Reply-To: <fabb9a1e0911041442k493c5d7cx493c2e5dac9d892c@mail.gmail.com>

On Wed, 4 Nov 2009, Sverre Rabbelier wrote:

> Heya,
> 
> On Wed, Nov 4, 2009 at 23:27, Daniel Barkalow <barkalow@iabervon.org> wrote:
> >> The marks option has been dumped in favor of 'feature import-marks='
> >> and 'feature export-marks=' which will be in the reroll of
> >> sr/gfi-options.
> >
> > Right, but the values of those have to be paths, and I think they should
> > be relative paths, relative to the helper's state directory.
> 
> Hmmm, I don't remember exactly what we decided the paths should be
> releative, pretty sure it was somewhere in .git/ though.

Well, gfi is used with different native systems, each of which will 
presumably put it somewhere different.

	-Daniel
*This .sig left intentionally blank*

^ permalink raw reply

* Re: [PATCH v2 11/13] Allow helpers to request the path to the .git  directory
From: Sverre Rabbelier @ 2009-11-04 23:18 UTC (permalink / raw)
  To: Daniel Barkalow; +Cc: Git List, Johannes Schindelin, Johan Herland
In-Reply-To: <alpine.LNX.2.00.0911041748150.14365@iabervon.org>

Heya,

On Thu, Nov 5, 2009 at 00:17, Daniel Barkalow <barkalow@iabervon.org> wrote:
> Well, gfi is used with different native systems, each of which will
> presumably put it somewhere different.

Right, but I meant for git itself :). I assume that you meant that
'gitdir' (or 'infodir' , whatever) should return the same path,
whatever we decide on?q

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* Re: [PATCH v2 11/13] Allow helpers to request the path to the .git directory
From: Daniel Barkalow @ 2009-11-04 23:25 UTC (permalink / raw)
  To: Sverre Rabbelier; +Cc: Git List, Johannes Schindelin, Johan Herland
In-Reply-To: <fabb9a1e0911041518h9e3d07dneba3056848e98f3e@mail.gmail.com>

On Thu, 5 Nov 2009, Sverre Rabbelier wrote:

> Heya,
> 
> On Thu, Nov 5, 2009 at 00:17, Daniel Barkalow <barkalow@iabervon.org> wrote:
> > Well, gfi is used with different native systems, each of which will
> > presumably put it somewhere different.
> 
> Right, but I meant for git itself :). I assume that you meant that
> 'gitdir' (or 'infodir' , whatever) should return the same path,
> whatever we decide on?q

Yeah. I think the base that git uses wasn't decided on, although I may 
have missed that part of the thread. In any case, I do think it should be 
the directory passed to the helper.

	-Daniel
*This .sig left intentionally blank*

^ permalink raw reply


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