custom extension for make/SourceRevision.gmk

[Erik Joelsson]

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.
> +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