Introduce options for Windows packaging script

[pbo-linaro]

Summary

Options:
--version: [required] version to build
--help: display this help
--x86: build and test x86 variant
--x64: build and test x64 variant

Note: At least one variant to build is required.

Example: build_llvm_release.bat --version 15.0.0 --x64

[thieta]

I wonder if we shouldn't just build both archs if nothing is specified to keep the old behavior.

llvm/utils/release/build_llvm_release.bat

13

Not sure I like --revision here. I would probably use --version instead.

[pbo-linaro]

In D135255#3839224, @thieta wrote:

I wonder if we shouldn't just build both archs if nothing is specified to keep the old behavior.

The first goal of all those patches, beyond cleanup, is to add arm64 support to this script.
Alas, it can't be run from an x64 machine (build yes, but not tests).
I think making explicit which targets you want is a better choice than enabling only two of the three targets.

I can still change to:

• nothing: x86 + x64
• add arm64 later, but not activated by default

Depends on your taste, I'm open to what you prefer :)

llvm/utils/release/build_llvm_release.bat

13

I'll switch back to version

[thieta]

In D135255#3839288, @pbo-linaro wrote:

I can still change to:

• nothing: x86 + x64
• add arm64 later, but not activated by default

I'll be happy with this. But I wonder what @hans thinks?

[hans]

+CarlosAlbertoEnciso who also worked on this script recently.

In D135255#3839316, @thieta wrote:

In D135255#3839288, @pbo-linaro wrote:

I can still change to:

• nothing: x86 + x64
• add arm64 later, but not activated by default

I'll be happy with this. But I wonder what @hans thinks?

I think the other option would be to detect what platform we're on, and build x86+x64 or arm64 depending on that.

But having less magic is also good, so just having to pass the flags sounds good to me.

[pbo-linaro]

I'm more in favor of less magic, and more explicit.
For giving more context, a lot of projects are relying on platform detection to compile things for Windows on Arm, and frankly, it's a pain.
Most are using python (platform.machine()), which changes depending if you execute an x64 or arm64 interpreter.
So, depending on which python you install, you're not targeting exactly what you expect.
The env var PROCESSOR_ARCHITECTURE can be different too, depending if you use powershell.exe, or cmd.exe. Nice.

Being able to say clearly "this is what is expected" is a good property.
When requesting this from user, IMHO, the *important* is that the script stops (and fails) and asks you exactly what you want:

43 echo choose one or several variants from: --x86 --x64
44 exit /b 1

So, you don't have to dig in script internals to understand what is done/expected.

[thieta]

I am fine with being explicit as well.

[pbo-linaro]

Replaced --revision by --version.

[pbo-linaro]

Fix conflict with 7zip version evaluation (same variable was used)

[hans]

Basically lgtm, just some nitty comments.

llvm/utils/release/build_llvm_release.bat

20

Maybe we should add --x86 to the example too, just to highlight that more than one can be used?

41

The if .. if syntax looks funny. Is this a batch thing?

44

Is it intentional that for missing version we goto usage, but for missing build variant we just print and exit?

[pbo-linaro]

Added --x86 to example, to show that several arch can be built with a single command.

[pbo-linaro]

llvm/utils/release/build_llvm_release.bat

41

batch IF does not have "and" operator.
https://stackoverflow.com/questions/2143187/logical-operators-and-or-in-dos-batch

I agree that's funny. If someone has a better syntax, I'll take it happily :)

44

That was chosen to avoid showing full help in this case (user will focus on latest lines, which is not the error).
Tried adding a separator, like "=======================", but it's not visual enough to focus attention IMHO.

[hans]

lgtm

[pbo-linaro]

llvm/utils/release/build_llvm_release.bat

44

To be more clear regarding your question, workflow for user will be:
1- try the script without args => show usage
2- "Oh, version is required" => call script with --version XXX
3- Instead of printing full help (user already saw it), just tell me is missing an arch.

[pbo-linaro]

@thieta Does that patch looks good to you?

If yes, would that be possible to merge it please?

[thieta]

LGTM - sorry for the delay. Want me to merge this?

[pbo-linaro]

No problem, thanks very much for coming back :)

That would be very nice if you could merge it. Thanks.
Same applies for the other patch (VS detection).

[thieta]

Landed this.

[thieta]

In D135255#3870699, @pbo-linaro wrote:

Same applies for the other patch (VS detection).

That seems already landed.

llvm/utils/release/build_llvm_release.bat

 @echo off
 setlocal enabledelayedexpansion
 
-if "%1"=="" goto usage
 goto begin
 
 :usage
 echo Script for building the LLVM installer on Windows,
 echo used for the releases at https://github.com/llvm/llvm-project/releases
 echo.
-echo Usage: build_llvm_release.bat ^<version^>
+echo Usage: build_llvm_release.bat --version ^<version^> [--x86,--x64]
 echo.
