Don't build a fake signature

When there is no user information in the configuration files, BuildSignature() sets
"unkown" as the name and uses the environment variables to create the
e-mail. This seems peculiar, since it means a consumer of the library
cannot use this method to figure out if this configuration does exist
but has to do it themselves.

As far as I can tell, this started as a way to provide something to the
reflog when the user has not set up the information as it's more
transient data and progress matters more than accuracy in that
case. This is now handled by libgit2 itself, so there is no need for
this behaviour inside the library.

This leaves us with the curious case of this fake signature only being
provided to a consumer of the library when this behaviour was due to
some internal users. It also makes it harder than it needs to be to know
if the signature the library provided is accurate.

Resolve this situation by returning null when there is no signature
configured. The internal users can then move to use a method which
throws a message mentioning the lack of a necessary signature.

3 files changed, 20 insertions, 45 deletions

diff --git a/LibGit2Sharp/Configuration.cs b/LibGit2Sharp/Configuration.cs
index 69133fc9..3cce3610 100644
--- a/LibGit2Sharp/Configuration.cs
+++ b/LibGit2Sharp/Configuration.cs
@@ -743,12 +743,8 @@ namespace LibGit2Sharp
         }
 
         /// <summary>
-        /// Builds a <see cref="Signature"/> based on current configuration.
-        /// <para>
-        ///    Name is populated from the user.name setting, and is "unknown" if unspecified.
-        ///    Email is populated from the user.email setting, and is built from
-        ///    <see cref="Environment.UserName"/> and <see cref="Environment.UserDomainName"/> if unspecified.
-        /// </para>
+        /// Builds a <see cref="Signature"/> based on current configuration. If it is not found or
+        /// some configuration is missing, <code>null</code> is returned.
         /// <para>
         ///    The same escalation logic than in git.git will be used when looking for the key in the config files:
         ///       - local: the Git file in the current repository
@@ -758,51 +754,30 @@ namespace LibGit2Sharp
         /// </para>
         /// </summary>
         /// <param name="now">The timestamp to use for the <see cref="Signature"/>.</param>
-        /// <returns>The signature.</returns>
+        /// <returns>The signature or null if no user identity can be found in the configuration.</returns>
         public virtual Signature BuildSignature(DateTimeOffset now)
         {
-            return BuildSignature(now, false);
-        }
-
-        internal Signature BuildSignature(DateTimeOffset now, bool shouldThrowIfNotFound)
-        {
-            const string userNameKey = "user.name";
-            var name = this.GetValueOrDefault<string>(userNameKey);
-            var normalizedName = NormalizeUserSetting(shouldThrowIfNotFound,
-                                                      userNameKey,
-                                                      name,
-                                                      () => "unknown");
-
-            const string userEmailKey = "user.email";
-            var email = this.GetValueOrDefault<string>(userEmailKey);
-            var normalizedEmail = NormalizeUserSetting(shouldThrowIfNotFound,
-                                                       userEmailKey,
-                                                       email,
-                                                       () => string.Format(CultureInfo.InvariantCulture,
-                                                                           "{0}@{1}",
-                                                                           Environment.UserName,
-                                                                           Environment.UserDomainName));
+            var name = this.GetValueOrDefault<string>("user.name");
+            var email = this.GetValueOrDefault<string>("user.email");
 
-            return new Signature(normalizedName, normalizedEmail, now);
-        }
-
-        private string NormalizeUserSetting(bool shouldThrowIfNotFound, string entryName, string currentValue, Func<string> defaultValue)
-        {
-            if (!string.IsNullOrEmpty(currentValue))
+            if (string.IsNullOrEmpty(name) || string.IsNullOrEmpty(email))
             {
-                return currentValue;
+                return null;
             }
 
-            string message = string.Format("Configuration value '{0}' is missing or invalid.", entryName);
+            return new Signature(name, email, now);
+        }
 
-            if (shouldThrowIfNotFound)
+        internal Signature BuildSignatureOrThrow(DateTimeOffset now)
+        {
+            var signature = BuildSignature(now);
+            if (signature == null)
             {
-                throw new LibGit2SharpException(message);
+                throw new LibGit2SharpException("This overload requires 'user.name' and 'user.email' to be set. " +
+                                                "Use a different overload or set those variables in the configuation");
             }
 
-            Log.Write(LogLevel.Warning, message);
-
-            return defaultValue();
+            return signature;
         }
 
         private ConfigurationSafeHandle Snapshot()
diff --git a/LibGit2Sharp/NoteCollection.cs b/LibGit2Sharp/NoteCollection.cs
index 7d8c756d..c722221c 100644
--- a/LibGit2Sharp/NoteCollection.cs
+++ b/LibGit2Sharp/NoteCollection.cs
@@ -173,7 +173,7 @@ namespace LibGit2Sharp
         /// <returns>The note which was just saved.</returns>
         public virtual Note Add(ObjectId targetId, string message, string @namespace)
         {
-            Signature author = repo.Config.BuildSignature(DateTimeOffset.Now, true);
+            Signature author = repo.Config.BuildSignatureOrThrow(DateTimeOffset.Now);
 
             return Add(targetId, message, author, author, @namespace);
         }
@@ -212,7 +212,7 @@ namespace LibGit2Sharp
         /// <param name="namespace">The namespace on which the note will be removed. It can be either a canonical namespace or an abbreviated namespace ('refs/notes/myNamespace' or just 'myNamespace').</param>
         public virtual void Remove(ObjectId targetId, string @namespace)
         {
-            Signature author = repo.Config.BuildSignature(DateTimeOffset.Now, true);
+            Signature author = repo.Config.BuildSignatureOrThrow(DateTimeOffset.Now);
 
             Remove(targetId, author, author, @namespace);
         }
diff --git a/LibGit2Sharp/RepositoryExtensions.cs b/LibGit2Sharp/RepositoryExtensions.cs
index 9c644667..f0c88ae2 100644
--- a/LibGit2Sharp/RepositoryExtensions.cs
+++ b/LibGit2Sharp/RepositoryExtensions.cs
@@ -236,7 +236,7 @@ namespace LibGit2Sharp
         /// <returns>The generated <see cref="LibGit2Sharp.Commit"/>.</returns>
         public static Commit Commit(this IRepository repository, string message, CommitOptions options)
         {
-            Signature author = repository.Config.BuildSignature(DateTimeOffset.Now, true);
+            Signature author = repository.Config.BuildSignatureOrThrow(DateTimeOffset.Now);
 
             return repository.Commit(message, author, options);
         }
@@ -270,7 +270,7 @@ namespace LibGit2Sharp
         /// <returns>The generated <see cref="LibGit2Sharp.Commit"/>.</returns>
         public static Commit Commit(this IRepository repository, string message, Signature author, CommitOptions options)
         {
-            Signature committer = repository.Config.BuildSignature(DateTimeOffset.Now, true);
+            Signature committer = repository.Config.BuildSignatureOrThrow(DateTimeOffset.Now);
 
             return repository.Commit(message, author, committer, options);
         }