RE: [11u] RFR: 8223678: Add Visual Studio Code workspace generation support (for native code)

2020-03-11 Thread Langer, Christoph
Hi Volker,

> > Out of interest, is the AssertEquals macro something worth backporting
> > at some point? Generally, I find its worthwhile if people are going to
> > be doing the same replacement in multiple backports.
> >
> 
> I actually wanted to answer something like "..lets wait until we get
> another backport requiring this.." but already before answering we
> already ran into such a case (see my answer to the other thread
> "8232748: "Build static versions of certain JDK libraries"). The only
> problem is that "8189861: Refactor CacheFind" which introduced
> AssertEquals is not a trivial downport.
> 
> Please let me know what you think?

I've just proposed the backport of "8189861: Refactor CacheFind". Can you 
rebase your change on that and see if it works?

Best regards
Christoph



Re: [11u] RFR: 8223678: Add Visual Studio Code workspace generation support (for native code)

2020-02-28 Thread Volker Simonis
On Wed, Feb 26, 2020 at 3:49 PM Andrew Hughes  wrote:
>
> On 26/02/2020 10:59, Volker Simonis wrote:
> > Hi,
> > I'd like to downport the support for Visual Studio Code project
> > creation to 11u. I think we will have to support 11u for quite some
> > years and it makes sense to have as good as possible tool support in
> > 11u as well:
> >
> > https://bugs.openjdk.java.net/browse/JDK-8223678
> > http://cr.openjdk.java.net/~simonis/webrevs/2020/8223678/
> >
> > This change is the third in a series of three changes (8210459,
> > 8218807, 8223678) which are required to get VS Code support in 11u.
> > The first two changes (8210459, 8218807) have already been approved
> > and pushed.
> >
> > This change only touch make files and actually only add new features
> > to the make system, so the normal build results shouldn't be affected.
> > I've tested that the build still works on Linux and Windows and "make
> > test-make" succeeds.
> >
> > This change actually creates the VS Code project files (i.e. "make
> > vscode-project-ccls"). See
> > http://hg.openjdk.java.net/jdk/jdk/raw-file/tip/doc/ide.html for more
> > details.
> >
> > It applies cleanly except the following two hunks. The first one had to
> > be moved back from "make/common/Utils.gmk" to
> > "make/common/MakeBase.gmk" where it still applies cleanly, because the
> > "RelativePath" macro is still in "MakeBase.gmk" in  jdk11u.
> >
> > ORIGINAL
> > 
> > diff -r a82a367b2d8c -r 2ae056696b15 make/common/Utils.gmk
> > --- a/make/common/Utils.gmkMon Jun 03 17:14:23 2019 -0700
> > +++ b/make/common/Utils.gmkMon Jun 03 10:28:03 2019 +0200
> > @@ -122,7 +122,8 @@
> >  # $2 - Directory to compute the relative path from
> >  RelativePath = \
> >  $(eval $1_prefix := $(call FindCommonPathPrefix, $1, $2)) \
> > -$(eval $1_dotdots := $(call DirToDotDot, $(patsubst $($(strip
> > $1)_prefix)/%, %, $2))) \
> > +$(eval $1_dotdots := $(call DirToDotDot, $(patsubst $($(strip
> > $1)_prefix)%, %, $2))) \
> > +$(eval $1_dotdots := $(if $($(strip $1)_dotdots),$($(strip
> > $1)_dotdots),.)) \
> >  $(eval $1_suffix := $(patsubst $($(strip $1)_prefix)/%, %, $1)) \
> >  $($(strip $1)_dotdots)/$($(strip $1)_suffix)
> >
> > JDK11u
> > ==
> > diff -r 8599975f5b33 make/common/MakeBase.gmk
> > --- a/make/common/MakeBase.gmkTue Feb 12 08:40:43 2019 +0100
> > +++ b/make/common/MakeBase.gmkMon Dec 23 22:10:46 2019 +0100
> > @@ -611,7 +611,8 @@
> >  # $2 - Directory to compute the relative path from
> >  RelativePath = \
> >  $(eval $1_prefix := $(call FindCommonPathPrefix, $1, $2)) \
> > -$(eval $1_dotdots := $(call DirToDotDot, $(patsubst $($(strip
> > $1)_prefix)/%, %, $2))) \
> > +$(eval $1_dotdots := $(call DirToDotDot, $(patsubst $($(strip
> > $1)_prefix)%, %, $2))) \
> > +$(eval $1_dotdots := $(if $($(strip $1)_dotdots),$($(strip
> > $1)_dotdots),.)) \
> >  $(eval $1_suffix := $(patsubst $($(strip $1)_prefix)/%, %, $1)) \
> >  $($(strip $1)_dotdots)/$($(strip $1)_suffix)
> >
> > For the second changed hunk, the simple call of the "AssertEquals"
> > macro had to be renamed to "assert-equals" and wrapped with "eval"
> > because the corresponding macros haven't been updated in jdk11u yet.
> > Notice that these are only test changes (for testing the make system
> > itself) and don't effect the build at all.
> >
> > ORIGINAL
> > 
> > diff -r a82a367b2d8c -r 2ae056696b15 test/make/TestMakeBase.gmk
> > --- a/test/make/TestMakeBase.gmkMon Jun 03 17:14:23 2019 -0700
> > +++ b/test/make/TestMakeBase.gmkMon Jun 03 10:28:03 2019 +0200
> > @@ -361,6 +361,18 @@
> >  RelativePath, \
> >  )
> >
> > +$(call AssertEquals, \
> > +$(call RelativePath, /foo/bar/baz/banan/kung, /foo/bar/baz), \
> > +./banan/kung, \
> > +RelativePath, \
> > +)
> > +
> > +$(call AssertEquals, \
> > +$(call RelativePath, /foo/bar/baz/banan/kung, /foo/bar/baz/), \
> > +./banan/kung, \
> > +RelativePath, \
> > +)
> >
> > JDK11u
> > ==
> > diff -r 8599975f5b33 test/make/TestMakeBase.gmk
> > --- a/test/make/TestMakeBase.gmkTue Feb 12 08:40:43 2019 +0100
> > +++ b/test/make/TestMakeBase.gmkMon Dec 23 22:10:46 2019 +0100
> > @@ -325,13 +325,25 @@
> >  RelativePath, \
> >  ))
> >
> > +$(eval $(call assert-equals, \
> > +$(call RelativePath, /foo/bar/baz/banan/kung, /foo/bar/baz), \
> > +./banan/kung, \
> > +RelativePath, \
> > +))
> > +
> > +$(eval $(call assert-equals, \
> > +$(call RelativePath, /foo/bar/baz/banan/kung, /foo/bar/baz/), \
> > +./banan/kung, \
> > +RelativePath, \
> > +))
> >
>
> Thanks for the comprehensive description of the changes. It looks fine
> to me, though I've only eyeballed the changes & backporting, as I'm not
> a potential consumer of the new rules. From that perspective, it doesn't
> look like it will disrupt any existing builds.
>

