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

Fix exclude filters on Windows #158

Merged
merged 1 commit into from Jan 9, 2018

Conversation

Projects
None yet
4 participants
@jessicalevine
Contributor

jessicalevine commented Nov 23, 2016

When building exclusion filters from CLI input on Windows, the backslash
path separators from os.path.realpath aren't properly escaped before
being built into a regex.

This refactors the process of building the filter into a function, and
includes alternate logic for constructing the filter on Windows in order
to escape the path separators without escaping the CLI input.

Fix exclude filters on Windows
When building exclusion filters from CLI input on Windows, the backslash
path separators from os.path.realpath aren't properly escaped before
being built into a regex.

This refactors the process of building the filter into a function, and
includes alternate logic for constructing the filter on Windows in order
to escape the path separators without escaping the CLI input.
# regex, but we do not want to escape the regex itself, so instead of
# using realpath, we escape the path and join it manually (realpath
# doesn't resolve symlinks on Windows anyway)
return re.compile(re.escape(os.getcwd() + "\\") + regex)

This comment has been minimized.

@latk

latk Dec 27, 2017

Member

This looks like a reasonable solution to fix the Windows filter problem, but do we really need a separate code path just for Windows? Wouldn't something like this also work?:

return re.compile(re.escape(os.getcwd() + os.path.sep) + regex)

Or do you see any drawbacks with that approach, e.g. around symlinks?

This comment has been minimized.

@barthoukes

barthoukes Mar 27, 2018

At line 505 in gcovr, "Return if the filename does not match the filter" it will always return in Windows. For me this is a serious issue... by removing 10 lines of code the gcovr tool is really working great for me!!!! wasted 2 days on this grgrrgr...

Around line 518 in gcovr, find following part of code:

    #
    # Return if the filename does not match the filter
    #
    filtered_fname = None
    for i in range(0, len(options.filter)):
        if options.filter[i].match(fname):
            filtered_fname = options.root_filter.sub('', fname)
            break
    if filtered_fname is None:
        if options.verbose:
            sys.stdout.write("  Filtering coverage data for file %s\n" % fname)
        return

and change this code in:

    #
    # Return if the filename does not match the filter
    #
    #filtered_fname = None
    #for i in range(0, len(options.filter)):
    #    if options.filter[i].match(fname):
    #        filtered_fname = options.root_filter.sub('', fname)
    #        break
    #if filtered_fname is None:
    #    if options.verbose:
    #        sys.stdout.write("  Filtering coverage data for file %s\n" % fname)
    #    return

Remember not to use tabs, only spaces are allowed!
This will cure the windows problem. If there are too many files, just exclude them the right way.

This comment has been minimized.

@latk

latk Mar 27, 2018

Member

@barthoukes Thank you for investigating this, and I'm sorry that you found gcovr difficult to use. But please note that this discussion should be limited to the changes in this pull request.

You seem to have found a related but different issue. Please consider opening a new issue for your problem (see our contributing guide). If this is a solution to your Stack Overflow question, consider answering there.

@latk latk added the needs review label Dec 27, 2017

@goriy

This comment has been minimized.

Contributor

goriy commented Jan 8, 2018

I have been using gcovr with this patch for several months on Windows (python 3.5). It works fine!

After upgrading python to version 3.6.4, gcovr began to crash in one of my scripts.

Traceback (most recent call last):
  File "C:\Python36\Scripts\gcovr", line 2338, in <module>
    options.exclude[i] = build_filter(options.exclude[i])
  File "C:\Python36\Scripts\gcovr", line 2333, in build_filter
    return re.compile(re.escape(os.getcwd() + "\\") + regex)
  File "C:\Python36\lib\re.py", line 233, in compile
    return _compile(pattern, flags)
  File "C:\Python36\lib\re.py", line 301, in _compile
    p = sre_compile.compile(pattern, flags)
  File "C:\Python36\lib\sre_compile.py", line 562, in compile
    p = sre_parse.parse(p, flags)
  File "C:\Python36\lib\sre_parse.py", line 855, in parse
    p = _parse_sub(source, pattern, flags & SRE_FLAG_VERBOSE, 0)
  File "C:\Python36\lib\sre_parse.py", line 416, in _parse_sub
    not nested and not items))
  File "C:\Python36\lib\sre_parse.py", line 502, in _parse
    code = _escape(source, this, state)
  File "C:\Python36\lib\sre_parse.py", line 401, in _escape
    raise source.error("bad escape %s" % escape, len(escape))
sre_constants.error: bad escape \p at position 35

When I changed builtin package re to third-party regexp (import regex as re instead of import re) - everything began to work again. There are many such advices on different sites, but I don't like this solution in this particular case.

One of gcovr command line parameters in my script looks like -e ".*\ports\.*".
I changed it to -e ".*\\ports\\.*" and error was gone!
So I think it's correct to escape regexps on command line when necessary (problem is in my script, not in patched gcovr), but it seems strange that it worked fine with python's versions 3.5.x.

@latk

This comment has been minimized.

Member

latk commented Jan 9, 2018

@goriy Thank you for testing this.

Note that the -e argument is a regex, so if you want a literal backslash you must escape it in the regex.

The Python 3.6 release notes state:

Unknown escapes consisting of \ and an ASCII letter in regular expressions will now cause an error.

So that's why the \p escape stopped working. This check is within the re module. The regex module does not warn on unknown escapes.

@latk

latk approved these changes Jan 9, 2018

@latk latk merged commit cd3d1e0 into gcovr:master Jan 9, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@latk latk removed the needs review label Jan 9, 2018

@goriy

This comment has been minimized.

Contributor

goriy commented Jan 9, 2018

@latk Thank you for clarification.

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