Merge "releasetools: Use "ota-downgrade=yes" for --override_timestamp."
diff --git a/tools/releasetools/ota_from_target_files.py b/tools/releasetools/ota_from_target_files.py
index e8ec50e..968fd77 100755
--- a/tools/releasetools/ota_from_target_files.py
+++ b/tools/releasetools/ota_from_target_files.py
@@ -24,12 +24,12 @@
 
   --downgrade
       Intentionally generate an incremental OTA that updates from a newer build
-      to an older one (based on timestamp comparison). "post-timestamp" will be
-      replaced by "ota-downgrade=yes" in the metadata file. A data wipe will
-      always be enforced, so "ota-wipe=yes" will also be included in the
-      metadata file. The update-binary in the source build will be used in the
-      OTA package, unless --binary flag is specified. Please also check the doc
-      for --override_timestamp below.
+      to an older one (e.g. downgrading from P preview back to O MR1).
+      "ota-downgrade=yes" will be set in the package metadata file. A data wipe
+      will always be enforced when using this flag, so "ota-wipe=yes" will also
+      be included in the metadata file. The update-binary in the source build
+      will be used in the OTA package, unless --binary flag is specified. Please
+      also check the comment for --override_timestamp below.
 
   -i  (--incremental_from) <file>
       Generate an incremental OTA using the given target-files zip as the
@@ -46,14 +46,19 @@
 
   --override_timestamp
       Intentionally generate an incremental OTA that updates from a newer build
-      to an older one (based on timestamp comparison), by overriding the
-      timestamp in package metadata. This differs from --downgrade flag: we know
-      for sure this is NOT an actual downgrade case, but two builds are cut in a
-      reverse order. A legit use case is that we cut a new build C (after having
-      A and B), but want to enfore an update path of A -> C -> B.  Specifying
-      --downgrade may not help since that would enforce a data wipe for C -> B
-      update. The value of "post-timestamp" will be set to the newer timestamp
-      plus one, so that the package can be pushed and applied.
+      to an older one (based on timestamp comparison), by setting the downgrade
+      flag in the package metadata. This differs from --downgrade flag, as we
+      don't enforce a data wipe with this flag. Because we know for sure this is
+      NOT an actual downgrade case, but two builds happen to be cut in a reverse
+      order (e.g. from two branches). A legit use case is that we cut a new
+      build C (after having A and B), but want to enfore an update path of A ->
+      C -> B. Specifying --downgrade may not help since that would enforce a
+      data wipe for C -> B update.
+
+      We used to set a fake timestamp in the package metadata for this flow. But
+      now we consolidate the two cases (i.e. an actual downgrade, or a downgrade
+      based on timestamp) with the same "ota-downgrade=yes" flag, with the
+      difference being whether "ota-wipe=yes" is set.
 
   --wipe_user_data
       Generate an OTA package that will wipe the user data partition when
@@ -184,7 +189,6 @@
 OPTIONS.patch_threshold = 0.95
 OPTIONS.wipe_user_data = False
 OPTIONS.downgrade = False
-OPTIONS.timestamp = False
 OPTIONS.extra_script = None
 OPTIONS.worker_threads = multiprocessing.cpu_count() // 2
 if OPTIONS.worker_threads == 0:
@@ -902,23 +906,16 @@
 
   if OPTIONS.downgrade:
     if not is_downgrade:
-      raise RuntimeError("--downgrade specified but no downgrade detected: "
-                         "pre: %s, post: %s" % (pre_timestamp, post_timestamp))
+      raise RuntimeError(
+          "--downgrade or --override_timestamp specified but no downgrade "
+          "detected: pre: %s, post: %s" % (pre_timestamp, post_timestamp))
     metadata["ota-downgrade"] = "yes"
-  elif OPTIONS.timestamp:
-    if not is_downgrade:
-      raise RuntimeError("--override_timestamp specified but no timestamp hack "
-                         "needed: pre: %s, post: %s" % (pre_timestamp,
-                                                        post_timestamp))
-    metadata["post-timestamp"] = str(long(pre_timestamp) + 1)
   else:
     if is_downgrade:
