--checkapi=equal checks the equality of two dumps
Switched from diff-based test, which often breaks for trivial changes.
For example, changing '/* @hide */' to '/** @hide */' shouldn't break
the equality of two dumps.
Similarly, adding license/copyright headers to dumped .aidl files
shouldn't break the equality.
Bug: 172476424
Test: m (triggers checks)
Change-Id: Iea2d3b9f8597977ab276828e943dd6e84debc8ce
diff --git a/Android.bp b/Android.bp
index 7ef8746..e24d18d 100644
--- a/Android.bp
+++ b/Android.bp
@@ -25,6 +25,7 @@
static_libs: [
"libbase",
"libcutils",
+ "libgtest",
],
// TODO(b/174366536): basic_stringbuf::overflow causes "ubsan: implicit-conversion"
// sanitize: {
@@ -183,6 +184,7 @@
"libaidl-common",
"libbase",
"libcutils",
+ "libgtest",
"liblog",
],
// Enable this to show additional information about what is being parsed during fuzzing.
diff --git a/aidl_checkapi.cpp b/aidl_checkapi.cpp
index 2b5664a..6337f8e 100644
--- a/aidl_checkapi.cpp
+++ b/aidl_checkapi.cpp
@@ -26,6 +26,7 @@
#include <android-base/result.h>
#include <android-base/strings.h>
+#include <gtest/gtest.h>
namespace android {
namespace aidl {
@@ -38,6 +39,24 @@
using std::string;
using std::vector;
+static std::string Dump(const AidlDefinedType& type) {
+ std::string dump;
+ type.Dump(CodeWriter::ForString(&dump).get());
+ return dump;
+}
+
+// Uses each type's Dump() and GTest utility(EqHelper).
+static bool CheckEquality(const AidlDefinedType& older, const AidlDefinedType& newer) {
+ using testing::internal::EqHelper;
+ auto older_file = older.GetLocation().GetFile();
+ auto newer_file = newer.GetLocation().GetFile();
+ auto result = EqHelper::Compare(older_file.data(), newer_file.data(), Dump(older), Dump(newer));
+ if (!result) {
+ AIDL_ERROR(newer) << result.failure_message();
+ }
+ return result;
+}
+
static vector<string> get_strict_annotations(const AidlAnnotatable& node) {
// This must be symmetrical (if you can add something, you must be able to
// remove it). The reason is that we have no way of knowing which interface a
@@ -367,15 +386,33 @@
return false;
}
+ const Options::CheckApiLevel level = options.GetCheckApiLevel();
+
std::vector<AidlDefinedType*> old_types = old_tns->AllDefinedTypes();
std::vector<AidlDefinedType*> new_types = new_tns->AllDefinedTypes();
+ bool compatible = true;
+
+ if (level == Options::CheckApiLevel::EQUAL) {
+ std::set<string> old_type_names;
+ for (const auto t : old_types) {
+ old_type_names.insert(t->GetCanonicalName());
+ }
+ for (const auto new_type : new_types) {
+ const auto found = old_type_names.find(new_type->GetCanonicalName());
+ if (found == old_type_names.end()) {
+ AIDL_ERROR(new_type) << "Added type: " << new_type->GetCanonicalName();
+ compatible = false;
+ continue;
+ }
+ }
+ }
+
map<string, AidlDefinedType*> new_map;
for (const auto t : new_types) {
new_map.emplace(t->GetCanonicalName(), t);
}
- bool compatible = true;
for (const auto old_type : old_types) {
const auto found = new_map.find(old_type->GetCanonicalName());
if (found == new_map.end()) {
@@ -385,6 +422,13 @@
}
const auto new_type = found->second;
+ if (level == Options::CheckApiLevel::EQUAL) {
+ if (!CheckEquality(*old_type, *new_type)) {
+ compatible = false;
+ }
+ continue;
+ }
+
if (!have_compatible_annotations(*old_type, *new_type)) {
compatible = false;
}
diff --git a/aidl_unittest.cpp b/aidl_unittest.cpp
index 90e05d8..0364ede 100644
--- a/aidl_unittest.cpp
+++ b/aidl_unittest.cpp
@@ -1874,6 +1874,19 @@
EXPECT_EQ(0, ::android::aidl::compile_aidl(options, io_delegate_));
}
+TEST_F(AidlTest, CheckApiForEquality) {
+ CaptureStderr();
+ Options options = Options::From("aidl --checkapi=equal old new");
+
+ io_delegate_.SetFileContents("old/p/IFoo.aidl",
+ "package p; interface IFoo{ @utf8InCpp @nullable String foo();}");
+ io_delegate_.SetFileContents("new/p/IFoo.aidl",
+ "package p; interface IFoo{ @utf8InCpp String foo();}");
+
+ EXPECT_FALSE(::android::aidl::check_api(options, io_delegate_));
+ EXPECT_THAT(GetCapturedStderr(), HasSubstr("+ @utf8InCpp String foo();"));
+}
+
TEST_F(AidlTest, DifferentOrderAnnotationsInCheckAPI) {
Options options = Options::From("aidl --checkapi old new");
io_delegate_.SetFileContents("old/p/IFoo.aidl",
diff --git a/build/aidl_api.go b/build/aidl_api.go
index 128da00..496e2a5 100644
--- a/build/aidl_api.go
+++ b/build/aidl_api.go
@@ -36,17 +36,11 @@
}, "optionalFlags", "imports", "outDir", "hashFile", "latestVersion")
aidlCheckApiRule = pctx.StaticRule("aidlCheckApiRule", blueprint.RuleParams{
- Command: `(${aidlCmd} ${optionalFlags} --checkapi ${old} ${new} && touch ${out}) || ` +
+ Command: `(${aidlCmd} ${optionalFlags} --checkapi=${checkApiLevel} ${old} ${new} && touch ${out}) || ` +
`(cat ${messageFile} && exit 1)`,
CommandDeps: []string{"${aidlCmd}"},
Description: "AIDL CHECK API: ${new} against ${old}",
- }, "optionalFlags", "old", "new", "messageFile")
-
- aidlDiffApiRule = pctx.StaticRule("aidlDiffApiRule", blueprint.RuleParams{
- Command: `if diff -r -B -I '//.*' -x '${hashFile}' '${old}' '${new}'; then touch '${out}'; else ` +
- `cat '${messageFile}' && exit 1; fi`,
- Description: "Check equality of ${new} and ${old}",
- }, "old", "new", "hashFile", "messageFile")
+ }, "optionalFlags", "old", "new", "messageFile", "checkApiLevel")
aidlVerifyHashRule = pctx.StaticRule("aidlVerifyHashRule", blueprint.RuleParams{
Command: `if [ $$(cd '${apiDir}' && { find ./ -name "*.aidl" -print0 | LC_ALL=C sort -z | xargs -0 sha1sum && echo ${version}; } | sha1sum | cut -d " " -f 1) = $$(read -r <'${hashFile}' hash extra; printf %s $$hash) ]; then ` +
@@ -190,10 +184,9 @@
return timestampFile
}
-func (m *aidlApi) checkCompatibility(ctx android.ModuleContext, oldDump apiDump, newDump apiDump) android.WritablePath {
+func (m *aidlApi) checkApi(ctx android.ModuleContext, oldDump, newDump apiDump, checkApiLevel string, messageFile android.Path) 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")
var optionalFlags []string
if m.properties.Stability != nil {
@@ -213,15 +206,18 @@
"old": oldDump.dir.String(),
"new": newDump.dir.String(),
"messageFile": messageFile.String(),
+ "checkApiLevel": checkApiLevel,
},
})
return timestampFile
}
-func (m *aidlApi) checkEquality(ctx android.ModuleContext, oldDump apiDump, newDump apiDump) android.WritablePath {
- newVersion := newDump.dir.Base()
- timestampFile := android.PathForModuleOut(ctx, "checkapi_"+newVersion+".timestamp")
+func (m *aidlApi) checkCompatibility(ctx android.ModuleContext, oldDump, newDump apiDump) android.WritablePath {
+ messageFile := android.PathForSource(ctx, "system/tools/aidl/build/message_check_compatibility.txt")
+ return m.checkApi(ctx, oldDump, newDump, "compatible", messageFile)
+}
+func (m *aidlApi) checkEquality(ctx android.ModuleContext, oldDump apiDump, newDump apiDump) android.WritablePath {
// 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
@@ -240,18 +236,7 @@
implicits = append(implicits, oldDump.files...)
implicits = append(implicits, newDump.files...)
implicits = append(implicits, formattedMessageFile)
- ctx.Build(pctx, android.BuildParams{
- Rule: aidlDiffApiRule,
- Implicits: implicits,
- Output: timestampFile,
- Args: map[string]string{
- "old": oldDump.dir.String(),
- "new": newDump.dir.String(),
- "hashFile": newDump.hashFile.Path().Base(),
- "messageFile": formattedMessageFile.String(),
- },
- })
- return timestampFile
+ return m.checkApi(ctx, oldDump, newDump, "equal", formattedMessageFile)
}
func (m *aidlApi) checkIntegrity(ctx android.ModuleContext, dump apiDump) android.WritablePath {
diff --git a/location.h b/location.h
index 0f5900c..9ccad24 100644
--- a/location.h
+++ b/location.h
@@ -42,6 +42,8 @@
// The first line of a file is line 1.
bool LocationKnown() const { return begin_.line != 0; }
+ std::string GetFile() const { return file_; }
+
friend std::ostream& operator<<(std::ostream& os, const AidlLocation& l);
friend class AidlNode;
diff --git a/options.cpp b/options.cpp
index 620edf6..97eb6b0 100644
--- a/options.cpp
+++ b/options.cpp
@@ -55,9 +55,9 @@
<< myname_ << " --dumpapi --out=DIR INPUT..." << endl
<< " Dump API signature of AIDL file(s) to DIR." << endl
<< endl
- << myname_ << " --checkapi OLD_DIR NEW_DIR" << endl
- << " Checkes whether API dump NEW_DIR is backwards compatible extension " << endl
- << " of the API dump OLD_DIR." << endl
+ << myname_ << " --checkapi[={compatible|equal}] OLD_DIR NEW_DIR" << endl
+ << " Check whether NEW_DIR API dump is {compatible|equal} extension " << endl
+ << " of the API dump OLD_DIR. Default: compatible" << endl
#endif
<< endl;
@@ -215,7 +215,7 @@
{"preprocess", no_argument, 0, 's'},
#ifndef _WIN32
{"dumpapi", no_argument, 0, 'u'},
- {"checkapi", no_argument, 0, 'A'},
+ {"checkapi", optional_argument, 0, 'A'},
#endif
{"apimapping", required_argument, 0, 'i'},
{"include", required_argument, 0, 'I'},
@@ -285,6 +285,16 @@
task_ = Options::Task::CHECK_API;
// to ensure that all parcelables in the api dumpes are structured
structured_ = true;
+ if (optarg) {
+ if (strcmp(optarg, "compatible") == 0)
+ check_api_level_ = CheckApiLevel::COMPATIBLE;
+ else if (strcmp(optarg, "equal") == 0)
+ check_api_level_ = CheckApiLevel::EQUAL;
+ else {
+ error_message_ << "Unsupported --checkapi level: '" << optarg << "'" << endl;
+ return;
+ }
+ }
}
break;
#endif
diff --git a/options.h b/options.h
index 363b639..418208a 100644
--- a/options.h
+++ b/options.h
@@ -79,6 +79,8 @@
enum class Task { UNSPECIFIED, COMPILE, PREPROCESS, DUMP_API, CHECK_API, DUMP_MAPPINGS };
+ enum class CheckApiLevel { COMPATIBLE, EQUAL };
+
enum class Stability { UNSPECIFIED, VINTF };
bool StabilityFromString(const std::string& stability, Stability* out_stability);
@@ -100,6 +102,8 @@
Task GetTask() const { return task_; }
+ CheckApiLevel GetCheckApiLevel() const { return check_api_level_; }
+
const set<string>& ImportDirs() const { return import_dirs_; }
const set<string>& ImportFiles() const { return import_files_; }
@@ -163,6 +167,7 @@
const string myname_;
Language language_ = Language::UNSPECIFIED;
Task task_ = Task::COMPILE;
+ CheckApiLevel check_api_level_ = CheckApiLevel::COMPATIBLE;
set<string> import_dirs_;
set<string> import_files_;
vector<string> preprocessed_files_;
diff --git a/options_unittest.cpp b/options_unittest.cpp
index f573fe6..9f07400 100644
--- a/options_unittest.cpp
+++ b/options_unittest.cpp
@@ -399,5 +399,51 @@
EXPECT_THAT(GetCapturedStderr(), testing::HasSubstr("unknown warning: foobar"));
}
+TEST(OptionsTests, CheckApi) {
+ const char* args[] = {
+ "aidl", "--checkapi", "old", "new", nullptr,
+ };
+ CaptureStderr();
+ auto options = GetOptions(args);
+ EXPECT_TRUE(options->Ok());
+ EXPECT_EQ("", GetCapturedStderr());
+ EXPECT_EQ(Options::Task::CHECK_API, options->GetTask());
+ EXPECT_EQ(Options::CheckApiLevel::COMPATIBLE, options->GetCheckApiLevel());
+}
+
+TEST(OptionsTests, CheckApiWithCompatible) {
+ const char* args[] = {
+ "aidl", "--checkapi=compatible", "old", "new", nullptr,
+ };
+ CaptureStderr();
+ auto options = GetOptions(args);
+ EXPECT_TRUE(options->Ok());
+ EXPECT_EQ("", GetCapturedStderr());
+ EXPECT_EQ(Options::Task::CHECK_API, options->GetTask());
+ EXPECT_EQ(Options::CheckApiLevel::COMPATIBLE, options->GetCheckApiLevel());
+}
+
+TEST(OptionsTests, CheckApiWithEqual) {
+ const char* args[] = {
+ "aidl", "--checkapi=equal", "old", "new", nullptr,
+ };
+ CaptureStderr();
+ auto options = GetOptions(args);
+ EXPECT_TRUE(options->Ok());
+ EXPECT_EQ("", GetCapturedStderr());
+ EXPECT_EQ(Options::Task::CHECK_API, options->GetTask());
+ EXPECT_EQ(Options::CheckApiLevel::EQUAL, options->GetCheckApiLevel());
+}
+
+TEST(OptionsTests, CheckApiWithUnknown) {
+ const char* args[] = {
+ "aidl", "--checkapi=unknown", "old", "new", nullptr,
+ };
+ CaptureStderr();
+ auto options = GetOptions(args);
+ EXPECT_FALSE(options->Ok());
+ EXPECT_THAT(GetCapturedStderr(), testing::HasSubstr("Unsupported --checkapi level: 'unknown'"));
+}
+
} // namespace aidl
} // namespace android