Re: [PATCH JGIT] Compute the author/commiter name and email from the git configuration

From: Yann Simon <yann.simon.fr@gmail.com>
Date: Feb 4, 2009, 08:42 UTC
Message-ID: <551f769b0902040042j23238581ia248c949ee395ff7@mail.gmail.com>
In-Reply-To: <20090203231357.GZ26880@spearce.org>

2009/2/4 Shawn O. Pearce <spearce@spearce.org>:

Show 63 quoted lines

> Yann Simon <yann.simon.fr@gmail.com> wrote:
>> index 7df90cd..5821f83 100644
>> --- a/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java
>> +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java
>> @@ -50,6 +50,8 @@
>>  import java.io.InputStreamReader;
>>  import java.io.OutputStreamWriter;
>>  import java.io.PrintWriter;
>> +import java.net.InetAddress;
>> +import java.net.UnknownHostException;
>>  import java.util.ArrayList;
>>  import java.util.Collections;
>>  import java.util.HashMap;
>> @@ -98,6 +100,8 @@ public static RepositoryConfig openUserConfig() {
>>
>>       private Map<String, Object> byName;
>>
>> +     private String hostname;
>> +
>>       private static final String MAGIC_EMPTY_VALUE = "%%magic%%empty%%";
>>
>>       RepositoryConfig(final Repository repo) {
>> @@ -308,6 +312,83 @@ public String getString(final String section, String subsection, final String na
>>               return result;
>>       }
>>
>> +     /**
>> +      * @return the author name as defined in the git variables
>> +      *         and configurations. If no name could be found, try
>> +      *         to use the system user name instead.
>> +      */
>> +     public String getAuthorName() {
>> +             return getUsernameInternal(Constants.GIT_AUTHOR_NAME_KEY);
>> +     }
>> +
>> +     /**
>> +      * @return the commiter name as defined in the git variables
>> +      *         and configurations. If no name could be found, try
>> +      *         to use the system user name instead.
>> +      */
>> +     public String getCommiterName() {
>> +             return getUsernameInternal(Constants.GIT_COMMITER_NAME_KEY);
>> +     }
>> +
>> +     private String getUsernameInternal(String gitVariableKey) {
>> +             // try to get the user name from the local and global configurations.
>> +             String username = getString("user", null, "name");
>> +
>> +             if (username == null) {
>> +                     // try to get the user name for the system property GIT_XXX_NAME
>> +                     username = System.getProperty(gitVariableKey);
>
> Shouldn't that be System.getenv()?
>
>> +     private String getUserEmailInternal(String gitVariableKey, boolean author) {
>> +             // try to get the email from the local and global configs.
>> +             String email = getString("user", null, "email");
>> +
>> +             if (email == null) {
>> +                     // try to get the email for the system property GIT_XXX_EMAIL
>> +                     email = System.getProperty(gitVariableKey);
>
> Again, System.getenv()?

Just a precision: One of the consequences of using System.getenv() is that it does not let java programs to define the value of the variable. System.getProperty(), on the other hand, offers a System.setProperty()

If we use System.getenv(), we cannot overwrite the value before.

But we maybe do not need to overwrite system variables (except for unit test... :) )

In that case, we can live with System.getenv().

A workaround would be to use a method like:
	private static String readEnvironmentVariable(String key) {
		String result = System.getProperty(key);
		if (result == null || result.length() == 0) {
			result = System.getenv(key);
		}
		return result;
	}

but I really dislike it. This is much more complex and make the results less predictable.

I change my code to use System.getenv() unless you got other ideas.

Yann