[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-build-dev
Subject:    Re: [11u] RFR: 8223678: Add Visual Studio Code workspace generation support (for native code)
From:       Volker Simonis <volker.simonis () gmail ! com>
Date:       2020-02-28 9:14:12
Message-ID: CA+3eh12wX8W2=Gn=eKFq2jYTQr+XFcrCyVVk0d-ReofP=GnK=g () mail ! gmail ! com
[Download RAW message or body]

On Wed, Feb 26, 2020 at 3:49 PM Andrew Hughes <gnu.andrew@redhat.com> 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.gmk    Mon Jun 03 17:14:23 2019 -0700
> > +++ b/make/common/Utils.gmk    Mon 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.gmk    Tue Feb 12 08:40:43 2019 +0100
> > +++ b/make/common/MakeBase.gmk    Mon 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.gmk    Mon Jun 03 17:14:23 2019 -0700
> > +++ b/test/make/TestMakeBase.gmk    Mon 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.gmk    Tue Feb 12 08:40:43 2019 +0100
> > +++ b/test/make/TestMakeBase.gmk    Mon 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 files work :)

> 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?

Best regards,
Volker

> 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  C5A0 CFDA 0F9B 3596 4222
>
[prev in list] [next in list] [prev in thread] [next in thread]