Hi Erik, > On 14 Sep 2018, at 19:21, Erik Joelsson <erik.joels...@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
