Skip to content

Add option VCPKG_QMAKE_USE_NMAKE in vcpkg_build_qmake and install_qt#8524

Merged
strega-nil merged 13 commits intomicrosoft:masterfrom
JackBoosY:dev/jack/add_prefer_nmake
Nov 23, 2020
Merged

Add option VCPKG_QMAKE_USE_NMAKE in vcpkg_build_qmake and install_qt#8524
strega-nil merged 13 commits intomicrosoft:masterfrom
JackBoosY:dev/jack/add_prefer_nmake

Conversation

@JackBoosY
Copy link
Contributor

@JackBoosY JackBoosY commented Oct 9, 2019

Since the jam problem of jom cannot be solved temporarily, add the PREFER_NMAKE option.

Related: #4692 #7061.

@JackBoosY
Copy link
Contributor Author

@voskrese @Neumann-A @TheScarfix @heydojo
Could you check this?
Thanks.

@Neumann-A
Copy link
Contributor

The problem with that change is I cannot set the variable from the triplet but have to go into the ports to change the call of vcpkg_build_qmake to include PREFER_NMAKE. It would be nice if there was a variable like VCPKG_PREFER_NMAKE which could be set from a triplet. (the same is true for PREFER_NINJA)
Also setting the ENV{CL} to include /MP is missing. So i will not test out this PR since it will take ages to compile without the MP flag ;)

@JackBoosY
Copy link
Contributor Author

@Neumann-A

The problem with that change is I cannot set the variable from the triplet but have to go into the ports to change the call of vcpkg_build_qmake to include PREFER_NMAKE. It would be nice if there was a variable like VCPKG_PREFER_NMAKE which could be set from a triplet. (the same is true for PREFER_NINJA)

This feature may require follow-up support.

Also setting the ENV{CL} to include /MP is missing. So i will not test out this PR since it will take ages to compile without the MP flag ;)

Added.

@JackBoosY JackBoosY marked this pull request as ready for review October 15, 2019 04:43
@JackBoosY JackBoosY requested a review from PhoebeHui October 21, 2019 06:00
@ras0219-msft
Copy link
Contributor