Hi Andrew,

thanks for looking at this downport. I've obviously verified that the
generated VS-Code project fi

Re: [11u] RFR: 8223678: Add Visual Studio Code workspace generation support (for native code)

2020-02-26 Thread Andrew Hughes
On 26/02/2020 10:59, Volker Simonis wrote:
> Hi,
> I'd like to downport the support for Visual Studio Code project
> creation to 11u. I think we will have to support 11u for quite some
> years and it makes sense to have as good as possible tool support in
> 11u as well:
> 
> https://bugs.openjdk.java.net/browse/JDK-8223678
> http://cr.openjdk.java.net/~simonis/webrevs/2020/8223678/
> 
> This change is the third in a series of three changes (8210459,
> 8218807, 8223678) which are required to get VS Code support in 11u.
> The first two changes (8210459, 8218807) have already been approved
> and pushed.
> 
> This change only touch make files and actually only add new features
> to the make system, so the normal build results shouldn't be affected.
> I've tested that the build still works on Linux and Windows and "make
> test-make" succeeds.
> 
> This change actually creates the VS Code project files (i.e. "make
> vscode-project-ccls"). See
> http://hg.openjdk.java.net/jdk/jdk/raw-file/tip/doc/ide.html for more
> details.
> 
> It applies cleanly except the following two hunks. The first one had to
> be moved back from "make/common/Utils.gmk" to
> "make/common/MakeBase.gmk" where it still applies cleanly, because the
> "RelativePath" macro is still in "MakeBase.gmk" in  jdk11u.
> 
> ORIGINAL
> 
> diff -r a82a367b2d8c -r 2ae056696b15 make/common/Utils.gmk
> --- a/make/common/Utils.gmkMon Jun 03 17:14:23 2019 -0700
> +++ b/make/common/Utils.gmkMon Jun 03 10:28:03 2019 +0200
> @@ -122,7 +122,8 @@
>  # $2 - Directory to compute the relative path from
>  RelativePath = \
>  $(eval $1_prefix := $(call FindCommonPathPrefix, $1, $2)) \
> -$(eval $1_dotdots := $(call DirToDotDot, $(patsubst $($(strip
> $1)_prefix)/%, %, $2))) \
> +$(eval $1_dotdots := $(call DirToDotDot, $(patsubst $($(strip
> $1)_prefix)%, %, $2))) \
> +$(eval $1_dotdots := $(if $($(strip $1)_dotdots),$($(strip
> $1)_dotdots),.)) \
>  $(eval $1_suffix := $(patsubst $($(strip $1)_prefix)/%, %, $1)) \
>  $($(strip $1)_dotdots)/$($(strip $1)_suffix)
> 
> JDK11u
> ==
> diff -r 8599975f5b33 make/common/MakeBase.gmk
> --- a/make/common/MakeBase.gmkTue Feb 12 08:40:43 2019 +0100
> +++ b/make/common/MakeBase.gmkMon Dec 23 22:10:46 2019 +0100
> @@ -611,7 +611,8 @@
>  # $2 - Directory to compute the relative path from
>  RelativePath = \
>  $(eval $1_prefix := $(call FindCommonPathPrefix, $1, $2)) \
> -$(eval $1_dotdots := $(call DirToDotDot, $(patsubst $($(strip
> $1)_prefix)/%, %, $2))) \
> +$(eval $1_dotdots := $(call DirToDotDot, $(patsubst $($(strip
> $1)_prefix)%, %, $2))) \
> +$(eval $1_dotdots := $(if $($(strip $1)_dotdots),$($(strip
> $1)_dotdots),.)) \
>  $(eval $1_suffix := $(patsubst $($(strip $1)_prefix)/%, %, $1)) \
>  $($(strip $1)_dotdots)/$($(strip $1)_suffix)
> 
> For the second changed hunk, the simple call of the "AssertEquals"
> macro had to be renamed to "assert-equals" and wrapped with "eval"
> because the corresponding macros haven't been updated in jdk11u yet.
> Notice that these are only test changes (for testing the make system
> itself) and don't effect the build at all.
> 
> ORIGINAL
> 
> diff -r a82a367b2d8c -r 2ae056696b15 test/make/TestMakeBase.gmk
> --- a/test/make/TestMakeBase.gmkMon Jun 03 17:14:23 2019 -0700
> +++ b/test/make/TestMakeBase.gmkMon Jun 03 10:28:03 2019 +0200
> @@ -361,6 +361,18 @@
>  RelativePath, \
>  )
> 
> +$(call AssertEquals, \
> +$(call RelativePath, /foo/bar/baz/banan/kung, /foo/bar/baz), \
> +./banan/kung, \
> +RelativePath, \
> +)
> +
> +$(call AssertEquals, \
> +$(call RelativePath, /foo/bar/baz/banan/kung, /foo/bar/baz/), \
> +./banan/kung, \
> +RelativePath, \
> +)
> 
> JDK11u
> ==
> diff -r 8599975f5b33 test/make/TestMakeBase.gmk
> --- a/test/make/TestMakeBase.gmkTue Feb 12 08:40:43 2019 +0100
> +++ b/test/make/TestMakeBase.gmkMon Dec 23 22:10:46 2019 +0100
> @@ -325,13 +325,25 @@
>  RelativePath, \
>  ))
> 
> +$(eval $(call assert-equals, \
> +$(call RelativePath, /foo/bar/baz/banan/kung, /foo/bar/baz), \
> +./banan/kung, \
> +RelativePath, \
> +))
> +
> +$(eval $(call assert-equals, \
> +$(call RelativePath, /foo/bar/baz/banan/kung, /foo/bar/baz/), \
> +./banan/kung, \
> +RelativePath, \
> +))
> 

Thanks for the comprehensive description of the changes. It looks fine
to me, though I've only eyeballed the changes & backporting, as I'm not
a potential consumer of the new rules. From that perspective, it doesn't
look like it will disrupt any existing builds.

Out of interest, is the AssertEquals macro something worth backporting
at some point? Generally, I find its worthwhile if people are going to
be doing the same replacement in multiple backports.

Thanks,
-- 
Andrew :)

Senior Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)

PGP Key: ed25519/0xCFDA0F9B35964222 (hkp://keys.gnupg.net)
Fingerprint = 5132 579D D154 0ED2 3E04