From 5ec6bbc3d542989f7af40a0d01a37a9a44295801 Mon Sep 17 00:00:00 2001 From: Dan Willemsen Date: Wed, 26 Apr 2017 17:37:17 -0700 Subject: [PATCH 1/2] Implement clean-path We shouldn't give non-clean paths to tools -- if a/b/../file was specified, we can simplify that path to a/file, and not need to create a/b just to make the path name work. The testcases come from golang's filepath.Clean tests, this should be compatible with that implementation. Bug: 37716307 Test: TEST_MAKE_clean_path=true m -j blueprint_tools Change-Id: I290a02b0a1e4a7c2b9255bca3c881589b521c402 --- core/definitions.mk | 135 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 135 insertions(+) diff --git a/core/definitions.mk b/core/definitions.mk index 10d26c45c..64bdfe6dd 100644 --- a/core/definitions.mk +++ b/core/definitions.mk @@ -3237,6 +3237,141 @@ $(foreach suite, $(LOCAL_COMPATIBILITY_SUITE), \ $(eval $(my_all_targets) : $(my_compat_files_$(suite)))) endef +########################################################### +## Path Cleaning +########################################################### + +# Remove "dir .." combinations (but keep ".. ..") +# +# $(1): The expanded path, where / is converted to ' ' to work with $(word) +define _clean-path-strip-dotdot +$(strip \ + $(if $(word 2,$(1)), + $(if $(call streq,$(word 2,$(1)),..), + $(if $(call streq,$(word 1,$(1)),..), + $(word 1,$(1)) $(call _clean-path-strip-dotdot,$(wordlist 2,$(words $(1)),$(1))) + , + $(call _clean-path-strip-dotdot,$(wordlist 3,$(words $(1)),$(1))) + ) + , + $(word 1,$(1)) $(call _clean-path-strip-dotdot,$(wordlist 2,$(words $(1)),$(1))) + ) + , + $(1) + ) +) +endef + +# Remove any leading .. from the path (in case of /..) +# +# Should only be called if the original path started with / +# $(1): The expanded path, where / is converted to ' ' to work with $(word) +define _clean-path-strip-root-dotdots +$(strip $(if $(call streq,$(firstword $(1)),..), + $(call _clean-path-strip-root-dotdots,$(wordlist 2,$(words $(1)),$(1))), + $(1))) +endef + +# Call _clean-path-strip-dotdot until the path stops changing +# $(1): Non-empty if this path started with a / +# $(2): The expanded path, where / is converted to ' ' to work with $(word) +define _clean-path-expanded +$(strip \ + $(eval _ep := $(call _clean-path-strip-dotdot,$(2))) + $(if $(1),$(eval _ep := $(call _clean-path-strip-root-dotdots,$(_ep)))) + $(if $(call streq,$(2),$(_ep)), + $(_ep), + $(call _clean-path-expanded,$(1),$(_ep)))) +endef + +# Clean the file path -- remove //, dir/.., extra . +# +# This should be the same semantics as golang's filepath.Clean +# +# $(1): The file path to clean +define clean-path +$(strip \ + $(if $(call streq,$(words $(1)),1), + $(eval _rooted := $(filter /%,$(1))) + $(eval _expanded_path := $(filter-out .,$(subst /,$(space),$(1)))) + $(eval _path := $(if $(_rooted),/)$(subst $(space),/,$(call _clean-path-expanded,$(_rooted),$(_expanded_path)))) + $(if $(_path), + $(_path), + . + ) + , + $(if $(call streq,$(words $(1)),0), + ., + $(error Call clean-path with only one path (without spaces)) + ) + ) +) +endef + +ifeq ($(TEST_MAKE_clean_path),true) + define my_test + $(if $(call streq,$(call clean-path,$(1)),$(2)),, + $(eval my_failed := true) + $(warning clean-path test '$(1)': expected '$(2)', got '$(call clean-path,$(1))')) + endef + my_failed := + + # Already clean + $(call my_test,abc,abc) + $(call my_test,abc/def,abc/def) + $(call my_test,a/b/c,a/b/c) + $(call my_test,.,.) + $(call my_test,..,..) + $(call my_test,../..,../..) + $(call my_test,../../abc,../../abc) + $(call my_test,/abc,/abc) + $(call my_test,/,/) + + # Empty is current dir + $(call my_test,,.) + + # Remove trailing slash + $(call my_test,abc/,abc) + $(call my_test,abc/def/,abc/def) + $(call my_test,a/b/c/,a/b/c) + $(call my_test,./,.) + $(call my_test,../,..) + $(call my_test,../../,../..) + $(call my_test,/abc/,/abc) + + # Remove doubled slash + $(call my_test,abc//def//ghi,abc/def/ghi) + $(call my_test,//abc,/abc) + $(call my_test,///abc,/abc) + $(call my_test,//abc//,/abc) + $(call my_test,abc//,abc) + + # Remove . elements + $(call my_test,abc/./def,abc/def) + $(call my_test,/./abc/def,/abc/def) + $(call my_test,abc/.,abc) + + # Remove .. elements + $(call my_test,abc/def/ghi/../jkl,abc/def/jkl) + $(call my_test,abc/def/../ghi/../jkl,abc/jkl) + $(call my_test,abc/def/..,abc) + $(call my_test,abc/def/../..,.) + $(call my_test,/abc/def/../..,/) + $(call my_test,abc/def/../../..,..) + $(call my_test,/abc/def/../../..,/) + $(call my_test,abc/def/../../../ghi/jkl/../../../mno,../../mno) + $(call my_test,/../abc,/abc) + + # Combinations + $(call my_test,abc/./../def,def) + $(call my_test,abc//./../def,def) + $(call my_test,abc/../../././../def,../../def) + + ifdef my_failed + $(error failed clean-path test) + endif +endif + ########################################################### ## Other includes ########################################################### From fa7ecfb75282cc5d9e1de1820a9f110187e838ed Mon Sep 17 00:00:00 2001 From: Dan Willemsen Date: Tue, 2 May 2017 23:55:44 -0700 Subject: [PATCH 2/2] Clean LOCAL_RESOURCE_DIR paths With LOCAL_USE_AAPT2, resource directories like a/b/../res cause problems, since ninja will canonicalize the path before creating the intermediate resource directory, so it creates /a/res while we give AAPT2 /a/b/../res, which fails to open. Bug: 37716307 Test: Switch LOCAL_USE_AAPT2:=true for TelecommUnitTests, mma Test: lunch aosp_marlin-userdebug; m -j Change-Id: Id0d167e68185a119390e7b7e3c344895e77ca0e3 --- core/package_internal.mk | 1 + core/static_java_library.mk | 1 + 2 files changed, 2 insertions(+) diff --git a/core/package_internal.mk b/core/package_internal.mk index f9f8e3590..6b07be621 100644 --- a/core/package_internal.mk +++ b/core/package_internal.mk @@ -92,6 +92,7 @@ ifeq (,$(LOCAL_RESOURCE_DIR)) LOCAL_RESOURCE_DIR := $(LOCAL_PATH)/res else need_compile_res := true + LOCAL_RESOURCE_DIR := $(foreach d,$(LOCAL_RESOURCE_DIR),$(call clean-path,$(d))) endif package_resource_overlays := $(strip \ diff --git a/core/static_java_library.mk b/core/static_java_library.mk index a8b010399..343b94989 100644 --- a/core/static_java_library.mk +++ b/core/static_java_library.mk @@ -39,6 +39,7 @@ need_compile_res := # A static Java library needs to explicily set LOCAL_RESOURCE_DIR. ifdef LOCAL_RESOURCE_DIR need_compile_res := true +LOCAL_RESOURCE_DIR := $(foreach d,$(LOCAL_RESOURCE_DIR),$(call clean-path,$(d))) endif ifdef LOCAL_USE_AAPT2 ifneq ($(LOCAL_STATIC_ANDROID_LIBRARIES),)