ede-compdb icon indicating copy to clipboard operation
ede-compdb copied to clipboard

Specify Ninja working directory

Open itollefsen opened this issue 9 years ago • 5 comments
trafficstars

As it works today, the compilation database for Ninja projects is generated from the configured build directory, where the build.ninja files resides.

However, I'm working on a project where the build command is to be executed from the project root directory. And files references are relative to project root, not build root.

Here's an example setup:

(ede-ninja-project "Test" :file (expand-file-name "~/src/test/.gitignore") :configurations '("debug" "release") :configuration-directories '("_build/debug" "_build/release") :configuration-default "debug" :compdb-file "build.ninja" :build-rules '("example-cxx" "example-c") )

The way ede-compdb currently works, it will change to the build directory ("_build/debug" for the default configuration) and build the compilation database from there (using "ninja -t compdb").

A contrived example of what the resulting entries look like:

{ "directory": "<$HOME>/src/test/_build/debug", "command": "clang++ -c -o _build/debug/test.o src/test.cpp", "file": "src/test.cpp" }

Since files are relative to project root, not relative to the build directory, this won't work.

In this case, "ninja -t compdb" has to be executed from the project root directory, so entries will look like this:

{ "directory": "<$HOME>/src", "command": "clang++ -c -o _build/debug/test.o src/test.cpp", "file": "src/test.cpp" }

I.e. the "directory" is now correctly pointing at the project root directory and then everything works as it should.

I made this work locally by changing insert-compd(...) along the lines of:

  • (let ((default-directory (file-name-directory compdb-path)))
  • (apply 'process-file (append `("ninja" nil t nil "-f" ,(oref this compdb-file) "-t" "compdb") (oref this :build-rules)))))
  • (let ((default-directory (directory-file-name (expand-file-name (oref this directory))))
  •    (complete-compdb-path (concat (file-name-directory compdb-path) (oref this compdb-file)))
    
    "%s"..." compdb-path)))))
  • (apply 'process-file (append `("ninja" nil t nil "-f" ,(format "%s" complete-compdb-path) "-t"

And project-rescane(...) along the same lines:

  • (let ((default-directory (current-configuration-directory this)))
  • (let ((default-directory (directory-file-name (expand-file-name (oref this directory))))
  •      (complete-compdb-path (concat (current-configuration-directory this) (oref this compdb-file))))
    
    (oset this phony-targets nil) (erase-buffer)
  •  (process-file "ninja" nil t t "-f" (oref this compdb-file) "-t" "targets" "all")
    
  •  (process-file "ninja" nil t t "-f" (format "%s" complete-compdb-path) "-t" "targets" "all")
    

Note that I'm not a Elisp developer, nor have I thoroughly investigated how ede-compdb works overall. For instance, I couldn't quite figure out how to use complete-compdb-path without the format function nor do I understand the fasination with file-name-directory. So just ignore the parts that aren't quite right, but the you get the idea.

In short, it looks like what is needed is a way to tell ede-compdb what it should use as its working directory when executing the Ninja command. At the very least a switch that will let users select either the current configuration directory (as it is today) or the project root directory (as I use). I'm guessing that should cover most use cases, but a general setting that lets users specify the directory will probably cover all.

itollefsen avatar May 09 '16 09:05 itollefsen

Should have done a preview of that. Apologies - I haven't used this editor before.

Here are the diffs again in a more readable format.

insert-compd(...) :

-  (let ((default-directory (file-name-directory compdb-path)))
-    (apply 'process-file (append `("ninja" nil t nil "-f" ,(oref this compdb-file) "-t" "compdb") (oref this :build-rules)))))
+  (let ((default-directory (directory-file-name (expand-file-name (oref this directory))))
+        (complete-compdb-path (concat (file-name-directory compdb-path) (oref this compdb-file))))
+    (apply 'process-file (append `("ninja" nil t nil "-f" `,(format "%s" complete-compdb-path) "-t" "compdb") (oref this :build-rules)))

project-rescan(...):

-    (let ((default-directory (current-configuration-directory this)))
+    (let ((default-directory (directory-file-name (expand-file-name (oref this directory))))
+          (complete-compdb-path (concat (current-configuration-directory this) (oref this compdb-file))))
       (oset this phony-targets nil)
       (erase-buffer)
-      (process-file "ninja" nil t t "-f" (oref this compdb-file) "-t" "targets" "all")
+      (process-file "ninja" nil t t "-f" (format "%s" complete-compdb-path) "-t" "targets" "all")

itollefsen avatar May 09 '16 09:05 itollefsen

I think I understand the problem, thanks. I need to think about how this could be addressed without breaking existing users. Also note that the fix would be made in cedet directly; as noted in the readme, the ede-compdb project is no longer maintained.

randomphrase avatar May 09 '16 11:05 randomphrase

Ah, I need to update my cedet - thanks!

itollefsen avatar May 09 '16 14:05 itollefsen

What's the preferred way of raising issue/posting suggestions when it's now part of cedet? cedet-devel list? I've added two hooks that I'd like to suggest, with pertinent information about the use case.

itollefsen avatar May 09 '16 18:05 itollefsen

Unfortunately I think cedet only accepts issues on the cedet-devel mailing list. Don't let that put you off - myself and others will be reading these requests.

randomphrase avatar May 09 '16 18:05 randomphrase