-echo Example: build_llvm_release.bat 14.0.4
+echo Options:
+echo --version: [required] version to build
+echo --help: display this help
+echo --x86: build and test x86 variant
+echo --x64: build and test x64 variant
 echo.
+echo Note: At least one variant to build is required.
+echo.
+echo Example: build_llvm_release.bat --version 15.0.0 --x86 --x64
 exit /b 1
 
 :begin
 
+::==============================================================================
+:: parse args
+set version=
+set help=
+set x86=
+set x64=
+call :parse_args %*
+
+if "%help%" NEQ "" goto usage
+
+if "%version%" == "" (
+    echo --version option is required
+    echo =============================
+    goto usage
+)
+
+if "%x64%" == "" if "%x86%" == "" (
+    echo nothing to build!
+    echo choose one or several variants from: --x86 --x64
+    exit /b 1
+)
+
+::==============================================================================
+:: check prerequisites
 REM Note:
 REM   7zip versions 21.x and higher will try to extract the symlinks in
 REM   llvm's git archive, which requires running as administrator.
 
 REM Check 7-zip version and/or administrator permissions.
-for /f "delims=" %%i in ('7z.exe ^| findstr /r "2[1-9].[0-9][0-9]"') do set version=%%i
-if not "%version%"=="" (
+for /f "delims=" %%i in ('7z.exe ^| findstr /r "2[1-9].[0-9][0-9]"') do set version_7z=%%i
+if not "%version_7z%"=="" (
   REM Unique temporary filename to use by the 'mklink' command.
   set "link_name=%temp%\%username%_%random%_%random%.tmp"
 
   REM As the 'mklink' requires elevated permissions, the symbolic link
   REM creation will fail if the script is not running as administrator.
   mklink /d "!link_name!" . 1>nul 2>nul
   if errorlevel 1 (
     echo.
     echo Script requires administrator permissions, or a 7-zip version 20.x or older.
-    echo Current version is "%version%"
+    echo Current version is "%version_7z%"
     exit /b 1
   ) else (
     REM Remove the temporary symbolic link.
     rd "!link_name!"
   )
 )
 
 REM Prerequisites:
 REM
 REM   Visual Studio 2019, CMake, Ninja, GNUWin32, SWIG, Python 3,
 REM   NSIS with the strlen_8192 patch,
 REM   Visual Studio 2019 SDK and Nuget (for the clang-format plugin),
 REM   Perl (for the OpenMP run-time).
 REM
 REM
 REM   For LLDB, SWIG version <= 3.0.8 needs to be used to work around
 REM   https://github.com/swig/swig/issues/769
+REM
 
-
+:: Detect Visual Studio
 set vsdevcmd=
 set vs_2019_prefix=C:\Program Files (x86)\Microsoft Visual Studio\2019
 :: try potential activated visual studio, then 2019, with different editions
 call :find_visual_studio "%VSINSTALLDIR%"
 call :find_visual_studio "%vs_2019_prefix%\Enterprise"
 call :find_visual_studio "%vs_2019_prefix%\Professional"
 call :find_visual_studio "%vs_2019_prefix%\Community"
 call :find_visual_studio "%vs_2019_prefix%\BuildTools"
 if not exist "%vsdevcmd%" (
   echo Can't find any installation of Visual Studio
   exit /b 1
 )
 echo Using VS devcmd: %vsdevcmd%
 
+::==============================================================================
 :: start echoing what we do
 @echo on
+
 set python32_dir=C:\Users\%USERNAME%\AppData\Local\Programs\Python\Python310-32
 set python64_dir=C:\Users\%USERNAME%\AppData\Local\Programs\Python\Python310
 
-set revision=llvmorg-%1
-set package_version=%1
+set revision=llvmorg-%version%
+set package_version=%version%
 set build_dir=%cd%\llvm_package_%package_version%
 
 echo Revision: %revision%
 echo Package version: %package_version%
 echo Build dir: %build_dir%
 echo.
 
 if exist %build_dir% (
set common_cmake_flags=^
   -DCMAKE_C_FLAGS="-DLIBXML_STATIC" ^
   -DCMAKE_CXX_FLAGS="-DLIBXML_STATIC" ^
   -DLLVM_ENABLE_PROJECTS="clang;clang-tools-extra;lld;compiler-rt;lldb;openmp"
 
 REM Preserve original path
 set OLDPATH=%PATH%
 
 REM Build the 32-bits and/or 64-bits binaries.
-call :do_build_32 || exit /b 1
-call :do_build_64 || exit /b 1
+if "%x86%" == "true" call :do_build_32 || exit /b 1
+if "%x64%" == "true" call :do_build_64 || exit /b 1
 exit /b 0
 
 ::==============================================================================
 :: Build 32-bits binaries.
 ::==============================================================================
 :do_build_32
 REM Restore original path
 set PATH=%OLDPATH%