RFR: 8210459: Add support for generating compile_commands.json

Robin Westberg robin.westberg at oracle.com
Wed Sep 19 07:25:44 UTC 2018


Hi Erik,

> On 14 Sep 2018, at 19:21, Erik Joelsson <erik.joelsson at oracle.com> wrote:
> 
>>> Main.gmk:888: I think you meant to add test-compile-commands here, which also requires you to define that target.
>> Fixed, the tests are now invoked from a top-level target. I also added a second test to ensure that the no-build-libraries property of this actually stays intact.
> This is ok, but the test will fail unless run from a clean output dir. This should probably be noted in a comment at least.

Yes, good point, I tried briefly to include a clean as a prerequisite target to the test, but it refused to execute sequentially. But perhaps it’s good enough with a comment, added one.

>>> In Awt2dLibraries.gmk, I think lines 733 and 735 needs to be fixed as well. Using FindLib is generally a good idea for this. I suspect there may be more such instances sprinkled around the makefiles.
>> Only fixed the last one, I think the first one is ok as is?
> The first one is sort of OK, but it's a questionable construct as the $(BUILD_LIBAWT_XAWT) variable may contain additional targets. We used to do it that way but these days I prefer the more explicit and precise FindLib. In this particular case you get a non needed dependency added when running compile-commands with ENABLE_HEADLESS_ONLY=true, which will not really affect anything so it doesn't really matter.

Yeah I certainly agree, but a quick grep shows that there’s about 50 such constructs present right now. I wouldn’t mind cleaning those up, but perhaps that should be in a separate change?

> Otherwise this looks good now.

Thanks, I’ll include the latest webrevs with a comment added:

Incremental: http://cr.openjdk.java.net/~rwestberg/8210459/webrev.01-02/
Full: http://cr.openjdk.java.net/~rwestberg/8210459/webrev.02/

Best regards,
Robin

> 
> /Erik
>> Webrev (incremental): http://cr.openjdk.java.net/~rwestberg/8210459/webrev.00-01/
>> Webrev (full): http://cr.openjdk.java.net/~rwestberg/8210459/webrev.01/
>> Testing: tier1, builds-tier5
>> 
>> Best regards,
>> Robin
>> 
>>> /Erik
>>> 
>>> 
>>> On 2018-09-10 06:36, Robin Westberg wrote:
>>>> Hi all,
>>>> 
>>>> Please review the following change that adds support for generating compile_commands.json as a top-level make target. This is a popular format for describing how to compile object files for a project, and is defined in [1]. This file can be used directly by IDEs such as Visual Studio Code and CLion as well as "language servers" such as cquery [2] and rtags [3].
>>>> 
>>>> While it’s currently possible to generate this file as part of a full build (using tools such as Bear [4], or simply parsing .cmdline files), this change aims to make it possible to generate the compile commands data without actually compiling the object files. This means that it’s for example possible to start using an IDE fairly quickly on freshly cloned source, instead of having to wait for a full build to complete.
>>>> 
>>>> This was originally inspired by the work done in [5] by Mikael Gerdin and Erik Joelsson, but has been through a few revisions since then. Erik has also provided a lot of assistance with the current version, thanks Erik!
>>>> 
>>>> Issue: https://bugs.openjdk.java.net/browse/JDK-8210459
>>>> Webrev: http://cr.openjdk.java.net/~rwestberg/8210459/webrev.00/
>>>> Testing: tier1, builds-tier5
>>>> 
>>>> Best regards,
>>>> Robin
>>>> 
>>>> [1] https://clang.llvm.org/docs/JSONCompilationDatabase.html
>>>> [2] https://github.com/cquery-project/cquery
>>>> [3] https://github.com/Andersbakken/rtags
>>>> [4] https://github.com/rizsotto/Bear
>>>> [5] http://mail.openjdk.java.net/pipermail/hotspot-dev/2017-March/026354.html




More information about the build-dev mailing list