Changeset View
Standalone View
src/query/ArcanistMercurialWorkingCopyRevisionHardpointQuery.php
| Show All 16 Lines | public function loadHardpoint(array $refs, $hardpoint) { | ||||
| yield $this->yieldRequests( | yield $this->yieldRequests( | ||||
| $refs, | $refs, | ||||
| array( | array( | ||||
| ArcanistWorkingCopyStateRef::HARDPOINT_COMMITREF, | ArcanistWorkingCopyStateRef::HARDPOINT_COMMITREF, | ||||
| )); | )); | ||||
| // TODO: This has a lot in common with the Git query in the same role. | // TODO: This has a lot in common with the Git query in the same role. | ||||
| // TODO: In Mercurial if commits are rebased or changed locally they are | |||||
Lint: TODO Comment: This comment has a TODO. | |||||
| // given brand new commit hashes, resulting in this query not being | |||||
| // able to identify revisions associated with the commit. This can | |||||
| // happen after landing a non-tip revision in a chain of dependent | |||||
| // revisions. This can be confusing as `arc which` will still be able | |||||
| // to identify the right revision by using the `base` ruleset. Maybe | |||||
| // this could re-use the same `base` ruleset logic as a fallback? | |||||
cspeckmimAuthorUnsubmitted Done Inline ActionsI also ran into this odd behavior which confused me at first. After landing a non-head revision I tried to land the last remaining head revision and it failed to find an associated revision. Locally the revision contains 2 changesets, which only the first's commit message was updated to include a Differential Revision: line upon creation of the original diff, but the tip commit does not have this line. However when using arc which it will indicate that it's associated to a revision. I dropped a TODO here because I'm not sure what the solution would be (utilizing base revset is a guess) and it's probably a larger change to try and resolve. cspeckmim: I also ran into this odd behavior which confused me at first. After landing a non-head revision… | |||||
epriestleyUnsubmitted Not Done Inline ActionsYeah, I think this is probably out of scope here, although I think (?) I'd expect arc to get this right anyway. A possible solution in general (to the problem of arc losing commit mappings across rebases) is arc rebase, which would let arc retain a mapping of "old commit X was rebased into new commit Y, so they're the same when figuring out revisions". Another "solution" is to run arc diff and update the diff with a silly message like "rebased lol", although I dislike this for various reasons. Still, I'd expect arc land <head2> to look at the ancestors between head2 and master, find the "Differential Revision" commit, and then figure out that "head2" can only be part of that revision, possibly prompting you ("Local range includes commits which don't match the revision, are you sure you want to land them?"). But almost certainly not really an issue with the behavior here, at least. epriestley: Yeah, I think this is probably out of scope here, although I think (?) I'd expect `arc` to get… | |||||
cspeckmimAuthorUnsubmitted Done Inline Actions
If I want to go spelunking later to see about improving/adding this behavior is this the right place to do this or is there another place this TODO should go? cspeckmim: >Still, I'd expect `arc land <head2>` to look at the ancestors between `head2` and `master`… | |||||
| $hashes = array(); | $hashes = array(); | ||||
| $map = array(); | $map = array(); | ||||
| foreach ($refs as $ref_key => $ref) { | foreach ($refs as $ref_key => $ref) { | ||||
| $commit = $ref->getCommitRef(); | $commit = $ref->getCommitRef(); | ||||
| $commit_hashes = array(); | $commit_hashes = array(); | ||||
| $commit_hashes[] = array( | $commit_hashes[] = array( | ||||
| ▲ Show 20 Lines • Show All 44 Lines • Show Last 20 Lines | |||||
This comment has a TODO.