Re: [PATCH] git-p4: Fix race between p4_edit and p4_change

From: Kevin Green <kevin.t.green@morganstanley.com>
Date: Apr 3, 2008, 18:45 UTC
Message-ID: <20080403184537.GH22542@morganstanley.com>
In-Reply-To: <200804032032.39860.simon@lst.de>

On 04/03/08 14:32:32, Simon Hausmann wrote:

Show 20 quoted lines

> On Wednesday 02 April 2008 00:28:56 Kevin Green wrote:
> > Hi,
> >
> > Ran into a nasty race today with git-p4.  The changelist Files: section was
> > showing up empty and it turned out to be a race between the p4_edit and
> > p4_change -o, e.g.
> >
> > $ p4 edit $file && p4 change -o
> >
> > will show no files in the Files: section.
> >
> > I attach a patch after my .sig as a suggested fix.  It simply loops over
> > the p4_changes -o as long as we're not finding any files (and we always
> > should since we just did a p4_edit!); sleeping for 3 secs in between to
> > allow Perforce to catch up with itself.
> 
> I don't mind the workaround in general as I agree this race is a bit nasy, but 
> shouldn't the sleep only happen if we didn't find any files? Right now even 
> if the server reacted immediately we still sleep for three seconds.
>

Oops. You're absolutely correct and that's not what I intended... (darn Python whitespace ;)

I didn't catch that logic error in my testing because I was expecting it to sleep anyhow...

Show 9 quoted lines

> 
> Last but not least we could of course also generate the entire Files: section 
> ourselves, using 'p4 change -o' just to get the rest of the template right.
> 
> I almost prefer the last approach, since we know the base depot path and the 
> relative paths of all edited/added files.
> 
> What do you think?
>

Thank you... That's the right approach. Stop as soon as we get to the Files: section and then just add in the depot + filepath string for each change...

--Kevin