Welcome to the Splat alpha!

We can use an OrderedDict, but it's not a guaranteed fix, as we can't expect a certain order from Python Entrypoints.

The recommendation, which will also speed up posting, is to place the following in .reviewboardrc:

    REPOSITORY_TYPE = 'git'

Or, on the command line:

    $ rbt post --repository-type=git
    $ post-review --repository-type=git

Since the issue is git repos with a global .p4config, the best option is to use the .reviewboardrc method. It'll prevent scanning, meaning Perforce won't even be tried.

By defining a repository type, you can easily shave a second or more off posting (since we don't have to execute a bunch of tools, and they don't have to do whatever lookups they do).

What you suggest might work for a small group, but doesn't really scale for a company with 13K+ employees. 

You are suggesting to ask each and every user to put REPOSITORY_TYPE = 'git' in .reviewboardrc file. This is almost certainly going to get push back by users. Additionally, what about users who also develop in other SCM types? putting this configuration in home directory will not work, isn't it?

I'm talking about the repositories, not for individual users. A Git repository using Review Board needs a .reviewboardrc file to link it up (unless your wrapper script is doing something specialized, in which case it can pass that parameter). It's not much different than needing to configure the Review Board URL and repository path.

The problem boils down to how Perforce handles configuration. It needs to be overridden somewhere. We can't hard-code the order, as that's just a bandaid around the real problem. As we've seen from other enterprise-sized companies, there's other combinations of repositories being used that cause the same sorts of problems (Subversion repos embedded inside Git, for instance, or Git inside of Subversion). There's no order we can pick that solves every situation. The only real way to solve this is the option we provide, which is what other companies of this size use.

OK, let me confirm if I understand your suggestion. You meant when we provision a git repo, we should drop in a .reviewboardrc file in it?

Yes, we recommend putting a .reviewboardrc in the root of each repository.

There are definitely improvements we can make to the SCM detection, and we've had a handful of similar bugs with different combinations of repositories (2785, 2801). Just using an OrderedDict may fix the issue for some but cause conflicts for others. I think ideally we want to:

- Make the perforce detection fancier than just seeing if there's a valid P4PORT and have it actually look to see if you're in a client directory.
- Make the other SCM detection use a breadth-first search up the file hierarchy from the current working directory, instead of having each tool do its own depth-first search.

Sounds good to me. I haven't read the code to find this out, but I want to confirm this following case with you:

If user has a .reviewboardrc in his/her home directory which contains some global post-review configurations in that file, say "SOME_CONFIG = A", then he/she clones a repo which contains a .reviewboardrc with content "REPOSITORY_TYPE = git". Will post-review pass both configs, i.e. 

SOME_CONFIG = A
REPOSITORY_TYPE = git

to postreview options?

Thanks
/Eric

Yep, it'll take both into account. Any user configuration will also override the repository configuration.

Thanks for this confirmation! For now, I'd just use OrderedDict() and stripping off the unused SCM types in our installation.

I hope you guys can get what you proposed in update#5 in future releases. Frankly, I think putting a .reviewboardrc in a bare Git repo is intrusive because that file has nothing to do with either the git project itself or the actual global git configuration for that repo.

How are you linking up repositories today? The .reviewboardrc method is what every company we know is using. When I was working there, that was the method we saw on the Git repositories we were working with that used Review Board.

I'm currently also one of the two guys working on the Git infrastructure. I don't think we drop this file in any of the bare repos on git server.

There is no linking to repos setup per se. Here is the workflow today: 

1. User clones a repo
2. Makes his changes, commit.
3. post-review that commit. with -d turned on, one can see postreview.py will try each supported SCM client in order. 

repository_info = tool.get_repository_info()

if repository_info is not None, then that client tool is returned.
 

Fix up for review at https://reviews.reviewboard.org/r/5271/

This will go into 0.5.6.

• Status:

  +	PendingReview

• Tags:

  +	Milestone-RBTools-Release0.5
  +	Component-RBTools

• Assigned to:

  +	chipx86

Fixed for 0.5.6 on release-0.5.x (4c39641)

• Status:

  -	PendingReview
  +	Fixed