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

Fix HistoricalStdlibVersions install to version #3930

Merged
merged 2 commits into from
Jun 18, 2024

Conversation

IanButterworth
Copy link
Sponsor Member

@IanButterworth IanButterworth commented Jun 18, 2024

Fixes #3929

Given this branch isn't touched on this CI

% julia  
               _
   _       _ _(_)_     |  Documentation: https://docs.julialang.org
  (_)     | (_) (_)    |
   _ _   _| |_  __ _   |  Type "?" for help, "]?" for Pkg help.
  | | | | | | |/ _` |  |
  | | |_| | | | (_| |  |  Version 1.10.4 (2024-06-04)
 _/ |\__'_|_|_|\__'_|  |  Official https://julialang.org/ release
|__/                   |

(@v1.10) pkg> activate --temp
  Activating new project at `/var/folders/1z/jf841bdj73bdj3vk7kc7f_3w0000gn/T/jl_0iHV55`

julia> import Pkg

julia> Pkg.add(name="HistoricalStdlibVersions", version="1.2")
   Resolving package versions...
    Updating `/private/var/folders/1z/jf841bdj73bdj3vk7kc7f_3w0000gn/T/jl_0iHV55/Project.toml`
⌃ [6df8b67a] + HistoricalStdlibVersions v1.2.4
    Updating `/private/var/folders/1z/jf841bdj73bdj3vk7kc7f_3w0000gn/T/jl_0iHV55/Manifest.toml`
⌃ [6df8b67a] + HistoricalStdlibVersions v1.2.4
        Info Packages marked with ⌃ have new versions available and may be upgradable.

julia> 
julia> Pkg.add(name="HistoricalStdlibVersions", version="1.2", io=iob)

julia>

@IanButterworth IanButterworth changed the title fix hsv install to version Fix HistoricalStdlibVersions install to version Jun 18, 2024
DilumAluthge
DilumAluthge previously approved these changes Jun 18, 2024
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
Copy link
Member

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?

Copy link
Sponsor Member Author

Choose a reason for hiding this comment

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

Go ahead

test/runtests.jl Outdated Show resolved Hide resolved
Co-authored-by: Dilum Aluthge <dilum@aluthge.com>
@DilumAluthge DilumAluthge added the merge me Merge when tests pass label Jun 18, 2024
@DilumAluthge
Copy link
Member

@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?

@IanButterworth
Copy link
Sponsor Member Author

Good catch https://github.com/KristofferC/Backporter/blob/f366a87a94c9d38fd107a1c53ccc4550f3687625/backporter.jl#L23

Fixed

@DilumAluthge DilumAluthge merged commit 8c99679 into JuliaLang:master Jun 18, 2024
7 checks passed
@DilumAluthge DilumAluthge removed the merge me Merge when tests pass label Jun 18, 2024
@@ -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
Copy link
Sponsor Member

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?

Copy link
Sponsor Member Author

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

Copy link
Member

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.

Copy link
Sponsor Member

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))

Copy link
Member

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.

Copy link
Member

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.

@IanButterworth IanButterworth deleted the ib/fix_hsv branch June 18, 2024 16:38
KristofferC pushed a commit that referenced this pull request Jun 22, 2024
* 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)
KristofferC pushed a commit that referenced this pull request Jul 5, 2024
* 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>
IanButterworth added a commit that referenced this pull request Jul 13, 2024
* 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)
(cherry picked from commit 280f702)
@IanButterworth IanButterworth mentioned this pull request Aug 5, 2024
6 tasks
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.

New HistoricalStdlibVersions version is breaking the Pkg tests
3 participants