Skip to content
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

Remove unused jl_stdout_obj import since it was removed in Julia 1.11 #237

Merged
merged 2 commits into from
Nov 7, 2024

Conversation

JackDunnNZ
Copy link
Collaborator

This jl_stdout_obj doesn't seem to be used anywhere in the package, and was removed in Julia 1.11: JuliaLang/julia#53250

Fixes #234

@schlichtanders
Copy link

Looking forward to get this merged! Currently RCall blocks me from transitioning to Julia 1.11

@schlichtanders
Copy link

@JackDunnNZ thank you for you work. What is needed to get this merged? I am really blocked by this and would like to speed up the process if possible

@cderv
Copy link
Contributor

cderv commented Oct 21, 2024

I believe we are all in wait of

And a discussion on what to do next should happen there.

Other PR like #210 are opened since more than 1 year with no reaction.

@PallHaraldsson
Copy link

PallHaraldsson commented Oct 22, 2024

bump. [My alternative suggestion is at JuliaLang, (partial) revert of the breaking change there.]

If this wasn't actually used, can't this be speedily merged? And presumably solving the problem (on master; and then tag a new version?).

@JackDunnNZ
Copy link
Collaborator Author

bump.

If this wasn't actually used, can't this be speedily merged? And presumably solving the problem (on master; and then tag a new version?).

Please read the issue linked above, @Non-Contradiction is unfortunately nowhere to be found

@PallHaraldsson
Copy link

PallHaraldsson commented Oct 22, 2024

I did and I realize @Non-Contradiction is absent, maybe only one(?) with merge access. I would be willing to fix this (get maintainer status), I see you @JackDunnNZ have some commits, presumably no access (nor any other contributor), why I also edited, mentioning my alternative at JulaLang.

It's bad that 1.11 broke this (possibly it wasn't documented API), though understandable PkgEval didn't catch (and thus probably thought ok); see issue I linked and commented on.

@JackDunnNZ
Copy link
Collaborator Author

Yes, unfortunately I have no commit access - I am merely an invested user that fixes things each time they have broken :)

@fingolfin
Copy link

There are other potential issues with this package. E.g. for some reason it doesn't seem to use the official Julia C headers, instead copying bits of it. This is inherently dangerous and fragile. I note that they copied a definition for struct jl_module_t. This definition is not matching the actual definition anymore. This may be OK (if they don't actually try to access any of the members of this struct), but overall I really wonder why this approach was chosen.

@PallHaraldsson
Copy link

My alternative fix, partial revert in JuliaLang has been approved (by you), though not yet merged.

It makes this PR redundant, and I believe fixes that package (for now).

Given that the package is doing something "inherently dangerous and fragile" it could break again later in 1.12+.

There's no way to take over a GitHub project (unless people die, maybe), wouldn't be a good policy. But in Julia registry there is I think a way to take over a project pointing to a forked project.

I don't know how things are handled in R, I'm a bit ignorant, is there some official procedure in case of maintainer disappearing?

@ViralBShah
Copy link
Contributor

Do people have commit access here? Perhaps this package should live in JuliaInterop and recruit a larger pool of maintainers?

@PallHaraldsson
Copy link

PallHaraldsson commented Oct 26, 2024

