[Webkit-unassigned] [Bug 235686] Generate compile_commands.json on macOS Builds

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=235686

Elliott Williams <emw at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #452313|review?                     |review-
              Flags|                            |

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

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20220217/285ab14c/attachment-0001.htm>