Skip to content

Fix GH-13932: Attempt to fix mbstring on windows build (msvc). #13933

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

Closed
wants to merge 1 commit into from

Conversation

devnexen
Copy link
Member

No description provided.

@nielsdos
Copy link
Member

These can get out of sync, use a define instead.

@devnexen
Copy link
Member Author

I see

@devnexen devnexen force-pushed the fix_mbstring_windows_build branch from 1abc71d to cec6e96 Compare April 10, 2024 08:13
Copy link
Member

@nielsdos nielsdos left a comment

Choose a reason for hiding this comment

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

Thanks.
This should go into branch PHP-8.3.
And then @bukka can cherry pick this into PHP-8.3.5

@nielsdos nielsdos linked an issue Apr 10, 2024 that may be closed by this pull request
@devnexen
Copy link
Member Author

Thanks. This should go into branch PHP-8.3. And then @bukka can cherry pick this into PHP-8.3.5

oh gotcha was not sure about this. cheers.

@devnexen
Copy link
Member Author

Merged via 2cfd9df1

@devnexen devnexen closed this Apr 10, 2024
@bukka
Copy link
Member

bukka commented Apr 10, 2024

it cannot go to 8.3.5 as it is already tagged. We might need to do immediate 8.3.6 instead.

@bwoebi
Copy link
Member

bwoebi commented Apr 10, 2024

@bukka I thought standard procedure were a retag for such cases? As long as it's not announced, that is. (At least retags did happen in the past.)

@nielsdos
Copy link
Member

@bukka I thought standard procedure were a retag for such cases? As long as it's not announced, that is. (At least retags did happen in the past.)

We no longer retag for good reasons: #11868

@bwoebi
Copy link
Member

bwoebi commented Apr 10, 2024

@nielsdos Ah, I just wanted to bring 8.2.9 as the example where we retagged in the past.
I do however think retagging is still the right way. People should not assume tag == release. But anyway, it seems policy now then? Then disregard my comments. I don't want to start a discussion here.

@nielsdos
Copy link
Member

@nielsdos Ah, I just wanted to bring 8.2.9 as the example where we retagged in the past. I do however think retagging is still the right way. People should not assume tag == release.

The additional problem however is how git handles force pushes to tags (see the issue).

@bwoebi
Copy link
Member

bwoebi commented Apr 10, 2024

@nielsdos While we are here, can you please remind me the reason why we have two different days for tagging and releasing? Can't the builds be run just on the HEAD of the php-x.y.z branch, triggered manually once ready? Rather than a tag.

@nielsdos
Copy link
Member

@nielsdos While we are here, can you please remind me the reason why we have two different days for tagging and releasing? Can't the builds be run just on the HEAD of the php-x.y.z branch, triggered manually once ready? Rather than a tag.

I have no idea honestly, I can only imagine it is to do last minute checks if something wrong went into the tag, but that's speculation.

@bukka
Copy link
Member

bukka commented Apr 10, 2024

No re-tagging is just completely wrong approach that goes against all good practices. So it's not going to be done in that way.

@bukka
Copy link
Member

bukka commented Apr 10, 2024

I think we should remove this delay of two days as well as it's a big flaw in our security disclosure handling.

@devnexen
Copy link
Member Author

I would suggest to never use VLA again :) regardless of compiler support, rarely needed in practice I think.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

mbstring cannot be built on Windows (commit f7e7370)
4 participants