-
Notifications
You must be signed in to change notification settings - Fork 68
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Vendored Guava should not use reflection #117
Changes from 6 commits
ec98115
43836aa
372659e
9e70d37
65ea927
d59cebc
e295dc5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,7 +3,6 @@ | |
import com.google.common.util.concurrent.AbstractFuture; | ||
import com.google.common.util.concurrent.AsyncFunction; | ||
import com.google.common.util.concurrent.ListenableFuture; | ||
import com.google.common.util.concurrent.MoreExecutors; | ||
|
||
import javax.annotation.Nullable; | ||
import java.lang.reflect.UndeclaredThrowableException; | ||
|
@@ -30,6 +29,7 @@ | |
* in a {@code UndeclaredThrowableException} so that this class can get | ||
* access to the cause. | ||
*/ | ||
@Deprecated | ||
class ChainingListenableFuture<I, O> | ||
extends AbstractFuture<O> implements Runnable { | ||
|
||
|
@@ -208,7 +208,7 @@ public void run() { | |
ChainingListenableFuture.this.outputFuture = null; | ||
} | ||
} | ||
}, Futures.newExecutorService()); | ||
}, MoreExecutors.directExecutor()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Undoing a change made in #114 in favor of the upstream version. We should generally avoid changing vendored upstream code. |
||
} catch (UndeclaredThrowableException e) { | ||
// Set the cause of the exception as this future's exception | ||
setException(e.getCause()); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
/* | ||
* Copyright (C) 2007 The Guava Authors | ||
* | ||
* 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 org.jenkinsci.plugins.workflow.support.concurrent; | ||
|
||
import com.google.common.annotations.GwtCompatible; | ||
import org.kohsuke.accmod.Restricted; | ||
import org.kohsuke.accmod.restrictions.NoExternalUse; | ||
import java.util.concurrent.Executor; | ||
|
||
/** | ||
* An {@link Executor} that runs each task in the thread that invokes {@link Executor#execute | ||
* execute}. | ||
*/ | ||
@GwtCompatible | ||
@Restricted(NoExternalUse.class) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Taken without change from Guava 30.1. Used only by the vendored code. Marked as restricted to prevent any further usages. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should this be added as a comment in the source? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the commit message should cover the provenance well enough:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Though There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah, i noticed that line too. I was going to make a special exception :) |
||
enum DirectExecutor implements Executor { | ||
INSTANCE; | ||
|
||
@Override | ||
public void execute(Runnable command) { | ||
command.run(); | ||
} | ||
|
||
@Override | ||
public String toString() { | ||
return "MoreExecutors.directExecutor()"; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This class is only used from
workflow-support
. Deprecating it so thatworkflow-support
can prefer the upstream version once core is on a recent version of Guava andworkflow-support
's baseline has been updated.