-
Notifications
You must be signed in to change notification settings - Fork 7.8k
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
single automated Windows build command, ext/skeleton addons #11125
Conversation
- Will compile to binary with single: `.\cmd\compile_x64.bat --php version --snap --shared --ts --deps pthread --ext name --option enable_extras` Same command appled to `ext` repo will download and compile all necceaary soucres. In addition skeleton has some base GitHub Actions CI setup to produce Windows binary directly to releases page, and if desired other OS platforms , no PECL needed.
I think there are some nice things in here. I see a check for 7.4, but since that version is not supported anymore, that check may be dropped. |
This is user focus, meaning the person needing to build Windows from source, so as the version should be passed or otherwise defaults to branch, where the script should have be edited to match branch.
Where are the hard coded paths? I guess you not aware of Build your own PHP on Windows. This PR just automates that as stated. Anyway, what operating system don't have certain aspects of there system paths fixed?
I'm not sure why you are replying to this, since you don't know Windows at all. Why you think I need feedback/help on anything? I have my own working solution that I have been using for a while.
Where is the syncing going on with master? There is two build scripts, one for PHP source and another for extensions. It can be local or remote.
|
I meant the paths to the visual studio installation. So these would be nice to have a single definition somewhere or at least reduce the hardcoding.
I am aware of this as I have a Windows VM where I sometimes do debugging in.
You made a PR, so that means you want feedback such that this can be integrated into php-src.
I don't, others do. I was speaking only for myself.
Your merge commits.
Yes the idea here sounds good. |
@TheTechsTech I am working on something similar with the existing test automation infrastructure. You have some interesting ideas in this, which I'd be interested in knowing more about. Appreciate if you have time 👀 on #12281 I believe for example that you may have more information about 32-bit windows builds and maybe even setting up conditional extensions. |
I checked it out, I don't think I can add much to what you have. I have nothing specific about 32bit. When I set up the build system it's intentions is to be a single local and GitHub Actions process. It just restructures there current documented multi multi step of there local windows build procesd to a single workflow step. The scripts I put forth is for one needing to develop an extension for whatever PHP version they need. The extension setup skeleton and CI actions is designed and expected to be customized by the user, with really small edits. Not sure you actually tried using the scripts here to build with those extensions enabled in your PR. |
@TheTechsTech I didn't sorry. It seemed too far a leap from the commands currently being used (which I may later understand is a good thing). For me looking at this, the fact it seemed to be missing MySQL, Postgres, Firebird and SNMP weirdness CI is doing was enough to say I'd be reducing visibility by jumping to this as a base; but I'm open to that being regrettable decision-making in the future and really loved the modular approach this seemed to suggest. |
@echo off | ||
if EXIST "C:\Program Files (x86)\Microsoft Visual Studio\2019\Community" ( | ||
"C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Auxiliary\Build\vcvars64.bat" && cmd\getopt.bat %* && set ARCH=x64&& cmd\make_php.bat | ||
) else if EXIST "C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise" ( | ||
"C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Auxiliary\Build\vcvars64.bat" && .\cmd\getopt.bat %* && set ARCH=x64&& .\cmd\make_php.bat | ||
) |
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.
These I simply used the system PATH for and assume the machine has been setup with a PATH to use the correct tooling. I Don't try to own that detail.
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.
What you not seeing/realizing everything is setup to be local to the folder you're in. The scripts folder itself can be zipped and extracted, and all building starts there.
The scripts actually downloading PHP if not present in folder current setup.
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.
so in theory a benefit of this approach is that I could run parallel builds without setting up separate users, or messing with the PATH variable.
Is that common for windows development?
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.
What? The whole thing is customized from there scripts, the reason it works. So NO! It's not following there current system steps.
And it still seems, that is the part you not realizing, you have to write/edit there steps over. You haven't run my build setup or anything, don't see how you can comment on anything. You don't use Windows daily, otherwise you realize why it's the way it is.
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.
Have you considered you are particularly bad at explaining yourself because of these hysterical outbursts?
I Still have no idea why I'd use this methodology, and was asking you a direct question which you've not answered.
set CRT=vs16 | ||
) | ||
|
||
if NOT DEFINED ARCH set ARCH=x64 |
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.
😂 I had no idea this was the syntax for optional environment vars [in batch files]... I Hate windows so much.
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.
That's the whole issue to begin with. You don't do Windows or like, and trying to understand what's happening.
It might be best you review there official building steps and look at the scripts they have in there.
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.
sorry, but compared to the syntax I am used to which works on multiple operating systems, is much shorter, and has more developer mindshare.
https://stackoverflow.com/a/2013589/1548557
Also this syntax is not present in the existing PHP build system.
I'm not evaluating this simply based upon familiarity. If it is part of a build system used by PHP; I'd like it to at least be internally similar and work towards other mechanisms.
Also while I joked, I added a ticket to my own investigation to see if this syntax could make things more readable.
Edit: I can see how the last part of my response is confrontational here. I apologise for my part in that. Please ignore it if you read it. I've edited it out.
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.
It's doesn't matter about your jokes. Most PHP only programmers have that problem. It just seem you trying to come up with a solution when you really don't know Windows at all.
Read there current official Windows process that I changed and the reason it works in a single step. You reading scripts with no Windows background and making judgement callsthen asking at the same time.
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.
Bro, I don't only code PHP; I don't code exclusively for windows, or tie myself to Microsoft, since maybe 15 years ago, although I do have projects that build on windows.
I know windows enough. Perhaps some darker edges of it, I don't know nearly as well as I know more competent systems that folks deploy to. In a market share, this is not an argument to have.
I like that you've wrapped up several commands, which still happen into one runnable entrypoint. I've asked you about this; because thus far, the official GitHub actions performs more steps than you have aggregated. Someone would need to add Firebase, MSSQL, pgSQL, MySQL, SNMP at least. But you've just littered files with a single example and called it done. This won't move forward without you taking the time to explain what and why you've done things.
Just read the wiki it says to do this. Then why is it editing centos and other OS targets as well? None of this is particularly well explained. Stop being mad, unless you want to be the smartest person that nobody is listening to. You submitted this in April; maybe if you can pause for a minute it can be merged before next April and we'll all recognise how smart a solution this is.
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.
I'm not sure why you still replying/comments on this PR.
I can't help you. Read https://wiki.php.net/internals/windows/stepbystepbuild which is mentioned in this PR. Download those scripts review them, then submit PR to original author.
You should actually read my first post of PR and the reply that went south the moment he mentions about Windows.
Thank you for the PR! But please stay civil. If users comment, and you don't deem their reply useful, just ignore it. No need to offend anybody. Regarding the PR: I don't think it makes sense to put a comprehensive build solution for Windows into php-src. There are no such solutions for other platforms, so what would be special about Windows here? Furthermore, we already have running CI (obviously in this repo), and there are working build solutions for Windows (https://github.com/cmb69/php-ftw, soon to be superseded by https://github.com/php/php-windows-builder which is already used to build PECL packages). To build PHP extension on Github CI on Windows, there is https://php/setup-php-sdk, or probably better https://github.com/php/php-windows-builder. And those users who want to do local builds can follow the instructions on https://wiki.php.net/internals/windows/stepbystepbuild_sdk_2 (and may quickly write a couple of lines to automate the repeated processes). So in my opinion, this PR should be rejected, if only to avoid even more complex maintainance (there is quite some code which basically is already part of https://github.com/php/php-sdk-binary-tools). What do you think, @shivammathur? |
Yes, I agree with rejecting this PR. Including the same functionality as Also, we have tooling now that one can use to build extensions as you pointed out, also we are doing PECL builds again. |
I see this would solve some standing issues with PECL and Windows .dll, #10850, grpc/grpc#32304, https://externals.io/message/117494.
Running:
.\cmd\compile_x64.bat --php version --snap --shared --ts --option enable_extras
Will download php-sdk and make an symbolic directory junction link for
everything to function as the Windows step-by-step build instructions states,
but without any more user interaction.
Should default to branch version if
--php
omitted.In case of a module/extension repo, the script will download the PHP version stated,
in addition download and compile dependency from source, an symbolic directory junction link
will also be made to get everything to function without any more user interaction.
Defaults current
8.2.5
version if--php
omitted.Here is demonstration of ext-parallel.
To run tests
.\cmd\windows_run_test.bat --php version --ts --arch x64
The skeleton additions also adds some base GitHub Actions CI setup to produce
Windows binary directly to releases page, in addition to other OS platforms
if desired, an example is libuv ext-uv extension at https://github.com/symplely/ext-uv/releases
The
.\cmd\getopt.bat
and.\cmd\deps_build.bat
should be the only filesneeding heavy modifying by extension author, and
deps_build.bat
must be renamed.