Re: [PATCH 1/5] add an extra level of indirection to main()

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

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

Hi Peff,

On Fri, 1 Jul 2016, Jeff King wrote:

> On Fri, Jul 01, 2016 at 10:04:44AM +0200, Johannes Schindelin wrote:
> 
> > On Fri, 1 Jul 2016, Jeff King wrote:
> > 
> > > I waffled between the two mechanisms. Opinions welcome.
> > 
> > I am happy you took the cmd_main() approach: we do have to play some
> > tricks on Windows, in particular in some upcoming changes that support
> > building with MS Visual C++ (we want to ensure that `argv` is in UTF-8,
> > which means that we actually have to use the UTF-16 versions and convert
> > them manually lest argv has the current Windows encoding of strings).
> > Which means that we still have to use that mingw_startup() trick you
> > mentioned, and which would have interfered had you chosen a similar
> > method.
> 
> I actually wondered if it would make sense to get rid of the
> mingw_main() macro, and do it here as just:
> 
>     #ifdef MINGW
>     mingw_startup();
>     #endif
> 
> or something. But I didn't look deeply at it, and anyway I am afraid to
> touch anything in that area because I can't even compile-test it.

Sure. There are a couple of patches in flight to support MSVC better, and
one part is a duplication of mingw_startup(). I would like to fix that
before merging, of course, and hope that it will naturally be helped by
your patch series.

> > > diff --git a/common-main.c b/common-main.c
> > > new file mode 100644
> > > index 0000000..2b96bbf
> > > --- /dev/null
> > > +++ b/common-main.c
> > > @@ -0,0 +1,12 @@
> > > +#include "git-compat-util.h"
> > > +
> > > +int main(int argc, char **av)
> > > +{
> > > +	/*
> > > +	 * This const trickery is explained in
> > > +	 * 84d32bf7678259c08406571cd6ce4b7a6724dcba
> > 
> > This commit message says that mingw_main() is declared with char **argv,
> > and that is the reason why we have to convert. Maybe spell that out here,
> > and then in a subsequent commit, we can fix the mingw_main() declaration?
> 
> The description was sufficiently long that I didn't want to try
> repeating it, and opted for a reference instead. If you're planning to
> fix mingw_main(), I'd be happy to do that as a preparatory patch, and
> then just skip this trickery entirely. :)

Deal:

½/5 is in 5c54dff5c54e68a1101d8fe37aefc6158fddd7f2 and the fixup for 1/5
is in 7b74f7aabb56b428c74f5983c066dc9ea8fe5116 in the 'common-main' branch
on https://github.com/dscho/git.

(I had to resolve merge conflicts in a couple of the later patches, so
feel free to just use the branch, but please note that I cherry-picked a
patch to let me compile with DEVELOPER=1 on Windows, so you might want to
drop 7b74f7aabb56b428c74f5983c066dc9ea8fe5116.)

Ciao,
Dscho