-
Notifications
You must be signed in to change notification settings - Fork 160
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
kernel: remove environ
arguments, remove sysenviron
#3111
Conversation
Ah, thanks for taking care of this. It's fine by me; I will shed no tears if we just do away with the environ passing entirely. |
I was motivated to do this because we just run into an issue with the GAPJulia bridge due to us being lazy and not passing the right |
Indeed: Most of the issues I've raised so far are in no way specific to Sage or even Python. The segfault we found could have happened to anyone using |
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.
Sure, no problem. I should have written tests to test environ passing, so now I'm glad I didn't (although this might have helped to locate the problem earlier...) Oh well.
It just as easily might not have. This code path is already technically covered by the scant existing libgap tests. Like I said, whether or not this problem actually occurs depends on some very particular circumstances. |
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.
Looks good.
This also solves problems I had using homalg via Julia.
ef6c27b
to
9e8e8fa
Compare
Codecov Report
@@ Coverage Diff @@
## master #3111 +/- ##
==========================================
+ Coverage 83.57% 83.62% +0.05%
==========================================
Files 686 684 -2
Lines 336777 332055 -4722
==========================================
- Hits 281462 277685 -3777
+ Misses 55315 54370 -945
|
Codecov Report
@@ Coverage Diff @@
## master #3111 +/- ##
==========================================
- Coverage 83.57% 83.57% -0.01%
==========================================
Files 686 686
Lines 336777 336779 +2
==========================================
- Hits 281462 281461 -1
- Misses 55315 55318 +3
|
Backported via 5867012 |
This is an alternative to PR #3106 by @embray based on the discussion there.
Closes #3106