1489126 - Cannot reland without running arc diff again

Status: RESOLVED FIXED

(Conduit :: Lando, defect, P2)

Description

[Jean-Yves Avenard [:jya]]

The following scenario occurred:

https://phabricator.services.mozilla.com/D5041 landed and then got backed out.

the reason for being backed out was that https://phabricator.services.mozilla.com/D5056 landing had failed and D5041 was dependent of it.

To re-land D5041, you must re-open the revision
then run arc diff to re-upload the commit again
Go back to the phabricator page and reland with lando...

It would be nice if there was a way to reland without having to re-run arc diff ; 

maybe the revision could be re-opened automatically too seeing that it got backed out.

Comment 2

[Mark Côté [:mcote]]

This is definitely a confusing problem.

Comment 3

[Steven MacLeod [:smacleod]]

(In reply to Mark Côté [:mcote] from comment #2)
> This is definitely a confusing problem.

It's mainly due to a "bug" in Phabricator itself, which I filed upstream as https://admin.phacility.com/PHI787. I've bumped the issue a bit again.

Comment 4

[Alexandre Poirot [:ochameau]]

+1

When you have a big patch serie being backed out, it quickly turns into a nightmare.
Having to go into each individual entry on phabricator, to scroll down and reopen each revision is tedious.
I actually did that the first time it occured to me, but then I thought that lando wasn't able to re-land a backed out serie at all. So I switched to landing manually via inbound and stopped using phabricator for this serie...

I learned about this bug today and now I know that we could fix that by re-uploading all the backed out changesets!

I think that there is two issues here:
* having to re-open each phabricator entry of a serie is tedious (we could reopen on backouts or allow to reopen a whole serie at once?)
* you have to re-upload patches to phabricator in order to land as a serie again via lando, but it is not discoverable
  Lando simply says "Blocked (I don't remember the popup but it mention an issue with the ancestor not being able to land)".
  It would be great if it mention why it is blocked and/or mention that you may have to re-push your patches to unlock it.
  It would be even better if we did not had to re-upload patches to reland ;)

Thanks for your tooling!

Comment 6

[(no longer active)]

I experienced something like this bug with https://lando.services.mozilla.com/D15109/, even though I had pushed that using phlay, not arc, FWIW.

I now have to use inbound because I don't know how to get Lando to work with this...

Comment 7

[Andreas Tolfsen ❲:ato❳]

I believe I just experienced this with
https://lando.services.mozilla.com/D17080/.

Comment 9

[Byron Campen [:bwc]]

Happened to me just now in bug 1531811.

Comment 10

[[ex-Mozilla] Agi Sferro | :agi]

Just experienced this with Bug 1512274

Comment 11

[Steven MacLeod [:smacleod]]

Attachment: differential: copy commit info to diff on commit sync (Bug 1489126).

Phabricator does not copy the commit meta data (such as author, commit
message, and id) to the diff when updating a revision from a synced
commit. We have requested this upstream[1] but will patch Phabricator
until it is fixed.

This change does not make Phabricator copy all commit information, only
the most important data that we care about. This might require a
followup as there is also a concerning TODO which indicates changes to
binary files might not be part of the diff that Phabricator updates the
revision with.

[1] https://admin.phacility.com/PHI787

Comment 12

[Steven MacLeod [:smacleod]]

This should be fixed after the next Phabricator deploy. Users will still need to manually re-open revisions (Bug 1507578) for now, but they won't have to re-upload diffs. Lando should happily land the diff created by Phabricator syncing the landed commit.