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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exclude flag broken on Windows #191

Closed
wolf99 opened this Issue Jan 9, 2018 · 13 comments

Comments

Projects
None yet
4 participants
@wolf99

wolf99 commented Jan 9, 2018

Since #152 was merged it seems like gcov is now generating detailed HTML reports on Windows. So the following works as expected - yay! 馃檶

> gcovr -p -b --html --html-details -r . -o "build/artifacts/gcov/GcovCoverageResults.html"

However it appears that the exclude flag is now broken:

>gcovr -p -b -e "^vendor.*|^build.*|^test.*|^lib.*" --html --html-details -r . -o "build/artifacts/gcov/GcovCoverageResults.html"
Traceback (most recent call last):
  File "C:\Users\Me\AppData\Local\Programs\Python\Python36\Scripts\gcovr.py", line 2326, in <module>
    options.exclude[i] = re.compile(os.path.realpath(options.exclude[i]))
  File "C:\Users\Me\AppData\Local\Programs\Python\Python36\lib\re.py", line 233, in compile
    return _compile(pattern, flags)
  File "C:\Users\Me\AppData\Local\Programs\Python\Python36\lib\re.py", line 301, in _compile
    p = sre_compile.compile(pattern, flags)
  File "C:\Users\Me\AppData\Local\Programs\Python\Python36\lib\sre_compile.py", line 562, in compile
    p = sre_parse.parse(p, flags)
  File "C:\Users\Me\AppData\Local\Programs\Python\Python36\lib\sre_parse.py", line 855, in parse
    p = _parse_sub(source, pattern, flags & SRE_FLAG_VERBOSE, 0)
  File "C:\Users\Me\AppData\Local\Programs\Python\Python36\lib\sre_parse.py", line 416, in _parse_sub
    not nested and not items))
  File "C:\Users\Me\AppData\Local\Programs\Python\Python36\lib\sre_parse.py", line 502, in _parse
    code = _escape(source, this, state)
  File "C:\Users\Me\AppData\Local\Programs\Python\Python36\lib\sre_parse.py", line 368, in _escape
    raise source.error("incomplete escape %s" % escape, len(escape))
sre_constants.error: incomplete escape \U at position 2
@goriy

This comment has been minimized.

Contributor

goriy commented Jan 9, 2018

Look at Pull Request #158. This patch solved similar problems for me.
It introduces another way of processing exclude filters on Windows (proper escape etc) and it really works!

@latk

This comment has been minimized.

Member

latk commented Jan 9, 2018

This will be fixed once #158 is merged. But even then, the filter is slightly incorrect because the current working directory is prepended. I.e. you would actually end up with this regex:

C:\\some\\path\\^vendor.*|^build.*|...

The alternation | is totally wrong in that regex, and the ^ anchor won't match at the expected position. Instead, you would currently have to use a filter like this:

-e '(vendor|build|...)'

(the trailing .* is not necessary.)

In the mid term, I think ALL filters need to be redesigned and/or clarified (see also #151). Some operate on relative paths, some on absolute paths, some regexes like this one are prepended with a path, 鈥.

So at the very least this is a documentation bug, independent from #158.

@goriy

This comment has been minimized.

Contributor

goriy commented Jan 9, 2018

@wolf99 It seems that you call gcovr directly from command line on Windows. How did you do that?
I had to create gcovr.bat and place it to Scripts directory near gcovr script itself to get the ability to call gcovr directly. I even thought to propose inclusion of such .bat to upstream, but I temporary stop myself because maybe there is better way to do it.

my gcovr.bat:

@echo off
set PP=%~dp0
python %PP%gcovr %*
@wolf99

This comment has been minimized.

wolf99 commented Jan 9, 2018

@goriy I added the Python\Python36\Scripts and Python\Python36\ directories to my PATH system variable and added .PY;.PYC to my PATHEXT system variable. This means Python *.py script execution is available at the CLI. Then I added the .py extension to the gcovr file in the scripts directory so that the system recognizes it as a Python file. Job done.

@goriy

This comment has been minimized.

Contributor

goriy commented Jan 9, 2018

@wolf99 I had tried that way too. Yes, it worked, but I decided that it's a bit better to create additional file only once without manual renaming every upgrade. I hoped you could find some way to run gcovr without renaming...

I've seen some python packages which install .exe wrapper into Scripts folder when installing by means of pip on Windows and without such wrapper when installing on Linux.
Unfortunately, I don't know how it works. Is it done in package itself or maybe by pip during installation or by some other way.
And, for example, if something like gcovr.bat is included into package, I also don't know is it possible to install this gcovr.bat only on Windows.

@latk

This comment has been minimized.

Member

latk commented Jan 9, 2018

I have merged #158 and am trying to figure out how to make the tests on Windows run (#189). All that's missing is the filter-related tests.

Thanks to #158 filters are now constructed in an unified manner. Unfortunately, #158 introduced a slight regression because it assumes filters are relative paths. The filter-test2 and #137 show the intent that filters can be absolute paths:

GCOVR_TEST_OPTIONS = -f `pwd`'/main.cpp'

If that test were adapted to use native Windows paths, we'd get C:\somepath\main.cpp which is not a valid regex due to the backslashes. Additionally, the solution in #158 prepends the current working directory, so we'd actually end up with C:\\somepath\\C:\somepath\main.cpp. It is possible to avoid prepending the work dir when the path already is absolute, but that doesn't solve the problem of unexpected regex escapes (here: \s).

This raises a choice:

  • Should the filters use native separators, which may have to be escaped to form a valid regex?
    A filter C:\somepath\main.cpp won't work, but C:\\somepath\\main.cpp will.

  • Or should all filters use forward slashes?
    A filter C:\somepath\main.cpp won't work, but C:/somepath/main.cpp will.

In both cases will a Windows path have to be adapted. Currently, absolute paths don't work under Windows, but relative paths with escaped backslashes do. Cygwin-style paths /c/somepath/main.cpp won't work either way.

I think requiring forward slashes is the better solution. This will break a few scripts. But it provides a clear mental model that's easy to communicate: 鈥渇ilters must use forward slashes, even on Windows鈥.

Pseudocode for filter matching then becomes:

def normalize_path(path):
  return os.path.realpath(path).replace(os.path.sep, '/')

def match(filter, path):
  path = normalize_path(path)
  if not os.path.isabs(filter):
    filter = re.escape(normalize_path(os.getcwd())) + '/' + filter
  return re.match(filter, path)

This code should work on both Windows and Unixish systems. The current solution of feeding regex patterns into os.path.realpath() under non-Windows systems is meaningless and could be removed this way.

What are your thoughts on this? @denniswjackson I'd also be interested in your perspective.

@goriy

This comment has been minimized.

Contributor

goriy commented Jan 9, 2018

Generally I don't like the first case (where it's allowed to use backslashes as path separators in filters) because there's no way to distinguish between path separator and regex escape.

