From 9569ddd4789000d27c649efeb34cd32f7403c981 Mon Sep 17 00:00:00 2001
From: Dan Willemsen <dwillemsen@google.com>
Date: Tue, 22 Jan 2019 19:38:56 -0800
Subject: [PATCH] Deprecate LOCAL_MODULE_TAGS := eng/debug

Uses a new BUILD_BROKEN_ENG_DEBUG_TAGS variable that defaults to true
until we mark all of the broken targets, then we'll switch it to false.

These are fairly trivial to fix, so I don't expect to keep this flag
around for long.

Test: build_test on aosp and internal master; check logs
Change-Id: I6176fec265e78aecff965b7cf1636f831f68a140
---
 Changes.md         | 15 +++++++++++++++
 core/base_rules.mk | 13 +++++++++++++
 core/config.mk     |  4 ++--
 core/envsetup.mk   |  7 ++-----
 4 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/Changes.md b/Changes.md
index 1ed6bf81d1..165be0194f 100644
--- a/Changes.md
+++ b/Changes.md
@@ -1,5 +1,20 @@
 # Build System Changes for Android.mk Writers
 
+## `LOCAL_MODULE_TAGS := eng debug` deprecation  {#LOCAL_MODULE_TAGS}
+
+`LOCAL_MODULE_TAGS` value `eng` and `debug` are being deprecated. They allowed
+modules to specify that they should always be installed on `-eng`, or `-eng`
+and `-userdebug` builds. This conflicted with the ability for products to
+specify which modules should be installed, effectively making it impossible to
+build a stripped down product configuration that did not include those modules.
+
+For the equivalent functionality, specify the modules in `PRODUCT_PACKAGES_ENG`
+or `PRODUCT_PACKAGES_DEBUG` in the appropriate product makefiles.
+
+Core android packages like `su` got added to the list in
+`build/make/target/product/base_system.mk`, but for device-specific modules
+there are often better base product makefiles to use instead.
+
 ## `USER` deprecation  {#USER}
 
 `USER` will soon be `nobody` in many cases due to the addition of a sandbox
diff --git a/core/base_rules.mk b/core/base_rules.mk
index 64e1069b59..07b427bb8d 100644
--- a/core/base_rules.mk
+++ b/core/base_rules.mk
@@ -162,6 +162,19 @@ ifneq ($(filter $(my_module_tags),user),)
   $(error user tag detected on module.)
 endif
 
+my_bad_module_tags := $(filter eng debug,$(my_module_tags))
+ifdef my_bad_module_tags
+  ifeq (true,$(LOCAL_UNINSTALLABLE_MODULE))
+    $(call pretty-warning,LOCAL_MODULE_TAGS := $(my_bad_module_tags) does not do anything for uninstallable modules)
+  endif
+  ifeq ($(BUILD_BROKEN_ENG_DEBUG_TAGS),false)
+    $(call pretty-error,LOCAL_MODULE_TAGS := $(my_bad_module_tags) is obsolete. See $(CHANGES_URL)#LOCAL_MODULE_TAGS)
+  else
+    $(call pretty-warning,LOCAL_MODULE_TAGS := $(my_bad_module_tags) is deprecated. See $(CHANGES_URL)#LOCAL_MODULE_TAGS)
+  endif
+  my_bad_module_tags :=
+endif
+
 # Only the tags mentioned in this test are expected to be set by module
 # makefiles. Anything else is either a typo or a source of unexpected
 # behaviors.
diff --git a/core/config.mk b/core/config.mk
index b0805fc27e..ed6658ea11 100644
--- a/core/config.mk
+++ b/core/config.mk
@@ -34,6 +34,7 @@ endif
 
 # Mark variables deprecated/obsolete
 CHANGES_URL := https://android.googlesource.com/platform/build/+/master/Changes.md
+.KATI_READONLY := CHANGES_URL
 $(KATI_obsolete_var PATH,Do not use PATH directly. See $(CHANGES_URL)#PATH)
 $(KATI_obsolete_var PYTHONPATH,Do not use PYTHONPATH directly. See $(CHANGES_URL)#PYTHONPATH)
 $(KATI_obsolete_var OUT,Use OUT_DIR instead. See $(CHANGES_URL)#OUT)
@@ -95,8 +96,6 @@ $(KATI_deprecated_var USER,Use BUILD_USERNAME instead. See $(CHANGES_URL)#USER)
 # This is marked as obsolete in envsetup.mk after reading the BoardConfig.mk
 $(KATI_deprecate_export It is a global setting. See $(CHANGES_URL)#export_keyword)
 
-CHANGES_URL :=
-
 # Used to force goals to build.  Only use for conditionally defined goals.
 .PHONY: FORCE
 FORCE:
@@ -224,6 +223,7 @@ BUILD_BROKEN_ANDROIDMK_EXPORTS :=
 BUILD_BROKEN_DUP_COPY_HEADERS :=
 BUILD_BROKEN_DUP_RULES :=
 BUILD_BROKEN_PHONY_TARGETS :=
+BUILD_BROKEN_ENG_DEBUG_TAGS :=
 
 # ###############################################################
 # Include sub-configuration files
diff --git a/core/envsetup.mk b/core/envsetup.mk
index 390a182f30..f7079fe602 100644
--- a/core/envsetup.mk
+++ b/core/envsetup.mk
@@ -271,7 +271,8 @@ vars := \
   BUILD_BROKEN_ANDROIDMK_EXPORTS \
   BUILD_BROKEN_DUP_COPY_HEADERS \
   BUILD_BROKEN_DUP_RULES \
-  BUILD_BROKEN_PHONY_TARGETS
+  BUILD_BROKEN_PHONY_TARGETS \
+  BUILD_BROKEN_ENG_DEBUG_TAGS
 
 $(foreach var,$(vars),$(eval $(var) := $$(strip $$($(var)))))
 
@@ -281,14 +282,10 @@ $(foreach var,$(vars), \
 
 .KATI_READONLY := $(vars)
 
-CHANGES_URL := https://android.googlesource.com/platform/build/+/master/Changes.md
-
 ifneq ($(BUILD_BROKEN_ANDROIDMK_EXPORTS),true)
 $(KATI_obsolete_export It is a global setting. See $(CHANGES_URL)#export_keyword)
 endif
 
-CHANGES_URL :=
-
 ###########################################
 # Now we can substitute with the real value of TARGET_COPY_OUT_RAMDISK
 ifeq ($(BOARD_BUILD_SYSTEM_ROOT_IMAGE),true)