Hi,

On Fri, 18 Jan 2008, Johannes Sixt wrote:

quoted text
> Johannes Schindelin wrote:
> 
> > - Possibly some of these commits could be folded back into
> >   f90524e(Add target architecture MinGW):
> > 
> >   96a27f1(MinGW: Implement gettimeofday()),
> >   2e05f891(Implement a rudimentary poll() emulation for Windows),
> >   142bda0(Fake implementions of getpwuid(), getuid(), and getpwnam() for
> > Windows),
> >   e799caf(Implement setitimer() and sigaction()),
> >   075fee7(Implement a wrapper of execve that can invoke shell scripts),
> >   495f0af(Work around misbehaved rename() on Windows),
> >   34cf7fd(Implement a pipe() replacement whose ends are not inherited to
> > children),
> >   4504323(Implement start_command() for Windows),
> >   b8e84a6(Implement a work-around for a misbehaved vsnprintf on Windows),
> >   08bbcb4(Windows: always chmod(, 0666) before unlink()),
> >   f6bbf12(Windows: Implement a wrapper of the open() function),
> >   56cedf3(Windows: Fix PRIuMAX definition),
> >   7458a97(Windows: Implement wrappers for gethostbyname(), socket(), and
> > connect()),
> >   ef25947(Windows: Fix ntohl() related warnings about printf formatting),
> >   b9db7ad(Windows: Implement a custom spawnve()), and
> >   47dacb3(compat/pread.c: Add foward decl to fix warning)
> 
> This would become a gigantic patch, which I really dislike. It's much 
> easier to follow (and bisect) if things appear in smaller pieces.

Yes, probably.  (See below for the bisection.)

quoted text
> > - d6596ed(gitk: Disable msgfmt on MinGW) and
> >   004fb4b(Fix renaming .gitk-new to .gitk on Windows if there is already a
> > .gitk)
> >   are gitk patches.
> >   Further, I think that d6596ed would be better done as an automatic
> >   detection of msgfmt's presence; on my Eee PC, there is no msgfmt
> >   either...
> 
> Let's do that later.

I think these are more or less independent of the rest.

quoted text
> > - 20fd16e(Windows: Use a customized struct stat that also has the
> > st_blocks member) should be folded into
> >   6f97065(Add a new lstat and fstat implementation based on Win32 API)
> >   (with a comment that you customized the struct stat, too)
> > 
> >   But then, without 20fd16e, git does not compile, so again I would rather
> >   fold that back into the MinGW commit.
> 
> The custom lstat() implementation cannot come after the custom struct 
> stat because we can't call Windows's stat() with a custom struct stat. 
> But I also don't want the custom lstat() in the code from the beginning 
> because it's merely an optimization.

Okay.

quoted text
> > - in git.git, the onelines are not terminated by "."
> 
> You mean commit messages?

I meant the subjects of the commit messages.  I.e. "Add target 
architecture MinGW.".  But that's such a minor issue.

quoted text
> > - I'd prefer f90524e(Add target architecture MinGW) to come last.
> >   Alternatively, you could cut out the Makefile change so that the series
> >   is still bisectable: MinGW will just not be supported until the very
> >   end.
> 
> I strongly disagree. The series is completely bisectable on *nix. But if 
> the Makefile change comes last, it becomes difficult to bisect on MinGW.

Hmm.  You're right, of course, for *nix.

But for MinGW I am not really sure, as you do not really get a 
fully functional system prior to all of the 42 patches...

I am really torn on this, because I can understand your point of view.

But when there would be an issue with MinGW, and I wanted to find out if 
it worked _at all_, it would be nice to have an easily determined commit 
where MinGW was supposed to be fully functional first, without a private 
tag or something.

Reading again what I wrote it appears that my opinion on that was strong; 
it is not.  I am not quite sure what would be best.  (In the end, I will 
always have the option to not care and let it be Junio's problem ;-)

quoted text
> > $ git grep __MINGW j6t/upstream
> > 
> > comes up with 26 hits.
> > 
> > The first of them: cache.h:381, function is_absolute_path().  That just
> > cries out loud to be "#ifdef DOS_STYLE_PATHS" instead of "#ifdef
> > __MINGW32__".
> > 
> > I guess there should also be -DHAS_NO_FORK_BUT_THREADS 
> > -DHAS_TMP_AND_TEMP -DHAS_WINSOCK2, but most of them look like 
> > -DDOS_STYLE_PATHS to me.
> 
> Doesn't this go too far? How many systems are there where not all of 
> them would be set at the same time?

I am not only thinking about other systems... it is also a pretty nice way 
of documenting _why_ this change was made.  With the possible exception of 
HAS_TMP_AND_TEMP, I really would like to see that.  So much so that I 
hereby offer to do the transform myself.

Ciao,
Dscho

-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html