Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F14849333
D15936.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Size
12 KB
Referenced Files
None
Subscribers
None
D15936.diff
View Options
diff --git a/resources/sql/autopatches/20160516.owners.01.dominion.sql b/resources/sql/autopatches/20160516.owners.01.dominion.sql
new file mode 100644
--- /dev/null
+++ b/resources/sql/autopatches/20160516.owners.01.dominion.sql
@@ -0,0 +1,2 @@
+ALTER TABLE {$NAMESPACE}_owners.owners_package
+ ADD dominion VARCHAR(32) NOT NULL COLLATE {$COLLATE_TEXT};
diff --git a/resources/sql/autopatches/20160516.owners.02.dominionstrong.sql b/resources/sql/autopatches/20160516.owners.02.dominionstrong.sql
new file mode 100644
--- /dev/null
+++ b/resources/sql/autopatches/20160516.owners.02.dominionstrong.sql
@@ -0,0 +1,2 @@
+UPDATE {$NAMESPACE}_owners.owners_package
+ SET dominion = 'strong' WHERE dominion = '';
diff --git a/src/applications/differential/editor/DifferentialTransactionEditor.php b/src/applications/differential/editor/DifferentialTransactionEditor.php
--- a/src/applications/differential/editor/DifferentialTransactionEditor.php
+++ b/src/applications/differential/editor/DifferentialTransactionEditor.php
@@ -1723,6 +1723,10 @@
$paths[] = $path_prefix.'/'.$changeset->getFilename();
}
+ // Save the affected paths; we'll use them later to query Owners. This
+ // uses the un-expanded paths.
+ $this->affectedPaths = $paths;
+
// Mark this as also touching all parent paths, so you can see all pending
// changes to any file within a directory.
$all_paths = array();
@@ -1733,9 +1737,6 @@
}
$all_paths = array_keys($all_paths);
- // Save the affected paths; we'll use them later to query Owners.
- $this->affectedPaths = $all_paths;
-
$path_ids =
PhabricatorRepositoryCommitChangeParserWorker::lookupOrCreatePaths(
$all_paths);
diff --git a/src/applications/owners/controller/PhabricatorOwnersDetailController.php b/src/applications/owners/controller/PhabricatorOwnersDetailController.php
--- a/src/applications/owners/controller/PhabricatorOwnersDetailController.php
+++ b/src/applications/owners/controller/PhabricatorOwnersDetailController.php
@@ -184,6 +184,13 @@
}
$view->addProperty(pht('Owners'), $owner_list);
+
+ $dominion = $package->getDominion();
+ $dominion_map = PhabricatorOwnersPackage::getDominionOptionsMap();
+ $spec = idx($dominion_map, $dominion, array());
+ $name = idx($spec, 'short', $dominion);
+ $view->addProperty(pht('Dominion'), $name);
+
$auto = $package->getAutoReview();
$autoreview_map = PhabricatorOwnersPackage::getAutoreviewOptionsMap();
$spec = idx($autoreview_map, $auto, array());
diff --git a/src/applications/owners/editor/PhabricatorOwnersPackageEditEngine.php b/src/applications/owners/editor/PhabricatorOwnersPackageEditEngine.php
--- a/src/applications/owners/editor/PhabricatorOwnersPackageEditEngine.php
+++ b/src/applications/owners/editor/PhabricatorOwnersPackageEditEngine.php
@@ -87,6 +87,9 @@
$autoreview_map = PhabricatorOwnersPackage::getAutoreviewOptionsMap();
$autoreview_map = ipull($autoreview_map, 'name');
+ $dominion_map = PhabricatorOwnersPackage::getDominionOptionsMap();
+ $dominion_map = ipull($dominion_map, 'name');
+
return array(
id(new PhabricatorTextEditField())
->setKey('name')
@@ -104,6 +107,16 @@
->setIsCopyable(true)
->setValue($object->getOwnerPHIDs()),
id(new PhabricatorSelectEditField())
+ ->setKey('dominion')
+ ->setLabel(pht('Dominion'))
+ ->setDescription(
+ pht('Change package dominion rules.'))
+ ->setTransactionType(
+ PhabricatorOwnersPackageTransaction::TYPE_DOMINION)
+ ->setIsCopyable(true)
+ ->setValue($object->getDominion())
+ ->setOptions($dominion_map),
+ id(new PhabricatorSelectEditField())
->setKey('autoReview')
->setLabel(pht('Auto Review'))
->setDescription(
diff --git a/src/applications/owners/editor/PhabricatorOwnersPackageTransactionEditor.php b/src/applications/owners/editor/PhabricatorOwnersPackageTransactionEditor.php
--- a/src/applications/owners/editor/PhabricatorOwnersPackageTransactionEditor.php
+++ b/src/applications/owners/editor/PhabricatorOwnersPackageTransactionEditor.php
@@ -21,6 +21,7 @@
$types[] = PhabricatorOwnersPackageTransaction::TYPE_PATHS;
$types[] = PhabricatorOwnersPackageTransaction::TYPE_STATUS;
$types[] = PhabricatorOwnersPackageTransaction::TYPE_AUTOREVIEW;
+ $types[] = PhabricatorOwnersPackageTransaction::TYPE_DOMINION;
$types[] = PhabricatorTransactions::TYPE_VIEW_POLICY;
$types[] = PhabricatorTransactions::TYPE_EDIT_POLICY;
@@ -50,6 +51,8 @@
return $object->getStatus();
case PhabricatorOwnersPackageTransaction::TYPE_AUTOREVIEW:
return $object->getAutoReview();
+ case PhabricatorOwnersPackageTransaction::TYPE_DOMINION:
+ return $object->getDominion();
}
}
@@ -62,6 +65,7 @@
case PhabricatorOwnersPackageTransaction::TYPE_DESCRIPTION:
case PhabricatorOwnersPackageTransaction::TYPE_STATUS:
case PhabricatorOwnersPackageTransaction::TYPE_AUTOREVIEW:
+ case PhabricatorOwnersPackageTransaction::TYPE_DOMINION:
return $xaction->getNewValue();
case PhabricatorOwnersPackageTransaction::TYPE_PATHS:
$new = $xaction->getNewValue();
@@ -120,6 +124,9 @@
case PhabricatorOwnersPackageTransaction::TYPE_AUTOREVIEW:
$object->setAutoReview($xaction->getNewValue());
return;
+ case PhabricatorOwnersPackageTransaction::TYPE_DOMINION:
+ $object->setDominion($xaction->getNewValue());
+ return;
}
return parent::applyCustomInternalTransaction($object, $xaction);
@@ -135,6 +142,7 @@
case PhabricatorOwnersPackageTransaction::TYPE_AUDITING:
case PhabricatorOwnersPackageTransaction::TYPE_STATUS:
case PhabricatorOwnersPackageTransaction::TYPE_AUTOREVIEW:
+ case PhabricatorOwnersPackageTransaction::TYPE_DOMINION:
return;
case PhabricatorOwnersPackageTransaction::TYPE_OWNERS:
$old = $xaction->getOldValue();
@@ -249,6 +257,26 @@
}
}
break;
+ case PhabricatorOwnersPackageTransaction::TYPE_DOMINION:
+ $map = PhabricatorOwnersPackage::getDominionOptionsMap();
+ foreach ($xactions as $xaction) {
+ $new = $xaction->getNewValue();
+
+ if (empty($map[$new])) {
+ $valid = array_keys($map);
+
+ $errors[] = new PhabricatorApplicationTransactionValidationError(
+ $type,
+ pht('Invalid'),
+ pht(
+ 'Dominion setting "%s" is not valid. '.
+ 'Valid settings are: %s.',
+ $new,
+ implode(', ', $valid)),
+ $xaction);
+ }
+ }
+ break;
case PhabricatorOwnersPackageTransaction::TYPE_PATHS:
if (!$xactions) {
continue;
diff --git a/src/applications/owners/query/PhabricatorOwnersPackageQuery.php b/src/applications/owners/query/PhabricatorOwnersPackageQuery.php
--- a/src/applications/owners/query/PhabricatorOwnersPackageQuery.php
+++ b/src/applications/owners/query/PhabricatorOwnersPackageQuery.php
@@ -351,6 +351,7 @@
}
$packages = $this->controlResults;
+ $weak_dominion = PhabricatorOwnersPackage::DOMINION_WEAK;
$matches = array();
foreach ($packages as $package_id => $package) {
@@ -373,6 +374,7 @@
if ($best_match && $include) {
$matches[$package_id] = array(
'strength' => $best_match,
+ 'weak' => ($package->getDominion() == $weak_dominion),
'package' => $package,
);
}
@@ -381,6 +383,18 @@
$matches = isort($matches, 'strength');
$matches = array_reverse($matches);
+ $first_id = null;
+ foreach ($matches as $package_id => $match) {
+ if ($first_id === null) {
+ $first_id = $package_id;
+ continue;
+ }
+
+ if ($match['weak']) {
+ unset($matches[$package_id]);
+ }
+ }
+
return array_values(ipull($matches, 'package'));
}
diff --git a/src/applications/owners/storage/PhabricatorOwnersPackage.php b/src/applications/owners/storage/PhabricatorOwnersPackage.php
--- a/src/applications/owners/storage/PhabricatorOwnersPackage.php
+++ b/src/applications/owners/storage/PhabricatorOwnersPackage.php
@@ -21,6 +21,7 @@
protected $status;
protected $viewPolicy;
protected $editPolicy;
+ protected $dominion;
private $paths = self::ATTACHABLE;
private $owners = self::ATTACHABLE;
@@ -51,6 +52,7 @@
return id(new PhabricatorOwnersPackage())
->setAuditingEnabled(0)
->setAutoReview(self::AUTOREVIEW_NONE)
+ ->setDominion(self::DOMINION_STRONG)
->setViewPolicy($view_policy)
->setEditPolicy($edit_policy)
->attachPaths(array())
@@ -83,6 +85,19 @@
);
}
+ public static function getDominionOptionsMap() {
+ return array(
+ self::DOMINION_STRONG => array(
+ 'name' => pht('Strong (Control All Paths)'),
+ 'short' => pht('Strong'),
+ ),
+ self::DOMINION_WEAK => array(
+ 'name' => pht('Weak (Control Unowned Paths)'),
+ 'short' => pht('Weak'),
+ ),
+ );
+ }
+
protected function getConfiguration() {
return array(
// This information is better available from the history table.
@@ -97,6 +112,7 @@
'mailKey' => 'bytes20',
'status' => 'text32',
'autoReview' => 'text32',
+ 'dominion' => 'text32',
),
) + parent::getConfiguration();
}
@@ -193,7 +209,7 @@
foreach (array_chunk(array_keys($fragments), 128) as $chunk) {
$rows[] = queryfx_all(
$conn,
- 'SELECT pkg.id, "strong" dominion, p.excluded, p.path
+ 'SELECT pkg.id, pkg.dominion, p.excluded, p.path
FROM %T pkg JOIN %T p ON p.packageID = pkg.id
WHERE p.path IN (%Ls) %Q',
$package->getTableName(),
diff --git a/src/applications/owners/storage/PhabricatorOwnersPackageTransaction.php b/src/applications/owners/storage/PhabricatorOwnersPackageTransaction.php
--- a/src/applications/owners/storage/PhabricatorOwnersPackageTransaction.php
+++ b/src/applications/owners/storage/PhabricatorOwnersPackageTransaction.php
@@ -11,6 +11,7 @@
const TYPE_PATHS = 'owners.paths';
const TYPE_STATUS = 'owners.status';
const TYPE_AUTOREVIEW = 'owners.autoreview';
+ const TYPE_DOMINION = 'owners.dominion';
public function getApplicationName() {
return 'owners';
@@ -156,6 +157,18 @@
$this->renderHandleLink($author_phid),
$old,
$new);
+ case self::TYPE_DOMINION:
+ $map = PhabricatorOwnersPackage::getDominionOptionsMap();
+ $map = ipull($map, 'short');
+
+ $old = idx($map, $old, $old);
+ $new = idx($map, $new, $new);
+
+ return pht(
+ '%s adjusted package dominion rules from "%s" to "%s".',
+ $this->renderHandleLink($author_phid),
+ $old,
+ $new);
}
return parent::getTitle();
diff --git a/src/docs/user/userguide/owners.diviner b/src/docs/user/userguide/owners.diviner
--- a/src/docs/user/userguide/owners.diviner
+++ b/src/docs/user/userguide/owners.diviner
@@ -45,6 +45,38 @@
which affect them in Diffusion or Differential.
+Dominion
+========
+
+The **Dominion** option allows you to control how ownership cascades when
+multiple packages own a path. The dominion rules are:
+
+**Strong Dominion.** This is the default. In this mode, the package will always
+own all files matching its configured paths, even if another package also owns
+them.
+
+For example, if the package owns `a/`, it will always own `a/b/c.z` even if
+another package owns `a/b/`. In this case, both packages will own `a/b/c.z`.
+
+This mode prevents users from stealing files away from the package by defining
+more narrow ownership rules in new packages, but enforces hierarchical
+ownership rules.
+
+**Weak Dominion.** In this mode, the package will only own files which do not
+match a more specific path in another package.
+
+For example, if the package owns `a/` but another package owns `a/b/`, the
+package will no longer consider `a/b/c.z` to be a file it owns because another
+package matches the path with a more specific rule.
+
+This mode lets you to define rules without implicit hierarchical ownership,
+but allows users to steal files away from a package by defining a more
+specific package.
+
+For more details on files which match multiple packages, see
+"Files in Multiple Packages", below.
+
+
Auto Review
===========
@@ -93,4 +125,6 @@
"Android Application", and "Design Assets".
(You can use an "exclude" rule if you want to make a different package with a
-more specific claim the owner of a file or subdirectory.)
+more specific claim the owner of a file or subdirectory. You can also change
+the **Dominion** setting for a package to let it give up ownership of paths
+owned by another package.)
File Metadata
Details
Attached
Mime Type
text/plain
Expires
Wed, Feb 5, 1:47 AM (20 h, 49 m)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7091058
Default Alt Text
D15936.diff (12 KB)
Attached To
Mode
D15936: Allow users to manage package dominion rules
Attached
Detach File
Event Timeline
Log In to Comment