-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Add all artifacts to maintainers and issue lists when wildcard is used #4108
Changes from all commits
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 |
---|---|---|
|
@@ -173,43 +173,49 @@ class ArtifactoryPermissionsUpdater { | |
|
||
if (definition.issues) { | ||
if (definition.github) { | ||
issueTrackersByPlugin.put(definition.name, definition.issues.collect { tracker -> | ||
if (tracker.isJira() || tracker.isGitHubIssues()) { | ||
def ret = [type: tracker.getType(), reference: tracker.getReference()] | ||
def viewUrl = tracker.getViewUrl() | ||
if (viewUrl) { | ||
ret += [ viewUrl: viewUrl ] | ||
} | ||
def reportUrl = tracker.getReportUrl() | ||
if (reportUrl) { | ||
ret += [ reportUrl: reportUrl ] | ||
ArrayList<String> names = new ArrayList(Arrays.asList(definition.getExtraNames())) | ||
names.add(definition.name) | ||
for (String name: names) { | ||
issueTrackersByPlugin.put(name, definition.issues.collect { tracker -> | ||
if (tracker.isJira() || tracker.isGitHubIssues()) { | ||
def ret = [type: tracker.getType(), reference: tracker.getReference()] | ||
def viewUrl = tracker.getViewUrl() | ||
if (viewUrl) { | ||
ret += [viewUrl: viewUrl] | ||
} | ||
def reportUrl = tracker.getReportUrl() | ||
if (reportUrl) { | ||
ret += [reportUrl: reportUrl] | ||
} | ||
return ret | ||
} | ||
return ret | ||
} | ||
return null | ||
}.findAll { it != null }) | ||
return null | ||
}.findAll { it != null }) | ||
} | ||
} else { | ||
throw new Exception("Issue trackers ('issues') support requires GitHub repository ('github')") | ||
} | ||
} | ||
|
||
String artifactId = definition.name | ||
for (String path : definition.paths) { | ||
if (path.substring(path.lastIndexOf('/') + 1) != artifactId) { | ||
String lastPathElement = path.substring(path.lastIndexOf('/') + 1) | ||
if (lastPathElement != artifactId && !lastPathElement.contains("*")) { | ||
// We could throw an exception here, but we actively abuse this for unusually structured components | ||
LOGGER.log(Level.WARNING, "Unexpected path: " + path + " for artifact ID: " + artifactId) | ||
if (lastPathElement.endsWith("-releaseblocker") || lastPathElement.endsWith("-releaseblock")) { | ||
LOGGER.log(Level.INFO, "Release blocked for artifact ID: " + artifactId) | ||
} else { | ||
LOGGER.log(Level.WARNING, "Unexpected path: " + path + " for artifact ID: " + artifactId) | ||
} | ||
} | ||
String groupId = path.substring(0, path.lastIndexOf('/')).replace('/', '.') | ||
|
||
String key = groupId + ":" + artifactId | ||
|
||
if (maintainersByComponent.containsKey(key)) { | ||
LOGGER.log(Level.WARNING, "Duplicate maintainers entry for component: " + key) | ||
if (lastPathElement.contains("*")) { | ||
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. Seems unnecessarily complicated, because |
||
for (String name: definition.getExtraNames()) { | ||
addMaintainers(maintainersByComponent, groupId + ":" + name, definition) | ||
} | ||
} else { | ||
addMaintainers(maintainersByComponent, groupId + ":" + artifactId, definition) | ||
} | ||
// A potential issue with this implementation is that groupId changes will result in lack of maintainer information for the old groupId. | ||
// In practice this will probably not be a problem when path changes here and subsequent release are close enough in time. | ||
// Alternatively, always keep the old groupId around for a while. | ||
maintainersByComponent.computeIfAbsent(key, { _ -> new ArrayList<>(Arrays.asList(definition.developers)) }) | ||
} | ||
|
||
String fileBaseName = file.name.replaceAll('\\.ya?ml$', '') | ||
|
@@ -358,6 +364,16 @@ class ArtifactoryPermissionsUpdater { | |
new File('checks-details.txt').text = details | ||
} | ||
|
||
private static void addMaintainers(HashMap<String, List<String>> maintainersByComponent, String key, Definition definition) { | ||
if (maintainersByComponent.containsKey(key)) { | ||
LOGGER.log(Level.WARNING, "Duplicate maintainers entry for component: " + key) | ||
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. Interestingly, we have some of these in the log. Anyone want to look into this? |
||
} | ||
// A potential issue with this implementation is that groupId changes will result in lack of maintainer information for the old groupId. | ||
// In practice this will probably not be a problem when path changes here and subsequent release are close enough in time. | ||
// Alternatively, always keep the old groupId around for a while. | ||
maintainersByComponent.computeIfAbsent(key, { _ -> new ArrayList<>(Arrays.asList(definition.developers)) }) | ||
} | ||
|
||
/** | ||
* Takes a directory with Artifactory API payloads and submits them to the appropriate Artifactory API, | ||
* creating/updating the specified objects identified through the file name. | ||
|
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.
Rather than add special handling for a weakly adhered to naming convention, if we add code for this case, it should just be a new attribute.
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.
Done in #4127