-
Notifications
You must be signed in to change notification settings - Fork 697
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
3.1.56 #1353
3.1.56 #1353
Conversation
I wonder why build-docker-image fails... Any ideas? |
Looks like an actual emscripten bug related to emscripten-core/emscripten#21456 |
Weird, I can't reproduce that error locally on |
Is emscripten-core/emscripten#21523 supposed fix this or is this a separate problem? |
Yup that is the fix. You can update this PR or create a new one once that has rolled in. |
Thank you! |
Hm, how did land? It looks like it did not contain https://chromium.googlesource.com/emscripten-releases/+/8385c9ea32774b0bb8d205cc329c5db38d90da31.. which means that mac builder should have failed in the same way as before. I was expecting that you would need to create a new RC after https://chromium.googlesource.com/emscripten-releases/+/8385c9ea32774b0bb8d205cc329c5db38d90da31 but I don't see that? Unless I'm missing something? |
Yes, looks like this 3.1.56 release does not include emscripten-core/emscripten#21523.. not sure how the CI passed. |
After emscripten-core/emscripten#21523 landed in emscripten-release, it still failed saying (I also reran the CI of #1355, which seemed to pass) |
Oh wait, do you mean, passing the CI here is not sufficient, and the release itself has to contain the commits? In that case I have to redo the whole release with the different emscripten commit... Should I do that? Is that gonna be the problem? |
Yes, you need create a new RC release and then I guess now we should call it 3.1.57 since 3.1.56 has now effectively been shipped. |
I guess that tests in question here were somehow running against |
We can modify releases here, can't we? I think we did that in the past. It seems better if the short-term confusion is outweighed by what that commit fixes. |
This means that CI run that update `latest` actually test the thing we are about to ship. We recently had a case where `latest` was broken but `tot` was fixes and we accidentally shipped a broken SDK version (#1353).
I think I would be OK with that. Some folks may already have downloaded the broken release.. but I think they should get updated if we update the hashes alone. |
You mean, manually only fix the emscripten hash here? But we should do that in emscripten-release too, no? |
Yes, either way we need to trigger a new LTO build on emscripten-release |
OK will start a new one from scratch... |
This means that CI run that update `latest` actually test the thing we are about to ship. We recently had a case where `latest` was broken but `tot` was fixes and we accidentally shipped a broken SDK version (#1353).
This means that CI run that update `latest` actually test the thing we are about to ship. We recently had a case where `latest` was broken but `tot` was fixes and we accidentally shipped a broken SDK version (#1353).
Yes, we need a new LTO build. But with that, we can trample this release. When people update their emsdk it will download the fixed release, so the confusion is only temporary. |
How can we "trample" this release? |
Basically you would re-run the One option would be to revert the 7815dca change locally before you run the script. Another option would be to run the script and then edit the resulting change. |
Sorry, yes. |
emscripten-core#1353 turned out to be broken, so this is the second try.
After #21518 landed, the previous 3.1.56 release (emscripten-core/emsdk#1353) turned out to be broken so we replaced 3.1.56 with the new one (emscripten-core/emsdk#1360). So here to be precise we update the date of the new 3.1.56 release.
No description provided.