With the exception, would this be used to print an error and fail if p4
isn't found?

The reason we return None upon failure is that not all installs will use p4,
and it actually shouldn't be failing if p4 is not installed. That check
there is to see if p4 help can be run, and if not, we assume that that type
of repository is just simply not supported on this system.

I might be misunderstanding, though.

What may be nice is to have some sort of option that lists all supported
repository types, or checks if a certain provided type is supported, so that
the calling script could verify this (basically an assertion check) before
calling post-review with the necessary options.

Christian

-- 
Christian Hammond - chip...@chipx86.com
Review Board - http://www.reviewboard.org
VMware, Inc. - http://www.vmware.com


On Thu, Aug 26, 2010 at 4:02 PM, RShelley <12gaugeme...@gmail.com> wrote:

> Well, I got past the P4PASSWD issue and then began getting the error
> "The current directory does not contain a checkout from a supported
> source code repository."  After much digging, I found that the error
> is actually wrong.
>
> The PerforceClient's "def_repository_info" function checks for a P4
> install first:
>
>    def get_repository_info(self):
>        if not check_install('p4 help'):
>            return None
>
>        data = execute(["p4", "info"], ignore_errors=True)
>
> If "p4" cannot be found for "p4 help", it returns None, and the
> calling function assumes that a repository wasn't found, not that P4
> couldn't be found.  As it turns out, the service running this script
> is changing the PATH and post-review cannot find P4, so this check
> fails.  More specifically, this is running from Hudson, and a recent
> upgrade of the app changed how environment variables cascaded to child
> processes, so when Hudson was upgraded, the PATH environment variables
> weren't propagated, and post-review couldn't find P4.
>
> I'd like to suggest that this "if" check actually throw an exception,
> and not simply returns None.  Thoughts?
>
> -Ryan
>
> On Aug 26, 1:47 am, Christian Hammond <chip...@chipx86.com> wrote:
> > I think it's definitely reasonable to have a parameter for this.
> >
> > The perforce.py mentioned above would be for Review Board. With
> post-review,
> > you'd be looking for postreview.py in rbtools.
> >
> > What I'd advise is to check out a copy of rbtools from Git and add a
> > --p4-ticket parameter. It shouldn't be too hard. You can see how the
> other
> > options work (which are defined near the bottom of the file), and make
> use
> > of the passed option to pass the right thing to p4. I don't know about
> the
> > ticketing stuff well enough to know what needs to be passed or set. If
> you
> > can get it working, though, I'll commit a patch so you won't have to
> > maintain a fork for too long.
> >
> > Christian
> >
> > --
> > Christian Hammond - chip...@chipx86.com
> > Review Board -http://www.reviewboard.org
> > VMware, Inc. -http://www.vmware.com
> >
> >
> >
> > On Wed, Aug 25, 2010 at 4:46 PM, RShelley <12gaugeme...@gmail.com>
> wrote:
> > > Dana, that sums it up pretty well.  My problem is that the post-commit
> > > command isn't being run by the user, it's run from a script on another
> > > box (so it's not using P4V).  "p4 login" is called first to attempt to
> > > login and obtain a ticket and then I can use the ticket for future
> > > commands, but post-review doesn't accept a ticket parameter and p4
> > > isn't taking it from the environment.  If I could pass it to post-
> > > review with the rest of the p4 properties, it'd be splendid.
> >
> > > On Aug 25, 3:04 pm, "Dana Lacoste" <dlaco...@aperio.com> wrote:
> > > > So, to summarize in case someone is searching through these archives
> > > > later:
> >
> > > > Perforce, when security is turned on (this is a p4d server setting)
> will
> > > > have different restrictions on what can and can't be done with the
> > > > password on the CLIENT (i.e. you can't necessarily simply set an
> > > > environment variable and run with it.  For reference, see the P4
> > > > SysAdmin Guide athttp://
> > >www.perforce.com/perforce/r10.1/manuals/p4sag/03_superuser.html#1
> > > > 081537 )
> >
> > > > User RShelley is running into issues with this and is having
> difficulty.
> >
> > > >http://reviews.reviewboard.org/r/1537/diff/hasa patch (which was
> > > > discarded) which "handles" this situation for user tag_98007work.
> >
> > > > But, if you're on Windows (or potentially in some other
> environments),
> > > > you're quite possibly using a .egg/.exe "compiled" version of Review
> > > > Board and therefore it may be difficult to implement patches to the
> .py
> > > > Python source.  If you use the interpreted version, then you can
> apply
> > > > the patch and things might work for you (if your environment is like
> > > > that of user tag_98007work )
> >
> > > > Does that seem to sum things up OK?
> >
> > > > Note that I don't see any of the problems described here, but I had
> to
> > > > do the following to get post-review to work for everyone:
> > > > 1 - Everyone has to have the following environment set: P4PORT,
> P4USER,
> > > > P4CLIENT (NOT P4PASSWD!)
> > > > 2 - Everyone has to be running P4V when they run post-review (I
> actually
> > > > added it as a tool to P4V so the users can right click on any
> changelist
> > > > and hit "submit to reviewboard" and nobody needs to use a command
> > > > line/shell command at all)
> > > > With perforce 2009.2/2010.1 this works fine with no problems : the
> > > > ticket from P4V is visible to the p4 command line (called from
> > > > post-review) and everything's fine (this is using latest RBTools
> > > > installed with the command "easy_install -U RBTools" and review board
> > > > 1.5 beta 2)
> >
> > > > Dana Lacoste
> >
> > > > -----Original Message-----
> > > > From: reviewboard@googlegroups.com [mailto:
> reviewbo...@googlegroups.com]
> >
> > > > On Behalf Of RShelley
> > > > Sent: Wednesday, August 25, 2010 1:37 PM
> > > > To: reviewboard
> > > > Subject: Re: post-review and P4PASSWD
> >
> > > > My head hurts... Perforce consistently gives me reasons to not like
> > > > it... Git can't come soon enough.
> >
> > > > Thanks for trying, I appreciate it!
> >
> > > --
> > > Want to help the Review Board project? Donate today at
> > >http://www.reviewboard.org/donate/
> > > Happy user? Let us know athttp://www.reviewboard.org/users/
> > > -~----------~----~----~----~------~----~------~--~---
> > > To unsubscribe from this group, send email to
> > > reviewboard+unsubscr...@googlegroups.com<reviewboard%2bunsubscr...@googlegroups.com>
> <reviewboard%2bunsubscr...@googlegr oups.com>
> > > For more options, visit this group at
> > >http://groups.google.com/group/reviewboard?hl=en
>
> --
> Want to help the Review Board project? Donate today at
> http://www.reviewboard.org/donate/
> Happy user? Let us know at http://www.reviewboard.org/users/
> -~----------~----~----~----~------~----~------~--~---
> To unsubscribe from this group, send email to
> reviewboard+unsubscr...@googlegroups.com<reviewboard%2bunsubscr...@googlegroups.com>
> For more options, visit this group at
> http://groups.google.com/group/reviewboard?hl=en
>

-- 
Want to help the Review Board project? Donate today at 
http://www.reviewboard.org/donate/
Happy user? Let us know at http://www.reviewboard.org/users/
-~----------~----~----~----~------~----~------~--~---
To unsubscribe from this group, send email to 
reviewboard+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/reviewboard?hl=en

Reply via email to