Re: [PATCH] stgit: allow spaces in filenames

[Catalin Marinas <catalin.marinas@gmail.com>]

Bryan Larsen <bryanlarsen@yahoo.com> wrote:
> The current version of stgit does not allow whitespace in filenames.
> This patch fixes that.  It also speeds up operations on large
> filesets considerably.

Thanks, I will apply it but I have a few comments below:

> +# __run: runs cmd using spawnvp.
> +#
> +#  The shell is avoided so it won't mess up our arguments.
> +# If args is very large, the command is run multiple times;
> +# args is split xargs style:  cmd is passed on each invocation.
> +# Unlike xargs, returns immediately if any non-zero return code 
> +# is received.
> +#
> +def __run(cmd, args=None):

I would prefer to add this as Python function documentation, i.e. with
"""...""" in the function body.

An additional thing, can you please convert all the tabs to spaces?
That's a better convention for a language like Python where you
delimit blocks by indentation.

> @@ -114,14 +132,16 @@ def __tree_status(files = [], tree_id = 
>      # unknown files
>      if unknown:
>          exclude_file = os.path.join(base_dir, 'exclude')
> -        extra_exclude = ''
> +        extra_exclude = []
>          if os.path.exists(exclude_file):
> -            extra_exclude += ' --exclude-from=%s' % exclude_file
> -        fout = os.popen('git-ls-files --others'
> -                        ' --exclude="*.[ao]" --exclude=".*"'
> -                        ' --exclude=TAGS --exclude=tags --exclude="*~"'
> -                        ' --exclude="#*"' + extra_exclude, 'r')
> +            extra_exclude.append('--exclude-from=%s' % exclude_file)
> +        fin, fout = os.popen2(['git-ls-files', '--others',
> +                        '--exclude=*.[ao]', '--exclude=.*'
> +                        '--exclude=TAGS', '--exclude=tags', '--exclude=*~',
> +                        '--exclude=#*'] + extra_exclude)
>          cache_files += [('?', line.strip()) for line in fout]
> +	fin.close()
> +	fout.close()

What's the reason for having 'fin' as well? It doesn't seem to be used
(this is found in other parts of the patch as well).

Something wrong happened to my git-ftp-push script. It looks like it
copied 'master' to stgit.git/ and not to stgit.git/refs/heads/. I
can't fix it until tonight. Before then, you could actually use wget
to pull the whole repository and just copy 'master' to
'refs/heads/master'. In the latest tree I created separate files for
each command.

I'm not sure whether the GIT guys are happy for us to use this mailing
list for StGIT. If the StGIT traffic increases, I will try to create a
separate mailing list (maybe using a site like sf.net).

-- 
Catalin