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 

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  

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

2020-02-26 Thread Volker Simonis
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, \
+))


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

2019-06-03 Thread Erik Joelsson

Looks good.

/Erik

On 2019-06-03 01:36, Robin Westberg wrote:

Hi Erik,


On 31 May 2019, at 18:15, Erik Joelsson  wrote:

Hello Robin,

On 2019-05-31 05:26, Robin Westberg wrote:

Hi Erik,


On 29 May 2019, at 17:22, Erik Joelsson  wrote:

Thanks, looks good!

Thanks for reviewing! Unfortunately I had to do a few more general changes to get 
the “ccls" indexer working properly as well, hope you don’t mind taking another 
look:

Webrevs:
  - Full: http://cr.openjdk.java.net/~rwestberg/8223678/webrev.03/
  - Inc: http://cr.openjdk.java.net/~rwestberg/8223678/webrev.02-03/

Looks good!

I sure appreciate adding tests when modifying the hairier utility macros.

Thanks again for reviewing!

I did a small tweak to the settings of one of the optional indexers (ccls), 
perhaps not something that needs a detailed review, but here’s the final 
version that I’m thinking of pushing:

Webrev:
  - Full: https://cr.openjdk.java.net/~rwestberg/8223678/webrev.04/
  - Inc: http://cr.openjdk.java.net/~rwestberg/8223678/webrev.03-04/

Best regards,
Robin


/Erik


Best regards,
Robin


/Erik

On 2019-05-29 06:52, Robin Westberg wrote:

Hi Erik,

Thanks for taking a look!


On 28 May 2019, at 15:52, Erik Joelsson  wrote:

Hello Robin,

Looks good.

Adding of ide.md is very nice, but could you try to format it like testing.md 
and building.md in regards to line lengths? I believe we are trying for 80 
chars in those to make them read reasonably in a standard editor.

Sure, did a bit of reflowing and updated some formatting in order to look more 
like the other .md files.

Webrevs:
  - Full: https://cr.openjdk.java.net/~rwestberg/8223678/webrev.02/
  - Inc: https://cr.openjdk.java.net/~rwestberg/8223678/webrev.01-02/

Best regards,
Robin


/Erik

On 2019-05-27 09:03, Robin Westberg wrote:

Hi all,

Please review this change that adds build system support for generating a 
Visual Studio Code workspace configured for working with the JDK native code. 
It configures the default C/C++ IntelliSense Engine to allow code 
completion/navigation and similar features. It also configures two executable 
targets (gtestLauncher and java) that can be built and debugged from the IDE.

The main target is "make vscode-project”, additional information can be found 
in doc/ide.[md|html].

Issue:
https://bugs.openjdk.java.net/browse/JDK-8223678

Webrev:
https://cr.openjdk.java.net/~rwestberg/8223678/webrev.01/

Testing:
Manual testing on Linux, MacOS and Windows

Thanks Erik Joelsson for taking a look at an earlier version of it!

Best regards,
Robin


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

2019-06-03 Thread Robin Westberg
Hi Erik,

> On 31 May 2019, at 18:15, Erik Joelsson  wrote:
> 
> Hello Robin,
> 
> On 2019-05-31 05:26, Robin Westberg wrote:
>> Hi Erik,
>> 
>>> On 29 May 2019, at 17:22, Erik Joelsson  wrote:
>>> 
>>> Thanks, looks good!
>> Thanks for reviewing! Unfortunately I had to do a few more general changes 
>> to get the “ccls" indexer working properly as well, hope you don’t mind 
>> taking another look:
>> 
>> Webrevs:
>>  - Full: http://cr.openjdk.java.net/~rwestberg/8223678/webrev.03/
>>  - Inc: http://cr.openjdk.java.net/~rwestberg/8223678/webrev.02-03/
> 
> Looks good!
> 
> I sure appreciate adding tests when modifying the hairier utility macros.

Thanks again for reviewing!

I did a small tweak to the settings of one of the optional indexers (ccls), 
perhaps not something that needs a detailed review, but here’s the final 
version that I’m thinking of pushing:

Webrev:
 - Full: https://cr.openjdk.java.net/~rwestberg/8223678/webrev.04/
 - Inc: http://cr.openjdk.java.net/~rwestberg/8223678/webrev.03-04/

Best regards,
Robin

> 
> /Erik
> 
>> Best regards,
>> Robin
>> 
>>> /Erik
>>> 
>>> On 2019-05-29 06:52, Robin Westberg wrote:
 Hi Erik,
 
 Thanks for taking a look!
 
> On 28 May 2019, at 15:52, Erik Joelsson  wrote:
> 
> Hello Robin,
> 
> Looks good.
> 
> Adding of ide.md is very nice, but could you try to format it like 
> testing.md and building.md in regards to line lengths? I believe we are 
> trying for 80 chars in those to make them read reasonably in a standard 
> editor.
 Sure, did a bit of reflowing and updated some formatting in order to look 
 more like the other .md files.
 
 Webrevs:
  - Full: https://cr.openjdk.java.net/~rwestberg/8223678/webrev.02/
  - Inc: https://cr.openjdk.java.net/~rwestberg/8223678/webrev.01-02/
 
 Best regards,
 Robin
 
> /Erik
> 
> On 2019-05-27 09:03, Robin Westberg wrote:
>> Hi all,
>> 
>> Please review this change that adds build system support for generating 
>> a Visual Studio Code workspace configured for working with the JDK 
>> native code. It configures the default C/C++ IntelliSense Engine to 
>> allow code completion/navigation and similar features. It also 
>> configures two executable targets (gtestLauncher and java) that can be 
>> built and debugged from the IDE.
>> 
>> The main target is "make vscode-project”, additional information can be 
>> found in doc/ide.[md|html].
>> 
>> Issue:
>> https://bugs.openjdk.java.net/browse/JDK-8223678
>> 
>> Webrev:
>> https://cr.openjdk.java.net/~rwestberg/8223678/webrev.01/
>> 
>> Testing:
>> Manual testing on Linux, MacOS and Windows
>> 
>> Thanks Erik Joelsson for taking a look at an earlier version of it!
>> 
>> Best regards,
>> Robin



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

2019-05-31 Thread Erik Joelsson

Hello Robin,

On 2019-05-31 05:26, Robin Westberg wrote:

Hi Erik,


On 29 May 2019, at 17:22, Erik Joelsson  wrote:

Thanks, looks good!

Thanks for reviewing! Unfortunately I had to do a few more general changes to get 
the “ccls" indexer working properly as well, hope you don’t mind taking another 
look:

Webrevs:
  - Full: http://cr.openjdk.java.net/~rwestberg/8223678/webrev.03/
  - Inc: http://cr.openjdk.java.net/~rwestberg/8223678/webrev.02-03/


Looks good!

I sure appreciate adding tests when modifying the hairier utility macros.

/Erik


Best regards,
Robin


/Erik

On 2019-05-29 06:52, Robin Westberg wrote:

Hi Erik,

Thanks for taking a look!


On 28 May 2019, at 15:52, Erik Joelsson  wrote:

Hello Robin,

Looks good.

Adding of ide.md is very nice, but could you try to format it like testing.md 
and building.md in regards to line lengths? I believe we are trying for 80 
chars in those to make them read reasonably in a standard editor.

Sure, did a bit of reflowing and updated some formatting in order to look more 
like the other .md files.

Webrevs:
  - Full: https://cr.openjdk.java.net/~rwestberg/8223678/webrev.02/
  - Inc: https://cr.openjdk.java.net/~rwestberg/8223678/webrev.01-02/

Best regards,
Robin


/Erik

On 2019-05-27 09:03, Robin Westberg wrote:

Hi all,

Please review this change that adds build system support for generating a 
Visual Studio Code workspace configured for working with the JDK native code. 
It configures the default C/C++ IntelliSense Engine to allow code 
completion/navigation and similar features. It also configures two executable 
targets (gtestLauncher and java) that can be built and debugged from the IDE.

The main target is "make vscode-project”, additional information can be found 
in doc/ide.[md|html].

Issue:
https://bugs.openjdk.java.net/browse/JDK-8223678

Webrev:
https://cr.openjdk.java.net/~rwestberg/8223678/webrev.01/

Testing:
Manual testing on Linux, MacOS and Windows

Thanks Erik Joelsson for taking a look at an earlier version of it!

Best regards,
Robin


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

2019-05-31 Thread Robin Westberg
Hi Volker,

