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

Propose a new (1.2.3) release #2378

Merged
merged 1 commit into from
Aug 22, 2023
Merged

Propose a new (1.2.3) release #2378

merged 1 commit into from
Aug 22, 2023

Conversation

loganek
Copy link
Collaborator

@loganek loganek commented Jul 21, 2023

I'm opening the PR to discuss the next, 1.2.3 release. The last version (1.2.2) was released two months ago and I think we've got quite a few significant changes (AOT improvements, bugfixes) to progress to the next release. Please let me know if the internal testing can be done any time soon; I'd be more than happy to help updating this PR with release notes if others agree this is the right time to make the release.

Also, I wasn't sure if we should update the patch or minor version, but after reviewing the changes, I feel like we should stick to 1.2 version.

cc: @wenyongh @xwang98

@wenyongh
Copy link
Contributor

@loganek Yes, agree to release 1.2.3 personally and after merging the exception handling feature (see #2382), we can prepare to release 1.3.0. We will test more cases internally, and let you know when it is done. Thanks.

@loganek
Copy link
Collaborator Author

loganek commented Aug 1, 2023

@wenyongh I just updated the branch including latest changes (there were a few minor changes) and updated the changelog. When do you think we'll be ready to make a release?

@wenyongh
Copy link
Contributor

wenyongh commented Aug 1, 2023

@wenyongh I just updated the branch including latest changes (there were a few minor changes) and updated the changelog. When do you think we'll be ready to make a release?

@loganek I am waiting several PRs to merge, #2410, #2394 and #2364. And there are several issues reported: #2391, #2397, #2401, #2412, do you agree to leave them alone in this release and try to fix them later? If yes, we may test again after the three PRs are merged, and then release the new version if there is no issue found.

@loganek
Copy link
Collaborator Author

loganek commented Aug 1, 2023

Good point on the open issues. Are they new issues or they also exist in the 1.2.2 release? If those bugs were introduced after 1.2.2 release, I'd say we should fix them before making a new release. Otherwise, we can probably make a new release given all the enhancements that were added to WAMR already (and since those bugs existed before, we're not making things any worse), and make another release once those issues are resolved. What do you think?

@wenyongh
Copy link
Contributor

wenyongh commented Aug 1, 2023

Good point on the open issues. Are they new issues or they also exist in the 1.2.2 release? If those bugs were introduced after 1.2.2 release, I'd say we should fix them before making a new release. Otherwise, we can probably make a new release given all the enhancements that were added to WAMR already (and since those bugs existed before, we're not making things any worse), and make another release once those issues are resolved. What do you think?

Agree.
#2391 and #2397 should also exist in 1.2.2.
#2401 seems also exist in 1.2.2 since fast-interpreter wasn't modified a lot after 1.2.2. This issue is complex and may take more time to fix it.
Not sure #2412 was introduced after 1.2.2 since we really modified AOT compiler a lot after 1.2.2. Could you help check?

@loganek
Copy link
Collaborator Author

loganek commented Aug 1, 2023

@eloparco just confirmed the issue #2412 was introduced after 1.2.2 release, so let's block the release until we get that fixed.

@wenyongh
Copy link
Contributor

wenyongh commented Aug 1, 2023

@eloparco just confirmed the issue #2412 was introduced after 1.2.2 release, so let's block the release until we get that fixed.

OK, got it.

@wenyongh
Copy link
Contributor

wenyongh commented Aug 9, 2023

@eloparco just confirmed the issue #2412 was introduced after 1.2.2 release, so let's block the release until we get that fixed.

@loganek, @eloparco Any progress on issue #2412? We have basically finished the test for internal test cases, I think it should be OK to release 1.2.3 if #2412 is resolved.

@Zzzabiyaka
Copy link
Contributor

Zzzabiyaka commented Aug 11, 2023

probably another bug we should address before release as well as #2412

#2453

Just so that we know about it.

I've run it with release version of wamrc and main branch version, works for released version and doesn't work for current version

Looks pretty similar to #2384

Feel free to ignore if we don't need it for release

@loganek
Copy link
Collaborator Author

loganek commented Aug 11, 2023

