Git development
 help / color / mirror / Atom feed
* Re: [PATCH] Teach "git remote" a mirror mode
From: Johannes Schindelin @ 2007-09-03 11:50 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vmyw4majd.fsf@gitster.siamese.dyndns.org>

Hi,

On Mon, 3 Sep 2007, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > When using the "--mirror" option to "git remote add", the refs will 
> > not be stored in the refs/remotes/ namespace, but in the same location 
> > as on the remote side.
> 
> Thanks.  With this and the "git remote rm" I think we would be in much 
> better shape.  Another thing that would be needed further before we can 
> rewriting git-clone would be the "guessing where HEAD points at" and we 
> would be in a very good shape.

Okay, that should be fixable.

> I notice you did not add any tests, though...

You noticed.  Darn.  ;-)

I briefly considered it, but decided I did not have the energy to cut a 
test suite for "git remote"...  Will do so today.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] Add a new lstat implementation based on Win32 API, and make stat use that implementation too.
From: David Kastrup @ 2007-09-03 11:51 UTC (permalink / raw)
  To: git
In-Reply-To: <Pine.LNX.4.64.0709031238400.28586@racer.site>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> On Mon, 3 Sep 2007, Marius Storm-Olsen wrote:
>
>> Robin Rosenberg said the following on 02.09.2007 22:27:
>> > s?ndag 02 september 2007 skrev Marius Storm-Olsen:
>> > > (Also, since Windows doesn't really handle symlinks, it's fine that 
>> > > stat just uses lstat)
>> > 
>> > It does now: See 
>> > http://msdn2.microsoft.com/en-us/library/aa363866.aspx
>> 
>> Yeah, I know about Vista's improved support for symbolic
>> links. However, I think we can let that lay for a while, until we
>> decide to make Git generate proper symlinks on Vista. I don't see
>> it as a 1st priority at the moment, and we can always add the
>> needed functionality in a separate stat() function later.
>
> ... and force everybody to upgrade to Vista,

Nonsense.  Supporting a feature is different from requiring a feature.

> thereby working for Microsoft for free?  You _know_ that I will
> oppose that change.

If Microsoft decides to shoot their users less in the foot than
previously, I don't think that we should take over the gun.

However, if the symbolic link semantics hinted at elsewhere indeed are
as broken as claimed and/or documented, the actual usefulness of
symbolic links seems so limited that we would not be doing their users
a favor by supporting relative symlinks.  And absolute links frankly
have very little place in a _work_ directory (and git does not
currently keep track of enough things in order to make it useful as a
filesystem snapshot system).

I would like to see actual test results to get a confirmation of
whether indeed relative symlinks are as broken under Vista as rumored.
If they are, it seems quite pointless supporting any symlinks under
Windows at the moment.  Until I see actual test results, I would give
Microsoft the benefit of doubt.

-- 
David Kastrup

^ permalink raw reply

* Re: [PATCH] Add a new lstat implementation based on Win32 API, and make stat use that implementation too.
From: Marius Storm-Olsen @ 2007-09-03 11:53 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Robin Rosenberg, Johannes Sixt, git
In-Reply-To: <Pine.LNX.4.64.0709031238400.28586@racer.site>

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

Johannes Schindelin said the following on 03.09.2007 13:39:
> On Mon, 3 Sep 2007, Marius Storm-Olsen wrote:
>> Robin Rosenberg said the following on 02.09.2007 22:27:
>>> It does now: See 
>>> http://msdn2.microsoft.com/en-us/library/aa363866.aspx
>> Yeah, I know about Vista's improved support for symbolic links.
>> However, I think we can let that lay for a while, until we decide
>> to make Git generate proper symlinks on Vista. I don't see it as
>> a 1st priority at the moment, and we can always add the needed
>> functionality in a separate stat() function later.
> 
> ... and force everybody to upgrade to Vista, thereby working for
> Microsoft for free?  You _know_ that I will oppose that change.

;-) I wouldn't dream of it!
Nah, my comment was more 'allow usage of proper Symlinks on Vista' at 
a later point. I would still argue that the default would be what we 
have today. So, it would have to be an option.
But seeing what they've done to the symlinks there, it might be far 
fetched. We'll worry about that (much) later..

-- 
.marius


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 187 bytes --]

^ permalink raw reply

* Re: strbuf API
From: Pierre Habouzit @ 2007-09-03 11:53 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Matthieu Moy, git
In-Reply-To: <7vejhgksza.fsf@gitster.siamese.dyndns.org>

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

