Re: [PATCH v2 3/3] git-p4: Add test case for complex branch import

[Pete Wyckoff <pw@padd.com>]

luke@diamand.org wrote on Sat, 21 Jan 2012 10:51 +0000:
> On 21/01/12 04:54, Junio C Hamano wrote:
> >Vitor Antunes<vitor.hda@gmail.com>  writes:
> >
> >>+		grep -q update file2&&
> >
> >Do you really need to use "-q" here?  Wouldn't it help if you wrote it
> >without it while debugging tests with "sh ./t9801-*.sh -v"?
> >
> >Also how does this series interact with the series Luke posted earlier on
> >branches and labels?
> 
> Vitor's series applies cleanly to my changes.
> 
> However, one thing I noticed in reading through is that it will
> break if you end up importing a P4 branch that has spaces (or other
> shell chars) in its name. A quick test confirms this.
> 
> - the code doesn't handle the names properly
> - git and p4 have different ideas about valid branch names
> 
> But before rejecting Vitor's changes because of that it would be
> worth considering whether we care (much). My own opinion is that if
> you have developers who are daft enough to put spaces or dollars in
> their branch names then their project is already doomed anyway....
> 
> Perhaps it would be enough just to issue a warning ("your project is
> doomed; start working on your CV") and skip such branch names rather
> than falling over with inexplicable error messages.

This doesn't seem like a big deal.  The read_pipe and
read_pipe_lines calls shoud be list-ified.  That gets rid
of the problem with shell interactions.

For git branch name reserved characters, a little function
to replace the bogus characters with "_" would avoid needing
to go work on the resume.  Anything in bad_ref_char() and
check_refname_component().  I agree this doesn't have to be
perfect.

This could be a new patch unrelated to Vitor's series, which
verifies branch names anywhere a new commit is made.

		-- Pete