-
Notifications
You must be signed in to change notification settings - Fork 162
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 calling AtExit methods after loading a workspace #2578
Conversation
Fixes #2558. This is unfortunately hard to test automatically, so I'd appresiate someone sanity checking it works. Basically, you should find saving history, and clean up of TemporaryDirectory, didn't work before this patch and does after this patch, if GAP is loaded with |
Fixes #2558. This is unfortunately hard to test automatically, so I'd appresiate someone sanity checking it works. Basically, you should find saving history didn't work before this patch and does after this patch, if GAP is started from a workspace. |
b1e0797
to
97fbb75
Compare
Codecov Report
@@ Coverage Diff @@
## master #2578 +/- ##
==========================================
+ Coverage 74.6% 74.61% +<.01%
==========================================
Files 481 481
Lines 242386 242392 +6
==========================================
+ Hits 180843 180862 +19
+ Misses 61543 61530 -13
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This fixes a regression I introduced when merging HPC-GAP, see e322531 - thanks for that!
The first commit could also be backported to GAP 4.9, As it fixes a regression compared to 4.8. (The second one could also be backported IMO)
But please add labels.
@mohamed-barakat please test if this solves your problem |
I've assigned this to 4.9.2 milestone - in a hope that @mohamed-barakat may confirm it quickly. Otherwise may be delayed till 4.9.3. |
I am happy, thanks. |
This patch just ensures we call "CLEAN_UP_PROGRAM" after restoring a workspace, mainly so the AtExit handlers are run.
While fixing this I noticed a different (small) bug, which is when restoring a workspace some handlers end up getting installed twice. The easiest fix for this (in my opinion) is to just skip the second time the handler is installed.
Fixes #2558