On Mon, Sep 03, 2007 at 09:18:01AM +0000, Junio C Hamano wrote:
> Pierre Habouzit <madcoder@debian.org> writes:
> 
> > On Mon, Sep 03, 2007 at 08:32:16AM +0000, Matthieu Moy wrote:
> >>  ...
> >> For example, it would be very tempting to compare files with
> >> "strcmp(buf1, buf2)", but that would just fail silently when the file
> >> contains a '\0' byte.
> >
> >   Indeed, OTHO doing that would be pretty silly, as embending NULs in a
> > strbuf is wrong, it's a _str_buf, not a random-binary-buffer. It's meant
> > to make the use of strings easier, not to use as generic purpose byte
> > buffers. Of course they can, but well, it's not what they are designed
> > for in the first place.
> 
> People, please realize strbuf "API" is not a serious API.  
> 
> It wasn't even intended to be anything more than just a
> quick-and-dirty implementation of fgets that can grow
> dynamically.  The other callers added by people to have it do
> general string manipulations were just bolted-on, not designed.
> I haven't taken a serious look at bstring nor any of the
> alternatives yet, but defending strbuf as if it was designed to
> be a sane API is just silly.

  actually I was defending the "enhanced" strbuf API I was proposing
before, sorry if that was unclear.

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* Re: [PATCH 00/15] i18n support for git-gui
From: Johannes Schindelin @ 2007-09-03 12:17 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20070903044938.GQ18160@spearce.org>

Hi,

On Mon, 3 Sep 2007, Shawn O. Pearce wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > sorry for the fsckup with the first two patches; both should be attributed 
> > to Christian Stimming.
> > 
> > For convenience, I also uploaded that patch series (on top of your 
> > "git-gui: Initialize Tcl's msgcat library for internationalization" in 
> > your "pu" branch) to the "for-shawn" branch in git-gui-i18n.
> 
> That was *much* easier than the small patch bomb.  ;-)

Hehe.  I somehow had the impression that there were only a handful of 
patches.  Imagine how surprised I was when format-patch came up with 15!  

> I've swallowed this series down and added it to my `master` branch. I 
> found one bug in the [mc "..."] string definitions and fixed it (below). 
> I also applied Michele Ballabio's "remove does in some UI strings" patch 
> on top. Those two changes I'm sure will impact some of the translated 
> strings.  I guess someone needs to run `make update-po` and ask 
> translators to revise the individual languages?

Yes, will do so.

The thing is: these four changes are easy enough that update-po will still 
match them, and mark them with "fuzzy".

Now, before I go and bug my dear translators, I will play around with a 
custom diff driver for .po files which should be easier on the eye: it is 
not in the least funny to see gazillions of changes which are just line 
number changes, and to miss out the real modifications.

BTW I like your "quiet" patch...

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH 0/5] gitweb: Support for arbitrary diffs
From: Catalin Marinas @ 2007-09-03 12:22 UTC (permalink / raw)
  To: Martin Koegler; +Cc: Petr Baudis, git
In-Reply-To: <11887443682216-git-send-email-mkoegler@auto.tuwien.ac.at>

Martin Koegler <mkoegler@auto.tuwien.ac.at> wrote:
> I develop the patches via StGit. If you think that I should publish them
> in a different way, please tell me.

You could use 'stg mail' to send the patches and it makes each patch a
direct child of the main series description (PATCH 0/5) rather than a
child of the previous patch. It looks nicer with a threaded view.

-- 
Catalin

^ permalink raw reply

* Re: strbuf API
From: Johannes Schindelin @ 2007-09-03 12:29 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Matthieu Moy, git, Pierre Habouzit
In-Reply-To: <7vejhgksza.fsf@gitster.siamese.dyndns.org>

Hi,

On Mon, 3 Sep 2007, Junio C Hamano wrote:

> Pierre Habouzit <madcoder@debian.org> writes:
> 
> > On Mon, Sep 03, 2007 at 08:32:16AM +0000, Matthieu Moy wrote:
> >>  ...
> >> For example, it would be very tempting to compare files with
> >> "strcmp(buf1, buf2)", but that would just fail silently when the file
> >> contains a '\0' byte.
> >
> >   Indeed, OTHO doing that would be pretty silly, as embending NULs in a
> > strbuf is wrong, it's a _str_buf, not a random-binary-buffer. It's meant
> > to make the use of strings easier, not to use as generic purpose byte
> > buffers. Of course they can, but well, it's not what they are designed
> > for in the first place.
> 
> People, please realize strbuf "API" is not a serious API.  
> 
> It wasn't even intended to be anything more than just a
> quick-and-dirty implementation of fgets that can grow
> dynamically.  The other callers added by people to have it do
> general string manipulations were just bolted-on, not designed.
> I haven't taken a serious look at bstring nor any of the
> alternatives yet, but defending strbuf as if it was designed to
> be a sane API is just silly.

