Fix --exclude-directories option #266
Conversation
|
Thank you for trying to fix this, I'll take a closer look during the next few days. Btw you forgot to commit one of the TBH I don't really know what the Are you using or trying to use this option in your projects? If so, may I ask how? I'd like to tweak the behaviour to be actually useful :) |
Codecov Report
@@ Coverage Diff @@
## master #266 +/- ##
==========================================
+ Coverage 89.77% 90.39% +0.61%
==========================================
Files 13 13
Lines 1555 1551 -4
Branches 271 268 -3
==========================================
+ Hits 1396 1402 +6
+ Misses 105 98 -7
+ Partials 54 51 -3
Continue to review full report at Codecov.
|
|
Hi @latk, I've just added the missing file. Sorry for the omission. BTW, it had been working fine for a long time, but we updated |
|
Ok, so I took a deeper look.Sorry, this is a bit long :) (1) Can you add your name to the AUTHORS file? (2) Please feel free to rebase and force-push freely on this PR. I'll probably do some rebasing and squashing of fixup commits anyway when I merge your work. (3) I see that you tried to test for an empty Would you like to do that, possibly as a separate PR? (4) Your --exclude test looks good :) (5) In the link_walker() function, I assume some further changes might be necessary: for root, dirs, files in os.walk(os.path.abspath(path), followlinks=True):
for exc in exclude_dirs:
+ dirs[:] = [d for d in dirs if not exc.match(os.path.join(root, d))]
- for d in dirs:
- m = exc.match(d)
- if m is not None:
- dirs[:] = [d for d in dirs if d is not m.group()]
yield (os.path.abspath(os.path.realpath(root)), dirs, files)or something like that. I.e. remove the (6) In the --exclude-directories test as currently written, nothing is currently excluded. I think ideally the build process is changed so that the object files are placed into a directory structure like Then we can test whether What are your thoughts on the above ideas? I'm not sure whether they are sensible and/or will work. Thank you very very much for submitting these test cases. The link_walker() function is one of the remaining areas I still don't quite understand since I took over maintenance for gcovr, so finally putting them under test is very helpful. I assume that the --exclude-directories option was broken since gcovr 3.4 (published Feb 2018) in a commit that made the other filters work under Windows. The breakage probably became more noticeable in gcovr 4.0 when the filtering system was redesigned. I'm sorry that this caused any inconvenience on your end. I'll publish a bugfix release when this is resolved. |
|
Hey, thanks for all the suggestions. |
|
Oh, and I might have done some minor modifications (renaming variables) in between the commits. I may remove them or do a separate commit/PR if you want a clean PR. |
|
Thanks for this fix, I'm unfortunately trying to go through the same problem, I don't know whether it's planned or not, but may I suggest also updati 8000 ng the doc about this option? Thanks a lot for your help. |
21b7de0 to
5274619
Compare
5274619 to
87a8a2c
Compare
|
Thank you @mkurdej for working on this, I've merged the changes and have published them as the gcovr 4.1 release. If you'd still like to write those --exclude/--exclude-directory validations + tests, I would of course be delighted :) |
|
@totoc1001 I'm sorry to hear that gcovr is slow for you. I've made a new release with the fixed --exclude-directories option so you can update and use it now! I've also added a section to the docs about the behaviour of this option and various interactions. E.g. you might also want to try out the There is one big efficiency problem remaining: gcovr doesn't know which coverage data files belong to which source files, and runs the If this is costing you hours, you might want to investigate how it works and whether it can be optimized. I'd be really glad to have a more efficient solution. As a last resort, consider running gcov yourself to produce the .gcov files, and then invoke gcovr with the I hope this helps! |
|
Hello,
Thanks a lot for your response, I will try to use it very soon, I'll let
you know if it works well for me.
In my case it seems that the -j option is not really useful.
About your point to consider running gcov myself to produce the .gcov
files, this would be a very good idea, but I work with a CPP project and
then I use lcov to generate a report, and lcov removes automatically all
the gcov files, it seems unfortunately not possible to keep those files.
It is quite sad because lcov and genhtml works 2 to 3 times faster than
gcovr. I don't really know why we have this loss of performance.
Thanks a lot for your help.
Regards,
Thomas
Le lun. 2 juil. 2018 23:41, Lukas Atkinson <notifications@github.com> a
écrit :
… @totoc1001 <https://github.com/totoc1001> I'm sorry to hear that gcovr is
slow for you. I've made a new release with the fixed --exclude-directories
option so you can update and use it now!
I've also added a section to the docs
<https://gcovr.com/guide.html#speeding-up-coverage-data-search> about the
behaviour of this option and various interactions. E.g. you might also want
to try out the -j option that turns on parallel execution, but it doesn't
always help.
There is one big efficiency problem remaining: gcovr doesn't know which
coverage data files belong to which source files, and runs the gcov tool
in various directories until something works. The code for that is in
gcovr.gcov.process_datafile()
<https://github.com/gcovr/gcovr/blob/4.1/gcovr/gcov.py#L514>. I don't
really understand this code, or why it is necessary.
If this is costing you hours, you might want to investigate how it works
and whether it can be optimized. I'd be really glad to have a more
efficient solution.
As a last resort, consider running gcov yourself to produce the .gcov
files, and then invoke gcovr with the --use-gcov-files option. This is a
bit more fragile because you'll have to pick the right gcov options, but it
can potentially be a lot faster.
I hope this helps!
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#266 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AEhNOpO6L2Ib2NbP2MhrMzDPgXztHxyZks5uCpOegaJpZM4U2BzI>
.
|
--exclude-directories.--exclude.matchmethod instead ofsearchinFilters(link_walker).