Adding Add<T> and AddRecursively<T> to PackBuilder. Adding some testing for them. Several edits and enhancements.

Author: AMSadek

LibGit2Sharp.Tests/PackBuilderFixture.cs

@@ -1,26 +1,32 @@
-using LibGit2Sharp.Tests.TestHelpers;
-using System;
+using System;
 using System.IO;
 using System.Linq;
+using LibGit2Sharp.Tests.TestHelpers;
 using Xunit;
 
 namespace LibGit2Sharp.Tests
 {
     public class PackBuilderFixture : BaseFixture
     {
         [Fact]
-        public void TestDefaultBuildDelegate()
+        public void TestDefaultPackDelegate()
         {
             TestIfSameRepoAfterPacking(null);
         }
 
         [Fact]
-        public void TestCommitsPerBranchBuildDelegate()
+        public void TestCommitsPerBranchPackDelegate()
         {
-            TestIfSameRepoAfterPacking(TestBuildDelegate);
+            TestIfSameRepoAfterPacking(AddingObjectIdsTestDelegate);
         }
 
-        internal void TestIfSameRepoAfterPacking(Action<PackBuilder> buildingDelegate)
+        [Fact]
+        public void TestCommitsPerBranchIdsPackDelegate()
+        {
+            TestIfSameRepoAfterPacking(AddingObjectsTestDelegate);
+        }
+
+        internal void TestIfSameRepoAfterPacking(Action<IRepository, PackBuilder> packDelegate)
         {
             // read a repo
             // pack with the provided action
@@ -29,22 +35,19 @@ internal void TestIfSameRepoAfterPacking(Action<PackBuilder> buildingDelegate)
             // compare
 
             string orgRepoPath = SandboxPackBuilderTestRepo();
-            SelfCleaningDirectory scd = BuildSelfCleaningDirectory(orgRepoPath + "_mrr");
-            string mrrRepoPath = scd.DirectoryPath;
-            string mrrRepoPathPackDirPath = mrrRepoPath + "/.git/objects";
+            string mrrRepoPath = SandboxPackBuilderTestRepo();
+            string mrrRepoPackDirPath = Path.Combine(mrrRepoPath + "/.git/objects");
 
-            DirectoryHelper.CopyFilesRecursively(new DirectoryInfo(orgRepoPath), new DirectoryInfo(mrrRepoPath));
-            DirectoryHelper.DeleteDirectory(mrrRepoPathPackDirPath);
-            Directory.CreateDirectory(mrrRepoPathPackDirPath);
-            Directory.CreateDirectory(mrrRepoPathPackDirPath + "/pack");
+            DirectoryHelper.DeleteDirectory(mrrRepoPackDirPath);
+            Directory.CreateDirectory(mrrRepoPackDirPath + "/pack");
 
-            PackBuilderOptions packBuilderOptions = new PackBuilderOptions(mrrRepoPathPackDirPath + "/pack");
+            PackBuilderOptions packBuilderOptions = new PackBuilderOptions(mrrRepoPackDirPath + "/pack");
 
             using (Repository orgRepo = new Repository(orgRepoPath))
             {
                 PackBuilderResults results;
-                if (buildingDelegate != null)
-                    results = orgRepo.ObjectDatabase.Pack(packBuilderOptions, buildingDelegate);
+                if (packDelegate != null)
+                    results = orgRepo.ObjectDatabase.Pack(packBuilderOptions, b => packDelegate(orgRepo, b));
                 else
                     results = orgRepo.ObjectDatabase.Pack(packBuilderOptions);
 
@@ -55,7 +58,7 @@ internal void TestIfSameRepoAfterPacking(Action<PackBuilder> buildingDelegate)
                 using (Repository mrrRepo = new Repository(mrrRepoPath))
                 {
                     // make sure the objects of the original repo are the same as the ones in the mirror repo
-                    // doing that by making sure the count is the same, and the set differnce is empty
+                    // doing that by making sure the count is the same, and the set difference is empty
                     Assert.True(mrrRepo.ObjectDatabase.Count() == orgRepo.ObjectDatabase.Count() && !mrrRepo.ObjectDatabase.Except(orgRepo.ObjectDatabase).Any());
 
                     Assert.Equal(orgRepo.Commits.Count(), mrrRepo.Commits.Count());
@@ -65,15 +68,36 @@ internal void TestIfSameRepoAfterPacking(Action<PackBuilder> buildingDelegate)
             }
         }
 
-        internal void TestBuildDelegate(PackBuilder builder)
+        internal void AddingObjectIdsTestDelegate(IRepository repo, PackBuilder builder)
         {
-            foreach (Branch branch in builder.Repository.Branches)
+            foreach (Branch branch in repo.Branches)
             {
                 foreach (Commit commit in branch.Commits)
                 {
                     builder.AddRecursively(commit.Id);
                 }
             }
+
+            foreach (Tag tag in repo.Tags)
+            {
+                builder.Add(tag.Target.Id);
+            }
+        }
+
+        internal void AddingObjectsTestDelegate(IRepository repo, PackBuilder builder)
+        {
+            foreach (Branch branch in repo.Branches)
+            {
+                foreach (Commit commit in branch.Commits)
+                {
+                    builder.AddRecursively(commit);
+                }
+            }
+
+            foreach (Tag tag in repo.Tags)
+            {
+                builder.Add(tag.Target);
+            }
         }
 
         [Fact]
@@ -149,7 +173,7 @@ public void ExceptionIfAddNullObjectID()
             {
                 Assert.Throws<ArgumentNullException>(() =>
                 {
-                    orgRepo.ObjectDatabase.Pack(packBuilderOptions, (PackBuilder builder) =>
+                    orgRepo.ObjectDatabase.Pack(packBuilderOptions, builder =>
                     {
                         builder.Add(null);
                     });
@@ -167,7 +191,7 @@ public void ExceptionIfAddRecursivelyNullObjectID()
             {
                 Assert.Throws<ArgumentNullException>(() =>
                 {
-                    orgRepo.ObjectDatabase.Pack(packBuilderOptions, (PackBuilder builder) =>
+                    orgRepo.ObjectDatabase.Pack(packBuilderOptions, builder =>
                     {
                         builder.AddRecursively(null);
                     });

LibGit2Sharp/ObjectDatabase.cs

@@ -645,44 +645,45 @@ public virtual MergeTreeResult MergeCommits(Commit ours, Commit theirs, MergeTre
         }
 
         /// <summary>
-        /// Pack the objects in the <see cref="ObjectDatabase"/> and write a pack (.pack) and index (.idx) files for it.
+        /// Packs all the objects in the <see cref="ObjectDatabase"/> and write a pack (.pack) and index (.idx) files for them.
         /// </summary>
         /// <param name="options">Packing options</param>
         /// This method will invoke the default action of packing all objects in an arbitrary order.
         /// <returns>Packing results</returns>
         public virtual PackBuilderResults Pack(PackBuilderOptions options)
         {
-            return InternalPack(options, (PackBuilder builder) =>
+            return InternalPack(options, builder =>
             {
-                foreach (GitObject obj in this)
+                foreach (GitObject obj in repo.ObjectDatabase)
                 {
                     builder.Add(obj.Id);
                 }
             });
         }
 
         /// <summary>
-        /// Pack the objects in the <see cref="ObjectDatabase"/> and write a pack (.pack) and index (.idx) files for it.
+        /// Packs objects in the <see cref="ObjectDatabase"/> chosen by the packDelegate action
+        /// and write a pack (.pack) and index (.idx) files for them
         /// </summary>
         /// <param name="options">Packing options</param>
-        /// <param name="buildDelegate">Packing action</param>
+        /// <param name="packDelegate">Packing action</param>
         /// <returns>Packing results</returns>
-        public virtual PackBuilderResults Pack(PackBuilderOptions options, Action<PackBuilder> buildDelegate)
+        public virtual PackBuilderResults Pack(PackBuilderOptions options, Action<PackBuilder> packDelegate)
         {
-            return InternalPack(options, buildDelegate);
+            return InternalPack(options, packDelegate);
         }
 
         /// <summary>
-        /// Pack the objects in the <see cref="ObjectDatabase"/> and write a pack (.pack) and index (.idx) files for it. For internal use only.
+        /// Packs objects in the <see cref="ObjectDatabase"/> and write a pack (.pack) and index (.idx) files for them.
+        /// For internal use only.
         /// </summary>
         /// <param name="options">Packing options</param>
-        /// <param name="buildDelegate">Packing action. If null is passed, the pack method will invoke the default action of packing
-        /// all objects in an arbitrary order.</param>
+        /// <param name="packDelegate">Packing action</param>
         /// <returns>Packing results</returns>
-        private PackBuilderResults InternalPack(PackBuilderOptions options, Action<PackBuilder> buildDelegate)
+        private PackBuilderResults InternalPack(PackBuilderOptions options, Action<PackBuilder> packDelegate)
         {
             Ensure.ArgumentNotNull(options, "options");
-            Ensure.ArgumentNotNull(buildDelegate, "buildDelegate");
+            Ensure.ArgumentNotNull(packDelegate, "packDelegate");
 
             PackBuilderResults results = new PackBuilderResults();
 
@@ -692,7 +693,7 @@ private PackBuilderResults InternalPack(PackBuilderOptions options, Action<PackB
                 builder.SetMaximumNumberOfThreads(options.MaximumNumberOfThreads);
 
                 // call the provided action
-                buildDelegate(builder);
+                packDelegate(builder);
 
                 // writing the pack and index files
                 builder.Write(options.PackDirectoryPath);

LibGit2Sharp/PackBuilder.cs

@@ -6,15 +6,15 @@
 namespace LibGit2Sharp
 {
     /// <summary>
-    /// Representation of a git packbuilder.
+    /// Representation of a git PackBuilder.
     /// </summary>
     public sealed class PackBuilder : IDisposable
     {
         private readonly PackBuilderSafeHandle packBuilderHandle;
         private readonly Repository repo;
 
         /// <summary>
-        /// Constructs a packbuilder from a repository.
+        /// Constructs a PackBuilder for a <see cref="Repository"/>.
         /// </summary>
         internal PackBuilder(Repository repository)
         {
@@ -25,81 +25,102 @@ internal PackBuilder(Repository repository)
         }
 
         /// <summary>
-        /// Inserts a single object to the packbuilder.
+        /// Inserts a single <see cref="GitObject"/> to the PackBuilder.
         /// For an optimal pack it's mandatory to insert objects in recency order, commits followed by trees and blobs. (quoted from libgit2 API ref)
         /// </summary>
-        /// <param name="gitObjectID">The object ID to be inserted.</param>
-        public void Add(ObjectId gitObjectID)
+        /// <param name="gitObject">The object to be inserted.</param>
+        /// <exception cref="ArgumentNullException">if the gitObject is null</exception>
+        public void Add<T>(T gitObject) where T : GitObject
         {
-            Ensure.ArgumentNotNull(gitObjectID, "gitObject");
+            Ensure.ArgumentNotNull(gitObject, "gitObject");
 
-            Proxy.git_packbuilder_insert(packBuilderHandle, gitObjectID, null);
+            Add(gitObject.Id);
         }
 
         /// <summary>
-        /// Recursively insert an object and its referenced objects. Inserts the object as well as any object it references.
+        /// Recursively inserts a <see cref="GitObject"/> and its referenced objects.
+        /// Inserts the object as well as any object it references.
         /// </summary>
-        /// <param name="gitObjectID">The object ID to be recursively inserted.</param>
-        public void AddRecursively(ObjectId gitObjectID)
+        /// <param name="gitObject">The object to be inserted recursively.</param>
+        /// <exception cref="ArgumentNullException">if the gitObject is null</exception>
+        public void AddRecursively<T>(T gitObject) where T : GitObject
         {
-            Ensure.ArgumentNotNull(gitObjectID, "gitObject");
+            Ensure.ArgumentNotNull(gitObject, "gitObject");
 
-            Proxy.git_packbuilder_insert_recur(packBuilderHandle, gitObjectID, null);
+            AddRecursively(gitObject.Id);
         }
 
         /// <summary>
-        /// Writes the new pack file and corresponding index file to path.
+        /// Inserts a single object to the PackBuilder by its <see cref="ObjectId"/>.
+        /// For an optimal pack it's mandatory to insert objects in recency order, commits followed by trees and blobs. (quoted from libgit2 API ref)
         /// </summary>
-        /// <param name="path">The path that pack and index files will be written to it.</param>
-        internal void Write(string path)
+        /// <param name="id">The object ID to be inserted.</param>
+        /// <exception cref="ArgumentNullException">if the id is null</exception>
+        public void Add(ObjectId id)
         {
-            Proxy.git_packbuilder_write(packBuilderHandle, path);
+            Ensure.ArgumentNotNull(id, "id");
+
+            Proxy.git_packbuilder_insert(packBuilderHandle, id, null);
         }
 
         /// <summary>
-        /// Disposes the packbuilder object.
+        /// Recursively inserts an object and its referenced objects by its <see cref="ObjectId"/>.
+        /// Inserts the object as well as any object it references.
+        /// </summary>
+        /// <param name="id">The object ID to be recursively inserted.</param>
+        /// <exception cref="ArgumentNullException">if the id is null</exception>
+        public void AddRecursively(ObjectId id)
+        {
+            Ensure.ArgumentNotNull(id, "id");
+
+            Proxy.git_packbuilder_insert_recur(packBuilderHandle, id, null);
+        }
+
+        /// <summary>
+        /// Disposes the PackBuilder object.
         /// </summary>
         public void Dispose()
         {
             packBuilderHandle.SafeDispose();
         }
 
+        /// <summary>
+        /// Writes the pack file and corresponding index file to path.
+        /// </summary>
+        /// <param name="path">The path that pack and index files will be written to it.</param>
+        internal void Write(string path)
+        {
+            Proxy.git_packbuilder_write(packBuilderHandle, path);
+        }
+
         /// <summary>
         /// Sets number of threads to spawn.
         /// </summary>
         /// <returns> Returns the number of actual threads to be used.</returns>
-        /// <param name="nThread">The Number of threads to spawn. An argument of 0 enusures using all available CPUs</param>
+        /// <param name="nThread">The Number of threads to spawn. An argument of 0 ensures using all available CPUs</param>
         internal int SetMaximumNumberOfThreads(int nThread)
         {
             // Libgit2 set the number of threads to 1 by default, 0 ensures git_online_cpus
             return (int)Proxy.git_packbuilder_set_threads(packBuilderHandle, (uint)nThread);
         }
 
         /// <summary>
-        /// Get the total number of objects the packbuilder will write out.
+        /// Number of objects the PackBuilder will write out.
         /// </summary>
         internal long ObjectsCount
         {
             get { return Proxy.git_packbuilder_object_count(packBuilderHandle); }
         }
 
         /// <summary>
-        /// Number of objects the packbuilder has already written out. 
-        /// This is only correct after the packfile has been written.
+        /// Number of objects the PackBuilder has already written out. 
+        /// This is only correct after the pack file has been written.
         /// </summary>
         internal long WrittenObjectsCount
         {
             get { return Proxy.git_packbuilder_written(packBuilderHandle); }
         }
 
-        /// <summary>
-        /// A refernce to the repository that the packbuilder is working on.
-        /// </summary>
-        public Repository Repository
-        {
-            get { return repo; }
-        }
-
         internal PackBuilderSafeHandle Handle
         {
             get { return packBuilderHandle; }
@@ -112,7 +133,7 @@ internal PackBuilderSafeHandle Handle
     public struct PackBuilderResults
     {
         /// <summary>
-        /// Number of objects the packbuilder has already written out. 
+        /// Number of objects the PackBuilder has already written out. 
         /// </summary>
         public long WrittenObjectsCount { get; internal set; }
     }
@@ -129,7 +150,7 @@ public sealed class PackBuilderOptions
         /// The default value for maximum number of threads to spawn is 0 which ensures using all available CPUs.
         /// <exception cref="ArgumentNullException">if packDirectory is null or empty</exception>
         /// <exception cref="DirectoryNotFoundException">if packDirectory doesn't exist</exception>
-        public PackBuilderOptions (string packDirectory)
+        public PackBuilderOptions(string packDirectory)
         {
             PackDirectoryPath = packDirectory;
             MaximumNumberOfThreads = 0;
@@ -176,7 +197,7 @@ public int MaximumNumberOfThreads
             {
                 return nThreads;
             }
-            
+
         }
 
         private string path;