Merge pull request #267 from jglick/s2m

[JENKINS-48543] Remove agent-to-controller calls

src/main/java/hudson/scm/CompareAgainstBaselineCallable.java

@@ -32,6 +32,8 @@ final class CompareAgainstBaselineCallable extends MasterToSlaveCallable<Polling
     private final ISVNAuthenticationProvider defaultAuthProvider;
     private final Map<String, ISVNAuthenticationProvider> authProviders;
     private final String nodeName;
+    private final boolean storeAuthToDisk = SubversionSCM.descriptor().isStoreAuthToDisk();
+    private final int workspaceFormat = SubversionSCM.descriptor().getWorkspaceFormat();
     private static final long serialVersionUID = 8200959096894789583L;
 
     CompareAgainstBaselineCallable(SVNRevisionState baseline, SVNLogHandler logHandler, String projectName,
@@ -112,7 +114,7 @@ static class ChangeState{
     private ChangeState checkInternal(String url,ISVNAuthenticationProvider authProvider, long baseRev, Map<String,Long> revs) throws SVNException {
         ChangeState changes = new ChangeState();
         final SVNURL svnurl = SVNURL.parseURIDecoded(url);
-        long nowRev = new SvnInfo(SubversionSCM.parseSvnInfo(svnurl, authProvider)).revision;
+        long nowRev = new SvnInfo(SubversionSCM.parseSvnInfo(svnurl, authProvider, storeAuthToDisk, workspaceFormat)).revision;
 
         changes.changes |= (nowRev>baseRev);
 

src/main/java/hudson/scm/SubversionChangeLogBuilder.java

@@ -114,7 +114,7 @@ public boolean run(@NonNull Map<String, List<SubversionSCM.External>> externalsM
             ISVNAuthenticationProvider authProvider =
                     CredentialsSVNAuthenticationProviderImpl
                             .createAuthenticationProvider(build.getParent(), scm, l, listener);
-            final SVNClientManager manager = SubversionSCM.createClientManager(authProvider).getCore();
+            final SVNClientManager manager = SubversionSCM.createClientManager(authProvider, SubversionSCM.descriptor().isStoreAuthToDisk(), SubversionSCM.descriptor().getWorkspaceFormat()).getCore();
             try {
                 SVNLogClient svnlc = manager.getLogClient();
                 PathContext context = getUrlForPath(workspace.child(l.getLocalDir()), authProvider);
@@ -244,13 +244,17 @@ private static TransformerHandler createTransformerHandler() {
 
     private static class GetContextForPath extends MasterToSlaveFileCallable<PathContext> {
         private final ISVNAuthenticationProvider authProvider;
+        private final boolean storeAuthToDisk;
+        private final int workspaceFormat;
 
         public GetContextForPath(ISVNAuthenticationProvider authProvider) {
             this.authProvider = authProvider;
+            storeAuthToDisk = SubversionSCM.descriptor().isStoreAuthToDisk();
+            workspaceFormat = SubversionSCM.descriptor().getWorkspaceFormat();
         }
 
         public PathContext invoke(File p, VirtualChannel channel) throws IOException {
-            final SvnClientManager manager = SubversionSCM.createClientManager(authProvider);
+            final SvnClientManager manager = SubversionSCM.createClientManager(authProvider, storeAuthToDisk, workspaceFormat);
             try {
                 final SVNWCClient svnwc = manager.getWCClient();
 

src/main/java/hudson/scm/SubversionTagAction.java

@@ -291,7 +291,7 @@ protected void perform(TaskListener listener) {
                 ISVNAuthenticationManager sam = new SVNAuthenticationManager(configDir, null, null);
                 sam.setAuthenticationProvider(new CredentialsSVNAuthenticationProviderImpl(upc));
                 final SvnClientManager cm = new SvnClientManager(
-                        SVNClientManager.newInstance(SubversionSCM.createDefaultSVNOptions(), sam)
+                        SVNClientManager.newInstance(SubversionSCM.createDefaultSVNOptions(SubversionSCM.descriptor().isStoreAuthToDisk()), sam), SubversionSCM.descriptor().getWorkspaceFormat()
                 );
                 try {
                     for (Entry<SvnInfo, String> e : tagSet.entrySet()) {

src/main/java/hudson/scm/SubversionWorkspaceSelector.java

@@ -23,7 +23,6 @@
  */
 package hudson.scm;
 
-import hudson.remoting.Channel;
 import org.tmatesoft.svn.core.SVNException;
 import org.tmatesoft.svn.core.internal.wc.admin.ISVNAdminAreaFactorySelector;
 import org.tmatesoft.svn.core.internal.wc.admin.SVNAdminArea14;
@@ -32,13 +31,8 @@
 import org.tmatesoft.svn.core.internal.wc2.SvnWcGeneration;
 
 import java.io.File;
-import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collection;
-import java.util.logging.Level;
-import java.util.logging.Logger;
-import jenkins.model.Jenkins;
-import jenkins.security.SlaveToMasterCallable;
 
 /**
  * {@link ISVNAdminAreaFactorySelector} that uses 1.4 compatible workspace for new check out,
@@ -64,7 +58,11 @@
  * @see SvnClientManager
  */
 public class SubversionWorkspaceSelector implements ISVNAdminAreaFactorySelector {
-    public SubversionWorkspaceSelector() {
+
+    private int workspaceFormat;
+
+    public SubversionWorkspaceSelector(int workspaceFormat) {
+        this.workspaceFormat = workspaceFormat;
         // don't upgrade the workspace.
         SVNAdminAreaFactory.setUpgradeEnabled(false);
     }
@@ -90,34 +88,11 @@ public Collection getEnabledFactories(File path, Collection factories, boolean w
     }
 
     /**
-     * {@link #getEnabledFactories(File, Collection, boolean)} method is called quite a few times
-     * during a Subversion operation, so consulting this value back with the controller each time is not practical
-     * performance wise. Therefore, we have {@link SubversionSCM} set this value, even though it's error prone.
+     * Constant for {@link #workspaceFormat} that indicates we opt for 1.7 working copy.
      *
-     * <p>
-     * Internally in SVNKit, these constants go up only to 1.6. We use {@link #WC_FORMAT_17} to indicate
+     * <p>Internally in SVNKit, these constants go up only to 1.6. We use {@link #WC_FORMAT_17} to indicate
      * 1.7 (but when that value is chosen, it is really {@link SvnClientManager} that does the work, not
      * {@link ISVNAdminAreaFactorySelector}).
-     */
-    public static volatile int workspaceFormat = SVNAdminArea14.WC_FORMAT;
-
-    public static void syncWorkspaceFormatFromMaster() {
-        Jenkins j = Jenkins.getInstanceOrNull();
-        if (j!=null)
-            workspaceFormat = j.getDescriptorByType(SubversionSCM.DescriptorImpl.class).getWorkspaceFormat();
-        else {
-            Channel c = Channel.current();
-            if (c!=null)    // just being defensive. cannot be null.
-                try {
-                    workspaceFormat = c.call(new GetWorkspaceFormatSlaveToMasterCallable());
-                } catch (IOException | InterruptedException e) {
-                    LOGGER.log(Level.WARNING, "Failed to retrieve Subversion workspace format",e);
-                }
-        }
-    }
-
-    /**
-     * Constant for {@link #workspaceFormat} that indicates we opt for 1.7 working copy.
      *
      * @deprecated Use {@link org.tmatesoft.svn.core.internal.wc17.db.ISVNWCDb#WC_FORMAT_17}
      */
@@ -129,13 +104,4 @@ public static void syncWorkspaceFormatFromMaster() {
      */
     public static final int OLD_WC_FORMAT_17 = 100;
 
-    private static final Logger LOGGER = Logger.getLogger(SubversionWorkspaceSelector.class.getName());
-
-    private static class GetWorkspaceFormatSlaveToMasterCallable extends SlaveToMasterCallable<Integer, RuntimeException> {  // TODO JENKINS-48543 bad design
-        private static final long serialVersionUID = 6494337549896104453L;
-
-        public Integer call()  {
-            return Jenkins.getInstance().getDescriptorByType(SubversionSCM.DescriptorImpl.class).getWorkspaceFormat();
-        }
-    }
 }

src/main/java/hudson/scm/SvnClientManager.java

@@ -25,10 +25,9 @@ public class SvnClientManager {
     private final SVNClientManager core;
     private final SvnWcGeneration wcgen;
 
-    public SvnClientManager(SVNClientManager core) {
+    public SvnClientManager(SVNClientManager core, int workspaceFormat) {
         this.core = core;
-        SubversionWorkspaceSelector.syncWorkspaceFormatFromMaster();
-        wcgen = SubversionWorkspaceSelector.workspaceFormat>= ISVNWCDb.WC_FORMAT_18 ? SvnWcGeneration.V17 : SvnWcGeneration.V16;
+        wcgen = workspaceFormat >= ISVNWCDb.WC_FORMAT_18 ? SvnWcGeneration.V17 : SvnWcGeneration.V16;
     }
 
     public SVNClientManager getCore() {

src/main/java/hudson/scm/UserProvidedCredential.java

@@ -60,6 +60,7 @@
 import java.util.logging.Logger;
 
 import static javax.servlet.http.HttpServletResponse.SC_FORBIDDEN;
+import jenkins.util.JenkinsJVM;
 
 /**
  * Represents the SVN authentication credential given by the user via the {@code <enterCredential>} form fragment.
@@ -86,6 +87,7 @@ public UserProvidedCredential(String username, String password, File keyFile) {
     }
 
     public UserProvidedCredential(String username, String password, File keyFile, AbstractProject inContextOf) {
+        JenkinsJVM.checkJenkinsJVM();
         this.username = username;
         this.password = password;
         this.keyFile = keyFile;
@@ -163,6 +165,7 @@ public class AuthenticationManagerImpl extends DefaultSVNAuthenticationManager {
 
         public AuthenticationManagerImpl(PrintWriter logWriter) {
             super(SVNWCUtil.getDefaultConfigurationDirectory(), true, username, password, keyFile, password);
+            JenkinsJVM.checkJenkinsJVM();
             this.logWriter = logWriter;
             SVNAuthStoreHandlerImpl.install(this);
         }

src/main/java/hudson/scm/subversion/CheckoutUpdater.java

@@ -68,8 +68,8 @@ public class CheckoutUpdater extends WorkspaceUpdater {
     public CheckoutUpdater() {}
 
     @Override
-    public UpdateTask createTask() {
-        return new SubversionUpdateTask();
+    public UpdateTask createTask(int workspaceFormat) {
+        return new SubversionUpdateTask(workspaceFormat);
     }
 
     @Extension
@@ -83,6 +83,12 @@ public String getDisplayName() {
     private static class SubversionUpdateTask extends UpdateTask {
         private static final long serialVersionUID = 8349986526712487762L;
 
+        private int workspaceFormat;
+
+        SubversionUpdateTask(int workspaceFormat) {
+            this.workspaceFormat = workspaceFormat;
+        }
+
         @Override
         @SuppressFBWarnings(value = "DM_DEFAULT_ENCODING", justification = "TODO needs triage")
         public List<External> perform() throws IOException, InterruptedException {
@@ -123,13 +129,13 @@ public List<External> perform() throws IOException, InterruptedException {
                 checkout.setExternalsHandler(SvnCodec.externalsHandler(svnuc.getExternalsHandler()));
 
                 // Statement to guard against JENKINS-26458.
-                if (SubversionWorkspaceSelector.workspaceFormat == SubversionWorkspaceSelector.OLD_WC_FORMAT_17) {
-                    SubversionWorkspaceSelector.workspaceFormat = ISVNWCDb.WC_FORMAT_17;
+                if (workspaceFormat == SubversionWorkspaceSelector.OLD_WC_FORMAT_17) {
+                    workspaceFormat = ISVNWCDb.WC_FORMAT_17;
                 }
 
                 // Workaround for SVNKIT-430 is to set the working copy format when
                 // a checkout is performed.
-                checkout.setTargetWorkingCopyFormat(SubversionWorkspaceSelector.workspaceFormat);
+                checkout.setTargetWorkingCopyFormat(workspaceFormat);
                 checkout.run();
             } catch (SVNCancelException e) {
                 if (isAuthenticationFailedError(e)) {

src/main/java/hudson/scm/subversion/NoopUpdater.java

@@ -47,7 +47,7 @@ public NoopUpdater() {
     }
 
     @Override
-    public UpdateTask createTask() {
+    public UpdateTask createTask(int workspaceFormat) {
         return new TaskImpl();
     }
 

src/main/java/hudson/scm/subversion/UpdateUpdater.java

@@ -60,8 +60,8 @@ public UpdateUpdater() {
     }
 
     @Override
-    public UpdateTask createTask() {
-        return new TaskImpl();
+    public UpdateTask createTask(int workspaceFormat) {
+        return new TaskImpl(workspaceFormat);
     }
 
     public static class TaskImpl extends UpdateTask {
@@ -70,6 +70,12 @@ public static class TaskImpl extends UpdateTask {
          */
         private static final long serialVersionUID = -5766470969352844330L;
 
+        private final int workspaceFormat;
+
+        public TaskImpl(int workspaceFormat) {
+            this.workspaceFormat = workspaceFormat;
+        }
+
         /**
          * Returns whether we can do a "svn update" or a "svn switch" or a "svn checkout"
          */
@@ -129,7 +135,7 @@ public List<External> perform() throws IOException, InterruptedException {
             SvnCommandToUse svnCommand = getSvnCommandToUse();
 
             if (svnCommand == SvnCommandToUse.CHECKOUT) {
-                return delegateTo(new CheckoutUpdater());
+                return delegateTo(new CheckoutUpdater(), workspaceFormat);
             }
 
             final SVNUpdateClient svnuc = clientManager.getUpdateClient();
@@ -182,28 +188,28 @@ public List<External> perform() throws IOException, InterruptedException {
                     if (errorCode == SVNErrorCode.WC_LOCKED) {
                         // work space locked. try fresh check out
                         listener.getLogger().println("Workspace appear to be locked, so getting a fresh workspace");
-                        return delegateTo(new CheckoutUpdater());
+                        return delegateTo(new CheckoutUpdater(), workspaceFormat);
                     }
                     if (errorCode == SVNErrorCode.WC_OBSTRUCTED_UPDATE) {
                         // HUDSON-1882. If existence of local files cause an update to fail,
                         // revert to fresh check out
                         listener.getLogger().println(e.getMessage()); // show why this happened. Sometimes this is caused by having a build artifact in the repository.
                         listener.getLogger().println("Updated failed due to local files. Getting a fresh workspace");
-                        return delegateTo(new CheckoutUpdater());
+                        return delegateTo(new CheckoutUpdater(), workspaceFormat);
                     }
                     if (errorCode == SVNErrorCode.WC_CORRUPT_TEXT_BASE || errorCode == SVNErrorCode.WC_CORRUPT || errorCode == SVNErrorCode.WC_UNWIND_EMPTY) {
                         // JENKINS-14550. if working copy is corrupted, revert to fresh check out
                         listener.getLogger().println(e.getMessage()); // show why this happened. Sometimes this is caused by having a build artifact in the repository.
                         listener.getLogger().println("Updated failed due to working copy corruption. Getting a fresh workspace");
-                        return delegateTo(new CheckoutUpdater());
+                        return delegateTo(new CheckoutUpdater(), workspaceFormat);
                     }
                     // trouble-shooting probe for #591
                     if (errorCode == SVNErrorCode.WC_NOT_LOCKED) {
                         Jenkins instance = Jenkins.getInstanceOrNull();
                         if (instance != null) {
                             listener.getLogger().println("Polled jobs are " + instance.getDescriptorByType(SCMTrigger.DescriptorImpl.class).getItemsBeingPolled());
                         }
-                        return delegateTo(new CheckoutUpdater());
+                        return delegateTo(new CheckoutUpdater(), workspaceFormat);
                     }
 
                   // recurse as long as we encounter nested SVNException

src/main/java/hudson/scm/subversion/UpdateWithCleanUpdater.java

@@ -48,8 +48,8 @@ public class UpdateWithCleanUpdater extends WorkspaceUpdater {
     public UpdateWithCleanUpdater() {}
 
     @Override
-    public UpdateTask createTask() {
-        return new TaskImpl();
+    public UpdateTask createTask(int workspaceFormat) {
+        return new TaskImpl(workspaceFormat);
     }
 
     // mostly "svn update" plus extra
@@ -59,6 +59,10 @@ public static class TaskImpl extends UpdateUpdater.TaskImpl {
          */
         private static final long serialVersionUID = -5120852266435704852L;
 
+        public TaskImpl(int workspaceFormat) {
+            super(workspaceFormat);
+        }
+
         @Override
         protected void preUpdate(ModuleLocation module, File local) throws SVNException, IOException {
             listener.getLogger().println("Cleaning up " + local);

src/main/java/hudson/scm/subversion/UpdateWithRevertUpdater.java

@@ -44,8 +44,8 @@ public class UpdateWithRevertUpdater extends WorkspaceUpdater {
     public UpdateWithRevertUpdater() {}
 
     @Override
-    public UpdateTask createTask() {
-        return new TaskImpl();
+    public UpdateTask createTask(int workspaceFormat) {
+        return new TaskImpl(workspaceFormat);
     }
 
     // mostly "svn update" plus extra
@@ -55,6 +55,10 @@ public static class TaskImpl extends UpdateUpdater.TaskImpl {
          */
         private static final long serialVersionUID = -8562813147341259328L;
 
+        public TaskImpl(int workspaceFormat) {
+            super(workspaceFormat);
+        }
+
         @Override
         protected void preUpdate(ModuleLocation module, File local) throws SVNException, IOException {
             listener.getLogger().println("Reverting " + local + " to depth " + module.getDepthOption() + " with ignoreExternals: " + module.isIgnoreExternalsOption());

src/main/java/hudson/scm/subversion/WorkspaceUpdater.java

@@ -32,6 +32,7 @@
 import hudson.scm.SubversionSCM.External;
 import hudson.scm.SubversionSCM.ModuleLocation;
 import hudson.scm.SvnClientManager;
+import org.jenkinsci.remoting.SerializableOnlyOverRemoting;
 import org.kohsuke.stapler.export.ExportedBean;
 import org.tmatesoft.svn.core.SVNCancelException;
 import org.tmatesoft.svn.core.SVNDepth;
@@ -41,7 +42,6 @@
 
 import java.io.File;
 import java.io.IOException;
-import java.io.Serializable;
 import java.util.Date;
 import java.util.List;
 
@@ -56,13 +56,13 @@
  * @since 1.23
  */
 @ExportedBean
-public abstract class WorkspaceUpdater extends AbstractDescribableImpl<WorkspaceUpdater> implements ExtensionPoint, Serializable {
+public abstract class WorkspaceUpdater extends AbstractDescribableImpl<WorkspaceUpdater> implements ExtensionPoint {
     private static final long serialVersionUID = 8902811304319899817L;
 
     /**
      * Creates the {@link UpdateTask} instance, which performs the actual check out / update.
      */
-    public abstract UpdateTask createTask();
+    public abstract UpdateTask createTask(int workspaceFormat);
 
     @Override
     public WorkspaceUpdaterDescriptor getDescriptor() {
@@ -93,7 +93,7 @@ protected static boolean isAuthenticationFailedError(SVNCancelException e) {
      * A number of contextual objects are defined as fields, to be used by the {@link #perform()} method.
      * These fields are set by {@link SubversionSCM} before the invocation.
      */
-    public static abstract class UpdateTask implements Serializable {
+    public static abstract class UpdateTask implements SerializableOnlyOverRemoting {
         // fields that are set by the caller as context for the perform method
 
         /**
@@ -172,8 +172,8 @@ protected List<External> delegateTo(UpdateTask t) throws IOException, Interrupte
          * Delegates the execution to another updater. This is most often useful to fall back to the fresh check out
          * by using {@link CheckoutUpdater}.
          */
-        protected final List<External> delegateTo(WorkspaceUpdater wu) throws IOException, InterruptedException {
-            return delegateTo(wu.createTask());
+        protected final List<External> delegateTo(WorkspaceUpdater wu, int workspaceFormat) throws IOException, InterruptedException {
+            return delegateTo(wu.createTask(workspaceFormat));
         }
 
         /**