- Code that is running from a plugin and on the master's JVM is guaranteed to never get null from this method (any cases where you do get null are bugs in core)
- Code that is running from a plugin and on a remote JVM should never be allowed to load the Jenkins class in their classloader, so should never use Jenkins.getInstance()... we are annotating the method with @Nullable so that such code can have some evolution time
- Code that is running in core and on one of two special paths should use the Jenkins.getInstanceOrNull() method so that the UI can be presented to users before the singleton has been instantiated / after the singleton has been destroyed
- The remaining 95% of uses in core (and 100% of uses in plugins) can safely assume that the instance is never null
This reverts commit bb7c8fcedb.
Closes#2090, I'll redo this as a PR... though if that PR is subject to multiple rounds of review before being merged then I will take that of evidence of the exact problem that committing directly was supposed to resolve... namely exponentially multiplying the effort required to make actual improvements to the code base.
- It is never too late to do the right thing.
- The vast majority of usages of `Jenkins.getInstance()` in core currently assume that its return value is non-null
- This commit changes those that are written to correctly check for non-null values will call `Jenkins.getInstanceOrNull()`
- We deprecate the `Jenkins.getActiveInstance()` madness
- I checked with @kohsuke who said not to bother with a PR and just commit this strongly opinionated change direct to master as a PR will just degrade into a bikeshedding.
In addition to cluttering the namespace, unused imports generate
compiler warnings, introducing noise and obscuring more important
compiler warnings. This change removes them.
Not fully solved, since the scannedDirs field can still grow to be large, but at least clearing files/dirsNotIncluded.
Also imposing a 5s timeout on the scan regardless of file count, and defining a user-customizable bound.
This code duplication in two sides of the if statement is undesirable.
Post-release, I need to come back here to introduce a variant of FileCallable that runs on the given FilePath but with InputStream to another FilePath.
Still do it so we capture FilePath.remote (which many JRE methods suppress),
but fix the wrapper Throwable.message to include cause.toString() according to convention.
If we ever decide to develop a compatibility thunk library for these changes to help promote deployment in plugins,
then these classes being a nested type is problematic as they'll fail to load under onld Jenkins
If a @SlaveToMaster Callable gets sent to the master, and if inside its call() method the code directly/indirectly performs file system access (for example on files and directories that it didn't receive as parameters), that is not meant to be the subject of the filter check.
Rather, the check is meant for FilePath that the other side sends to us, when it points to a local file.
So it makes more sense to compute the effective FilePathFilter during the deserialization, as opposed to doing so when the actual operation is attempted.
Various FileCallable impls in FilePath fits this pattern, as it drags outer FilePath instance with itself. In addition, this change addresses other situations correctly, where FilePath is used as arguments and return values of other Callables, such as the following:
FilePath f = channel.call(new SlaveWillDoSomeComputationAndGiveMeTheResultAsFile());
f.copyTo(build.getRootDir().child("data.xml"))
"f.copyTo" happens in the context of Channel.current()==null, but we need to check that 'f' is legit and not something like
new FilePath(new File("/etc/shadow"))
- If a write results in a file creation, do the create check, to align with the POSIX semantics.
- Don't make pointless mkdirs check if the directory already exists
- read vs readSymlink difference is dangerous because read() can be called for symlinks in other context as well.
java.lang.NullPointerException: null
at hudson.model.AbstractBuild.setWorkspace(AbstractBuild.java:296)
at hudson.model.ProjectTest.testCheckout(ProjectTest.java:423)
Daniel Beck