Merge branch 'stable-2.6'

* stable-2.6:
  Sort library JARs by last modified time
  Defer loading site libraries until database opens
  Prevent account's full name from being set to empty string
  Update 2.6 release notes with mod_rewrite change
  Update 2.6 release notes with more trivial_rebase fixes
  Vertically align the "Choose:" header on the Become Any Account page
  Cannot Become Any Account for account whose full name is empty string
  init: Default database type to JDBC when url is present
  Remove old library JARs when upgrading

ReleaseNotes/ReleaseNotes-2.6.txt

@@ -427,11 +427,12 @@ responses are protected from accidential sniffing and treatment as
 HTML thanks to Gson encoding HTML control characters using Unicode
 character escapes within JSON strings.
 
-* Apache reverse proxies need `AllowEncodedSlashes NoDecode`
+* Apache reverse proxies must switch to mod_rewrite
 +
-When Apache is used as a reverse proxy the NoDecode option
-must be set for AllowEncodedSlashes to prevent Apache from
-mangling Gerrit REST API URLs.
+When Apache is used as a reverse proxy the server must be reconfigured
+to use mod_rewrite and AllowEncodedSlashes.  For updated information
+link:http://gerrit-documentation.googlecode.com/svn/Documentation/2.6/config-reverseproxy.html#_apache_2_configuration[
+review the Apache 2 Configuration documentation].
 
 Project Dashboards
 ~~~~~~~~~~~~~~~~~~
@@ -1517,6 +1518,15 @@ default time unit.
 ** Use change-url flag for ChangeId
 ** Prevent exception for empty commit
 ** Fix pylint errors
+** Call `gerrit review` instead of `gerrit approve`
+** Make the private key argument optional
+** Support alternative ssh executable, for example `plink`
+** Support custom review labels
+** Correctly handle empty patch ID
++
+If only one of the patch IDs is empty, it should not be considered
+a trivial rebase.
+
 ** Use plain python instead of python2.6
 +
 Windows installation only has python.exe

gerrit-httpd/src/main/java/com/google/gerrit/httpd/auth/become/BecomeAnyAccountLoginServlet.java

