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

[Yann Simon <yann.simon.fr@gmail.com>]

2009/2/4 Shawn O. Pearce <spearce@spearce.org>:
> 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()?
>
>> +     public String getHostname() {
>> +             if (hostname == null) {
>> +                     InetAddress localMachine;
>> +                     try {
>> +                             localMachine = InetAddress.getLocalHost();
>> +                             hostname = localMachine.getHostName();
>> +                     } catch (UnknownHostException e) {
>> +                             // we do nothing
>> +                     }
>> +             }
>> +             return hostname;
>
> Do we want getHostName() or getCanonicalHostName() here?
>
> I think we'd want getCanonicalHostName().

Yes, indeed. I change this.

>
> Should we be caching this at the RepositoryConfig level, or at the
> whole JVM level (in a static).  If the application is long-running
> its likely to keep the same RepositoryConfig instance around for
> the life of that JVM, so we'd only make this request once.  Thus any
> change in hostname while the application is running would probably
> not take effect until after restart.  But any long running app is
> also likely to access more than one Repository, and thus more than
> one RepositoryConfig, so they should at least use consistent names,
> even if the underlying hostname has changed.
>
> IMHO, just cache it in a static on first demand.

OK, that make sense.
Should the call be thread-safe if it can be called from different instances?

Yann