> On 29 May 2019, at 16:01, Volker Simonis  wrote:
> 
> On Wed, May 29, 2019 at 3:43 PM Robin Westberg
>  wrote:
>> 
>> Hi Volker,
>> 
>>> On 28 May 2019, at 17:33, Volker Simonis  wrote:
>>> 
>>> Hi Robin,
>>> 
>>> thanks a lot for doing this!
>>> 
>>> I have just a quick question. Do you know if any of the VSC indexers
>>> (default, clangd) support call hierarchies (i.e. "called by",
>>> "callers" of a function/method) and "used by" for variables/class
>>> fields?
>> 
>> Sure, I can make a quick summary of the various pros and cons of the 
>> indexers that I’ve found so far. They are all moving pretty fast though, so 
>> didn’t think it was a good fit for the documentation file.
>> 
>> In general, VS Code itself only just recently gained proper support for 
>> displaying call hierarchies: 
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__code.visualstudio.com_updates_v1-5F33-23-5Fcall-2Dhierarchy=DwIFaQ=RoP1YumCXCgaWHvlZYR8PZh8Bv7qIrMUB65eapI_JnE=UJlIwvo0Thp_BYS_NWFT0ryBTIkcL2KhsFz8CKsa4GY=82pYT4_vb0KoCg2J7f8ZHebXioM2IbSDUpVgSuUAz-A=yY4u7WPyw8IMAQOytIDpU3MePzQMuGgCTyIP4XuHXbI=
>>  - but alternative indexers have worked around this by showing them 
>> differently.
>> 
>> Default (Microsoft - C/C++ for Visual Studio Code)
>> + Easy to setup, as no additional dependencies are needed
>> + Good “go to definition”, the only one that can make sense of the 
>> template+macro stuff in log.hpp for example
>> - Find references (used by) not done yet (said to be coming in the June 
>> update: 
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_microsoft_vscode-2Dcpptools_issues_15=DwIFaQ=RoP1YumCXCgaWHvlZYR8PZh8Bv7qIrMUB65eapI_JnE=UJlIwvo0Thp_BYS_NWFT0ryBTIkcL2KhsFz8CKsa4GY=82pYT4_vb0KoCg2J7f8ZHebXioM2IbSDUpVgSuUAz-A=usQi8Sluzbg7sm51fLwBgFVSkY8uj_ZkOgFhT6clIjw=)
>> - Call hierarchies also not there (scheduled for September apparently: 
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_microsoft_vscode-2Dcpptools_issues_16=DwIFaQ=RoP1YumCXCgaWHvlZYR8PZh8Bv7qIrMUB65eapI_JnE=UJlIwvo0Thp_BYS_NWFT0ryBTIkcL2KhsFz8CKsa4GY=82pYT4_vb0KoCg2J7f8ZHebXioM2IbSDUpVgSuUAz-A=0nJoexh1jJuqrNQ5bhxtpwdzS9LhGYhzKyEmFbM1D8A=)
>> 
>> clangd
>> + Actively developed and part of the llvm/clang project
>> + Support for find references
>> - ..but not call hierarchies yet
>> - Full project indexing is still flagged as experimental, and currently 
>> seems to fail when editing commonly used header files for example
>> 
>> Full feature list: 
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__clang.llvm.org_extra_clangd_Features.html-23complete-2Dlist-2Dof-2Dfeatures=DwIFaQ=RoP1YumCXCgaWHvlZYR8PZh8Bv7qIrMUB65eapI_JnE=UJlIwvo0Thp_BYS_NWFT0ryBTIkcL2KhsFz8CKsa4GY=82pYT4_vb0KoCg2J7f8ZHebXioM2IbSDUpVgSuUAz-A=-59_vkfX4TV8qO7PoHopjLBC1c2Qu-HjneQnNM49Zps=
>> 
>> rtags
>> This is currently the most feature-complete indexer I think. But the VS Code 
>> integration is a bit minimal and not part of the rtags project itself, and 
>> it is missing things like indexer progress.
>> + The indexer is actively developed and has been around for quite some time
>> - You will probably have to build the indexer from source
>> + Full support for call hierarchies and find references
>> - VS Code integration a bit limited, missing convenience features like 
>> switch between header/source file, showing method/class documentation on 
>> hover.
>> 
>> There are even more indexers of course, a promising one used to be “cquery", 
>> but that project seems to be defunct now. It lives on in the “ccls" indexer, 
>> but ithat one is a bit tricky to configure unless you use clang for building 
>> the JDK as well.
>> 
>> So in summary, after the summer the default indexer might be the obvious 
>> best choice, but right now it depends on which features you think are the 
>> most important I guess..
>> 
> 
> Hi Robin,
> 
> thanks a lot for the great summary! Incidentally I've just did some
> Google research as well and basically came to the same conclusion.
> "Cquery" seemed quite promising and also pretends to have call
> hierarchies. But it seems to be defunct and I've also found some bug
> reports about problems with the call hierarchies.

