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

Update to julia 1.10 #156

Merged
merged 8 commits into from
Feb 5, 2024
Merged

Update to julia 1.10 #156

merged 8 commits into from
Feb 5, 2024

Conversation

benegee
Copy link
Collaborator

@benegee benegee commented Jan 8, 2024

No description provided.

@sloede
Copy link
Member

sloede commented Jan 8, 2024

TBH, for the time being I would just add 1.10 as an additional configuration to check. IMHO it would be fine to only run it with coverage disabled.

Feel free to ping me once I should review anything

@benegee
Copy link
Collaborator Author

benegee commented Jan 8, 2024

This fortunately fixes #152 .

But there is some issue with PackageCompiler:

- PackageCompiler: compiling fresh sysimage (incremental=false)
error during bootstrap:
LoadError("/tmp/jl_HeuRo1/sysimage_packagecompiler_47e340e8-ae21-11ee-2456-a7170323e6fe.jl", 18, LoadError("/opt/hostedtoolcache/julia/1.10.0/x64/share/julia/stdlib/v1.10/Test/src/Test.jl", 3, LoadError("/opt/hostedtoolcache/julia/1.10.0/x64/share/julia/stdlib/v1.10/Test/src/precompile.jl", 1, MethodError(Base.RedirectStdStream(1, true), (Core.CoreSTDOUT(),), 0x0000000000008634))))

This happens when Base.require(Base, :Test) is called from the new_sysimage_source_path script in PackageCompiler.jl's create_fresh_base_sysimage.

I noticed it works when filter_stdlibs is set to false, instead of the current true. The difference is:

sysimage_stdlibs list in PackageCompiler.jl for filter_stdlibs = false:

["ArgTools", "Artifacts", "Base64", "CRC32c", "Dates", "Downloads", "FileWatching", "Future", "InteractiveUtils", "LibCURL", "LibCURL_jll", "LibGit2", "LibGit2_jll", "LibSSH2_jll", "Libdl", "LinearAlgebra", "Logging", "Markdown", "MbedTLS_jll", "Mmap", "MozillaCACerts_jll", "NetworkOptions", "OpenBLAS_jll", "Pkg", "Printf", "REPL", "Random", "SHA", "Serialization", "Sockets", "TOML", "Tar", "UUIDs", "Unicode", "libblastrampoline_jll", "nghttp2_jll", "p7zip_jll"]

sysimage_stdlibs list in PackageCompiler.jl for filter_stdlibs = true:

["LibGit2", "OpenLibm_jll", "InteractiveUtils", "REPL", "ArgTools", "OpenBLAS_jll", "SparseArrays", "Markdown", "Random", "DelimitedFiles", "LibCURL_jll", "LazyArtifacts", "Statistics", "SuiteSparse", "SHA", "LibGit2_jll", "Dates", "Sockets", "NetworkOptions", "Zlib_jll", "p7zip_jll", "Serialization", "Test", "LibCURL", "Distributed", "Logging", "nghttp2_jll", "Tar", "Mmap", "Base64", "SuiteSparse_jll", "CompilerSupportLibraries_jll", "LinearAlgebra", "libblastrampoline_jll", "MozillaCACerts_jll", "Future", "TOML", "MbedTLS_jll", "SharedArrays", "UUIDs", "Unicode", "Printf", "Pkg", "Libdl", "Artifacts", "FileWatching", "LibSSH2_jll", "Downloads"]

In particular the latter contains Test.

However, both choices worked with julia 1.9.

@benegee
Copy link
Collaborator Author

benegee commented Jan 9, 2024

So, we could matrix-test with Julia versions 1.9.3 (which does not fall victim to #152) and with 1.10.*. Coverage would only be tested with either of them. Still it would be nice to have PackageCompiler working with 1.10. Any ideas @sloede ?

Copy link

codecov bot commented Jan 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (e0966cf) 97.98% compared to head (a3b4bd1) 97.98%.
Report is 9 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #156   +/-   ##
=======================================
  Coverage   97.98%   97.98%           
=======================================
  Files          13       13           
  Lines         547      547           
=======================================
  Hits          536      536           
  Misses         11       11           
Flag Coverage Δ
unittests 97.98% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@benegee
Copy link
Collaborator Author

benegee commented Feb 5, 2024

I would like to have green lights on main again. So, for the time being, I would like to use julia 1.10 as default, and 1.9.3 for testing package compiler.

@benegee benegee requested a review from sloede February 5, 2024 11:31
Copy link
Member

@sloede sloede left a comment

Choose a reason for hiding this comment

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

Instead of guarding all these PC.jl runs against usage with v1.10 in the if conditions, why don't you just remove the package-compiler from the test matrix in line 35 (https://github.com/trixi-framework/libtrixi/pull/156/files#diff-b803fcb7f17ed9235f1e5cb1fcd2f5d3b2838429d4368ae4c57ce4436577f03fR35)? Shouldn't this then do exactly what you want: run the default items in the test matrix except for the PC.jl stuff, which you explicitly add for Julia v1.9.3 in the include section. Or am I missing something?

@benegee
Copy link
Collaborator Author

benegee commented Feb 5, 2024

Yeah, that is way more clever!

@benegee benegee requested a review from sloede February 5, 2024 17:15
@sloede sloede merged commit dab41c7 into main Feb 5, 2024
10 checks passed
@sloede sloede deleted the go-for-julia-1-10 branch February 5, 2024 17:31
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.

2 participants