`moz-phab patch --apply-to here` should apply to the original base revision and rebase, rather than applying the raw diff on HEAD
Categories
(Conduit :: moz-phab, enhancement, P2)
Tracking
(Not tracked)
People
(Reporter: Gijs, Assigned: padenot)
References
Details
Attachments
(2 files)
Lando appears to "just" apply diffs to git HEAD.
It would be nice if, just like the abilities moz-phab patch recently grew / is growing, it would instead apply to the base rev that phab advertises, and tries to rebase (and perhaps fall back to the current logic if that fails due to not finding the base rev, or something).
This would allow more patches to land without developers having to manually rebase and resubmit (and then additionally encountering a "no reviewer has approved the current patch" warning, and sometimes a "pending builds" one, too!).
| Assignee | ||
Comment 1•1 year ago
|
||
https://github.com/padenot/review/commit/93529e640f952fe4fcce30ab0224c83e060992c5 does some of that, but has some shortcomings, such as the inability to update the "base" revision for a particular commit (or at least I haven't found an endpoint to do so).
So this breaks whenever the original commit of a patch stack was something that didn't exist on the canonical repo, e.g. you've created a commit, then later decided it wasn't useful, so abandoned, it, etc.
Maybe we can be a bit pragmatic and switch to an earlier base, loosely based on dates instead of hashes.
Updated•3 months ago
|
| Assignee | ||
Comment 2•1 month ago
|
||
moz-phab patch used to apply a patch directly wherever --apply-to
pointed, trusting that commit to be valid. It sometimes isn't: the
diff's base commit can be missing locally, or exist only as part of
another, unlanded patch stack. This is unconditional -- it happens for
every patch invocation, regardless of --apply-to (or its default).
The patch now always applies at the diff's original base commit first,
then rebases onto whatever --apply-to names as the target:
- If the diff's base commit is public (landed), use it as-is.
- Otherwise (not found at all, or found but not public -- eg. only
reachable from another, unlanded stack, so fetching from upstream
will never find it), fall back to the most recent
autoland-to-mozilla-central merge that landed at or before the
patch's own timestamp (the later of the diff's creation date and the
revision's last-modified date).
Implemented for git and jj (which delegates to its colocated git
backend); hg gets an equivalent phase- and revset-based implementation
but is lightly tested here.
Updated•1 month ago
|
Comment 3•1 month ago
|
||
I didn't notice this bug when I completed this on the Lando side in Bug 2047605. Morphing the bug to cover the moz-phab use case instead.
| Assignee | ||
Comment 4•1 month ago
|
||
Rather than always resolving a public base and rebasing, first try
applying the patch directly at the target -- this is cheap and avoids
rebuilds from files only touched by a rebase. Only if that direct
application fails (eg. the diff's base commit is missing locally, or
belongs to another, unlanded patch stack) do we fall back to
resolving a public base and rebasing onto the target.
A failed direct attempt needs no special cleanup: retrying just calls
before_patch again at the resolved base, same as running patch
twice in a row already does -- the existing branch/bookmark-name
collision handling picks a fresh name, and the abandoned commits are
left for the usual VCS-specific garbage collection.
Updated•12 days ago
|
Comment 5•7 days ago
|
||
Authored by https://github.com/padenot
https://github.com/mozilla-conduit/review/commit/cc5359ee646a654d44ec55d9ad1d7c46a3de8f96
[main] Bug 1987220 - Always apply at a resolved public base, then rebase to target, with a twist. DOT_GITHUB_OVERRIDE r=sheehan
Authored by https://github.com/padenot
https://github.com/mozilla-conduit/review/commit/59a6520debf87c284a1686d9e346e4f7cf710279
[main] Bug 1987220 - try applying at the target before falling back to the rebase strategy. r=sheehan
Description
•