SLING-13253 - update-local-site: shallow-clone the site checkout

README.md

@@ -216,16 +216,20 @@ After `release:perform` has staged the artifacts, drive the rest with the CLI:
    release guide asks for when a brand new module is released.
 
    The CLI keeps its own checkout of `sling-site` and never looks for one you may already have. It
-   defaults to `$HOME/.sling-cli/sling-site` — inside the container `/root/.sling-cli/sling-site`, which
-   is discarded with the container unless you mount it — and can be pointed elsewhere with
-   `--site-checkout`. Use a directory dedicated to this rather than a clone you work in: `master` is
-   checked out and hard-reset before each run, so anything uncommitted there is discarded, and the
-   release is then committed and pushed from it.
+   defaults to `$HOME/.sling-cli/sling-site`, which inside the container is `/root/.sling-cli/sling-site`
+   — so by default it is discarded with the container and re-cloned on every run. Only the tip of `master`
+   is cloned, which keeps that to about 25 MB rather than the ~380 MB a full clone of the site would take,
+   but to avoid re-cloning altogether mount a directory that outlives the container:
 
        docker run --env-file=./docker-env \
            -v "$HOME/.sling-cli:/root/.sling-cli" \
            apache/sling-committer-cli release finalize --repository=$STAGING_REPOSITORY_ID --execution-mode=AUTO
 
+   Use `--site-checkout` to put the checkout somewhere else. Prefer a directory dedicated to this purpose:
+   `master` is checked out and hard-reset before each run, so anything uncommitted there is discarded, and
+   the release is then committed and pushed from it. Pointing it at a clone you work in will not truncate
+   its history — the shallow fetch is only requested for a checkout that is already shallow.
+
    The news page is deliberately *not* part of `finalize` — the release guide only asks for a news entry
    when a release warrants an announcement. Run it by hand for those:
 

src/main/java/org/apache/sling/cli/impl/release/UpdateLocalSiteCommand.java

@@ -46,6 +46,7 @@
 import org.apache.sling.cli.impl.nexus.StagingRepository;
 import org.apache.sling.cli.impl.people.Member;
 import org.apache.sling.cli.impl.people.MembersFinder;
+import org.eclipse.jgit.api.FetchCommand;
 import org.eclipse.jgit.api.Git;
 import org.eclipse.jgit.api.ResetCommand.ResetType;
 import org.eclipse.jgit.api.errors.GitAPIException;
@@ -356,17 +357,18 @@
 
         if (!Paths.get(checkout).toFile().exists()) {
             createCheckoutParent(checkout);
-            try (Git ignored = Git.cloneRepository()
-                    .setURI(SITE_GIT_URL)
-                    .setProgressMonitor(new TextProgressMonitor())
-                    .setDirectory(new File(checkout))
-                    .setBranch(SITE_BRANCH)
-                    .call()) {
-                LOGGER.info("Cloned {} into {}.", SITE_GIT_URL, checkout);
-            }
+            cloneSite(SITE_GIT_URL, checkout);
         } else {
             try (Git git = Git.open(new File(checkout))) {
-                git.fetch().setProgressMonitor(new TextProgressMonitor()).call();
+                FetchCommand fetch = git.fetch().setProgressMonitor(new TextProgressMonitor());
+                // Stay shallow on refresh so a reused checkout does not grow into a full clone - but only
+                // when it is shallow already. Asking for depth 1 against a full clone truncates its history
+                // permanently (git writes .git/shallow, and recovering needs --unshallow), and the checkout
+                // location is configurable, so it may be a clone someone else depends on.
+                if (!git.getRepository().getObjectDatabase().getShallowCommits().isEmpty()) {
+                    fetch.setDepth(1);
+                }
+                fetch.call();
                 // discard working tree changes before switching branches: a modified file whose content
                 // differs between the current branch and the published one makes the checkout fail with a
                 // conflict, and the whole point of this location being configurable is that it may be a
@@ -381,6 +383,30 @@
         }
     }
 