Since the only reason to use this PREFER_NMAKE is in the case that JOM deadlocks (and we only have @Neumann-A's report of that currently), I think the best option would be to just assume it as a global setting instead of a function argument. This should be as easy as removing it from the function argument list and replacing _bc_PREFER_NMAKE with the global setting name (e.g. VCPKG_QMAKE_USE_NMAKE)

@JackBoosY
Copy link
Contributor Author

/azp run

@PhoebeHui PhoebeHui self-assigned this Dec 20, 2019
@JackBoosY JackBoosY changed the title add option PREFER_NMAKE to vcpkg_build_qmake and install_qt. Add option PREFER_NMAKE to vcpkg_build_qmake and install_qt. Dec 20, 2019
@JackBoosY JackBoosY changed the title Add option PREFER_NMAKE to vcpkg_build_qmake and install_qt. Add option VCPKG_QMAKE_USE_NMAKE in vcpkg_build_qmake and install_qt. Dec 20, 2019
@PhoebeHui PhoebeHui changed the title Add option VCPKG_QMAKE_USE_NMAKE in vcpkg_build_qmake and install_qt. Add option VCPKG_QMAKE_USE_NMAKE in vcpkg_build_qmake and install_qt Dec 20, 2019
@Neumann-A
Copy link
Contributor

VCPKG_QMAKE_USE_NMAKE seems to work as intended. Build parts of Qt as part of another paraview branch updated with Qt 5.14 a few times and did not observe any problem with it.

@JackBoosY
Copy link
Contributor Author

/azp run

@BillyONeal
Copy link
Member

Starting package 3/7: qt5-imageformats:x86-windows
Building package qt5-imageformats[core]:x86-windows...
-- Using cached D:/vcpkg/downloads/qtimageformats-everywhere-src-5.12.8.tar.xz
-- Extracting source D:/vcpkg/downloads/qtimageformats-everywhere-src-5.12.8.tar.xz
-- Using source at D:/vcpkg/buildtrees/qt5-imageformats/src/5.12.8-3efb8f5273
-- Configuring x86-windows-rel
-- Configuring x86-windows-rel done
-- Configuring x86-windows-dbg
-- Configuring x86-windows-dbg done
-- Package build-x86-windows-dbg
CMake Error at scripts/cmake/vcpkg_execute_required_process.cmake:72 (message):
    Command failed: "C:/Program Files (x86)/Microsoft Visual Studio/2019/Enterprise/VC/Tools/MSVC/14.26.28801/bin/Hostx64/x86/nmake.exe" -j 65
    Working Directory: D:/vcpkg/buildtrees/qt5-imageformats/x86-windows-dbg
    Error code: 2
    See logs for more information:
      D:\vcpkg\buildtrees\qt5-imageformats\package-build-x86-windows-dbg-err.log

Call Stack (most recent call first):
  scripts/cmake/vcpkg_build_qmake.cmake:40 (vcpkg_execute_required_process)
  scripts/cmake/vcpkg_build_qmake.cmake:75 (run_jom)
  installed/x86-windows/share/qt5/qt_build_submodule.cmake:12 (vcpkg_build_qmake)
  installed/x86-windows/share/qt5/qt_submodule_installation.cmake:8 (qt_build_submodule)
  ports/qt5-imageformats/portfile.cmake:54 (qt_submodule_installation)
  scripts/ports.cmake:76 (include)


Error: Building package qt5-imageformats:x86-windows failed with: BUILD_FAILED
Please ensure you're using the latest portfiles with `.\vcpkg update`, then
submit an issue at https://github.com/Microsoft/vcpkg/issues including:
  Package: qt5-imageformats:x86-windows
  Vcpkg version: 2020.02.04-nohash

Additionally, attach any relevant sections from the log files above.

Looks like qt might not be ready for this :(

@BillyONeal BillyONeal self-requested a review June 11, 2020 06:39
@JackBoosY
Copy link
Contributor Author

@BillyONeal Because using nmake to build will use more time.

@Neumann-A
Copy link
Contributor

Looks like qt might not be ready for this :(

Qt is designed to be used with nmake.... just saying (it even tells you to use nmake at the end of one of the qt5-base config logs)

 Any reason we wouldn't want this setting to be the default if it fixes a known hang?

As has been said above it is a bit slower.

@Neumann-A
Copy link
Contributor

Neumann-A commented Jun 11, 2020

@BillyONeal: Please check logs next time:

NMAKE : fatal error U1065: invalid option 'j'
Stop.

So move the -j option into INVOKE e.g.
set(INVOKE "${JOM}" -j ${VCPKG_CONCURRENCY})
and
set(INVOKE "${MAKE}" -j ${VCPKG_CONCURRENCY})

@BillyONeal
Copy link
Member

Any reason we wouldn't want this setting to be the default if it fixes a known hang?

As has been said above it is a bit slower.

I see the hang almost every time so it might not end up slower for some :)

@Neumann-A
Copy link
Contributor

due to the additional comment in #4692 (comment) should this PR be made mergeable again?

@JackBoosY JackBoosY marked this pull request as ready for review September 10, 2020 07:51
@ras0219
Copy link
Contributor

ras0219 commented Sep 10, 2020

To add some data: on my local 3970X, qt5-base with JOM takes 8.06 minutes but with NMake takes 19.62 minutes

@strega-nil
Copy link
Contributor

@JackBoosY could you remerge with master?

@JackBoosY
Copy link
Contributor Author

@strega-nil Ready.

@strega-nil strega-nil merged commit 5cd25ee into microsoft:master Nov 23, 2020
@strega-nil
Copy link
Contributor

Thanks @JackBoosY !

@JackBoosY JackBoosY deleted the dev/jack/add_prefer_nmake branch November 24, 2020 01:24
@JackBoosY JackBoosY added category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist category:tool-update The issue is with build tool or build script, which requires update or should be executed correctly labels Nov 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist category:tool-update The issue is with build tool or build script, which requires update or should be executed correctly

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants