Add resolution list #4
Conversation
|
|
|
Starting tests for sync_labels |
|
Test action opened |
|
Test action reopened |
|
Test action closed |
|
Test action labeled |
|
Label s: positive review can not be added. Please use the corresponding functionality of GitHub |
|
Label s: needs review can not be removed. Please add the state-label which should replace it |
1 similar comment
|
Label s: needs review can not be removed. Please add the state-label which should replace it |
|
Label s: positive review can not be added due to s: needs work! |
|
Label s: positive review can not be added due to s: needs work! |
|
Label s: positive review can not be removed. Please add the state-label which should replace it |
|
Label s: positive review can not be removed. Please add the state-label which should replace it |
|
Label s: needs work can not be added due to s: positive review! |
1 similar comment
|
Label s: needs work can not be added due to s: positive review! |
|
Label s: needs review can not be added due to s: positive review! |
1 similar comment
|
Label s: needs review can not be added due to s: positive review! |
|
Test action converted_to_draft |
|
Test action submitted |
|
Label s: needs work can not be removed. Please add the state-label which should replace it |
1 similar comment
|
Label s: needs work can not be removed. Please add the state-label which should replace it |
|
Label s: needs review can not be removed. Please add the state-label which should replace it |
1 similar comment
|
Label s: needs review can not be removed. Please add the state-label which should replace it |
|
Label s: needs work can not be removed. Please add the state-label which should replace it |
1 similar comment
|
Label s: needs work can not be removed. Please add the state-label which should replace it |
|
Label s: positive review can not be added. Please use the corresponding functionality of GitHub |
1 similar comment
|
Label s: positive review can not be added. Please use the corresponding functionality of GitHub |
### 📚 Description Trac branch `u/gh-collares/gap-gc` from sagemath#34701, now migrated to GitHub. Currently based atop sagemath#35093; will rebase once that is merged. The rest of the description below is copied from sagemath#34701: A refactor in sagemath#27946 introduced "unprotected" (not surrounded by `GAP_Enter`/`GAP_Leave`) `GAP_ValueGlobalVariable` calls. I believe this might be a GC hazard, because after updating to GAP 4.12.1 I started seeing aarch64 crashes on NixOS infrastructure such as: ``` #0 0x0000fffff79740e8 in wait4 () #1 0x0000fffff5dc6b78 in print_enhanced_backtrace () #2 0x0000fffff5dc8190 in sigdie () #3 0x0000fffff5dcb1c0 in cysigs_signal_handler () #4 0x0000fffff7ffb7cc in __kernel_rt_sigreturn () #5 0x0000ffff99a0bf28 in ConvString () #6 0x0000000000000000 in ?? () #7 0x0000000000000000 in ?? () #8 0x0000000000000000 in ?? () #9 0x0000ffff99989930 in Pr () #10 0x0000ffff9998aa18 in CloseOutput () #11 0x0000ffff99884828 in capture_stdout () at /build/sage- src-9.7/pkgs/sagemath-standard/sage/libs/gap/element.pyx:154 ... ``` I also see cases where `capture_stdout` throws errors such as `sage.libs.gap.util.GAPError: Error, Length: <list> must be a list (not the integer 255)` and then crashes. Both types of errors are fixed by this ticket. Note that I am nesting `GAP_Enter`/`GAP_Leave` calls because I didn't remove the preexisting calls inside `capture_stdout`. That's because I feared removing the innermost calls might create a new footgun (and I believe nested `GAP_Enter`/`GAP_Leave` calls are explicitly supported), but removing them should cause no problem. Removing them might even be preferable for performance reasons, I don't know. Fixes sagemath#34701 ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> <!-- If your change requires a documentation PR, please link it appropriately --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> - [x] I have made sure that the title is self-explanatory and the description concisely explains the PR. - [x] I have linked an issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation accordingly. ### ⌛ Dependencies <!-- List all open pull requests that this PR logically depends on --> <!-- - #xyz: short description why this is a dependency - #abc: ... --> - sagemath#35093: GAP 4.12.2 upgrade, which touches the same function and should land first. URL: sagemath#35114 Reported by: Mauricio Collares Reviewer(s): Dima Pasechnik
📚 Description
According to sagemath/trac-to-github#117 (comment) the resolution list is taken into account for label synchronization, too.
📝 Checklist
⌛ Dependencies