Add global .cquery file #702

nick96 · 2018-06-09T14:47:43Z

This PR implements the feature requested in #648.

The key changes are:

On *nix looks for a .cquery file in $XDG_CONFIG_HOME/cquery or in $HOME if this fails
On Windows looks for a .cquery file in the %APPDATA

I've tested this a bit on my machine but haven't tested out the Windows specific stuff as a I don't have access to a Windows machine so I hope it works 😄

nick96 · 2018-06-10T03:53:16Z

It looks like the tests are failing because of:

https://github.com/cquery-project/cquery/blob/9a9b2b373e4995066e135481fd677b266b8f0893/src/cache_manager.cc#L76

I don't really understand the logic of having assert(false) in FakeCacheManager so am not sure how to go about fixing this

jacobdufault · 2018-06-10T17:04:39Z

src/platform_win.cc

+	  std::wstring_convert<convert_type, wchar_t> converter;
+	  cfg_path = converter.to_bytes(roaming_stream.str());
+	}
+	CoTaskMemFree(static_cast<void*>(roaming_path));


https://msdn.microsoft.com/en-us/library/windows/desktop/ms680722(v=vs.85).aspx it sounds like this should only be used if the memory was allocated using CoTaskMemAlloc or CoTaskMemRealloc.

Btw, is there a sample somewhere to do this? I'd recommend referencing any doc pages here.

In the docs for https://msdn.microsoft.com/en-us/library/bb762188(VS.85).aspx, it says call is responsible for freeing ppszPath (in this case roaming_path) using CoTaskMemFree. I have moved the call inside the block where the we use it because CoTaskMemFree does nothing when the parameter is NULL.

I have referenced the docs in a comment above the call as you recommended

jacobdufault · 2018-06-10T17:05:05Z

src/project.cc

@@ -472,6 +472,15 @@ std::vector<Project::Entry> LoadFromDirectoryListing(ProjectConfig* config) {
                     }
                   });

+  optional<std::string> maybe_cfg = GetGlobalConfigDirectory();
+  if (folder_args.empty() && maybe_cfg) {
+    std::string cfg(*maybe_cfg + ".cquery");


nit: std::string config = *maybe_config + ".cquery";

jacobdufault · 2018-06-10T17:05:25Z

src/project.cc

  if (FileExists(project->project_dir + ".cquery"))
    return LoadFromDirectoryListing(project);
+  else if (GetGlobalConfigDirectory() &&
+           FileExists(*GetGlobalConfigDirectory() + ".cquery"))


jacobdufault · 2018-06-10T17:06:43Z

src/project.cc

@@ -472,6 +472,15 @@ std::vector<Project::Entry> LoadFromDirectoryListing(ProjectConfig* config) {
                     }
                   });

+  optional<std::string> maybe_cfg = GetGlobalConfigDirectory();


Is the purpose of this block only for the LOG_S(INFO)? In that case we can move the LOG_S(...) into LoadFromDirectoryListing which should eliminate the need for this block.

No sure what you're referring to here? It that the right block?

jacobdufault · 2018-06-10T17:09:02Z

src/platform_posix.cc

+    EnsureEndsInSlash(config);
+    config += "cquery";
+    if (!FileExists(config)) {
+      MakeDirectoryRecursive(AbsolutePath(config, true));


MakeDirectoryRecursive(AbsolutePath(config, false /*validate*/));

If validate is true then cquery may abort since validate means the directory should exist.

Good catch, have fixed it up

jacobdufault · 2018-06-10T17:09:28Z

src/platform_posix.cc

+  } else if (home) {
+    config = home;
+  } else {
+    return {};


nit: return nullopt;

But instead of having a separate return here (returns in the middle of a func make control-flow hard to read) you can refactor this function so that config is an optional. The final call to EnsureEndsInSlash will need to be if (config) EnsureEndsInSlash(*config);

The refactor definitely makes it cleaner, have done as you recommended

jacobdufault · 2018-06-10T17:12:02Z

The CI failure is unrelated, master is broken at the moment. We don't have any tests that would validate this logic.

jacobdufault · 2018-06-10T18:06:27Z

One thing to make sure is that a local compile_commands.json will override the global .cquery file.

nick96 · 2018-06-11T05:17:21Z

I've fixed it so compile_commands.json is preferred over the global .cquery. Have explained the rational in 9c53e2b. The cleanest way I could think of was adding a default parameter to LoadFromDirectoryListing. Previous calls it the function don't need to be changed but I think it makes it clearer what we want to do (load a global config) and doesn't required having to redo a whole bunch of checks.

jacobdufault

We should make sure to write up the logic of how compilation commands are selected, since the logic is a bit complex/subtle.

jacobdufault · 2018-06-20T03:00:30Z

src/project.cc

+      [&folder_args, &files, &use_global_config](const std::string& path) {
+        if (SourceFileLanguage(path) != LanguageId::Unknown) {
+          files.push_back(path);
+        } else if (GetBaseName(path) == ".cquery" && !use_global_config) {


So this is disabling a local .cquery file in the source tree if there is a global config? I think we should honor a local .cquery over the global one even if there is not a .cquery at the top level.

In retrospect I can see how this is not very clear. My rational was that if LoadFromDirectoryListing is called with use_global_config set to true then the caller is intending for the global config to be loaded. I'll change it so that the local .cquery is loaded in all cases

This platform specific function is used to get the user's home directory. This gives us a base from where to get the global config file for. Currently WIP as I'm not sure how to implement this for Windows.

This is a tentative implementation, I don't have access to windows so haven't tested it but based on MSDN and StackOverflow this seems to be the way to go.

As discussed in jacobdufault#648, fallback to $HOME if $XDG_CINFIG_HOME is not found.

Previously the global .cquery config was being loaded before the project specific compile_commands.json file. The global .cquery config is intended as a fallback when there are no other configs, so we don't want this. To make it explicit that the global .cquery is being loaded, add a new parameter to LoadFromDirectorylisting (use_global_config) which, if true, will load the global .cquery. This parameter defaults to false, so no changes are required to existing calls.

nick96 · 2018-06-20T12:37:41Z

To document the process of selecting compilation commands, I think adding a second paragraph under the compile_commands.json header of the wiki home page with something like what I've written below would be sufficient.

There are three options for specifying the compilation commands that cquery uses for a project.

Use a compile_commands.json as generated by your build tool. This can either be in the project root directory or the build directory stemming from the root (the project root is preferred)
Use a .cquery file in the root
Use a .cquery file the configuration directory of your system (i.e. %RoamingAppData%/cquery/, $XDG_CONFIG_HOME/cquery/ or $HOME)

The selection order of the compilation commands is:
compile_commands.json > local .cquery > global .cquery

jacobdufault · 2018-06-25T23:11:49Z

I can do a squash and merge, but if you'd like to preserve commit history this needs to be rebased. Let me know which you prefer.

nick96 · 2018-06-26T04:04:24Z

I think a squash and merge is probably best. There's nothing in the commit history of particular importance.

* Create `GetHomedirectory()` function This platform specific function is used to get the user's home directory. This gives us a base from where to get the global config file for. Currently WIP as I'm not sure how to implement this for Windows. * Find and parse .cquery in home dir if not in project dir * GetHomeDirectory() -> GetGlobalConfigDirectory() * Windows specific implementation of `GetGlobalConfigDirectory()` This is a tentative implementation, I don't have access to windows so haven't tested it but based on MSDN and StackOverflow this seems to be the way to go. * Update *nix implementation of `GetGlobalConfigDirectory()` As discussed in jacobdufault#648, fallback to $HOME if $XDG_CINFIG_HOME is not found. * Use a global .cquery file if none found in the project dir * Add rational to usage of XDG_CONFIG_HOME * Create "cquery" directory in .config if it does not exist * Fix up on Windows * Only call `CoTaskMemFree` if `ShGetKnownFolderpath` passes * Refactor config to optional to improve control flow * Add comment referencing MSDN blog on getting config dir on Windows * Implicitly call string constructor using = operator * Add braces around if-else block * Fixup some errors introduced when making config optional * config -> cfg * Fixup crash when assigning to config * Load global .cquery config at end of LoadCompilationEntriesfromdirectory Previously the global .cquery config was being loaded before the project specific compile_commands.json file. The global .cquery config is intended as a fallback when there are no other configs, so we don't want this. To make it explicit that the global .cquery is being loaded, add a new parameter to LoadFromDirectorylisting (use_global_config) which, if true, will load the global .cquery. This parameter defaults to false, so no changes are required to existing calls. * Add reference for usage of CoTaskMemFree after SHGetKnownFolderPath * Load the local .cquery in all cases that it is present

liushapku · 2018-12-17T06:36:55Z

Reading this thread and also the commit message, I get completely lost about the the position of the global configuration file. Which one is it:

~/.cquery
~/.config/cquery/settings
~/.config/cquery
~/.config/.cquery

nick96 · 2018-12-17T09:05:14Z

For the global config file you can either have a .cquery file in your $HOME directory or at $XDG_CONFIG_HOME/cquery

liushapku · 2018-12-19T10:21:08Z

I just found a better way to set the global options which is to add a cquery.cfg file in the same folder of cqeury-clang; according to the configuration of clang. The ~/.cquery file is not respected when a compile_commands.json file exists for the project.

For me, there is no system wide gcc installed, neither the header files and libraries for the c++17. So I installed a gcc in my home folder. And I need to use --gcc-toolchain=my_gcc_path to make clang work.
I need to combine the project compile_commands.json with the --gcc-toolchain specification. The ~/.cquery approach does not work here.

nick96 · 2019-01-12T14:20:10Z

Am I right in reading that you want to have the options set by ~/.cquery and compile_commands.json?

I didn't know about having cquery.cfg in the same folder as cquery-clang. It could be a bit annoying having config files in that directory. I like having in a more centralised place, preferable in an XDG directory but the home directory also works.

Having a global config setting the baseline options and the a project local config overriding these as necessary would probably be quite useful, @jacobdufault thoughts?

liushapku · 2019-01-14T01:18:01Z

@nick96, I didn't mean that I expect to add an cquery.cfg in the installation folder by this cqeury-project. I just wanted to share there exists another approach to fill the needs like what I had. It is the end users that need to add that file in their own interest.

In summary, there are at least three ways to set the config for cquery:

use the project level compile_commands.json. If this exists, the ~/.cquery is ignored
use the user level ~/.cquery or $XDG_CONFIG_HOME/cquery. This is the fallback when item 1 does not exist.
use the cquery.cfg in the same folder of the cquery binary. This also works in the user level like item 2. But, more flexible in that the configuration here is combined with item 1 or item 2.

nick96 force-pushed the master branch from 3df340d to 31bfc04 Compare June 9, 2018 15:19

jacobdufault reviewed Jun 10, 2018

View reviewed changes

jacobdufault reviewed Jun 20, 2018

View reviewed changes

Nick Spain added 20 commits June 20, 2018 21:19

Create GetHomedirectory() function

80f5a36

This platform specific function is used to get the user's home directory. This gives us a base from where to get the global config file for. Currently WIP as I'm not sure how to implement this for Windows.

Find and parse .cquery in home dir if not in project dir

0e46b12

GetHomeDirectory() -> GetGlobalConfigDirectory()

f5e9327

Windows specific implementation of GetGlobalConfigDirectory()

2088655

This is a tentative implementation, I don't have access to windows so haven't tested it but based on MSDN and StackOverflow this seems to be the way to go.

Update *nix implementation of GetGlobalConfigDirectory()

c064647

As discussed in jacobdufault#648, fallback to $HOME if $XDG_CINFIG_HOME is not found.

Use a global .cquery file if none found in the project dir

5dc2539

Add rational to usage of XDG_CONFIG_HOME

003afd8

Create "cquery" directory in .config if it does not exist

a4961c7

Fix up on Windows

52bf7a5

Only call CoTaskMemFree if ShGetKnownFolderpath passes

f053939

Refactor config to optional to improve control flow

2e17fcb

Add comment referencing MSDN blog on getting config dir on Windows

2967f8f

Implicitly call string constructor using = operator

d03144e

Add braces around if-else block

16007e7

Fixup some errors introduced when making config optional

df188bf

config -> cfg

0671d5f

Fixup crash when assigning to config

ca91a37

Add reference for usage of CoTaskMemFree after SHGetKnownFolderPath

1acc50f

Load the local .cquery in all cases that it is present

4bbba0b

nick96 force-pushed the master branch from 2f140fe to 4bbba0b Compare June 20, 2018 11:44

Merge branch 'master' into master

7cb22ae

jacobdufault merged commit 76a7ea6 into jacobdufault:master Jun 29, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add global .cquery file #702

Add global .cquery file #702

nick96 commented Jun 9, 2018

nick96 commented Jun 10, 2018

jacobdufault Jun 10, 2018

nick96 Jun 11, 2018

nick96 Jun 11, 2018

jacobdufault Jun 10, 2018

jacobdufault Jun 10, 2018

jacobdufault Jun 10, 2018

nick96 Jun 11, 2018

jacobdufault Jun 10, 2018

nick96 Jun 11, 2018

jacobdufault Jun 10, 2018

nick96 Jun 11, 2018

jacobdufault commented Jun 10, 2018

jacobdufault commented Jun 10, 2018

nick96 commented Jun 11, 2018

jacobdufault left a comment

jacobdufault Jun 20, 2018

nick96 Jun 20, 2018

nick96 commented Jun 20, 2018

jacobdufault commented Jun 25, 2018

nick96 commented Jun 26, 2018

liushapku commented Dec 17, 2018

nick96 commented Dec 17, 2018

liushapku commented Dec 19, 2018

nick96 commented Jan 12, 2019

liushapku commented Jan 14, 2019 •

edited

Loading

Add global .cquery file #702

Add global .cquery file #702

Conversation

nick96 commented Jun 9, 2018

nick96 commented Jun 10, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jacobdufault commented Jun 10, 2018

jacobdufault commented Jun 10, 2018

nick96 commented Jun 11, 2018

jacobdufault left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nick96 commented Jun 20, 2018

jacobdufault commented Jun 25, 2018

nick96 commented Jun 26, 2018

liushapku commented Dec 17, 2018

nick96 commented Dec 17, 2018

liushapku commented Dec 19, 2018

nick96 commented Jan 12, 2019

liushapku commented Jan 14, 2019 • edited Loading

liushapku commented Jan 14, 2019 •

edited

Loading