msys2 / MSYS2-packages

Package scripts for MSYS2.
https://packages.msys2.org
BSD 3-Clause "New" or "Revised" License
1.29k stars 489 forks source link

makepkg-mingw ccache build option doesn't work #562

Closed mati865 closed 8 years ago

mati865 commented 8 years ago

makepkg-mingw executes makepkg with makepkg_mingwXY.config:

  if [ -f "/${_mingw}/bin/gcc.exe" ]; then
    MSYSTEM=${_msystem} \
    PATH=/${_mingw}/bin:$(echo $PATH | tr ':' '\n' | awk '$0 != "/opt/bin"' | paste -sd:) \
    /usr/bin/makepkg --config /etc/makepkg_${_mingw}.conf $@ || exit 1

default config: BUILDENV=(!distcc color !ccache check !sign) If we uncomment !ccache to ccache makepkg will run following:

    # use ccache if it is requested (check buildenv and PKGBUILD opts)
    if check_buildoption "ccache" "y" && [[ -d /usr/lib/ccache/bin ]]; then
        export PATH="/usr/lib/ccache/bin:$PATH"
        ccache=1
    fi

This not only makes ccache useless for makepkg-mingw but also modifies path set by makepkg-mingw (gcc in /usr/bin will be first choice instead of /mingwXY/bin).

My suggestion is to change BUILDENV=(!distcc color !ccache check !sign) like this BUILDENV=(!distcc color !ccache_mingw check !sign) and add proper check in makepkg:

    # use ccache_mingw if it is requested (check buildenv and PKGBUILD opts)
    if check_buildoption "ccache_mingw" "y" && [[ -d $MINGW_PREFIX/lib/ccache/bin ]]; then
        export PATH="/$MINGW_PREFIX/lib/ccache/bin:$PATH"
        ccache=1
    fi
mati865 commented 8 years ago

Another way would be to check before if check_buildoption "ccache" "y" && [[ -d /usr/lib/ccache/bin ]]; then if MINGW_PREFIX is defined:

  1. not defined: use normal ccache code
  2. defined: use this code
    if check_buildoption "ccache" "y" && [[ -d $MINGW_PREFIX/lib/ccache/bin ]]; then
        export PATH="/$MINGW_PREFIX/lib/ccache/bin:$PATH"
        ccache=1
    fi
elieux commented 8 years ago

I was gonna complain about the second solution (it looked nice except the "ccache_mingw" part), but now I see you edited the code. :)

I think the second solution is better, but maybe MINGW_PREFIX is not the right variable to check. We could introduce a new variable like MINGW_BUILDING=1.

mati865 commented 8 years ago

Yeah I missed "ccache"_mingw" and realized it seconds after clicking Comment button :).

I agree that new variable would be safer option. However since I'm not right person to decide about it I'm leaving fixing it for MSYS2 team.

mati865 commented 8 years ago

I've found first broken package because of mixing msys2 gcc with mingw packages. mingw-w64-graphite2 check for gcc instead of x86_64-w64-mingw32-gcc. And because PATH="/usr/lib/ccache/bin:$PATH" is exported by ccache option check we have breakage at linking.

I suggest not to use ccache option in makepkg_mingwXY.config until this issue is fixed.

elieux commented 8 years ago

OT: I'm wondering, what's the nature of the breakage? I don't know much about how ccache works, but it calls gcc internally, right? If /usr/lib/ccache/bin/gcc was told to call /mingw{32,64}/bin/gcc, shouldn't it just work?

elieux commented 8 years ago

Can you make a pull request with the changes described in https://github.com/Alexpux/MSYS2-packages/issues/562#issuecomment-211341136?

mati865 commented 8 years ago

/usr/lib/ccache/bin/gcc links to /usr/bin/gcc and /mingw{32,64}/lib/ccache/bin/gcc links to /mingw{32,64}/bin/gcc.

In /usr/lib/ccache/bin there are files c++ cpp gcc x86_64-pc-msys-g++cc g++ x86_64-pc-msys-c++ x86_64-pc-msys-gcc, they all point to /usr/bin files. So when makepkg does export PATH="/usr/lib/ccache/bin:$PATH" it makes files to be build using /usr/bin/gcc (which is configred with --prefix=/usr --libexecdir=/usr/lib and /usr/lib/gcc/x86_64-pc-msys/5.3.0/../../../../x86_64-pc-msys/bin/ld.exe linker). However rest of the binaries, libs, includes in $PATH are located at /mingw{32,64}.

mati865 commented 8 years ago

I'm trying to test it but compilation of pacman fails. EDIT: Ok, compiled it with --nocheck. Tomorrow I'll do tests.

mati865 commented 8 years ago

For some reason MinGW cmake (MSYS2 cmake is fine) doesn't like wrappers. It is unable do identify gcc which results wrong configuration and compile errors:

