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

Generated compile_commands.json uses relative paths for directory field #307

Closed
cbarrete opened this issue Jun 26, 2023 · 8 comments
Closed

Comments

@cbarrete
Copy link
Contributor

Hi,

I'm having issues using the generated the output from the compilation-database subtarget, which seems to boil down to clangd not liking relative directory fields.

Here's how to reproduce the issue using examples/with_prelude:

> buck2 init --git
...
> buck2 build cpp/hello_world:main[compilation-database]
...
> find -name compile_commands.json
./buck-out/v2/gen/root/524f8da68ea2a374/cpp/hello_world/__main__/compile_commands.json
> ln -s ./buck-out/v2/gen/root/524f8da68ea2a374/cpp/hello_world/__main__/compile_commands.json .
> nvim cpp/hello_world/main.cpp

At this point clangd starts, but it generates a bunch of erroneous diagnostics and doesn't let me go to the definition of print_hello. If I edit compile_commands.json to make directory an absolute path and open main.cpp again, everything works.

Arguably, clangd could support relative paths, but looking around on the internet indicates that this is unlikely (also clang-tidy depends on absolute directory paths).

Could the directory path that Buck2 generates be made absolute?

Thanks!

@ndmitchell
Copy link
Contributor

The problem is that if you write out absolute paths, its no longer possible to cache the compilation database between users. It's fairly annoying.

@knopp
Copy link

knopp commented Jul 6, 2023

Is sharing compilation database between users a common thing to do? It's an artefact of build tool, why would you share it instead of generate per user?

Another question is whether the utility of being able to share the compilation database outweigh not being able to use clangd, without postprocessing the compilation database, which seems rather annoying.

@ndmitchell
Copy link
Contributor

Sharing between users is super common, since it speeds everything up.

That said, sharing a useless pile of bytes that doesn't work has no value. Maybe the toolchain should have a boolean as to whether it generates absolute paths (with no caching) or relative paths (which are cached). I'm trying to find out how we deal with this internally, since we use clangd somehow.

@ndmitchell
Copy link
Contributor

It seems like internally we use Bxl as a wrapper that takes the compilation-database target, turns the paths to absolute, and then uses that for clangd. A bit fiddly, but does ensure that everyone can share most of the work.

@cbarrete
Copy link
Contributor Author

cbarrete commented Jul 6, 2023

Sharing between users is super common, since it speeds everything up.

I would expect generating the compilation database to be very fast, is it actually worth speeding up via caching?
And if it is the case for huge monorepos, should this be the exception rather than the rule? Meaning, buck2 would by default "just work", but larger organizations could set up caching/sharing if it becomes relevant for them.

@bobyangyf
Copy link
Contributor

There's downstream consumers that care if the file was cached or updated, so it gets a bit tricky when we write out these paths if you want to turn something absolute.

@cbarrete
Copy link
Contributor Author

There's downstream consumers that care if the file was cached or updated, so it gets a bit tricky when we write out these paths if you want to turn something absolute.

Could you elaborate @bobyangyf? (I somehow missed the replies on this issue, but recently hit this again)

More generally, my understanding is that the preferred solution for this would be to use other scripts to generate compilation databases. I don't necessarily mind this, but it is somewhat confusing that there exists a first class full-compilation-database on all C++ targets, but that it is essentially broken out of the box.

@cbarrete
Copy link
Contributor Author

For the record, I have been using the following script lately:

load("@prelude//utils:utils.bzl", "flatten")
load("@prelude//cxx/comp_db.bzl", "CxxCompilationDbInfo")
load("@prelude//cxx/compile.bzl", "CxxSrcCompileCommand")

def _make_entry(ctx: bxl.Context, compile_command: CxxSrcCompileCommand) -> dict:
    args = compile_command.cxx_compile_cmd.base_compile_cmd.copy()

    # This prevents clangd from jumping into `buck-out` using Go To Definition,
    # which significantly improves user experience.
    args.add(["-I", "."])
    args.add(compile_command.cxx_compile_cmd.argsfile.cmd_form)
    args.add(compile_command.args)
    ctx.output.ensure_multiple(args)

    return {
        "file": compile_command.src,
        "directory": ctx.fs.abs_path_unsafe(ctx.root()),
        "arguments": args,
    }

def make_compilation_database(ctx: bxl.Context, actions):
    db = []
    for name, analysis_result in ctx.analysis(flatten(ctx.cli_args.targets)).items():
        comp_db_info = analysis_result.providers().get(CxxCompilationDbInfo)
        if comp_db_info:
            db += [_make_entry(ctx, cc) for cc in comp_db_info.info.values()]

    db_file = actions.declare_output("compile_commands.json")
    actions.write_json(
        db_file.as_output(),
        db,
        with_inputs = True,
        pretty = True,
    )
    return db_file

def _gen_impl(ctx: bxl.Context):
    actions = ctx.bxl_actions().actions
    ctx.output.print(ctx.output.ensure(make_compilation_database(ctx, actions)))

gen = bxl_main(
    impl = _gen_impl,
    cli_args = {
        "targets": cli_args.list(cli_args.target_expr()),
    },
)

What I like about is is that:

  • It materializes all relevant inputs, but does not actually build the targets, so only e.g. argsfiles and codegen are run, automagically
  • It heavily leverages the prelude so that there's a good chance that it is correct
  • It doesn't use the relatively heavy compilation database subtargets, which I found difficult to work with
  • It is split out so that other scripts (e.g. for clang-tidy integration) can reuse the core logic to generate a compilation database
  • It's quite small and easy to grok

cbarrete added a commit to cbarrete/buck2 that referenced this issue Nov 15, 2024
This is a diffent approach than facebook#510. The
main differences are:
- All required dependencies, such as generated code, are materialized, so that
  tools that use the compilation database work can find those and work properly.
- Files that are included in multiple targets result in multiple entries in the
  compilation database.

Closes facebook#307
facebook-github-bot pushed a commit that referenced this issue Dec 20, 2024
Summary:
This is a diffent approach than #510. The main differences are:
- All required dependencies, such as generated code, are materialized, so that tools that use the compilation database work can find those and work properly.
- Files that are included in multiple targets result in multiple entries in the compilation database.

Closes #307

Pull Request resolved: #810

Reviewed By: scottcao

Differential Revision: D66984456

Pulled By: cjhopman

fbshipit-source-id: e53c5cfdcf32e4c34331d7343c06a486a971d27e
cbarrete added a commit to cbarrete/buck2 that referenced this issue Jan 17, 2025
The move to `cxx_internal_tools` broke open source users that had a custom `make_comp_db` to work
around facebook#307.

This commit makes it configurable so that OSS users can again provide their own script to generate
legal `compile_commands.json` files.

A few remaining tools are made `PUBLIC` so that OSS users can simply reuse them.
facebook-github-bot pushed a commit to facebook/buck2-prelude that referenced this issue Jan 22, 2025
Summary:
The move to `cxx_internal_tools` broke open source users that had a custom `make_comp_db` to work around facebook/buck2#307.

This commit makes it configurable so that OSS users can again provide their own script to generate legal `compile_commands.json` files.

A few remaining tools are made `PUBLIC` so that OSS users can simply reuse them.

X-link: facebook/buck2#847

Reviewed By: dtolnay

Differential Revision: D68327054

fbshipit-source-id: 3c553e7d575928e07728e7525138f45d9b6f2413
facebook-github-bot pushed a commit that referenced this issue Jan 22, 2025
Summary:
The move to `cxx_internal_tools` broke open source users that had a custom `make_comp_db` to work around #307.

This commit makes it configurable so that OSS users can again provide their own script to generate legal `compile_commands.json` files.

A few remaining tools are made `PUBLIC` so that OSS users can simply reuse them.

Pull Request resolved: #847

Reviewed By: dtolnay

Differential Revision: D68327054

fbshipit-source-id: 3c553e7d575928e07728e7525138f45d9b6f2413
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants