Skip to content

Prepare for Windows CI with Github Actions - #9595

Closed
mvorisek wants to merge 7 commits into
php:PHP-8.0from
mvorisek:prepare_for_win_gh_actions_testing
Closed

mvorisek wants to merge 7 commits into
php:PHP-8.0from
mvorisek:prepare_for_win_gh_actions_testing

Conversation

@mvorisek

Copy link
Copy Markdown
Contributor

isolated from #8392 PR, these changes are mostly renames

before merge, please squash into one commit

@iluuu1994

Copy link
Copy Markdown
Member

Why is moving all these files needed?

@mvorisek

Copy link
Copy Markdown
Contributor Author

it is not needed, but some CI directories are prefixed with dot, some not, this unifies the file structure

@iluuu1994

Copy link
Copy Markdown
Member

Then please keep them as is.

@iluuu1994 iluuu1994 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Comment thread .github/workflows/push.yml Outdated
Comment thread .github/actions/configure-x64/action.yml Outdated
Comment thread azure/setup.yml Outdated
Comment thread buildconf.bat Outdated
Comment thread win32/build/configure.bat Outdated
Comment thread win32/build/phpize.bat Outdated
Comment thread ext/standard/tests/file/windows_acls/common.inc Outdated
@mvorisek
mvorisek force-pushed the prepare_for_win_gh_actions_testing branch 2 times, most recently from c37298c to c70e625 Compare September 30, 2022 16:32
@mvorisek

Copy link
Copy Markdown
Contributor Author

Then please keep them as is.

the intention is to separate CI files/directories more which is quite common and helpful for quick orientation in the file structure, would you be fine with prefixing the CI directories (and unifying with .github dir) in master?

@iluuu1994

Copy link
Copy Markdown
Member

would you be fine with prefixing the CI directories (and unifying with .github dir) in master?

I know Git can sometimes recognize renames but it has still been a pain for me in the past, so I think we should keep this as is.

@mvorisek
mvorisek force-pushed the prepare_for_win_gh_actions_testing branch from 707d6cf to 1c588ed Compare September 30, 2022 17:13
@mvorisek
mvorisek force-pushed the prepare_for_win_gh_actions_testing branch from bb96c0a to 56110d3 Compare September 30, 2022 21:07
@mvorisek

mvorisek commented Oct 1, 2022

Copy link
Copy Markdown
Contributor Author

@iluuu1994 the MacOS CI error is unrelated, can you please do a final review and if all ok, please squash and merge

@iluuu1994 iluuu1994 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! I'll merge this soon.

@mvorisek

mvorisek commented Oct 7, 2022

Copy link
Copy Markdown
Contributor Author

@iluuu1994 is soon now? :)

@iluuu1994

Copy link
Copy Markdown
Member

Merged as b43e494. Note: In PHP-8.2 appveyor/setup_hmailserver.php was added so we might want to move that one too.

@iluuu1994 iluuu1994 closed this Oct 9, 2022
@mvorisek
mvorisek deleted the prepare_for_win_gh_actions_testing branch October 9, 2022 16:52
@mvorisek

mvorisek commented Oct 9, 2022

Copy link
Copy Markdown
Contributor Author

Thank you, I added your comment about hmailserver into #9650.

I would like to add Windows 32 bit testing as well. Almost everything is done, but #9650 is blocking the PR, as it causes like every 3rd CI run or so to fail. Do you have any idea what can be the problem and do you have access to Windows to run like hundred of full testing cycles of x86/32 bit build to reproduce?

@iluuu1994

Copy link
Copy Markdown
Member

I don't have a Windows machine so I can't help there. Apparently MSVC now has a address sanitizer, you could try that.

https://devblogs.microsoft.com/cppblog/address-sanitizer-for-msvc-now-generally-available/

@cmb69

cmb69 commented Oct 9, 2022

Copy link
Copy Markdown
Member

Apparently MSVC now has a address sanitizer, you could try that.

See #6964

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.

3 participants