Ok, I can understand that. But I don't think that check should be during the def_repository_info function. I imagine the "check_install" function should probably happen somewhere higher up at the beginning of the process instead of it's current location where the error that would come back from a P4 install not finding the "p4" executable is currently that a checkout couldn't be found. That led me astray for quite some time.
On Aug 29, 3:42 pm, Christian Hammond <chip...@chipx86.com> wrote: > 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/hasapatch (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...@googlegr > > > > oups.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 athttp://www.reviewboard.org/users/ > > -~----------~----~----~----~------~----~------~--~--- > > To unsubscribe from this group, send email to > > reviewboard+unsubscr...@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 For more options, visit this group at http://groups.google.com/group/reviewboard?hl=en