Second case seems much more attractive to me. I think that it's not a big problem to use only forward slashes as path separators in filters. Although they seem less "native" on Windows, but escaped double-backslashes seem even worse! It's impossible to use native windows paths "as is" anyway.

And I consider it a great advantage to be able to use the same scripts on different platforms.

As a result I agree that requiring forward slashes is the better solution.

@wolf99

This comment has been minimized.

wolf99 commented Jan 10, 2018

I agree that the second option is more immediately attractive, even just from a visual standpoint.

To play devil's advocate though, if I was a Windows-native developer, I might simply copy and paste the path from Windows Explorer or the Command Prompt - which would include back slashes and be wrong for both options thus leaving the hypothetical me confused.

Not to say that Gcovr should support such a scenario, but are there other cross platform Python scripts that also deal with regex input that we could look at to see how they expect such input and how it is managed there?

@dennykhoerniawan

This comment has been minimized.

dennykhoerniawan commented Feb 1, 2018

Hi, sorry if my question is dumb, but how are we suppose to use this exclude filter now on windows? can someone give an example?

I am new to gcovr, and I dont know much about regex. I have a gtest setup and want to exclude all my gtest framework code, I tried the following, but none were working:

-e '.*\\libraries\\TestFrameworks\\.*'
-e '.*gtest.*'
-e '.*gtest.cc'

Thanks in advance

@latk

This comment has been minimized.

Member

latk commented Feb 1, 2018

@dennykhoerniawan That's actually a very good question! Unfortunately, filtering is a bit of a mess.

For a moment I thought the new --exclude-directories option would help, but I'm pretty sure it's currently broken (the filter regex is created to match an absolute path, but it is actually used with a single folder name, not a path).

The -e/--exclude option should work if you installed gcovr directly from GitHub, and not the 3.3 release from PyPI. An option like -e ".*gtest" should exclude all sourcefiles where the path contains "gtest" 鈥 but on Windows the filters currently have to be a relative path that is under the current working directory. This will work:

project dir: C:\someproject
current dir: C:\someproject
--exclude                  .*gtest
filter this: C:\someproject\libraries\gtest\...

But this will not:

project dir: C:\someproject
current dir: C:\someproject\build
--exclude                        .*gtest
filter this: C:\someproject\libraries\gtest\...

Please report your results, because I'm not 100% sure about this myself. If this didn't work, please create a new issue where you explain

  • your project layout, incl. what you want to exclude,
  • the path from where you run gcovr,
  • the exact options with which you run gcovr,
  • and (the relevant parts of) the output when you run gcovr with those options in --verbose mode. This output explains which paths gcovr sees and why they were excluded/included. For a large project there might be lots of output, so consider saving the output as a file (gcovr .... >log.txt) and showing us an excerpt.

Thank you for your help. Without these reports, I wouldn't know about all the stuff that needs fixing :)

@latk

This comment has been minimized.

Member

latk commented May 22, 2018

I have finally implemented a new filtering system, see #257. Filters now use forward slashes on all systems incl. Windows. Please test & critique the design. Once that PR is merged, I intend to prepare a gcovr 4.0 release.

@latk

This comment has been minimized.

Member

latk commented Jun 3, 2018

Ok, PR #257 (filter redesign) has been merged 馃帀

@latk latk closed this Jun 3, 2018

@wolf99

This comment has been minimized.

wolf99 commented Jun 5, 2018

Well done @latk, you've been putting in a lot of hard work on gcovr - it is much appreciated by those of us that use it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment