danielbarter / mini_compile_commands

GNU General Public License v3.0
37 stars 3 forks source link

allow generating during build by building from an output dir #7

Closed Alan-Chen99 closed 4 months ago

Alan-Chen99 commented 5 months ago

solves https://github.com/danielbarter/mini_compile_commands/issues/5

danielbarter commented 5 months ago

@Alan-Chen99: I very much appreciate you taking a shot at this, but I am pretty skeptical.

Changing where a build happens is introducing timing impurities into the build process (the standard build directory might be on a fast file system, and the store might be on a slow file system), and unfortunately, building software is riddled with concurrency bugs that will be inevitably triggered by such a change.

Moreover, from a developer perspective, it is very hard to see how these hook based approaches are useful. Why would I ever want to have a compile_commands.json for a source tree, stored on a read only file system? The whole point is that I want lsp to work for a source tree that I am actively working on.

Also, you have mixed in a bunch of linting and non essential files (nixd stuff, the flake) that make the PR harder to review. This is really a mix of 3 PRs, which you need to split apart.

I think the correct solution here is is that mini_compile_commands_server.py should generate a compile_commands.json parameterized over the build directory (for example, this could be a nix function). This would make it relocatable, and you could evaluate it for wherever you end up storing your source tree.

Alan-Chen99 commented 5 months ago

sorry for not being clear.

I made this to run on library dependencies, and interpreters (python, emacs, etc), since I find that I often need to browser their source code, but I dont intend to actually modify them.

I actually tried to make a relocatable solution, but this basically never work (since you often have generated files like config.h, etc)

danielbarter commented 4 months ago

Hi @Alan-Chen99: I am gonna close because I think this adds too much complexity for a feature which I don't really understand, and personally don't find useful.

You are more than welcome to maintain a fork, or drastically simplify the MR.