custom extension for make/SourceRevision.gmk

[Christian Thalinger]

> On Jul 19, 2018, at 12:08 PM, Erik Joelsson <erik.joelsson at oracle.com> wrote:
> 
> Hello,
> On 2018-07-19 07:43, Christian Thalinger wrote:
>> 
>> 
>>> On Jul 18, 2018, at 3:28 PM, Christian Thalinger <cthalinger at twitter.com <mailto:cthalinger at twitter.com>> wrote:
>>> 
>>> 
>>> 
>>>> On Jul 18, 2018, at 1:46 PM, Erik Joelsson <erik.joelsson at oracle.com <mailto:erik.joelsson at oracle.com>> wrote:
>>>> 
>>>> Hello Christian,
>>>> 
>>>> Sometimes we need hooks both close to the beginning and close to the end of the file, and in that case we create a SourceBundle-post.gmk. The recommended position of the post inclusion is right before the typical "all: $(TARGETS)" declaration. This file has the all target depend explicitly on a list of phony targets and no TARGETS variable, so I would recommend changing that to building a TARGETS variable like we usually do. That way you can create a SourceBundle-post.gmk and clear the TARGETS variable from any targets you don't want to run from the open file. Does that sound ok?
>>> 
>>> Yes, that would be great.  In JDK 11, please :-)
>> 
>> Ok, this is the only way I could make it work:
>> 
>> diff --git a/make/SourceRevision.gmk b/make/SourceRevision.gmk
>> index 10dd943..13ea407 100644
>> --- a/make/SourceRevision.gmk
>> +++ b/make/SourceRevision.gmk
>> @@ -28,7 +28,7 @@ default: all
>>  include $(SPEC)
>>  include MakeBase.gmk
>>  
>> -$(eval $(call IncludeCustomExtension, SourceRevision.gmk))
>> +$(eval $(call IncludeCustomExtension, SourceRevision-pre.gmk))
>>  
>>  ################################################################################
>>  # Keep track of what source revision is used to create the build, by creating
>> @@ -94,11 +94,14 @@ ifneq ($(and $(HG), $(wildcard $(TOPDIR)/.hg)), )
>>  
>>    $(eval $(call CreateSourceRevisionFile, $(STORED_SOURCE_REVISION)))
>>  
>> -  store-source-revision: $(STORED_SOURCE_REVISION)
>> +  hg-store-source-revision: $(STORED_SOURCE_REVISION)
>>  
>>    $(eval $(call CreateSourceRevisionFile, $(SOURCE_REVISION_TRACKER)))
>>  
>> -  create-source-revision-tracker: $(SOURCE_REVISION_TRACKER)
>> +  hg-create-source-revision-tracker: $(SOURCE_REVISION_TRACKER)
>> +
> These assignments should be using :=. Applies further down as well.
>> +  STORE_SOURCE_REVISION_TARGET = hg-store-source-revision
>> +  CREATE_SOURCE_REVISION_TRACKER_TARGET = hg-create-source-revision-tracker
>>  
>>  else
>>    # Not using HG
>> @@ -106,26 +109,39 @@ else
>>    ifneq ($(wildcard $(STORED_SOURCE_REVISION)), )
>>      # We have a stored source revision (.src-rev)
>>  
>> -    store-source-revision:
>> +    src-store-source-revision:
>>         $(call LogInfo, No mercurial configuration present$(COMMA) not updating .src-rev)
>>  
>>      $(SOURCE_REVISION_TRACKER): $(STORED_SOURCE_REVISION)
>>         $(install-file)
>>  
>> -    create-source-revision-tracker: $(SOURCE_REVISION_TRACKER)
>> +    src-create-source-revision-tracker: $(SOURCE_REVISION_TRACKER)
>>    else
>>      # We don't have a stored source revision. Can't do anything, really.
>>  
>> -    store-source-revision:
>> +    src-store-source-revision:
>>         $(call LogWarn, Error: No mercurial configuration present$(COMMA) cannot create .src-rev)
>>         exit 2
>>  
>> -    create-source-revision-tracker:
>> +    src-create-source-revision-tracker:
>>         $(call LogWarn, Warning: No mercurial configuration present and no .src-rev)
>>    endif
>>  
>> +  STORE_SOURCE_REVISION_TARGET = src-store-source-revision
>> +  CREATE_SOURCE_REVISION_TRACKER_TARGET = src-create-source-revision-tracker
>> +
>>  endif
>>  
>> +################################################################################
>> +
>> +$(eval $(call IncludeCustomExtension, SourceRevision-post.gmk))
>> +
>> +################################################################################
>> +
> I would suggest using the variables directly on the all: line instead of declaring more phony targets. 

Well, the issue is this:

exploded-image: exploded-image-base release-file

  release-file: create-source-revision-tracker

store-source-revision:
        +($(CD) $(TOPDIR)/make && $(MAKE) $(MAKE_ARGS) -f SourceRevision.gmk store-source-revision)

create-source-revision-tracker:
        +($(CD) $(TOPDIR)/make && $(MAKE) $(MAKE_ARGS) -f SourceRevision.gmk create-source-revision-tracker)

We need these targets because all isn’t really used.

>> +store-source-revision: $(STORE_SOURCE_REVISION_TARGET)
>> +
>> +create-source-revision-tracker: $(CREATE_SOURCE_REVISION_TRACKER_TARGET)
>> +
>>  all: store-source-revision create-source-revision-tracker
>>  
>>  FRC: # Force target
>> 
> Do you really need the separate variables? Since both of them are built by all anyway, I would just have one variable TARGETS to which you add everything you wish to build and finish with "all: $(TARGETS)".
> 
> /Erik