+    /**
+     * Clones {@code uri} into {@code checkout}, fetching only the tip of the published branch: the site
+     * content is edited and committed on top of it, never inspected historically. A full clone of the site
+     * repository is a few hundred MB of history against ~15 MB of content, and by default this clone
+     * happens on every run, since the checkout lives inside the container unless pointed at a directory
+     * that outlives it.
+     *
+     * <p>Takes the uri rather than reading {@link #SITE_GIT_URL} so the clone can be exercised against a
+     * local repository instead of reaching gitbox.
+     */
+    static void cloneSite(String uri, String checkout) throws GitAPIException, IOException {
+        try (Git ignored = Git.cloneRepository()
+                .setURI(uri)
+                .setProgressMonitor(new TextProgressMonitor())
+                .setDirectory(new File(checkout))
+                .setCloneAllBranches(false)
+                .setBranchesToClone(List.of("refs/heads/" + SITE_BRANCH))
+                .setBranch(SITE_BRANCH)
+                .setDepth(1)
+                .call()) {
+            LOGGER.info("Cloned {} into {}.", uri, checkout);
+        }
+    }
+
     /**
      * Creates the directory holding the checkout, restricted to its owner where the filesystem supports it,
      * so nothing else can tamper with content that is about to be committed to the website.

src/test/java/org/apache/sling/cli/impl/release/SiteRepository.java

@@ -104,6 +104,11 @@ String checkout() {
         return checkout.toString();
     }
 
+    /** The upstream as a {@code file://} uri, so a clone from it honours a requested depth. */
+    String upstreamUri() {
+        return upstream.toUri().toString();
+    }
+
     Path checkoutPath() {
         return checkout;
     }

src/test/java/org/apache/sling/cli/impl/release/UpdateLocalSiteCommandTest.java

@@ -287,6 +287,49 @@ public void testAnUpToDateEntryDoesNotHideAMissingSibling() throws Exception {
                 logCapture.containsMessage("has no entry for Apache Sling API 2.20.0 ([org.apache.sling.api.absent])"));
     }
 
