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

create_fresh_base_sysimage(): Pass "" as the BUILDROOT for Julia 1.12+ #996

Closed
wants to merge 2 commits into from

Conversation

DilumAluthge
Copy link
Member

@DilumAluthge DilumAluthge commented Oct 31, 2024

Fixes #989.

This fixes the immediate bug in #989, which is that Base.jl incorrectly assumes that our first positional argument is the value of the BUILDROOT. The fix is to pass a second positional argument "", which is then taken by Base.jl to be the BUILDROOT.

This bugfix PR will eventually be followed up by #997, which is a larger PR that will actually take advantage of the new BuildSettings mechanism added to Julia 1.12 in JuliaLang/julia#54387

@DilumAluthge DilumAluthge changed the title Pass "" as the BUILDROOT for Julia 1.12+ Pass "" as the BUILDROOT for Julia 1.12+ Oct 31, 2024
@DilumAluthge DilumAluthge changed the title Pass "" as the BUILDROOT for Julia 1.12+ create_fresh_base_sysimage(): Pass "" as the BUILDROOT for Julia 1.12+ Oct 31, 2024
Copy link

codecov bot commented Oct 31, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.56%. Comparing base (38629b0) to head (6e15a4e).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #996   +/-   ##
=======================================
  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.


🚨 Try these New Features:

@DilumAluthge
Copy link
Member Author

I think that it's fine to just always pass this argument unconditionally, because AFAICT it will just be ignored on older Julia versions. But if we want, we could instead gate this behind an if VERSION >= v"1.12-".

@DilumAluthge
Copy link
Member Author

Also, I went ahead and bumped the version number (from 2.1.22 to 2.1.23), so that we can tag a new release after we merge this PR.

I determined the bump as follows:

  1. This PR is a bugfix PR.
  2. There is currently only one commit on master since the last release, and that commit is a CI-only commit.

@DilumAluthge
Copy link
Member Author

@KristofferC Any objection to moving forward with this PR as a bug fix (and then doing the more involved work in #997)?

@DilumAluthge
Copy link
Member Author

Closing in favor of #1002.

@DilumAluthge DilumAluthge deleted the dpa/pass-buildroot branch November 21, 2024 00:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Regression in Julia 1.12: Creating fresh base sysimage (incremental=false) fails with error during bootstrap
1 participant