Two points:

- it can be turned into an API (pretty easy, really, since it is _really_ 
  small), and

- it is easy to hack on, because it is _not_ a full-blown cover-them-all 
  library.  As such, we can tweak it the way _we_ want.  And if that means 
  that we do not NUL terminate, so be it.  And if that means that we turn 
  it into a rope structure, so be it.

IOW it might be utterly incomplete, but at least it can evolve in a manner 
that we like.  It does not need to be a *kaboom-here-we-have-a-big-dump*.

I know that we had something like that with LibXDiff.  I was not 
particularly happy with the process, but it turned out quite well, as 
Davide helped us a lot.

But the diff situation is completely different from the string situation:

- we did not have _anything_ in the way of diff generation,

- it is pretty well defined what a diff generation library should do, and

- diff generation is not trivial, and it would have taken us quite a long 
  time to write it ourselves.

All three points are not true with string manipulation.

But code talks, and as I do not have anything presentable (yet), I'll just 
shut up, until I have.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] Add a new lstat implementation based on Win32 API, and make stat use that implementation too.
From: Johannes Schindelin @ 2007-09-03 12:33 UTC (permalink / raw)
  To: Marius Storm-Olsen; +Cc: Robin Rosenberg, Johannes Sixt, git
In-Reply-To: <46DBF5AC.6070400@trolltech.com>

Hi,

On Mon, 3 Sep 2007, Marius Storm-Olsen wrote:

> Johannes Schindelin said the following on 03.09.2007 13:39:
> > On Mon, 3 Sep 2007, Marius Storm-Olsen wrote:
> > > Robin Rosenberg said the following on 02.09.2007 22:27:
> > > > It does now: See http://msdn2.microsoft.com/en-us/library/aa363866.aspx
> > > Yeah, I know about Vista's improved support for symbolic links.
> > > However, I think we can let that lay for a while, until we decide
> > > to make Git generate proper symlinks on Vista. I don't see it as
> > > a 1st priority at the moment, and we can always add the needed
> > > functionality in a separate stat() function later.
> > 
> > ... and force everybody to upgrade to Vista, thereby working for
> > Microsoft for free?  You _know_ that I will oppose that change.
> 
> ;-) I wouldn't dream of it!

Hehe.

> Nah, my comment was more 'allow usage of proper Symlinks on Vista' at a 
> later point. I would still argue that the default would be what we have 
> today. So, it would have to be an option.

Okay, I could live with that.

> But seeing what they've done to the symlinks there, it might be far 
> fetched. We'll worry about that (much) later..

Yes, it is funny how they do it over and over and over again.  Embrace, 
"Extend", Extinguish.  And I thought that eventually people would be 
clever enough to realise...

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] Add a new lstat and fstat implementation based on Win32 API
From: Marius Storm-Olsen @ 2007-09-03 12:38 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Git Mailing List, Johannes Schindelin, Johannes Sixt
In-Reply-To: <46DBFA2A.7050003@trolltech.com>

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

Marius Storm-Olsen said the following on 03.09.2007 14:12:
>  With the our own implementations of lstat & fstat, the following test cases
>  are now fixed:
>      t4116-apply-reverte.sh
>          ok 3: apply in reverse
>      t4200-rerere.sh
>          ok 17: young records still live
>  However, the following test cases seems to fail now:
>      t6024-recursive-merge.sh
>          FAIL 1: setup tests
>          FAIL 3: result contains a conflict
>          FAIL 4: virtual trees were processed
>          FAIL 5: refuse to merge binaries
>  
>  See attached test case logs.

How about that, reading the diff backwards :-)
Oh well, you see the issues based on the logs. Would be nice if you 
guys could also give the testcases a run to see if you get the same 
result.

-- 
.marius


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 187 bytes --]

^ permalink raw reply

* Re: [ANNOUNCE] GIT 1.5.3
From: David Kastrup @ 2007-09-03 12:58 UTC (permalink / raw)
  To: git
In-Reply-To: <46DBF0BB.3070605@op5.se>

Andreas Ericsson <ae@op5.se> writes:

> Junio C Hamano wrote:
>>
>> Assuming that we do not give the old git-p4import script
>> packaged in "git-p4 package", would the following patch be all
>> that is needed, or do we need other things in the spec file?
>>
>> -- snipsnap clipcrap --
>> +Obsoletes:	git-p4
>
> That depends. If packages outside of git requires the git-p4 package
> to function then this will not suffice and a line saying
>
> 	Provides: git-p4
>
> would have to be added instead.

Not instead, but in addition IIRC (it obsoletes the package and
provides the feature).  But that would be nonsensical if the outside
package indeed requires git-p4 and we don't have it in our current
RPM: the purpose of dependencies is to not have things break silently,
and lying about what we provide would be wrong.

-- 
David Kastrup

^ permalink raw reply

* Re: [ANNOUNCE] GIT 1.5.3
From: Andreas Ericsson @ 2007-09-03 13:02 UTC (permalink / raw)
  To: David Kastrup; +Cc: git
In-Reply-To: <868x7nj482.fsf@lola.quinscape.zz>

David Kastrup wrote:
> Andreas Ericsson <ae@op5.se> writes:
> 
>> Junio C Hamano wrote:
>>> Assuming that we do not give the old git-p4import script
>>> packaged in "git-p4 package", would the following patch be all
>>> that is needed, or do we need other things in the spec file?
>>>
>>> -- snipsnap clipcrap --
>>> +Obsoletes:	git-p4
>> That depends. If packages outside of git requires the git-p4 package
>> to function then this will not suffice and a line saying
>>
>> 	Provides: git-p4
>>
>> would have to be added instead.
> 
> Not instead, but in addition IIRC (it obsoletes the package and
> provides the feature).  But that would be nonsensical if the outside
> package indeed requires git-p4 and we don't have it in our current
> RPM: the purpose of dependencies is to not have things break silently,
> and lying about what we provide would be wrong.
> 

No, it really is instead. A package obsoleting one of the features it
provides itself would be insane.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

^ permalink raw reply

* Re: [ANNOUNCE] GIT 1.5.3
From: David Kastrup @ 2007-09-03 13:17 UTC (permalink / raw)
  To: git
In-Reply-To: <46DC05F8.7000709@op5.se>

Andreas Ericsson <ae@op5.se> writes:

> David Kastrup wrote:
>> Andreas Ericsson <ae@op5.se> writes:
>>
>>> Junio C Hamano wrote:
>>>> Assuming that we do not give the old git-p4import script
>>>> packaged in "git-p4 package", would the following patch be all
>>>> that is needed, or do we need other things in the spec file?
>>>>
>>>> -- snipsnap clipcrap --
>>>> +Obsoletes:	git-p4
>>> That depends. If packages outside of git requires the git-p4 package
>>> to function then this will not suffice and a line saying
>>>
>>> 	Provides: git-p4
>>>
>>> would have to be added instead.
>>
>> Not instead, but in addition IIRC (it obsoletes the package and
>> provides the feature).  But that would be nonsensical if the outside
>> package indeed requires git-p4 and we don't have it in our current
>> RPM: the purpose of dependencies is to not have things break silently,
>> and lying about what we provide would be wrong.
>
> No, it really is instead. A package obsoleting one of the features
> it provides itself would be insane.

Not according to my understanding: "Obsoletes:" concerns _packages_,
while "Provides:" concerns features.  So it is perfectly feasible to
obsolete a previously separate package and provide its functionality.

And indeed, our "emacs-auctex" package _both_ obsoletes _and_ provides
"auctex".  And it is not like we did not learn this the hard way...

-- 
David Kastrup

^ permalink raw reply

* Re: [PATCH] Add a new lstat and fstat implementation based on Win32 API
From: Johannes Schindelin @ 2007-09-03 13:33 UTC (permalink / raw)
  To: Marius Storm-Olsen; +Cc: Johannes Sixt, Git Mailing List, Johannes Sixt
In-Reply-To: <46DBFA2A.7050003@trolltech.com>

Hi,

On Mon, 3 Sep 2007, Marius Storm-Olsen wrote:

