HomePhabricator

Fix several safety issues with repository URIs

Description

Fix several safety issues with repository URIs

Summary:
See PHI234. Several issues here:

  • The warning about observing a repository in Read/Write mode checks the raw I/O type, not the effective I/O type. That means we can fail to warn if other URIs are set to "Default", and "Default" is "Read/Write" in practice.
  • There's just an actual typo which prevents the "Observe" version of this error from triggering properly.

Additionally, add more forceful warnings that "Observe" and "Mirror" mean that you want to replace a repository with another one, not that we somehow merge branches selectively. It isn't necessarily obvious that "Observe" doesn't mean "merge/union", since the reasons it can't in the general case are somewhat subtle (conflicts between refs with the same names, detecting ref deletion).

Test Plan:
Read documentation. Hit the error locally by trying to "Observe" while in Read/Write mode:

Screen Shot 2017-11-30 at 12.09.30 PM.png (988×1 px, 160 KB)

Reviewers: amckinley

Reviewed By: amckinley

Differential Revision: https://secure.phabricator.com/D18810

Details

Provenance
epriestleyAuthored on Nov 30 2017, 8:13 PM
epriestleyPushed on Nov 30 2017, 10:06 PM
Reviewer
amckinley
Differential Revision
D18810: Fix several safety issues with repository URIs
Parents
rPf786c86a6a42: Don't require the "gd" extension be installed in order to run unit tests
Branches
Unknown
Tags
Unknown
Build Status
Buildable 18910
Build 25500: Run Core Tests

Event Timeline

This makes it rather annoying to create a new observed repository, as you first need to set all 6 (if both https and ssh access are configured) default URIs to read only before adding the URI to Observe.

Previously the default Read/Write statuses would automatically change to Read-only after the URI to Observe was added, so for the common case of setting up a new repository that has no commits and has not been enabled yet I'm not sure what bug this is fixing.