Skip to content

Conversation

bdbaddog
Copy link
Contributor

@bdbaddog bdbaddog commented May 19, 2020

Closes #3355

Adds tool 'compilation_db', which adds Builder CompilationDatabase

env['COMPILATIONDB_USE_ABSPATH'] decides whether target and source file paths are relative or absolute.

Contributor Checklist:

  • I have created a new test or updated the unit tests to cover the new/changed functionality.
  • I have updated CHANGES.txt (and read the README.rst)
  • I have updated the appropriate documentation

@bdbaddog bdbaddog requested a review from mwichmann May 19, 2020 20:23
@bdbaddog bdbaddog self-assigned this May 19, 2020
@bdbaddog bdbaddog changed the title Add compilation db for use by clang tools and cquery (fix issue #3355) [WIP] Add compilation db for use by clang tools and cquery (fix issue #3355) May 19, 2020
@bdbaddog
Copy link
Contributor Author

@ivankravets - just pushed removal of debug output.

bdbaddog added 7 commits May 26, 2020 18:54
…r to clear source and set default target as 'compile_commands.json'.
…relative paths. If user doesn't specify target CompilationDatabase('my_output.json') SCons will swap what is by default the source to be the target and clear the source list. If no target specified use compile_commands.json. If more than one item specified as source, source is set to empty list
@bdbaddog bdbaddog changed the title [WIP] Add compilation db for use by clang tools and cquery (fix issue #3355) Add compilation db for use by clang tools and cquery (fix issue #3355) Jun 6, 2020
@bdbaddog bdbaddog added the clang clang support label Jun 6, 2020
@bdbaddog
Copy link
Contributor Author

bdbaddog commented Jun 6, 2020

Ok. This is more or less ready to merge.

Notes on usage and a question.

  1. If you call as env.CompilationDatabase('my_file.json') that argument will get assigned to the source list. I've set the emitter to use the source list (only if it has one element) as the target.
  2. If you call as env.CompilationDatabase() the emitter will fill in a default target as 'compile_commands.json'
  3. If you call as env.CompilationDatabase(['source1','source2']), the emitter will drop your list of sources.

I think all the above are reasonable, but my query is should any of the above magic transformations yield a warning from SCons?

@acmorrow @mwichmann thoughts?

@bdbaddog
Copy link
Contributor Author

bdbaddog commented Jun 6, 2020

Almost forgot.. Need to add some more to the test.

<tool name="compilation_db">
<summary>
<para>
Sets up &b-CompilationDatabase; builder which generates a clang tooling compatible compilation database.
Copy link
Collaborator

@mwichmann mwichmann Jun 7, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd reorder this:

Sets up the &b-CompilationDatabase; builder which generates a compilation database for clang tooling.

Since this will be separated from the Builder spec in the output documentation, use a link: &b-link-CompilationDatabase;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.

<summary>
<para>
The &b-CompilationDatabase; builder writes a
<ulink url="https://clang.llvm.org/docs/JSONCompilationDatabase.html">clang formatted compilation
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"clang formatted" doesn't seem to mean anything. If it wants to refer to the data format, say "JSON formatted". If it want to just make reference to clang, say something like "Compilation database according to the LLVM specification"

the C, C++, assembly source/target pairs.
</para>
<para>
NOTE: You must load the compilation_db tool prior to specifying any part of your build or some source/target
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use markup: &t-compilation_db;

<para>
This is a boolean flag to instruct &b-link-CompilationDatabase; to
write the <literal>file</literal> and <literal>target</literal> members
in the target file with absolute or relative path.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is confusing: target and targets refer to different things. Suggest make the second one "compilation database".

in the target file with absolute or relative path.
</para>
<para>
The default value is False.
Copy link
Collaborator

@mwichmann mwichmann Jun 7, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reword? boolean... with absolute or relative path. ... The default value is False. Text should indicate what False means (yes, you can deduce from name of var, but...). I'd propose (considering the previous comment also): "

This is a boolean flag to instruct the &b-link-CompilationDatabase; builder to record the <literal>file</literal> and <literal>target</literal> members in the compilation database using absolute paths. The default value is <constant>False</constant>, indicating relative paths will be used.

The &b-CompilationDatabase; builder writes a
<ulink url="https://clang.llvm.org/docs/JSONCompilationDatabase.html">clang formatted compilation
database
</ulink> which is consumed by a number of clang tools, editors, and other tools.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be useful to say for what... "which is consumed by" - to what end?

</ulink> which is consumed by a number of clang tools, editors, and other tools.
</para>
<para>
If you don't specify any files, the builder will default to <filename>compile_commands.json</filename>.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This must be where you had the question about supplying args to a builder spec, which our tooling doesn't provide for. Still, this sounds awkward: "if you don't speciffy any files"? The Builder Objects section in the manpage talks about calling builders, but doesn't seem to correlate with this usage?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the emitter..


Currently SCons supports two variations. One is to output source and target files with paths relative to the
top
of the SCons build, or to output those files with their absolute path.
Copy link
Collaborator

@mwichmann mwichmann Jun 7, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Usage mismatch: two variations. One is... or to.... I'd express this as:

&SCons; can output source and target files either with paths relative to the top of the build, or with absolute paths

The next para can be combined into this one as it's the same concept being described...

Copy link
Collaborator

@mwichmann mwichmann Jun 7, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or

The compilation database can be populated with source and target files either using paths relative to the top of the build, or using absolute paths

"directory": env.Dir("#").abspath,
"command": command,
"file": env["__COMPILATIONDB_USOURCE"][0],
"target": env['__COMPILATIONDB_UTARGET'][0]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'target' is not called out in the LLVM Spec, will that cause problems?

The contracts for each field in the command object are:

  • directory: The working directory of the compilation. All paths specified in the command or file fields must be either absolute or relative to this directory.
  • file: The main translation unit source processed by this compilation step. This is used by tools as the key into the compilation database. There can be multiple command objects for the same file, for example if the same source file is compiled with different configurations.
  • command: The compile command executed. After JSON unescaping, this must be a valid command to rerun the exact compilation step for the translation unit in the environment the build system uses. Parameters use shell quoting and shell escaping of quotes, with ‘"’ and ‘\’ being the only special characters. Shell expansion is not supported.
  • arguments: The compile command executed as list of strings. Either arguments or command is required.
  • output: The name of the output created by this compilation step. This field is optional. It can be used to distinguish different processing modes of the same input file.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm.. No idea. I know the current logic is in use and has been pulled in from MongoDB's source to several other projects without changing the field name?
@acmorrow @ivankravets thoughts?

@bdbaddog
Copy link
Contributor Author

bdbaddog commented Jun 8, 2020

Merging.

@bdbaddog bdbaddog merged commit ce32ce7 into SCons:master Jun 8, 2020
@bdbaddog bdbaddog deleted the add_compilation_db branch June 8, 2020 19:24
if not line.startswith(b'#link'):
opts, args = getopt.getopt(sys.argv[1:], 'o:s:')
for opt, arg in opts:
if opt == '-o':
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this broke some windows tests using mylink.py which get fed MSVC like args. Originally the code handled MSVC like args with things like a[:5].lower() == '/out:': out = a[5:]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang clang support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can we export compile_commands.json?
3 participants