Re: [PATCH] stgit: allow spaces in filenames

2005-07-14 Thread Catalin Marinas
On Wed, 2005-07-13 at 15:26 -0700, Junio C Hamano wrote:
 Catalin Marinas [EMAIL PROTECTED] writes:
 
  I'd very much like to stay on the same list.  By the same logic, cogito 
  should have it's own list as well...
 
  I'd like this too and it's probably OK with a low traffic (we'll see if
  we receive complaints :-) ).
 
 I'd like to keep Porcelain discussions on this list for two
 reasons:

That's great then. I will keep the StGIT discussions on this list.

--
Catalin


-
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] stgit: allow spaces in filenames

2005-07-14 Thread Bryan Larsen



Does it make that big difference if the commands are invoked via the
shell? I haven't run any tests.



It wasn't for the time difference that I bypassed the shell, it was to 
support spaces and other strange characters  in parameters.  It's easy 
to use spawnvp than it is to escape the parameters properly.


Bryan

-
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] stgit: allow spaces in filenames (second try)

2005-07-14 Thread Catalin Marinas
Bryan Larsen [EMAIL PROTECTED] 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.

 Signed-off-by: Bryan Larsen [EMAIL PROTECTED]

Applied. It will be visible tonight via the ftp mirror.

One note about patch description. I would prefer to have the
convention of the Linux kernel patches:

---
Short description line

Longer
description

Signed-off-by: ...
---

A future export command with support for sendpatcheset will take the
short description line and use it as a subject. Also, it would be nice
for the longer description to be wrapped somewhere before column 80
(~72 would be OK).

-- 
Catalin

-
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] stgit: allow spaces in filenames

2005-07-13 Thread Catalin Marinas
Bryan Larsen [EMAIL PROTECTED] 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

-
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] stgit: allow spaces in filenames

2005-07-13 Thread Bryan Larsen

Catalin Marinas wrote:

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.


I would have hoped that emacs py-mode would do the right thing. 
Anybody know how to make it do what Catalin wants?






+fin, fout = os.popen2(['git-ls-files', '--others',



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).


popen does not support bypassing the shell by using vectors of 
arguments.  Only popen2 and friends have this capability.


Unfortunate, yes.



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).



I'd very much like to stay on the same list.  By the same logic, cogito 
should have it's own list as well...


Bryan
-
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] stgit: allow spaces in filenames

2005-07-13 Thread Linus Torvalds


On Wed, 13 Jul 2005, Matthias Urlichs wrote:

 Hi, Bryan Larsen wrote:
 
  +   r=os.spawnvp(os.P_WAIT, args_l[0], args_l + args[i:min(i+1000, 
  len(args))])
 
 The max length for argv is 32k IIRC, so 1000 is 28-byte file names.

I think 32k may be the posix-mandated minimum. Linux does 128kB, so 1000 
should likely be ok for most projects.

That said, xargs should do things like that right.

Linus
-
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] stgit: allow spaces in filenames

2005-07-13 Thread Jerry Seutter

Bryan Larsen wrote:

Catalin Marinas wrote:


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.



I would have hoped that emacs py-mode would do the right thing. 
Anybody know how to make it do what Catalin wants?


Yeah, the default emacs mode seems to be to Do The Wrong Thing.  I have 
this in my .emacs, YMMV.


(setq-default indent-tabs-mode nil) ; Don't insert tab characters.
(setq-default tab-width 4)  ; If there are tabs, display
; as 4 spaces.

You can set the tab-width to something much larger to make existing tabs 
obvious.


Jerry
-
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] stgit: allow spaces in filenames

2005-07-13 Thread Catalin Marinas
On Wed, 2005-07-13 at 14:17 -0400, Bryan Larsen wrote:
 Catalin Marinas wrote:
 I would have hoped that emacs py-mode would do the right thing. 
 Anybody know how to make it do what Catalin wants?

It looks like the python-mode in my emacs does the right thing. You
could add something like below in your .emacs file:

(add-hook 'python-mode-hook
  #'(lambda ()
  (setq indent-tabs-mode nil)))

Otherwise, select the whole buffer and do a M-x untabify.

  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).
 
 popen does not support bypassing the shell by using vectors of 
 arguments.  Only popen2 and friends have this capability.

But the manual says that it is not possible to get the exit code of the
child process with popen2 (at least not in python 2.3). You would need
to use the Popen3 and Popen4 classes in the popen2 module.

Does it make that big difference if the commands are invoked via the
shell? I haven't run any tests.

  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).
  
 
 I'd very much like to stay on the same list.  By the same logic, cogito 
 should have it's own list as well...

I'd like this too and it's probably OK with a low traffic (we'll see if
we receive complaints :-) ).

Catalin

-
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html