[build] Expunge use of CONFIGURATIONS (Continuation) #2353
Conversation
Makefile
Outdated
| @@ -167,20 +167,20 @@ TPN_LICENSE_FILES = $(shell grep -h '<LicenseFile>' external/*.tpnitems src/*.tp | |||
| | sed -E 's,<LicenseFile>(.*)</LicenseFile>,\1,g;s,.\(MSBuildThisFileDirectory\),$(topdir)/external/,g' \ | |||
| | tr \\ / ) | |||
|
|
|||
| # Usage: $(call CREATE_THIRD_PARTY_NOTICES,configuration,path,licenseType,includeExternalDeps,includeBuildDeps) | |||
| # Usage: $(call CREATE_THIRD_PARTY_NOTICES_RULE,path,licenseType,includeExternalDeps,includeBuildDeps) | |||
| define CREATE_THIRD_PARTY_NOTICES_RULE | |||
| prepare-tpn:: $(2) | |||
There was a problem hiding this comment.
Missed that spot. $(2) -> $(1)
Makefile
Outdated
| @@ -167,20 +167,20 @@ TPN_LICENSE_FILES = $(shell grep -h '<LicenseFile>' external/*.tpnitems src/*.tp | |||
| | sed -E 's,<LicenseFile>(.*)</LicenseFile>,\1,g;s,.\(MSBuildThisFileDirectory\),$(topdir)/external/,g' \ | |||
| | tr \\ / ) | |||
|
|
|||
| # Usage: $(call CREATE_THIRD_PARTY_NOTICES,configuration,path,licenseType,includeExternalDeps,includeBuildDeps) | |||
| # Usage: $(call CREATE_THIRD_PARTY_NOTICES_RULE,path,licenseType,includeExternalDeps,includeBuildDeps) | |||
| define CREATE_THIRD_PARTY_NOTICES_RULE | |||
| prepare-tpn:: $(2) | |||
|
|
|||
| $(2) $(topdir)/$(2): build-tools/ThirdPartyNotices/ThirdPartyNotices.csproj \ | |||
There was a problem hiding this comment.
Missed that spot. $(2) -> $(1)
Makefile
Outdated
| @@ -102,9 +102,9 @@ prepare-external: | |||
| git submodule update --init --recursive | |||
| nuget restore $(SOLUTION) | |||
| nuget restore Xamarin.Android-Tests.sln | |||
| (cd external/xamarin-android-tools && make prepare CONFIGURATION=$(CONFIGURATION)) | |||
There was a problem hiding this comment.
Why are you changing this line?
I believe that this is "implicitly" problematic. Consider the following two makefiles:
A.mk:
CONFIGURATION=Foo
all:
make -f B.mk allB.mk:
CONFIGURATION=Default
all:
@echo CONFIGURATION=$(CONFIGURATION)The intention is that A.mk "controls" B.mk, as A.mk invokes B.mk in its all target. However, if no $(CONFIGURATION) value is specified, which value does B.mk print: the one in B.mk, or the one in A.mk?
$ make -f A.mk
make -f B.mk all
CONFIGURATION=DefaultIt uses the default value in B.mk. Meaning A.mk is not in control; defaults are left at the discretion of B.mk.
If we do override on the command line, things work as intended, but it's the default case which doesn't:
$ make -f A.mk CONFIGURATION=NewConfig
make -f B.mk all
CONFIGURATION=NewConfigWe're in a similar situation here where Makefile is A.mk, and external/Java.Interop/Makefile is B.mk: If/when $(CONFIGURATION) is not explicitly overridden on the command line -- which will presumably happen a majority of the time -- then external/Java.Interop/Makefile will use the default $(CONFIGURATION) value specified in external/Java.Interop/Makefile, not Makefile.
This is fine now, as both Makefile and external/Java.Interop/Makefile have the same default $(CONFIGURATION) value, but if that changes in the future, things can become inconsistent.
I thus believe/assert that it is safer to always override $(CONFIGURATION), as is currently done, so that Makefile is always in control.
There were some leftovers of useless $(CONFIGURATION)
luhenry
force-pushed
the
remove-CONFIGURATIONS
branch
from
6babf0a
to
b534d41
Compare
There were some leftovers of useless $(CONFIGURATION)