Skip to content
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

[analyzer] Fix check for response files #3474

Merged

Conversation

ferkulat
Copy link
Contributor

Instead of checking for @ in the file name, check if a file name
starts with @.
Fixes #3463.

Codechecker supports response files. These are files which you can pass to
clang to work around limits in command line length.
Those files names start with a @.
The CI/CD solution "Jenkins" creates workspaces by putting @ in the path name.
A file entry in compile_commands.json could look like:
"file": "/home/workspace/branchname@2/somefile.cpp"

In this case all files in the compile data base are treated as response file.
And the analyzer skips all files and does nothing.

With the help of @csordasmarton this got fixed by checking if a file starts with an @.

Instead of checking for `@` in the file name, check if a file name
starts with `@`.
Fixes Ericsson#3463.

Codechecker supports response files. These are files which you can pass to
clang to work around limits in command line length.
Those files names start with a `@`.
The CI/CD solution "Jenkins" creates workspaces by putting `@` in the path name.
A file entry in compile_commands.json could look like:
"file": "/home/workspace/branchname@2/somefile.cpp"

In this case all files in the compile data base are treated as response file.
And the analyzer skips all files and does nothing.

With the help of @csordasmarton this got fixed by checking if a file starts with an `@`.
Copy link
Contributor

@csordasmarton csordasmarton left a comment

Choose a reason for hiding this comment

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

Please add a test case for this change. Otherwise LGTM!

@@ -1178,7 +1178,7 @@ def extend_compilation_database_entries(compilation_database):

entry['command'] = ' '.join(cmd)

if '@' in entry['file']:
if entry['file'].startswith('@'):
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please add the following test case to https://github.com/Ericsson/codechecker/blob/master/analyzer/tests/unit/test_log_parser.py file:

    def test_source_file_contains_at_sign(self):
        """
        Test source file which path contains '@' sign in path.

        Source file path can contain '@' sign which doesn't mean it is a
        response file.
        """
        with tempfile.TemporaryDirectory(suffix='@') as tmp_dir:
            src_file_path = shutil.copy(self.src_file_path, tmp_dir)

            with open(self.compile_command_file_path, "w",
                      encoding="utf-8", errors="ignore") as f:
                f.write(json.dumps([dict(
                    directory=tmp_dir,
                    command=f"g++ {src_file_path}",
                    file=src_file_path
                )]))
            
        build_actions, _ = log_parser.parse_unique_log(load_json_or_empty(
            self.compile_command_file_path), self.__this_dir)

        self.assertEqual(len(build_actions), 1)

        build_action = build_actions[0]
        self.assertEqual(build_action.source, src_file_path)

Just to make sure if it works properly.

@csordasmarton csordasmarton added analyzer 📈 Related to the analyze commands (analysis driver) bugfix 🔨 labels Nov 2, 2021
@csordasmarton csordasmarton added this to the release 6.18.0 milestone Nov 2, 2021
Copy link
Contributor

@csordasmarton csordasmarton left a comment

Choose a reason for hiding this comment

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

LGTM!

@csordasmarton
Copy link
Contributor

@ferkulat Thank you for this patch 😊

@csordasmarton csordasmarton merged commit 902d989 into Ericsson:master Nov 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analyzer 📈 Related to the analyze commands (analysis driver) bugfix 🔨
Projects
None yet
Development

Successfully merging this pull request may close these issues.

files with a path that contains '@' are skipped
2 participants