+    @Test
+    public void testRefreshingAFullCheckoutDoesNotMakeItShallow() throws Exception {
+        // SiteRepository clones in full, standing in for someone pointing --site-checkout at a normal
+        // clone: refreshing it must not truncate their history, which asking for depth 1 would do
+        try (Git git = Git.open(new File(site.checkout()))) {
+            assertTrue(
+                    "precondition: the fixture clone is not shallow",
+                    git.getRepository().getObjectDatabase().getShallowCommits().isEmpty());
+        }
+
+        UpdateLocalSiteCommand.ensureRepo(site.checkout());
+
+        try (Git git = Git.open(new File(site.checkout()))) {
+            assertTrue(
+                    "a full checkout must stay full",
+                    git.getRepository().getObjectDatabase().getShallowCommits().isEmpty());
+        }
+        assertFalse(
+                "no shallow marker may be written",
+                Files.exists(java.nio.file.Path.of(site.checkout(), ".git", "shallow")));
+    }
+
+    @Test
+    public void testAFreshCloneIsShallowAndSingleBranch() throws Exception {
+        // the clone the container makes on every run: only the tip of the published branch, so a few
+        // hundred MB of site history is not fetched to edit two files
+        java.nio.file.Path target = site.checkoutPath().getParent().resolve("fresh-clone");
+
+        UpdateLocalSiteCommand.cloneSite(site.upstreamUri(), target.toString());
+
+        try (Git git = Git.open(target.toFile())) {
+            assertFalse(
+                    "the clone must be shallow",
+                    git.getRepository().getObjectDatabase().getShallowCommits().isEmpty());
+            assertEquals(SiteRepository.BRANCH, git.getRepository().getBranch());
+            assertEquals(
+                    "only the published branch should be fetched",
+                    "+refs/heads/" + SiteRepository.BRANCH + ":refs/remotes/origin/" + SiteRepository.BRANCH,
+                    git.getRepository().getConfig().getString("remote", "origin", "fetch"));
+        }
+        assertTrue(Files.exists(target.resolve("src/main/jbake/content/releases.md")));
+    }
+
     private RepositoryService serviceResolving(String artifactId) throws IOException {
         RepositoryService repositoryService = mock(RepositoryService.class);
         when(repositoryService.getArtifactIdsFromPomUrls(any(), any(), any())).thenReturn(Set.of(artifactId));

src/test/java/org/apache/sling/cli/impl/release/UpdateLocalSiteGitTest.java

@@ -0,0 +1,164 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.sling.cli.impl.release;
+
+import java.io.File;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+
+import org.apache.sling.cli.impl.Credentials;
+import org.apache.sling.cli.impl.ExecutionMode;
+import org.apache.sling.cli.impl.people.Member;
+import org.eclipse.jgit.api.Git;
+import org.eclipse.jgit.lib.PersonIdent;
+import org.eclipse.jgit.revwalk.RevCommit;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+/**
+ * Exercises a <em>shallow</em> site checkout against a real JGit and real repositories.
+ *
+ * <p>{@link UpdateLocalSiteCommandTest} already drives the command against a real repository; what is
+ * specific here is that the checkout is shallow, so a JGit upgrade that breaks shallow fetches, or a
+ * server that refuses a push from a shallow clone, is caught here rather than during a release.
+ *
+ * <p>Everything happens between two local repositories, so no network access is involved.
+ */
+public class UpdateLocalSiteGitTest {
+
+    @Rule
+    public final TemporaryFolder tmp = new TemporaryFolder();
+
+    private Path upstream;
+    private Path checkout;
+
+    @Before
+    public void createRepositories() throws Exception {
+        upstream = tmp.newFolder("upstream.git").toPath();
+        Path seed = tmp.newFolder("seed").toPath();
+
+        try (Git bare = Git.init()
+                .setBare(true)
+                .setInitialBranch(UpdateLocalSiteCommand.SITE_BRANCH)
+                .setDirectory(upstream.toFile())
+                .call()) {
+            assertTrue(bare.getRepository().getDirectory().exists());
+        }
+
+        // seed the upstream with the two files the site update edits
+        try (Git git = Git.init()
+                .setInitialBranch(UpdateLocalSiteCommand.SITE_BRANCH)
+                .setDirectory(seed.toFile())
+                .call()) {
+            Path content = Files.createDirectories(seed.resolve("src/main/jbake/content"));
+            Files.writeString(content.resolve("releases.md"), "seeded\n", StandardCharsets.UTF_8);
+            git.add().addFilepattern("src").call();
+            git.commit()
+                    .setMessage("seed")
+                    .setAuthor("Seed", "seed@example.org")
+                    .setCommitter("Seed", "seed@example.org")
+                    .call();
+            git.remoteAdd()
+                    .setName("origin")
+                    .setUri(new org.eclipse.jgit.transport.URIish(
+                            upstream.toUri().toString()))
+                    .call();
+            git.push().call();
+        }
+
+        // a shallow, single-branch clone, matching what the command creates for the real site
+        checkout = tmp.newFolder("checkout").toPath();
+        try (Git cloned = Git.cloneRepository()
+                .setURI(upstream.toUri().toString())
+                .setDirectory(checkout.toFile())
+                .setCloneAllBranches(false)
+                .setBranch(UpdateLocalSiteCommand.SITE_BRANCH)
+                .setDepth(1)
+                .call()) {
+            assertEquals(
+                    UpdateLocalSiteCommand.SITE_BRANCH, cloned.getRepository().getBranch());
+        }
+    }
+
+    @Test
+    public void shallowCloneIsUsable() throws Exception {
+        try (Git git = Git.open(checkout.toFile())) {
+            assertFalse(
+                    "the clone should be shallow",
+                    git.getRepository().getObjectDatabase().getShallowCommits().isEmpty());
+        }
+    }
+
+    @Test
+    public void changesAreCommittedAndPushedWithTheReleaseManagerAsAuthorAndCommitter() throws Exception {
+        UpdateLocalSiteCommand.ensureRepo(checkout.toString());
+        Files.writeString(checkout.resolve("src/main/jbake/content/releases.md"), "released\n", StandardCharsets.UTF_8);
+
+        UpdateLocalSiteCommand.commitAndPushSiteChanges(
+                checkout.toString(),
+                "Released Apache Sling Foo 1.2.0",
+                "unused in AUTO",
+                ExecutionMode.AUTO,
+                new Credentials("johndoe", "secret"),
+                new Member("johndoe", "John Doe", true));
+
+        try (Git upstreamGit = Git.open(new File(upstream.toString()))) {
+            RevCommit head = upstreamGit.log().setMaxCount(1).call().iterator().next();
+            assertEquals("Released Apache Sling Foo 1.2.0", head.getFullMessage());
+
+            PersonIdent author = head.getAuthorIdent();
+            PersonIdent committer = head.getCommitterIdent();
+            assertEquals("John Doe", author.getName());
+            assertEquals("johndoe@apache.org", author.getEmailAddress());
+            // the container has no git identity, so the committer must be set explicitly too
+            assertEquals("John Doe", committer.getName());
+            assertEquals("johndoe@apache.org", committer.getEmailAddress());
+        }
+    }
+
+    @Test
+    public void dryRunDoesNotPush() throws Exception {
+        UpdateLocalSiteCommand.ensureRepo(checkout.toString());
+        Files.writeString(checkout.resolve("src/main/jbake/content/releases.md"), "released\n", StandardCharsets.UTF_8);
+        String upstreamHeadBefore = upstreamHead();
+
+        UpdateLocalSiteCommand.commitAndPushSiteChanges(
+                checkout.toString(),
+                "Released Apache Sling Foo 1.2.0",
+                "unused in DRY_RUN",
+                ExecutionMode.DRY_RUN,
+                new Credentials("johndoe", "secret"),
+                new Member("johndoe", "John Doe", true));
+
+        assertEquals("nothing may be pushed in a dry run", upstreamHeadBefore, upstreamHead());
+    }
+
+    private String upstreamHead() throws Exception {
+        try (Git git = Git.open(new File(upstream.toString()))) {
+            return git.log().setMaxCount(1).call().iterator().next().getName();
+        }
+    }
+}