Re: custom extension for make/SourceRevision.gmk

2018-08-27 Thread Christian Thalinger 
Done: https://bugs.openjdk.java.net/browse/JDK-8210008

> On Aug 23, 2018, at 4:56 PM, Erik Joelsson  wrote:
>

Re: custom extension for make/SourceRevision.gmk

2018-08-23 Thread Erik Joelsson 



On 2018-08-20 04:27, Christian Thalinger wrote:



On Jul 19, 2018, at 9:17 PM, Christian Thalinger 
mailto:cthalin...@twitter.com>> wrote:




On Jul 19, 2018, at 2:31 PM, Erik Joelsson > wrote:


I can do that. Do you have a bug?


No.


Sorry, was on vacation… I don’t see the change in the repo. Did you 
file one?


I left for vacation too, still am for a few more days. I don't think I 
got around to this before that.


/Erik


/Erik


On 2018-07-19 10:57, Christian Thalinger wrote:



On Thu, Jul 19, 2018 at 1:11 PM Erik Joelsson 
mailto:erik.joels...@oracle.com>> wrote:


This looks good to me, but will need coordination when pushed
as I said earlier.


Do you want to push it so it’s easier?

/Erik


On 2018-07-19 10:04, Christian Thalinger wrote:




On Jul 19, 2018, at 12:57 PM, Erik Joelsson
mailto:erik.joels...@oracle.com>>
wrote:



On 2018-07-19 09:54, Christian Thalinger wrote:




On Jul 19, 2018, at 12:44 PM, Erik Joelsson
mailto:erik.joels...@oracle.com>> wrote:


On 2018-07-19 09:16, Christian Thalinger wrote:



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.


Ah, the all target is tricking me and should be removed if
not called from anywhere. Then your suggested patch is good
(except for missing the :=).


Do you want me to remove the all: target?


Yes, that would be a good cleanup to avoid confusion.


How about this:

diff --git a/make/SourceRevision.gmk b/make/SourceRevision.gmk
index 10dd943..6d4a706 100644
--- a/make/SourceRevision.gmk
+++ b/make/SourceRevision.gmk
@@ -1,5 +1,5 @@
 #
-# Copyright (c) 2016, Oracle and/or its affiliates. All
rights reserved.
+# Copyright (c) 2016, 2018, Oracle and/or its affiliates. All
rights reserved.
 # DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
 #
 # This code is free software; you can redistribute it and/or
modify it
@@ -23,12 +23,10 @@
 # questions.
 #

-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 +92,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)
+
+ STORE_SOURCE_REVISION_TARGET := hg-store-source-revision
+ CREATE_SOURCE_REVISION_TRACKER_TARGET :=
hg-create-source-revision-tracker

 else
   # Not using HG
@@ -106,28 +107,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

-all: store-source-revision create-source-revision-tracker

+
+
+$(eval $(call IncludeCustomExtension, SourceRevision-post.gmk))
+

+
+

Re: custom extension for make/SourceRevision.gmk

2018-08-20 Thread Christian Thalinger 



> On Jul 19, 2018, at 9:17 PM, Christian Thalinger  
> wrote:
> 
> 
> 
>> On Jul 19, 2018, at 2:31 PM, Erik Joelsson > > wrote:
>> 
>> I can do that. Do you have a bug?
>> 
> No.

Sorry, was on vacation… I don’t see the change in the repo. Did you file one?

>> /Erik
>> 
>> On 2018-07-19 10:57, Christian Thalinger wrote:
>>> 
>>> 
>>> On Thu, Jul 19, 2018 at 1:11 PM Erik Joelsson >> > wrote:
>>> This looks good to me, but will need coordination when pushed as I said 
>>> earlier.
>>> 
>>> 
>>> Do you want to push it so it’s easier?
>>> 
>>> /Erik
>>> 
>>> On 2018-07-19 10:04, Christian Thalinger wrote:
 
 
> On Jul 19, 2018, at 12:57 PM, Erik Joelsson  > wrote:
> 
> 
> 
> On 2018-07-19 09:54, Christian Thalinger wrote:
>> 
>> 
>>> On Jul 19, 2018, at 12:44 PM, Erik Joelsson >> > wrote:
>>> 
>>> 
>>> On 2018-07-19 09:16, Christian Thalinger wrote:
 
 
 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.
 
>>> Ah, the all target is tricking me and should be removed if not called 
>>> from anywhere. Then your suggested patch is good (except for missing 
>>> the :=).
>> 
>> Do you want me to remove the all: target?
>> 
> Yes, that would be a good cleanup to avoid confusion.
 
 How about this:
 
 diff --git a/make/SourceRevision.gmk b/make/SourceRevision.gmk
 index 10dd943..6d4a706 100644
 --- a/make/SourceRevision.gmk
 +++ b/make/SourceRevision.gmk
 @@ -1,5 +1,5 @@
  #
 -# Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
 +# Copyright (c) 2016, 2018, Oracle and/or its affiliates. All rights 
 reserved.
  # DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  #
  # This code is free software; you can redistribute it and/or modify it
 @@ -23,12 +23,10 @@
  # questions.
  #
  
 -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 +92,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)
 +
 +  STORE_SOURCE_REVISION_TARGET := hg-store-source-revision
 +  CREATE_SOURCE_REVISION_TRACKER_TARGET := 
 hg-create-source-revision-tracker
  
  else
# Not using HG
 @@ -106,28 +107,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
  
 -all: store-source-revision

Re: custom extension for make/SourceRevision.gmk

2018-07-19 Thread Christian Thalinger 



> On Jul 19, 2018, at 2:31 PM, Erik Joelsson  wrote:
> 
> I can do that. Do you have a bug?
> 
No.
> /Erik
> 
> On 2018-07-19 10:57, Christian Thalinger wrote:
>> 
>> 
>> On Thu, Jul 19, 2018 at 1:11 PM Erik Joelsson > > wrote:
>> This looks good to me, but will need coordination when pushed as I said 
>> earlier.
>> 
>> 
>> Do you want to push it so it’s easier?
>> 
>> /Erik
>> 
>> On 2018-07-19 10:04, Christian Thalinger wrote:
>>> 
>>> 
 On Jul 19, 2018, at 12:57 PM, Erik Joelsson >>> > wrote:
 
 
 
 On 2018-07-19 09:54, Christian Thalinger wrote:
> 
> 
>> On Jul 19, 2018, at 12:44 PM, Erik Joelsson > > wrote:
>> 
>> 
>> On 2018-07-19 09:16, Christian Thalinger wrote:
>>> 
>>> 
>>> 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.
>>> 
>> Ah, the all target is tricking me and should be removed if not called 
>> from anywhere. Then your suggested patch is good (except for missing the 
>> :=).
> 
> Do you want me to remove the all: target?
> 
 Yes, that would be a good cleanup to avoid confusion.
>>> 
>>> How about this:
>>> 
>>> diff --git a/make/SourceRevision.gmk b/make/SourceRevision.gmk
>>> index 10dd943..6d4a706 100644
>>> --- a/make/SourceRevision.gmk
>>> +++ b/make/SourceRevision.gmk
>>> @@ -1,5 +1,5 @@
>>>  #
>>> -# Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
>>> +# Copyright (c) 2016, 2018, Oracle and/or its affiliates. All rights 
>>> reserved.
>>>  # DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>>>  #
>>>  # This code is free software; you can redistribute it and/or modify it
>>> @@ -23,12 +23,10 @@
>>>  # questions.
>>>  #
>>>  
>>> -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 +92,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)
>>> +
>>> +  STORE_SOURCE_REVISION_TARGET := hg-store-source-revision
>>> +  CREATE_SOURCE_REVISION_TRACKER_TARGET := 
>>> hg-create-source-revision-tracker
>>>  
>>>  else
>>># Not using HG
>>> @@ -106,28 +107,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
>>>  
>>> -all: store-source-revision create-source-revision-tracker
>>> +
>>> +
>>> +$(eval $(call IncludeCustomExtension, SourceRevision-post.gmk))
>>> +
>>> +
>>> +
>>> +store-source-revision:

Re: custom extension for make/SourceRevision.gmk

2018-07-19 Thread Erik Joelsson 

I can do that. Do you have a bug?

/Erik


On 2018-07-19 10:57, Christian Thalinger wrote:



On Thu, Jul 19, 2018 at 1:11 PM Erik Joelsson 
mailto:erik.joels...@oracle.com>> wrote:


This looks good to me, but will need coordination when pushed as I
said earlier.


Do you want to push it so it’s easier?

/Erik


On 2018-07-19 10:04, Christian Thalinger wrote:




On Jul 19, 2018, at 12:57 PM, Erik Joelsson
mailto:erik.joels...@oracle.com>> wrote:



On 2018-07-19 09:54, Christian Thalinger wrote:




On Jul 19, 2018, at 12:44 PM, Erik Joelsson
mailto:erik.joels...@oracle.com>>
wrote:


On 2018-07-19 09:16, Christian Thalinger wrote:



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.


Ah, the all target is tricking me and should be removed if not
called from anywhere. Then your suggested patch is good
(except for missing the :=).


Do you want me to remove the all: target?


Yes, that would be a good cleanup to avoid confusion.


How about this:

diff --git a/make/SourceRevision.gmk b/make/SourceRevision.gmk
index 10dd943..6d4a706 100644
--- a/make/SourceRevision.gmk
+++ b/make/SourceRevision.gmk
@@ -1,5 +1,5 @@
 #
-# Copyright (c) 2016, Oracle and/or its affiliates. All rights
reserved.
+# Copyright (c) 2016, 2018, Oracle and/or its affiliates. All
rights reserved.
 # DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
 #
 # This code is free software; you can redistribute it and/or
modify it
@@ -23,12 +23,10 @@
 # questions.
 #


-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 +92,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)
+
+ STORE_SOURCE_REVISION_TARGET := hg-store-source-revision
+ CREATE_SOURCE_REVISION_TRACKER_TARGET :=
hg-create-source-revision-tracker


 else
# Not using HG
@@ -106,28 +107,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


-all: store-source-revision create-source-revision-tracker

+
+
+$(eval $(call IncludeCustomExtension, SourceRevision-post.gmk))
+

+
+
+store-source-revision: $(STORE_SOURCE_REVISION_TARGET)
+
+create-source-revision-tracker:
$(CREATE_SOURCE_REVISION_TRACKER_TARGET)


 FRC: # Force target


-.PHONY: all store-source-revision create-source-revision-tracker
+.PHONY: store-source-revision create-source-revision-tracker

Re: custom extension for make/SourceRevision.gmk

2018-07-19 Thread Christian Thalinger 
On Thu, Jul 19, 2018 at 1:11 PM Erik Joelsson 
wrote:

> This looks good to me, but will need coordination when pushed as I said
> earlier.
>

Do you want to push it so it’s easier?

/Erik
>
> On 2018-07-19 10:04, Christian Thalinger wrote:
>
>
>
> On Jul 19, 2018, at 12:57 PM, Erik Joelsson 
> wrote:
>
>
>
> On 2018-07-19 09:54, Christian Thalinger wrote:
>
>
>
> On Jul 19, 2018, at 12:44 PM, Erik Joelsson 
> wrote:
>
>
> On 2018-07-19 09:16, Christian Thalinger wrote:
>
>
>
> 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.
>
> Ah, the all target is tricking me and should be removed if not called from
> anywhere. Then your suggested patch is good (except for missing the :=).
>
>
> Do you want me to remove the all: target?
>
> Yes, that would be a good cleanup to avoid confusion.
>
>
> How about this:
>
> diff --git a/make/SourceRevision.gmk b/make/SourceRevision.gmk
> index 10dd943..6d4a706 100644
> --- a/make/SourceRevision.gmk
> +++ b/make/SourceRevision.gmk
> @@ -1,5 +1,5 @@
>  #
> -# Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
> +# Copyright (c) 2016, 2018, Oracle and/or its affiliates. All rights
> reserved.
>  # DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>  #
>  # This code is free software; you can redistribute it and/or modify it
> @@ -23,12 +23,10 @@
>  # questions.
>  #
>
>
> -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 +92,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)
> +
> +  STORE_SOURCE_REVISION_TARGET := hg-store-source-revision
> +  CREATE_SOURCE_REVISION_TRACKER_TARGET :=
> hg-create-source-revision-tracker
>
>
>  else
># Not using HG
> @@ -106,28 +107,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
>
>
> -all: store-source-revision create-source-revision-tracker
>
> +
> +
> +$(eval $(call IncludeCustomExtension, SourceRevision-post.gmk))
> +
>
> +
> +
> +store-source-revision: $(STORE_SOURCE_REVISION_TARGET)
> +
> +create-source-revision-tracker: $(CREATE_SOURCE_REVISION_TRACKER_TARGET)
>
>
>  FRC: # Force target
>
>
> -.PHONY: all store-source-revision create-source-revision-tracker
> +.PHONY: store-source-revision create-source-revision-tracker
>
>
>

Re: custom extension for make/SourceRevision.gmk

2018-07-19 Thread Erik Joelsson 
This looks good to me, but will need coordination when pushed as I said 
earlier.


/Erik


On 2018-07-19 10:04, Christian Thalinger wrote:



On Jul 19, 2018, at 12:57 PM, Erik Joelsson > wrote:




On 2018-07-19 09:54, Christian Thalinger wrote:



On Jul 19, 2018, at 12:44 PM, Erik Joelsson 
mailto:erik.joels...@oracle.com>> wrote:



On 2018-07-19 09:16, Christian Thalinger wrote:



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.

Ah, the all target is tricking me and should be removed if not 
called from anywhere. Then your suggested patch is good (except for 
missing the :=).


Do you want me to remove the all: target?


Yes, that would be a good cleanup to avoid confusion.


How about this:

diff --git a/make/SourceRevision.gmk b/make/SourceRevision.gmk
index 10dd943..6d4a706 100644
--- a/make/SourceRevision.gmk
+++ b/make/SourceRevision.gmk
@@ -1,5 +1,5 @@
 #
-# Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
+# Copyright (c) 2016, 2018, Oracle and/or its affiliates. All rights 
reserved.

 # DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
 #
 # This code is free software; you can redistribute it and/or modify it
@@ -23,12 +23,10 @@
 # questions.
 #


-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 +92,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)
+
+ STORE_SOURCE_REVISION_TARGET := hg-store-source-revision
+ CREATE_SOURCE_REVISION_TRACKER_TARGET := 
hg-create-source-revision-tracker



 else
# Not using HG
@@ -106,28 +107,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


-all: store-source-revision create-source-revision-tracker
+
+
+$(eval $(call IncludeCustomExtension, SourceRevision-post.gmk))
+
+
+
+store-source-revision: $(STORE_SOURCE_REVISION_TARGET)
+
+create-source-revision-tracker: $(CREATE_SOURCE_REVISION_TRACKER_TARGET)


 FRC: # Force target


-.PHONY: all store-source-revision create-source-revision-tracker
+.PHONY: store-source-revision create-source-revision-tracker

Re: custom extension for make/SourceRevision.gmk

2018-07-19 Thread Christian Thalinger 



> On Jul 19, 2018, at 12:57 PM, Erik Joelsson  wrote:
> 
> 
> 
> On 2018-07-19 09:54, Christian Thalinger wrote:
>> 
>> 
>>> On Jul 19, 2018, at 12:44 PM, Erik Joelsson >> > wrote:
>>> 
>>> 
>>> On 2018-07-19 09:16, Christian Thalinger wrote:
 
 
 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.
 
>>> Ah, the all target is tricking me and should be removed if not called from 
>>> anywhere. Then your suggested patch is good (except for missing the :=).
>> 
>> Do you want me to remove the all: target?
>> 
> Yes, that would be a good cleanup to avoid confusion.

How about this:

diff --git a/make/SourceRevision.gmk b/make/SourceRevision.gmk
index 10dd943..6d4a706 100644
--- a/make/SourceRevision.gmk
+++ b/make/SourceRevision.gmk
@@ -1,5 +1,5 @@
 #
-# Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
+# Copyright (c) 2016, 2018, Oracle and/or its affiliates. All rights reserved.
 # DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
 #
 # This code is free software; you can redistribute it and/or modify it
@@ -23,12 +23,10 @@
 # questions.
 #
 
-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 +92,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)
+
+  STORE_SOURCE_REVISION_TARGET := hg-store-source-revision
+  CREATE_SOURCE_REVISION_TRACKER_TARGET := hg-create-source-revision-tracker
 
 else
   # Not using HG
@@ -106,28 +107,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
 
-all: store-source-revision create-source-revision-tracker
+
+
+$(eval $(call IncludeCustomExtension, SourceRevision-post.gmk))
+
+
+
+store-source-revision: $(STORE_SOURCE_REVISION_TARGET)
+
+create-source-revision-tracker: $(CREATE_SOURCE_REVISION_TRACKER_TARGET)
 
 FRC: # Force target
 
-.PHONY: all store-source-revision create-source-revision-tracker
+.PHONY: store-source-revision create-source-revision-tracker

Re: custom extension for make/SourceRevision.gmk

2018-07-19 Thread Erik Joelsson 




On 2018-07-19 09:54, Christian Thalinger wrote:



On Jul 19, 2018, at 12:44 PM, Erik Joelsson > wrote:



On 2018-07-19 09:16, Christian Thalinger wrote:



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.

Ah, the all target is tricking me and should be removed if not called 
from anywhere. Then your suggested patch is good (except for missing 
the :=).


Do you want me to remove the all: target?


Yes, that would be a good cleanup to avoid confusion.

/Erik


/Erik

+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

Re: custom extension for make/SourceRevision.gmk

2018-07-19 Thread Christian Thalinger 



> On Jul 19, 2018, at 12:44 PM, Erik Joelsson  wrote:
> 
> 
> On 2018-07-19 09:16, Christian Thalinger wrote:
>> 
>> 
>> 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.
>> 
> Ah, the all target is tricking me and should be removed if not called from 
> anywhere. Then your suggested patch is good (except for missing the :=).

Do you want me to remove the all: target?

> 
> /Erik
 +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
>> 
>

Re: custom extension for make/SourceRevision.gmk

2018-07-19 Thread Christian Thalinger 



> On Jul 19, 2018, at 12:46 PM, Erik Joelsson  wrote:
> 
> One thing though. Since you are renaming the top file, we need to coordinate 
> pushing of this change since we are relying on the old name.

We can keep the old name, if you want, but that wouldn’t be very consistent 
with other files.

> 
> /Erik
> 
> 
> On 2018-07-19 09:44, Erik Joelsson wrote:
>> 
>> On 2018-07-19 09:16, Christian Thalinger wrote:
>>> 
>>> 
>>> 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.
>>> 
>> Ah, the all target is tricking me and should be removed if not called from 
>> anywhere. Then your suggested patch is good (except for missing the :=).
>> 
>> /Erik
> +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
>>> 
>> 
>

Re: custom extension for make/SourceRevision.gmk

2018-07-19 Thread Erik Joelsson 
One thing though. Since you are renaming the top file, we need to 
coordinate pushing of this change since we are relying on the old name.


/Erik


On 2018-07-19 09:44, Erik Joelsson wrote:


On 2018-07-19 09:16, Christian Thalinger wrote:



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.

Ah, the all target is tricking me and should be removed if not called 
from anywhere. Then your suggested patch is good (except for missing 
the :=).


/Erik

+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

Re: custom extension for make/SourceRevision.gmk

2018-07-19 Thread Erik Joelsson 



On 2018-07-19 09:16, Christian Thalinger wrote:



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.

Ah, the all target is tricking me and should be removed if not called 
from anywhere. Then your suggested patch is good (except for missing the 
:=).


/Erik

+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

Re: custom extension for make/SourceRevision.gmk

2018-07-19 Thread Christian Thalinger 



