This is the mail archive of the crossgcc@sourceware.org mailing list for the crossgcc project.

See crosstool-NG for lots more information.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] kernel/mingw64: add mingw64 kernel type

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

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]