* Re: [PATCH] Add a new lstat and fstat implementation based on Win32 API
From: Marius Storm-Olsen @ 2007-09-04 13:56 UTC (permalink / raw)
To: Johannes Sixt, Johannes Schindelin; +Cc: Johannes Sixt, Git Mailing List
In-Reply-To: <46DD473A.8010602@trolltech.com>
[-- Attachment #1: Type: text/plain, Size: 548 bytes --]
Marius Storm-Olsen said the following on 04.09.2007 13:53:
> In the meantime, I've pushed out a new patch
> http://repo.or.cz/w/git/mingw/4msysgit.git?a=commitdiff;h=683775c00d9fb95bcbe4632f95b67a96b902fa59
>
> /me starts another test run, to see how our tests are doing now..
Neat, with the custom stat() changes cherry-picked on top of
4msysgit.git 'devel' branch, I only have one failing testcase
t6024-recursive-merge.sh
when running
$ NO_SYMLINKS=1 make -k
The rest are passing with flying colors!
--
.marius
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 187 bytes --]
^ permalink raw reply
* Re: [PATCH] Functions for updating refs.
From: Johannes Schindelin @ 2007-09-04 13:45 UTC (permalink / raw)
To: Carlos Rica; +Cc: git, Junio C Hamano
In-Reply-To: <46DD6020.4050401@gmail.com>
Hi,
On Tue, 4 Sep 2007, Carlos Rica wrote:
> Signed-off-by: Carlos Rica <jasampler@gmail.com>
> ---
>
> They are designed to be reused also from other builtins,
> like the recently changed builtin-tag.c and the upcoming
> builtin-reset.c, and perhaps also from builtin-fetch--tool.c.
This should go into the commit message.
> +int update_ref_or_die(const char *action, const char *refname,
> + const unsigned char *sha1,
> + const unsigned char *oldval, int flags)
Should this not be "void"? And should it not use update_ref_or_error()?
Otherwise I like it.
Ciao,
Dscho
^ permalink raw reply
* [PATCH] Functions for updating refs.
From: Carlos Rica @ 2007-09-04 13:39 UTC (permalink / raw)
To: git, Junio C Hamano, Johannes Schindelin
Signed-off-by: Carlos Rica <jasampler@gmail.com>
---
They are designed to be reused also from other builtins,
like the recently changed builtin-tag.c and the upcoming
builtin-reset.c, and perhaps also from builtin-fetch--tool.c.
builtin-update-ref.c | 8 ++------
refs.c | 32 ++++++++++++++++++++++++++++++++
refs.h | 8 ++++++++
send-pack.c | 12 ++++--------
4 files changed, 46 insertions(+), 14 deletions(-)
diff --git a/builtin-update-ref.c b/builtin-update-ref.c
index 8339cf1..bd7fe4d 100644
--- a/builtin-update-ref.c
+++ b/builtin-update-ref.c
@@ -62,10 +62,6 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)
if (oldval && *oldval && get_sha1(oldval, oldsha1))
die("%s: not a valid old SHA1", oldval);
- lock = lock_any_ref_for_update(refname, oldval ? oldsha1 : NULL, ref_flags);
- if (!lock)
- die("%s: cannot lock the ref", refname);
- if (write_ref_sha1(lock, sha1, msg) < 0)
- die("%s: cannot update the ref", refname);
- return 0;
+ return update_ref_or_die(msg, refname, sha1,
+ oldval ? oldsha1 : NULL, ref_flags);
}
diff --git a/refs.c b/refs.c
index 09a2c87..4fd5065 100644
--- a/refs.c
+++ b/refs.c
@@ -1455,3 +1455,35 @@ int for_each_reflog(each_ref_fn fn, void *cb_data)
{
return do_for_each_reflog("", fn, cb_data);
}
+
+int update_ref_or_die(const char *action, const char *refname,
+ const unsigned char *sha1,
+ const unsigned char *oldval, int flags)
+{
+ static struct ref_lock *lock;
+ lock = lock_any_ref_for_update(refname, oldval, flags);
+ if (!lock)
+ die("Cannot lock the ref '%s'.", refname);
+ if (write_ref_sha1(lock, sha1, action) < 0)
+ die("Cannot update the ref '%s'.", refname);
+ return 0;
+}
+
+int update_ref_or_error(const char *action, const char *refname,
+ const unsigned char *sha1,
+ const unsigned char *oldval, int quiet)
+{
+ static struct ref_lock *lock;
+ lock = lock_any_ref_for_update(refname, oldval, 0);
+ if (!lock) {
+ if (!quiet)
+ error("Cannot lock the ref '%s'.", refname);
+ return 1;
+ }
+ if (write_ref_sha1(lock, sha1, action) < 0) {
+ if (!quiet)
+ error("Cannot update the ref '%s'.", refname);
+ return 1;
+ }
+ return 0;
+}
diff --git a/refs.h b/refs.h
index f234eb7..3d0100e 100644
--- a/refs.h
+++ b/refs.h
@@ -64,4 +64,12 @@ extern int rename_ref(const char *oldref, const char *newref, const char *logmsg
/** resolve ref in nested "gitlink" repository */
extern int resolve_gitlink_ref(const char *name, const char *refname, unsigned char *result);
+/** lock a ref and then write its file */
+int update_ref_or_die(const char *action, const char *refname,
+ const unsigned char *sha1,
+ const unsigned char *oldval, int flags);
+int update_ref_or_error(const char *action, const char *refname,
+ const unsigned char *sha1,
+ const unsigned char *oldval, int quiet);
+
#endif /* REFS_H */
diff --git a/send-pack.c b/send-pack.c
index 9fc8a81..1907684 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -313,14 +313,10 @@ static int send_pack(int in, int out, struct remote *remote, int nr_refspec, cha
if (delete_ref(rs.dst, NULL)) {
error("Failed to delete");
}
- } else {
- lock = lock_any_ref_for_update(rs.dst, NULL, 0);
- if (!lock)
- error("Failed to lock");
- else
- write_ref_sha1(lock, ref->new_sha1,
- "update by push");
- }
+ } else
+ update_ref_or_error("update by push",
+ rs.dst, ref->new_sha1,
+ NULL, 0);
free(rs.dst);
}
}
--
1.5.0
^ permalink raw reply related
* Re: [PATCH] Rework strbuf API and semantics.
From: Andreas Ericsson @ 2007-09-04 13:34 UTC (permalink / raw)
To: Johannes Schindelin, git
In-Reply-To: <20070904115317.GA3381@artemis.corp>
Pierre Habouzit wrote:
> On Tue, Sep 04, 2007 at 11:11:45AM +0000, Johannes Schindelin wrote:
>> Hi,
>>
>> On Tue, 4 Sep 2007, Pierre Habouzit wrote:
>>
>> - IMHO the same goes for strbuf_free() instead of strbuf_wipe(), and
>
> Well, I don't like strbuf_free, because it's opposed to
> strbuf_alloc/new whatever, that would be functions that create and
> release a struct strbuf * (meaning allocating the struct shell as well).
> Here you only free the internal buffer, not the shell, so _free() would
> be a very bad name.
>
> In my own coding rules, I have _new/_delete and _init/_wipe functions,
> the former acts on pointers, the latter on the structs. Hence the
> naming. Though, looking at git's code, it seems that the usual name for
> this operation is _release. So would you go for strbuf_release ?
>
release is indeed better. To me, "wipe" means zeroing out (possibly after
an optional number of passes writing trash to the memory area) rather than
actually freeing any memory.
>
>> - it would be nice to split this patch into
>>
>> - the API change (with _minimal_ changes to anything outside of
>> strbuf.[ch]), and
>>
>> - the cleanups in the rest of the code.
>
> I'll try to do that, but it's quite hard to achieve knowing that in
> many places of the current state of master, there are embeded NUL's
> (accounted in ->len). I'll try to see what I can split, but it's
> unlikely I'll be able to have an intermediate _working_ state (I mean
> that would pass the testsuite) with the new API/semantics _and_ without
> touching a lot less of the rest of the code.
>
Always having a compilable/operational tree is pretty nice primarily due
to bisect. If the code turns from working to non-working in between,
make sure to jot it down in the commit message.
It might even be better to add the embedded NUL handling code in the
first patch, modify the callers in the second, and remove the embedded
NUL handling in a third.
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply
* Re: [PATCH] Fix "cvs log" to use UTC timezone instead of local
From: Linus Torvalds @ 2007-09-04 13:22 UTC (permalink / raw)
To: Jonas Berlin; +Cc: git
In-Reply-To: <11889090932256-git-send-email-xkr47@outerspace.dyndns.org>
On Tue, 4 Sep 2007, Jonas Berlin wrote:
>
> The timestamp format used in "cvs log" output does not include a
> timezone, and must thus be in UTC timezone. The timestamps from git on
> the other hand contain timezone information for each commit timestamp,
> but git-cvsserver discarded this information and used the timestamps
> without adjusting the time accordingly. The patch adds code to apply
> the timezone offset to produce a UTC timestamp.
I think this is wrong.
Git *internally* stores things in UTC anyway, so if there are any local
date format things, it's because git-cvsserver.perl has read the dates
using some format where git has turned its internal date into a local
date.
So instead of turning it back into UTC here, I think git-cvsserver should
be changed to ask for the date in the native git format in the first
place.
That can be done various ways:
- use the "raw log format" which has dates as seconds-since-UTC (and with
an *informational* timezone thing that should then just be ignored).
This is likely the best approach, since anything but this will
almost invariably result in some potentially broken TZ conversion
back-and-forth..
- if it really wants to use the pretty-printing support, git-cvsserver
should probably be changed to do something like
TZ=UTC git rev-list --pretty --date=local
which will pretty-print the date in local time format rather than in
the timezone that the commit was done in, and then the TZ=UTC obviously
says that the "local" zone is UTC.
Anything else *will* be broken, or will be converting back-and-forth.
For example, I think your patch may fix "cvs log", but I'm seeing some
suspiciously similar code in the "cvs annotate" handling, so I suspect
that would need it too.
If instead of trying to convert things to UTC on demand, git-cvsserver
just asks for the git date stamps in UTC in the first place, none of the
places should ever need any timezone conversion.
Linus
^ permalink raw reply
* Re: HFS+ Unicode weirdness
From: David Symonds @ 2007-09-04 13:07 UTC (permalink / raw)
To: Wincent Colaiuta; +Cc: git
In-Reply-To: <052099D2-F79B-4063-82D3-BFB5D0102A55@wincent.com>
On 04/09/07, Wincent Colaiuta <win@wincent.com> wrote:
> On a brand new clone of git.git the file "gitweb/test/Märchen" is
> provoking some weird behaviour running on Mac OS X and the toy HFS+
> filesystem. Note how the unmodified checkout of the file is shown as
> "untracked" by "git st", but on deleting the file it's shown as
> "deleted". If I build a copy of Git based on the clean working tree
> then the resulting build has a version number of "1.5.x-dirty".
>
> Any suggestions on where to start investigating the cause of this?
> About the only lead I have is that if I create a file with that name
> by typing it's name it's encoded as "Ma\314\210rchen", but the file
> in the git.git repo is encoded as "M\303\244rchen".
It's because OS X is using composite Unicode encoding (storing "ä" as
"a" + umlaut). Git doesn't canonicalise things, since OS X does its
jiggery-pokery of that kind of stuff in CoreFramework, which I believe
Git doesn't use.
Dave.
^ permalink raw reply
* Re: [PATCH] Add a new lstat and fstat implementation based on Win32 API
From: Johannes Sixt @ 2007-09-04 13:03 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Marius Storm-Olsen, Johannes Sixt, Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0709041324420.28586@racer.site>
Johannes Schindelin schrieb:
> On Tue, 4 Sep 2007, Johannes Sixt wrote:
>> Reading the implementation of time(), it starts with GetLocalTime(),
>> determines whether daylight saving is in effect, and continues with
>> another round of timezone adjustment - mind you: _not_ a timezone
>> reversal (!!). Doesn't this look extremely bogus?
>>
>> It seems we really need a wrapper for time().
>
> I absolutely concur. Something like this (most of it is blatantly copied
> from Marius' patch)?
Well, I don't think it'll make a difference. The tiny test program below
prints twice the same number. My analysis of the time() implementation is
obviously flawed.
-- Hannes
#include <windows.h>
#include <stdio.h>
#include <time.h>
int main()
{
time_t t = time(NULL);
FILETIME ft;
GetSystemTimeAsFileTime(&ft);
long long winTime = ((long long)ft.dwHighDateTime << 32)
+ ft.dwLowDateTime;
winTime -= 116444736000000000LL;
winTime /= 10000000;
printf("%d %d\n", t, (int) winTime);
return 0;
}
^ permalink raw reply
* Re: HFS+ Unicode weirdness
From: Johannes Schindelin @ 2007-09-04 13:00 UTC (permalink / raw)
To: Wincent Colaiuta; +Cc: git
In-Reply-To: <052099D2-F79B-4063-82D3-BFB5D0102A55@wincent.com>
Hi,
On Tue, 4 Sep 2007, Wincent Colaiuta wrote:
> On a brand new clone of git.git the file "gitweb/test/M?rchen" is provoking
> some weird behaviour running on Mac OS X and the toy HFS+ filesystem.
Please search the mail archives. This has come up quite a couple of
times.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] Add a new lstat and fstat implementation based on Win32 API
From: Johannes Schindelin @ 2007-09-04 12:57 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Marius Storm-Olsen, Johannes Sixt, Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0709041324420.28586@racer.site>
Hi,
On Tue, 4 Sep 2007, Johannes Schindelin wrote:
> On Tue, 4 Sep 2007, Johannes Sixt wrote:
>
> > Johannes Schindelin schrieb:
> > > On Tue, 4 Sep 2007, Johannes Sixt wrote:
> > > > Therefore, I've pushed out a fixup patch at the top of mingw.git's
> > > > devel branch that converts mtime to local time
> > >
> > > On Linux, we compare to UTC to begin with, right? We should do that
> > > here, too... So if time(NULL) does not return UTC on MinGW, we have
> > > to wrap that function, too.
> >
> > According to MSDN, time(NULL) returns "the number of seconds elapsed
> > since [epoch] according to the system clock". Please don't ask me what
> > "the system clock" is.
>
> I think I know. From my QEmu adventures I know that DOS/Windows expects
> the system clock to be set to local time, in contrast to _all_ other
> operating systems.
Now I am utterly confused. MSDN says
FILETIME
Contains a 64-bit value representing the number of 100-nanosecond
intervals since January 1, 1601 (UTC).
Hmm.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] Add a new lstat and fstat implementation based on Win32 API
From: Johannes Schindelin @ 2007-09-04 12:46 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Marius Storm-Olsen, Johannes Sixt, Git Mailing List
In-Reply-To: <46DD433A.5040604@eudaptics.com>
Hi,
On Tue, 4 Sep 2007, Johannes Sixt wrote:
> Johannes Schindelin schrieb:
> > On Tue, 4 Sep 2007, Johannes Sixt wrote:
> > > Therefore, I've pushed out a fixup patch at the top of mingw.git's devel
> > > branch that converts mtime to local time
> >
> > On Linux, we compare to UTC to begin with, right? We should do that here,
> > too... So if time(NULL) does not return UTC on MinGW, we have to wrap that
> > function, too.
>
> According to MSDN, time(NULL) returns "the number of seconds elapsed since
> [epoch] according to the system clock". Please don't ask me what "the system
> clock" is.
I think I know. From my QEmu adventures I know that DOS/Windows expects
the system clock to be set to local time, in contrast to _all_ other
operating systems.
> Reading the implementation of time(), it starts with GetLocalTime(),
> determines whether daylight saving is in effect, and continues with
> another round of timezone adjustment - mind you: _not_ a timezone
> reversal (!!). Doesn't this look extremely bogus?
>
> It seems we really need a wrapper for time().
I absolutely concur. Something like this (most of it is blatantly copied
from Marius' patch)?
-- snip --
diff --git a/git-compat-util.h b/git-compat-util.h
index 172e828..2984319 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -470,6 +470,17 @@ static inline int git_unlink(const char *pathname)
{
#include <time.h>
struct tm *gmtime_r(const time_t *timep, struct tm *result);
struct tm *localtime_r(const time_t *timep, struct tm *result);
+static inline time_t mingw_time(void *dummy)
+{
+ FILETIME ft;
+ GetSystemTimeAsFileTime(&ft);
+ long long winTime = ((long long)ft.dwHighDateTime << 32) +
ft.dwLowDateTime;
+ winTime -= 116444736000000000LL; /* Windows to Unix Epoch
conversion */
+ winTime /= 10000000; /* Nano to seconds resolution
*/
+ return (time_t)winTime;
+
+}
+#define time mingw_time
#define hstrerror strerror
char *mingw_getcwd(char *pointer, int len);
-- snap --
Ciao,
Dscho
^ permalink raw reply related
* [PATCH] Fix "cvs log" to use UTC timezone instead of local
From: Jonas Berlin @ 2007-09-04 12:31 UTC (permalink / raw)
To: git; +Cc: Jonas Berlin
The timestamp format used in "cvs log" output does not include a
timezone, and must thus be in UTC timezone. The timestamps from git on
the other hand contain timezone information for each commit timestamp,
but git-cvsserver discarded this information and used the timestamps
without adjusting the time accordingly. The patch adds code to apply
the timezone offset to produce a UTC timestamp.
Signed-off-by: Jonas Berlin <xkr47@outerspace.dyndns.org>
---
Could it perhaps be that git previously reported timestamps in UTC
instead of including a timezone?
git-cvsserver.perl | 11 ++++++++++-
t/t9400-git-cvsserver-server.sh | 24 ++++++++++++++++++++++++
2 files changed, 34 insertions(+), 1 deletions(-)
diff --git a/git-cvsserver.perl b/git-cvsserver.perl
index 13dbd27..5ae9933 100755
--- a/git-cvsserver.perl
+++ b/git-cvsserver.perl
@@ -23,6 +23,7 @@ use Fcntl;
use File::Temp qw/tempdir tempfile/;
use File::Basename;
use Getopt::Long qw(:config require_order no_ignore_case);
+use Time::Local;
my $VERSION = '@@GIT_VERSION@@';
@@ -1686,7 +1687,15 @@ sub req_log
print "M ----------------------------\n";
print "M revision 1.$revision->{revision}\n";
# reformat the date for log output
- $revision->{modified} = sprintf('%04d/%02d/%02d %s', $3, $DATE_LIST->{$2}, $1, $4 ) if ( $revision->{modified} =~ /(\d+)\s+(\w+)\s+(\d+)\s+(\S+)/ and defined($DATE_LIST->{$2}) );
+ if ( $revision->{modified} =~ /(\d+)\s+(\w+)\s+(\d+)\s+(\d\d):(\d\d):(\d\d) ([-+])(\d\d)(\d\d)/ and defined($DATE_LIST->{$2}) )
+ {
+ my $off = $8 * 3600 + $9 * 60;
+ my $time = timegm($6, $5, $4, $1, $DATE_LIST->{$2}-1, $3 - 1900);
+ $off = -$off if ( $7 eq "-" );
+ $time -= $off;
+ my ( $sec, $min, $hour, $mday, $mon, $year ) = gmtime($time);
+ $revision->{modified} = sprintf('%04d/%02d/%02d %02d:%02d:%02d', $year + 1900, $mon + 1, $mday, $hour, $min, $sec);
+ }
$revision->{author} =~ s/\s+.*//;
$revision->{author} =~ s/^(.{8}).*/$1/;
print "M date: $revision->{modified}; author: $revision->{author}; state: " . ( $revision->{filehash} eq "deleted" ? "dead" : "Exp" ) . "; lines: +2 -3\n";
diff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh
index 641303e..254eab7 100755
--- a/t/t9400-git-cvsserver-server.sh
+++ b/t/t9400-git-cvsserver-server.sh
@@ -405,4 +405,28 @@ test_expect_success 'cvs update (merge no-op)' \
GIT_CONFIG="$git_config" cvs -Q update &&
diff -q merge ../merge'
+#------------
+# CVS LOG
+#------------
+
+cd "$WORKDIR"
+test_expect_success 'cvs log (check that timestamps are in UTC)' \
+ 'echo stamp > stamp &&
+ git add stamp &&
+ TZ=GMT-01 git commit -q -m "Add stamp" &&
+ git push gitcvs.git >/dev/null &&
+ GIT_STAMP=$(git-show --pretty=format:%ct --name-only stamp | grep -v stamp) &&
+ [ "$GIT_STAMP" ] &&
+ cd cvswork &&
+ GIT_CONFIG="$git_config" cvs -Q update &&
+ CVS_STAMP=$(GIT_CONFIG="$git_config" cvs log stamp | perl -e '\''
+ use Time::Local;
+ while(<>) {
+ last if(/^date:/);
+ }
+ my ($dummy,$y,$m,$d,$H,$M,$S) = split(m!\D+!);
+ print timegm($S,$M,$H,$d,$m-1,$y-1900);
+ '\'') &&
+ test "$CVS_STAMP" = "$GIT_STAMP"'
+
test_done
--
1.5.1.6
^ permalink raw reply related
* HFS+ Unicode weirdness
From: Wincent Colaiuta @ 2007-09-04 12:30 UTC (permalink / raw)
To: git
On a brand new clone of git.git the file "gitweb/test/Märchen" is
provoking some weird behaviour running on Mac OS X and the toy HFS+
filesystem. Note how the unmodified checkout of the file is shown as
"untracked" by "git st", but on deleting the file it's shown as
"deleted". If I build a copy of Git based on the clean working tree
then the resulting build has a version number of "1.5.x-dirty".
Any suggestions on where to start investigating the cause of this?
About the only lead I have is that if I create a file with that name
by typing it's name it's encoded as "Ma\314\210rchen", but the file
in the git.git repo is encoded as "M\303\244rchen".
Cheers,
Wincent
$ git clone git://git.kernel.org/pub/scm/git/git.git
Initialized empty Git repository in /tmp/git/.git/
remote: Generating pack...
remote: Counting objects: 12259
Done counting 58277 objects.
remote: Deltifying 58277 objects...
remote: 100% (58277/58277) done
Indexing 58277 objects...
remote: Total 58277 (delta 40699), reused 58034 (delta 40521)
100% (58277/58277) done
Resolving 40699 deltas...
100% (40699/40699) done
$ cd git
/tmp/git
$ git st
# On branch master
# Untracked files:
# (use "git add <file>..." to include in what will be committed)
#
# gitweb/test/Märchen
nothing added to commit but untracked files present (use "git add" to
track)
$ ls -laF gitweb/test
total 24
drwxr-xr-x 5 wincent wheel 170 Sep 4 14:18 ./
drwxr-xr-x 9 wincent wheel 306 Sep 4 14:18 ../
-rw-r--r-- 1 wincent wheel 17 Sep 4 14:18 Ma??rchen
-rw-r--r-- 1 wincent wheel 31 Sep 4 14:18 file with spaces
-rw-r--r-- 1 wincent wheel 37 Sep 4 14:18 file+plus+sign
$ rm gitweb/test/Märchen
$ git st
# On branch master
# Changed but not updated:
# (use "git add/rm <file>..." to update what will be committed)
#
# deleted: gitweb/test/Märchen
#
no changes added to commit (use "git add" and/or "git commit -a")
^ permalink raw reply
* Re: [PATCH] Add a new lstat and fstat implementation based on Win32 API
From: Marius Storm-Olsen @ 2007-09-04 11:53 UTC (permalink / raw)
To: Johannes Sixt, Johannes Schindelin; +Cc: Johannes Sixt, Git Mailing List
In-Reply-To: <46DD433A.5040604@eudaptics.com>
[-- Attachment #1: Type: text/plain, Size: 1183 bytes --]
Johannes Sixt said the following on 04.09.2007 13:36:
> Johannes Schindelin schrieb:
>> On Tue, 4 Sep 2007, Johannes Sixt wrote:
>>> Therefore, I've pushed out a fixup patch at the top of
>>> mingw.git's devel branch that converts mtime to local time
>> On Linux, we compare to UTC to begin with, right? We should do
>> that here, too... So if time(NULL) does not return UTC on MinGW,
>> we have to wrap that function, too.
>
> According to MSDN, time(NULL) returns "the number of seconds
> elapsed since [epoch] according to the system clock". Please don't
> ask me what "the system clock" is.
>
> Reading the implementation of time(), it starts with
> GetLocalTime(), determines whether daylight saving is in effect,
> and continues with another round of timezone adjustment - mind you:
> _not_ a timezone reversal (!!). Doesn't this look extremely bogus?
>
> It seems we really need a wrapper for time().
Hmm, could be.
In the meantime, I've pushed out a new patch
http://repo.or.cz/w/git/mingw/4msysgit.git?a=commitdiff;h=683775c00d9fb95bcbe4632f95b67a96b902fa59
/me starts another test run, to see how our tests are doing now..
--
.marius
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 187 bytes --]
^ permalink raw reply
* Re: [PATCH] Rework strbuf API and semantics.
From: Pierre Habouzit @ 2007-09-04 11:53 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0709041209280.28586@racer.site>
[-- Attachment #1: Type: text/plain, Size: 2909 bytes --]
On Tue, Sep 04, 2007 at 11:11:45AM +0000, Johannes Schindelin wrote:
> Hi,
>
> On Tue, 4 Sep 2007, Pierre Habouzit wrote:
>
> > A strbuf can be used to store byte arrays, or as an extended string
> > library. The `buf' member can be passed to any C legacy string function,
> > because strbuf operations always ensure there is a terminating \0 at the
> > end of the buffer, not accounted in the `len' field of the structure.
> >
> > A strbuf can be used to generate a string/buffer whose final size is
> > not really known, and then "strbuf_detach" can be used to get the built
> > buffer, and keep the wrapping "strbuf" structure usable for further work
> > again.
> >
> > Other interesting feature: buffer_ensure(sb, size) ensure that there
> > is enough allocated space in `sb' to put `size' new octets of data in
> > the buffer. It helps avoiding reallocating data for nothing when the
> > problem the strbuf helps to solve has a known typical size.
>
> I like the general idea of this!
>
> However, some comments are due:
>
> - IMHO strbuf_grow() would be more descriptive than buffer_ensure(),
Yeah, strbuf_ensure (and not buffer_ensure sorry :P) isn't a clever
name. strbuf_grow looks better indeed.
> - IMHO the same goes for strbuf_free() instead of strbuf_wipe(), and
Well, I don't like strbuf_free, because it's opposed to
strbuf_alloc/new whatever, that would be functions that create and
release a struct strbuf * (meaning allocating the struct shell as well).
Here you only free the internal buffer, not the shell, so _free() would
be a very bad name.
In my own coding rules, I have _new/_delete and _init/_wipe functions,
the former acts on pointers, the latter on the structs. Hence the
naming. Though, looking at git's code, it seems that the usual name for
this operation is _release. So would you go for strbuf_release ?
> - it would be nice to split this patch into
>
> - the API change (with _minimal_ changes to anything outside of
> strbuf.[ch]), and
>
> - the cleanups in the rest of the code.
I'll try to do that, but it's quite hard to achieve knowing that in
many places of the current state of master, there are embeded NUL's
(accounted in ->len). I'll try to see what I can split, but it's
unlikely I'll be able to have an intermediate _working_ state (I mean
that would pass the testsuite) with the new API/semantics _and_ without
touching a lot less of the rest of the code.
But I really understand that it helps seeing what the new API
improves, whereas right now it may not be that obvious because of the
(not so) many new lines in strbuf.[hc].
I'll try to roll a new patchset ASAP.
--
·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] Add a new lstat and fstat implementation based on Win32 API
From: Johannes Sixt @ 2007-09-04 11:36 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Marius Storm-Olsen, Johannes Sixt, Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0709041145230.28586@racer.site>
Johannes Schindelin schrieb:
> On Tue, 4 Sep 2007, Johannes Sixt wrote:
>> Therefore, I've pushed out a fixup patch at the top of mingw.git's devel
>> branch that converts mtime to local time
>
> On Linux, we compare to UTC to begin with, right? We should do that here,
> too... So if time(NULL) does not return UTC on MinGW, we have to wrap
> that function, too.
According to MSDN, time(NULL) returns "the number of seconds elapsed since
[epoch] according to the system clock". Please don't ask me what "the system
clock" is.
Reading the implementation of time(), it starts with GetLocalTime(),
determines whether daylight saving is in effect, and continues with another
round of timezone adjustment - mind you: _not_ a timezone reversal (!!).
Doesn't this look extremely bogus?
It seems we really need a wrapper for time().
-- Hannes
^ permalink raw reply
* Re: [PATCH] Add a new lstat and fstat implementation based on Win32 API
From: Johannes Sixt @ 2007-09-04 11:28 UTC (permalink / raw)
To: Marius Storm-Olsen; +Cc: Johannes Schindelin, Johannes Sixt, Git Mailing List
In-Reply-To: <46DD3FC9.2020706@trolltech.com>
Marius Storm-Olsen schrieb:
> Johannes Sixt said the following on 04.09.2007 12:53:
>> Marius Storm-Olsen schrieb:
>>> Johannes Sixt said the following on 04.09.2007 09:41:
>>> http://repo.or.cz/w/git/mingw/4msysgit.git?a=commitdiff;h=f15974add93bdfa92775c77c00e7c65aefd42127
>>
>>
>> Looks good, although you should now handle INVALID_HANDLE_VALUE at the
>> beginning of git_fstat() like this:
>
> Actually, that's already handled.
It's not a big deal: It's an unlikely code path, actually an indication of a
coding error, so you can leave your version.
>> Ok, I just tested FileTimeToLocalFileTime() in a tight loop, and I can
>> run it 100,000,000 times per second. So I'm confident that there won't
>> be any noticable degradation with my proposed change.
>
> Ok. I haven't done the performance test with Git yet, but we'll see. If
> it's not noticeable, I'll add it to all the timestamps we have.
Ack.
-- Hannes
^ permalink raw reply
* Re: [PATCH] Add a new lstat and fstat implementation based on Win32 API
From: Marius Storm-Olsen @ 2007-09-04 11:21 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Johannes Schindelin, Johannes Sixt, Git Mailing List
In-Reply-To: <46DD3943.8040403@eudaptics.com>
[-- Attachment #1: Type: text/plain, Size: 2435 bytes --]
Johannes Sixt said the following on 04.09.2007 12:53:
> Marius Storm-Olsen schrieb:
>> Johannes Sixt said the following on 04.09.2007 09:41:
>>> Thanks a lot! I've pushed it out in mingw.git's master.
>> Ops, already in master branch?
>
> Yes, it looked so polished ;)
>
>> http://repo.or.cz/w/git/mingw/4msysgit.git?a=commitdiff;h=f15974add93bdfa92775c77c00e7c65aefd42127
>
> Looks good, although you should now handle INVALID_HANDLE_VALUE at the
> beginning of git_fstat() like this:
>
> HANDLE fh = (HANDLE)_get_osfhandle(fd);
> if (fh == INVALID_HANDLE_VALUE)
> return -1; /* errno has been set */
>
> if (GetFileInformationByHandle(...
Actually, that's already handled.
GetFileType will report FILE_TYPE_UNKNOWN (0), and GetLastError() will
return a result != NO_ERROR (<-- so you can follow the codepath, but
it actually returns ERROR_INVALID_HANDLE (6))
So, it will fall all the way through the function, and end up
returning -1, with errno = EBADF.
(I use the
case FILE_TYPE_UNKNOWN:
if (GetLastError() != NO_ERROR)
break;
construct, since the documentation says that an FILE_TYPE_UNKNOWN is
still a _valid_ handle iff GetLastError() returns NO_ERROR. So, then
we pass it on to the normal fstat function to let that figure out what
we're dealing with)
>> Ok, I can give it a performance test, but I tend to agree with David
>> Kastrup there. It would be better if we rather fix the places where we
>> check with the local timestamp instead; depending of course on how many
>> places we actually do this.
>> We'll see how much the timezone conversion in the custom stat functions
>> actually hurt us performance wise.
>
> I'd make the decision on the grounds of a perfomance test. If it turns out
> that the penalty is bearable, we should keep this stuff private to the MinGW
> build. Otherwise, we would need MinGW specific code at the call sites
> (unless we can hide the opposite conversion in some other wrapper function).
>
> ... time passes ...
>
> Ok, I just tested FileTimeToLocalFileTime() in a tight loop, and I can run
> it 100,000,000 times per second. So I'm confident that there won't be any
> noticable degradation with my proposed change.
Ok. I haven't done the performance test with Git yet, but we'll see.
If it's not noticeable, I'll add it to all the timestamps we have.
--
.marius
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 187 bytes --]
^ permalink raw reply
* Re: [PATCH] Rework strbuf API and semantics.
From: Johannes Schindelin @ 2007-09-04 11:11 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: git
In-Reply-To: <11888956802504-git-send-email-madcoder@debian.org>
Hi,
On Tue, 4 Sep 2007, Pierre Habouzit wrote:
> A strbuf can be used to store byte arrays, or as an extended string
> library. The `buf' member can be passed to any C legacy string function,
> because strbuf operations always ensure there is a terminating \0 at the
> end of the buffer, not accounted in the `len' field of the structure.
>
> A strbuf can be used to generate a string/buffer whose final size is
> not really known, and then "strbuf_detach" can be used to get the built
> buffer, and keep the wrapping "strbuf" structure usable for further work
> again.
>
> Other interesting feature: buffer_ensure(sb, size) ensure that there
> is enough allocated space in `sb' to put `size' new octets of data in
> the buffer. It helps avoiding reallocating data for nothing when the
> problem the strbuf helps to solve has a known typical size.
I like the general idea of this!
However, some comments are due:
- IMHO strbuf_grow() would be more descriptive than buffer_ensure(),
- IMHO the same goes for strbuf_free() instead of strbuf_wipe(), and
- it would be nice to split this patch into
- the API change (with _minimal_ changes to anything outside of
strbuf.[ch]), and
- the cleanups in the rest of the code.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] Add a new lstat and fstat implementation based on Win32 API
From: Johannes Sixt @ 2007-09-04 10:53 UTC (permalink / raw)
To: Marius Storm-Olsen; +Cc: Johannes Schindelin, Johannes Sixt, Git Mailing List
In-Reply-To: <46DD3153.8060805@trolltech.com>
Marius Storm-Olsen schrieb:
> Johannes Sixt said the following on 04.09.2007 09:41:
>> Thanks a lot! I've pushed it out in mingw.git's master.
>
> Ops, already in master branch?
Yes, it looked so polished ;)
> http://repo.or.cz/w/git/mingw/4msysgit.git?a=commitdiff;h=f15974add93bdfa92775c77c00e7c65aefd42127
Looks good, although you should now handle INVALID_HANDLE_VALUE at the
beginning of git_fstat() like this:
HANDLE fh = (HANDLE)_get_osfhandle(fd);
if (fh == INVALID_HANDLE_VALUE)
return -1; /* errno has been set */
if (GetFileInformationByHandle(...
> Ok, I can give it a performance test, but I tend to agree with David
> Kastrup there. It would be better if we rather fix the places where we
> check with the local timestamp instead; depending of course on how many
> places we actually do this.
> We'll see how much the timezone conversion in the custom stat functions
> actually hurt us performance wise.
I'd make the decision on the grounds of a perfomance test. If it turns out
that the penalty is bearable, we should keep this stuff private to the MinGW
build. Otherwise, we would need MinGW specific code at the call sites
(unless we can hide the opposite conversion in some other wrapper function).
... time passes ...
Ok, I just tested FileTimeToLocalFileTime() in a tight loop, and I can run
it 100,000,000 times per second. So I'm confident that there won't be any
noticable degradation with my proposed change.
-- Hannes
^ permalink raw reply
* Re: [PATCH] Add a new lstat and fstat implementation based on Win32 API
From: Johannes Schindelin @ 2007-09-04 10:48 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Marius Storm-Olsen, Johannes Sixt, Git Mailing List
In-Reply-To: <46DD0C16.70101@eudaptics.com>
Hi,
On Tue, 4 Sep 2007, Johannes Sixt wrote:
> Marius Storm-Olsen schrieb:
> > Johannes Schindelin wrote:
> > > To make it easier on others, I just uploaded it into the "teststat"
> > > branch on 4msysgit.git (subject to removal in a few days).
> >
> > Ok, I've updated the patch in the 4msysgit.git repo, 'teststat' branch.
> > RFC, and please test.
>
> Thanks a lot! I've pushed it out in mingw.git's master.
>
> The reason that t4200-rerere.sh fails is that we now store UTC in st_mtime.
> However, for the garbage-collection we compare this entry to a local time
> stamp.
Thanks for the explanation.
> Therefore, I've pushed out a fixup patch at the top of mingw.git's devel
> branch that converts mtime to local time
On Linux, we compare to UTC to begin with, right? We should do that here,
too... So if time(NULL) does not return UTC on MinGW, we have to wrap
that function, too.
Ciao,
Dscho
^ permalink raw reply
* [ANNOUNCE] cgit v0.6
From: Lars Hjemli @ 2007-09-04 10:42 UTC (permalink / raw)
To: git
cgit v0.6 (a fast web interface for git) is now available for download:
tarballs - http://hjemli.net/git/cgit
cloning - git://hjemli.net/pub/git/cgit
It's been a while since the last release, so here's a summary of the changes:
* improved site navigation
* improved tree/source browser
* support for shortlog on summary page
* support for multiple snapshot formats (tar.gz, tar.bz, zip)
* support for specifying configfile via environment (great for testing)
* uses git 1.5.3 as a submodule
* bug-fixes
Note: Please take a look at the README
(http://hjemli.net/git/cgit/tree/README) for build instructions,
especially regarding the git subdirectory. And if you've built a
previous version of cgit and provided an override for "prefix" in
cgit.conf, please s/prefix/CGIT_SCRIPT_PATH/ in your cgit.conf before
running "make install".
--
Shortlog since v0.5:
Jeffrey C. Ollie (1):
Rename dirlink to gitlink.
Lars Hjemli (34):
ui-diff: emit table/tr/td at better locations
ui-diff: close td/tr/table properly
cgit.css: make it validate
ui-tree: unify with ui-view, use path to select tree/blob
ui-tree: make blob viewer generate valid html
Add and use cgit_tree_link()
ui-shared: use strcmp() to compare strings
ui-tree: html/css cleanup
Add git_log_link() and fix bug in generic repolink function
Add cgit_commit_link() + support for id=sha1 to commit view
ui-log: honor id=sha1 on querystring
ui-commit: use cgit_commit_link() for parent links
Add cgit_diff_link()
Add more menuitems on repo pages
Add version info from git-describe
Include querystring as part of cached filename for repo summary page
Change "files" to "tree"
Change S/L/T to summary/log/tree
Add setting to enable/disable extra links on index page
Do not include current path in the "tree" menu link
Add trim_end() and use it to remove trailing slashes from repo paths
Add ofs argument to cgit_log_link and use it in ui-log.c
Add ui-tag.c
cgit_print_snapshot_links: use url to specify snapshot name
Add support for line number in url fragment
Make ui-tag.c generate valid xhtml
ui-snapshot: whitespace/formatting cleanup
Rewrite the makefile + gen-version.sh
Makefile: add target "get-git"
Delete submodules.sh and prepare for using git-submodule
Use git-1.5.3 as submodule
Set xdemitconf_t.findfunc=NULL
Update README
cgit v0.6
Michael Krelin (14):
make config adjustable via env variable CGIT_CONFIG
make gen-version.sh run even if . is not in PATH
add support for snapshot tarballs
css: adjust vertical-align of commit info th cells
added a chk_non_negative check
compress .tar.gz using gzip as a filter
introduced .tar.bz2 snapshots
add plain uncompressed tar snapshort format
added snapshot filename to the link
introduce cgit_repobasename
shorten snapshot names to repo basename
allow selective enabling of snapshots
fix: changed view link to blob in summary.
link raw blob from tree file view
Ondrej Jirman (6):
Fixed unexpected tags in html output.
Use & instead of & in URLs.
Handle single-line and empty commit subjects
Add option to disable pager to cgit_print_log().
Check for NULL commit buffer in cgit_parse_commit()
Implemented configurable HEAD shortlog on summary page.
^ permalink raw reply
* Re: [PATCH 2/3] archive: specfile support (--pretty=format: in archive files)
From: Johannes Schindelin @ 2007-09-04 10:41 UTC (permalink / raw)
To: Andreas Ericsson
Cc: Junio C Hamano, René Scharfe, Git Mailing List,
Michael Gernoth, Thomas Glanzmann
In-Reply-To: <46DCF0EF.9020604@op5.se>
Hi,
On Tue, 4 Sep 2007, Andreas Ericsson wrote:
> Junio C Hamano wrote:
> > Ren? Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
> >
> > > The attribute is useful for creating auto-updating specfiles. It is
> > > limited by the underlying function format_commit_message(), though.
> > > E.g. currently there is no placeholder for git-describe like output,
> > > and expanded specfiles can't contain NUL bytes. That can be fixed
> > > in format_commit_message() later and will then benefit users of
> > > git-log, too.
> >
> > Interesting. I however wonder if "specfile" is a good name for this
> > attribute, although I admit I do not think of anything better offhand.
>
> "releasefile", perhaps?
Maybe we should not so much name it by purpose, but by function. How
about "substformat" for the attribute name, and replacing any
$Format:blablub$ inside those files with something a la
--pretty=format:blablub?
Ciao,
Dscho
^ permalink raw reply
* Re: Calculating tree nodes
From: Johannes Schindelin @ 2007-09-04 10:33 UTC (permalink / raw)
To: Jon Smirl; +Cc: Martin Langhoff, Shawn O. Pearce, Git Mailing List
In-Reply-To: <9e4733910709032237y65ccafdai4889078533908fb0@mail.gmail.com>
Hi,
On Tue, 4 Sep 2007, Jon Smirl wrote:
> In my scheme the path info is moved into the file object nodes and the
> SHA list is in the commit node.
And how should this "SHA list" be any different from a single tree object,
except that you now merge it with the commit object?
Really, go back to the mail Martin mentioned. Having all objects in one
list kills performance.
> Diffing two trees in the scheme is quite fast. Just get their commit
> objects into RAM and compare the lists of SHAs.
No, it is not fast. Just loading the complete list into RAM is likely
much, much slower than a simple diff _right_ _now_.
Hth,
Dscho
^ permalink raw reply
* Re: [PATCH] Add a new lstat and fstat implementation based on Win32 API
From: Marius Storm-Olsen @ 2007-09-04 10:20 UTC (permalink / raw)
To: Johannes Sixt, Johannes Schindelin; +Cc: Johannes Sixt, Git Mailing List
In-Reply-To: <46DD0C16.70101@eudaptics.com>
[-- Attachment #1: Type: text/plain, Size: 1880 bytes --]
Johannes Sixt said the following on 04.09.2007 09:41:
> Marius Storm-Olsen schrieb:
>> Johannes Schindelin wrote:
>>> To make it easier on others, I just uploaded it into the
>>> "teststat" branch on 4msysgit.git (subject to removal in a few
>>> days).
>> Ok, I've updated the patch in the 4msysgit.git repo, 'teststat'
>> branch. RFC, and please test.
>
> Thanks a lot! I've pushed it out in mingw.git's master.
Ops, already in master branch?
Ok, I found out that the custom fstat function was incomplete, so I
completed it. However, since you've already pushed it to your main
branch, I've added it as a separate commit to the 4msysgit.git
'teststat' branch. It might also explain some of the testfailures we
were seeing, but I haven't finished the test run yet. (So, consider
the patch something to play with, and don't commit it to your 'master'
branch yet! ;-)
http://repo.or.cz/w/git/mingw/4msysgit.git?a=commitdiff;h=f15974add93bdfa92775c77c00e7c65aefd42127
> The reason that t4200-rerere.sh fails is that we now store UTC in
> st_mtime. However, for the garbage-collection we compare this entry
> to a local time stamp. Therefore, I've pushed out a fixup patch at
> the top of mingw.git's devel branch that converts mtime to local
> time
> (http://repo.or.cz/w/git/mingw.git?a=commitdiff;h=1b62ecb31068af06c2fa7664f06c6c36316aac2c).
> Would you kindly conduct the performance test with this patch? I'm
> afraid that this makes us substantially slower.
Ok, I can give it a performance test, but I tend to agree with David
Kastrup there. It would be better if we rather fix the places where we
check with the local timestamp instead; depending of course on how
many places we actually do this.
We'll see how much the timezone conversion in the custom stat
functions actually hurt us performance wise.
--
.marius
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 187 bytes --]
^ permalink raw reply
* Re: [ANNOUNCE] git/gitweb.git repository
From: Petr Baudis @ 2007-09-04 9:38 UTC (permalink / raw)
To: Andreas Ericsson; +Cc: Junio C Hamano, git, jnareb, ltuikov
In-Reply-To: <46DCF41D.2070905@op5.se>
On Tue, Sep 04, 2007 at 07:58:53AM CEST, Andreas Ericsson wrote:
> Junio C Hamano wrote:
>> * Incremental blame
>> It does not seem to break the blame, but at least from where I
>> sit accessing repo.or.cz this does not look incremental to me.
>> The entire browser session freezes until the blame page
>> displays in full. My local installation behaves the same way.
>
> This is most likely due to what browser you're using. Some don't
> start rendering until they've read the entire output of an URL.
The incremental blame uses AJAX, so this shouldn't be an issue. But for
some browsers it seems to take long time to actually render the new ids
or something; I was reluctant about moving this particular one from next
to master because I have also seen problems with it and wanted to do
more tests. The benefit is mostly on files with long history buried deep
in the past.
--
Petr "Pasky" Baudis
Early to rise and early to bed makes a male healthy and wealthy and dead.
-- James Thurber
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox