-
Notifications
You must be signed in to change notification settings - Fork 50
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
Generate compilation database on load #285
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
839321a
to
9ea4dc9
Compare
DavisVaughan
approved these changes
Jun 25, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works on macOS for many packages for me, works on Windows for vctrs with the newest changes!
2317247
to
b4f4557
Compare
Co-authored-by: Davis Vaughan <[email protected]>
I've added a currently unexported util to help set this up: |
743b170
to
5d6a2a1
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
LSPs like clangd provide intellisense features for native files but they need to know the build flags (mostly include flags) for each root file in the project (flags for included files are inferred from root files). These tools are not able to infer the flags by themselves and so require the generation of a "compilation database": https://clang.llvm.org/docs/JSONCompilationDatabase.html.
For R packages the flags are generated by
R CMD build
and depend on the package Makevars file, possibly generated fromconfigure
. Generating the database after aload_all()
seems like a reasonable update point to keep track of the changing source files (which can be added or removed during development) while ensuring the presence of an up-to-date Makevars file.There are two main approaches for the generation of a compilation database. The first is to intercept the compiler calls during an actual build to collect arguments. This is the approach used by tools like
bear
and it works great in conjunction withR CMD build
. The other is to generate build commands, e.g. withmake --dry-run
.@kevinushey suggested we could use the latter approach with
R CMD shlib --dry-run
. This is a nice idea because it means we don't need any external tools to create the database. This PR implements this approach. A few caveats:For some reason the
shlib
command does not add include flags forLinkingTo
dependencies, so I had to inject those manually.In addition to
--dry-run
we need to pass--always-make
tomake
. See sneak '--always-make' into R CMD SHLIB invocation rstudio/rstudio#11917 for the motivation and the workaround reused here.We need to pass target source files to
shlib
. These can easily be inferred whenMakevars
does not overrideOBJECTS
. When it is overridden, we need to retrieve the contents of this variable, and then match the object files to potential sources. This sometimes clashes, e.g. in igraph there are a .c and .f files that match a single object file. We just select one file in this case and ignore the others. This should not be a big deal and should be a rare occurrence.There could be make directives interspersed between compile commands in the dry-run output (this happens with rlang). To detect actual commands, we simply select all lines starting with a compiler command. These commands (CC, CXX, FC, etc) could be retrieved from
R CMD config
but this tool seems extremely slow. Instead we retrieve them from the makefiles manually. These commands should be in the same order as the source files passed toshlib
. A consistency check verifies that we have as many commands as source files and that each command indeed references the corresponding source file (typically in a-f
flag).The compilation database is not generated by default, the user must explicitly opt in by inserting this line in their
DESCRIPTION
file:In addition the
compile_commands.json
file generated at the root should be gitignored and Rbuildignored. We'll provide a usethis function to accomplish these tasks.The tools for generating this database should probably be moved and reexported from pkgbuild in the future (cc @gaborcsardi), but I'm implementing them here as I'd like to include the feature in the next release of pkgload.
Untested on Windows but should work on Unixes. Tested on macOS with readxl, rlang, igraph, and r-tree-sitter.
Edit: now working on Windows