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

Open PRs for compat entries to weak dependencies #458

Merged
merged 20 commits into from
Jan 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Project.toml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
name = "CompatHelper"
uuid = "aa819f21-2bde-4658-8897-bab36330d9b7"
authors = ["Dilum Aluthge", "Brown Center for Biomedical Informatics", "contributors"]
version = "3.10.0"
version = "3.11.0"

[deps]
Base64 = "2a0f44e3-6c83-55bd-87e4-b1978d98bd5f"
Expand Down
48 changes: 26 additions & 22 deletions src/dependencies.jl
Original file line number Diff line number Diff line change
Expand Up @@ -18,35 +18,39 @@ end

function get_project_deps(project_file::AbstractString; include_jll::Bool=false)
project_deps = Set{DepInfo}()
dep_section = Dict{DepInfo,String}()
project = TOML.parsefile(project_file)

if haskey(project, "deps")
deps = project["deps"]
add_compat_section!(project)
compat = project["compat"]

for dep in deps
name = dep[1]
uuid = UUIDs.UUID(dep[2])

# Ignore JLL packages if flag set
# Do NOT ignore stdlib packages.
if !endswith(lowercase(strip(name)), "_jll") || include_jll
package = Package(name, uuid)
compat_entry = DepInfo(package)
dep_entry = convert(String, strip(get(compat, name, "")))

if !isempty(dep_entry)
compat_entry.version_spec = semver_spec(dep_entry)
compat_entry.version_verbatim = dep_entry
for section in ["deps", "weakdeps"]
if haskey(project, section)
deps = project[section]
add_compat_section!(project)
compat = project["compat"]

for dep in deps
name = dep[1]
uuid = UUIDs.UUID(dep[2])

# Ignore JLL packages if flag set
# Do NOT ignore stdlib packages.
if !endswith(lowercase(strip(name)), "_jll") || include_jll
package = Package(name, uuid)
compat_entry = DepInfo(package)
dep_entry = convert(String, strip(get(compat, name, "")))

if !isempty(dep_entry)
compat_entry.version_spec = semver_spec(dep_entry)
compat_entry.version_verbatim = dep_entry
end

push!(project_deps, compat_entry)
get!(dep_section, compat_entry, section)
Copy link
Member

Choose a reason for hiding this comment

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

what if a package is in multiple sections? E.g. [deps] and [weakdeps] is a documented compatibility strategy

Copy link
Contributor Author

@sethaxen sethaxen Nov 20, 2023

Choose a reason for hiding this comment

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

Ah, I had thought it would make sense to always note that is in [deps] vs [weakdeps] (or in the future [extras]), but yes, the case covered in that docs page is one where one would want to highlight [weakdeps]. Perhaps then it makes sense to list no section in the PR title when the dependency is only in [deps] and otherwise give a comma-separated list of sections (currently [weakdeps] or [deps], [weakdeps])

Copy link
Member

Choose a reason for hiding this comment

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

If it’s only the title it seems like not too big of a deal either way, just wanted to make sure semantically it was OK. Maybe a simple choice is to just use deps in the title when present by looping over weak deps then deps (or vice versa if you prefer)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, that's currently how this works. get!(dep_section, compat_entry, section) ensures that dep_section[compat_entry] is only set once to a section for a given package, and if the package was found in the deps section, it's already set, so weakdeps will be ignored.

Copy link
Member

Choose a reason for hiding this comment

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

If possible, I'd like the titles for regular deps to remain the same as the old titles, and for the new title format to only be used for weakdeps.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@DilumAluthge this has already been handled in 3e52390

end

push!(project_deps, compat_entry)
end
end
end

return project_deps
return project_deps, dep_section
end

