@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
Fix some straggling qsprintf() warnings in repository import
Summary:
Ref T13217. See <https://discourse.phabricator-community.org/t/unsafe-raw-string-warnings-while-importing-git-commits/2191>.
Hunt down and fix two more `qsprintf()` things.
I just converted the "performance optimization" into a normal, safe call since we're dealing with far less SVN stuff nowadays and the actual issue has been lost in the mists of time. If it resurfaces, we can take another look.
Test Plan: Imported some commits, no longer saw these warnings in the daemon logs.
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T13217
Differential Revision: https://secure.phabricator.com/D19869
+12
-15
+2
-1
src/applications/diffusion/query/pathchange/DiffusionPathChangeQuery.php
+2
-1
src/applications/diffusion/query/pathchange/DiffusionPathChangeQuery.php
+10
-14
src/applications/repository/worker/commitchangeparser/PhabricatorRepositoryCommitChangeParserWorker.php
+10
-14
src/applications/repository/worker/commitchangeparser/PhabricatorRepositoryCommitChangeParserWorker.php
···
114
114
PhabricatorRepositoryCommit $commit,
115
115
array $changes) {
116
116
117
+
$conn = $repository->establishConnection('w');
118
+
117
119
$repository_id = (int)$repository->getID();
118
120
$commit_id = (int)$commit->getID();
119
-
120
-
// NOTE: This SQL is being built manually instead of with qsprintf()
121
-
// because some SVN changes affect an enormous number of paths (millions)
122
-
// and this showed up as significantly slow on a profile at some point.
123
121
124
122
$changes_sql = array();
125
123
foreach ($changes as $change) {
126
-
$values = array(
124
+
$changes_sql[] = qsprintf(
125
+
$conn,
126
+
'(%d, %d, %d, %nd, %nd, %d, %d, %d, %d)',
127
127
$repository_id,
128
128
(int)$change->getPathID(),
129
129
$commit_id,
130
-
nonempty((int)$change->getTargetPathID(), 'null'),
131
-
nonempty((int)$change->getTargetCommitID(), 'null'),
130
+
nonempty((int)$change->getTargetPathID(), null),
131
+
nonempty((int)$change->getTargetCommitID(), null),
132
132
(int)$change->getChangeType(),
133
133
(int)$change->getFileType(),
134
134
(int)$change->getIsDirect(),
135
-
(int)$change->getCommitSequence(),
136
-
);
137
-
$changes_sql[] = '('.implode(', ', $values).')';
135
+
(int)$change->getCommitSequence());
138
136
}
139
137
140
-
$conn_w = $repository->establishConnection('w');
141
-
142
138
queryfx(
143
-
$conn_w,
139
+
$conn,
144
140
'DELETE FROM %T WHERE commitID = %d',
145
141
PhabricatorRepository::TABLE_PATHCHANGE,
146
142
$commit_id);
147
143
148
144
foreach (PhabricatorLiskDAO::chunkSQL($changes_sql) as $chunk) {
149
145
queryfx(
150
-
$conn_w,
146
+
$conn,
151
147
'INSERT INTO %T
152
148
(repositoryID, pathID, commitID, targetPathID, targetCommitID,
153
149
changeType, fileType, isDirect, commitSequence)