Yeah, cquery did work okay when I tried it, but had some issues. However, the 
“spiritual” successor ccls has fixed all the issues I had with cquery, I spent 
a bit more time on configuring it today and finally figured out why it didn’t 
work!

> For me "Called Hierarchy", and "Find References" (in this order) are
> really the most important IDE features. I'm using Eclipse and if you
> setup your HotSpot project correctly, these features work pretty well
> and reliably.
> 
> Have you personally tried the rtags "call hierarchies" / "find
> references" support in VSC? From your summary it sounds like it's
> worth giving it a try.

Only tried it a bit, but I know other HotSpot developers that use it with emacs 
/ vim. But the VS Code integration is really bare-bones, so a bit hard to use 
it 

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

2019-05-31 Thread Robin Westberg
Hi Erik,

> On 29 May 2019, at 17:22, Erik Joelsson  wrote:
> 
> Thanks, looks good!

Thanks for reviewing! Unfortunately I had to do a few more general changes to 
get the “ccls" indexer working properly as well, hope you don’t mind taking 
another look:

Webrevs:
 - Full: http://cr.openjdk.java.net/~rwestberg/8223678/webrev.03/
 - Inc: http://cr.openjdk.java.net/~rwestberg/8223678/webrev.02-03/

Best regards,
Robin

> 
> /Erik
> 
> On 2019-05-29 06:52, Robin Westberg wrote:
>> Hi Erik,
>> 
>> Thanks for taking a look!
>> 
>>> On 28 May 2019, at 15:52, Erik Joelsson  wrote:
>>> 
>>> Hello Robin,
>>> 
>>> Looks good.
>>> 
>>> Adding of ide.md is very nice, but could you try to format it like 
>>> testing.md and building.md in regards to line lengths? I believe we are 
>>> trying for 80 chars in those to make them read reasonably in a standard 
>>> editor.
>> Sure, did a bit of reflowing and updated some formatting in order to look 
>> more like the other .md files.
>> 
>> Webrevs:
>>  - Full: https://cr.openjdk.java.net/~rwestberg/8223678/webrev.02/
>>  - Inc: https://cr.openjdk.java.net/~rwestberg/8223678/webrev.01-02/
>> 
>> Best regards,
>> Robin
>> 
>>> /Erik
>>> 
>>> On 2019-05-27 09:03, Robin Westberg wrote:
 Hi all,
 
 Please review this change that adds build system support for generating a 
 Visual Studio Code workspace configured for working with the JDK native 
 code. It configures the default C/C++ IntelliSense Engine to allow code 
 completion/navigation and similar features. It also configures two 
 executable targets (gtestLauncher and java) that can be built and debugged 
 from the IDE.
 
 The main target is "make vscode-project”, additional information can be 
 found in doc/ide.[md|html].
 
 Issue:
 https://bugs.openjdk.java.net/browse/JDK-8223678
 
 Webrev:
 https://cr.openjdk.java.net/~rwestberg/8223678/webrev.01/
 
 Testing:
 Manual testing on Linux, MacOS and Windows
 
 Thanks Erik Joelsson for taking a look at an earlier version of it!
 
 Best regards,
 Robin



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

2019-05-29 Thread Erik Joelsson

Thanks, looks good!

/Erik

On 2019-05-29 06:52, Robin Westberg wrote:

Hi Erik,

Thanks for taking a look!


On 28 May 2019, at 15:52, Erik Joelsson  wrote:

Hello Robin,

Looks good.

Adding of ide.md is very nice, but could you try to format it like testing.md 
and building.md in regards to line lengths? I believe we are trying for 80 
chars in those to make them read reasonably in a standard editor.

Sure, did a bit of reflowing and updated some formatting in order to look more 
like the other .md files.

Webrevs:
  - Full: https://cr.openjdk.java.net/~rwestberg/8223678/webrev.02/
  - Inc: https://cr.openjdk.java.net/~rwestberg/8223678/webrev.01-02/

Best regards,
Robin


/Erik

On 2019-05-27 09:03, Robin Westberg wrote:

Hi all,

