[tools] Add compilation database generator for Bazel #3226

nettle · 2021-03-12T13:43:10Z

The tool takes bazel build command as an input parameter
and executes Bazel Action Query (bazel aquery)
to extract all compile commands then saves them
to compile_commands.json file.

csordasmarton · 2021-03-16T11:25:30Z

tools/bazel/compile_commands/compile_commands.py

@@ -0,0 +1,255 @@
+#!/usr/bin/env python3


It would be better to rename this module and the upper directory to bazel_to_compile_commands. It is more expressive than just compile_commands.

Also it would be good to put this script into the bin folder during the package phase similar to this:

codechecker/Makefile

Lines 49 to 57 in e02baa7

build_tu_collector:

$(MAKE) -C $(ROOT)/tools/tu_collector build

package_tu_collector: build_tu_collector package_dir_structure

# Copy tu_collector files.

cp -rp $(CC_TOOLS)/tu_collector/build/tu_collector/tu_collector $(CC_BUILD_LIB_DIR) && \

chmod u+x $(CC_BUILD_LIB_DIR)/tu_collector/tu_collector.py && \

cd $(CC_BUILD_DIR) && \

ln -sf ../lib/python3/tu_collector/tu_collector.py bin/tu_collector

Thanks for reviewing @csordasmarton!
I'll check and update the patch as soon as I have time.

Meanwhile I have a few questions:

Regarding the module name I think bazel_to_compile_commands has kind of wrong meaning, it is not a converter.
Probably bazel_compile_commands would be better. Agree?

I put this module into bazel folder, is it OK?

How can find information about verification infrastructure you use?
Are there any guidelines? Check list?

@nettle

Yes, you are right, bazel_compile_commands will be good.

You can put this into the bazel folder (tools/bazel/bazel_compile_commands/bazel_compile_commands.py)

No, there isn't any guidline for this. This is how we built our others tools (https://github.com/Ericsson/codechecker/tree/master/tools, https://github.com/Ericsson/codechecker/tree/master/analyzer/tools). And it is good to be consistent in the whole repository.

Done:

Renamed to bazel_compile_commands

Using bazel folder

Added copying to bin folder

tools/bazel/tests/Makefile

csordasmarton · 2021-03-16T13:02:11Z

tools/bazel/setup.py

+    long_description = fh.read()
+
+setuptools.setup(
+    name="bazel-compile-commands",


Suggested change

name="bazel-compile-commands",

name="bazel-to-compile-commands",

csordasmarton · 2021-03-16T13:05:53Z

tools/bazel/compile_commands/compile_commands.py

+    parser.add_argument("-v", "--verbosity",
+                        default=0,
+                        action="count",
+                        help="increase output verbosity (e.g., -v or -vv)")


In other commands this option has no value, it will enabled the debug logging. Maybe we can use the same logic here too:

codechecker/tools/tu_collector/tu_collector/tu_collector.py

Lines 567 to 577 in e02baa7

parser.add_argument('-v', '--verbose',

action='store_true',

dest='verbose',

help="Enable debug level logging.")

args = parser.parse_args()

# --- Checking the existence of input files. --- #

if 'verbose' in args and args.verbose:

LOG.setLevel(logging.DEBUG)

Done:

Verbosity option (-v or --verbose) works the same way as in tu_collector

csordasmarton · 2021-03-16T13:36:42Z

tools/bazel/README.md

+make venv
+source $PWD/venv/bin/activate
+
+# Build and install plist-to-html package.


Suggested change

# Build and install plist-to-html package.

# Build and install bazel-to-compile-commands package.

csordasmarton · 2021-03-16T14:05:58Z

tools/bazel/compile_commands/compile_commands.py

+
+    bazel_info = split_to_list(build_command)[0] + " info "


Suggested change

bazel_info = split_to_list(build_command)[0] + " info "

cmd = shlex.split(build_command)

bazel_info = cmd[0] + " info "

Yes, I agree that we can use shlex.split however split_to_list is more convenient and safer.
I would keep it if you dont mind.

csordasmarton · 2021-03-16T14:07:27Z

tools/bazel/compile_commands/compile_commands.py

+    logging.info("Bazel work dir:      %s", bazel_work_dir)
+
+    logging.info("Bazel build options: %s", build_command)
+    data = bazel_aquery(build_command)


Use the cmd here, so this function will expect a list instead of a string. And there will be no need to call the split function on the argument.

Yes, I agree that we can use shlex.split however split_to_list is more convenient and safer.
I would keep it if you dont mind.

csordasmarton · 2021-03-16T14:09:49Z

tools/bazel/compile_commands/compile_commands.py

+    """
+    Run shell command
+    """
+    cmd = split_to_list(command)


On the call side pass the command as a list of string and there will be no need to split the argument here.

Yes, I agree that we can use shlex.split however split_to_list is more convenient and safer.
I would keep it if you dont mind.

csordasmarton · 2021-03-16T14:12:58Z

tools/bazel/compile_commands/compile_commands.py

+    if out:
+        data = json.loads(out)
+        return data
+    else:
+        return False


Now the function may return with None (see the code above L101), a dictionary or a Boolean (False) value. Couldn't we return here with None if the output is empty?

Suggested change

if out:

data = json.loads(out)

return data

else:

return False

if out:

return json.loads(out)

csordasmarton · 2021-03-16T14:20:18Z

tools/bazel/compile_commands/compile_commands.py

+
+def get_full_path(file, directories):
+    """
+    Return full path for a file and try to prepend directories


This function will return not only with the full path but the parent diectory too. It would be better to return only with the full path or None and use the os.path.dirname to get the parent directory for the file if its not None on the call side.

Done:

Changed the name and docstring of the function since get_full_path is meisleading

csordasmarton · 2021-03-16T14:21:50Z

Please also fix the commit message: [tols] -> [tools]

nettle · 2021-03-26T09:39:55Z

Hi @csordasmarton,

I've updated the patch upon comments.
Hopefully now it has all required infrastructural parts :)
Please note that a few test cases I've added are just a placeholder and more tests will be added later.
The patch is also missing some functional improvements which I plan to add later.
I guess it is important to move consecutively, so I consider this patch as earlier version of compile commands generator for Bazel, it will be evolving.

csordasmarton

I have some small comments otherwise its LGTM!

csordasmarton · 2021-03-31T08:54:53Z

tools/bazel/.noserc

+stop=1
+
+# do not capture stdout
+nocapture=1


Suggested change

nocapture=1

#nocapture=1

tools/bazel/tests/unit/test_compile_commands.py

csordasmarton · 2021-03-31T10:18:07Z

tools/bazel/bazel_compile_commands/bazel_compile_commands.py

+    parser.add_argument("-b", "--build",
+                        help="bazel build command arguments")
+    parser.add_argument("-v", "--verbosity",
+                        default=0,


I think the default log level should be 1 because if I run this command there will be no output at all and it can confuse the users: did this command run successfully or not?

The tool takes bazel build command as an input parameter and executes Bazel Action Query (bazel aquery) to extract all compile commands then saves them to compile_commands.json file.

nettle · 2021-04-06T10:31:13Z

Hi @csordasmarton,
The patch is updated, please take a look!

csordasmarton

It's LGTM! Thank you for your time that you implemented it 😊

Do you plan to add anything to this PR or can we merge it?

nettle · 2021-04-06T16:41:03Z

Thanks for review @csordasmarton!
I think this patch is good to go now.
I will prepare some changes and submit them later as a new PR, if you dont mind.

nettle requested a review from csordasmarton as a code owner March 12, 2021 13:43

csordasmarton suggested changes Mar 16, 2021

View reviewed changes

csordasmarton added this to the release 6.16.0 milestone Mar 16, 2021

csordasmarton added the tools 🛠️ Meta-tag for all the additional tools supplied with CodeChecker: plist2html, tu_collector, etc. label Mar 16, 2021

nettle force-pushed the bazel-compile-commands branch 2 times, most recently from 6223890 to 22a168b Compare March 25, 2021 09:03

nettle changed the title ~~[tols] Add compilation database generator for Bazel~~ [tools] Add compilation database generator for Bazel Mar 25, 2021

nettle force-pushed the bazel-compile-commands branch from 22a168b to 54ecd04 Compare March 25, 2021 11:43

nettle requested a review from bruntib as a code owner March 25, 2021 11:43

nettle force-pushed the bazel-compile-commands branch 2 times, most recently from d91df39 to 84e4174 Compare March 25, 2021 18:14

csordasmarton suggested changes Mar 31, 2021

View reviewed changes

nettle force-pushed the bazel-compile-commands branch from 84e4174 to e981664 Compare April 6, 2021 09:48

[tools] Add compilation database generator for Bazel

3a3a928

The tool takes bazel build command as an input parameter and executes Bazel Action Query (bazel aquery) to extract all compile commands then saves them to compile_commands.json file.

nettle force-pushed the bazel-compile-commands branch from e981664 to 3a3a928 Compare April 6, 2021 10:10

csordasmarton approved these changes Apr 6, 2021

View reviewed changes

csordasmarton added documentation 📖 Changes to documentation. test ☑️ Adding or refactoring tests labels Apr 6, 2021

csordasmarton merged commit a628f5e into Ericsson:master Apr 6, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[tools] Add compilation database generator for Bazel #3226

[tools] Add compilation database generator for Bazel #3226

nettle commented Mar 12, 2021

csordasmarton Mar 16, 2021

nettle Mar 16, 2021

csordasmarton Mar 18, 2021

nettle Mar 26, 2021

csordasmarton Mar 16, 2021

nettle Mar 26, 2021

csordasmarton Mar 16, 2021

nettle Mar 26, 2021

csordasmarton Mar 16, 2021

nettle Mar 26, 2021

csordasmarton Mar 16, 2021

nettle Mar 26, 2021

csordasmarton Mar 16, 2021

nettle Mar 26, 2021

csordasmarton Mar 16, 2021

nettle Mar 26, 2021

csordasmarton Mar 16, 2021

nettle Mar 26, 2021

csordasmarton Mar 16, 2021

nettle Mar 26, 2021

csordasmarton commented Mar 16, 2021

nettle commented Mar 26, 2021

csordasmarton left a comment

csordasmarton Mar 31, 2021

nettle Apr 6, 2021

csordasmarton Mar 31, 2021

nettle Apr 6, 2021

nettle commented Apr 6, 2021

csordasmarton left a comment

nettle commented Apr 6, 2021

	build_tu_collector:
	$(MAKE) -C $(ROOT)/tools/tu_collector build

	package_tu_collector: build_tu_collector package_dir_structure
	# Copy tu_collector files.
	cp -rp $(CC_TOOLS)/tu_collector/build/tu_collector/tu_collector $(CC_BUILD_LIB_DIR) && \
	chmod u+x $(CC_BUILD_LIB_DIR)/tu_collector/tu_collector.py && \
	cd $(CC_BUILD_DIR) && \
	ln -sf ../lib/python3/tu_collector/tu_collector.py bin/tu_collector

	name="bazel-compile-commands",
	name="bazel-to-compile-commands",

	parser.add_argument('-v', '--verbose',
	action='store_true',
	dest='verbose',
	help="Enable debug level logging.")

	args = parser.parse_args()

	# --- Checking the existence of input files. --- #

	if 'verbose' in args and args.verbose:
	LOG.setLevel(logging.DEBUG)

	# Build and install plist-to-html package.
	# Build and install bazel-to-compile-commands package.

-    bazel_info = split_to_list(build_command)[0] + " info "
+    cmd = shlex.split(build_command)
+    bazel_info = cmd[0] + " info "

[tools] Add compilation database generator for Bazel #3226

[tools] Add compilation database generator for Bazel #3226

Conversation

nettle commented Mar 12, 2021

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

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

csordasmarton commented Mar 16, 2021

nettle commented Mar 26, 2021

csordasmarton left a 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

nettle commented Apr 6, 2021

csordasmarton left a comment

Choose a reason for hiding this comment

nettle commented Apr 6, 2021