[webkit-reviews] review denied: [Bug 235686] Generate compile_commands.json on macOS Builds : [Attachment 452313] patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Thu Feb 17 11:59:56 PST 2022
Elliott Williams <emw at apple.com> has denied Brandon
<brandonstewart at apple.com>'s request for review:
Bug 235686: Generate compile_commands.json on macOS Builds
https://bugs.webkit.org/show_bug.cgi?id=235686
Attachment 452313: patch
https://bugs.webkit.org/attachment.cgi?id=452313&action=review
--- Comment #27 from Elliott Williams <emw at apple.com> ---
Comment on attachment 452313
--> https://bugs.webkit.org/attachment.cgi?id=452313
patch
View in context: https://bugs.webkit.org/attachment.cgi?id=452313&action=review
Some small nits, but overall I'm glad to see this is coming together!
If you are still thinking about how to automate that last step (running
Tools/Scripts/generate-compile-commands), I think this'll work inside the
Makefile:
debug d: force
@$(call set_webkit_configuration,--debug)
@$(call
invoke_xcode,,,GCC_PREPROCESSOR_DEFINITIONS='$(GCC_PREPROCESSOR_ADDITIONS)
$$(inherited)')
$(SCRIPTS_PATH)/generate-compile-commands $(shell perl
-I$(SCRIPTS_PATH) -Mwebkitdirs -e 'setConfiguration(); print productDir()' --
$(SDKROOT:%=--sdk %) --debug)
and do the same for the `release` recipe, but pass `--release` :)
> Makefile.shared:56
> +ifeq (, $(findstring EXPORT_COMPILE_COMMANDS, $(ARGS)))
> + XCODE_OPTIONS += EXPORT_COMPILE_COMMANDS=YES;
> + XCODE_OPTIONS += GCC_PRECOMPILE_PREFIX_HEADER=NO;
> + XCODE_OPTIONS += CLANG_ENABLE_MODULE_DEBUGGING=NO;
> + XCODE_OPTIONS += COMPILER_INDEX_STORE_ENABLE=NO;
> +endif
> +
I think you should pass EXPORT_COMPILE_COMMANDS directly to Make, not via ARGS,
because it has Make-specific logic and isn't _just_ an xcode build setting.
```
ifneq (,$(EXPORT_COMPILE_COMMANDS))
XCODE_OPTIONS += ...
endif
```
This also makes the UX a bit nicer, since you'll type `make
EXPORT_COMPILE_COMMANDS=YES` instead of `make
ARGS=EXPORT_COMPILE_COMMANDS=YES`.
> PerformanceTests/DecoderTest/Configurations/DebugRelease.xcconfig:39
> +COMPILE_COMMANDS_ = "";
This setting isn't necessary: xcconfig variables are empty by default, so it's
fine to let OTHER_CFLAGS expand $(COMPILE_COMMANDS_) without us defining it.
Also, xcconfig doesn't do string quoting, so "" are parsed as literal quote
marks here.
> Tools/Scripts/generate-compile-commands:1
> +#!/bin/sh
Use `#!/bin/sh -e` to exit if there's an error inside the script.
More information about the webkit-reviews
mailing list