refactor: windows build #12281
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,63 @@ | ||
| # Windows build scripts | ||
|
|
||
| Originally used only for GitHub actions, these scripts can help windows users to be able to build locally. | ||
| While still very new (and open to feedback and [pull requests](https://github.com/php/php-src/fork)) | ||
|
|
||
| ## Pre-requisites | ||
|
|
||
| - Windows 2019 or newer [Microsoft](https://www.microsoft.com/en-us/evalcenter/download-windows-server-2019) | ||
| - Administrative access | ||
| - Chocolatey installed to use as a package manager [Install instructions](https://chocolatey.org/install) | ||
| - MySQL installed [see our CI setup](.github/actions/setup-windows/action.yml) | ||
| - SQL Server Installed [see our CI setup](.github/actions/setup-windows/action.yml) | ||
| - Postgres Installed [see our CI setup](.github/actions/setup-windows/action.yml) | ||
| - Git installed | ||
| - Unzip utility installed | ||
| - 7zip utility installed | ||
| - Mirosoft Visual Studio 2019 Community or newer [Microsoft](https://visualstudio.microsoft.com/vs/older-downloads/) | ||
|
|
||
| ## Intended order of operations | ||
|
|
||
| 1. Setup pre-requisites | ||
| 2. Setup environment [env.bat](./scripts/windows/env.bat) | ||
| 3. Build PHP [build.bat](./scripts/windows/build.bat) | ||
| 4. Run PHP Tests [test.bat](./scripts/windows/test.bat) | ||
| 5. Cleanup [cleanup.bat](./scripts/windows/cleanup.bat) | ||
|
|
||
| ## env.bat | ||
|
|
||
| The purpose of this script is simply to setup environment. It is intended to be run via command-line `cmd` under windows as an administrator. | ||
| You can adjust values in this script to: | ||
|
There was a problem hiding this comment. We shouldn't encourage editing committed files. Why can't these values be set using actual env variables? There was a problem hiding this comment. The CI edits committed files today. Did you not know this? Did nobody tell you? Is there a plan to first change that? I can wait. There was a problem hiding this comment. CI is never going to create new commits based on the edited files, but users developing/compiling locally might accidentally include edits to build files in unrelated commits. An env var (and/or command line arg?) would help to mitigate this problem. There was a problem hiding this comment. This isn't creating new commits @DaveRandom the script has always edited |
||
|
|
||
| - Get more workers (theoretically runs tests faster) | ||
| - Adjust temporary and working diretories (although It is untested) | ||
| - Change between platform target (32-bit vs 64-bit) | ||
| - Change Branch | ||
| - Change PHP build SDK | ||
| - Masquerades as being inside a CI environment so the scripts do not exit early | ||
|
|
||
| In the future, it would be nice if some configuration for removing extension enabling and disabling, and selecting components, such as `mysql`, `mssql`, `psql`, `firebird`, `snmp` became optional via this script. | ||
|
|
||
| ## build.bat | ||
|
|
||
| The purpose of this script is to compile PHP, after setting up environment. It is intended to be run via command-line `cmd` under windows as an administrator. | ||
|
There was a problem hiding this comment. I'm not sure build.bat is a good starting point for local builds. It contains hard-coded flags to configure.bat for CI. Usually a build of PHP doesn't require all bundled extensions. There was a problem hiding this comment. I Agree, but this is what you have available. If you have an example of windows builds with optional extensions building, I'd enjoy your contribution or making the contribution later. This is exactly what the purpose of this readme is. How to use, what it does, and I believe there is a part of it about welcoming improvements. There was a problem hiding this comment. Please also see #12281 (comment) as making dependencies optional would also affect cleanup |
||
|
|
||
| It coordinates running `build_task.bat`. The reason for that separation is unclear at this time, but build does pre-flight checks and downloads things according to the env. `build_task.bat` then performs the build. | ||
|
|
||
| ## test.bat | ||
|
|
||
| The purpose of this script is to test PHP, after setting up environment, and compiling. It is intended to be run via command-line `cmd` under windows as an administrator. | ||
|
|
||
| It coordinates running `test_task.bat`. The reason for that separation is unclear at this time. | ||
|
|
||
| ## test_task.bat | ||
|
|
||
| The purpose of this script is to test PHP, after setting up environment, and compiling. It is intended to be run via test.bat and requires administrative privileges to operate successfully. | ||
|
|
||
| This file has a lot of logic. In addition to building the tests, it performs setup of PHP ini, modifications to configuration files for tests, and even fetching dependencies. | ||
|
|
||
| ## cleanup.bat | ||
|
|
||
| The purpose of this script is to shutdown less common services which tests instantiate. Test instance of Firebird and SNMPTrap, as well as clearing temporary files. | ||
|
|
||
| The script is a work in progress based on one engineer machine. Please contribute to improve this. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| @echo off | ||
| echo "Stopping Firebird Server..." | ||
| sc query FirebirdServerTestInstance >nul 2>&1 | ||
| if errorlevel 1 ( | ||
| echo "FirebirdServerTestInstance Service does not exist. Nothing to do." | ||
| ) else ( | ||
| sc stop FirebirdServerTestInstance | ||
| sc delete FirebirdServerTestInstance | ||
| echo "Stopped FirebirdServerTestInstance" | ||
| ) | ||
|
|
||
| echo "Stopping SNMPTrap..." | ||
| sc query SNMPTrap >nul 2>&1 | ||
| if errorlevel 1 ( | ||
| echo "SNMPTrap Service does not exist. Nothing to do." | ||
| ) else ( | ||
| sc stop SNMPTrap | ||
| sc delete SNMPTrap | ||
| echo "Stopped SNMPTrap" | ||
| ) | ||
|
|
||
| if EXIST C:\tests_tmp rmdir /s /q C:\tests_tmp | ||
| if EXIST %PHP_BUILD_DIR%\test_file_cache rmdir /s /q %PHP_BUILD_DIR%\test_file_cache | ||
|
Comment on lines
+1
to
+23
There was a problem hiding this comment. I run a separate version of this locally. This is like the <10th iteration, but I want to get folks thinking on it. The tests crowd people's machines with lots of (I think frankly trash); but I can also see to make this fully flexible, folks really should have the choice to just turn off a service. How then to ensure the services names are known globally rather than shared by convention like this? |
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| @echo off | ||
|
|
||
| set WIN_SCRIPTS_DIR=%~dp0 | ||
|
There was a problem hiding this comment. This doesn't need to be configurable There was a problem hiding this comment. No, but it is shared, so to make the batch files more readable. |
||
| set SCRIPT_DRIVE=%~d0 | ||
| echo WIN_SCRIPTS_DIR=%WIN_SCRIPTS_DIR% | ||
| echo SCRIPT_DRIVE=%SCRIPT_DRIVE% | ||
|
|
||
| if /i "%LOCALBUILD%" equ "True" ( | ||
| set PHP_BUILD_CACHE_BASE_DIR=C:\build-cache | ||
| set PHP_BUILD_OBJ_DIR=C:\obj | ||
| set PHP_BUILD_CACHE_SDK_DIR=C:\build-cache\sdk | ||
| set PHP_BUILD_SDK_BRANCH=php_src-2.2.1-dev | ||
| set PHP_BUILD_CRT=vs16 | ||
| set PLATFORM=x64 | ||
| set THREAD_SAFE=1 | ||
| set INTRINSICS=AVX2 | ||
| set PARALLEL= -j2 | ||
| set OPCACHE=1 | ||
|
|
||
| set CI=True | ||
|
There was a problem hiding this comment. If the scripts are intended to be run locally, we should remove the guard instead. There was a problem hiding this comment. In part of your review, you acknowledge these scripts may not run in github actions. In other places you do not. In any case these scripts are replacing lots of manual commands, so I think this shows that we are running CI (unattended) scripts locally, rather than writing a specific purpose local script. I Have every desire to see if I can help folks not use this for CI. Right now GitHub actions, and my own computer, are the only places it is used. |
||
| set BRANCH=master | ||
| ) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,15 +1,19 @@ | ||
| if /i "%CI%" neq "True" ( | ||
| echo for CI only | ||
| exit /b 3 | ||
| ) | ||
|
|
||
| echo WIN_SCRIPTS_DIR=%WIN_SCRIPTS_DIR% | ||
| echo SCRIPT_DRIVE=%SCRIPT_DRIVE% | ||
|
|
||
| set SDK_RUNNER=%PHP_BUILD_CACHE_SDK_DIR%\phpsdk-%PHP_BUILD_CRT%-%PLATFORM%.bat | ||
| rem SDK_RUNNER=%PHP_BUILD_CACHE_SDK_DIR%\phpsdk-%PHP_BUILD_CRT%-%PLATFORM%.bat | ||
| if not exist "%SDK_RUNNER%" ( | ||
| echo "%SDK_RUNNER%" doesn't exist | ||
| exit /b 3 | ||
| ) | ||
|
|
||
| cmd /c %SDK_RUNNER% -t %WIN_SCRIPTS_DIR%\test_task.bat | ||
| if %errorlevel% neq 0 exit /b 3 | ||
|
|
||
| exit /b 0 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,13 +1,16 @@ | ||
| if /i "%CI%" neq "True" ( | ||
| echo for CI only | ||
| exit /b 3 | ||
| ) | ||
|
|
||
| echo WIN_SCRIPTS_DIR=%WIN_SCRIPTS_DIR% | ||
| echo SCRIPT_DRIVE=%SCRIPT_DRIVE% | ||
|
There was a problem hiding this comment. These echos serve no purpose, please remove them. There was a problem hiding this comment. They actually serve the purpose of knowing the values have been passed. I'm happy to comment them, but they don't cost so much. Is this myopic attention to detail really necessary? It really didn't show in the scripts as they were when I first started reading them.
|
||
|
|
||
| set NO_INTERACTION=1 | ||
| set REPORT_EXIT_STATUS=1 | ||
| set SKIP_IO_CAPTURE_TESTS=1 | ||
|
|
||
| call %WIN_SCRIPTS_DIR%\find-target-branch.bat | ||
| if "%BRANCH%" neq "master" ( | ||
| set STABILITY=stable | ||
| ) else ( | ||
|
|
@@ -30,18 +33,15 @@ set PDO_MYSQL_TEST_PASS=%MYSQL_PWD% | |
| set PDO_MYSQL_TEST_HOST=%MYSQL_TEST_HOST% | ||
| set PDO_MYSQL_TEST_PORT=%MYSQL_TEST_PORT% | ||
| set PDO_MYSQL_TEST_DSN=mysql:host=%PDO_MYSQL_TEST_HOST%;port=%PDO_MYSQL_TEST_PORT%;dbname=test | ||
| if %errorlevel% neq 0 exit /b 3 | ||
|
|
||
| rem setup PostgreSQL related exts | ||
| set PGUSER=postgres | ||
| set PGPASSWORD=Password12! | ||
| rem set PGSQL_TEST_CONNSTR=host=127.0.0.1 dbname=test port=5432 user=postgres password=Password12! | ||
| git checkout "./ext/pgsql/tests/config.inc" | ||
|
There was a problem hiding this comment. To reset the file to it's checked out state. This was explicitly written in #12179, which we interacted on. Like the drop database if not exists, this is so that people outside of GitHub actions can safely use this script without a clean-room environment, which nobody is providing. It will cost nothing in GitHub actions. |
||
| echo ^<?php $conn_str = "host=127.0.0.1 dbname=test port=5432 user=%PGUSER% password=%PGPASSWORD%"; ?^> >> "./ext/pgsql/tests/config.inc" | ||
| set PDO_PGSQL_TEST_DSN=pgsql:host=127.0.0.1 port=5432 dbname=test user=%PGUSER% password=%PGPASSWORD% | ||
|
There was a problem hiding this comment. this incredibly weird command, that doesn't explain what it does at all was replaced by very readable SQL. |
||
| if %errorlevel% neq 0 exit /b 3 | ||
|
|
||
| rem setup ODBC related exts | ||
|
|
@@ -52,7 +52,7 @@ set PDOTEST_DSN=odbc:%ODBC_TEST_DSN% | |
|
|
||
| rem setup Firebird related exts | ||
| curl -sLo Firebird.zip https://github.com/FirebirdSQL/firebird/releases/download/v3.0.9/Firebird-3.0.9.33560-0_x64.zip | ||
| 7z x -y -oC:\Firebird Firebird.zip | ||
|
There was a problem hiding this comment. This would not be needed for a clean-room environment, as it should not be possible to overwrite files that did not exist. |
||
| set PDO_FIREBIRD_TEST_DATABASE=C:\test.fdb | ||
| set PDO_FIREBIRD_TEST_DSN=firebird:dbname=%PDO_FIREBIRD_TEST_DATABASE% | ||
| set PDO_FIREBIRD_TEST_USER=SYSDBA | ||
|
|
@@ -72,10 +72,10 @@ if "%PLATFORM%" == "x64" ( | |
| ) else ( | ||
| set OPENSSLDIR="C:\Program Files (x86)\Common Files\SSL" | ||
| ) | ||
| if /i "%CI%" equ "True" ( | ||
| rmdir /s /q %OPENSSLDIR% >nul 2>&1 | ||
| ) | ||
| if NOT exist %OPENSSLDIR% mkdir %OPENSSLDIR% | ||
| if %errorlevel% neq 0 exit /b 3 | ||
| copy %DEPS_DIR%\template\ssl\openssl.cnf %OPENSSLDIR% | ||
| if %errorlevel% neq 0 exit /b 3 | ||
|
|
@@ -84,21 +84,21 @@ set OPENSSL_CONF= | |
| rem set SSLEAY_CONF= | ||
|
|
||
| rem prepare for OPcache | ||
| if "%OPCACHE%" equ "1" set OPCACHE_OPTS=-d opcache.enable=1 -d opcache.enable_cli=1 -d opcache.protect_memory=1 -d opcache.jit_buffer_size=64M -d opcache.jit_max_root_traces=1000000 -d opcache.jit_max_side_traces=1000000 -d opcache.jit_max_exit_counters=1000000 -d opcache.jit_hot_loop=1 -d opcache.jit_hot_func=1 -d opcache.jit_hot_return=1 -d opcache.jit_hot_side_exit=1 -d opcache.jit=tracing | ||
| rem work-around for failing to dl(mysqli) with OPcache (https://github.com/php/php-src/issues/8508) | ||
| if "%OPCACHE%" equ "1" set OPCACHE_OPTS=%OPCACHE_OPTS% -d extension=mysqli | ||
|
|
||
| rem prepare for enchant | ||
| if NOT EXIST %SCRIPT_DRIVE%\usr\local\lib\enchant-2 mkdir %SCRIPT_DRIVE%\usr\local\lib\enchant-2 | ||
|
There was a problem hiding this comment. I'd like to call out that these also would not affect a clean-room environment, such as GitHub actions; and are not that readable to most competent engineers, because unlike Microsoft developers; in 30 years (probably took a week of frustration), posix devs created short-syntax |
||
| if %errorlevel% neq 0 exit /b 3 | ||
| copy %DEPS_DIR%\bin\libenchant2_hunspell.dll %SCRIPT_DRIVE%\usr\local\lib\enchant-2 | ||
| if %errorlevel% neq 0 exit /b 3 | ||
| if NOT EXIST %SCRIPT_DRIVE%\usr\local\share\enchant\hunspell mkdir %SCRIPT_DRIVE%\usr\local\share\enchant\hunspell | ||
| if %errorlevel% neq 0 exit /b 3 | ||
| echo Fetching enchant dicts | ||
| pushd %SCRIPT_DRIVE%\usr\local\share\enchant\hunspell | ||
| powershell -Command wget http://windows.php.net/downloads/qa/appveyor/ext/enchant/dict.zip -OutFile dict.zip | ||
| unzip -o dict.zip | ||
| del /q dict.zip | ||
| popd | ||
|
|
||
|
|
@@ -115,6 +115,7 @@ hMailServer.exe /verysilent | |
| cd %APPVEYOR_BUILD_FOLDER% | ||
| %PHP_BUILD_DIR%\php.exe -dextension_dir=%PHP_BUILD_DIR% -dextension=com_dotnet appveyor\setup_hmailserver.php | ||
|
|
||
| if EXIST %PHP_BUILD_DIR%\test_file_cache rmdir /s /q %PHP_BUILD_DIR%\test_file_cache | ||
|
There was a problem hiding this comment. In this case instead of |
||
| mkdir %PHP_BUILD_DIR%\test_file_cache | ||
| rem generate php.ini | ||
| echo extension_dir=%PHP_BUILD_DIR% > %PHP_BUILD_DIR%\php.ini | ||
|
|
@@ -130,6 +131,7 @@ for %%i in (ldap oci8_12c pdo_oci) do ( | |
|
|
||
| set TEST_PHPDBG_EXECUTABLE=%PHP_BUILD_DIR%\phpdbg.exe | ||
|
|
||
| if EXIST C:\tests_tmp rmdir /s /q C:\tests_tmp | ||
| mkdir c:\tests_tmp | ||
|
|
||
| set TEST_PHP_JUNIT=c:\junit.out.xml | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why? All jobs start from a clean slate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then this will cost nothing. 😄
I Get it, this script and the approach to maintaining them; are that the environment it happens to work in today, is great and will always be there and supported.
I Do not believe this to be the case.
Can you find me a case where this one-liner command will cause problems?
I Can tell you where problems will happen for anyone not using GitHub actions if they attempt to run a script more than once without a complete clean-room environment, which PHP core does not provide.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A Bit of background on these. If we did a modular PHP build, the commands here would be moved to batch files, folks with chocolatey (listed in the readme) could run. Not only do I think this would be more readable. It would help people to know how to build a box for building Windows. I think it's 1-2 iterations away, which is why I wanted to centralise this setup code for MySQL and PGSQL here (Unknown why MsSQL doesn't need any setup)