function clone_all_registries(f::Function, registry_list::Vector{Pkg.RegistrySpec})
Expand Down
4 changes: 3 additions & 1 deletion src/main.jl
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,12 @@ function main(

for subdir in options.subdirs
project_file = @mock joinpath(local_clone_path, subdir, "Project.toml")
deps = get_project_deps(project_file; include_jll=options.include_jll)
deps, dep_sections = get_project_deps(project_file; include_jll=options.include_jll)

populate_dep_versions_from_reg!(deps; options)

for dep in deps
dep_section = dep_sections[dep]
pr = @mock make_pr_for_new_version(
api,
repo,
Expand All @@ -65,6 +66,7 @@ function main(
options,
subdir,
local_clone_path,
dep_section,
)

if !isnothing(pr)
Expand Down
12 changes: 10 additions & 2 deletions src/utilities/new_versions.jl
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,14 @@ function subdir_string(subdir::AbstractString)
end
end

function section_string(section::AbstractString)
return section == "deps" ? "" : " in [$(section)]"
end

function pr_info(
compat_entry_verbatim::Nothing,
name::AbstractString,
section_str::AbstractString,
compat_entry_for_latest_version::AbstractString,
compat_entry::AbstractString,
subdir_string::AbstractString,
Expand All @@ -54,7 +59,7 @@ function pr_info(
pr_title_prefix::String,
)
new_pr_title = m"""
$(pr_title_prefix)CompatHelper: add new compat entry for $(name) at version
$(pr_title_prefix)CompatHelper: add new compat entry for $(name)$(section_str) at version
$(compat_entry_for_latest_version)$(subdir_string)$(pr_title_parenthetical)
"""

Expand All @@ -73,6 +78,7 @@ end
function pr_info(
compat_entry_verbatim::AbstractString,
name::AbstractString,
section_str::AbstractString,
compat_entry_for_latest_version::AbstractString,
compat_entry::AbstractString,
subdir_string::AbstractString,
Expand All @@ -81,7 +87,7 @@ function pr_info(
pr_title_prefix::String,
)
new_pr_title = m"""
$(pr_title_prefix)CompatHelper: bump compat for $(name) to
$(pr_title_prefix)CompatHelper: bump compat for $(name)$(section_str) to
$(compat_entry_for_latest_version)$(subdir_string)$(pr_title_parenthetical)
"""

Expand Down Expand Up @@ -209,6 +215,7 @@ function make_pr_for_new_version(
options::Options,
subdir::String,
local_clone_path::AbstractString,
dep_section::String,
)
if !continue_with_pr(dep, options.bump_compat_containing_equality_specifier)
return nothing
Expand All @@ -222,6 +229,7 @@ function make_pr_for_new_version(
new_pr_title, new_pr_body = pr_info(
dep.version_verbatim,
dep.package.name,
section_string(dep_section),
compat_entry_for_latest_version,
brand_new_compat,
subdir_string(subdir),
Expand Down
25 changes: 21 additions & 4 deletions test/dependencies.jl
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,27 @@ end
@testset "get_project_deps" begin
project = joinpath(@__DIR__, "deps", "Project.toml")

deps = CompatHelper.get_project_deps(project; include_jll=true)
deps, dep_section = CompatHelper.get_project_deps(project; include_jll=true)
@test length(deps) == 5
@test issetequal(keys(dep_section), deps)
for (k, s) in pairs(dep_section)
if k.package.name ∈ ["Bex_jll", "Skix"]
@test s == "weakdeps"
else
@test s == "deps"
end
end

deps, dep_section = CompatHelper.get_project_deps(project; include_jll=false)
@test length(deps) == 3
deps = CompatHelper.get_project_deps(project; include_jll=false)
@test length(deps) == 2
@test issetequal(keys(dep_section), deps)
for (k, s) in pairs(dep_section)
if k.package.name == "Skix"
@test s == "weakdeps"
else
@test s == "deps"
end
end
end

@testset "clone_all_registries" begin
Expand Down Expand Up @@ -131,7 +148,7 @@ end
@test isfile(project_file)
for use_existing_registries in [true, false]
options = CompatHelper.Options(; use_existing_registries)
deps = CompatHelper.get_project_deps(project_file)
deps, _ = CompatHelper.get_project_deps(project_file)
for dep in deps
@test dep.latest_version === nothing
end
Expand Down
9 changes: 9 additions & 0 deletions test/deps/Project.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,16 @@ Foobar_jll = "6ca821de-e512-569d-89d9-0b16ce691416"
Baz = "ea10d353-3f73-51f8-a26c-33c1cb351aa5"
LinearAlgebra = "37e2e46d-f89d-539d-b4ee-838fcccc9c8e"

[weakdeps]
Bex_jll = "3db6da90-6ed3-11ee-0779-f549c8e3e90d"
Skix = "3db6da90-6ed3-11ee-0779-f549c8e3e90d"

[extensions]
Ext = ["Bex_jll", "Skix"]

[compat]
Foobar_jll = "1"
Baz = "1"
Bex_jll = "1"
Skix = "1"
julia = "1.6"
100 changes: 100 additions & 0 deletions test/integration_tests.jl
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,106 @@ function run_integration_tests(
end
end

sleep(1) # Prevent hitting the GH Secondary Rate Limits

@testset "master_9" begin
with_master_branch(templates("master_9"), url, "master") do master_9
withenv(env...) do
CompatHelper.main(
ENV,
ci_cfg;
pr_title_prefix="$(GLOBAL_PR_TITLE_PREFIX) [test-9c] ",
master_branch=master_9,
entry_type=KeepEntry(),
)
end
end
end

sleep(1) # Prevent hitting the GH Secondary Rate Limits

@testset "master_10" begin
with_master_branch(templates("master_10"), url, "master") do master_10
withenv(env...) do
CompatHelper.main(
ENV,
ci_cfg;
pr_title_prefix="$(GLOBAL_PR_TITLE_PREFIX) [test-10a] ",
master_branch=master_10,
entry_type=DropEntry(),
)

sleep(1) # Prevent hitting the GH Secondary Rate Limits

CompatHelper.main(
ENV,
ci_cfg;
pr_title_prefix="$(GLOBAL_PR_TITLE_PREFIX) [test-10b] ",
master_branch=master_10,
entry_type=KeepEntry(),
)

sleep(1) # Prevent hitting the GH Secondary Rate Limits

CompatHelper.main(
ENV,
ci_cfg;
pr_title_prefix="$(GLOBAL_PR_TITLE_PREFIX) [test-10c] ",
master_branch=master_10,
entry_type=KeepEntry(),
)
end
end
end

sleep(1) # Prevent hitting the GH Secondary Rate Limits

@testset "master_11" begin
with_master_branch(templates("master_11"), url, "master") do master_11
withenv(env...) do
CompatHelper.main(
ENV,
ci_cfg;
pr_title_prefix="$(GLOBAL_PR_TITLE_PREFIX) [test-11c] ",
master_branch=master_11,
entry_type=KeepEntry(),
)
end
end
end

sleep(1) # Prevent hitting the GH Secondary Rate Limits

@testset "master_12" begin
with_master_branch(templates("master_12"), url, "master") do master_12
withenv(env...) do
CompatHelper.main(
ENV,
ci_cfg;
pr_title_prefix="$(GLOBAL_PR_TITLE_PREFIX) [test-12a] ",
master_branch=master_12,
entry_type=DropEntry(),
)
end
end
end

sleep(1) # Prevent hitting the GH Secondary Rate Limits

@testset "master_13" begin
with_master_branch(templates("master_13"), url, "master") do master_13
withenv(env...) do
CompatHelper.main(
ENV,
ci_cfg;
pr_title_prefix="$(GLOBAL_PR_TITLE_PREFIX) [test-13a] ",
master_branch=master_13,
entry_type=DropEntry(),
)
end
end
end

return _cleanup_old_branches(url)
end

Expand Down
6 changes: 4 additions & 2 deletions test/main.jl
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,9 @@
"GITHUB_REPOSITORY" => "CompatHelper.jl", "GITHUB_TOKEN" => "token"
) do
prs = CompatHelper.main()
@test length(prs) == 2
@test length(prs) == 3
@test prs[1] isa GitHub.PullRequest
@test prs[2] isa GitHub.PullRequest
end
end
end
Expand Down Expand Up @@ -96,8 +97,9 @@
) do
delete!(ENV, "GITHUB_REPOSITORY")
prs = CompatHelper.main()
@test length(prs) == 2
@test length(prs) == 3
@test prs[1] isa GitLab.MergeRequest
@test prs[2] isa GitLab.MergeRequest
end
end
end
Expand Down
28 changes: 28 additions & 0 deletions test/templates/master_10/Project.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
name = "compathelper_integration_test_repo"
uuid = "0e97b93d-f6aa-40c8-b749-d2f6bf239ed7"
authors = ["Brown Center for Biomedical Informatics"]
description = "Test Project.toml with both dependencies and weak dependencies, compat entries; major ver, ~"
version = "0.1.0"

[weakdeps]
BioSequences = "7e6ae17a-c86d-528c-b3b9-7f778a29fe59"
DataFrames = "a93c6f00-e57d-5684-b7b6-d8193f3e46c0"
Flux = "587475ba-b771-5e3f-ad9e-33799f191a9c"
IterableTables = "1c8ee90f-4401-5389-894e-7a04a3dc0f4d"
Nettle_jll = "4c82536e-c426-54e4-b420-14f461c4ed8b"
PGFPlotsX = "8314cec4-20b6-5062-9cdb-752b83310925"
Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"

[extensions]
Ext = ["BioSequences", "DataFrames", "Flux", "IterableTables", "Nettle_jll", "PGFPlotsX"]

[compat]
IterableTables = "1"
PGFPlotsX = "~1.0.0"
julia = "1.2"

[extras]
Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"

[targets]
test = ["Test"]
20 changes: 20 additions & 0 deletions test/templates/master_11/Project.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
name = "compathelper_integration_test_repo"
uuid = "0e97b93d-f6aa-40c8-b749-d2f6bf239ed7"
authors = ["Brown Center for Biomedical Informatics"]
description = "Test Project.toml with weak dependency where package does not exist"
version = "0.1.0"

[weakdeps]
ThisPackageDoesNotExist = "2c58d03c-75de-479c-aa0b-96bf0b358d76"

[extensions]
Ext = "ThisPackageDoesNotExist"

[compat]
julia = "1.2"

[extras]
Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"

[targets]
test = ["Test"]
Loading