> There was a problem with racy conditions, which this revision fixes.
> The problem was that fstat was using the builtin implementation, which for
> for some reason is off by some amount of seconds. (This is probably due to
> some leap-year issue in one of the implementations. However, Microsoft tells
> us to use 116444736000000000 in http://support.microsoft.com/kb/167296, so
> I'll stick with that.)
> Also, since both stat and lstat proved to be rather slow, having our own
> version of fstat is probably also wise. At least we now control all the
> stat'ing, so we _know_ they are compatible.
> Also note that this revision makes git_lstat call itself after modifying
> the filename, instead of the builtin stat, for the same reasons.

At least some of these informations should go into the commit message, 
too.

> With the our own implementations of lstat & fstat, the following test cases
> are now fixed:
>     t4116-apply-reverte.sh
>         ok 3: apply in reverse
>     t4200-rerere.sh
>         ok 17: young records still live
> However, the following test cases seems to fail now:
>     t6024-recursive-merge.sh
>         FAIL 1: setup tests
>         FAIL 3: result contains a conflict
>         FAIL 4: virtual trees were processed
>         FAIL 5: refuse to merge binaries
> 
> See attached test case logs.
> Are some of these test cases unstable, so the result will fluctuate on
> Windows?

I saw some funny stuff on Windows, like test cases succeeding when run 
interactively, but failing when run from "make test".

BTW it would have been way easier to apply your patch, had you followed 
SubmittingPatches...

To make it easier on others, I just uploaded it into the "teststat" branch 
on 4msysgit.git (subject to removal in a few days).

First comment: it seems git_fstat() is not declared properly, so there are 
quite a few compiler warnings.

Running the tests now.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] Add a new lstat and fstat implementation based on Win32 API
From: Johannes Sixt @ 2007-09-03 13:49 UTC (permalink / raw)
  To: Marius Storm-Olsen; +Cc: Git Mailing List, Johannes Schindelin, Johannes Sixt
In-Reply-To: <46DBFA2A.7050003@trolltech.com>

Marius Storm-Olsen schrieb:
> There was a problem with racy conditions, which this revision fixes.
> The problem was that fstat was using the builtin implementation, which for
> for some reason is off by some amount of seconds. (This is probably due to
> some leap-year issue in one of the implementations. However, Microsoft 
> tells
> us to use 116444736000000000 in http://support.microsoft.com/kb/167296, so
> I'll stick with that.)

Thanks for the analysis and new patch. Indeed, FILETIME is UTC, which is 
good; the native implementation returns local time, if I read the code 
(of msvcrt) correctly.

> With the our own implementations of lstat & fstat, the following test cases
> are now fixed:
>     t4116-apply-reverte.sh
>         ok 3: apply in reverse
>     t4200-rerere.sh
>         ok 17: young records still live
> However, the following test cases seems to fail now:
>     t6024-recursive-merge.sh
>         FAIL 1: setup tests
>         FAIL 3: result contains a conflict
>         FAIL 4: virtual trees were processed
>         FAIL 5: refuse to merge binaries
> 
> See attached test case logs.
> Are some of these test cases unstable, so the result will fluctuate on
> Windows?

I see many more failures. in:

t4001-diff-rename.sh: 5
t4101-apply-nonl.sh: all
t4102-apply-rename.sh: 2,3,4
t4116-apply-reverse.sh: 3
t4200-rerere.sh: 12,13,17
t5515-fetch-merge-logic.sh: 54
etc...

There is something fishy going on.

But at least the failure in t4116 is easy to work around:

diff --git a/t/t4116-apply-reverse.sh b/t/t4116-apply-reverse.sh
index fc2f622..f3e0c4a 100755
--- a/t/t4116-apply-reverse.sh
+++ b/t/t4116-apply-reverse.sh
@@ -43,7 +43,8 @@ test_expect_success 'apply in reverse' '
         git reset --hard second &&
         git apply --reverse --binary --index patch &&
         git diff >diff &&
-       git diff /dev/null diff
+       : > empty &&
+       git diff empty diff

  '

-- Hannes

^ permalink raw reply related

* Re: [PATCH] Add a new lstat and fstat implementation based on Win32 API
From: Marius Storm-Olsen @ 2007-09-03 13:52 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Johannes Sixt, Git Mailing List, Johannes Sixt
In-Reply-To: <Pine.LNX.4.64.0709031428080.28586@racer.site>

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

Johannes Schindelin said the following on 03.09.2007 15:33:
> On Mon, 3 Sep 2007, Marius Storm-Olsen wrote:
>> There was a problem with racy conditions, which this revision fixes.
>> The problem was that fstat was using the builtin implementation, which for
>> for some reason is off by some amount of seconds. (This is probably due to
>> some leap-year issue in one of the implementations. However, Microsoft tells
>> us to use 116444736000000000 in http://support.microsoft.com/kb/167296, so
>> I'll stick with that.)
>> Also, since both stat and lstat proved to be rather slow, having our own
>> version of fstat is probably also wise. At least we now control all the
>> stat'ing, so we _know_ they are compatible.
>> Also note that this revision makes git_lstat call itself after modifying
>> the filename, instead of the builtin stat, for the same reasons.
> 
> At least some of these informations should go into the commit message, 
> too.

Sure

>> With the our own implementations of lstat & fstat, the following test cases
>> are now fixed:
>>     t4116-apply-reverte.sh
>>         ok 3: apply in reverse
>>     t4200-rerere.sh
>>         ok 17: young records still live
>> However, the following test cases seems to fail now:
>>     t6024-recursive-merge.sh
>>         FAIL 1: setup tests
>>         FAIL 3: result contains a conflict
>>         FAIL 4: virtual trees were processed
>>         FAIL 5: refuse to merge binaries
>>
>> See attached test case logs.
>> Are some of these test cases unstable, so the result will fluctuate on
>> Windows?
> 
> I saw some funny stuff on Windows, like test cases succeeding when run 
> interactively, but failing when run from "make test".

Ok, I ran 'make test', so maybe that's it? I'll rerun them later.

> BTW it would have been way easier to apply your patch, had you followed 
> SubmittingPatches...

Heh, I actually tried, using the Thunderbird way. Of course the 
attachments are non-conforming :-) What was the problem? Whitespace 
issues, Windows EOL, attachments, or all of the above? :-)

