-
Notifications
You must be signed in to change notification settings - Fork 3
Resolve symlinks in all plist/yaml files under the classic codechecker rule #14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
1be9fc4
540546a
9dd3fda
25cca5f
4b4bb49
afc7296
90ecb5c
a0a536f
6af6175
26d382d
36a802f
980cefd
e0da77f
4691ba1
f038f51
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,11 +27,17 @@ | |
| CODECHECKER_ENV = "{codechecker_env}" | ||
| COMPILE_COMMANDS = "{compile_commands}" | ||
|
|
||
| START_PATH = r"\/(?:(?!\.\s+)\S)+" | ||
| BAZEL_PATHS = { | ||
| r"\/sandbox\/processwrapper-sandbox\/\S*\/execroot\/": "/execroot/", | ||
| START_PATH + r"\/worker\/build\/[0-9a-fA-F]{16}\/root\/": "", | ||
| START_PATH + r"\/[0-9a-fA-F]{32}\/execroot\/": "", | ||
| # Running with Build Barn produces path output like: | ||
| # /worker/build/b301eed7f2bf2fd8/root/local_path.cc | ||
| # By removing the part until bin we have resolved the path to a local file | ||
| r"\/worker\/build\/[a-z0-9]{16}\/root\/": "", | ||
| # Sometimes the previous part is followed by this, before the local_path: | ||
| r"bazel-out\/k8-fastbuild\/bin\/": "", | ||
| # Virtual includes seems to be just a folder between the package and the | ||
| # folder containing the header files, like this: | ||
| # /path/to/package/_virtual_include/folder_with_headers/header.h | ||
| r"\/_virtual_includes\/" : "/", | ||
| } | ||
|
|
||
|
|
||
|
|
@@ -280,18 +286,17 @@ def resolve_symlinks(): | |
| files_processed = 0 | ||
| for root, _, files in os.walk(analyze_outdir): | ||
| for filename in files: | ||
| if re.search("clang-tidy", filename): | ||
furtib marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| filepath = os.path.join(root, filename) | ||
| if os.path.splitext(filepath)[1] == ".plist": | ||
| resolve_plist_symlinks(filepath) | ||
| elif os.path.splitext(filepath)[1] == ".yaml": | ||
| resolve_yaml_symlinks(filepath) | ||
| files_processed += 1 | ||
| filepath = os.path.join(root, filename) | ||
| if os.path.splitext(filepath)[1] == ".plist": | ||
| resolve_plist_symlinks(filepath) | ||
| elif os.path.splitext(filepath)[1] == ".yaml": | ||
| resolve_yaml_symlinks(filepath) | ||
| files_processed += 1 | ||
| logging.info("Processed file paths in %d files", files_processed) | ||
| def update_file_paths(): | ||
| """ Fix bazel sandbox paths and resolve symbolic links in generated files to real paths """ | ||
| fix_bazel_paths() | ||
| """ Resolve symlinks from local jobs, then try fixing path from remote executors """ | ||
furtib marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| resolve_symlinks() | ||
| fix_bazel_paths() | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay, whats the explanation here? Why the change of order? If possible, resolve, than fall back to replacing?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Exactly, we should not shoot ourselves in the foot, if we can use the syslinks |
||
|
|
||
|
|
||
| def parse(): | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I love the new explanations, but I think I'm gonna need a bit more info as to whether we've truly reproduced these with the new regexes. In particular, by omitting
START_PATH, you seem to produce absolute paths when symlinking fails, whereas the old regexes seem to produce relative paths always. Do we need to do this?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have not reproduced what these regex did. Only the middle one is reused, just without the START_PATH; this is most likely a remote executor-specific setting, our testing solution did not produce such a START_PATH. We should inquire more about these regexes. For now, it seems like for local runs, they are unnecessary; for remote jobs, it could be setup-dependent.