Add jl_stdout_obj back fixing regression of r-juliacall #56340
Conversation
There was a problem hiding this comment.
Julia's ABI and even most of its API are not stable and change with every release of Julia. (I state this as a matter of fact, not because I necessarily condone it -- indeed I think it would be crucial for Julia on the long run to define a subset of its API (and perhaps even ABI) which it commits to keeping stable, and which then on the C header level is "separate" so that developers who wish to target this API have a realistic chance to get that API and only that (and not end up using other, unstable APIs "by accident").
Based on this I'd say having to adapt to each Julia minor release is par for the course, and if there is no active maintainer this project will stop working at to some point on way or another.
However, I also see no harm in restoring this particular API for the time being, it doesn't cost us much and if it helps people to bridge over the time until they have active maintainers for JuliaCall, why not...
|
Thanks for approving (CI Failure is unrelated (or well must be). You would support it in 1.11.2? Add a backport label?
so true...
Right, I'm not sure when is the right time for that, if not now. At least something must be minimal very unlikely to change.
Right, I think maybe we need to be careful what we add to the exported API, and this never should have been there... It's annoying when stuff gets dropped from the unstable "API", we are likely converging on a stable API?! I wander how much more it needs to change. At least we could bless a subset of it, similar to Python is doing? [I'm not saying "jl_stdout_obj back" should be declared stable API indefinitely, the PR here is just a workaround, though I'm unclear on downside of never dropping again.]
Exactly, why I'm doing this. I do not rule out dropping it later, or changing to no-op... This wasn't used by Julia itself (unlike for very similar jl_stdout_obj), likely why it was dropped. |
|
Bump. [If not merged for 1.11.2,] add regression 1.11 label to the issue (and backport 1.11 here). |
|
Bump (on merge if still wanted): FYI: The alternative is now in motion (with the original developer, otherwise absent, finally answering email), some other people will take over and maintain at JuliaInterop, see: JuliaInterop/JuliaCall#237 (comment) But I still think this can be merged, it's still helpful (and I believe very safe here), at least if people have older version of the package installed (or some other using this), or taking over that project simply gets delayed. CI failures are false alarms. |
PallHaraldsson
force-pushed
the
patch-27
branch
2 times, most recently
from
4e00fab to
12aef8d
Compare
|
Maybe just merge this, because of the hold-up with the alternative (and since it's approved, and not a problem to have in). If merged then get into 1.11.8 or not really of much help soon. |
PallHaraldsson
force-pushed
the
patch-27
branch
from
12aef8d to
ad83b84
Compare
Fixes #56339 (very partial revert of #53250 (comment) see also: JuliaInterop/JuliaCall#237 alternative, but maintainer is absent).
Please backport to 1.11.2 (#56228), since it looks bad for Julia to break packages (that PkgEval can't reach, so understandable).
A.
Adding
back, I think a Warning (or TODO) comment can't go there at the end of the line.
[I also added the code, not sure if some kind of dummy export is possible. This could even be a no-op but seemingly not bad with the original code, it's at least not large.]
B.
[I also really want to drop lisp_prompt export in that file, which is possible and desirable... while keeping --lisp CLI option, i.e. only dropping for C API export.
Feel free drop that from same file, and merge too.]