From 456cdadd2e8bdcab33bf8e1c3df7ab50375180c2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Kato=20St=C3=B8len?= <kato.stoelen@gmail.com>
Date: Fri, 27 Nov 2020 13:17:34 +0100
Subject: [PATCH] Add option to load tag targets before disposing the
 repository

When using GitTags, the libgit2sharp repository is disposed when the
tags are returned. Hence, accessing a tag's target properties, which are
lazily loaded, throws an exception.

Added an optional parameter (loadTargets) to GitTags. When set to true,
the target of all the tags are loaded before the repository is disposed,
enabling access to those. The loadTargets parameter defaults to false,
to make this an opt-in feature.

Fixes #72
---
 src/Cake.Git/GitAliases.Tags.cs | 48 +++++++++++++++++++++++++++++++--
 test.cake                       | 16 +++++++++++
 2 files changed, 62 insertions(+), 2 deletions(-)

diff --git a/src/Cake.Git/GitAliases.Tags.cs b/src/Cake.Git/GitAliases.Tags.cs
index 8d9be67..0681723 100644
--- a/src/Cake.Git/GitAliases.Tags.cs
+++ b/src/Cake.Git/GitAliases.Tags.cs
@@ -12,8 +12,12 @@ namespace Cake.Git
     partial class GitAliases
     {
         /// <summary>
-        /// Gets a list of all tags from the repo
+        /// Gets a list of all tags from the repository.
         /// </summary>
+        /// <remarks>
+        /// If you need to access the targets of the tags use <see cref="GitTags(ICakeContext, DirectoryPath, bool)"/>
+        /// to make sure targerts are loaded.
+        /// </remarks>
         /// <param name="context"></param>
         /// <param name="repositoryDirectoryPath"></param>
         /// <exception cref="ArgumentNullException"></exception>
@@ -29,8 +33,48 @@ public static List<Tag> GitTags(
             if (repositoryDirectoryPath == null)
                 throw new ArgumentNullException(nameof(repositoryDirectoryPath));
 
+            return context.GitTags(repositoryDirectoryPath, false);
+        }
+
+        /// <summary>
+        /// Gets a list of all tags from the repository with the option to load targets of the tags.
+        /// </summary>
+        /// <remarks>
+        /// If you need to access the targets of the tags, set <paramref name="loadTargets"/> to <see langword="true"/>.
+        /// This will make sure that the targets are loaded before the <see cref="Repository"/> is disposed.
+        /// Otherwise, accessing a tag's target will throw an exception.
+        /// </remarks>
+        /// <param name="context"></param>
+        /// <param name="repositoryDirectoryPath"></param>
+        /// <param name="loadTargets">A value indicating whether targets of the tags should be loaded.</param>
+        /// <exception cref="ArgumentNullException"></exception>
+        [CakeMethodAlias]
+        [CakeAliasCategory("Tags")]
+        public static List<Tag> GitTags(
+            this ICakeContext context,
+            DirectoryPath repositoryDirectoryPath,
+            bool loadTargets)
+        {
+            if (context == null)
+                throw new ArgumentNullException(nameof(context));
+
+            if (repositoryDirectoryPath == null)
+                throw new ArgumentNullException(nameof(repositoryDirectoryPath));
+
             var retval =
-                context.UseRepository(repositoryDirectoryPath, repo => SortedTags(repo.Tags, t => t));
+                context.UseRepository(
+                    repositoryDirectoryPath,
+                    repo => SortedTags(
+                        repo.Tags,
+                        t =>
+                        {
+                            if (loadTargets)
+                            {
+                                _ = t.PeeledTarget;
+                            }
+
+                            return t;
+                        }));
 
             return retval;
         }
diff --git a/test.cake b/test.cake
index 3047679..273b529 100644
--- a/test.cake
+++ b/test.cake
@@ -527,6 +527,20 @@ Task("Git-AllTags-Annotated")
         throw new Exception("test-annotated-tag-objectish not found");
 });
 
+Task("Git-AllTags-Targets")
+    .IsDependentOn("Git-Tag-Annotated")
+    .Does(() =>
+{
+    var tags = GitTags(testInitalRepo, loadTargets: true);
+
+    foreach (var tag in tags)
+    {
+        // When loadTargets is true, this should not throw an exception
+        _ = tag.Target;
+        _ = tag.PeeledTarget;
+    }
+});
+
 Task("Git-Describe-Generic")
     .IsDependentOn("Git-Tag")
     .Does(() =>
@@ -909,6 +923,7 @@ Task("Default-Tests")
     .IsDependentOn("Git-Checkout")
     .IsDependentOn("Git-AllTags")
     .IsDependentOn("Git-AllTags-Annotated")
+    .IsDependentOn("Git-AllTags-Targets")
     .IsDependentOn("Git-Clean");
 
 Task("Local-Tests")
@@ -944,6 +959,7 @@ Task("Local-Tests")
     .IsDependentOn("Git-Checkout")
     .IsDependentOn("Git-AllTags")
     .IsDependentOn("Git-AllTags-Annotated")
+    .IsDependentOn("Git-AllTags-Targets")
     .IsDependentOn("Git-Clean");
 
 ///////////////////////////////////////////////////////////////////////////////