@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.)
hq.recaptime.dev/wiki/Phorge
phorge
phabricator
For changesets that affect binaries, use the new binary file content hash as an effect hash
Summary: Ref T13522. When changesets update an image, we currently compute no effect hash. A content hash of the image (or other binary file) is a reasonable effect hash, and enalbes effect-hash-based behavior, including hiding files in intradiffs.
Test Plan:
- Created a revision affecting `cat.png` and `quack.txt` (currently, there must be 2+ changesets to trigger the hide logic).
- Updated it with the exact same changes.
- Viewed revision:
- Saw the image renderered in the interdiff.
- Applied patch.
- Ran `bin/differential rebuild-changesets ...`.
- Viewed revision:
- Saw both changesets collapse as unchanged.
Maniphest Tasks: T13522
Differential Revision: https://secure.phabricator.com/D21174
+103
-2
+66
src/applications/differential/engine/DifferentialChangesetEngine.php
+66
src/applications/differential/engine/DifferentialChangesetEngine.php
···
2
2
3
3
final class DifferentialChangesetEngine extends Phobject {
4
4
5
+
private $viewer;
6
+
7
+
public function setViewer(PhabricatorUser $viewer) {
8
+
$this->viewer = $viewer;
9
+
return $this;
10
+
}
11
+
12
+
public function getViewer() {
13
+
return $this->viewer;
14
+
}
15
+
5
16
public function rebuildChangesets(array $changesets) {
6
17
assert_instances_of($changesets, 'DifferentialChangeset');
18
+
19
+
$changesets = $this->loadChangesetFiles($changesets);
7
20
8
21
foreach ($changesets as $changeset) {
9
22
$this->detectGeneratedCode($changeset);
···
13
26
$this->detectCopiedCode($changesets);
14
27
}
15
28
29
+
private function loadChangesetFiles(array $changesets) {
30
+
$viewer = $this->getViewer();
31
+
32
+
$file_phids = array();
33
+
foreach ($changesets as $changeset) {
34
+
$file_phid = $changeset->getNewFileObjectPHID();
35
+
if ($file_phid !== null) {
36
+
$file_phids[] = $file_phid;
37
+
}
38
+
}
39
+
40
+
if ($file_phids) {
41
+
$files = id(new PhabricatorFileQuery())
42
+
->setViewer($viewer)
43
+
->withPHIDs($file_phids)
44
+
->execute();
45
+
$files = mpull($files, null, 'getPHID');
46
+
} else {
47
+
$files = array();
48
+
}
49
+
50
+
foreach ($changesets as $changeset_key => $changeset) {
51
+
$file_phid = $changeset->getNewFileObjectPHID();
52
+
if ($file_phid === null) {
53
+
continue;
54
+
}
55
+
56
+
$file = idx($files, $file_phid);
57
+
if (!$file) {
58
+
unset($changesets[$changeset_key]);
59
+
continue;
60
+
}
61
+
62
+
$changeset->attachNewFileObject($file);
63
+
}
64
+
65
+
return $changesets;
66
+
}
67
+
16
68
17
69
/* -( Generated Code )----------------------------------------------------- */
18
70
···
84
136
if ($changeset->getHunks()) {
85
137
$new_data = $changeset->makeNewFile();
86
138
return PhabricatorHash::digestForIndex($new_data);
139
+
}
140
+
141
+
if ($changeset->getNewFileObjectPHID()) {
142
+
$file = $changeset->getNewFileObject();
143
+
144
+
// See T13522. For now, the "contentHash" is not really a content hash
145
+
// for files >4MB. This is okay: we will just always detect them as
146
+
// changed, which is the safer behavior.
147
+
148
+
$hash = $file->getContentHash();
149
+
if ($hash !== null) {
150
+
$hash = sprintf('file-hash:%s', $hash);
151
+
return PhabricatorHash::digestForIndex($hash);
152
+
}
87
153
}
88
154
89
155
return null;
+1
src/applications/differential/management/PhabricatorDifferentialRebuildChangesetsWorkflow.php
+1
src/applications/differential/management/PhabricatorDifferentialRebuildChangesetsWorkflow.php
-2
src/applications/differential/parser/DifferentialChangesetParser.php
-2
src/applications/differential/parser/DifferentialChangesetParser.php
+31
src/applications/differential/storage/DifferentialChangeset.php
+31
src/applications/differential/storage/DifferentialChangeset.php
···
25
25
private $authorityPackages;
26
26
private $changesetPackages;
27
27
28
+
private $newFileObject = self::ATTACHABLE;
29
+
private $oldFileObject = self::ATTACHABLE;
30
+
28
31
const TABLE_CACHE = 'differential_changeset_parse_cache';
29
32
30
33
const METADATA_TRUSTED_ATTRIBUTES = 'attributes.trusted';
···
457
460
458
461
public function isGeneratedChangeset() {
459
462
return $this->getChangesetAttribute(self::ATTRIBUTE_GENERATED);
463
+
}
464
+
465
+
public function getNewFileObjectPHID() {
466
+
$metadata = $this->getMetadata();
467
+
return idx($metadata, 'new:binary-phid');
468
+
}
469
+
470
+
public function getOldFileObjectPHID() {
471
+
$metadata = $this->getMetadata();
472
+
return idx($metadata, 'old:binary-phid');
473
+
}
474
+
475
+
public function attachNewFileObject(PhabricatorFile $file) {
476
+
$this->newFileObject = $file;
477
+
return $this;
478
+
}
479
+
480
+
public function getNewFileObject() {
481
+
return $this->assertAttached($this->newFileObject);
482
+
}
483
+
484
+
public function attachOldFileObject(PhabricatorFile $file) {
485
+
$this->oldFileObject = $file;
486
+
return $this;
487
+
}
488
+
489
+
public function getOldFileObject() {
490
+
return $this->assertAttached($this->oldFileObject);
460
491
}
461
492
462
493
+5
src/applications/differential/storage/DifferentialDiff.php
+5
src/applications/differential/storage/DifferentialDiff.php
···
232
232
233
233
$changesets = $diff->getChangesets();
234
234
235
+
// TODO: This is "safe", but it would be better to propagate a real user
236
+
// down the stack.
237
+
$viewer = PhabricatorUser::getOmnipotentUser();
238
+
235
239
id(new DifferentialChangesetEngine())
240
+
->setViewer($viewer)
236
241
->rebuildChangesets($changesets);
237
242
238
243
return $diff;