Skip to content

Commit a270125

Browse files
rsandellrsandell
authored
Merge pull request #137 from sboardwell/add-checks-post-caching
[JENKINS-73769] account for empty libraries or changed library path
2 parents 07a7137 + 4d646c2 commit a270125

File tree

4 files changed

+186
-27
lines changed

4 files changed

+186
-27
lines changed

‎src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryAdder.java‎

Lines changed: 69 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import java.io.File;
3535
import java.io.IOException;
3636
import java.io.InputStream;
37+
import java.net.MalformedURLException;
3738
import java.net.URL;
3839
import java.util.ArrayList;
3940
import java.util.Base64;
@@ -133,8 +134,13 @@
133134
if (cfg instanceof LibraryResolver.ResolvedLibraryConfiguration) {
134135
source = ((LibraryResolver.ResolvedLibraryConfiguration) cfg).getSource();
135136
}
136-
librariesAdded.put(name, new LibraryRecord(name, version, kindTrusted, changelog, cfg.getCachingConfiguration(), source));
137-
retrievers.put(name, cfg.getRetriever());
137+
LibraryRetriever retriever = cfg.getRetriever();
138+
String libraryPath = null;
139+
if (retriever instanceof SCMBasedRetriever) {
140+
libraryPath = ((SCMBasedRetriever) retriever).getLibraryPath();
141+
}
142+
librariesAdded.put(name, new LibraryRecord(name, version, kindTrusted, changelog, cfg.getCachingConfiguration(), source, libraryPath));
143+
retrievers.put(name, retriever);
138144
}
139145
}
140146
for (String name : librariesAdded.keySet()) {
@@ -147,7 +153,7 @@
147153
build.addAction(new LibrariesAction(new ArrayList<>(librariesAdded.values())));
148154
// Now actually try to retrieve the libraries.
149155
for (LibraryRecord record : librariesAdded.values()) {
150-
listener.getLogger().println("Loading library " + record.name + "@" + record.version);
156+
listener.getLogger().println("Loading library " + record.getLogString());
151157
for (URL u : retrieve(record, retrievers.get(record.name), listener, build, execution)) {
152158
additions.add(new Addition(u, record.trusted));
153159
}
@@ -166,10 +172,11 @@
166172

167173
private enum CacheStatus {
168174
VALID,
175+
EMPTY,
169176
DOES_NOT_EXIST,
170177
EXPIRED;
171178
}
172-
179+
173180
private static CacheStatus getCacheStatus(@NonNull LibraryCachingConfiguration cachingConfiguration, @NonNull final FilePath versionCacheDir)
174181
throws IOException, InterruptedException
175182
{
@@ -178,6 +185,9 @@ private static CacheStatus getCacheStatus(@NonNull LibraryCachingConfiguration c
178185

179186
if(versionCacheDir.exists()) {
180187
if ((versionCacheDir.lastModified() + cachingMilliseconds) > System.currentTimeMillis()) {
188+
if (getUrlsForLibDir(versionCacheDir).isEmpty()) {
189+
return CacheStatus.EMPTY;
190+
}
181191
return CacheStatus.VALID;
182192
} else {
183193
return CacheStatus.EXPIRED;
@@ -187,6 +197,9 @@ private static CacheStatus getCacheStatus(@NonNull LibraryCachingConfiguration c
187197
}
188198
} else {
189199
if (versionCacheDir.exists()) {
200+
if (getUrlsForLibDir(versionCacheDir).isEmpty()) {
201+
return CacheStatus.EMPTY;
202+
}
190203
return CacheStatus.VALID;
191204
} else {
192205
return CacheStatus.DOES_NOT_EXIST;
@@ -199,6 +212,7 @@ static List<URL> retrieve(@NonNull LibraryRecord record, @NonNull LibraryRetriev
199212
String name = record.name;
200213
String version = record.version;
201214
boolean changelog = record.changelog;
215+
String libraryLogString = record.getLogString();
202216
LibraryCachingConfiguration cachingConfiguration = record.cachingConfiguration;
203217
FilePath libDir = new FilePath(execution.getOwner().getRootDir()).child("libs/" + record.getDirectoryName());
204218
Boolean shouldCache = cachingConfiguration != null;
@@ -207,7 +221,7 @@ static List<URL> retrieve(@NonNull LibraryRecord record, @NonNull LibraryRetriev
207221
final FilePath lastReadFile = new FilePath(versionCacheDir, LibraryCachingConfiguration.LAST_READ_FILE);
208222

209223
if(shouldCache && cachingConfiguration.isExcluded(version)) {
210-
listener.getLogger().println("Library " + name + "@" + version + " is excluded from caching.");
224+
listener.getLogger().println("Library " + libraryLogString + " is excluded from caching.");
211225
shouldCache = false;
212226
}
213227

@@ -218,38 +232,51 @@ static List<URL> retrieve(@NonNull LibraryRecord record, @NonNull LibraryRetriev
218232
retrieveLock.readLock().lockInterruptibly();
219233
try {
220234
CacheStatus cacheStatus = getCacheStatus(cachingConfiguration, versionCacheDir);
221-
if (cacheStatus == CacheStatus.DOES_NOT_EXIST || cacheStatus == CacheStatus.EXPIRED) {
235+
if (cacheStatus == CacheStatus.DOES_NOT_EXIST || cacheStatus == CacheStatus.EXPIRED || cacheStatus == CacheStatus.EMPTY) {
222236
retrieveLock.readLock().unlock();
223237
retrieveLock.writeLock().lockInterruptibly();
224238
try {
225239
boolean retrieve = false;
226240
switch (getCacheStatus(cachingConfiguration, versionCacheDir)) {
227241
case VALID:
228-
listener.getLogger().println("Library " + name + "@" + version + " is cached. Copying from home.");
229-
break;
242+
listener.getLogger().println("Library " + libraryLogString + " is cached. Copying from home.");
243+
break;
244+
case EMPTY:
245+
listener.getLogger().println("Library " + libraryLogString + " should have been cached but is empty, re-caching.");
246+
deleteCacheDirIfExists(versionCacheDir);
247+
retrieve = true;
248+
break;
230249
case DOES_NOT_EXIST:
231250
retrieve = true;
232251
break;
233252
case EXPIRED:
234253
long cachingMinutes = cachingConfiguration.getRefreshTimeMinutes();
235-
listener.getLogger().println("Library " + name + "@" + version + " is due for a refresh after " + cachingMinutes + " minutes, clearing.");
236-
if (versionCacheDir.exists()) {
237-
versionCacheDir.deleteRecursive();
238-
versionCacheDir.withSuffix("-name.txt").delete();
239-
}
240-
retrieve = true;
241-
break;
254+
listener.getLogger().println("Library " + libraryLogString + " is due for a refresh after " + cachingMinutes + " minutes, clearing.");
255+
deleteCacheDirIfExists(versionCacheDir);
256+
retrieve = true;
257+
break;
242258
}
243259

244260
if (retrieve) {
245-
listener.getLogger().println("Caching library " + name + "@" + version);
261+
listener.getLogger().println("Caching library " + libraryLogString);
246262
versionCacheDir.mkdirs();
247263
// try to retrieve the library and delete the versionCacheDir if it fails
248264
try {
249265
retriever.retrieve(name, version, changelog, versionCacheDir, run, listener);
266+
if (getUrlsForLibDir(versionCacheDir).isEmpty()) {
267+
// Get job name and build number from run
268+
String jobName = run.getParent().getFullName();
269+
String message = "Library " + libraryLogString + " is empty after retrieval in job " + jobName + ". Cleaning up cache directory.";
270+
listener.getLogger().println(message);
271+
// Log a warning at controller level as well
272+
LOGGER.log(Level.WARNING, message);
273+
deleteCacheDirIfExists(versionCacheDir);
274+
throw new AbortException("Library " + libraryLogString + " is empty.");
275+
}
276+
listener.getLogger().println("Library " + libraryLogString + " successfully cached.");
250277
} catch (Exception e) {
251-
listener.getLogger().println("Failed to cache library " + name + "@" + version + ". Error message: " + e.getMessage() + ". Cleaning up cache directory.");
252-
versionCacheDir.deleteRecursive();
278+
listener.getLogger().println("Failed to cache library " + libraryLogString + ". Error message: " + e.getMessage() + ". Cleaning up cache directory.");
279+
deleteCacheDirIfExists(versionCacheDir);
253280
throw e;
254281
}
255282
}
@@ -258,7 +285,7 @@ static List<URL> retrieve(@NonNull LibraryRecord record, @NonNull LibraryRetriev
258285
retrieveLock.writeLock().unlock();
259286
}
260287
} else {
261-
listener.getLogger().println("Library " + name + "@" + version + " is cached. Copying from home.");
288+
listener.getLogger().println("Library " + libraryLogString + " is cached. Copying from home.");
262289
}
263290

264291
lastReadFile.touch(System.currentTimeMillis());
@@ -289,6 +316,25 @@ static List<URL> retrieve(@NonNull LibraryRecord record, @NonNull LibraryRetriev
289316
}
290317
}
291318
}
319+
List<URL> urls = getUrlsForLibDir(libDir, record);
320+
if (urls.isEmpty()) {
321+
throw new AbortException("Library " + name + " expected to contain at least one of src or vars directories");
322+
}
323+
return urls;
324+
}
325+
326+
private static void deleteCacheDirIfExists(FilePath versionCacheDir) throws IOException, InterruptedException {
327+
if (versionCacheDir.exists()) {
328+
versionCacheDir.deleteRecursive();
329+
versionCacheDir.withSuffix("-name.txt").delete();
330+
}
331+
}
332+
333+
private static List<URL> getUrlsForLibDir(FilePath libDir) throws MalformedURLException, IOException, InterruptedException {
334+
return getUrlsForLibDir(libDir, null);
335+
}
336+
337+
private static List<URL> getUrlsForLibDir(FilePath libDir, LibraryRecord record) throws MalformedURLException, IOException, InterruptedException {
292338
List<URL> urls = new ArrayList<>();
293339
FilePath srcDir = libDir.child("src");
294340
if (srcDir.isDirectory()) {
@@ -297,13 +343,12 @@ static List<URL> retrieve(@NonNull LibraryRecord record, @NonNull LibraryRetriev
297343
FilePath varsDir = libDir.child("vars");
298344
if (varsDir.isDirectory()) {
299345
urls.add(varsDir.toURI().toURL());
300-
for (FilePath var : varsDir.list("*.groovy")) {
301-
record.variables.add(var.getBaseName());
346+
if (record != null) {
347+
for (FilePath var : varsDir.list("*.groovy")) {
348+
record.variables.add(var.getBaseName());
349+
}
302350
}
303351
}
304-
if (urls.isEmpty()) {
305-
throw new AbortException("Library " + name + " expected to contain at least one of src or vars directories");
306-
}
307352
return urls;
308353
}
309354

‎src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryRecord.java‎

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,13 +24,18 @@
2424

2525
package org.jenkinsci.plugins.workflow.libs;
2626

27+
import java.nio.file.Paths;
2728
import java.util.Collections;
2829
import java.util.Set;
2930
import java.util.TreeSet;
3031
import jenkins.security.HMACConfidentialKey;
32+
33+
import org.apache.commons.lang.StringUtils;
3134
import org.kohsuke.stapler.export.Exported;
3235
import org.kohsuke.stapler.export.ExportedBean;
3336

37+
import edu.umd.cs.findbugs.annotations.CheckForNull;
38+
3439
/**
3540
* Record of a library being used in a particular build.
3641
*/
@@ -46,6 +51,8 @@ public final class LibraryRecord {
4651
final boolean trusted;
4752
final boolean changelog;
4853
final LibraryCachingConfiguration cachingConfiguration;
54+
final String libraryPath;
55+
private String logString;
4956
private String directoryName;
5057

5158
/**
@@ -54,15 +61,33 @@ public final class LibraryRecord {
5461
* @param trusted Whether the library is trusted. Typically determined by {@link LibraryResolver#isTrusted}, but see also {@link LibraryStep}.
5562
* @param changelog Whether we should include any SCM changes in this library in the build's changelog.
5663
* @param cachingConfiguration If non-null, contains cache-related configuration.
64+
* @param libraryPath The path to the library within the repository. Used in {@link SCMBasedRetriever}. If null, the library is assumed to be at the root of the repository.
5765
* @param source A string describing the source of the configuration of this library. Typically the class name of a {@link LibraryResolver}, sometimes with additional data, but see also {@link LibraryStep}.
5866
*/
59-
LibraryRecord(String name, String version, boolean trusted, boolean changelog, LibraryCachingConfiguration cachingConfiguration, String source) {
67+
LibraryRecord(String name, String version, boolean trusted, boolean changelog, LibraryCachingConfiguration cachingConfiguration, String source, @CheckForNull String libraryPath) {
6068
this.name = name;
6169
this.version = version;
6270
this.trusted = trusted;
6371
this.changelog = changelog;
6472
this.cachingConfiguration = cachingConfiguration;
65-
this.directoryName = directoryNameFor(name, version, String.valueOf(trusted), source);
73+
logString = this.name + "@" + this.version;
74+
if (onTheRoadToNowhere(libraryPath)) {
75+
this.libraryPath = "";
76+
this.directoryName = directoryNameFor(name, version, String.valueOf(trusted), source);
77+
} else {
78+
this.libraryPath = libraryPath;
79+
this.directoryName = directoryNameFor(name, version, String.valueOf(trusted), source, libraryPath);
80+
logString += ":" + libraryPath;
81+
}
82+
}
83+
84+
private boolean onTheRoadToNowhere(String libraryPath) {
85+
if (StringUtils.isBlank(libraryPath)) {
86+
return true;
87+
}
88+
String currentDir = Paths.get("").toAbsolutePath().normalize().toString();
89+
String libraryDir = Paths.get(libraryPath).toAbsolutePath().normalize().toString();
90+
return currentDir.equals(libraryDir);
6691
}
6792

6893
@Exported
@@ -80,6 +105,13 @@ public String getDirectoryName() {
80105
return directoryName;
81106
}
82107

108+
/**
109+
* Returns a string representation to be used for example in the {@link LibraryAdder}.
110+
*/
111+
public String getLogString() {
112+
return logString;
113+
}
114+
83115
@Exported
84116
public String getVersion() {
85117
return version;

‎src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryStep.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,11 @@ else if (retriever instanceof SCMRetriever) {
218218
verifyRevision(((SingleSCMSource) ((SCMSourceRetriever) retriever).getScm()).getScm(), name, run, listener);
219219
}
220220

221-
LibraryRecord record = new LibraryRecord(name, version, trusted, changelog, cachingConfiguration, source);
221+
String libraryPath = null;
222+
if (retriever instanceof SCMBasedRetriever) {
223+
libraryPath = ((SCMBasedRetriever) retriever).getLibraryPath();
224+
}
225+
LibraryRecord record = new LibraryRecord(name, version, trusted, changelog, cachingConfiguration, source, libraryPath);
222226
LibrariesAction action = run.getAction(LibrariesAction.class);
223227
if (action == null) {
224228
action = new LibrariesAction(Lists.newArrayList(record));

0 commit comments

Comments
 (0)