From abf7858f3308a8c9815aac3f9a0094f43fed1ae6 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Fri, 8 Jun 2018 13:25:07 -0400 Subject: [PATCH 1/5] [JENKINS-51779] Avoid com.google.common.collect.Iterators.skip (#3481) * [JENKINS-51779] Avoid com.google.common.collect.Iterators.skip. * Reviewers preferred for the new method to be restricted for now. (cherry picked from commit a846c366fe1569805548544118d65d908de5644a) --- core/src/main/java/hudson/util/Iterators.java | 20 +++++++++++++ core/src/main/java/hudson/util/RunList.java | 2 +- .../jenkins/widgets/HistoryPageFilter.java | 2 +- .../test/java/hudson/util/IteratorsTest.java | 29 +++++++++++++++---- 4 files changed, 46 insertions(+), 7 deletions(-) diff --git a/core/src/main/java/hudson/util/Iterators.java b/core/src/main/java/hudson/util/Iterators.java index 74d82950dc..8d93d72262 100644 --- a/core/src/main/java/hudson/util/Iterators.java +++ b/core/src/main/java/hudson/util/Iterators.java @@ -23,6 +23,7 @@ */ package hudson.util; +import com.google.common.annotations.Beta; import com.google.common.base.Predicates; import com.google.common.collect.ImmutableList; @@ -35,6 +36,9 @@ import java.util.AbstractList; import java.util.Arrays; import java.util.Set; import java.util.HashSet; +import javax.annotation.Nonnull; +import org.kohsuke.accmod.Restricted; +import org.kohsuke.accmod.restrictions.NoExternalUse; /** * Varios {@link Iterator} implementations. @@ -403,4 +407,20 @@ public class Iterators { public interface CountingPredicate { boolean apply(int index, T input); } + + /** + * Similar to {@link com.google.common.collect.Iterators#skip} except not {@link Beta}. + * @param iterator some iterator + * @param count a nonnegative count + */ + @Restricted(NoExternalUse.class) + public static void skip(@Nonnull Iterator iterator, int count) { + if (count < 0) { + throw new IllegalArgumentException(); + } + while (iterator.hasNext() && count-- > 0) { + iterator.next(); + } + } + } diff --git a/core/src/main/java/hudson/util/RunList.java b/core/src/main/java/hudson/util/RunList.java index d9b8adb7dc..bdc9cb61f4 100644 --- a/core/src/main/java/hudson/util/RunList.java +++ b/core/src/main/java/hudson/util/RunList.java @@ -150,7 +150,7 @@ public class RunList extends AbstractList { public List subList(int fromIndex, int toIndex) { List r = new ArrayList(); Iterator itr = iterator(); - Iterators.skip(itr,fromIndex); + hudson.util.Iterators.skip(itr, fromIndex); for (int i=toIndex-fromIndex; i>0; i--) { r.add(itr.next()); } diff --git a/core/src/main/java/jenkins/widgets/HistoryPageFilter.java b/core/src/main/java/jenkins/widgets/HistoryPageFilter.java index 269f5e44fc..521ace8b79 100644 --- a/core/src/main/java/jenkins/widgets/HistoryPageFilter.java +++ b/core/src/main/java/jenkins/widgets/HistoryPageFilter.java @@ -24,7 +24,6 @@ package jenkins.widgets; import com.google.common.collect.Iterables; -import com.google.common.collect.Iterators; import hudson.model.AbstractBuild; import hudson.model.Job; import hudson.model.ParameterValue; @@ -32,6 +31,7 @@ import hudson.model.ParametersAction; import hudson.model.Queue; import hudson.model.Run; import hudson.search.UserSearchProperty; +import hudson.util.Iterators; import hudson.widgets.HistoryWidget; import javax.annotation.Nonnull; diff --git a/core/src/test/java/hudson/util/IteratorsTest.java b/core/src/test/java/hudson/util/IteratorsTest.java index 679df76a65..4236ca359d 100644 --- a/core/src/test/java/hudson/util/IteratorsTest.java +++ b/core/src/test/java/hudson/util/IteratorsTest.java @@ -32,6 +32,7 @@ import java.util.Iterator; import java.util.List; import org.junit.Test; +import org.jvnet.hudson.test.Issue; /** * @author Kohsuke Kawaguchi @@ -77,9 +78,27 @@ public class IteratorsTest { assertEquals("[]", com.google.common.collect.Iterators.toString(Iterators.limit(asList(1,2,4,6).iterator(), EVEN))); } - public static final CountingPredicate EVEN = new CountingPredicate() { - public boolean apply(int index, Integer input) { - return input % 2 == 0; - } - }; + public static final CountingPredicate EVEN = (index, input) -> input % 2 == 0; + + @Issue("JENKINS-51779") + @Test + public void skip() { + List lst = Iterators.sequence(1, 4); + Iterator it = lst.iterator(); + Iterators.skip(it, 0); + assertEquals("[1, 2, 3]", com.google.common.collect.Iterators.toString(it)); + it = lst.iterator(); + Iterators.skip(it, 1); + assertEquals("[2, 3]", com.google.common.collect.Iterators.toString(it)); + it = lst.iterator(); + Iterators.skip(it, 2); + assertEquals("[3]", com.google.common.collect.Iterators.toString(it)); + it = lst.iterator(); + Iterators.skip(it, 3); + assertEquals("[]", com.google.common.collect.Iterators.toString(it)); + it = lst.iterator(); + Iterators.skip(it, 4); + assertEquals("[]", com.google.common.collect.Iterators.toString(it)); + } + } From 2a7b56e8385beae1a11290b6e576423799487fbc Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Fri, 8 Jun 2018 13:55:29 -0400 Subject: [PATCH 2/5] [JENKINS-51819] When using a custom ArtifactManager, Run.delete must call ArtifactManager.delete. (cherry picked from commit 9b0a0e71b8a6b92227e6af80ec7202aa0bf44b5b) --- core/src/main/java/hudson/model/Run.java | 4 ++ test/src/test/java/hudson/model/RunTest.java | 46 ++++++++++++++++++-- 2 files changed, 47 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/hudson/model/Run.java b/core/src/main/java/hudson/model/Run.java index 10ec9f6574..a3bc3375be 100644 --- a/core/src/main/java/hudson/model/Run.java +++ b/core/src/main/java/hudson/model/Run.java @@ -1554,6 +1554,10 @@ public abstract class Run ,RunT extends Run build) {} + @Override public void archive(FilePath workspace, Launcher launcher, BuildListener listener, Map artifacts) throws IOException, InterruptedException {} + @Override public VirtualFile root() { + return VirtualFile.forFile(Jenkins.get().getRootDir()); // irrelevant + } + public static final class Factory extends ArtifactManagerFactory { + @DataBoundConstructor public Factory() {} + @Override public ArtifactManager managerFor(Run build) { + return new Mgr(); + } + @TestExtension("deleteArtifactsCustom") public static final class DescriptorImpl extends ArtifactManagerFactoryDescriptor {} + } + } + } From 4178bad7413244ed071d291d79d3e9c9630df513 Mon Sep 17 00:00:00 2001 From: Oleg Nenashev Date: Fri, 8 Jun 2018 23:11:38 +0200 Subject: [PATCH 3/5] [JENKINS-51541,JENKINS-51551,...] - Remoting 3.21 + Allow passing custom CommandTransport implementations to SlaveComputer from ComputerLauncher (#3455) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [JENKINS-51541] - Introduce new SlaveComputer#setChannel() method which takes custom ChannelBuilder and CommandTransport * [JENKINS-51551] - Pick Remoting version with the API patch * [JENKINS-51541] - Listener is nullable according to the documentation * [JENKINS-51551,JENKINS-51223,JENKINS-50965] - Update Remoting to 3.21 * [JENKINS-51541] - Restrict SlaveComputer#setChannel(ChannelBuilder cb, …) to Beta-use only (cherry picked from commit c405ae701c4bca18f7b4edc6e5cd3bc053b7ee60) --- .../java/hudson/slaves/SlaveComputer.java | 39 +++++++++++++++++++ pom.xml | 2 +- 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/hudson/slaves/SlaveComputer.java b/core/src/main/java/hudson/slaves/SlaveComputer.java index c1b5166e71..fc8c04eef3 100644 --- a/core/src/main/java/hudson/slaves/SlaveComputer.java +++ b/core/src/main/java/hudson/slaves/SlaveComputer.java @@ -23,6 +23,7 @@ */ package hudson.slaves; +import com.google.common.io.NullOutputStream; import hudson.AbortException; import hudson.FilePath; import hudson.Functions; @@ -39,6 +40,7 @@ import hudson.model.User; import hudson.remoting.Channel; import hudson.remoting.ChannelBuilder; import hudson.remoting.ChannelClosedException; +import hudson.remoting.CommandTransport; import hudson.remoting.Launcher; import hudson.remoting.VirtualChannel; import hudson.security.ACL; @@ -60,6 +62,8 @@ import jenkins.slaves.systemInfo.SlaveSystemInfo; import jenkins.util.SystemProperties; import org.acegisecurity.context.SecurityContext; import org.acegisecurity.context.SecurityContextHolder; +import org.kohsuke.accmod.Restricted; +import org.kohsuke.accmod.restrictions.Beta; import org.kohsuke.stapler.HttpRedirect; import org.kohsuke.stapler.HttpResponse; import org.kohsuke.stapler.QueryParameter; @@ -69,6 +73,7 @@ import org.kohsuke.stapler.WebMethod; import org.kohsuke.stapler.interceptor.RequirePOST; import javax.annotation.CheckForNull; +import javax.annotation.Nonnull; import javax.annotation.OverridingMethodsMustInvokeSuper; import javax.servlet.ServletException; import java.io.File; @@ -416,6 +421,40 @@ public class SlaveComputer extends Computer { setChannel(channel,launchLog,listener); } + /** + * Creates a {@link Channel} from the given Channel Builder and Command Transport. + * This method can be used to allow {@link ComputerLauncher}s to create channels not based on I/O streams. + * + * @param cb + * Channel Builder. + * To print launch logs this channel builder should have a Header Stream defined + * (see {@link ChannelBuilder#getHeaderStream()}) in this argument or by one of {@link ChannelConfigurator}s. + * @param commandTransport + * Command Transport + * @param listener + * Gets a notification when the channel closes, to perform clean up. Can be {@code null}. + * By the time this method is called, the cause of the termination is reported to the user, + * so the implementation of the listener doesn't need to do that again. + * @since TODO + */ + @Restricted(Beta.class) + public void setChannel(@Nonnull ChannelBuilder cb, + @Nonnull CommandTransport commandTransport, + @CheckForNull Channel.Listener listener) throws IOException, InterruptedException { + for (ChannelConfigurator cc : ChannelConfigurator.all()) { + cc.onChannelBuilding(cb,this); + } + + OutputStream headerStream = cb.getHeaderStream(); + if (headerStream == null) { + LOGGER.log(Level.WARNING, "No header stream defined when setting channel for computer {0}. " + + "Launch log won't be printed", this); + headerStream = new NullOutputStream(); + } + Channel channel = cb.build(commandTransport); + setChannel(channel, headerStream, listener); + } + /** * Shows {@link Channel#classLoadingCount}. * @since 1.495 diff --git a/pom.xml b/pom.xml index aeba985b12..7b6592c7a0 100644 --- a/pom.xml +++ b/pom.xml @@ -107,7 +107,7 @@ THE SOFTWARE. 3.0.0 - 3.20 + 3.21 2.60 From 8696646ffd916d6028c0fbc71793bf997364e750 Mon Sep 17 00:00:00 2001 From: Oleg Nenashev Date: Fri, 15 Jun 2018 20:09:28 +0200 Subject: [PATCH 4/5] Merge pull request #3489 from oleg-nenashev/bug/JENKINS-51889 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit [JENKINS-51889,JENKINS-51955,JENKINS-51971] - Stop using Guava’s NullOutputStream, document setChannel API (cherry picked from commit e4e06333ee3c4dcdf8c945f85691e1dbb639d5e7) --- .../java/hudson/slaves/SlaveComputer.java | 28 +++++++++++++------ .../java/hudson/util/StreamTaskListener.java | 23 +++++++++------ 2 files changed, 34 insertions(+), 17 deletions(-) diff --git a/core/src/main/java/hudson/slaves/SlaveComputer.java b/core/src/main/java/hudson/slaves/SlaveComputer.java index fc8c04eef3..15f106144a 100644 --- a/core/src/main/java/hudson/slaves/SlaveComputer.java +++ b/core/src/main/java/hudson/slaves/SlaveComputer.java @@ -23,7 +23,6 @@ */ package hudson.slaves; -import com.google.common.io.NullOutputStream; import hudson.AbortException; import hudson.FilePath; import hudson.Functions; @@ -385,7 +384,15 @@ public class SlaveComputer extends Computer { private final Object channelLock = new Object(); - public void setChannel(InputStream in, OutputStream out, TaskListener taskListener, Channel.Listener listener) throws IOException, InterruptedException { + /** + * Creates a {@link Channel} from the given stream and sets that to this agent. + * + * Same as {@link #setChannel(InputStream, OutputStream, OutputStream, Channel.Listener)}, but for + * {@link TaskListener}. + */ + public void setChannel(@Nonnull InputStream in, @Nonnull OutputStream out, + @Nonnull TaskListener taskListener, + @CheckForNull Channel.Listener listener) throws IOException, InterruptedException { setChannel(in,out,taskListener.getLogger(),listener); } @@ -408,7 +415,9 @@ public class SlaveComputer extends Computer { * By the time this method is called, the cause of the termination is reported to the user, * so the implementation of the listener doesn't need to do that again. */ - public void setChannel(InputStream in, OutputStream out, OutputStream launchLog, Channel.Listener listener) throws IOException, InterruptedException { + public void setChannel(@Nonnull InputStream in, @Nonnull OutputStream out, + @CheckForNull OutputStream launchLog, + @CheckForNull Channel.Listener listener) throws IOException, InterruptedException { ChannelBuilder cb = new ChannelBuilder(nodeName,threadPoolForRemoting) .withMode(Channel.Mode.NEGOTIATE) .withHeaderStream(launchLog); @@ -435,7 +444,7 @@ public class SlaveComputer extends Computer { * Gets a notification when the channel closes, to perform clean up. Can be {@code null}. * By the time this method is called, the cause of the termination is reported to the user, * so the implementation of the listener doesn't need to do that again. - * @since TODO + * @since 2.127 */ @Restricted(Beta.class) public void setChannel(@Nonnull ChannelBuilder cb, @@ -449,7 +458,6 @@ public class SlaveComputer extends Computer { if (headerStream == null) { LOGGER.log(Level.WARNING, "No header stream defined when setting channel for computer {0}. " + "Launch log won't be printed", this); - headerStream = new NullOutputStream(); } Channel channel = cb.build(commandTransport); setChannel(channel, headerStream, listener); @@ -548,14 +556,18 @@ public class SlaveComputer extends Computer { /** * Sets up the connection through an existing channel. - * @param channel the channel to use; warning: callers are expected to have called {@link ChannelConfigurator} already + * @param channel the channel to use; warning: callers are expected to have called {@link ChannelConfigurator} already. + * @param launchLog Launch log. If not {@code null}, will receive launch log messages + * @param listener Channel event listener to be attached (if not {@code null}) * @since 1.444 */ - public void setChannel(Channel channel, OutputStream launchLog, Channel.Listener listener) throws IOException, InterruptedException { + public void setChannel(@Nonnull Channel channel, + @CheckForNull OutputStream launchLog, + @CheckForNull Channel.Listener listener) throws IOException, InterruptedException { if(this.channel!=null) throw new IllegalStateException("Already connected"); - final TaskListener taskListener = new StreamTaskListener(launchLog); + final TaskListener taskListener = launchLog != null ? new StreamTaskListener(launchLog) : TaskListener.NULL; PrintStream log = taskListener.getLogger(); channel.setProperty(SlaveComputer.class, this); diff --git a/core/src/main/java/hudson/util/StreamTaskListener.java b/core/src/main/java/hudson/util/StreamTaskListener.java index bd2b6dc87e..b2b2e4fd62 100644 --- a/core/src/main/java/hudson/util/StreamTaskListener.java +++ b/core/src/main/java/hudson/util/StreamTaskListener.java @@ -44,6 +44,9 @@ import java.util.logging.Level; import java.util.logging.Logger; import org.kohsuke.stapler.framework.io.WriterOutputStream; +import javax.annotation.CheckForNull; +import javax.annotation.Nonnull; + // TODO: AbstractTaskListener is empty now, but there are dependencies on that e.g. Ruby Runtime - JENKINS-48116) // The change needs API deprecation policy or external usages cleanup. @@ -56,7 +59,9 @@ import org.kohsuke.stapler.framework.io.WriterOutputStream; * @author Kohsuke Kawaguchi */ public class StreamTaskListener extends AbstractTaskListener implements TaskListener, Closeable { + @Nonnull private PrintStream out; + @CheckForNull private Charset charset; /** @@ -66,15 +71,15 @@ public class StreamTaskListener extends AbstractTaskListener implements TaskList * or use {@link #fromStdout()} or {@link #fromStderr()}. */ @Deprecated - public StreamTaskListener(PrintStream out) { + public StreamTaskListener(@Nonnull PrintStream out) { this(out,null); } - public StreamTaskListener(OutputStream out) { + public StreamTaskListener(@Nonnull OutputStream out) { this(out,null); } - public StreamTaskListener(OutputStream out, Charset charset) { + public StreamTaskListener(@Nonnull OutputStream out, @CheckForNull Charset charset) { try { if (charset == null) this.out = (out instanceof PrintStream) ? (PrintStream)out : new PrintStream(out, false); @@ -87,18 +92,18 @@ public class StreamTaskListener extends AbstractTaskListener implements TaskList } } - public StreamTaskListener(File out) throws IOException { + public StreamTaskListener(@Nonnull File out) throws IOException { this(out,null); } - public StreamTaskListener(File out, Charset charset) throws IOException { + public StreamTaskListener(@Nonnull File out, @CheckForNull Charset charset) throws IOException { // don't do buffering so that what's written to the listener // gets reflected to the file immediately, which can then be // served to the browser immediately this(Files.newOutputStream(asPath(out)), charset); } - private static Path asPath(File out) throws IOException { + private static Path asPath(@Nonnull File out) throws IOException { try { return out.toPath(); } catch (InvalidPathException e) { @@ -115,7 +120,7 @@ public class StreamTaskListener extends AbstractTaskListener implements TaskList * @throws IOException if the file could not be opened. * @since 1.651 */ - public StreamTaskListener(File out, boolean append, Charset charset) throws IOException { + public StreamTaskListener(@Nonnull File out, boolean append, @CheckForNull Charset charset) throws IOException { // don't do buffering so that what's written to the listener // gets reflected to the file immediately, which can then be // served to the browser immediately @@ -127,7 +132,7 @@ public class StreamTaskListener extends AbstractTaskListener implements TaskList ); } - public StreamTaskListener(Writer w) throws IOException { + public StreamTaskListener(@Nonnull Writer w) throws IOException { this(new WriterOutputStream(w)); } @@ -155,7 +160,7 @@ public class StreamTaskListener extends AbstractTaskListener implements TaskList @Override public Charset getCharset() { - return charset; + return charset != null ? charset : Charset.defaultCharset(); } private void writeObject(ObjectOutputStream out) throws IOException { From 654c92cc24dc1282afc58cee407f8522e7d699ab Mon Sep 17 00:00:00 2001 From: Oleg Nenashev Date: Wed, 30 May 2018 14:23:21 +0200 Subject: [PATCH 5/5] [JENKINS-51608] - Improve diagnostics of errorneous cases in ClassicPluginStrategy#getShortName() (cherry picked from commit 9bd13323c84c38f98091e2deffed0b7ae1cc5d68) --- core/src/main/java/hudson/ClassicPluginStrategy.java | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/core/src/main/java/hudson/ClassicPluginStrategy.java b/core/src/main/java/hudson/ClassicPluginStrategy.java index bc00b2a0c9..6503d4b639 100644 --- a/core/src/main/java/hudson/ClassicPluginStrategy.java +++ b/core/src/main/java/hudson/ClassicPluginStrategy.java @@ -25,6 +25,8 @@ package hudson; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; + +import java.io.FileNotFoundException; import java.io.InputStream; import java.nio.file.Files; import java.nio.file.InvalidPathException; @@ -110,11 +112,20 @@ public class ClassicPluginStrategy implements PluginStrategy { @Override public String getShortName(File archive) throws IOException { Manifest manifest; + if (!archive.exists()) { + throw new FileNotFoundException("Failed to load " + archive + ". The file does not exist"); + } else if (!archive.isFile()) { + throw new FileNotFoundException("Failed to load " + archive + ". It is not a file"); + } + if (isLinked(archive)) { manifest = loadLinkedManifest(archive); } else { try (JarFile jf = new JarFile(archive, false)) { manifest = jf.getManifest(); + } catch (IOException ex) { + // Mention file name in the exception + throw new IOException("Failed to load " + archive, ex); } } return PluginWrapper.computeShortName(manifest, archive.getName());