> On Jul 19, 2018, at 12:08 PM, Erik Joelsson  wrote:
> 
> Hello,
> On 2018-07-19 07:43, Christian Thalinger wrote:
>> 
>> 
>>> On Jul 18, 2018, at 3:28 PM, Christian Thalinger >> > wrote:
>>> 
>>> 
>>> 
 On Jul 18, 2018, at 1:46 PM, Erik Joelsson >>> > 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

Re: custom extension for make/SourceRevision.gmk

2018-07-19 Thread Erik Joelsson 

Hello,

On 2018-07-19 07:43, Christian Thalinger wrote:



On Jul 18, 2018, at 3:28 PM, Christian Thalinger 
mailto:cthalin...@twitter.com>> wrote:




On Jul 18, 2018, at 1:46 PM, Erik Joelsson > 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

Re: custom extension for make/SourceRevision.gmk

2018-07-19 Thread Christian Thalinger 



> On Jul 18, 2018, at 3:28 PM, Christian Thalinger  
> wrote:
> 
> 
> 
>> On Jul 18, 2018, at 1:46 PM, Erik Joelsson  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)
+
+  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))
+
+
+
+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

Re: custom extension for make/SourceRevision.gmk

2018-07-18 Thread Christian Thalinger 



> On Jul 18, 2018, at 1:46 PM, Erik Joelsson  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 :-)

> 
> /Erik
> 
> 
> On 2018-07-18 10:31, Christian Thalinger wrote:
>> Here at Twitter our OpenJDK clone lives in a GIT repository and we would 
>> like to use the source-revision feature of the release file.
>> 
>> I have all the custom extension logic in place to create the revision 
>> tracker:
>> 
>> $ cat 
>> build/macosx-x86_64-normal-server-release/support/src-rev/source-revision-tracker
>> .:ea60d3b1efc0
>> 
>> but the way make/SourceRevision.gmk is structured (the custom extensions is 
>> included at the top of the file) always triggers the warning:
>> 
>> $ make
>> Building target 'default (exploded-image)' in configuration 
>> 'macosx-x86_64-normal-server-release'
>> Warning: No mercurial configuration present and no .src-rev
>> Finished building target 'default (exploded-image)' in configuration 
>> 'macosx-x86_64-normal-server-release'
>> 
>> Is there a way so silence the warning without restructuring the upstream 
>> file?  And if no, can we change it so it works?
>

Re: custom extension for make/SourceRevision.gmk

2018-07-18 Thread Erik Joelsson 

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?


/Erik


On 2018-07-18 10:31, Christian Thalinger wrote:

Here at Twitter our OpenJDK clone lives in a GIT repository and we would like 
to use the source-revision feature of the release file.

I have all the custom extension logic in place to create the revision tracker:

$ cat 
build/macosx-x86_64-normal-server-release/support/src-rev/source-revision-tracker
.:ea60d3b1efc0

but the way make/SourceRevision.gmk is structured (the custom extensions is 
included at the top of the file) always triggers the warning:

$ make
Building target 'default (exploded-image)' in configuration 
'macosx-x86_64-normal-server-release'
Warning: No mercurial configuration present and no .src-rev
Finished building target 'default (exploded-image)' in configuration 
'macosx-x86_64-normal-server-release'

Is there a way so silence the warning without restructuring the upstream file?  
And if no, can we change it so it works?

custom extension for make/SourceRevision.gmk

2018-07-18 Thread Christian Thalinger 
Here at Twitter our OpenJDK clone lives in a GIT repository and we would like 
to use the source-revision feature of the release file.

I have all the custom extension logic in place to create the revision tracker:

$ cat 
build/macosx-x86_64-normal-server-release/support/src-rev/source-revision-tracker
 
.:ea60d3b1efc0

but the way make/SourceRevision.gmk is structured (the custom extensions is 
included at the top of the file) always triggers the warning:

$ make
Building target 'default (exploded-image)' in configuration 
'macosx-x86_64-normal-server-release'
Warning: No mercurial configuration present and no .src-rev
Finished building target 'default (exploded-image)' in configuration 
'macosx-x86_64-normal-server-release'

Is there a way so silence the warning without restructuring the upstream file?  
And if no, can we change it so it works?