Skip to content

Commit

Permalink
fetch_all.py improvements
Browse files Browse the repository at this point in the history 
* Use "Short Name" as fallback for "Name"
* Download files concurrently
* Generate .info files concurrently
* Increase thread count for CIPD
* Improve logging around missing licenses (and adds a missing one)
* Remove less-used (never-used?) flags
* Make --update-all be the default
* Always overwrite all files (crbug/1078529)
* Don't clobber non-committed local modifications to BUILD.gn
* Show gradle output (so it's clear things are happening)

The script now runs in < 10 seconds (for a no-op) on my machine.

Bug: 1078529
Change-Id: I4eefb7e1442d647854ffa6bbedbf371382fa3cd8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2218561
Commit-Queue: Andrew Grieve <agrieve@chromium.org>
Reviewed-by: Peter Wen <wnwen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#772393}
  • Loading branch information
@agrieve
agrieve authored and Commit Bot committed May 27, 2020
1 parent 53a68dd commit 9e82839
Show file tree
Hide file tree
Showing 11 changed files with 331 additions and 212 deletions.
32 changes: 8 additions & 24 deletions third_party/android_deps/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,20 +27,16 @@ Full steps to add a new third party library:

1. Add dependency to `build.gradle`

2. Run `fetch_all.py` to verify the new `build.gradle` file and that all
dependencies could be downloaded properly. This does not modify your
workspace. If not, fix your `build.gradle` file and try again.

3. Run `fetch_all.py --update-all` to update your current workspace with the
changes. This will update, among other things, your top-level DEPS file,
and print a series of commands to create new CIPD packages.
2. Run `fetch_all.py` to update your current workspace with the changes. This
will update, among other things, your top-level DEPS file, and print a series
of commands to create new CIPD packages.
- Every package in CIPD has to have a unique tag other wise it will cause
problems. The cipd commands output by `fetch_all.py --update-all` already
check for uniqueness of the tag before uploading a new version. You can
also supply a new suffix in your package using FALLBACK_PROPERTIES in
problems. The cipd commands output by `fetch_all.py` already check for
uniqueness of the tag before uploading a new version. You can also supply
a new suffix in your package using PROPERTY_OVERRIDES in
third_party/android_deps/buildSrc/src/main/groovy/ChromiumDepGraph.goovy

4. Run the commands printed at step 3 to create new and updated packages
3. Run the commands printed at step 3 to create new and updated packages
via cipd.
- In order to do this, you will need write access. Ask anyone from this
[list][owners_link] to add you to the group.
Expand All @@ -49,23 +45,11 @@ Full steps to add a new third party library:
can break all builds, and there is no easy way to delete an instance.
- If this happens, file an infra-trooper bug immediately.

5. Thoroughly test your change on a clean checkout.
- Run the following command:
`rm -rf third_party/android_deps/libs/[!O]* && third_party/android_deps/fetch_all.py --update-all`.
- This ensures that all your deps are fresh. You do not need to run the
commands printed out in this step.

5. Create a commit & follow [`//docs/adding_to_third_party.md`][docs_link] for
4. Create a commit & follow [`//docs/adding_to_third_party.md`][docs_link] for
the review.
- This is not necessary if you are only upgrading existing packages or
adding packages from the same source and license (e.g. gms)

Note that if you're not satisfied with the results, you can use
`fetch_all.py --reset-workspace` to reset your workspace to its HEAD state,
including the original CIPD symlinks under
`third_party/android_deps/repository/`. This commands preserves local
`build.gradle` modifications.

If you are updating any of the gms dependencies, please ensure that the license
file that they use, explained in the [README.chromium][readme_chromium_link] is
up-to-date with the one on android's [website][android_sdk_link], last updated
Expand Down
20 changes: 15 additions & 5 deletions third_party/android_deps/buildSrc/src/main/groovy/BuildConfigGenerator.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import org.gradle.api.DefaultTask
import org.gradle.api.tasks.TaskAction

import java.util.regex.Pattern
import java.util.concurrent.Executors
import java.util.concurrent.TimeUnit

/**
* Task to download dependencies specified in {@link ChromiumPlugin} and configure the
Expand Down Expand Up @@ -98,6 +100,7 @@ class BuildConfigGenerator extends DefaultTask {

// 2. Import artifacts into the local repository
def dependencyDirectories = []
def downloadExecutor = Executors.newCachedThreadPool()
graph.dependencies.values().each { dependency ->
if (excludeDependency(dependency, onlyPlayServices)) {
return
Expand Down Expand Up @@ -129,14 +132,21 @@ class BuildConfigGenerator extends DefaultTask {
new File("${normalisedRepoPath}/${dependency.licensePath}").text)
} else if (!dependency.licenseUrl?.trim()?.isEmpty()) {
File destFile = new File("${absoluteDepDir}/LICENSE")
downloadFile(dependency.id, dependency.licenseUrl, destFile)
if (destFile.text.contains("<html")) {
throw new RuntimeException("Found HTML in LICENSE file. Please add an "
+ "override to ChromiumDepGraph.groovy for ${dependency.id}.")
downloadExecutor.submit {
downloadFile(dependency.id, dependency.licenseUrl, destFile)
if (destFile.text.contains("<html")) {
throw new RuntimeException("Found HTML in LICENSE file. Please add an "
+ "override to ChromiumDepGraph.groovy for ${dependency.id}.")
}
}
} else {
getLogger().warn("Missing license for ${dependency.id}.")
getLogger().warn("License Name was: ${dependency.licenseName}")
}
}
}
downloadExecutor.shutdown()
downloadExecutor.awaitTermination(Long.MAX_VALUE, TimeUnit.NANOSECONDS);

// 3. Generate the root level build files
updateBuildTargetDeclaration(graph, "${normalisedRepoPath}/BUILD.gn", onlyPlayServices)
Expand Down Expand Up @@ -583,7 +593,7 @@ class BuildConfigGenerator extends DefaultTask {
if (sourceUrl.contains("://opensource.org/licenses")) {
throw new RuntimeException("Found templated license URL for dependency "
+ id + ": " + sourceUrl
+ ". You will need to edit FALLBACK_PROPERTIES for this dep.")
+ ". You will need to edit PROPERTY_OVERRIDES for this dep.")
}
connection = urlObj.openConnection()
switch (connection.getResponseCode()) {
Expand Down
34 changes: 18 additions & 16 deletions third_party/android_deps/buildSrc/src/main/groovy/ChromiumDepGraph.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class ChromiumDepGraph {
// Some libraries don't properly fill their POM with the appropriate licensing information.
// It is provided here from manual lookups. Note that licenseUrl must provide textual content
// rather than be an html page.
final def FALLBACK_PROPERTIES = [
final def PROPERTY_OVERRIDES = [
'androidx_multidex_multidex': new PropertyOverride(
url: 'https://maven.google.com/androidx/multidex/multidex/2.0.0/multidex-2.0.0.aar'),
'com_android_tools_desugar_jdk_libs': new PropertyOverride(
Expand Down Expand Up @@ -189,7 +189,8 @@ class ChromiumDepGraph {
licenseUrl: "https://raw.githubusercontent.com/google-ar/arcore-android-sdk/master/LICENSE",
licenseName: "Apache 2.0"),
'commons_cli_commons_cli': new PropertyOverride(
licenseName: "Apache 2.0"),
licenseName: "Apache 2.0",
licenseUrl: "https://raw.githubusercontent.com/apache/commons-cli/master/LICENSE.txt"),
'javax_annotation_javax_annotation_api': new PropertyOverride(
isShipped: false, // Annotations are stripped by R8.
licenseName: "CDDLv1.1",
Expand Down Expand Up @@ -325,13 +326,13 @@ class ChromiumDepGraph {
dep.isShipped = true
}

FALLBACK_PROPERTIES.each { id, fallbackProperties ->
PROPERTY_OVERRIDES.each { id, fallbackProperties ->
if (fallbackProperties?.isShipped != null) {
def dep = dependencies.get(id)
if (dep != null) {
dep.isShipped = fallbackProperties.isShipped
} else {
project.logger.warn("FALLBACK_PROPERTIES has stale dep: " + id)
project.logger.warn("PROPERTY_OVERRIDES has stale dep: " + id)
}
}
}
Expand Down Expand Up @@ -409,6 +410,10 @@ class ChromiumDepGraph {

// Get rid of irrelevant indent that might be present in the XML file.
def description = pomContent.description?.text()?.trim()?.replaceAll(/\s+/, " ")
def displayName = pomContent.name?.text()
if (!displayName) {
displayName = dependency.module.id.name
}

return customizeDep(new DependencyDescription(
id: id,
Expand All @@ -425,7 +430,7 @@ class ChromiumDepGraph {
fileName: artifact.file.name,
description: description,
url: pomContent.url?.text(),
displayName: pomContent.name?.text(),
displayName: displayName,
exclude: false,
cipdSuffix: "cr0",
))
Expand All @@ -448,7 +453,7 @@ class ChromiumDepGraph {
dep.licensePath = "licenses/GNU_v2_with_Classpath_Exception_1991.txt"
}

def fallbackProperties = FALLBACK_PROPERTIES.get(dep.id)
def fallbackProperties = PROPERTY_OVERRIDES.get(dep.id)
if (fallbackProperties != null) {
project.logger.debug("Using fallback properties for ${dep.id}")
if (fallbackProperties.licenseName != null) {
Expand Down Expand Up @@ -487,21 +492,18 @@ class ChromiumDepGraph {
}

private resolveLicenseInformation(String id, GPathResult pomContent) {
def licenseName = ''
def licenseUrl = ''

def error = ''
GPathResult licenses = pomContent?.licenses?.license
if (!licenses || licenses.size() == 0) {
error = "No license found on ${id}"
return ["License Missing Error", ""]
} else if (licenses.size() > 1) {
error = "More than one license found on ${id}"
def allUrls = ''
for (def license : licenses) {
allUrls += license.url.text() + " "
}
return ["Multiple Licenses Error: ${allUrls}", ""]
}

if (error.isEmpty()) return [licenses[0].name.text(), licenses[0].url.text()]

project.logger.warn(error)
return ['', '']
return [licenses[0].name.text(), licenses[0].url.text()]
}

static class DependencyDescription {
Expand Down
Loading

0 comments on commit 9e82839

Please sign in to comment.