-      raise RuntimeError("Downgrade detected based on timestamp check: "
-                         "pre: %s, post: %s. Need to specify "
-                         "--override_timestamp OR --downgrade to allow "
-                         "building the incremental." % (pre_timestamp,
-                                                        post_timestamp))
-    metadata["post-timestamp"] = post_timestamp
+      raise RuntimeError(
+          "Downgrade detected based on timestamp check: pre: %s, post: %s. "
+          "Need to specify --override_timestamp OR --downgrade to allow "
+          "building the incremental." % (pre_timestamp, post_timestamp))
 
 
 def GetPackageMetadata(target_info, source_info=None):
@@ -926,7 +923,7 @@
 
   It generates a dict() that contains the info to be written into an OTA
   package (META-INF/com/android/metadata). It also handles the detection of
-  downgrade / timestamp override / data wipe based on the global options.
+  downgrade / data wipe based on the global options.
 
   Args:
     target_info: The BuildInfo instance that holds the target build info.
@@ -967,11 +964,12 @@
   else:
     metadata['pre-device'] = target_info.device
 
-  # Detect downgrades, or fill in the post-timestamp.
+  # Use the actual post-timestamp, even for a downgrade case.
+  metadata['post-timestamp'] = target_info.GetBuildProp('ro.build.date.utc')
+
+  # Detect downgrades and set up downgrade flags accordingly.
   if is_incremental:
     HandleDowngradeMetadata(metadata, target_info, source_info)
-  else:
-    metadata['post-timestamp'] = target_info.GetBuildProp('ro.build.date.utc')
 
   return metadata
 
@@ -1796,7 +1794,7 @@
       OPTIONS.downgrade = True
       OPTIONS.wipe_user_data = True
     elif o == "--override_timestamp":
-      OPTIONS.timestamp = True
+      OPTIONS.downgrade = True
     elif o in ("-o", "--oem_settings"):
       OPTIONS.oem_source = a.split(',')
     elif o == "--oem_no_mount":
@@ -1874,19 +1872,12 @@
     sys.exit(1)
 
   if OPTIONS.downgrade:
-    # Sanity check to enforce a data wipe.
-    if not OPTIONS.wipe_user_data:
-      raise ValueError("Cannot downgrade without a data wipe")
-
     # We should only allow downgrading incrementals (as opposed to full).
     # Otherwise the device may go back from arbitrary build with this full
     # OTA package.
     if OPTIONS.incremental_source is None:
       raise ValueError("Cannot generate downgradable full OTAs")
 
-  assert not (OPTIONS.downgrade and OPTIONS.timestamp), \
-      "Cannot have --downgrade AND --override_timestamp both"
-
   # Load the build info dicts from the zip directly or the extracted input
   # directory. We don't need to unzip the entire target-files zips, because they
   # won't be needed for A/B OTAs (brillo_update_payload does that on its own).
diff --git a/tools/releasetools/test_ota_from_target_files.py b/tools/releasetools/test_ota_from_target_files.py
index 213e830..e472363 100644
--- a/tools/releasetools/test_ota_from_target_files.py
+++ b/tools/releasetools/test_ota_from_target_files.py
@@ -532,31 +532,7 @@
             'post-build-incremental' : 'build-version-incremental-target',
             'post-sdk-level' : '27',
             'post-security-patch-level' : '2017-12-01',
-            'pre-device' : 'product-device',
-            'pre-build' : 'build-fingerprint-source',
-            'pre-build-incremental' : 'build-version-incremental-source',
-        },
-        metadata)
-
-  def test_GetPackageMetadata_overrideTimestamp(self):
-    target_info_dict = copy.deepcopy(self.TEST_TARGET_INFO_DICT)
-    source_info_dict = copy.deepcopy(self.TEST_SOURCE_INFO_DICT)
-    self._test_GetPackageMetadata_swapBuildTimestamps(
-        target_info_dict, source_info_dict)
-
-    target_info = BuildInfo(target_info_dict, None)
-    source_info = BuildInfo(source_info_dict, None)
-    common.OPTIONS.incremental_source = ''
-    common.OPTIONS.timestamp = True
-    metadata = GetPackageMetadata(target_info, source_info)
-    self.assertDictEqual(
-        {
-            'ota-type' : 'BLOCK',
-            'post-build' : 'build-fingerprint-target',
-            'post-build-incremental' : 'build-version-incremental-target',
-            'post-sdk-level' : '27',
-            'post-security-patch-level' : '2017-12-01',
-            'post-timestamp' : '1500000001',
+            'post-timestamp' : '1400000000',
             'pre-device' : 'product-device',
             'pre-build' : 'build-fingerprint-source',
             'pre-build-incremental' : 'build-version-incremental-source',