-
Notifications
You must be signed in to change notification settings - Fork 31
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
Add compat with HistoricalStdlibVersions v2 #384
Conversation
Hmm, this will need some more work to be backwards compatible. |
1fbdea5
to
5be30ce
Compare
Alrighty, that should do the trick 🤞 Also closes #382. |
5be30ce
to
3cd11c6
Compare
On second thoughts, I made HistoricalStdlibVersions v2 a requirement since we now require the new |
src/BinaryBuilderBase.jl
Outdated
# is a StdlibInfo object, but on earlier versions it's a tuple so we have to | ||
# reconstruct the old tuple data structure from the StdlibInfo data | ||
# structures in HistoricalStdlibVersions.STDLIBS_BY_VERSION. | ||
if VERSION > v"1.11" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure this condition is correct. Can you elaborate on words what you mean?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, in JuliaLang/Pkg.jl#3911 the DictStdLibs
definition was changed: https://github.com/JuliaLang/Pkg.jl/pull/3911/files#diff-8387f5e55b4815b8e65004b6d2e4039e1f5cb8525cc9c513c69bee31fec44890L3-R14
Its values are now the new StdlibInfo
type instead of a Tuple{String,Union{VersionNumber,Nothing}}
. The STDLIBS_BY_VERSION
variable is defined using DictStdLibs
: https://github.com/JuliaLang/Pkg.jl/blob/e6880bc9d8a04d95df6e341c76786219a4efc33f/src/HistoricalStdlibs.jl#L22
In BinaryBuilderBase we populate STDLIBS_BY_VERSION
using the values from HistoricalStdlibVersions.STDLIBS_BY_VERSION
, which changed in JuliaPackaging/HistoricalStdlibVersions.jl#23 to use the new StdlibInfo
.
So when BinaryBuilderBase is loaded on nightly using the new Pkg version (which was added to nightly in JuliaLang/julia#55112) but with the old compat of HistoricalStdlibs v1, it'll try to insert a dictionary of tuple values from HistoricalStdlibs into the dictionary in Pkg of StdlibInfo
values, and fail. This change will make it only insert them directly on Julia 1.12+ with the new Pkg version, and otherwise fall back to recreating the tuple form and inserting those.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, actually I'm an idiot 🤦 I did not notice that v2 can do the whole thing for us: https://github.com/JuliaPackaging/HistoricalStdlibVersions.jl/blob/5879c5f690795208481c60b904f4af4e8c1eeef8/src/HistoricalStdlibVersions.jl#L38
Fixed in 966be26.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My point was that if you want to say "from v1.12 onwards" then > v"1.11"
was wrong, because that matches already v"1.11.1"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, yes you're quite right.
This fixes support for nightly with the new Pkg version: JuliaLang/Pkg.jl#3911.
3cd11c6
to
966be26
Compare
I these cases bumping the version number of the package makes life easier for everybody and increases the probability of a new release happening. |
Bumped in 63702e3. |
This fixes support for nightly with the new Pkg version: JuliaLang/Pkg.jl#3911. Would it be possible to make a new release once it's merged?
Example failure from the Clang.jl tests:
Close #382.