> To make it easier on others, I just uploaded it into the "teststat" branch 
> on 4msysgit.git (subject to removal in a few days).

Cool, thanks

> First comment: it seems git_fstat() is not declared properly, so there are 
> quite a few compiler warnings.

/me slaps self. Right, sorry 'bout that. I'll amend the declaration in 
git-compat-util.h.

> Running the tests now.

Great, thanks!
-- 
.marius


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 187 bytes --]

^ permalink raw reply

* Re: [PATCH] Add a new lstat and fstat implementation based on Win32 API
From: Johannes Sixt @ 2007-09-03 13:53 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Marius Storm-Olsen, Git Mailing List, Johannes Sixt
In-Reply-To: <Pine.LNX.4.64.0709031428080.28586@racer.site>

Johannes Schindelin schrieb:
> I saw some funny stuff on Windows, like test cases succeeding when run 
> interactively, but failing when run from "make test".

That's very likely the issue that we work around by inserting "sleep 1" 
at strategic points, which is a timing (race condition) issue and does 
not depend on interactive vs. "make test".

-- Hannes

^ permalink raw reply

* [PATCH] gitk: fix in procedure drawcommits
From: Michele Ballabio @ 2007-09-03 13:59 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: git

This patch indroduces a check before unsetting an array element.

Without this, gitk may complain with

	can't unset "prevlines(...)": no such element in array

when scrolling the history view, bugging the user.

Signed-off-by: Michele Ballabio <barra_cuda@katamail.com>
---

I don't really know if this is the right fix, but it seems to work.

An example of this error:

can't unset "prevlines(a3b4383d69e0754346578c85ba8ff7c05bd88705)": no such element in array
can't unset "prevlines(a3b4383d69e0754346578c85ba8ff7c05bd88705)": no such element in array
    while executing
"unset prevlines($lid)"
    (procedure "drawcommits" line 39)
    invoked from within
"drawcommits $row $endrow"
    (procedure "drawfrac" line 10)
    invoked from within
"drawfrac $f0 $f1"
    (procedure "scrollcanv" line 3)
    invoked from within
"scrollcanv .tf.histframe.csb 0.00672513 0.0087015"


The first "bad" commit is 9f1afe05c3ab7228e21ba3666c6e35d693149b37
(merged in 1.3.0-rc1):

	gitk: New improved gitk

 gitk |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/gitk b/gitk