@@ -173,7 +173,7 @@ class BecomeAnyAccountLoginServlet extends HttpServlet {
         String displayName;
         if (a.getUserName() != null) {
           displayName = a.getUserName();
-        } else if (a.getFullName() != null) {
+        } else if (a.getFullName() != null && !a.getFullName().isEmpty()) {
           displayName = a.getFullName();
         } else if (a.getPreferredEmail() != null) {
           displayName = a.getPreferredEmail();

gerrit-httpd/src/main/resources/com/google/gerrit/httpd/auth/become/BecomeAnyAccount.html

@@ -68,7 +68,7 @@
         </tr>
 
         <tr>
-          <th>Choose:</th>
+          <th style="vertical-align: top;">Choose:</th>
           <td id="userlist"/>
         </tr>
       </table>

gerrit-pgm/src/main/java/com/google/gerrit/pgm/Init.java

@@ -21,7 +21,6 @@ import com.google.gerrit.common.PageLinks;
 import com.google.gerrit.pgm.init.Browser;
 import com.google.gerrit.pgm.init.InitFlags;
 import com.google.gerrit.pgm.init.InitModule;
-import com.google.gerrit.pgm.init.ReloadSiteLibrary;
 import com.google.gerrit.pgm.init.SitePathInitializer;
 import com.google.gerrit.pgm.util.ConsoleUI;
 import com.google.gerrit.pgm.util.Die;
@@ -121,12 +120,6 @@ public class Init extends SiteProgram {
       protected void configure() {
         bind(ConsoleUI.class).toInstance(ui);
         bind(File.class).annotatedWith(SitePath.class).toInstance(sitePath);
-        bind(ReloadSiteLibrary.class).toInstance(new ReloadSiteLibrary() {
-          @Override
-          public void reload() {
-            Init.super.loadSiteLib();
-          }
-        });
       }
     });
 

gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/InitDatabase.java

@@ -16,6 +16,8 @@ package com.google.gerrit.pgm.init;
 
 import static com.google.inject.Stage.PRODUCTION;
 
+import com.google.common.base.Strings;
+import com.google.common.collect.Sets;
 import com.google.gerrit.pgm.util.ConsoleUI;
 import com.google.gerrit.server.config.SitePaths;
 import com.google.inject.Binding;
@@ -53,7 +55,7 @@ class InitDatabase implements InitStep {
   public void run() {
     ui.header("SQL Database");
 
-    Set<String> allowedValues = new TreeSet<String>();
+    Set<String> allowedValues = Sets.newTreeSet();
     Injector i = Guice.createInjector(PRODUCTION, new DatabaseConfigModule(site));
     List<Binding<DatabaseConfigInitializer>> dbConfigBindings =
         i.findBindingsByType(new TypeLiteral<DatabaseConfigInitializer>() {});
@@ -64,6 +66,11 @@ class InitDatabase implements InitStep {
       }
     }
 
+    if (!Strings.isNullOrEmpty(database.get("url"))
+        && Strings.isNullOrEmpty(database.get("type"))) {
+      database.set("type", "jdbc");
+    }
+
     String dbType =
         database.select("Database server type", "type", "h2", allowedValues);
 

gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/Libraries.java

@@ -78,6 +78,7 @@ class Libraries {
     dl.setName(get(cfg, n, "name"));
     dl.setJarUrl(get(cfg, n, "url"));
     dl.setSHA1(get(cfg, n, "sha1"));
+    dl.setRemove(get(cfg, n, "remove"));
     field.set(this, dl);
   }
 

gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/LibraryDownloader.java

@@ -14,8 +14,10 @@
 
 package com.google.gerrit.pgm.init;
 
+import com.google.common.base.Strings;
 import com.google.gerrit.pgm.util.ConsoleUI;
 import com.google.gerrit.pgm.util.Die;
+import com.google.gerrit.pgm.util.IoUtil;
 import com.google.gerrit.server.config.SitePaths;
 import com.google.inject.Inject;
 
@@ -26,6 +28,7 @@ import java.io.File;
 import java.io.FileInputStream;
 import java.io.FileNotFoundException;
 import java.io.FileOutputStream;
+import java.io.FilenameFilter;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
@@ -40,20 +43,18 @@ import java.security.NoSuchAlgorithmException;
 class LibraryDownloader {
   private final ConsoleUI ui;
   private final File lib_dir;
-  private final ReloadSiteLibrary reload;
 
   private boolean required;
   private String name;
   private String jarUrl;
   private String sha1;
+  private String remove;
   private File dst;
 
   @Inject
-  LibraryDownloader(final ReloadSiteLibrary reload, final ConsoleUI ui,
-      final SitePaths site) {
+  LibraryDownloader(ConsoleUI ui, SitePaths site) {
     this.ui = ui;
     this.lib_dir = site.lib_dir;
-    this.reload = reload;
   }
 
   void setName(final String name) {
@@ -68,6 +69,10 @@ class LibraryDownloader {
     this.sha1 = sha1;
   }
 
+  void setRemove(String remove) {
+    this.remove = remove;
+  }
+
   void downloadRequired() {
     this.required = true;
     download();
@@ -123,6 +128,7 @@ class LibraryDownloader {
     }
 
     try {
+      removeStaleVersions();
       doGetByHttp();
       verifyFileChecksum();
     } catch (IOException err) {
@@ -155,7 +161,29 @@ class LibraryDownloader {
       }
     }
 
-    reload.reload();
+    if (dst.exists()) {
+      IoUtil.loadJARs(dst);
+    }
+  }
+
+  private void removeStaleVersions() {
+    if (!Strings.isNullOrEmpty(remove)) {
+      String[] names = lib_dir.list(new FilenameFilter() {
+        @Override
+        public boolean accept(File dir, String name) {
+          return name.matches("^" + remove + "$");
+        }
+      });
+      if (names != null) {
+        for (String old : names) {
+          String bak = "." + old + ".backup";
+          ui.message("Renaming %s to %s", old, bak);
+          if (!new File(lib_dir, old).renameTo(new File(lib_dir, bak))) {
+            throw new Die("cannot rename " + old);
+          }
+        }
+      }
+    }
   }
 
   private void doGetByHttp() throws IOException {

gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/ReloadSiteLibrary.java

@@ -1,20 +0,0 @@
-// Copyright (C) 2009 The Android Open Source Project
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.google.gerrit.pgm.init;
-
-/** Requests the site's {@code lib/} directory be scanned again. */
-public interface ReloadSiteLibrary {
-  public void reload();
-}

gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/IoUtil.java

@@ -14,9 +14,19 @@
 
 package com.google.gerrit.pgm.util;
 
+import com.google.common.collect.Sets;
+
+import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.net.MalformedURLException;
+import java.net.URL;
+import java.net.URLClassLoader;
+import java.util.Arrays;
+import java.util.Set;
 
 public final class IoUtil {
   public static void copyWithThread(final InputStream src,
@@ -42,6 +52,47 @@ public final class IoUtil {
     }.start();
   }
 
+  public static void loadJARs(File... jars) {
+    ClassLoader cl = IoUtil.class.getClassLoader();
+    if (!(cl instanceof URLClassLoader)) {
+      throw noAddURL("Not loaded by URLClassLoader", null);
+    }
+    @SuppressWarnings("resource")
+    URLClassLoader urlClassLoader = (URLClassLoader) cl;
+
+    Method addURL;
+    try {
+      addURL = URLClassLoader.class.getDeclaredMethod("addURL", URL.class);
+      addURL.setAccessible(true);
+    } catch (SecurityException e) {
+      throw noAddURL("Method addURL not available", e);
+    } catch (NoSuchMethodException e) {
+      throw noAddURL("Method addURL not available", e);
+    }
+
+    Set<URL> have = Sets.newHashSet(Arrays.asList(urlClassLoader.getURLs()));
+    for (File path : jars) {
+      try {
+        URL url = path.toURI().toURL();
+        if (have.add(url)) {
+          addURL.invoke(cl, url);
+        }
+      } catch (MalformedURLException e) {
+        throw noAddURL("addURL " + path + " failed", e);
+      } catch (IllegalArgumentException e) {
+        throw noAddURL("addURL " + path + " failed", e);
+      } catch (IllegalAccessException e) {
+        throw noAddURL("addURL " + path + " failed", e);
+      } catch (InvocationTargetException e) {
+        throw noAddURL("addURL " + path + " failed", e.getCause());
+      }
+    }
+  }
+
+  private static UnsupportedOperationException noAddURL(String m, Throwable why) {
+    String prefix = "Cannot extend classpath: ";
+    return new UnsupportedOperationException(prefix + m, why);
+  }
   private IoUtil() {
   }
 }

gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/SiteLibraryBasedDataSourceProvider.java

@@ -0,0 +1,81 @@
+// Copyright (C) 2013 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.pgm.util;
+
+import com.google.common.primitives.Longs;
+import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.gerrit.server.config.SitePaths;
+import com.google.gerrit.server.schema.DataSourceProvider;
+import com.google.gerrit.server.schema.DataSourceType;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+
+import org.eclipse.jgit.lib.Config;
+
+import java.io.File;
+import java.io.FileFilter;
+import java.util.Arrays;
+import java.util.Comparator;
+
+import javax.sql.DataSource;
+
+/** Loads the site library if not yet loaded. */
+@Singleton
+public class SiteLibraryBasedDataSourceProvider extends DataSourceProvider {
+  private final File libdir;
+  private boolean init;
+
+  @Inject
+  SiteLibraryBasedDataSourceProvider(SitePaths site,
+      @GerritServerConfig Config cfg,
+      DataSourceProvider.Context ctx,
+      DataSourceType dst) {
+    super(site, cfg, ctx, dst);
+    libdir = site.lib_dir;
+  }
+
+  public synchronized DataSource get() {
+    if (!init) {
+      loadSiteLib();
+      init = true;
+    }
+    return super.get();
+  }
+
+  private void loadSiteLib() {
+    File[] jars = libdir.listFiles(new FileFilter() {
+      @Override
+      public boolean accept(File path) {
+        String name = path.getName();
+        return (name.endsWith(".jar") || name.endsWith(".zip"))
+            && path.isFile();
+      }
+    });
+    if (jars != null && 0 < jars.length) {
+      Arrays.sort(jars, new Comparator<File>() {
+        @Override
+        public int compare(File a, File b) {
+          // Sort by reverse last-modified time so newer JARs are first.
+          int cmp = Longs.compare(b.lastModified(), a.lastModified());
+          if (cmp != 0) {
+            return cmp;
+          }
+          return a.getName().compareTo(b.getName());
+        }
+      });
+      IoUtil.loadJARs(jars);
+    }
+  }
+}

gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/SiteProgram.java

@@ -41,19 +41,9 @@ import org.eclipse.jgit.lib.Config;
 import org.kohsuke.args4j.Option;
 
 import java.io.File;
-import java.io.FileFilter;
-import java.lang.reflect.InvocationTargetException;
-import java.lang.reflect.Method;
-import java.net.MalformedURLException;
-import java.net.URL;
-import java.net.URLClassLoader;
 import java.sql.SQLException;
 import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Comparator;
-import java.util.HashSet;
 import java.util.List;
-import java.util.Set;
 
 import javax.sql.DataSource;
 
@@ -78,77 +68,8 @@ public abstract class SiteProgram extends AbstractProgram {
     }
   }
 
-  /** Load extra JARs from {@code lib/} subdirectory of {@link #getSitePath()} */
-  protected void loadSiteLib() {
-    final File libdir = new File(getSitePath(), "lib");
-    final File[] list = libdir.listFiles(new FileFilter() {
-      @Override
-      public boolean accept(File path) {
-        if (!path.isFile()) {
-          return false;
-        }
-        return path.getName().endsWith(".jar") //
-            || path.getName().endsWith(".zip");
-      }
-    });
-    if (list != null && 0 < list.length) {
-      Arrays.sort(list, new Comparator<File>() {
-        @Override
-        public int compare(File a, File b) {
-          return a.getName().compareTo(b.getName());
-        }
-      });
-      addToClassLoader(list);
-    }
-  }
-
-  private void addToClassLoader(final File[] additionalLocations) {
-    final ClassLoader cl = getClass().getClassLoader();
-    if (!(cl instanceof URLClassLoader)) {
-      throw noAddURL("Not loaded by URLClassLoader", null);
-    }
-
-    final URLClassLoader ucl = (URLClassLoader) cl;
-    final Set<URL> have = new HashSet<URL>();
-    have.addAll(Arrays.asList(ucl.getURLs()));
-
-    final Method m;
-    try {
-      m = URLClassLoader.class.getDeclaredMethod("addURL", URL.class);
-      m.setAccessible(true);
-    } catch (SecurityException e) {
-      throw noAddURL("Method addURL not available", e);
-    } catch (NoSuchMethodException e) {
-      throw noAddURL("Method addURL not available", e);
-    }
-
-    for (final File path : additionalLocations) {
-      try {
-        final URL url = path.toURI().toURL();
-        if (have.add(url)) {
-          m.invoke(cl, url);
-        }
-      } catch (MalformedURLException e) {
-        throw noAddURL("addURL " + path + " failed", e);
-      } catch (IllegalArgumentException e) {
-        throw noAddURL("addURL " + path + " failed", e);
-      } catch (IllegalAccessException e) {
-        throw noAddURL("addURL " + path + " failed", e);
-      } catch (InvocationTargetException e) {
-        throw noAddURL("addURL " + path + " failed", e.getCause());
-      }
-    }
-  }
-
-  private static UnsupportedOperationException noAddURL(String m, Throwable why) {
-    final String prefix = "Cannot extend classpath: ";
-    return new UnsupportedOperationException(prefix + m, why);
-  }
-
   /** @return provides database connectivity and site path. */
   protected Injector createDbInjector(final DataSourceProvider.Context context) {
-    loadSiteLib();
-
     final File sitePath = getSitePath();
     final List<Module> modules = new ArrayList<Module>();
 
@@ -164,9 +85,10 @@ public abstract class SiteProgram extends AbstractProgram {
       @Override
       protected void configure() {
         bind(DataSourceProvider.Context.class).toInstance(context);
-        bind(Key.get(DataSource.class, Names.named("ReviewDb"))).toProvider(
-            DataSourceProvider.class).in(SINGLETON);
-        listener().to(DataSourceProvider.class);
+        bind(Key.get(DataSource.class, Names.named("ReviewDb")))
+          .toProvider(SiteLibraryBasedDataSourceProvider.class)
+          .in(SINGLETON);
+        listener().to(SiteLibraryBasedDataSourceProvider.class);
       }
     });
     Module configModule = new GerritServerConfigModule();

gerrit-pgm/src/main/resources/com/google/gerrit/pgm/libraries.config

@@ -17,8 +17,10 @@
   name = Bouncy Castle Crypto v144
   url = http://www.bouncycastle.org/download/bcprov-jdk16-144.jar
   sha1 = 6327a5f7a3dc45e0fd735adb5d08c5a74c05c20c
+  remove = bcprov-.*[.]jar
 
 [library "mysqlDriver"]
   name = MySQL Connector/J 5.1.21
   url = http://repo2.maven.org/maven2/mysql/mysql-connector-java/5.1.21/mysql-connector-java-5.1.21.jar
   sha1 = 7abbd19fc2e2d5b92c0895af8520f7fa30266be9
+  remove = mysql-connector-java-.*[.]jar

gerrit-pgm/src/test/java/com/google/gerrit/pgm/init/LibrariesTest.java

@@ -30,16 +30,14 @@ import java.io.FileNotFoundException;
 public class LibrariesTest extends TestCase {
   public void testCreate() throws FileNotFoundException {
     final SitePaths site = new SitePaths(new File("."));
-    final ReloadSiteLibrary reload = createStrictMock(ReloadSiteLibrary.class);
     final ConsoleUI ui = createStrictMock(ConsoleUI.class);
 
     replay(ui);
-    replay(reload);
 
     Libraries lib = new Libraries(new Provider<LibraryDownloader>() {
       @Override
       public LibraryDownloader get() {
-        return new LibraryDownloader(reload, ui, site);
+        return new LibraryDownloader(ui, site);
       }
     });
 
@@ -47,6 +45,5 @@ public class LibrariesTest extends TestCase {
     assertNotNull(lib.mysqlDriver);
 
     verify(ui);
-    verify(reload);
   }
 }

gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/Account.java

@@ -169,7 +169,11 @@ public final class Account {
 
   /** Set the full name of the user ("Given-name Surname" style). */
   public void setFullName(final String name) {
-    fullName = name != null ? name.trim() : null;
+    if (name != null && !name.trim().isEmpty()) {
+      fullName = name.trim();
+    } else {
+      fullName = null;
+    }
   }
 
   /** Email address the user prefers to be contacted through. */

gerrit-server/src/main/java/com/google/gerrit/server/schema/DataSourceProvider.java

@@ -39,18 +39,30 @@ import javax.sql.DataSource;
 
 /** Provides access to the DataSource. */
 @Singleton
-public final class DataSourceProvider implements Provider<DataSource>,
+public class DataSourceProvider implements Provider<DataSource>,
     LifecycleListener {
-  private final DataSource ds;
+  private final SitePaths site;
+  private final Config cfg;
+  private final Context ctx;
+  private final DataSourceType dst;
+  private DataSource ds;
 
   @Inject
-  DataSourceProvider(final SitePaths site,
-      @GerritServerConfig final Config cfg, Context ctx, DataSourceType dst) {
-    ds = open(site, cfg, ctx, dst);
+  protected DataSourceProvider(SitePaths site,
+      @GerritServerConfig Config cfg,
+      Context ctx,
+      DataSourceType dst) {
+    this.site = site;
+    this.cfg = cfg;
+    this.ctx = ctx;
+    this.dst = dst;
   }
 
   @Override
   public synchronized DataSource get() {
+    if (ds == null) {
+      ds = open(site, cfg, ctx, dst);
+    }
     return ds;
   }