[PATCH] kernel/mingw64: add mingw64 kernel type

Diorcet Yann diorcet.yann@gmail.com
Tue Oct 23 22:11:00 GMT 2012


Le 23/10/2012 23:54, Yann E. MORIN a écrit :
> Yann, All,
>
> Thanks for posting this again.
>
> Overall, the patch looks OK. It built fine on my machine. Yeah! :-)
>
> However, I have a few comments:
I have difficulty to understand all the stuff in crosstool-ng ... i have 
only done a working patch (not a beautiful one).
>   1- I'd like to see how hard it would be to commonalise the mingw{32,64}
>      variants together, eg. take advantage on CT_ARCH_64 to decide what
>      mingw variant to build.
>      This would avoid duplicating code path for mingw.
Yes i tried, but the configuration stuff is still a mystery for me. I 
don't see how to do it without lost 10h.
>   2- In your patch, the DirectX and DDK options are in the mingw64 C library
>      menu, but are used in the mingw64 kernel build script. That's wrong.
>      If the option is used in the kernel script, it shall be in the kernel
>      menu, not the C library menu.
Indeed
>   3- Also, this is incoherent with the 32-bit variant, for which the DirectX
>      option *is* in the C library menu. I'd like that the two variants have
>      a similar layout in the menu, so that it does not erquire a completely
>      schyzophrenic user to discover where the options are. ;-)
>      I see that for the 64-bit variant, the DirectX is provided by the kernel
>      headers, whereas it is provided by a separate package and build in the
>      libc_finish step for the 32-bit variant. If we have to change the 32-bit
>      variant so it more closely ressemble the 64-bit one, so be it.
>      That will help us commonalise the build scripts and the config files.
>   4- Other comments in-lined in the patch, below.
>
> On Tuesday 23 October 2012 Yann Diorcet wrote:
>> # HG changeset patch
>> # User Yann Diorcet<diorcet.yann@gmail.com>
>> # Date 1351016048 -7200
>> # Node ID a34dced6ae17acc54d132e91805208971e6ac7f4
>> # Parent  946d6d133a90935465e3582b54b60c0d7e4e6397
>> kernel/mingw64: add mingw64 kernel type
>> script: add script for kernel and libc for mingw64
>> samples: add 64 bits Windows target sample
>> Signed-off-by: Yann Diorcet<diorcet.yann@gmail.com>
> As this is primarily adding a new 'kernel', I'd suggest that the commit
> log be something like:
>
>      kernel/mingw64: new kernel
>
>      Add mingw64 as a new kernel for x86_64.
>      As a dependency, add the mingw64 C library.
>
>      Add a sample to build a mingw64 toolchain.
>
>      sob: you
>
> If we can commonalise the two mingw variants, then I'd like to see
> a patch series, with pathes like:
>      [PATCH 1/2] kernel/mingw32: rename to mingw to preapare 64-bit variant
>      [PATCH 2/2] kernel/mingw: add 64-bit variant
>
> Use your imagination to complete the commit log. ;-)
>
> [--SNIP--]
>> diff -r 946d6d133a90 -r a34dced6ae17 config/kernel/mingw64.in
>> --- /dev/null	Thu Jan 01 00:00:00 1970 +0000
>> +++ b/config/kernel/mingw64.in	Tue Oct 23 20:14:08 2012 +0200
>> @@ -0,0 +1,33 @@
> This file is *very* similar to the 32-bit variant, and would gain in being
> folded together.
i don't know how to use a same kernel.in with different kernel type 
using different scripts...
> [--SNIP--]
>> diff -r 946d6d133a90 -r a34dced6ae17 config/libc/mingw64.in
>> --- /dev/null	Thu Jan 01 00:00:00 1970 +0000
>> +++ b/config/libc/mingw64.in	Tue Oct 23 20:14:08 2012 +0200
>> @@ -0,0 +1,33 @@
> Ditto, very similar.
>
>> diff -r 946d6d133a90 -r a34dced6ae17 config/libc/mingw64.in.2
>> --- /dev/null	Thu Jan 01 00:00:00 1970 +0000
>> +++ b/config/libc/mingw64.in.2	Tue Oct 23 20:14:08 2012 +0200
>> @@ -0,0 +1,10 @@
>> +# Part-2 of mingw C library options: development libraries
>> +
>> +config MINGW_DIRECTX
>> +    bool
>> +    prompt "Include DirectX development files"
>> +
>> +config MINGW_DDK
>> +    bool
>> +    prompt "Include DDK development files"
>> +
> As said above, these options should go in mingw64.in.
>
> [--SNIP--]
>> diff -r 946d6d133a90 -r a34dced6ae17 samples/x86_64-w64-mingw32/crosstool.config
>> --- /dev/null	Thu Jan 01 00:00:00 1970 +0000
>> +++ b/samples/x86_64-w64-mingw32/crosstool.config	Tue Oct 23 20:14:08 2012 +0200
>> @@ -0,0 +1,18 @@
> Using a defconfig. Good! :-)
>
> [--SNIP--]
>> diff -r 946d6d133a90 -r a34dced6ae17 scripts/build/kernel/mingw64.sh
>> --- /dev/null	Thu Jan 01 00:00:00 1970 +0000
>> +++ b/scripts/build/kernel/mingw64.sh	Tue Oct 23 20:14:08 2012 +0200
>> @@ -0,0 +1,83 @@
>> +# This file declares functions to install the kernel headers for mingw64
>> +# Copyright 2012 Yann Diorcet
>> +# Licensed under the GPL v2. See COPYING in the root of this package
>> +
>> +CT_DoKernelTupleValues() {
>> +    CT_TARGET_KERNEL="mingw32"
> mingw_32_, really?
Yes really .. i was surprise too..;
see 
http://sourceforge.net/apps/trac/mingw-w64/wiki/Cross%20Win32%20and%20Win64%20compiler
all the configure use a target like this x86_64-w64-mingw32
> [--SNIP--]
>> +do_kernel_headers() {
> You need to declare variables you use in this function as being local to
> the function:
>      local sdk_opts
>      local -a mingw_opts
>
>> +    CT_DoStep INFO "Installing kernel headers"
>> +
>> +    mkdir -p "${CT_HEADERS_DIR}"
>> +#    cp -r ${CT_SRC_DIR}/mingw-w64-v${CT_W64API_VERSION}/include/*   \
>> +#          ${CT_HEADERS_DIR}
> Code commented means it is not used. Remove it.
>
>> +    if [ -n "${CT_MINGW_DIRECTX}" ]; then
> Do not expect that a non-empty string is equal to 'y'.
> I prefer that we really check against 'y':
>      if [ "${foo}" = "y" ]; then
>          printf "foo is set to 'y'\n"
>      fi
>
> or against 'n':
>      if [ -z "${foo}" ]; then
>          printf "foo is not set (aka 'n')\n"
>      fi
>
>> +        sdk_opt="directx"
>> +        if [ -n "${CT_MINGW_DDK}" ]; then
>> +            sdk_opt="all"
>> +        fi
>> +    else
>> +        if [ -n "${CT_MINGW_DDK}" ]; then
>> +            sdk_opt="ddk"
>> +        else
>> +            sdk_opt="none"
>> +        fi
>> +    fi
> Although your construct is corect, we usually use another one when there
> are two (or three) bollean variables:
>
>      case "${CT_MINGW_DIRECTX}:${CT_MINGW_DDK}" in
>          y:y)    sdk_opt="all";;
>          y:)     sdk_opt="directx";;
>          :y)     sdk_opt="ddk";;
>          :)      sdk_opt="none";;
>      esac
>
> Much more compact, as easy to read. ;-)
>
>> +    CT_mkdir_pushd "${CT_BUILD_DIR}/build-header-build-${CT_BUILD}"
>> +    gmp_opts+=( "host=${CT_TARGET}" )
>> +    gmp_opts+=( "prefix=${CT_SYSROOT_DIR}" )
>> +    gmp_opts+=( "sdk=${sdk_opt}" )
> You're in the mingw kernel, not in gmp:
> s/gmp_opts/mingw_opts/
>
>> +    do_mingw64_headers_backend "${gmp_opts[@]}"
>> +
>> +    CT_Popd
>> +
>> +    CT_DoExecLog ALL cp -r ${CT_SYSROOT_DIR}/${CT_TARGET}/include/*   \
>> +                     ${CT_HEADERS_DIR}
> Always quotes variables, especially those that are paths.
>
>> +    CT_EndStep
>> +}
>> +
>> +# Build MINGW64 headers
>> +#     Parameter     : description               : type      : default
>> +#     host          : machine to run on         : tuple     : (none)
>> +#     prefix        : prefix to install into    : dir       : (none)
>> +#     cflags        : host cflags to use        : string    : (empty)
> Last line shoule be something like:
>     sdk    : sdk parts to build : string : (empty)
>
>> +do_mingw64_headers_backend() {
> Why do you need to use a backend here? In crosstool-NG, we use a backend
> when the component needs to be built multiple times, not to abstract so
> semantic.
>
> There's no such need here.
>
>> +    local host
>> +    local prefix
>> +    local cflags
>> +    local arg
>> +    local sdk
>> +
>> +    for arg in "$@"; do
>> +        eval "${arg// /\\ }"
>> +    done
>> +
>> +    CT_DoLog EXTRA "Configuring Headers"
>> +
>> +    CT_DoExecLog CFG                                \
>> +    "${CT_SRC_DIR}/mingw-w64-v${CT_W64API_VERSION}/mingw-w64-headers/configure" \
>> +	--build=${CT_BUILD}			    \
>> +        --host=${host}                              \
>> +        --prefix="${prefix}"                        \
>> +        --enable-sdk="${sdk}"
>> +
>> +    CT_DoLog EXTRA "Compile Headers"
>> +    CT_DoExecLog ALL make
>> +
>> +    CT_DoLog EXTRA "Installing Headers"
>> +    CT_DoExecLog ALL make install
>> +}
>> diff -r 946d6d133a90 -r a34dced6ae17 scripts/build/libc/mingw64.sh
>> --- /dev/null	Thu Jan 01 00:00:00 1970 +0000
>> +++ b/scripts/build/libc/mingw64.sh	Tue Oct 23 20:14:08 2012 +0200
>> @@ -0,0 +1,52 @@
>> +do_libc_get() {
> Nit-picking: trailing white space.
>
>> +    CT_GetFile "mingw-w64-v${CT_W64API_VERSION}" \
>> +http://downloads.sourceforge.net/sourceforge/mingw-w64
> Ah, I see that the mingw64 kernel and C library are from the same source
> archive. Yes, you have to CT_GetFile nad CT_Extract it in both places.
> Good.
>
>> +}
>> +
>> +do_libc_extract() {
>> +	CT_Extract "mingw-w64-v${CT_W64API_VERSION}"
>> +}
>> +
>> +do_libc_check_config() {
>> +    :
>> +}
>> +
>> +do_libc_start_files() {
>> +    CT_DoStep INFO "Installing C library headers"
>> +
>> +    # It seems mingw is strangely set up to look into /mingw instead of
>> +    # /usr (notably when looking for the headers). This symlink is
>> +    # here to workaround this, and seems to be here to last... :-/
>> +    CT_DoExecLog ALL ln -sv "${CT_TARGET}" "${CT_SYSROOT_DIR}/mingw"
> I do not like the way the sysroot is organised... :-/
> I do not have enough bacgground in this 'kernel' to understand how
> everything should fit together. :-(
look 
http://sourceforge.net/apps/trac/mingw-w64/wiki/Cross%20Win32%20and%20Win64%20compiler 

it's quite simple... it correspond to the command "ln -s 
/mypath/x86_64-w64-mingw32 /mypath/mingw"
> Let's keep it that way for now...
>
>> +    CT_EndStep
>> +}
>> +
>> +do_libc() {
>> +    CT_DoStep INFO "Building MinGW64 files"
>> +
>> +    CT_DoLog EXTRA "Configuring W64-CRT"
>> +
>> +    mkdir -p "${CT_BUILD_DIR}/build-w64crt"
>> +    cd "${CT_BUILD_DIR}/build-w64crt"
>> +
>> +    CT_DoExecLog CFG                                                  \
>> +    "${CT_SRC_DIR}/mingw-w64-v${CT_W64API_VERSION}/mingw-w64-crt/configure" \
>> +        --prefix=${CT_SYSROOT_DIR}                                    \
>> +        --host=${CT_TARGET}
>> +
>> +    CT_DoLog EXTRA "Building W64-CRT"
>> +    CT_DoExecLog ALL make ${JOBSFLAGS}
>> +
>> +    CT_DoLog EXTRA "Installing W64-CRT"
>> +    CT_DoExecLog ALL make install
>> +
>> +    CT_EndStep
>> +}
>> +
>> +do_libc_finish() {
>> +    CT_DoStep INFO "Installing MinGW64 Development libraries"
>> +
>> +    CT_EndStep
>> +}
> If this function deos nothing, no need to print anything at all (like in
> the do_libc_check_config step).
Sure ... :D
> Regards,
> Yann E. MORIN.
>

PS: is there a way to restart the build from a step (and how know steps)?

Regards,
Yann Diorcet


--
For unsubscribe information see http://sourceware.org/lists.html#faq



More information about the crossgcc mailing list