Apparently nobody has commit access here, or the one or few that do are not listening (for over a year for
@ChrisRackauckas' trivial (not-too badly needed? timeout) PR here: https://github.com/Non-Contradiction/JuliaCall/pull/211/files and 2+ weeks for this badly needed fix here, and they main guy abandoned his latest work since 2020).

I agree this could live in JuliaInterop (at least if R users would look there, or get linked there), that would mean forking it, but I'm thinking, in R community, would it still link here? Even changing docs is not possible (here) like only works in 1.11.2 (after my PR merged), and I'm not sure where R people get this from if not directly from Github, or what docs they would see.

This would need to change:
https://cran.r-project.org/web/packages/JuliaCall/index.html

I don't know CRAN policies.

Maybe this indicates Julia isn't that much used from R, calling in that direction at least? [Neither RCall.jl to call R from Julia? Or interop that way, way more popular?]

Could/should this package, maybe not just be moved (good first step) but rather integrated into RCall.jl? I.e. like PythonCall.jl is for both directions, and Python-to-Julia direction is just a trivial addon in that package.

@ViralBShah
Copy link
Contributor

I have emailed @Non-Contradiction requesting transfer to JuliaInterop.

@Non-Contradiction
Copy link
Collaborator

Dear all, thank you very much. After consideration, I agree with @ViralBShah and will transfer the package to JuliaInterop. I once granted commit access to some people. But it seems that they are busy too. It seems that transftering the package to JuliaInterop will allow a broader pool of maintainers and is the right choice.

@Non-Contradiction
Copy link
Collaborator

I agree that not using Julia's official header is dangerous and fragile.
It really was an ad hoc approach in the early days of JuliaCall.
On the R side, every C code compilation must happen at the package installation stage, otherwise compilation will happen every time at the loading of the package (which is very slow and intolerable, especially in the early days....).
Then JuliaCall basically needs to have Julia's official header with it. And also JuliaCall needed to consider that user may have different versions of Julia instead of a single version to use with JuliaCall. So I tried to use a minimal subset of header to avoid the conflicting between versions and yet have a functional package. But this approach is really fragile, and the existence of this issue proves exactly this.

@ViralBShah
Copy link
Contributor

@Non-Contradiction I have given you owner access on JuliaInterop to move the package. Once you move, I will reduce the permission level but still make sure you have full access to your package.

Thanks for the explanation. I understand the challenge better, and others may be better suited to chime in on what can be done to improve things.

@schlichtanders
Copy link

The package is transferred. Awesome 🚀 .

Looking forward to have r-JuliaCall ready for julia 1.11 soon

@ViralBShah
Copy link
Contributor

Should this PR be merged? Who else can help here - I am happy to add them to have commit access.

@ChrisRackauckas
Copy link
Member

It's a bit scary since it looks like the CI didn't run. Someone who knows how to fix R CI should probably make sure to fix that and so something runs, and merge this if it passes?

@JackDunnNZ
Copy link
Collaborator Author

Not sure why it's not running here, but I resynced this branch to master and the CI passed on my repo (except for R-devel)

https://github.com/JackDunnNZ/JuliaCall/actions/runs/11719241531

@ChrisRackauckas ChrisRackauckas merged commit c9fc09d into JuliaInterop:master Nov 7, 2024
@ChrisRackauckas
Copy link
Member

I'll need to be added as a maintainer on CRAN though in order to do a new CRAN release.

@ViralBShah
Copy link
Contributor

@dmbates Would you know how we go about adding ourselves to the maintainers for the Julia facing packages in CRAN?

@JackDunnNZ
Copy link
Collaborator Author

I found this: https://stackoverflow.com/a/78334023

It looks like basically the new maintainer needs to proceed with a release, and @Non-Contradiction will receive an email to confirm the change

@Non-Contradiction
Copy link
Collaborator

Thanks everyone! If everything is ready, then I am very happy to make a new release on CRAN. Just @ me or email me when any thing is needed to be done on my side.

@bachmeil
Copy link

In case anyone else is hitting this error:

Error: jl_stdout_obj - /home/office/.julia/juliaup/julia-1.11.1+0.x64.linux.gnu/bin/../lib/libjulia.so.1.11: undefined symbol: jl_stdout_obj

You'll need to install from the repo until the CRAN package is updated:

library(devtools)
install_github("https://github.com/JuliaInterop/JuliaCall")

@ViralBShah
Copy link
Contributor

Do we have some volunteers to help make the release on the CRAN side?

@palday
Copy link

palday commented Nov 13, 2024

Do we have some volunteers to help make the release on the CRAN side?

maybe @Non-Contradiction ?

I will also have a bit more time on Friday (15 November).

@schlichtanders
Copy link

Would love to see this being released on CRAN and Conda-forge.

@ikosmidis
Copy link

ikosmidis commented Nov 28, 2024

Hi all! Thanks for fixing this. Any updates when this will be on CRAN?

In case someone is planning or doing that already, and just in case you haven't figured this out (and as briefly mentioned earlier), here are some steps for releasing in CRAN.

If @ChrisRackauckas takes over maintenance, then @ChrisRackauckas must submit the package to CRAN.

The easiest way forward is to update the DESCRIPTION file with a change of maintainer, e.g.

Authors@R: c(
    person(given = "Christopher", family = "Rackauckas", email = "accounts@chrisrackauckas.com", role = c("aut", "cre")),
    person(given = "Changcheng", family = "Li", role = "aut", comment = "earlier version by"),
    person(given = "Randy", family = "Lai",  role = "ctb"),
    person(given = "Dmitri", family = "Grominski", role = "ctb"),
    person(given = "Nagi", family = "Teramo", role = "ctb")
    )

The above explicitly recognises that @Non-Contradiction produced earlier versions and that @ChrisRackauckas is the current maintainer (see, e.g., https://cran.r-project.org/package=betareg for an example of how that would look on CRAN pages). It may be worth adding other people who contributed to the release using the "ctb" role.

Then, you can just use devtools::release() to submit, which will also guide you to what one can do to minimize the chance of breaking CRAN rules.

When submitted, @ChrisRackauckas will receive an email to approve the submission and @Non-Contradiction will receive an email to approve the change of maintainership. Then, the CRAN team will either release or come back with any issues that need to be taken care of, inviting for a new submission.

I am happy to help, but it should be straightforward with the above steps.

@PallHaraldsson
Copy link

PallHaraldsson commented Nov 28, 2024

Any updates when this will be on CRAN?

I don't know anything about that, but given the holdup here, my alternative PR could be merged (it doesn't need to be either or):

JuliaLang/julia#56340 (comment)

Maybe just merge this, because of the hold-up with the alternative (and since it's approved, and not a problem to have in).

Then the package here any version, without the PR here would just work, assuming my trivial PR there is backported.

@marc-milgrom-ncl
Copy link

In case anyone else is hitting this error:

Error: jl_stdout_obj - /home/office/.julia/juliaup/julia-1.11.1+0.x64.linux.gnu/bin/../lib/libjulia.so.1.11: undefined symbol: jl_stdout_obj

You'll need to install from the repo until the CRAN package is updated:

library(devtools)
install_github("https://github.com/JuliaInterop/JuliaCall")

I've installed from the repo and still get the error. It fails devtools::check() unless I specify JULIA_HOME to 1.9. Is the PR merged yet?

@Non-Contradiction
Copy link
Collaborator

Thanks everyone! It seems to me that the main issue is solved. I will do more test/check for the package and make a release on CRAN next week.

@ChrisRackauckas
Copy link
Member

If @ChrisRackauckas takes over maintenance, then @ChrisRackauckas must submit the package to CRAN.

I see, I thought it was the other way around. I thought I needed to be added as a maintainer, and then I could submit to CRAN. So I was waiting on that. Either way @Non-Contradiction it would be helpful to get the CRAN status so we can keep this on-going. It's great work and I don't want it to get lost in the sands of time!

@ikosmidis
Copy link

Indeed, if things have not changed since the last change of maintenance event I was involved in (~2017), when @ChrisRackauckas submits @Non-Contradiction will get an email with the following content

Someone has submitted the package cranly to CRAN. You are receiving this email to confirm the submission as the maintainer of this package. To confirm the submission to CRAN, follow or copy & paste the following link into your browser:

After approving the submission, CRAN should come back to @Non-Contradiction and ask about the maintenance change.

If the process has changed then the CRAN team are pretty good in telling authors what should be done.

@schlichtanders
Copy link

The CRAN release seems to have worked. I am using juliacall via conda/CondaPkg.jl
can someone trigger a release there too?
https://anaconda.org/r/r-juliacall

@ViralBShah
Copy link
Contributor

ViralBShah commented Dec 10, 2024

Does that need figuring out who made the last release, and pinging them?

@schlichtanders
Copy link

@Non-Contradiction can you help with the anaconda release?

@Non-Contradiction
Copy link
Collaborator

@schlichtanders I just checked and see that conda-forge/r-juliacall is already the 0.17.6 version and is more downloaded, although the r/r-juliacall is still the 0.17.5 version. I will check about how to update the r/r-juliacall channel.

@schlichtanders
Copy link

schlichtanders commented Dec 21, 2024

thanks, I indeed was confused by r/r-juliacall and thought that it is not yet available at all. Great that conda-forge is already released!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

juliacall fails in julia 1.11 with undefined symbol: jl_stdout_obj