@wenyongh: @eloparco is currently on holiday and so there wasn't any progress on that ticket I'm afraid.

@Zzzabiyaka: is it the same commit that breaks the compiler?

@wenyongh: If both issues are caused by the same faulty commit (#2244), I wonder if we could roll it back to not block the release and merge it again with the fix included?

cc: @yamt who's the author of that commit.

@yamt
Copy link
Collaborator

yamt commented Aug 14, 2023

@wenyongh: If both issues are caused by the same faulty commit (#2244), I wonder if we could roll it back to not block the release and merge it again with the fix included?

cc: @yamt who's the author of that commit.

by "both issues", which issues do you mean?
#2412 and?

@wenyongh
Copy link
Contributor

@wenyongh: If both issues are caused by the same faulty commit (#2244), I wonder if we could roll it back to not block the release and merge it again with the fix included?
cc: @yamt who's the author of that commit.

by "both issues", which issues do you mean? #2412 and?

@yamt I think @loganek means issue #2453? See the above reply post by @Zzzabiyaka:

probably another bug we should address before release as well as #2412

#2453

Just so that we know about it.

I've run it with release version of wamrc and main branch version, works for released version and doesn't work for current version

Looks pretty similar to #2384

Feel free to ignore if we don't need it for release

@Zzzabiyaka
Copy link
Contributor

Hi I'm sorry

I'll close #2453. I was just running tests on the old version of wamr (v1.2.2) that's why they were failing

Not an issue anymore. Thanks @yamt for clarifying

@loganek
Copy link
Collaborator Author

loganek commented Aug 17, 2023

@wenyongh looks like the critical issues have been resolved; I also rebased ono top of main and updated release notes. Are we ready to make the release now or do you see any blockers?

@wenyongh
Copy link
Contributor

@wenyongh looks like the critical issues have been resolved; I also rebased ono top of main and updated release notes. Are we ready to make the release now or do you see any blockers?

Yes, seems most of the critical issues were resolved. There are two issues left in Windows platform, one was introduced after 1.2.2 and I submitted PR #2474 to fix it; the other is an old issue, see issue #2242, I hasn't tested the method that yamt mentioned (use RtlAddFunctionTable to register .pdata section), I just submitted PR #2475 to fix it, but it is not a best solution.

Another issue is #2401, currently we are trying to fix it and hope to submit patch soon.

My suggestion is to wait a bit more time to merge these PRs and then test again, and if no issues found, we can create the release. What's you opinion?

@wenyongh
Copy link
Contributor

PS: @XuJun2019 just submitted PR #2476 to fix #2401.

@wenyongh
Copy link
Contributor

@loganek I have merged all the left PRs and done some test, it looks good and no issue was found yet. Could you help update this PR? Let's create the release after this PR is merged?

@loganek
Copy link
Collaborator Author

loganek commented Aug 18, 2023

Thanks a lot. I'm on holiday till the end of the week but will update the PR on Monday

@loganek
Copy link
Collaborator Author

loganek commented Aug 21, 2023

@wenyongh I just updated the PR with the latest changes

@wenyongh wenyongh merged commit 7f996a4 into bytecodealliance:main Aug 22, 2023
368 checks passed
@wenyongh
Copy link
Contributor

@loganek Many thanks. The release 1.2.3 was successfully created after some fixings:
https://github.com/bytecodealliance/wasm-micro-runtime/releases/tag/WAMR-1.2.3

@wenyongh
Copy link
Contributor

BTW, I also merged branch main into dev/wasi-libc-windows, there is a conflict in file tests/wamr-test-suites/wasi-test-script/run_wasi_tests.sh, I manually fixed it. The changes to the current file is as below, please check whether there is issue and help fix if found:

diff --git a/tests/wamr-test-suites/wasi-test-script/run_wasi_tests.sh b/tests/wamr-test-suites/wasi-test-script/run_wasi_tests.sh
index b5f7fad0..bebd3a16 100755
--- a/tests/wamr-test-suites/wasi-test-script/run_wasi_tests.sh
+++ b/tests/wamr-test-suites/wasi-test-script/run_wasi_tests.sh
@@ -27,6 +27,7 @@ readonly WAMR_DIR="${WORK_DIR}/../../../.."
 readonly IWASM_CMD="${IWASM_EXE} \
     --allow-resolve=google-public-dns-a.google.com \
     --addr-pool=::1/128,127.0.0.1/32"
+
 readonly IWASM_CMD_STRESS="${IWASM_CMD} --max-threads=8"
 readonly WAMRC_CMD="${WORK_DIR}/../../../../wamr-compiler/build/wamrc"
 readonly C_TESTS="tests/c/testsuite/"
@@ -34,14 +35,18 @@ readonly ASSEMBLYSCRIPT_TESTS="tests/assemblyscript/testsuite/"
 readonly THREAD_PROPOSAL_TESTS="tests/proposals/wasi-threads/"
 readonly THREAD_INTERNAL_TESTS="${WAMR_DIR}/core/iwasm/libraries/lib-wasi-threads/test/"
 readonly LIB_SOCKET_TESTS="${WAMR_DIR}/core/iwasm/libraries/lib-socket/test/"
+readonly STRESS_TESTS=("spawn_stress_test.wasm" "stress_test_threads_creation.wasm")
 
 run_aot_tests () {
     local tests=("$@")
+    local iwasm="${IWASM_CMD}"
     for test_wasm in ${tests[@]}; do
         local extra_stress_flags=""
-        if [[ "$test_wasm" =~ "stress" ]]; then
-            extra_stress_flags="--max-threads=8"
-        fi
+        for stress_test in "${STRESS_TESTS[@]}"; do
+            if [ "$test_wasm" == "$stress_test" ]; then
+                iwasm="${IWASM_CMD_STRESS}"
+            fi
+        done
 
         test_aot="${test_wasm%.wasm}.aot"
         test_json="${test_wasm%.wasm}.json"
@@ -61,7 +66,6 @@ run_aot_tests () {
         fi
 
         ${IWASM_CMD} $extra_stress_flags $test_aot
-
         ret=${PIPESTATUS[0]}
 
         echo "expected=$expected, actual=$ret"
@@ -75,15 +79,18 @@ if [[ $MODE != "aot" ]];then
     $PYTHON_EXE -m venv wasi-env && source wasi-env/${VENV_BIN_DIR}/activate
     $PYTHON_EXE -m pip install -r test-runner/requirements.txt
 
-    # Stress test requires max-threads=8 so it's run separately
-    if [[ -e "${THREAD_INTERNAL_TESTS}spawn_stress_test.wasm" ]]; then
-        ${IWASM_CMD_STRESS} ${THREAD_INTERNAL_TESTS}spawn_stress_test.wasm
-        ret=${PIPESTATUS[0]}
-        if [ "${ret}" -ne 0 ]; then
-            echo "Stress test spawn_stress_test FAILED with code " ${ret}
-            exit_code=${ret}
+    # Stress tests require max-threads=8 so they're executed separately
+    for stress_test in "${STRESS_TESTS[@]}"; do
+        if [[ -e "${THREAD_INTERNAL_TESTS}${stress_test}" ]]; then
+            echo "${stress_test}" is a stress test
+            ${IWASM_CMD_STRESS} ${THREAD_INTERNAL_TESTS}${stress_test}
+            ret=${PIPESTATUS[0]}
+            if [ "${ret}" -ne 0 ]; then
+                echo "Stress test ${stress_test} FAILED with code " ${ret}
+                exit_code=${ret}
+            fi
         fi
-    fi
+    done
 
     TEST_RUNTIME_EXE="${IWASM_CMD}" $PYTHON_EXE test-runner/wasi_test_runner.py \
             -r adapters/wasm-micro-runtime.py \
@@ -114,4 +121,4 @@ else
     done
 fi
 
-exit ${exit_code}
\ No newline at end of file
+exit ${exit_code}

@loganek
Copy link
Collaborator Author

loganek commented Aug 23, 2023

Thanks @wenyongh , the change looks ok to me, I think you correctly resolved the conflict.

victoryang00 pushed a commit to victoryang00/wamr-aot-gc-checkpoint-restore that referenced this pull request May 27, 2024
Update the version number to 1.2.3 and update the release notes.
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.

4 participants