@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
Make "bin/files" parsing of working set arguments more consistent
Summary:
Fixes T13326. In D20571, I slightly generalized construction of an iterator over a set of files, but missed some code in other "bin/files ..." commands which was also affected.
Today, basically all of these workflows define their own "--all" and "names" flags. Pull these definitions up and implement them more consistently.
Test Plan: Ran multiple different `bin/files` commands with different combinations of arguments, saw consistent handling of iterator construction.
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T13326
Differential Revision: https://secure.phabricator.com/D20614
+166
-222
+7
-23
src/applications/files/management/PhabricatorFilesManagementCompactWorkflow.php
+7
-23
src/applications/files/management/PhabricatorFilesManagementCompactWorkflow.php
···
4
4
extends PhabricatorFilesManagementWorkflow {
5
5
6
6
protected function didConstruct() {
7
+
$arguments = $this->newIteratorArguments();
8
+
$arguments[] = array(
9
+
'name' => 'dry-run',
10
+
'help' => pht('Show what would be compacted.'),
11
+
);
12
+
7
13
$this
8
14
->setName('compact')
9
15
->setSynopsis(
10
16
pht(
11
17
'Merge identical files to share the same storage. In some cases, '.
12
18
'this can repair files with missing data.'))
13
-
->setArguments(
14
-
array(
15
-
array(
16
-
'name' => 'dry-run',
17
-
'help' => pht('Show what would be compacted.'),
18
-
),
19
-
array(
20
-
'name' => 'all',
21
-
'help' => pht('Compact all files.'),
22
-
),
23
-
array(
24
-
'name' => 'names',
25
-
'wildcard' => true,
26
-
),
27
-
));
19
+
->setArguments($arguments);
28
20
}
29
21
30
22
public function execute(PhutilArgumentParser $args) {
31
23
$console = PhutilConsole::getConsole();
32
24
33
25
$iterator = $this->buildIterator($args);
34
-
if (!$iterator) {
35
-
throw new PhutilArgumentUsageException(
36
-
pht(
37
-
'Either specify a list of files to compact, or use `%s` '.
38
-
'to compact all files.',
39
-
'--all'));
40
-
}
41
-
42
26
$is_dry_run = $args->getArg('dry-run');
43
27
44
28
foreach ($iterator as $file) {
+8
-22
src/applications/files/management/PhabricatorFilesManagementCycleWorkflow.php
+8
-22
src/applications/files/management/PhabricatorFilesManagementCycleWorkflow.php
···
4
4
extends PhabricatorFilesManagementWorkflow {
5
5
6
6
protected function didConstruct() {
7
+
$arguments = $this->newIteratorArguments();
8
+
$arguments[] = array(
9
+
'name' => 'key',
10
+
'param' => 'keyname',
11
+
'help' => pht('Select a specific storage key to cycle to.'),
12
+
);
13
+
7
14
$this
8
15
->setName('cycle')
9
16
->setSynopsis(
10
17
pht('Cycle master key for encrypted files.'))
11
-
->setArguments(
12
-
array(
13
-
array(
14
-
'name' => 'key',
15
-
'param' => 'keyname',
16
-
'help' => pht('Select a specific storage key to cycle to.'),
17
-
),
18
-
array(
19
-
'name' => 'all',
20
-
'help' => pht('Change encoding for all files.'),
21
-
),
22
-
array(
23
-
'name' => 'names',
24
-
'wildcard' => true,
25
-
),
26
-
));
18
+
->setArguments($arguments);
27
19
}
28
20
29
21
public function execute(PhutilArgumentParser $args) {
30
22
$iterator = $this->buildIterator($args);
31
-
if (!$iterator) {
32
-
throw new PhutilArgumentUsageException(
33
-
pht(
34
-
'Either specify a list of files to cycle, or use --all to cycle '.
35
-
'all files.'));
36
-
}
37
23
38
24
$format_map = PhabricatorFileStorageFormat::getAllFormats();
39
25
$engines = PhabricatorFileStorageEngine::loadAllEngines();
+22
-33
src/applications/files/management/PhabricatorFilesManagementEncodeWorkflow.php
+22
-33
src/applications/files/management/PhabricatorFilesManagementEncodeWorkflow.php
···
4
4
extends PhabricatorFilesManagementWorkflow {
5
5
6
6
protected function didConstruct() {
7
+
$arguments = $this->newIteratorArguments();
8
+
9
+
$arguments[] = array(
10
+
'name' => 'as',
11
+
'param' => 'format',
12
+
'help' => pht('Select the storage format to use.'),
13
+
);
14
+
15
+
$arguments[] = array(
16
+
'name' => 'key',
17
+
'param' => 'keyname',
18
+
'help' => pht('Select a specific storage key.'),
19
+
);
20
+
21
+
$arguments[] = array(
22
+
'name' => 'force',
23
+
'help' => pht(
24
+
'Re-encode files which are already stored in the target '.
25
+
'encoding.'),
26
+
);
27
+
7
28
$this
8
29
->setName('encode')
9
30
->setSynopsis(
10
31
pht('Change the storage encoding of files.'))
11
-
->setArguments(
12
-
array(
13
-
array(
14
-
'name' => 'as',
15
-
'param' => 'format',
16
-
'help' => pht('Select the storage format to use.'),
17
-
),
18
-
array(
19
-
'name' => 'key',
20
-
'param' => 'keyname',
21
-
'help' => pht('Select a specific storage key.'),
22
-
),
23
-
array(
24
-
'name' => 'all',
25
-
'help' => pht('Change encoding for all files.'),
26
-
),
27
-
array(
28
-
'name' => 'force',
29
-
'help' => pht(
30
-
'Re-encode files which are already stored in the target '.
31
-
'encoding.'),
32
-
),
33
-
array(
34
-
'name' => 'names',
35
-
'wildcard' => true,
36
-
),
37
-
));
32
+
->setArguments($arguments);
38
33
}
39
34
40
35
public function execute(PhutilArgumentParser $args) {
41
36
$iterator = $this->buildIterator($args);
42
-
if (!$iterator) {
43
-
throw new PhutilArgumentUsageException(
44
-
pht(
45
-
'Either specify a list of files to encode, or use --all to '.
46
-
'encode all files.'));
47
-
}
48
37
49
38
$force = (bool)$args->getArg('force');
50
39
+41
-49
src/applications/files/management/PhabricatorFilesManagementIntegrityWorkflow.php
+41
-49
src/applications/files/management/PhabricatorFilesManagementIntegrityWorkflow.php
···
4
4
extends PhabricatorFilesManagementWorkflow {
5
5
6
6
protected function didConstruct() {
7
+
$arguments = $this->newIteratorArguments();
8
+
9
+
$arguments[] = array(
10
+
'name' => 'strip',
11
+
'help' => pht(
12
+
'DANGEROUS. Strip integrity hashes from files. This makes '.
13
+
'files vulnerable to corruption or tampering.'),
14
+
);
15
+
16
+
$arguments[] = array(
17
+
'name' => 'corrupt',
18
+
'help' => pht(
19
+
'Corrupt integrity hashes for given files. This is intended '.
20
+
'for debugging.'),
21
+
);
22
+
23
+
$arguments[] = array(
24
+
'name' => 'compute',
25
+
'help' => pht(
26
+
'Compute and update integrity hashes for files which do not '.
27
+
'yet have them.'),
28
+
);
29
+
30
+
$arguments[] = array(
31
+
'name' => 'overwrite',
32
+
'help' => pht(
33
+
'DANGEROUS. Recompute and update integrity hashes, overwriting '.
34
+
'invalid hashes. This may mark corrupt or dangerous files as '.
35
+
'valid.'),
36
+
);
37
+
38
+
$arguments[] = array(
39
+
'name' => 'force',
40
+
'short' => 'f',
41
+
'help' => pht(
42
+
'Execute dangerous operations without prompting for '.
43
+
'confirmation.'),
44
+
);
45
+
46
+
7
47
$this
8
48
->setName('integrity')
9
49
->setSynopsis(pht('Verify or recalculate file integrity hashes.'))
10
-
->setArguments(
11
-
array(
12
-
array(
13
-
'name' => 'all',
14
-
'help' => pht('Affect all files.'),
15
-
),
16
-
array(
17
-
'name' => 'strip',
18
-
'help' => pht(
19
-
'DANGEROUS. Strip integrity hashes from files. This makes '.
20
-
'files vulnerable to corruption or tampering.'),
21
-
),
22
-
array(
23
-
'name' => 'corrupt',
24
-
'help' => pht(
25
-
'Corrupt integrity hashes for given files. This is intended '.
26
-
'for debugging.'),
27
-
),
28
-
array(
29
-
'name' => 'compute',
30
-
'help' => pht(
31
-
'Compute and update integrity hashes for files which do not '.
32
-
'yet have them.'),
33
-
),
34
-
array(
35
-
'name' => 'overwrite',
36
-
'help' => pht(
37
-
'DANGEROUS. Recompute and update integrity hashes, overwriting '.
38
-
'invalid hashes. This may mark corrupt or dangerous files as '.
39
-
'valid.'),
40
-
),
41
-
array(
42
-
'name' => 'force',
43
-
'short' => 'f',
44
-
'help' => pht(
45
-
'Execute dangerous operations without prompting for '.
46
-
'confirmation.'),
47
-
),
48
-
array(
49
-
'name' => 'names',
50
-
'wildcard' => true,
51
-
),
52
-
));
50
+
->setArguments($arguments);
53
51
}
54
52
55
53
public function execute(PhutilArgumentParser $args) {
···
120
118
}
121
119
122
120
$iterator = $this->buildIterator($args);
123
-
if (!$iterator) {
124
-
throw new PhutilArgumentUsageException(
125
-
pht(
126
-
'Either specify a list of files to affect, or use "--all" to '.
127
-
'affect all files.'));
128
-
}
129
121
130
122
$failure_count = 0;
131
123
$total_count = 0;
+45
-60
src/applications/files/management/PhabricatorFilesManagementMigrateWorkflow.php
+45
-60
src/applications/files/management/PhabricatorFilesManagementMigrateWorkflow.php
···
4
4
extends PhabricatorFilesManagementWorkflow {
5
5
6
6
protected function didConstruct() {
7
+
$arguments = $this->newIteratorArguments();
8
+
9
+
$arguments[] = array(
10
+
'name' => 'engine',
11
+
'param' => 'storage-engine',
12
+
'help' => pht('Migrate to the named storage engine.'),
13
+
);
14
+
15
+
$arguments[] = array(
16
+
'name' => 'dry-run',
17
+
'help' => pht('Show what would be migrated.'),
18
+
);
19
+
20
+
$arguments[] = array(
21
+
'name' => 'min-size',
22
+
'param' => 'bytes',
23
+
'help' => pht(
24
+
'Do not migrate data for files which are smaller than a given '.
25
+
'filesize.'),
26
+
);
27
+
28
+
$arguments[] = array(
29
+
'name' => 'max-size',
30
+
'param' => 'bytes',
31
+
'help' => pht(
32
+
'Do not migrate data for files which are larger than a given '.
33
+
'filesize.'),
34
+
);
35
+
36
+
$arguments[] = array(
37
+
'name' => 'copy',
38
+
'help' => pht(
39
+
'Copy file data instead of moving it: after migrating, do not '.
40
+
'remove the old data even if it is no longer referenced.'),
41
+
);
42
+
43
+
$arguments[] = array(
44
+
'name' => 'local-disk-source',
45
+
'param' => 'path',
46
+
'help' => pht(
47
+
'When migrating from a local disk source, use the specified '.
48
+
'path as the root directory.'),
49
+
);
50
+
7
51
$this
8
52
->setName('migrate')
9
53
->setSynopsis(pht('Migrate files between storage engines.'))
10
-
->setArguments(
11
-
array(
12
-
array(
13
-
'name' => 'engine',
14
-
'param' => 'storage_engine',
15
-
'help' => pht('Migrate to the named storage engine.'),
16
-
),
17
-
array(
18
-
'name' => 'dry-run',
19
-
'help' => pht('Show what would be migrated.'),
20
-
),
21
-
array(
22
-
'name' => 'min-size',
23
-
'param' => 'bytes',
24
-
'help' => pht(
25
-
'Do not migrate data for files which are smaller than a given '.
26
-
'filesize.'),
27
-
),
28
-
array(
29
-
'name' => 'max-size',
30
-
'param' => 'bytes',
31
-
'help' => pht(
32
-
'Do not migrate data for files which are larger than a given '.
33
-
'filesize.'),
34
-
),
35
-
array(
36
-
'name' => 'all',
37
-
'help' => pht('Migrate all files.'),
38
-
),
39
-
array(
40
-
'name' => 'copy',
41
-
'help' => pht(
42
-
'Copy file data instead of moving it: after migrating, do not '.
43
-
'remove the old data even if it is no longer referenced.'),
44
-
),
45
-
array(
46
-
'name' => 'names',
47
-
'wildcard' => true,
48
-
),
49
-
array(
50
-
'name' => 'from-engine',
51
-
'param' => 'engine',
52
-
'help' => pht('Migrate files from the named storage engine.'),
53
-
),
54
-
array(
55
-
'name' => 'local-disk-source',
56
-
'param' => 'path',
57
-
'help' => pht(
58
-
'When migrating from a local disk source, use the specified '.
59
-
'path as the root directory.'),
60
-
),
61
-
));
54
+
->setArguments($arguments);
62
55
}
63
56
64
57
public function execute(PhutilArgumentParser $args) {
···
97
90
$target_engine = PhabricatorFile::buildEngine($target_key);
98
91
99
92
$iterator = $this->buildIterator($args);
100
-
if (!$iterator) {
101
-
throw new PhutilArgumentUsageException(
102
-
pht(
103
-
'Either specify a list of files to migrate, or use `%s` '.
104
-
'to migrate all files.',
105
-
'--all'));
106
-
}
107
-
108
93
$is_dry_run = $args->getArg('dry-run');
109
94
110
95
$min_size = (int)$args->getArg('min-size');
+18
-30
src/applications/files/management/PhabricatorFilesManagementRebuildWorkflow.php
+18
-30
src/applications/files/management/PhabricatorFilesManagementRebuildWorkflow.php
···
4
4
extends PhabricatorFilesManagementWorkflow {
5
5
6
6
protected function didConstruct() {
7
+
$arguments = $this->newIteratorArguments();
8
+
9
+
$arguments[] = array(
10
+
'name' => 'dry-run',
11
+
'help' => pht('Show what would be updated.'),
12
+
);
13
+
14
+
$arguments[] = array(
15
+
'name' => 'rebuild-mime',
16
+
'help' => pht('Rebuild MIME information.'),
17
+
);
18
+
19
+
$arguments[] = array(
20
+
'name' => 'rebuild-dimensions',
21
+
'help' => pht('Rebuild image dimension information.'),
22
+
);
23
+
7
24
$this
8
25
->setName('rebuild')
9
26
->setSynopsis(pht('Rebuild metadata of old files.'))
10
-
->setArguments(
11
-
array(
12
-
array(
13
-
'name' => 'all',
14
-
'help' => pht('Update all files.'),
15
-
),
16
-
array(
17
-
'name' => 'dry-run',
18
-
'help' => pht('Show what would be updated.'),
19
-
),
20
-
array(
21
-
'name' => 'rebuild-mime',
22
-
'help' => pht('Rebuild MIME information.'),
23
-
),
24
-
array(
25
-
'name' => 'rebuild-dimensions',
26
-
'help' => pht('Rebuild image dimension information.'),
27
-
),
28
-
array(
29
-
'name' => 'names',
30
-
'wildcard' => true,
31
-
),
32
-
));
27
+
->setArguments($arguments);
33
28
}
34
29
35
30
public function execute(PhutilArgumentParser $args) {
36
31
$console = PhutilConsole::getConsole();
37
32
38
33
$iterator = $this->buildIterator($args);
39
-
if (!$iterator) {
40
-
throw new PhutilArgumentUsageException(
41
-
pht(
42
-
'Either specify a list of files to update, or use `%s` '.
43
-
'to update all files.',
44
-
'--all'));
45
-
}
46
34
47
35
$update = array(
48
36
'mime' => $args->getArg('rebuild-mime'),
+25
-5
src/applications/files/management/PhabricatorFilesManagementWorkflow.php
+25
-5
src/applications/files/management/PhabricatorFilesManagementWorkflow.php
···
3
3
abstract class PhabricatorFilesManagementWorkflow
4
4
extends PhabricatorManagementWorkflow {
5
5
6
+
protected function newIteratorArguments() {
7
+
return array(
8
+
array(
9
+
'name' => 'all',
10
+
'help' => pht('Operate on all files.'),
11
+
),
12
+
array(
13
+
'name' => 'names',
14
+
'wildcard' => true,
15
+
),
16
+
array(
17
+
'name' => 'from-engine',
18
+
'param' => 'storage-engine',
19
+
'help' => pht('Operate on files stored in a specified engine.'),
20
+
),
21
+
);
22
+
}
23
+
6
24
protected function buildIterator(PhutilArgumentParser $args) {
7
25
$viewer = $this->getViewer();
8
-
$names = $args->getArg('names');
9
26
10
27
$is_all = $args->getArg('all');
28
+
29
+
$names = $args->getArg('names');
11
30
$from_engine = $args->getArg('from-engine');
12
31
13
32
$any_constraint = ($from_engine || $names);
···
15
34
if (!$is_all && !$any_constraint) {
16
35
throw new PhutilArgumentUsageException(
17
36
pht(
18
-
'Use "--all" to migrate all files, or choose files to migrate '.
19
-
'with "--names" or "--from-engine".'));
37
+
'Specify which files to operate on, or use "--all" to operate on '.
38
+
'all files.'));
20
39
}
21
40
22
41
if ($is_all && $any_constraint) {
23
42
throw new PhutilArgumentUsageException(
24
43
pht(
25
-
'You can not migrate all files with "--all" and also migrate only '.
26
-
'a subset of files with "--from-engine" or "--names".'));
44
+
'You can not operate on all files with "--all" and also operate '.
45
+
'on a subset of files by naming them explicitly or using '.
46
+
'constraint flags like "--from-engine".'));
27
47
}
28
48
29
49
// If we're migrating specific named files, convert the names into IDs