[PATCH 1/2] Use git status -z

Colin Walters walters at verbum.org
Wed Nov 26 13:05:26 UTC 2014


On Tue, Nov 25, 2014, at 02:50 PM, David Shea wrote:

> -    git_files = check_output("git status --porcelain", shell=True)
> +    git_files = check_output("git status --porcelain -z", shell=True)

This is not your bug, but seeing "shell=True" always makes me shudder.  It's perfectly safe here, but I've seen it in other contexts where someone later comes by and adds a user-provided argument.  In a web context, that's an instant remote shell.  In a local context, it's a reliability issue with filenames that might contain shell metacharacters.

> +for gf in git_iter:
>      # If the file is being removed, or is not in git, or is not part of this
>      # commit, ignore it
>      if gf[0] in ('D', '?', ' '):
>          continue
> -    # If the file is being renamed, grab the target filename
> +
> +    # If the file is being renamed, the target filename (which we want) is on
> +    # the current line, and the next line is the source, which we want to skip
>      if gf[0] == 'R':
> -        target = gf[3:].split(' -> ')[1]
> -        if is_python(target):
> -            pylint_files.append(target)
> -    elif is_python(gf[3:]):
> +        git_iter.next()

We're also skipping the next file if something is not in git, which is
not what we want, right?  Something like:

if gf[0] in ('?', ' '):
    continue
elif gf[0] == 'D':
    git_iter.next()


More information about the anaconda-patches mailing list