index 300fdce..527b716 100755
--- a/gitk
+++ b/gitk
@@ -3697,7 +3697,9 @@ proc drawcommits {row {endrow {}}} {
 
 	if {[info exists lineends($r)]} {
 	    foreach lid $lineends($r) {
-		unset prevlines($lid)
+		if {[info exists prevlines($lid)]} {
+		    unset prevlines($lid)
+	        }
 	    }
 	}
 	set rowids [lindex $rowidlist $r]
-- 
1.5.3

^ permalink raw reply related

* Re: [PATCH] Add a new lstat and fstat implementation based on Win32 API
From: Johannes Schindelin @ 2007-09-03 14:35 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Marius Storm-Olsen, Git Mailing List, Johannes Sixt
In-Reply-To: <46DC11F4.5060406@eudaptics.com>

Hi,

On Mon, 3 Sep 2007, Johannes Sixt wrote:

> Johannes Schindelin schrieb:
> > I saw some funny stuff on Windows, like test cases succeeding when run
> > interactively, but failing when run from "make test".
> 
> That's very likely the issue that we work around by inserting "sleep 1" at
> strategic points, which is a timing (race condition) issue and does not
> depend on interactive vs. "make test".

Makes sense to me now.  Especially around t5510 -- t5701 I see those (and 
I do not run "make test" often, since it takes _ages_.

Thanks,
Dscho

^ permalink raw reply

* Re: [PATCH] Add a new lstat and fstat implementation based on Win32 API
From: Johannes Schindelin @ 2007-09-03 14:38 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Marius Storm-Olsen, Git Mailing List, Johannes Sixt
In-Reply-To: <46DC10FE.1080805@eudaptics.com>

Hi,

On Mon, 3 Sep 2007, Johannes Sixt wrote:

> Marius Storm-Olsen schrieb:
> > There was a problem with racy conditions, which this revision fixes.
> > The problem was that fstat was using the builtin implementation, which for
> > for some reason is off by some amount of seconds. (This is probably due to
> > some leap-year issue in one of the implementations. However, Microsoft
> > tells
> > us to use 116444736000000000 in http://support.microsoft.com/kb/167296, so
> > I'll stick with that.)
> 
> Thanks for the analysis and new patch. Indeed, FILETIME is UTC, which is
> good; the native implementation returns local time, if I read the code (of
> msvcrt) correctly.
> 
> > With the our own implementations of lstat & fstat, the following test cases
> > are now fixed:
> >     t4116-apply-reverte.sh
> >         ok 3: apply in reverse
> >     t4200-rerere.sh
> >         ok 17: young records still live
> > However, the following test cases seems to fail now:
> >     t6024-recursive-merge.sh
> >         FAIL 1: setup tests
> >         FAIL 3: result contains a conflict
> >         FAIL 4: virtual trees were processed
> >         FAIL 5: refuse to merge binaries
> > 
> > See attached test case logs.
> > Are some of these test cases unstable, so the result will fluctuate on
> > Windows?
> 
> I see many more failures. in:
> 
> t4001-diff-rename.sh: 5
> t4101-apply-nonl.sh: all
> t4102-apply-rename.sh: 2,3,4
> t4116-apply-reverse.sh: 3
> t4200-rerere.sh: 12,13,17
> t5515-fetch-merge-logic.sh: 54
> etc...

Funny.  I do not get most of these:

t4200-rerere,
t5510-fetch,
t5515-fetch-merge-logic,
t5700-clone-reference,
t5701-clone-local,
t7004

This is all on top of 4msysgit's "devel" branch, and the t5* and t7004 
failed there already.

Oh, and I actually run from sh, not from cmd.  (Wouldn't you have 
guessed?)

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] Add a new lstat and fstat implementation based on Win32 API
From: Johannes Schindelin @ 2007-09-03 14:39 UTC (permalink / raw)
  To: Marius Storm-Olsen; +Cc: Johannes Sixt, Git Mailing List, Johannes Sixt
In-Reply-To: <46DC1186.2010008@trolltech.com>

Hi,

On Mon, 3 Sep 2007, Marius Storm-Olsen wrote:

> Johannes Schindelin said the following on 03.09.2007 15:33:
>
> > BTW it would have been way easier to apply your patch, had you 
> > followed SubmittingPatches...
> 
> Heh, I actually tried, using the Thunderbird way. Of course the 
> attachments are non-conforming :-) What was the problem? Whitespace 
> issues, Windows EOL, attachments, or all of the above? :-)

git am said that the patch was empty.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] Add a new lstat and fstat implementation based on Win32 API
From: Marius Storm-Olsen @ 2007-09-03 16:15 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Johannes Sixt, Git Mailing List, Johannes Sixt
In-Reply-To: <Pine.LNX.4.64.0709031536060.28586@racer.site>

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

Johannes Schindelin wrote:
> On Mon, 3 Sep 2007, Johannes Sixt wrote:
>> I see many more failures. in:
>>
>> t4001-diff-rename.sh: 5
>> t4101-apply-nonl.sh: all
>> t4102-apply-rename.sh: 2,3,4
>> t4116-apply-reverse.sh: 3
>> t4200-rerere.sh: 12,13,17
>> t5515-fetch-merge-logic.sh: 54
>> etc...
> 
> Funny.  I do not get most of these:
> 
> t4200-rerere,
> t5510-fetch,
> t5515-fetch-merge-logic,
> t5700-clone-reference,
> t5701-clone-local,
> t7004
> 
> This is all on top of 4msysgit's "devel" branch, and the t5* and
> t7004 failed there already.
> 
> Oh, and I actually run from sh, not from cmd.  (Wouldn't you have 
> guessed?)

