Page MenuHomePhabricator
Files F13195198

D19426.diff
No OneTemporary
Actions

D19426.diff
View Options

diff --git a/src/applications/differential/controller/DifferentialController.php b/src/applications/differential/controller/DifferentialController.php
--- a/src/applications/differential/controller/DifferentialController.php
+++ b/src/applications/differential/controller/DifferentialController.php
@@ -76,6 +76,16 @@
$repository_phid,
$changeset_path);
+ // If this particular changeset is generated code and the package does
+ // not match generated code, remove it from the list.
+ if ($changeset->isGeneratedChangeset()) {
+ foreach ($packages as $key => $package) {
+ if ($package->getMustMatchUngeneratedPaths()) {
+ unset($packages[$key]);
+ }
+ }
+ }
+
$this->pathPackageMap[$changeset_path] = $packages;
foreach ($packages as $package) {
$this->packageChangesetMap[$package->getPHID()][] = $changeset;
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
@@ -7,11 +7,13 @@
private $isCloseByCommit;
private $repositoryPHIDOverride = false;
private $didExpandInlineState = false;
- private $affectedPaths;
private $firstBroadcast = false;
private $wasBroadcasting;
private $isDraftDemotion;
+ private $ownersDiff;
+ private $ownersChangesets;
+
public function getEditorApplicationClass() {
return 'PhabricatorDifferentialApplication';
}
@@ -968,13 +970,20 @@
return array();
}
- if (!$this->affectedPaths) {
+ $diff = $this->ownersDiff;
+ $changesets = $this->ownersChangesets;
+
+ $this->ownersDiff = null;
+ $this->ownersChangesets = null;
+
+ if (!$changesets) {
return array();
}
- $packages = PhabricatorOwnersPackage::loadAffectedPackages(
+ $packages = PhabricatorOwnersPackage::loadAffectedPackagesForChangesets(
$repository,
- $this->affectedPaths);
+ $diff,
+ $changesets);
if (!$packages) {
return array();
}
@@ -1255,9 +1264,12 @@
$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;
+ // If this change affected paths, save the changesets so we can apply
+ // Owners rules to them later.
+ if ($paths) {
+ $this->ownersDiff = $diff;
+ $this->ownersChangesets = $changesets;
+ }
// Mark this as also touching all parent paths, so you can see all pending
// changes to any file within a directory.
diff --git a/src/applications/differential/herald/HeraldDifferentialRevisionAdapter.php b/src/applications/differential/herald/HeraldDifferentialRevisionAdapter.php
--- a/src/applications/differential/herald/HeraldDifferentialRevisionAdapter.php
+++ b/src/applications/differential/herald/HeraldDifferentialRevisionAdapter.php
@@ -120,9 +120,10 @@
$repository = $this->loadRepository();
if ($repository) {
- $packages = PhabricatorOwnersPackage::loadAffectedPackages(
+ $packages = PhabricatorOwnersPackage::loadAffectedPackagesForChangesets(
$repository,
- $this->loadAffectedPaths());
+ $this->getDiff(),
+ $this->loadChangesets());
$this->affectedPackages = $packages;
}
}
diff --git a/src/applications/differential/storage/DifferentialDiff.php b/src/applications/differential/storage/DifferentialDiff.php
--- a/src/applications/differential/storage/DifferentialDiff.php
+++ b/src/applications/differential/storage/DifferentialDiff.php
@@ -827,16 +827,18 @@
DifferentialChangeset $changeset) {
$is_generated_trusted = self::isTrustedGeneratedCode($changeset);
-
- $changeset->setTrustedChangesetAttribute(
- DifferentialChangeset::ATTRIBUTE_GENERATED,
- $is_generated_trusted);
+ if ($is_generated_trusted) {
+ $changeset->setTrustedChangesetAttribute(
+ DifferentialChangeset::ATTRIBUTE_GENERATED,
+ $is_generated_trusted);
+ }
$is_generated_untrusted = self::isUntrustedGeneratedCode($changeset);
-
- $changeset->setUntrustedChangesetAttribute(
- DifferentialChangeset::ATTRIBUTE_GENERATED,
- $is_generated_untrusted);
+ if ($is_generated_untrusted) {
+ $changeset->setUntrustedChangesetAttribute(
+ DifferentialChangeset::ATTRIBUTE_GENERATED,
+ $is_generated_untrusted);
+ }
}
private static function isTrustedGeneratedCode(
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
@@ -147,9 +147,9 @@
return ($this->getStatus() == self::STATUS_ARCHIVED);
}
- public function setName($name) {
- $this->name = $name;
- return $this;
+ public function getMustMatchUngeneratedPaths() {
+ // TODO: For now, there's no way to actually configure this.
+ return false;
}
public function loadOwners() {
@@ -181,6 +181,82 @@
return self::loadPackagesForPaths($repository, $paths);
}
+ public static function loadAffectedPackagesForChangesets(
+ PhabricatorRepository $repository,
+ DifferentialDiff $diff,
+ array $changesets) {
+ assert_instances_of($changesets, 'DifferentialChangeset');
+
+ $paths_all = array();
+ $paths_ungenerated = array();
+
+ foreach ($changesets as $changeset) {
+ $path = $changeset->getAbsoluteRepositoryPath($repository, $diff);
+
+ $paths_all[] = $path;
+
+ if (!$changeset->isGeneratedChangeset()) {
+ $paths_ungenerated[] = $path;
+ }
+ }
+
+ if (!$paths_all) {
+ return array();
+ }
+
+ $packages_all = self::loadAffectedPackages(
+ $repository,
+ $paths_all);
+
+ // If there are no generated changesets, we can't possibly need to throw
+ // away any packages for matching only generated paths. Just return the
+ // full set of packages.
+ if ($paths_ungenerated === $paths_all) {
+ return $packages_all;
+ }
+
+ $must_match_ungenerated = array();
+ foreach ($packages_all as $package) {
+ if ($package->getMustMatchUngeneratedPaths()) {
+ $must_match_ungenerated[] = $package;
+ }
+ }
+
+ // If no affected packages have the "Ignore Generated Paths" flag set, we
+ // can't possibly need to throw any away.
+ if (!$must_match_ungenerated) {
+ return $packages_all;
+ }
+
+ if ($paths_ungenerated) {
+ $packages_ungenerated = self::loadAffectedPackages(
+ $repository,
+ $paths_ungenerated);
+ } else {
+ $packages_ungenerated = array();
+ }
+
+ // We have some generated paths, and some packages that ignore generated
+ // paths. Take all the packages which:
+ //
+ // - ignore generated paths; and
+ // - didn't match any ungenerated paths
+ //
+ // ...and remove them from the list.
+
+ $must_match_ungenerated = mpull($must_match_ungenerated, null, 'getID');
+ $packages_ungenerated = mpull($packages_ungenerated, null, 'getID');
+ $packages_all = mpull($packages_all, null, 'getID');
+
+ foreach ($must_match_ungenerated as $package_id => $package) {
+ if (!isset($packages_ungenerated[$package_id])) {
+ unset($packages_all[$package_id]);
+ }
+ }
+
+ return $packages_all;
+ }
+
public static function loadOwningPackages($repository, $path) {
if (empty($path)) {
return array();

File Metadata

Mime Type
text/plain
Expires
Mon, May 13, 10:12 PM (2 w, 4 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6276366
Default Alt Text
D19426.diff (7 KB)

Event Timeline

Phabricator · Built by Phacility