Dump current AIDL API and make sure that it is always updated
Previously, the API directory (which is usually ./api directory) was
updated only when the new version of the AIDL interface is created.
This was not ideal as 1) comparing the differences since the last
version is difficult (because the new dump is stored in a new
directory), and 2) the review is done quite late in time.
This change makes the AIDL API review process be similar to that of
regular Java APIs. Whenever an AIDL interface is updated, it is compared
against the 'current' dump of the interface. Whenever there is a
difference, users are instructed to update the current dump by invoking
`m <name>-update-api`. Since this will update the api directory, the
change has to be reviewed by the AIDL API reviewers. The current can be
frozen to a new version by invoking `m <name>-freeze-api` as before.
Bug: 147433177
Test: m
Test: m test-piece-1-update-api; api_dir/current is created
Test: m test-piece-1-freeze-api; api_dir/4 is created and Android.bp is
updated
Test: modify IFoo.aidl and run m. The build fails with the error message
suggesting to run `m test-piece-1-update-api`
Merged-In: I1e305027311cb065f872e4a11f62bcf95629a1df
(cherry picked from commit 5d9c28bfebdbdc9f7916e036c8be087ccc6f13ab)
Change-Id: I1e305027311cb065f872e4a11f62bcf95629a1df
diff --git a/build/aidl_interface.go b/build/aidl_interface.go
index 435d9bd..fa62d2a 100644
--- a/build/aidl_interface.go
+++ b/build/aidl_interface.go
@@ -33,7 +33,7 @@
"github.com/google/blueprint/proptools"
)
-var (
+const (
aidlInterfaceSuffix = "_interface"
aidlMetadataSingletonName = "aidl_metadata_json"
aidlApiDir = "aidl_api"
@@ -43,6 +43,10 @@
langNdk = "ndk"
langNdkPlatform = "ndk_platform"
+ currentVersion = "current"
+)
+
+var (
pctx = android.NewPackageContext("android/aidl")
aidlDirPrepareRule = pctx.StaticRule("aidlDirPrepareRule", blueprint.RuleParams{
@@ -96,17 +100,6 @@
CommandDeps: []string{"${aidlCmd}"},
}, "imports", "outDir")
- aidlFreezeApiRule = pctx.AndroidStaticRule("aidlFreezeApiRule",
- blueprint.RuleParams{
- Command: `mkdir -p ${to} && rm -rf ${to}/* && ` +
- `${bpmodifyCmd} -w -m ${name} -parameter versions -a ${version} ${bp} && ` +
- `cp -rf ${apiDir}/. ${to} && ` +
- `find ${to} -type f -name "*.aidl" | xargs -n 1 bash -c ` +
- `'cat ${apiPreamble} $$0 > $$0.temp && mv $$0.temp $$0' && ` +
- `touch ${out}`,
- CommandDeps: []string{"${bpmodifyCmd}"},
- }, "to", "name", "version", "bp", "apiDir", "apiPreamble")
-
aidlCheckApiRule = pctx.StaticRule("aidlCheckApiRule", blueprint.RuleParams{
Command: `(${aidlCmd} ${optionalFlags} --checkapi ${old} ${new} && touch ${out}) || ` +
`(cat ${messageFile} && exit 1)`,
@@ -138,7 +131,6 @@
func init() {
pctx.HostBinToolVariable("aidlCmd", "aidl")
- pctx.HostBinToolVariable("bpmodifyCmd", "bpmodify")
pctx.SourcePathVariable("aidlToJniCmd", "system/tools/aidl/build/aidl_to_jni.py")
android.RegisterModuleType("aidl_interface", aidlInterfaceFactory)
android.RegisterModuleType("aidl_mapping", aidlMappingFactory)
@@ -429,6 +421,9 @@
// for triggering api check for version X against version X-1
checkApiTimestamps android.WritablePaths
+ // for triggering updating current API
+ updateApiTimestamp android.WritablePath
+
// for triggering freezing API as the new version
freezeApiTimestamp android.WritablePath
}
@@ -437,8 +432,8 @@
return filepath.Join(aidlApiDir, m.properties.BaseName)
}
-// Version of the interface at ToT if it is frozen
-func (m *aidlApi) validateCurrentVersion(ctx android.ModuleContext) string {
+// `m <iface>-freeze-api` will freeze ToT as this version
+func (m *aidlApi) nextVersion(ctx android.ModuleContext) string {
if len(m.properties.Versions) == 0 {
return "1"
} else {
@@ -446,7 +441,7 @@
i, err := strconv.Atoi(latestVersion)
if err != nil {
- ctx.PropertyErrorf("versions", "must be integers")
+ ctx.PropertyErrorf("versions", "%q is not an integer", latestVersion)
return ""
}
@@ -454,11 +449,17 @@
}
}
-func (m *aidlApi) createApiDumpFromSource(ctx android.ModuleContext) (apiDir android.WritablePath, apiFiles android.WritablePaths, hashFile android.WritablePath) {
+type apiDump struct {
+ dir android.Path
+ files android.Paths
+ hashFile android.OptionalPath
+}
+
+func (m *aidlApi) createApiDumpFromSource(ctx android.ModuleContext) apiDump {
srcs, imports := getPaths(ctx, m.properties.Srcs)
if ctx.Failed() {
- return
+ return apiDump{}
}
var importPaths []string
@@ -469,6 +470,10 @@
}
})
+ var apiDir android.WritablePath
+ var apiFiles android.WritablePaths
+ var hashFile android.WritablePath
+
apiDir = android.PathForModuleOut(ctx, "dump")
aidlRoot := android.PathForModuleSrc(ctx, m.properties.AidlRoot)
for _, src := range srcs {
@@ -500,39 +505,41 @@
"latestVersion": latestVersion,
},
})
- return apiDir, apiFiles, hashFile
+ return apiDump{apiDir, apiFiles.Paths(), android.OptionalPathForPath(hashFile)}
}
-func (m *aidlApi) freezeApiDumpAsVersion(ctx android.ModuleContext, apiDumpDir android.Path, apiFiles android.Paths, version string) android.WritablePath {
- timestampFile := android.PathForModuleOut(ctx, "freezeapi_"+version+".timestamp")
+func (m *aidlApi) makeApiDumpAsVersion(ctx android.ModuleContext, dump apiDump, version string) android.WritablePath {
+ timestampFile := android.PathForModuleOut(ctx, "updateapi_"+version+".timestamp")
modulePath := android.PathForModuleSrc(ctx).String()
-
- var implicits android.Paths
- implicits = append(implicits, apiFiles...)
-
apiPreamble := android.PathForSource(ctx, "system/tools/aidl/build/api_preamble.txt")
- implicits = append(implicits, apiPreamble)
- ctx.ModuleBuild(pctx, android.ModuleBuildParams{
- Rule: aidlFreezeApiRule,
- Description: "Freezing AIDL API of " + m.properties.BaseName + " as version " + version,
- Implicits: implicits,
- Output: timestampFile,
- Args: map[string]string{
- "to": filepath.Join(modulePath, m.apiDir(), version),
- "apiDir": apiDumpDir.String(),
- "name": m.properties.BaseName,
- "version": version,
- "bp": android.PathForModuleSrc(ctx, "Android.bp").String(),
- "apiPreamble": apiPreamble.String(),
- },
- })
+ targetDir := filepath.Join(modulePath, m.apiDir(), version)
+ rb := android.NewRuleBuilder()
+ // Wipe the target directory and then copy the API dump into the directory
+ rb.Command().Text("mkdir -p " + targetDir)
+ rb.Command().Text("rm -rf " + targetDir + "/*")
+ rb.Command().Text("cp -rf " + dump.dir.String() + "/. " + targetDir).Implicits(dump.files)
+ if version != currentVersion {
+ // If this is making a new frozen (i.e. non-current) version of the interface,
+ // Place a preamble to prevent people from accidentally modifying the dumps.
+ // And also modify Android.bp file to add the new version to the 'versions' property.
+ rb.Command().Text("find " + targetDir + " -type f -name \"*.aidl\"").
+ Text("| xargs -n 1 bash -c 'cat ").Input(apiPreamble).Text(" $0 > $0.temp && mv $0.temp $0'")
+ rb.Command().BuiltTool(ctx, "bpmodify").
+ Text("-w -m " + m.properties.BaseName).
+ Text("-parameter versions -a " + version).
+ Text(android.PathForModuleSrc(ctx, "Android.bp").String())
+ }
+ rb.Command().Text("touch").Output(timestampFile)
+
+ rb.Build(pctx, ctx, "dump_aidl_api"+m.properties.BaseName+"_"+version,
+ "Making AIDL API of "+m.properties.BaseName+" as version "+version)
return timestampFile
}
-func (m *aidlApi) checkCompatibility(ctx android.ModuleContext, oldApiDir android.Path, oldApiFiles android.Paths, newApiDir android.Path, newApiFiles android.Paths) android.WritablePath {
- newVersion := newApiDir.Base()
+func (m *aidlApi) checkCompatibility(ctx android.ModuleContext, oldDump apiDump, newDump apiDump) android.WritablePath {
+ newVersion := newDump.dir.Base()
timestampFile := android.PathForModuleOut(ctx, "checkapi_"+newVersion+".timestamp")
messageFile := android.PathForSource(ctx, "system/tools/aidl/build/message_check_compatibility.txt")
@@ -542,8 +549,8 @@
}
var implicits android.Paths
- implicits = append(implicits, oldApiFiles...)
- implicits = append(implicits, newApiFiles...)
+ implicits = append(implicits, oldDump.files...)
+ implicits = append(implicits, newDump.files...)
implicits = append(implicits, messageFile)
ctx.ModuleBuild(pctx, android.ModuleBuildParams{
Rule: aidlCheckApiRule,
@@ -551,87 +558,118 @@
Output: timestampFile,
Args: map[string]string{
"optionalFlags": strings.Join(optionalFlags, " "),
- "old": oldApiDir.String(),
- "new": newApiDir.String(),
+ "old": oldDump.dir.String(),
+ "new": newDump.dir.String(),
"messageFile": messageFile.String(),
},
})
return timestampFile
}
-func (m *aidlApi) checkEquality(ctx android.ModuleContext, oldApiDir android.Path, oldApiFiles android.Paths, oldHashFile android.OptionalPath,
- newApiDir android.Path, newApiFiles android.Paths, newHashFile android.Path) android.WritablePath {
- newVersion := newApiDir.Base()
+func (m *aidlApi) checkEquality(ctx android.ModuleContext, oldDump apiDump, newDump apiDump) android.WritablePath {
+ newVersion := newDump.dir.Base()
timestampFile := android.PathForModuleOut(ctx, "checkapi_"+newVersion+".timestamp")
+
+ // Use different messages depending on whether platform SDK is finalized or not.
+ // In case when it is finalized, we should never allow updating the already frozen API.
+ // If it's not finalized, we let users to update the current version by invoking
+ // `m <name>-update-api`.
messageFile := android.PathForSource(ctx, "system/tools/aidl/build/message_check_equality.txt")
- var implicits android.Paths
- implicits = append(implicits, oldApiFiles...)
- implicits = append(implicits, newApiFiles...)
- implicits = append(implicits, messageFile)
- if oldHashFile.Valid() {
- implicits = append(implicits, oldHashFile.Path())
+ sdkIsFinal := ctx.Config().DefaultAppTargetSdkInt() != android.FutureApiLevel
+ if sdkIsFinal {
+ messageFile = android.PathForSource(ctx, "system/tools/aidl/build/message_check_equality_release.txt")
}
- implicits = append(implicits, newHashFile)
+ formattedMessageFile := android.PathForModuleOut(ctx, "message_check_equality.txt")
+ rb := android.NewRuleBuilder()
+ rb.Command().Text("sed").Flag(" s/%s/" + m.properties.BaseName + "/ ").Input(messageFile).Text(" > ").Output(formattedMessageFile)
+ rb.Build(pctx, ctx, "format_message_"+m.properties.BaseName, "")
+
+ var implicits android.Paths
+ implicits = append(implicits, oldDump.files...)
+ implicits = append(implicits, newDump.files...)
+ implicits = append(implicits, formattedMessageFile)
+ if oldDump.hashFile.Valid() {
+ implicits = append(implicits, oldDump.hashFile.Path())
+ }
+ implicits = append(implicits, newDump.hashFile.Path())
ctx.ModuleBuild(pctx, android.ModuleBuildParams{
Rule: aidlDiffApiRule,
Implicits: implicits,
Output: timestampFile,
Args: map[string]string{
- "old": oldApiDir.String(),
- "new": newApiDir.String(),
- "messageFile": messageFile.String(),
- "oldHashFile": oldHashFile.String(),
- "newHashFile": newHashFile.String(),
+ "old": oldDump.dir.String(),
+ "new": newDump.dir.String(),
+ "messageFile": formattedMessageFile.String(),
+ "oldHashFile": oldDump.hashFile.String(),
+ "newHashFile": newDump.hashFile.String(),
},
})
return timestampFile
}
func (m *aidlApi) GenerateAndroidBuildActions(ctx android.ModuleContext) {
- currentVersion := m.validateCurrentVersion(ctx)
- currentDumpDir, currentApiFiles, currentHashFile := m.createApiDumpFromSource(ctx)
-
- if ctx.Failed() {
- return
+ // An API dump is created from source and it is compared against the API dump of the
+ // 'current' (yet-to-be-finalized) version. By checking this we enforce that any change in
+ // the AIDL interface is gated by the AIDL API review even before the interface is frozen as
+ // a new version.
+ totApiDump := m.createApiDumpFromSource(ctx)
+ currentApiDir := android.ExistentPathForSource(ctx, ctx.ModuleDir(), m.apiDir(), currentVersion)
+ var currentApiDump apiDump
+ if currentApiDir.Valid() {
+ currentApiDump = apiDump{
+ dir: currentApiDir.Path(),
+ files: ctx.Glob(filepath.Join(currentApiDir.Path().String(), "**/*.aidl"), nil),
+ hashFile: android.ExistentPathForSource(ctx, ctx.ModuleDir(), m.apiDir(), currentVersion, ".hash"),
+ }
+ checked := m.checkEquality(ctx, currentApiDump, totApiDump)
+ m.checkApiTimestamps = append(m.checkApiTimestamps, checked)
+ } else {
+ // The "current" directory might not exist, in case when the interface is first created or
+ // the interface is not versioned.
+ // For the former case, instruct user to create one by executing `m <name>-update-api`.
+ // For the latter case, don't bother.
+ if len(m.properties.Versions) > 0 {
+ rb := android.NewRuleBuilder()
+ rb.Command().Text(fmt.Sprintf(`echo "API dump for the current version of AIDL intercace %s does not exist."`, ctx.ModuleName()))
+ rb.Command().Text(fmt.Sprintf(`echo "Run m %s-update-api"`, m.properties.BaseName))
+ // This file will never be created. Otherwise, the build will pass simply by running 'm; m'.
+ alwaysChecked := android.PathForModuleOut(ctx, "checkapi_current.timestamp")
+ rb.Command().Text("false").ImplicitOutput(alwaysChecked)
+ rb.Build(pctx, ctx, "check_current_aidl_api", "")
+ // TODO(b/147433177) uncomment the below line when all aidl_interface modules
+ // are with 'current' API dump.
+ //m.checkApiTimestamps = append(m.checkApiTimestamps, alwaysChecked)
+ }
}
- m.freezeApiTimestamp = m.freezeApiDumpAsVersion(ctx, currentDumpDir, currentApiFiles.Paths(), currentVersion)
-
- apiDirs := make(map[string]android.Path)
- apiFiles := make(map[string]android.Paths)
+ // Also check that version X is backwards compatible with version X-1.
+ // "current" is checked against the latest version.
+ var dumps []apiDump
for _, ver := range m.properties.Versions {
apiDir := android.PathForModuleSrc(ctx, m.apiDir(), ver)
- apiDirs[ver] = apiDir
- apiFiles[ver] = ctx.Glob(filepath.Join(apiDir.String(), "**/*.aidl"), nil)
+ dumps = append(dumps, apiDump{
+ dir: apiDir,
+ files: ctx.Glob(filepath.Join(apiDir.String(), "**/*.aidl"), nil),
+ hashFile: android.ExistentPathForSource(ctx, ctx.ModuleDir(), m.apiDir(), ver, ".hash"),
+ })
}
- apiDirs[currentVersion] = currentDumpDir
- apiFiles[currentVersion] = currentApiFiles.Paths()
-
- // Check that version X is backward compatible with version X-1
- for i, newVersion := range m.properties.Versions {
- if i != 0 {
- oldVersion := m.properties.Versions[i-1]
- checkApiTimestamp := m.checkCompatibility(ctx, apiDirs[oldVersion], apiFiles[oldVersion], apiDirs[newVersion], apiFiles[newVersion])
- m.checkApiTimestamps = append(m.checkApiTimestamps, checkApiTimestamp)
+ if currentApiDir.Valid() {
+ dumps = append(dumps, currentApiDump)
+ }
+ for i, _ := range dumps {
+ if i == 0 {
+ continue
}
+ checked := m.checkCompatibility(ctx, dumps[i-1], dumps[i])
+ m.checkApiTimestamps = append(m.checkApiTimestamps, checked)
}
- // ... and that the currentVersion (ToT) is backwards compatible with or
- // equal to the latest frozen version
- if len(m.properties.Versions) >= 1 {
- latestVersion := m.properties.Versions[len(m.properties.Versions)-1]
- var checkApiTimestamp android.WritablePath
- if ctx.Config().DefaultAppTargetSdkInt() != android.FutureApiLevel {
- // If API is frozen, don't allow any change to the API
- latestHashFile := android.OptionalPathForModuleSrc(ctx, proptools.StringPtr(filepath.Join(m.apiDir(), latestVersion, ".hash")))
- checkApiTimestamp = m.checkEquality(ctx, apiDirs[latestVersion], apiFiles[latestVersion], latestHashFile,
- apiDirs[currentVersion], apiFiles[currentVersion], currentHashFile)
- } else {
- // If not, allow backwards compatible changes to the API
- checkApiTimestamp = m.checkCompatibility(ctx, apiDirs[latestVersion], apiFiles[latestVersion], apiDirs[currentVersion], apiFiles[currentVersion])
- }
- m.checkApiTimestamps = append(m.checkApiTimestamps, checkApiTimestamp)
- }
+ // API dump from source is updated to the 'current' version. Triggered by `m <name>-update-api`
+ m.updateApiTimestamp = m.makeApiDumpAsVersion(ctx, totApiDump, currentVersion)
+
+ // API dump from source is frozen as the next stable version. Triggered by `m <name>-freeze-api`
+ nextVersion := m.nextVersion(ctx)
+ m.freezeApiTimestamp = m.makeApiDumpAsVersion(ctx, totApiDump, nextVersion)
}
func (m *aidlApi) AndroidMk() android.AndroidMkData {
@@ -641,6 +679,10 @@
targetName := m.properties.BaseName + "-freeze-api"
fmt.Fprintln(w, ".PHONY:", targetName)
fmt.Fprintln(w, targetName+":", m.freezeApiTimestamp.String())
+
+ targetName = m.properties.BaseName + "-update-api"
+ fmt.Fprintln(w, ".PHONY:", targetName)
+ fmt.Fprintln(w, targetName+":", m.updateApiTimestamp.String())
},
}
}
@@ -807,9 +849,10 @@
if !i.hasVersion() {
return ""
} else {
- i, err := strconv.Atoi(i.latestVersion())
+ ver := i.latestVersion()
+ i, err := strconv.Atoi(ver)
if err != nil {
- ctx.PropertyErrorf("versions", "must be integers")
+ ctx.PropertyErrorf("versions", "%q is not an integer", ver)
return ""
}
diff --git a/build/aidl_test.go b/build/aidl_test.go
index 6cd45b3..547df47 100644
--- a/build/aidl_test.go
+++ b/build/aidl_test.go
@@ -102,6 +102,7 @@
"libbinder_ndk.map.txt": nil,
"system/tools/aidl/build/api_preamble.txt": nil,
"system/tools/aidl/build/message_check_compatibility.txt": nil,
+ "system/tools/aidl/build/message_check_equality.txt": nil,
}
cc.GatherRequiredFilesForTest(fs)
diff --git a/build/message_check_equality.txt b/build/message_check_equality.txt
index fc5d063..3fee410 100644
--- a/build/message_check_equality.txt
+++ b/build/message_check_equality.txt
@@ -1,7 +1,6 @@
###############################################################################
-# ERROR: AIDL API change detected on a released platform #
+# ERROR: AIDL API change detected #
###############################################################################
-Above AIDL file(s) has changed and this is NEVER allowed on a release platform
-(i.e., PLATFORM_VERSION_CODENAME is REL). If a device is shipped with this
-change by ignoring this message, it has a high risk of breaking later when a
-module using the interface is updated, e.g., Maineline modules.
+Above AIDL file(s) has changed. Run `m %s-update-api` to reflect the changes
+to the current version so that it is reviewed by
+android-aidl-api-council@google.com
diff --git a/build/message_check_equality_release.txt b/build/message_check_equality_release.txt
new file mode 100644
index 0000000..fc5d063
--- /dev/null
+++ b/build/message_check_equality_release.txt
@@ -0,0 +1,7 @@
+###############################################################################
+# ERROR: AIDL API change detected on a released platform #
+###############################################################################
+Above AIDL file(s) has changed and this is NEVER allowed on a release platform
+(i.e., PLATFORM_VERSION_CODENAME is REL). If a device is shipped with this
+change by ignoring this message, it has a high risk of breaking later when a
+module using the interface is updated, e.g., Maineline modules.