-- The C compiler identification is unknown
-- The CXX compiler identification is unknown
-- Check for working C compiler: D:/msys64/mingw64/lib/ccache/bin/gcc
-- Check for working C compiler: D:/msys64/mingw64/lib/ccache/bin/gcc -- works
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working CXX compiler: D:/msys64/mingw64/lib/ccache/bin/g++
-- Check for working CXX compiler: D:/msys64/mingw64/lib/ccache/bin/g++ -- works
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
...

Another way to do it would be exporting CC and CXX variables. It would simplify ccache check inside makepkg and remove little overhead caused by wrappers. However there are 38 packages exporting CC inside their pkgbuild thus they will not use ccahce. Also testing for MINGW_CHOST seems to be OK since it's defined only if we build using makepkg-mingw.

    # use ccache if it is requested (check buildenv and PKGBUILD opts)
    if [[ ! -v MINGW_CHOST ]] && check_buildoption "ccache" "y" && [[ -d /usr/lib/ccache/bin ]] \
    || [[ -v MINGW_CHOST ]] && check_buildoption "ccache" "y" && [[ -d $MINGW_PREFIX/lib/ccache/bin ]]; then
        export CC="ccache cc" CXX="ccache g++"
        ccache=1
    fi
mingwandroid commented 8 years ago

We could patch CMake?

mati865 commented 8 years ago

It fails even if wrapper executes gcc with arguments (without ccache). CMakeError.log:

Compiling the C compiler identification source file "CMakeCCompilerId.c" failed.
Compiler: D:/msys64/mingw64/lib/ccache/bin/gcc 
Build flags: -march=x86-64;-mtune=generic;-O2;-pipe
Id flags: 

The output was:
%1 is not valid Win32 application

Compiling the C compiler identification source file "CMakeCCompilerId.c" failed.
Compiler: D:/msys64/mingw64/lib/ccache/bin/gcc 
Build flags: -march=x86-64;-mtune=generic;-O2;-pipe
Id flags: -c

The output was:
%1 is not valid Win32 application

Compiling the C compiler identification source file "CMakeCCompilerId.c" failed.
Compiler: D:/msys64/mingw64/lib/ccache/bin/gcc 
Build flags: -march=x86-64;-mtune=generic;-O2;-pipe
Id flags: -Aa

The output was:
%1 is not valid Win32 application

Checking whether the C compiler is IAR using "" did not match "IAR .+ Compiler":
Compiling the CXX compiler identification source file "CMakeCXXCompilerId.cpp" failed.
Compiler: D:/msys64/mingw64/lib/ccache/bin/g++ 
Build flags: -march=x86-64;-mtune=generic;-O2;-pipe
Id flags: 

The output was:
%1 is not valid Win32 application

Compiling the CXX compiler identification source file "CMakeCXXCompilerId.cpp" failed.
Compiler: D:/msys64/mingw64/lib/ccache/bin/g++ 
Build flags: -march=x86-64;-mtune=generic;-O2;-pipe
Id flags: -c

The output was:
%1 is not valid Win32 application

Checking whether the CXX compiler is IAR using "" did not match "IAR .+ Compiler":

Compressed build directory: build-x86_64.zip

mingwandroid commented 8 years ago

CreateProcess can't handle shell scripts. You'd need to patch CMake source code to look for '#!' and invoke through 'sh.exe' I reckon.

mati865 commented 8 years ago

I've looked at the code and I'm giving up. If some1 want to take a look at it process is created in /cmake-3.4.1/Source/kwsys/ProcessWin32.c:1842:

  /* Create the process.  */
  (!CreateProcessW(0, cp->Commands[index], 0, 0, TRUE, creationFlags, 0,
                  0, &si->StartupInfo, &cp->ProcessInformation[index]) &&
    (error = GetLastError()));
elieux commented 8 years ago

I think the solution is a bit more prosaic: Create wrapper (Windows-)executables that invoke ccache correctly. Ideally .exe ones, but maybe .cmd ones will work, too.

mati865 commented 8 years ago

I cannot find clean and not causing too much overhead solution about .exe wrapper.

elieux commented 8 years ago

What do you mean? Can the Git wrapper help you? https://github.com/git-for-windows/git/blob/master/compat/win32/git-wrapper.c

elieux commented 8 years ago

On the other hand, a simple wrapper containing exec() compiled with msys2/gcc should work even better.

mati865 commented 8 years ago

I was trying with execv() but argv[0] in Windows isn't like in Linux and I couldn't find way to get it working.

elieux commented 8 years ago

Hmm. Maybe you could combine it with GetModuleFileName. But let's probably wait and see if #573 is accepted.

mingwandroid commented 8 years ago

GetModuleFileName fix in ccache itself sounds like the best approach overall.