8000 Do not use realpath for DirectoryPrefixFilter by whart222 · Pull Request #712 · gcovr/gcovr · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Do not use realpath for DirectoryPrefixFilter #712

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 9 commits into from
Mar 8, 2023
Merged

Do not use realpath for DirectoryPrefixFilter #712

merged 9 commits into from
Mar 8, 2023

Conversation

whart222
Copy link
Member
@whart222 whart222 commented Feb 5, 2023 •
edited by Spacetown
Loading

A minor change to apply realpath() to the path provided to DirectoryPrefixFilter. This ensures that both the pattern used to match directories and the directory paths are converted to realpaths.

Closes #635

@Spacetown Spacetown added this to the Upcoming release milestone Feb 5, 2023
@Spacetown Spacetown added Type: Enhancement Filters related to filters, include/exclude, path handling labels Feb 5, 2023
@Spacetown
Copy link
Member

Hi William, this place is a little bit wired, see #635 (comment).
This will fix problems if the root directory is a symlink but break all project configurations symlinks are used inside the project structure. See the failing tests.
For the first problem we've added a warning:

gcovr/gcovr/__main__.py

Lines 271 to 278 in dce78fc

logger.warning(
"Your project --root directory seems to contain a symlink. "
"This will EXCLUDE source files in your root directory! "
"To fix this, you may have to add a --filter option:\n"
" --filter='%s'\n"
"For details, see https://github.com/gcovr/gcovr/issues/635",
re.escape(root_dir_realpath),
)

@Spacetown Spacetown removed this from the Upcoming release milestone Feb 5, 2023
@Spacetown Spacetown mentioned this pull request Feb 6, 2023
Closed
@whart222
Copy link
Member Author
whart222 commented Feb 6, 2023

I don't deny that this is an odd situation. However, the real impact of this PR is to ensure consistency in the gcovr logic. The pattern is updated using a call to realpath() [line 256 of utils.py], so then the user path should also be updated with a call to realpath(). Right?

@Spacetown Spacetown added this to the 6.0 milestone Feb 7, 2023
@Spacetown
Copy link
Member

@whart222 I've checked how to fix the erros of the CI. The linked test can be fixed by a filter but the other tests I've not managed.

After thinking a little bit about this default filter with the root directory I was thinking about removing the realpath from the init instead of adding it to the match. The reason is that this filter should match all files in the root directory. I don't understand why using realpath here.

Can you check my branch https://github.com/Spacetown/gcovr/tree/whart222 which is based on yours? With this branch the test are still working and it should solve your configuration of the home directory described in #635. In this case we can also remove the warning about a symlinked root directory.

@codecov
8000 Copy link
codecov bot commented Feb 15, 2023 •
edited
Loading

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.01 ⚠️

Comparison is base (85b17c5) 95.57% compared to head (08d5280) 95.56%.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master     #712      +/-   ##
==========================================
- Coverage   95.57%   95.56%   -0.01%     
==========================================
  Files          28       28              
  Lines        3862     3858       -4     
  Branches      671      670       -1     
==========================================
- Hits         3691     3687       -4     
  Misses         93       93              
  Partials       78       78
Flag Coverage Δ
ubuntu-20.04 94.26% <100.00%> (-0.01%) ⬇️
windows-2019 95.24% <100.00%> (+0.07%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
gcovr/__main__.py 93.60% <ø> (-0.08%) ⬇️
gcovr/tests/test_gcovr.py 98.83% <ø> (ø)
gcovr/utils.py 92.74% <100.00%> (-0.04%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@Spacetown Spacetown changed the title Convert DirectoryPrefixFilter with realpath Do not use realpath for DirectoryPrefixFilter Feb 15, 2023
@Spacetown
Copy link
Member

@whart222 Did you check if your problem is solved with the current version?

@Spacetown Spacetown requested a review from latk February 26, 2023 21:09
Spacetown
Spacetown approved these changes Feb 26, 2023
View reviewed changes
Copy link
Member
@Spacetown Spacetown left a comment

Choose a reason for hiding this comment

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

LGFM

@Spacetown Spacetown merged commit e2edaca into gcovr:master Mar 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Spacetown Spacetown Spacetown approved these changes

@latk latk Awaiting requested review from latk

Assignees
No one assigned
Labels
Filters related to filters, include/exclude, path handling Type: Enhancement
Projects
None yet
Milestone
6.0
Development

Successfully merging this pull request may close these issues.

gcovr not scanning files if source files located below virtual path
2 participants
@whart222 @Spacetown
0