Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Windows buildenv: Use CMake for extraction #11524

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

fwcd
Copy link
Member

@fwcd fwcd commented May 1, 2023

As suggested in #11510 (comment), this migrates the Windows buildenv script to use CMake for unzipping, which should no longer require a dependency on 7-Zip or a custom PowerShell implementation.

Moved to a separate PR for more extensive testing, especially w.r.t performance.

To do:

@JoergAtGithub
Copy link
Member

I wonder if it's possible to download and extract the VCPKG buildenv using ExternalProject_Add in the CMakeList.txt instead of windows_buildenv.bat?
Than windows_buildenv.bat would only set the URL (and checksum) of the buildenv as CMake variables.

@daschuer
Copy link
Member

daschuer commented May 1, 2023

Doing it in the CMakeList.txt has the side effect that it will introduce a strict coupleing of the build process with the environment download. I like to avoid that, to make the switching branches as lightweight as possible.

Currently it is perfectly possible to build older PR branches with the most recent vcpkg environment. This helps to limit bandwidth and diskspace during development. The environment update is an optional step.

@JoergAtGithub
Copy link
Member

Currently it is perfectly possible to build older PR branches with the most recent vcpkg environment.

This wouldn't change, if you do it as propose above, it will remain two independent steps. Each would require user action:

  1. windows_buildenv.bat would write the URL of the buildenv to a CMake setting, as it does for many other settings already
  2. The CMake configuration step would only download and extract a new buildenv, if the specified buildenv is not there (which can only change if you run windows_buildenv.bat)

But this way we would get rid of all the native tools for download and extracting - and we could use generic code for both, Windows and MacOS.

@daschuer
Copy link
Member

daschuer commented May 1, 2023

Ah, yes. It could be an independent configure step.
From the user perspective, nothing is more easy then double click a bash/batch script. Maybe we can think of a solution that makes the best of both worlds.

@daschuer
Copy link
Member

daschuer commented May 2, 2023

I have measured on a Windows 10 device and the result is quite surprising. I have used the command two times to sort out caching issues. The reported time is the smallest.

timecmd powershell -executionpolicy bypass -File unzip.ps1
command took 0:1:41.35 (101.35s total)
timecmd "C:\Program Files\7-Zip\7z.exe" x
command took 0:1:38.98 (98.98s total)
timecmd cmake -E tar xzf 
command took 0:0:50.06 (50.06s total)

The winzip performance is the poorest for reference:

timecmd C:\Program Files\WinZip\WZUNZIP.EXE -d 
command took 0:4:9.08 (249.08s total)

Conclusion: move to cmake!

@JoergAtGithub
Copy link
Member

Conclusion: move to cmake!

You missed to test the builtin Windows unzip function (which is the only one, that is always available) and we use as last fallback. This is really slooow...

@daschuer
Copy link
Member

daschuer commented May 2, 2023

There is no need to unzip it, if cmake is not installed. So we can prompt the user for cmake instead of crawling the system for unzip functions.

@JoergAtGithub
Copy link
Member

The difficult part is to find the CMake executable when you start windows_buildenv.bat by double-click.
CMake.exe is only in the path, if you execute CMake configuration from inside an IDE.

@JosepMaJAZ
Copy link
Contributor

We don't need to detect cmake itself.
We just need to enforce that this script is called from the visual studio shells, or to call it explicitely

start cmd.exe:
cmake --version
Command not found

start X64 Native tools for Visual studio:
cmake --version
cmake version 3.20.21032501-MSVC_2

Back to when we used scons, we had a script that preloaded the visual studio vars: vcvars64.bat (this is for 64bits).
We can check serveral environment variables to know if we are inside the VS shells, like "VSCMD_VER"

Also, I see that I have this one declared on the system, not requiring the lauch of the shell:
VS2019INSTALLDIR=C:\Program Files (x86)\Microsoft Visual Studio\2019\Community

This is the path of the bat:
"C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Auxiliary\Build\vcvars64.bat"

So a test if VSCMD_VER is set and if not, check which "VS20XXINSTALLDIR" we have defined (2019, 2022..) and calling "%VS20XXINSTALLDIR%\VC\Auxiliary\Build\vcvars64.bat" could do it.

Copy link

This PR is marked as stale because it has been open 90 days with no activity.

@github-actions github-actions bot added the stale Stale issues that haven't been updated for a long time. label Jan 26, 2024
@github-actions github-actions bot removed the stale Stale issues that haven't been updated for a long time. label Oct 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants