Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Test that clangd --check works at HEAD. #50901

Merged
merged 13 commits into from
Mar 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions .ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,12 @@ targets:
- ci/**
- flutter_frontend_server/**

- name: Linux clangd
recipe: engine_v2/engine_v2
bringup: true
properties:
config_name: linux_unopt_debug_no_rbe

- name: Linux linux_unopt
recipe: engine_v2/engine_v2
timeout: 120
Expand Down Expand Up @@ -407,6 +413,12 @@ targets:
drone_dimensions:
- os=Mac-13

- name: Mac clangd
recipe: engine_v2/engine_v2
bringup: true
properties:
config_name: mac_unopt_debug_no_rbe

- name: Mac mac_unopt
recipe: engine_v2/engine_v2
properties:
Expand Down
10 changes: 10 additions & 0 deletions .clangd
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# Used to allow clangd to run on CI platforms as part of //tools/clangd_check.
#
# Intended to have no effect elsewhere.
#
# See also:
# - https://clangd.llvm.org/config#compileflags
# - https://github.com/clangd/clangd/issues/662
CompileFlags:
Add: -Wno-unknown-warning-option
Remove: [-m*, -f*]
32 changes: 32 additions & 0 deletions ci/builders/linux_unopt_debug_no_rbe.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
{
"_comment": [
"This build is only used to generate compile_commands.json for clangd ",
"and should not be used for any other purpose."
],
"builds": [
{
"drone_dimensions": ["device_type=none", "os=Linux", "cores=32"],
"gn": [
"--runtime-mode",
"debug",
"--unoptimized",
"--prebuilt-dart-sdk",
"--no-rbe",
"--no-goma"
],
"name": "linux_unopt_debug_no_rbe",
"ninja": {
"config": "linux_unopt_debug_no_rbe",
"targets": ["flutter/build/dart:copy_dart_sdk"]
},
"tests": [
{
"language": "dart",
"name": "clangd",
"script": "flutter/tools/clangd_check/bin/main.dart",
"parameters": ["--clangd=buildtools/linux-x64/clang/bin/clangd"]
}
]
}
]
}
38 changes: 38 additions & 0 deletions ci/builders/mac_unopt_debug_no_rbe.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
{
"_comment": [
"This build is only used to generate compile_commands.json for clangd ",
"and should not be used for any other purpose."
],
"builds": [
{
"drone_dimensions": [
"device_type=none",
"os=Mac-13",
"cpu=x86",
"mac_model=Macmini8,1"
],
"gn": [
"--runtime-mode",
"debug",
"--unoptimized",
"--prebuilt-dart-sdk",
"--no-rbe",
"--no-goma",
"--xcode-symlinks"
],
"name": "mac_unopt_debug_no_rbe",
"ninja": {
"config": "mac_unopt_debug_no_rbe",
"targets": ["flutter/build/dart:copy_dart_sdk"]
},
"tests": [
{
"language": "dart",
"name": "clangd",
"script": "flutter/tools/clangd_check/bin/main.dart",
"parameters": ["--clangd=buildtools/mac-x64/clang/bin/clangd"]
}
]
}
]
}
1 change: 1 addition & 0 deletions ci/licenses_golden/licenses_flutter
Original file line number Diff line number Diff line change
Expand Up @@ -41990,6 +41990,7 @@ ORIGIN: ../../../flutter/vulkan/vulkan_utilities.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/vulkan/vulkan_window.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/vulkan/vulkan_window.h + ../../../flutter/LICENSE
TYPE: LicenseType.bsd
FILE: ../../../flutter/.clangd
FILE: ../../../flutter/.pylintrc
FILE: ../../../flutter/assets/asset_manager.cc
FILE: ../../../flutter/assets/asset_manager.h
Expand Down
27 changes: 27 additions & 0 deletions tools/clangd_check/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# `clangd_check`

`clangd_check` is a tool to run clangd on a codebase and check for diagnostics.

The practical use of this tool is intentionally limited; it's designed to
provide a quick way to verify that `clangd` is able to parse and analyze a
C++ codebase.

## Usage

```sh
dart ./tools/clangd_check/bin/main.dart
```

On success, and with no diagnostics, `clangd_check` will exit with status 0.

By default, `clangd_check` will try to infer the path of `clangd`, as well as
the path to `--compile-commands-dir` based on what artifacts are present in
`$ENGINE/src/out`.

You can also specify the path to `clangd` and `--compile-commands-dir` manually:

```sh
dart ./tools/clangd_check/bin/main.dart \
--clangd ../buildtools/mac-arm64/clang/bin/clangd \
--compile-commands-dir ../out/host_Debug_unopt_arm64
```
131 changes: 131 additions & 0 deletions tools/clangd_check/bin/main.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'dart:convert';
Copy link
Member

Choose a reason for hiding this comment

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

nit: Needs the engine copyright header.

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.

import 'dart:io' as io;

import 'package:args/args.dart';
import 'package:engine_repo_tools/engine_repo_tools.dart';
import 'package:path/path.dart' as p;

void main(List<String> args) {
final Engine? engine = Engine.tryFindWithin();
final ArgParser parser = ArgParser()
..addFlag(
'help',
abbr: 'h',
help: 'Print this usage information.',
negatable: false,
)
..addOption(
'clangd',
help: 'Path to clangd. Defaults to deriving the path from compile_commands.json.',
)
..addOption(
'compile-commands-dir',
help: 'Path to a directory containing compile_commands.json.',
defaultsTo: engine?.latestOutput()?.compileCommandsJson.parent.path,
);
final ArgResults results = parser.parse(args);
if (results['help'] as bool) {
io.stdout.writeln(parser.usage);
return;
}

final String? compileCommandsDir = results['compile-commands-dir'] as String?;
if (compileCommandsDir == null) {
io.stderr.writeln('Must provide a path to compile_commands.json');
io.exitCode = 1;
return;
}
final io.File compileCommandsFile = io.File(p.join(compileCommandsDir, 'compile_commands.json'));
if (!compileCommandsFile.existsSync()) {
io.stderr.writeln('No compile_commands.json found in $compileCommandsDir');
io.exitCode = 1;
return;
}

final List<Object?> compileCommands = json.decode(compileCommandsFile.readAsStringSync()) as List<Object?>;
if (compileCommands.isEmpty) {
io.stderr.writeln('Unexpected: compile_commands.json is empty');
io.exitCode = 1;
return;
}

String? clangd = results['clangd'] as String?;
final Map<String, Object?> entry = compileCommands.first! as Map<String, Object?>;
final String checkFile;
if (entry case {
'command': final String command,
'file': final String file,
}) {
// Given a path like ../../flutter/foo.cc, we want to check foo.cc.
Copy link
Member

Choose a reason for hiding this comment

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

Okay, I think I see. The path of file is relative to the path given by the directory entry in the entry map. You might be able to avoid manipulating the path of file entirely if you change the working directory for the clangd command to the directory path.

checkFile = p.split(file).skip(3).join(p.separator);
Copy link
Member

Choose a reason for hiding this comment

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

Some comments here about what's being skipped would be helpful.

// On CI, the command path is different from the local path.
// Find the engine root and derive the clangd path from there.
if (clangd == null) {
// Strip the command to find the path to the engine root.
// i.e. "command": "/path/to/engine/src/... arg1 arg2 ..."
//
// This now looks like "../../flutter/buildtools/{platform}/{...}"
final String commandPath = p.dirname(command.split(' ').first);

// Find the canonical path to the command (i.e. resolve "../" and ".")
//
// This now looks like "/path/to/engine/src/flutter/buildtools/{platform}/{...}"
final String path = p.canonicalize(
p.join(compileCommandsDir, commandPath),
);

// Extract which platform we're building for (e.g. linux-x64, mac-arm64, mac-x64).
final String platform = RegExp(
r'buildtools/([^/]+)/',
).firstMatch(path)!.group(1)!;

// Find the engine root and derive the clangd path from there.
final Engine compileCommandsEngineRoot = Engine.findWithin(path);
clangd = p.join(
// engine/src/flutter
compileCommandsEngineRoot.flutterDir.path,
// buildtools
'buildtools',
// {platform}
platform,
// clangd
'clang',
'bin',
'clangd',
);
}
} else {
io.stderr.writeln('Unexpected: compile_commands.json has an unexpected format');
io.stderr.writeln('First entry: ${const JsonEncoder.withIndent(' ').convert(entry)}');
io.exitCode = 1;
return;
}

// Run clangd.
try {
final io.ProcessResult result = io.Process.runSync(clangd, <String>[
'--compile-commands-dir',
compileCommandsDir,
'--check=$checkFile',
]);
io.stdout.write(result.stdout);
io.stderr.write(result.stderr);
if ((result.stderr as String).contains('Path specified by --compile-commands-dir does not exist')) {
io.stdout.writeln('clangd_check failed: --compile-commands-dir does not exist');
io.exitCode = 1;
} else if ((result.stderr as String).contains('Failed to resolve path')) {
io.stdout.writeln('clangd_check failed: --check file does not exist');
io.exitCode = 1;
} else {
io.exitCode = result.exitCode;
}
} on io.ProcessException catch (e) {
io.stderr.writeln('Failed to run clangd: $e');
io.stderr.writeln(const JsonEncoder.withIndent(' ').convert(entry));
io.exitCode = 1;
}
}
68 changes: 68 additions & 0 deletions tools/clangd_check/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
# Copyright 2013 The Flutter Authors. All rights reserved.
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.

name: clangd_check
publish_to: none

# Do not add any dependencies that require more than what is provided in
# //third_party/dart/pkg or //third_party/dart/third_party/pkg.
# In particular, package:test is not usable here.

# If you do add packages here, make sure you can run `pub get --offline`, and
# check the .packages and .package_config to make sure all the paths are
# relative to this directory into //third_party/dart

environment:
sdk: '>=3.2.0-0 <4.0.0'

dependencies:
args: any
engine_repo_tools: any
path: any
source_span: any

dev_dependencies:
async_helper: any
expect: any
litetest: any
process_fakes: any
smith: any

dependency_overrides:
args:
path: ../../../third_party/dart/third_party/pkg/args
async:
path: ../../../third_party/dart/third_party/pkg/async
async_helper:
path: ../../../third_party/dart/pkg/async_helper
collection:
path: ../../../third_party/dart/third_party/pkg/collection
engine_repo_tools:
path: ../pkg/engine_repo_tools
expect:
path: ../../../third_party/dart/pkg/expect
file:
path: ../../../third_party/dart/third_party/pkg/file/packages/file
git_repo_tools:
path: ../pkg/git_repo_tools
litetest:
path: ../../testing/litetest
meta:
path: ../../../third_party/dart/pkg/meta
path:
path: ../../../third_party/dart/third_party/pkg/path
platform:
path: ../../third_party/pkg/platform
process:
path: ../../third_party/pkg/process
process_fakes:
path: ../pkg/process_fakes
process_runner:
path: ../../third_party/pkg/process_runner
smith:
path: ../../../third_party/dart/pkg/smith
source_span:
path: ../../../third_party/dart/third_party/pkg/source_span
term_glyph:
path: ../../../third_party/dart/third_party/pkg/term_glyph
1 change: 1 addition & 0 deletions tools/pub_get_offline.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
os.path.join(ENGINE_DIR, 'tools', 'api_check'),
os.path.join(ENGINE_DIR, 'tools', 'build_bucket_golden_scraper'),
os.path.join(ENGINE_DIR, 'tools', 'clang_tidy'),
os.path.join(ENGINE_DIR, 'tools', 'clangd_check'),
os.path.join(ENGINE_DIR, 'tools', 'compare_goldens'),
os.path.join(ENGINE_DIR, 'tools', 'const_finder'),
os.path.join(ENGINE_DIR, 'tools', 'dir_contents_diff'),
Expand Down