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

Ignore lack of project.toml for removed stdlibs when julia_version != VERSION #3362

Closed
wants to merge 3 commits into from

Conversation

staticfloat
Copy link
Sponsor Member

Because DelimitedFiles was removed, the assumption in this conditional
(that if something is a stdlib, it exists locally) has been broken.
Because I don't think it's reasonable to go out and clone stdlibs at
this point, I'm just skipping the dependency (And any of its
dependencies) because this codepath is primarily used by BinaryBuilder,
where we really only care about JLLs anyway, which are never
unregistered.

…= VERSION`

Because DelimitedFiles was removed, the assumption in this conditional
(that if something is a stdlib, it exists locally) has been broken.
Because I don't think it's reasonable to go out and clone stdlibs at
this point, I'm just skipping the dependency (And any of its
dependencies) because this codepath is primarily used by BinaryBuilder,
where we really only care about JLLs anyway, which are never
unregistered.
@staticfloat
Copy link
Sponsor Member Author

MWE of the error:

using Pkg, HistoricalStdlibVersions, Base.BinaryPlatforms
empty!(Pkg.Types.STDLIBS_BY_VERSION)
append!(Pkg.Types.STDLIBS_BY_VERSION, HistoricalStdlibVersions.STDLIBS_BY_VERSION)
empty!(Pkg.Types.UNREGISTERED_STDLIBS)
merge!(Pkg.Types.UNREGISTERED_STDLIBS, HistoricalStdlibVersions.UNREGISTERED_STDLIBS)

julia_version = v"1.5"
mktempdir() do dir
    Pkg.activate(dir) do
        Pkg.add([Pkg.PackageSpec(name="Zstd_jll")]; platform=Platform("x86_64", "linux"), julia_version)
    end
end

@KristofferC
Copy link
Sponsor Member

KristofferC commented Feb 6, 2023

Maybe this is not the right place but I've been trying to understand what exactly BinaryBuilder wants from Pkg that requires all this stdlib shenanigans (cc @giordano). I am somewhat certain that the current stuff is a local maxima based on everyone having partial knowledge about the problem.

where we really only care about JLLs anyway,

Regarding this: For example in 20ba6dd I made an option where jlls are not considered stdlibs at all and are always resolved from the registry. That seemed to work well enough that it got made into a monkey patch for libjulia in Yggdrasil: JuliaPackaging/Yggdrasil@fedc934#diff-29b8d08819b1a971a0a4f552117850f0569b1947bc3390d698455c6e3b7557b7. Is that what is actually needed? If we can just ignore stdlibs then it seems the whole stdlib history stuff is kind of pointless and what we really want is just to be able to consider jlls normal packages?

@staticfloat
Copy link
Sponsor Member Author

I made an option where jlls are not considered stdlibs at all and are always resolved from the registry.

The only part of the julia_version stuff that I care about is resolving JLLs. I think when I originally wrote the julia_version support, @IanButterworth got kind of excited for the ability to resolve a manifest for other Julia versions (e.g. write out a manifest using Julia v1.8 for Julia v1.6 that you could then load without doing a further resolve (you'd still need to instantiate, of course)) but I don't think that actually ended up getting used anywhere, so I'm happy to most of this stuff removed from Pkg if possible.

What we really want is just to be able to consider jlls normal packages?

I don't think this is sufficient... the issue is that we need Pkg to download things for a particular version of Julia. This is important when we're e.g. building a new version of a JLL that ships with Julia. As it stands now, we also don't have tight enough constraints within the registry on some of the JLL stdlibs, but we can go back and fix that, so it's not that big of a deal.

Maybe this is not the right place

I think a good place for this discussion is on the JLLPrefixes issue tracker. That's the package where I'm trying to isolate all Pkg-resolution-related behavior for BinaryBuilder. The idea is that you pass it a few JLL names and a Platform object, and it uses the Pkg resolver to figure out all the different JLLs that you need to download, grabs them for that platform (including Julia version compatibility constraints!) and then you can copy them all into a shared prefix. I have a test suite in that package that is fairly short and sweet, and should help you to understand in greater clarity exactly what we want.

@KristofferC
Copy link
Sponsor Member

This should probably re enable the disabled tests for adding jlls etc?

@KristofferC
Copy link
Sponsor Member

KristofferC commented Feb 21, 2023

Thinking about it, JuliaLang/julia#48671 might make this PR redundant.

Edit: Or maybe not since DelimitedFiles will not be considered an stdlib by Pkg..

@staticfloat
Copy link
Sponsor Member Author

I have double-checked with both the tip of master and backports-release-1.9. They both work just fine with this MWE as well as the larger tests in JLLPrefixes. So from my perspective, we can consider this closed now.

@KristofferC
Copy link
Sponsor Member

We should still re-enable the tests I presume?

@giordano giordano deleted the sf/removed_stdlibs branch March 20, 2023 13:49
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.

None yet

2 participants