I was also running the tests under sh, however on top of mingw.git, and
not 4msysgit.git.

--
.marius


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 187 bytes --]

^ permalink raw reply

* Re: [PATCH] Add a new lstat and fstat implementation based on Win32 API
From: Marius Storm-Olsen @ 2007-09-03 16:22 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Johannes Sixt, Git Mailing List, Johannes Sixt
In-Reply-To: <Pine.LNX.4.64.0709031538320.28586@racer.site>

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

Johannes Schindelin wrote:
> On Mon, 3 Sep 2007, Marius Storm-Olsen wrote:
>> Johannes Schindelin said the following on 03.09.2007 15:33:
>>> BTW it would have been way easier to apply your patch, had you 
>>> followed SubmittingPatches...
>> Heh, I actually tried, using the Thunderbird way. Of course the 
>> attachments are non-conforming :-) What was the problem? Whitespace
>>  issues, Windows EOL, attachments, or all of the above? :-)
> 
> git am said that the patch was empty.

Hmm, must be the attachments then. I'll use the 4msysgit.git repo from
now on. I assume it'll be ok if I +push to the teststat branch?

--
.marius


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 187 bytes --]

^ permalink raw reply

* [PATCH] verify-tag: also grok CR/LFs in the tag signature
From: Johannes Schindelin @ 2007-09-03 16:51 UTC (permalink / raw)
  To: git, gitster


On some people's favorite platform, gpg outputs signatures
with CR/LF line endings.  So verify-tag has to play nice with
them.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	I just pushed a fix to 4msysgit.git which prevents the generation 
	of such tag signatures, but it's always better to be a little more 
	graceful.

	Note that the match less strict than before, but if you really 
	have to start a line in a signed tag with that magic BEGIN 
	PGP SIGNATURE string, you cannot be helped anyway.

 builtin-verify-tag.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/builtin-verify-tag.c b/builtin-verify-tag.c
index 5c1314d..8cfefcf 100644
--- a/builtin-verify-tag.c
+++ b/builtin-verify-tag.c
@@ -35,7 +35,7 @@ static int run_gpg_verify(const char *buf, unsigned long size, int verbose)
 
 	/* find the length without signature */
 	len = 0;
-	while (len < size && prefixcmp(buf + len, PGP_SIGNATURE "\n")) {
+	while (len < size && prefixcmp(buf + len, PGP_SIGNATURE)) {
 		eol = memchr(buf + len, '\n', size - len);
 		len += eol ? eol - (buf + len) + 1 : size - len;
 	}
-- 
1.5.3.2.g46909

^ permalink raw reply related

* Re: [PATCH] Add a new lstat and fstat implementation based on Win32 API
From: Johannes Schindelin @ 2007-09-03 16:55 UTC (permalink / raw)
  To: Marius Storm-Olsen; +Cc: Johannes Sixt, Git Mailing List, Johannes Sixt
In-Reply-To: <46DC3320.1010408@trolltech.com>

Hi,

On Mon, 3 Sep 2007, Marius Storm-Olsen wrote:

> Johannes Schindelin wrote:
> 
> > t4200-rerere,
> > t5510-fetch,
> > t5515-fetch-merge-logic,
> > t5700-clone-reference,
> > t5701-clone-local,
> > t7004
> > 
> > This is all on top of 4msysgit's "devel" branch, and the t5* and t7004 
> > failed there already.
> > 
> > Oh, and I actually run from sh, not from cmd.  (Wouldn't you have 
> > guessed?)
> 
> I was also running the tests under sh, however on top of mingw.git, and 
> not 4msysgit.git.

I switched back recently, as more tests are succeeding in 
4msysgit.git's "devel" ATM.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] Add a new lstat and fstat implementation based on Win32 API
From: Johannes Schindelin @ 2007-09-03 16:56 UTC (permalink / raw)
  To: Marius Storm-Olsen; +Cc: Johannes Sixt, Git Mailing List, Johannes Sixt
In-Reply-To: <46DC34A9.8020308@trolltech.com>

Hi,

On Mon, 3 Sep 2007, Marius Storm-Olsen wrote:

> I'll use the 4msysgit.git repo from now on. I assume it'll be ok if I 
> +push to the teststat branch?

I should think so.

Ciao,
Dscho

^ 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