Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add argument to qmk compile to build compile_commands.json for you #8916

Closed
wants to merge 15 commits into from

Conversation

xton
Copy link
Contributor

@xton xton commented Apr 25, 2020

Description

I use clangd as a languageserver along with coc.vim[0]. It requires a compilation database[1] to help it figure out how to find all the required files for a given compilation pass. This PR adds a flag (-c) to qmk compile which creates that for you. Now your dev environment can figure out what #include QMK_KEYBOARD_H means.

[0] https://github.com/neoclide/coc.nvim/wiki/Language-servers#ccobjective-c
[1] https://clang.llvm.org/docs/JSONCompilationDatabase.html

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • Keyboard (addition or update)
  • Keymap/layout/userspace (addition or update)
  • Documentation

Issues Fixed or Closed by This PR

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • I have tested the changes and verified that they work and don't break anything (as well as I can manage).

@zvecr zvecr added cli qmk cli command python labels Apr 25, 2020
@zvecr zvecr requested review from skullydazed, Erovia and a team April 25, 2020 21:49
@xton
Copy link
Contributor Author

xton commented Apr 25, 2020

Looks like ccls and CLion use the compile_commands.json file also, though I did not test them.

@GauthierPLM
Copy link

GauthierPLM commented May 1, 2020

Hello,

I‘d also enjoy to have the possibility to generate a compilation database to have a better support in Clion.

I tried to use the scan_build and compiledb tools but they failed to intercept commands when building the firmware in MSYS2.

@skullydazed
Copy link
Member

Thanks for your work here!

We'll want to have a separate command for this, rather than adding this into the compile command. In general we like to keep our commands small and focused. Can you rework this around that?

We'll also want to find a better place than compile_commands_json.py for your functions, especially in light of #8666, but I don't have immediate guidance for that. It's something to keep in mind while you work on it.

@xton
Copy link
Contributor Author

xton commented May 9, 2020

@skullydazed do you have any ideas about what the new command should be called? I'm struggling to find something that's both concise and specific enough.

@xton xton force-pushed the compile_commands branch from fca1204 to b671c40 Compare May 9, 2020 20:52
@skullydazed
Copy link
Member

I don't have a good suggestion right now. I'll think on it.

@xton
Copy link
Contributor Author

xton commented May 9, 2020

ptal @skullydazed

I moved the feature into it's own command named compiledb. This command uses the same config options as compile for simplicity.

I also merged the library code into the cli/compiledb.py since there wasn't very much of it.

Also updated docs, moving the section on this command under the "developer" heading.

@tzarc
Copy link
Member

tzarc commented May 10, 2020

I use parallel compilation for my builds, with output sync turned on.
The effective result is that it executes:

MAKEFLAGS="-j16 -O" qmk compiledb -kb massdrop/ctrl -km default

If I do this, then the "make clean" stage of the qmk compiledb never exits and stalls.

For clarity, I'm "fixing" this by doing:

MAKEFLAGS="" qmk compiledb -kb massdrop/ctrl -km default

But for the uninitiated this will probably be hard to diagnose.

@xton
Copy link
Contributor Author

xton commented May 10, 2020

@tzarc do you think there would be any downside to blanking out that (and other?) env vars in my make invocations in these commands? I wouldn't think so...

@xton
Copy link
Contributor Author

xton commented May 19, 2020

@tzarc do you have some other environmental make settings enabled? I can't reproduce the hang you're seeing or understand why output-sync would cause it: when I invoke make clean I'm sending all output to /dev/null.

But! Looking into this I noticed a different bug: I was hardcoding make in my make clean while the main make command switches to gmake when It's available. I've just corrected this. Could you try with the latest commit?

@tzarc
Copy link
Member

tzarc commented May 20, 2020

@xton - yes, invoking qmk from a Makefile will set $MAKEFLAGS on the environment, which propagates to child processes.
The MAKEFLAGS="-j16 -O" was the cut-down version of what's getting executed; manually applying the same environment variables when invoking make.

@stale
Copy link

stale bot commented Jul 4, 2020

Thank you for your contribution!
This pull request has been automatically marked as stale because it has not had activity in the last 45 days. It will be closed in 30 days if no further activity occurs. Please feel free to give a status update now, or re-open when it's ready.
For maintainers: Please label with awaiting review, breaking_change, in progress, or on hold to prevent the issue from being re-flagged.

@stale
Copy link

stale bot commented Aug 3, 2020

Thank you for your contribution!
This pull request has been automatically closed because it has not had activity in the last 30 days. Please feel free to give a status update now, ping for review, or re-open when it's ready.

@xton
Copy link
Contributor Author

xton commented Sep 7, 2020

Seems I can't re-open this PR, so I've filed a new one (with changes suggested by @tzarc ) #10264

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants