All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marius Storm-Olsen <mstormo@gmail.com>
To: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
Cc: GIT Mailing-list <git@vger.kernel.org>,
	"Shawn O. Pearce" <spearce@spearce.org>
Subject: Re: MSVC build broken (on cygwin)
Date: Fri, 02 Oct 2009 10:07:10 +0200	[thread overview]
Message-ID: <4AC5B4AE.5070307@gmail.com> (raw)
In-Reply-To: <4AC4E2C2.6030509@ramsay1.demon.co.uk>

Ramsay Jones said the following on 01.10.2009 19:11:
> Hi Marius,
> 
> I know that I'm somewhat late to comment on your recent MSVC
> build patches, but I was busy at the time; better late than
> never... maybe ;-)
> 
> While the patches were traversing the list, I was feeling
> somewhat nervous about the effect of the patches on the
> cygwin build; in fact I remember thinking that they had
> *probably* broken the build. But I was busy...
> 
> Well I finally found time, yesterday, to take a closer look.
> I spent 10-15 minutes squinting at the code in order to
> convince myself that you had in fact *not* broken the cygwin
> build. :)
...
> [Note: I was a little surprised that it got that far, since I didn't
> expect it to find the zlib header files. However, I have set the
> INCLUDE environment variable which msvc is respecting! yeah, a bit old
> fashioned!  Having also set the LIB environment variable, I was then
> a bit surprised that the linker didn't find the library; until I
> noticed that my library is called libz.lib *not* zlib.lib!]
...
Clone the git://repo.or.cz/msvcgit.git, and run the 
setup_32bit_env.cmd script in there, and you should have everything 
you need to both compile and link Git with MSVC.

> Note that the patch below includes some line-wrapping which you can
> ignore if you like, it just makes the Makefile easier to read.
> The only change that matters is inserting a space between -DWIN32 and
> -D_CONSOLE.
> 
> Anyway, the point is *not* to get the msvc build to work for me; rather
> it is to understand why the build *works* for you. ;-)

First of all, thanks for the thorough report! :)
Second, I just recompiled, and it magically works for me. Why is a 
good question, since I also think it shouldn't at this point. The 
_WIN32 define is added by the compiler, and the WIN32 is added by 
windows.h, so our define guards *should* be testing for the _WIN32 define.

To how the guarded code reacts, I preprocessed the run-command.c with 
both version of the command line, and the result was the same:

( 9:54:49 - D:\msvc\git)
 > cl -E ... -DWIN32-D_CONSOLE ... run-command.c | grep run_thread
run-command.c
static unsigned __stdcall run_thread(void *data)
         async->tid = (HANDLE) _beginthreadex(((void *)0), 0, 
run_thread, async, 0, ((void *)0));

( 9:55:43 - D:\msvc\git)
 > cl -E ... -DWIN32 -D_CONSOLE ... run-command.c | grep run_thread
run-command.c
static unsigned __stdcall run_thread(void *data)
         async->tid = (HANDLE) _beginthreadex(((void *)0), 0, 
run_thread, async, 0, ((void *)0));

So, obviously, some magic in there is making it work for me. I have a 
hard time locating the magic in question though. :-/
That being said, does adding the space between the defines fix the 
MSVC compilation using Cygwin's GNU Make? It's none-the-less a correct 
patch, so you get an ack from me. Thanks!

Acked-by: Marius Storm-Olsen <mstormo@gmail.com>

--
.marius

  reply	other threads:[~2009-10-02  8:07 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-10-01 17:11 MSVC build broken (on cygwin) Ramsay Jones
2009-10-02  8:07 ` Marius Storm-Olsen [this message]
2009-10-02  8:23   ` Alex Riesen
2009-10-02  8:49     ` Marius Storm-Olsen
2009-10-03 20:06       ` Ramsay Jones
2009-10-03 20:29         ` Marius Storm-Olsen
2009-10-03 19:36   ` Ramsay Jones

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4AC5B4AE.5070307@gmail.com \
    --to=mstormo@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=ramsay@ramsay1.demon.co.uk \
    --cc=spearce@spearce.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.