Hello!
I got 3 of these messages about a patch I have upstream, which seems excessive. I also just managed to grab an aarch64 box and test it and got no errors, so I'd appreciate someone double checking that those werent 3 false positives please.
Thank you! Guinevere Larsen She/Her/Hers -------- Forwarded Message -------- Subject: [Linaro-TCWG-CI] 2 patches in gdb: Failure Date: Mon, 25 Sep 2023 05:37:49 +0000 (UTC) From: ci_notify@linaro.org Reply-To: linaro-toolchain@lists.linaro.org To: blarsen@redhat.com
Dear contributor, our automatic CI has detected problems related to your patch(es). Please find some details below. If you have any questions, please follow up on linaro-toolchain@lists.linaro.org mailing list, Libera's #linaro-tcwg channel, or ping your favourite Linaro toolchain developer on the usual project channel.
In CI config tcwg_gdb_check/master-aarch64 after:
| 2 patches in gdb | Patchwork URL: https://patchwork.sourceware.org/patch/76616 | ca7351280f6 gdb/infrun: simplify process_event_stop_test | 0e77782d402 gdb/record: print frame information when exiting a recursive call | ... applied on top of baseline commit: | be8e8313099 Automatic date update in version.in
Results changed to # reset_artifacts: -10 # build_abe gdb -- --prefix /usr --disable install: # FAILED # build_abe dejagnu: # build_abe check_gdb --: # First few build errors in logs: # 00:10:07 /home/tcwg-build/workspace/tcwg_gnu_0/abe/snapshots/gdb.git~master/gdb/infrun.c:7012:17: error: ‘curr_frame_id’ was not declared in this scope; did you mean ‘outer_frame_id’? # 00:10:07 /home/tcwg-build/workspace/tcwg_gnu_0/abe/snapshots/gdb.git~master/gdb/infrun.c:7185:14: error: ‘curr_frame_id’ was not declared in this scope; did you mean ‘outer_frame_id’? # 00:10:07 /home/tcwg-build/workspace/tcwg_gnu_0/abe/snapshots/gdb.git~master/gdb/infrun.c:7185:31: error: ‘original_frame_id’ was not declared in this scope # 00:10:07 /home/tcwg-build/workspace/tcwg_gnu_0/abe/snapshots/gdb.git~master/gdb/infrun.c:7615:8: error: ‘curr_frame_id’ was not declared in this scope; did you mean ‘outer_frame_id’? # 00:10:07 /home/tcwg-build/workspace/tcwg_gnu_0/abe/snapshots/gdb.git~master/gdb/infrun.c:7615:25: error: ‘original_frame_id’ was not declared in this scope # 00:10:07 /home/tcwg-build/workspace/tcwg_gnu_0/abe/snapshots/gdb.git~master/gdb/infrun.c:7663:11: error: ‘curr_frame_id’ was not declared in this scope; did you mean ‘outer_frame_id’? # 00:10:07 /home/tcwg-build/workspace/tcwg_gnu_0/abe/snapshots/gdb.git~master/gdb/infrun.c:7663:28: error: ‘original_frame_id’ was not declared in this scope # 00:10:09 make[1]: *** [Makefile:1923: infrun.o] Error 1 # 00:10:09 make: *** [Makefile:12387: all-gdb] Error 2 # 00:09:18 /home/tcwg-build/workspace/tcwg_gnu_0/abe/snapshots/gdb.git~master/gdb/infrun.c:7012:17: error: ‘curr_frame_id’ was not declared in this scope; did you mean ‘outer_frame_id’? # 00:09:18 /home/tcwg-build/workspace/tcwg_gnu_0/abe/snapshots/gdb.git~master/gdb/infrun.c:7185:14: error: ‘curr_frame_id’ was not declared in this scope; did you mean ‘outer_frame_id’? # 00:09:18 /home/tcwg-build/workspace/tcwg_gnu_0/abe/snapshots/gdb.git~master/gdb/infrun.c:7185:31: error: ‘original_frame_id’ was not declared in this scope # 00:09:18 /home/tcwg-build/workspace/tcwg_gnu_0/abe/snapshots/gdb.git~master/gdb/infrun.c:7615:8: error: ‘curr_frame_id’ was not declared in this scope; did you mean ‘outer_frame_id’? # 00:09:18 /home/tcwg-build/workspace/tcwg_gnu_0/abe/snapshots/gdb.git~master/gdb/infrun.c:7615:25: error: ‘original_frame_id’ was not declared in this scope # 00:09:18 /home/tcwg-build/workspace/tcwg_gnu_0/abe/snapshots/gdb.git~master/gdb/infrun.c:7663:11: error: ‘curr_frame_id’ was not declared in this scope; did you mean ‘outer_frame_id’? # 00:09:18 /home/tcwg-build/workspace/tcwg_gnu_0/abe/snapshots/gdb.git~master/gdb/infrun.c:7663:28: error: ‘original_frame_id’ was not declared in this scope # 00:09:22 make[1]: *** [Makefile:1924: infrun.o] Error 1 # 00:09:43 make: *** [Makefile:12387: all-gdb] Error 2
From # reset_artifacts: -10 # build_abe gdb -- --prefix /usr --disable install: -2 # build_abe dejagnu: -1 # build_abe check_gdb --: 0
-----------------8<--------------------------8<--------------------------8<-------------------------- The information below can be used to reproduce a debug environment:
Current build : https://ci.linaro.org/job/tcwg_gdb_check--master-aarch64-precommit/642/artif... Reference build : https://ci.linaro.org/job/tcwg_gdb_check--master-aarch64-build/326/artifact/...
Hello Guinevere,
Guinevere Larsen blarsen@redhat.com writes:
I got 3 of these messages about a patch I have upstream, which seems excessive. I also just managed to grab an aarch64 box and test it and got no errors, so I'd appreciate someone double checking that those werent 3 false positives please.
I'm sorry for the false positive and the extra messages (Could you forward them (with full headers) to me, please? Perhaps we can prevent sending identical messages in the case of an unexpected mishap).
And thank you for testing on an aarch64 box that the patches are indeed ok.
What happened here is that the In-Reply-To: fields of the v2 messages all point to the v1 patch. IIUC, normally the v2 cover letter would point to the v1 patch, and patches 1 and 2 would point to the v2 cover letter. This confused patchwork's series detection and so our CI applied the v2 2/2 patch on top of the v1 patch instead of the v2 1/2 patch. This of course caused a build failure.
Not sure how to avoid this problem in the future. Perhaps we could add an heuristic to detect this case and skip precommit testing? The heuristic could be something like looking for a "[v<n> x/y]" prefix and checking whether it is consistent among all patches in the series.
Thank you for bring this to our attention!
On 26/09/2023 18:04, Thiago Jung Bauermann wrote:
Hello Guinevere,
Guinevere Larsen blarsen@redhat.com writes:
I got 3 of these messages about a patch I have upstream, which seems excessive. I also just managed to grab an aarch64 box and test it and got no errors, so I'd appreciate someone double checking that those werent 3 false positives please.
I'm sorry for the false positive and the extra messages (Could you forward them (with full headers) to me, please? Perhaps we can prevent sending identical messages in the case of an unexpected mishap).
Yes of course! I'm not very knowledgeable in email, did you get all headers with this forward, or is it better if I attach them to a private email to you?
And thank you for testing on an aarch64 box that the patches are indeed ok.
What happened here is that the In-Reply-To: fields of the v2 messages all point to the v1 patch. IIUC, normally the v2 cover letter would point to the v1 patch, and patches 1 and 2 would point to the v2 cover letter. This confused patchwork's series detection and so our CI applied the v2 2/2 patch on top of the v1 patch instead of the v2 1/2 patch. This of course caused a build failure.
Yeah, I don't understand what happened with my git, but lucky that it did bring up the multiple replies thing at least.
I wonder if that is related? Could the CI have looked at all emails, found the parent which is a real patch, and applied 2/2 for all emails, regardless of which email was being tested due to some patch series heuristic?
Not sure how to avoid this problem in the future. Perhaps we could add an heuristic to detect this case and skip precommit testing? The heuristic could be something like looking for a "[v<n> x/y]" prefix and checking whether it is consistent among all patches in the series.
Thank you for bring this to our attention!
No problem! Thank you for the effort of improving GDB!
Guinevere Larsen blarsen@redhat.com writes:
On 26/09/2023 18:04, Thiago Jung Bauermann wrote:
Hello Guinevere,
Guinevere Larsen blarsen@redhat.com writes:
I got 3 of these messages about a patch I have upstream, which seems excessive. I also just managed to grab an aarch64 box and test it and got no errors, so I'd appreciate someone double checking that those werent 3 false positives please.
I'm sorry for the false positive and the extra messages (Could you forward them (with full headers) to me, please? Perhaps we can prevent sending identical messages in the case of an unexpected mishap).
Yes of course! I'm not very knowledgeable in email, did you get all headers with this forward,
No, I only got Subject, Date, From, Reply-To and To.
or is it better if I attach them to a private email to you?
Yes, normally there's a "forward message as attachment" option which sends the complete email with all headers.
And thank you for testing on an aarch64 box that the patches are indeed ok.
What happened here is that the In-Reply-To: fields of the v2 messages all point to the v1 patch. IIUC, normally the v2 cover letter would point to the v1 patch, and patches 1 and 2 would point to the v2 cover letter. This confused patchwork's series detection and so our CI applied the v2 2/2 patch on top of the v1 patch instead of the v2 1/2 patch. This of course caused a build failure.
Yeah, I don't understand what happened with my git, but lucky that it did bring up the multiple replies thing at least.
I wonder if that is related? Could the CI have looked at all emails, found the parent which is a real patch, and applied 2/2 for all emails, regardless of which email was being tested due to some patch series heuristic?
The patch series detection is done by Patchwork, and our CI just uses its API to get the series and apply it. What I'm proposing is a sanity check in our CI to detect whether Patchwork got the series right or not.
I believe that what you are proposing would have to be implemented in Patchwork itself. I'll look into whether we can make that improvement to Patchwork.
linaro-toolchain@lists.linaro.org
-
Guinevere Larsen -
Thiago Jung Bauermann