Please review this change that adds build system support for generating a 
Visual Studio Code workspace configured for working with the JDK native code. 
It configures the default C/C++ IntelliSense Engine to allow code 
completion/navigation and similar features. It also configures two executable 
targets (gtestLauncher and java) that can be built and debugged from the IDE.

The main target is "make vscode-project”, additional information can be found 
in doc/ide.[md|html].

Issue:
https://bugs.openjdk.java.net/browse/JDK-8223678

Webrev:
https://cr.openjdk.java.net/~rwestberg/8223678/webrev.01/

Testing:
Manual testing on Linux, MacOS and Windows

Thanks Erik Joelsson for taking a look at an earlier version of it!

Best regards,
Robin


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

2019-05-29 Thread Volker Simonis
On Wed, May 29, 2019 at 3:43 PM Robin Westberg
 wrote:
>
> Hi Volker,
>
> > On 28 May 2019, at 17:33, Volker Simonis  wrote:
> >
> > Hi Robin,
> >
> > thanks a lot for doing this!
> >
> > I have just a quick question. Do you know if any of the VSC indexers
> > (default, clangd) support call hierarchies (i.e. "called by",
> > "callers" of a function/method) and "used by" for variables/class
> > fields?
>
> Sure, I can make a quick summary of the various pros and cons of the indexers 
> that I’ve found so far. They are all moving pretty fast though, so didn’t 
> think it was a good fit for the documentation file.
>
> In general, VS Code itself only just recently gained proper support for 
> displaying call hierarchies: 
> https://code.visualstudio.com/updates/v1_33#_call-hierarchy - but alternative 
> indexers have worked around this by showing them differently.
>
> Default (Microsoft - C/C++ for Visual Studio Code)
> + Easy to setup, as no additional dependencies are needed
> + Good “go to definition”, the only one that can make sense of the 
> template+macro stuff in log.hpp for example
> - Find references (used by) not done yet (said to be coming in the June 
> update: https://github.com/microsoft/vscode-cpptools/issues/15)
> - Call hierarchies also not there (scheduled for September apparently: 
> https://github.com/microsoft/vscode-cpptools/issues/16)
>
> clangd
> + Actively developed and part of the llvm/clang project
> + Support for find references
> - ..but not call hierarchies yet
> - Full project indexing is still flagged as experimental, and currently seems 
> to fail when editing commonly used header files for example
>
> Full feature list: 
> https://clang.llvm.org/extra/clangd/Features.html#complete-list-of-features
>
> rtags
> This is currently the most feature-complete indexer I think. But the VS Code 
> integration is a bit minimal and not part of the rtags project itself, and it 
> is missing things like indexer progress.
> + The indexer is actively developed and has been around for quite some time
> - You will probably have to build the indexer from source
> + Full support for call hierarchies and find references
> - VS Code integration a bit limited, missing convenience features like switch 
> between header/source file, showing method/class documentation on hover.
>
> There are even more indexers of course, a promising one used to be “cquery", 
> but that project seems to be defunct now. It lives on in the “ccls" indexer, 
> but ithat one is a bit tricky to configure unless you use clang for building 
> the JDK as well.
>
> So in summary, after the summer the default indexer might be the obvious best 
> choice, but right now it depends on which features you think are the most 
> important I guess..
>

Hi Robin,

thanks a lot for the great summary! Incidentally I've just did some
Google research as well and basically came to the same conclusion.
"Cquery" seemed quite promising and also pretends to have call
hierarchies. But it seems to be defunct and I've also found some bug
reports about problems with the call hierarchies.

For me "Called Hierarchy", and "Find References" (in this order) are
really the most important IDE features. I'm using Eclipse and if you
setup your HotSpot project correctly, these features work pretty well
and reliably.

Have you personally tried the rtags "call hierarchies" / "find
references" support in VSC? From your summary it sounds like it's
worth giving it a try.

Thanks again for caring about these topics! Hotspot development with
IDE support has always been a pain and every improvement in this area
will be highly welcome :)

Best regards,
Volker

> Best regards,
> Robin
>
> >
> > Regards,
> > Volker
> >
> > On Mon, May 27, 2019 at 6:03 PM Robin Westberg
> >  wrote:
> >>
> >> Hi all,
> >>
> >> Please review this change that adds build system support for generating a 
> >> Visual Studio Code workspace configured for working with the JDK native 
> >> code. It configures the default C/C++ IntelliSense Engine to allow code 
> >> completion/navigation and similar features. It also configures two 
> >> executable targets (gtestLauncher and java) that can be built and debugged 
> >> from the IDE.
> >>
> >> The main target is "make vscode-project”, additional information can be 
> >> found in doc/ide.[md|html].
> >>
> >> Issue:
> >> https://bugs.openjdk.java.net/browse/JDK-8223678
> >>
> >> Webrev:
> >> https://cr.openjdk.java.net/~rwestberg/8223678/webrev.01/
> >>
> >> Testing:
> >> Manual testing on Linux, MacOS and Windows
> >>
> >> Thanks Erik Joelsson for taking a look at an earlier version of it!
> >>
> >> Best regards,
> >> Robin
>


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

2019-05-29 Thread Robin Westberg
Hi Erik,

Thanks for taking a look!

> On 28 May 2019, at 15:52, Erik Joelsson  wrote:
> 
> Hello Robin,
> 
> Looks good.
> 
> Adding of ide.md is very nice, but could you try to format it like testing.md 
> and building.md in regards to line lengths? I believe we are trying for 80 
> chars in those to make them read reasonably in a standard editor.

Sure, did a bit of reflowing and updated some formatting in order to look more 
like the other .md files.

Webrevs:
 - Full: https://cr.openjdk.java.net/~rwestberg/8223678/webrev.02/
 - Inc: https://cr.openjdk.java.net/~rwestberg/8223678/webrev.01-02/

Best regards,
Robin

> 
> /Erik
> 
> On 2019-05-27 09:03, Robin Westberg wrote:
>> Hi all,
>> 
>> Please review this change that adds build system support for generating a 
>> Visual Studio Code workspace configured for working with the JDK native 
>> code. It configures the default C/C++ IntelliSense Engine to allow code 
>> completion/navigation and similar features. It also configures two 
>> executable targets (gtestLauncher and java) that can be built and debugged 
>> from the IDE.
>> 
>> The main target is "make vscode-project”, additional information can be 
>> found in doc/ide.[md|html].
>> 
>> Issue:
>> https://bugs.openjdk.java.net/browse/JDK-8223678
>> 
>> Webrev:
>> https://cr.openjdk.java.net/~rwestberg/8223678/webrev.01/
>> 
>> Testing:
>> Manual testing on Linux, MacOS and Windows
>> 
>> Thanks Erik Joelsson for taking a look at an earlier version of it!
>> 
>> Best regards,
>> Robin



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

2019-05-29 Thread Robin Westberg
Hi Volker,

> On 28 May 2019, at 17:33, Volker Simonis  wrote:
> 
> Hi Robin,
> 
> thanks a lot for doing this!
> 
> I have just a quick question. Do you know if any of the VSC indexers
> (default, clangd) support call hierarchies (i.e. "called by",
> "callers" of a function/method) and "used by" for variables/class
> fields?

Sure, I can make a quick summary of the various pros and cons of the indexers 
that I’ve found so far. They are all moving pretty fast though, so didn’t think 
it was a good fit for the documentation file.

In general, VS Code itself only just recently gained proper support for 
displaying call hierarchies: 
https://code.visualstudio.com/updates/v1_33#_call-hierarchy - but alternative 
indexers have worked around this by showing them differently. 

Default (Microsoft - C/C++ for Visual Studio Code)
+ Easy to setup, as no additional dependencies are needed
+ Good “go to definition”, the only one that can make sense of the 
template+macro stuff in log.hpp for example
- Find references (used by) not done yet (said to be coming in the June update: 
https://github.com/microsoft/vscode-cpptools/issues/15)
- Call hierarchies also not there (scheduled for September apparently: 
https://github.com/microsoft/vscode-cpptools/issues/16)

clangd
+ Actively developed and part of the llvm/clang project
+ Support for find references
- ..but not call hierarchies yet
- Full project indexing is still flagged as experimental, and currently seems 
to fail when editing commonly used header files for example

Full feature list: 
https://clang.llvm.org/extra/clangd/Features.html#complete-list-of-features

rtags
This is currently the most feature-complete indexer I think. But the VS Code 
integration is a bit minimal and not part of the rtags project itself, and it 
is missing things like indexer progress.
+ The indexer is actively developed and has been around for quite some time
- You will probably have to build the indexer from source
+ Full support for call hierarchies and find references
- VS Code integration a bit limited, missing convenience features like switch 
between header/source file, showing method/class documentation on hover.

There are even more indexers of course, a promising one used to be “cquery", 
but that project seems to be defunct now. It lives on in the “ccls" indexer, 
but ithat one is a bit tricky to configure unless you use clang for building 
the JDK as well.

So in summary, after the summer the default indexer might be the obvious best 
choice, but right now it depends on which features you think are the most 
important I guess..

Best regards,
Robin

> 
> Regards,
> Volker
> 
> On Mon, May 27, 2019 at 6:03 PM Robin Westberg
>  wrote:
>> 
>> Hi all,
>> 
>> Please review this change that adds build system support for generating a 
>> Visual Studio Code workspace configured for working with the JDK native 
>> code. It configures the default C/C++ IntelliSense Engine to allow code 
>> completion/navigation and similar features. It also configures two 
>> executable targets (gtestLauncher and java) that can be built and debugged 
>> from the IDE.
>> 
>> The main target is "make vscode-project”, additional information can be 
>> found in doc/ide.[md|html].
>> 
>> Issue:
>> https://bugs.openjdk.java.net/browse/JDK-8223678
>> 
>> Webrev:
>> https://cr.openjdk.java.net/~rwestberg/8223678/webrev.01/
>> 
>> Testing:
>> Manual testing on Linux, MacOS and Windows
>> 
>> Thanks Erik Joelsson for taking a look at an earlier version of it!
>> 
>> Best regards,
>> Robin



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

2019-05-28 Thread Volker Simonis
Hi Robin,

thanks a lot for doing this!

I have just a quick question. Do you know if any of the VSC indexers
(default, clangd) support call hierarchies (i.e. "called by",
"callers" of a function/method) and "used by" for variables/class
fields?

Regards,
Volker

On Mon, May 27, 2019 at 6:03 PM Robin Westberg
 wrote:
>
> Hi all,
>
> Please review this change that adds build system support for generating a 
> Visual Studio Code workspace configured for working with the JDK native code. 
> It configures the default C/C++ IntelliSense Engine to allow code 
> completion/navigation and similar features. It also configures two executable 
> targets (gtestLauncher and java) that can be built and debugged from the IDE.
>
> The main target is "make vscode-project”, additional information can be found 
> in doc/ide.[md|html].
>
> Issue:
> https://bugs.openjdk.java.net/browse/JDK-8223678
>
> Webrev:
> https://cr.openjdk.java.net/~rwestberg/8223678/webrev.01/
>
> Testing:
> Manual testing on Linux, MacOS and Windows
>
> Thanks Erik Joelsson for taking a look at an earlier version of it!
>
> Best regards,
> Robin


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

2019-05-28 Thread Erik Joelsson

Hello Robin,

Looks good.

Adding of ide.md is very nice, but could you try to format it like 
testing.md and building.md in regards to line lengths? I believe we are 
trying for 80 chars in those to make them read reasonably in a standard 
editor.


/Erik

On 2019-05-27 09:03, Robin Westberg wrote:

Hi all,

Please review this change that adds build system support for generating a 
Visual Studio Code workspace configured for working with the JDK native code. 
It configures the default C/C++ IntelliSense Engine to allow code 
completion/navigation and similar features. It also configures two executable 
targets (gtestLauncher and java) that can be built and debugged from the IDE.

The main target is "make vscode-project”, additional information can be found 
in doc/ide.[md|html].

Issue:
https://bugs.openjdk.java.net/browse/JDK-8223678

Webrev:
https://cr.openjdk.java.net/~rwestberg/8223678/webrev.01/

Testing:
Manual testing on Linux, MacOS and Windows

Thanks Erik Joelsson for taking a look at an earlier version of it!

Best regards,
Robin


RFR: 8223678: Add Visual Studio Code workspace generation support (for native code)

2019-05-27 Thread Robin Westberg
Hi all,

Please review this change that adds build system support for generating a 
Visual Studio Code workspace configured for working with the JDK native code. 
It configures the default C/C++ IntelliSense Engine to allow code 
completion/navigation and similar features. It also configures two executable 
targets (gtestLauncher and java) that can be built and debugged from the IDE.

The main target is "make vscode-project”, additional information can be found 
in doc/ide.[md|html].

Issue:
https://bugs.openjdk.java.net/browse/JDK-8223678

Webrev:
https://cr.openjdk.java.net/~rwestberg/8223678/webrev.01/

Testing:
Manual testing on Linux, MacOS and Windows

Thanks Erik Joelsson for taking a look at an earlier version of it! 

Best regards,
Robin