Merge #443

443: Only clone once, not once per subdir r=ericphanson a=ericphanson Closes #442 This PR has two commits (+ formatting); the first lets tests pass locally on my machine, by specifying the `init-branch` instead of relying on it being set to `master`. The second does the change to close #442. Besides speeding things up and possibly avoiding weird cloning issues, this also makes the code more "compositional" and simplifies the tests. edit: I've also pushed an additional commit to re-use the same local clone for each PR. It seems wasteful to make so many clones, and in #442 for some reason for me the first clone seems to work and the subsequent ones fail. Co-authored-by: Eric Hanson <[email protected]> Co-authored-by: mattBrzezinski <[email protected]>
JuliaRegistries · Sep 24, 2023 · e1ce1c4 · e1ce1c4
2 parents 8ac9f11 + 7b8e19a
commit e1ce1c4
Show file tree

Hide file tree

Showing 6 changed files with 98 additions and 114 deletions.
diff --git a/src/dependencies.jl b/src/dependencies.jl
@@ -1,28 +1,19 @@
 const LOCAL_REPO_NAME = "REPO"
 
-function get_project_deps(
-    api::Forge,
-    ci::CIService,
-    repo::Union{GitHub.Repo,GitLab.Project};
-    options::Options,
-    subdir::AbstractString,
+function get_local_clone(
+    api::Forge, ci::CIService, repo::Union{GitHub.Repo,GitLab.Project}; options
 )
-    mktempdir() do f
-        url_with_auth = get_url_with_auth(api, ci, repo)
-        local_path = joinpath(f, LOCAL_REPO_NAME)
-        @mock git_clone(url_with_auth, local_path)
-
-        @mock cd(local_path) do
-            master_branch = @mock git_get_master_branch(options.master_branch)
-            @mock git_checkout(master_branch)
-        end
-
-        # Get all the compat dependencies from the local Project.toml file
-        project_file = @mock joinpath(local_path, subdir, "Project.toml")
-        deps = get_project_deps(project_file; include_jll=options.include_jll)
-
-        return deps
+    f = mktempdir()
+    url_with_auth = get_url_with_auth(api, ci, repo)
+    local_path = joinpath(f, LOCAL_REPO_NAME)
+    @mock git_clone(url_with_auth, local_path)
+
+    @mock cd(local_path) do
+        master_branch = @mock git_get_master_branch(options.master_branch)
+        @mock git_checkout(master_branch)
     end
+
+    return local_path
 end
 
 function get_project_deps(project_file::AbstractString; include_jll::Bool=false)

diff --git a/src/main.jl b/src/main.jl
@@ -47,8 +47,11 @@ function main(
 
     api, repo = get_api_and_repo(ci_cfg)
 
+    local_clone_path = get_local_clone(api, ci_cfg, repo; options)
+
     for subdir in options.subdirs
-        deps = get_project_deps(api, ci_cfg, repo; options, subdir)
+        project_file = @mock joinpath(local_clone_path, subdir, "Project.toml")
+        deps = get_project_deps(project_file; include_jll=options.include_jll)
 
         if options.use_existing_registries
             get_existing_registries!(deps, options.depot; options)
@@ -58,7 +61,14 @@ function main(
 
         for dep in deps
             pr = @mock make_pr_for_new_version(
-                api, repo, dep, options.entry_type, ci_cfg; options, subdir
+                api,
+                repo,
+                dep,
+                options.entry_type,
+                ci_cfg;
+                options,
+                subdir,
+                local_clone_path,
             )
 
             if !isnothing(pr)

diff --git a/src/utilities/new_versions.jl b/src/utilities/new_versions.jl
@@ -208,6 +208,7 @@ function make_pr_for_new_version(
     env=ENV,
     options::Options,
     subdir::String,
+    local_clone_path::AbstractString,
 )
     if !continue_with_pr(dep, options.bump_compat_containing_equality_specifier)
         return nothing
@@ -250,60 +251,50 @@ function make_pr_for_new_version(
             repo_git_url = @mock get_url_with_auth(forge, ci_cfg, repo)
         end
 
-        # In a temp dir, grab the repo, make the changes, push and make a PR
-        with_temp_dir(; cleanup=true) do tmpdir
-            # Clone Repo Locally
+        # Go to our local clone
+        cd(local_clone_path)
+
+        # Checkout master branch
+        master_branch_name = git_get_master_branch(options.master_branch)
+        git_checkout(master_branch_name)
+
+        # Create compathelper branch and check it out
+        new_branch_name = "compathelper/new_version/$(get_random_string())"
+        git_branch(new_branch_name; checkout=true)
+
+        # Add new compat entry to project.toml, bump the version if needed,
+        # and write it out
+        modify_project_toml(
+            dep.package.name,
+            joinpath(local_clone_path, subdir),
+            brand_new_compat,
+            options.bump_version,
+        )
+        git_add()
+
+        # Commit changes and make PR
+        @info("Attempting to commit...")
+        commit_was_success = git_commit(new_pr_title; env=env)
+        if commit_was_success
+            @info("Commit was a success")
             api_retry() do
-                git_clone(repo_git_url, LOCAL_REPO_NAME, pkey_filename)
+                @mock git_push(
+                    "origin", new_branch_name, pkey_filename; force=true, env=env
+                )
             end
-            cd(joinpath(tmpdir, LOCAL_REPO_NAME))
 
-            # Checkout master branch
-            master_branch_name = git_get_master_branch(options.master_branch)
-            git_checkout(master_branch_name)
-
-            # Create compathelper branch and check it out
-            new_branch_name = "compathelper/new_version/$(get_random_string())"
-            git_branch(new_branch_name; checkout=true)
-
-            # Add new compat entry to project.toml, bump the version if needed,
-            # and write it out
-            modify_project_toml(
-                dep.package.name,
-                joinpath(tmpdir, LOCAL_REPO_NAME, subdir),
-                brand_new_compat,
-                options.bump_version,
+            new_pr, _ = create_new_pull_request(
+                forge, repo, new_branch_name, master_branch_name, new_pr_title, new_pr_body
             )
-            git_add()
-
-            # Commit changes and make PR
-            @info("Attempting to commit...")
-            commit_was_success = git_commit(new_pr_title; env=env)
-            if commit_was_success
-                @info("Commit was a success")
-                api_retry() do
-                    @mock git_push(
-                        "origin", new_branch_name, pkey_filename; force=true, env=env
-                    )
-                end
-
-                new_pr, _ = create_new_pull_request(
-                    forge,
-                    repo,
-                    new_branch_name,
-                    master_branch_name,
-                    new_pr_title,
-                    new_pr_body,
-                )
 
-                options.cc_user && cc_mention_user(forge, repo, new_pr; env=env)
-                options.unsub_from_prs && unsub_from_pr(forge, new_pr)
-                force_ci_trigger(
-                    forge, new_pr_title, new_branch_name, pkey_filename; env=env
-                )
+            options.cc_user && cc_mention_user(forge, repo, new_pr; env=env)
+            options.unsub_from_prs && unsub_from_pr(forge, new_pr)
+            force_ci_trigger(forge, new_pr_title, new_branch_name, pkey_filename; env=env)
 
-                created_pr = new_pr
-            end
+            # Return to the master branch
+            git_checkout(master_branch_name)
+
+            created_pr = new_pr
         end
     end
 

diff --git a/test/dependencies.jl b/test/dependencies.jl
@@ -1,35 +1,23 @@
-@testset "get_project_deps" begin
-    @testset "no jll" begin
-        apply([git_clone_patch, project_toml_patch, cd_patch]) do
-            options = CompatHelper.Options()
-            subdir = only(options.subdirs)
-            deps = CompatHelper.get_project_deps(
-                GitForge.GitHub.GitHubAPI(; token=GitHub.Token("token")),
-                GitHubActions(),
-                GitHub.Repo(; full_name="foobar");
-                options=options,
-                subdir=subdir,
-            )
-
-            @test length(deps) == 1
-        end
+@testset "get_local_clone" begin
+    apply([git_clone_patch, cd_patch]) do
+        options = CompatHelper.Options()
+        local_path = CompatHelper.get_local_clone(
+            GitForge.GitHub.GitHubAPI(; token=GitHub.Token("token")),
+            GitHubActions(),
+            GitHub.Repo(; full_name="foobar");
+            options,
+        )
+        @test local_path isa String
     end
+end
 
-    @testset "include_jll" begin
-        apply([git_clone_patch, project_toml_patch, cd_patch]) do
-            options = CompatHelper.Options(; include_jll=true)
-            subdir = only(options.subdirs)
-            deps = CompatHelper.get_project_deps(
-                GitForge.GitHub.GitHubAPI(; token=GitHub.Token("token")),
-                GitHubActions(),
-                GitHub.Repo(; full_name="foobar");
-                options=options,
-                subdir=subdir,
-            )
-
-            @test length(deps) == 2
-        end
-    end
+@testset "get_project_deps" begin
+    project = joinpath(@__DIR__, "deps", "Project.toml")
+
+    deps = CompatHelper.get_project_deps(project; include_jll=true)
+    @test length(deps) == 2
+    deps = CompatHelper.get_project_deps(project; include_jll=false)
+    @test length(deps) == 1
 end
 
 @testset "clone_all_registries" begin

diff --git a/test/utilities/git.jl b/test/utilities/git.jl
@@ -22,7 +22,7 @@ QQDtEmQvWdgz+HtIuTG1ySJ9FYO6LeCEXHtQX78aOfNaj2jqLTXHdqrMr0V5exJcNV4XSc
 @testset "git_push" begin
     function create_local_remote(dir::AbstractString)
         remote_path = joinpath(dir, "localremote.git")
-        run(`git init --bare $remote_path`)
+        run(`git init --initial-branch=master --bare $remote_path`)
 
         return remote_path
     end
@@ -35,7 +35,7 @@ QQDtEmQvWdgz+HtIuTG1ySJ9FYO6LeCEXHtQX78aOfNaj2jqLTXHdqrMr0V5exJcNV4XSc
                 local_remote_path = create_local_remote(local_remote_dir)
 
                 cd(f) do
-                    run(`git init`)
+                    run(`git init --initial-branch=master`)
                     run(`git remote add origin $local_remote_path`)
 
                     run(`touch foobar.txt`)
@@ -60,7 +60,7 @@ QQDtEmQvWdgz+HtIuTG1ySJ9FYO6LeCEXHtQX78aOfNaj2jqLTXHdqrMr0V5exJcNV4XSc
                 local_remote_path = create_local_remote(local_remote_dir)
 
                 cd(f) do
-                    run(`git init`)
+                    run(`git init --initial-branch=master`)
                     run(`git remote add origin $local_remote_path`)
 
                     run(`touch foobar.txt`)
@@ -104,7 +104,7 @@ QQDtEmQvWdgz+HtIuTG1ySJ9FYO6LeCEXHtQX78aOfNaj2jqLTXHdqrMr0V5exJcNV4XSc
                 local_remote_path = create_local_remote(local_remote_dir)
 
                 cd(f) do
-                    run(`git init`)
+                    run(`git init --initial-branch=master`)
                     run(`git remote add origin $local_remote_path`)
 
                     run(`touch foobar.txt`)
@@ -127,7 +127,7 @@ end
 @testset "git_reset" begin
     mktempdir() do f
         cd(f) do
-            run(`git init`)
+            run(`git init --initial-branch=master`)
 
             @test !isfile("foobar.txt")
 
@@ -163,7 +163,7 @@ end
     @testset "success" begin
         mktempdir() do f
             cd(f) do
-                run(`git init`)
+                run(`git init --initial-branch=master`)
                 run(`touch foobar.txt`)
                 CompatHelper.git_add()
 
@@ -175,7 +175,7 @@ end
     @testset "failure" begin
         mktempdir() do f
             cd(f) do
-                run(`git init`)
+                run(`git init --initial-branch=master`)
                 run(`touch foobar.txt`)
                 CompatHelper.git_add()
 
@@ -194,7 +194,7 @@ end
     @testset "no checkout" begin
         mktempdir() do f
             cd(f) do
-                run(`git init`)
+                run(`git init --initial-branch=master`)
                 run(`touch foobar.txt`)
                 CompatHelper.git_add()
                 CompatHelper.git_commit("Message")
@@ -218,7 +218,7 @@ end
     @testset "with checkout" begin
         mktempdir() do f
             cd(f) do
-                run(`git init`)
+                run(`git init --initial-branch=master`)
                 run(`touch foobar.txt`)
                 CompatHelper.git_add()
                 CompatHelper.git_commit("Message")
@@ -241,7 +241,7 @@ end
 
     mktempdir() do f
         cd(f) do
-            run(`git init`)
+            run(`git init --initial-branch=master`)
             run(`touch foo.txt`)
             run(`touch bar.txt`)
 
@@ -314,7 +314,7 @@ end
 
     mktempdir() do f
         cd(f) do
-            run(`git init`)
+            run(`git init --initial-branch=master`)
             # Need to create a commit before hand, see below
             # https://stackoverflow.com/a/63480330/1327636
             run(`touch foobar.txt`)
@@ -334,7 +334,7 @@ end
         branch = "master"
         mktempdir() do f
             cd(f) do
-                run(`git init`)
+                run(`git init --initial-branch=$(branch)`)
                 run(`touch foobar.txt`)
                 CompatHelper.git_add()
                 CompatHelper.git_commit("Message")
@@ -350,7 +350,7 @@ end
         branch = "main"
         mktempdir() do f
             cd(f) do
-                run(`git init`)
+                run(`git init --initial-branch=master`)
                 run(`git branch -m $branch`)
                 run(`touch foobar.txt`)
                 CompatHelper.git_add()

diff --git a/test/utilities/new_versions.jl b/test/utilities/new_versions.jl
@@ -435,6 +435,7 @@ end
                 CompatHelper.GitHubActions();
                 options,
                 subdir,
+                local_clone_path=mktempdir(),
             ),
         )
     end
@@ -456,6 +457,7 @@ end
                     CompatHelper.GitHubActions();
                     options,
                     subdir,
+                    local_clone_path=mktempdir(),
                 ),
             )
         end
@@ -509,6 +511,7 @@ end
                         CompatHelper.GitHubActions();
                         options,
                         subdir,
+                        local_clone_path=tmpdir,
                     )
                     @test pr isa GitHub.PullRequest
 
@@ -530,6 +533,7 @@ end
                             CompatHelper.GitHubActions();
                             options,
                             subdir,
+                            local_clone_path=tmpdir,
                         )
                         @test pr isa GitHub.PullRequest
                     end