Skip to content
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

Fix debug crash #1894

Merged
merged 3 commits into from
Jan 23, 2025
Merged

Fix debug crash #1894

merged 3 commits into from
Jan 23, 2025

Conversation

duckdoom5
Copy link
Contributor

This fixes crashes when running the parser in the debug builds, due to the assert getting triggered. This will also fix CI for #1893.

Implementation is based on #1819.

Additionally I've added a commit that updates the assert to trigger a debug break, to automatically get notified of any errors. It will now also trigger in DebugOpt builds

@tritao
Copy link
Collaborator

tritao commented Jan 23, 2025

Seems like its failing due to: https://github.blog/changelog/2024-04-16-deprecation-notice-v3-of-the-artifact-actions/

Mind updating the version of the action too? Probably enough to just switch to v4.

Not having much luck lately with CI deprecations 🫤

@duckdoom5
Copy link
Contributor Author

Seems like its failing due to: https://github.blog/changelog/2024-04-16-deprecation-notice-v3-of-the-artifact-actions/

Mind updating the version of the action too? Probably enough to just switch to v4.

Not having much luck lately with CI deprecations 🫤

Ah yeah, I did update it in #1893. I could port that over to this commit if you'd like to test this PR?

@tritao
Copy link
Collaborator

tritao commented Jan 23, 2025

I thought we need to get this merged first, right? Because you said this fixes the CI for the other PR. In that case then yes.

@duckdoom5
Copy link
Contributor Author

Sure thing, let me get that done in a bit. It's not really 'needed', but then we can merge these both with green checks.

@duckdoom5
Copy link
Contributor Author

That should do it I think

@duckdoom5
Copy link
Contributor Author

duckdoom5 commented Jan 23, 2025

Ah I missed one line when I ported this over. Fixed that now (intermediate-Release-x64 should be added by the other commit)

@tritao
Copy link
Collaborator

tritao commented Jan 23, 2025

Looks pretty good, nice work :)

@tritao tritao merged commit c25c7df into mono:main Jan 23, 2025
3 of 4 checks passed
@duckdoom5 duckdoom5 deleted the fix-debug-crash branch January 23, 2025 21:57
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.

2 participants