Skip to content

Conversation

ufechner7
Copy link
Contributor

@ufechner7 ufechner7 commented Jul 30, 2024

Fixes #951

This PR does the following:

  1. Fix embedding_wrapper.c
  2. Start running CI on Julia 1.11.

Note: for now, we skip the incremental=false tests on Julia 1.11 due to #976.

@ufechner7
Copy link
Contributor Author

The tests for macOS still fail for reasons that I do not understand. Could some have a look at the error messages who knows macOS better than me?

@PatrickHaecker
Copy link
Contributor

Thanks for doing the PR!

I am a bit puzzled: To my understanding the code change in embedding_wrapper.c should not have any influence on Julia <= 1.10.
Has it really worked before your PR or is the CI failing on macOS a different issue independent of the 1.11 change?

@ufechner7
Copy link
Contributor Author

ufechner7 commented Jul 30, 2024

CI fails since May 2023, since this commit: 33428b2 Would still be nice to understand and fix the CI failures.

@PatrickHaecker
Copy link
Contributor

PatrickHaecker commented Jul 30, 2024

Yes, that would be nice. But I suggest that we track that in a different topic.

`$(get_julia_cmd()) --sysimage=$sysimage -e 'using Pkg; Pkg.precompile()'`

probably has a non-zero return value when run, but without access to a macOS Julia installation that is indeed hard to impossible to debug.

@ufechner7
Copy link
Contributor Author

ufechner7 commented Jul 30, 2024

I created a separate issue for the CI failures on Mac: #955

@DilumAluthge DilumAluthge mentioned this pull request Oct 15, 2024
27 tasks
@DilumAluthge DilumAluthge marked this pull request as draft October 16, 2024 01:30
@DilumAluthge DilumAluthge changed the title Make Julia 1.11 work, fix #951 Fix embedding_wrapper.c for Julia 1.11 Oct 16, 2024
@DilumAluthge
Copy link
Member

@IanButterworth It looks like you modified this file most recently. Would you be able to review this PR?

@DilumAluthge DilumAluthge changed the title Fix embedding_wrapper.c for Julia 1.11 Fix embedding_wrapper.c for Julia 1.11 (and run CI on 1.11) Oct 16, 2024
Copy link
Member

@topolarity topolarity left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't help bring up PackageCompiler on 1.11 in general, but I can sign off on this change @DilumAluthge

@DilumAluthge
Copy link
Member

Thanks @topolarity!

@DilumAluthge DilumAluthge marked this pull request as ready for review October 16, 2024 02:01
@DilumAluthge DilumAluthge removed the request for review from IanButterworth October 16, 2024 02:06
Copy link

codecov bot commented Oct 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.56%. Comparing base (0e04008) to head (7f15dda).
Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #954   +/-   ##
=======================================
  Coverage   84.56%   84.56%           
=======================================
  Files           3        3           
  Lines         823      823           
=======================================
  Hits          696      696           
  Misses        127      127           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@DilumAluthge
Copy link
Member

DilumAluthge commented Oct 16, 2024

The Julia 1.11 CI seems to be failing on all platforms. For example, look at this CI log, which is Julia 1.11, 64-bit, on Linux. (I've removed the verbose listing of all the environment variables in the active process.)

error during bootstrap:

LoadError("/tmp/jl_3Qidyp/sysimage_packagecompiler_bfb1e672-8b81-11ef-3d17-c7f4e932fd41.jl", 57, LoadError("/opt/hostedtoolcache/julia/1.11.0/x64/share/julia/stdlib/v1.11/Pkg/src/Pkg.jl", 3, LoadError("/opt/hostedtoolcache/julia/1.11.0/x64/share/julia/stdlib/v1.11/Pkg/src/precompile.jl", 101, Base.IOError("could not spawn setenv('' a registries/Registry.tar.gz -tgzip registries/Registry.tar,[ ... env vars ...]): permission denied (EACCES)", -13))))

_spawn_primitive at ./process.jl:140

_spawn at ./process.jl:157 [inlined]

@staticfloat To me, it looks like we are hitting this line in Pkg.jl's precompile.jl file:

https://github.com/JuliaLang/Pkg.jl/blob/27c1b1ee5cf15571eb5e54707e812d646ac1dde3/src/precompile.jl#L86-L86

cmd = `$(Pkg.PlatformEngines.exe7z()) a "registries/Registry.tar.gz" -tgzip "registries/Registry.tar"`

But it seems like Pkg.PlatformEngines.exe7z() is returning an empty string?

@DilumAluthge
Copy link
Member

I've opened a separate issue for the bug that is causing Julia 1.11 CI to fail here: #976

@DilumAluthge DilumAluthge changed the title Fix embedding_wrapper.c for Julia 1.11 (and run CI on 1.11) Fix embedding_wrapper.c for Julia 1.11; also, run CI on 1.11, but skip the incremental=false tests on 1.11 Oct 17, 2024
@DilumAluthge
Copy link
Member

Alright, for now, let's skip the incremental=false tests on Julia 1.11.

@DilumAluthge
Copy link
Member

Okay, that change (skipping the incremental=false tests on Julia 1.11) seems to have fixed CI.

Once CI has finished running and is all green, I'll merge this PR and make a new release.

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

Successfully merging this pull request may close these issues.

Incompatibility with Julia 1.11-rc1 and 1.11-rc2
4 participants