recaptime.dev / phorge
@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
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

Add a heursitic for initial pushes which are really imports

Summary:
Fixes T7298. There are two ways to import a repository that you want to host, today:

- Create it as "hosted", then push everything to it.
- Create it as "imported", let it import, then switch it to "hosted".
- (Neither of these work with SVN.)

We don't specifically recommend one or the other, although I believe both should work, and most users seem to go with the first one.

In the first workflow, the new empty repository imports completely and gets marked "imported", so our default behavior is then to publish commits. This can generate a lot of email/notification/feed spam.

If you're a fancy expert you might turn off "publish" before pushing, but normal users will frequently miss this.

Instead, when we receive an "import-like" push to an empty repository, put the repository back into "importing" after we accept the changes.

This has to be heuristic since we can't know for sure if a push is an import or new commits, but here's a simple rule that should do pretty well. We can refine it if necessary.

Test Plan:
- Created a new empty repository.
- Added some debugging code; verified the "commit count" and "empty" rules were calculated properly.
- Pushed 8+ commits and saw the repo go into "importing", import, and leave "importing".
- Pushed 8+ commits again and saw them publish.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7298

Differential Revision: https://secure.phabricator.com/D11827

epriestley f6915a79 8599145b

+71
+71
src/applications/diffusion/engine/DiffusionCommitHookEngine.php
··· 173 173 throw $caught; 174 174 } 175 175 176 + // If this went through cleanly, detect pushes which are actually imports 177 + // of an existing repository rather than an addition of new commits. If 178 + // this push is importing a bunch of stuff, set the importing flag on 179 + // the repository. It will be cleared once we fully process everything. 180 + 181 + if ($this->isInitialImport($all_updates)) { 182 + $repository = $this->getRepository(); 183 + 184 + $repository->openTransaction(); 185 + $repository->beginReadLocking(); 186 + $repository = $repository->reload(); 187 + $repository->setDetail('importing', true); 188 + $repository->save(); 189 + $repository->endReadLocking(); 190 + $repository->saveTransaction(); 191 + } 192 + 176 193 if ($this->emailPHIDs) { 177 194 // If Herald rules triggered email to users, queue a worker to send the 178 195 // mail. We do this out-of-process so that we block pushes as briefly ··· 1188 1205 } 1189 1206 1190 1207 return $map; 1208 + } 1209 + 1210 + private function isInitialImport(array $all_updates) { 1211 + $repository = $this->getRepository(); 1212 + 1213 + $vcs = $repository->getVersionControlSystem(); 1214 + switch ($vcs) { 1215 + case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN: 1216 + // There is no meaningful way to import history into Subversion by 1217 + // pushing. 1218 + return false; 1219 + default: 1220 + break; 1221 + } 1222 + 1223 + // Now, apply a heuristic to guess whether this is a normal commit or 1224 + // an initial import. We guess something is an initial import if: 1225 + // 1226 + // - the repository is currently empty; and 1227 + // - it pushes more than 7 commits at once. 1228 + // 1229 + // The number "7" is chosen arbitrarily as seeming reasonable. We could 1230 + // also look at author data (do the commits come from multiple different 1231 + // authors?) and commit date data (is the oldest commit more than 48 hours 1232 + // old), but we don't have immediate access to those and this simple 1233 + // heruistic might be good enough. 1234 + 1235 + $commit_count = 0; 1236 + $type_commit = PhabricatorRepositoryPushLog::REFTYPE_COMMIT; 1237 + foreach ($all_updates as $update) { 1238 + if ($update->getRefType() != $type_commit) { 1239 + continue; 1240 + } 1241 + $commit_count++; 1242 + } 1243 + 1244 + if ($commit_count <= 7) { 1245 + // If this pushes a very small number of commits, assume it's an 1246 + // initial commit or stack of a few initial commits. 1247 + return false; 1248 + } 1249 + 1250 + $any_commits = id(new DiffusionCommitQuery()) 1251 + ->setViewer($this->getViewer()) 1252 + ->withRepository($repository) 1253 + ->setLimit(1) 1254 + ->execute(); 1255 + 1256 + if ($any_commits) { 1257 + // If the repository already has commits, this isn't an import. 1258 + return false; 1259 + } 1260 + 1261 + return true; 1191 1262 } 1192 1263 1193 1264 }