-
-
Notifications
You must be signed in to change notification settings - Fork 266
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
Fix HistoricalStdlibVersions install to version #3930
Conversation
test/runtests.jl
Outdated
@@ -42,7 +42,7 @@ Logging.with_logger((islogging || Pkg.DEFAULT_IO[] == devnull) ? Logging.Console | |||
iob = IOBuffer() | |||
Pkg.activate(; temp = true) | |||
try | |||
Pkg.add("HistoricalStdlibVersions", io=iob) # Needed for custom julia version resolve tests | |||
Pkg.add(name="HistoricalStdlibVersions", version="1.2", io=iob) # Needed for custom julia version resolve tests |
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.
Can we also take this opportunity to specify the UUID?
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.
Go ahead
Co-authored-by: Dilum Aluthge <dilum@aluthge.com>
@IanButterworth The backport label for 1.10 has no hyphen, but the backport label for 1.11 does have a hyphen. Is that something we need to fix? Does this break the Backporter script? |
@@ -42,7 +42,7 @@ Logging.with_logger((islogging || Pkg.DEFAULT_IO[] == devnull) ? Logging.Console | |||
iob = IOBuffer() | |||
Pkg.activate(; temp = true) | |||
try | |||
Pkg.add("HistoricalStdlibVersions", io=iob) # Needed for custom julia version resolve tests | |||
Pkg.add(name="HistoricalStdlibVersions", version="1.2", uuid="6df8b67a-e8a0-4029-b4b7-ac196fe72102", io=iob) # Needed for custom julia version resolve tests |
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.
Maybe we should parse the compat bound from the Project.toml?
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 thought about that, but what if we start doing "1.2,2"
. Part of the problem is this code path isn't hit on Pkg's own CI.
We might just want to teach Base.runtests
to run stdlib tests via Pkg.test
to handle test deps properly
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.
Is Pkg.test
guaranteed to pick up the correct version of the stdlib? (Because we're not doing the "modify the stdlib UUID trick.)
If we can get Pkg.test("StdlibName")
to work correctly in Base.runtests
, I agree that's probably the best solution.
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 thought about that, but what if we start doing
"1.2,2"
.
That's exactly why I want this. Pkg.add(;..., version=VersionSpec(...))
works, so you can do something like the following:
hsv_compat_val = TOML.parsefile(project_path)["compat"]["HistoricalStdlibVersions"]
Pkg.add(;name="HistoricalStdlibVersions", version=Pkg.Types.semver_spec(hsv_compat_val))
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.
IIRC, we do some shenanigans with the JULIA_PROJECT
and JULIA_LOAD_PATH
. So we need to make sure that Pkg.test
still works correctly in that case.
Also, how will this work with testgroups? For stdlibs, Base.runtests
is supposed to look at the test/testgroups
file, and run each testgroup in parallel. Would we have to teach Pkg.test
to understand stdlib testgroups? I'm not sure if we want to add that kind of functionality as public API in Pkg, but maybe we can add it as a private/internal Pkg API that we only use for stdlib tests in Base.runtests
.
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.
Something like Pkg.test("StdlibName"; _stdlib_testgroup = "desired_test_group")
might be sufficient. And we keep _stdlib_testgroup
as private/internal API only.
* fix hsv install to version * Update test/runtests.jl Co-authored-by: Dilum Aluthge <dilum@aluthge.com> --------- Co-authored-by: Dilum Aluthge <dilum@aluthge.com> (cherry picked from commit 8c99679)
* fix hsv install to version * Update test/runtests.jl Co-authored-by: Dilum Aluthge <dilum@aluthge.com> --------- Co-authored-by: Dilum Aluthge <dilum@aluthge.com>
Fixes #3929
Given this branch isn't touched on this CI