Re: [PATCH] stgit: allow spaces in filenames

From: Catalin Marinas <catalin.marinas@gmail.com>
Date: Jul 13, 2005, 10:17 UTC
Message-ID: <tnxll4bdn7b.fsf@arm.com>
In-Reply-To: <20050713083805.18461.87079.sendpatchset@bryan-larsens-ibook-g4.local>

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:

Show 9 quoted lines

> +# __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.

Show 20 quoted lines

> @@ -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