Open Bug 1709608 Opened 4 years ago Updated 2 years ago

Lando allows re-landing committed versions of DREVs, which may result in empty binary files

Categories

(Conduit :: Lando, defect, P2)

Tracking

(Not tracked)

People

(Reporter: emilio, Unassigned)

References

Details

(Keywords: conduit-triaged)

https://phabricator.services.mozilla.com/D113264 had an exif_resolution.jpg image, which was a copy of testing/web-platform/tests/density-size-correction/resources/exif-resolution-valid-hires.jpg. But somehow when I landed the patch it was empty. It seems Phabricator also thinks the file is empty, so this is probably a moz-phab issue.

I notice it bounced and got relanded. Did you re-submit with moz-phab after the backout or not? Because the second landing just landed the phabricator version of the previous landing, but it's a "known" issue that when the final landed versions of commits are recorded in phab, they don't include binary files, so you need to re-upload after a backout. So if you didn't re-upload this is a phabricator bug rather than a moz-phab one. (If you did re-upload then it's a moz-phab bug, but the bug is that it didn't think it needed to update that DREV)

Flags: needinfo?(emilio)

I did not resubmit it, because that revision was totally fine.

Flags: needinfo?(emilio)

https://phabricator.services.mozilla.com/D113359 is also affected, I wonder this is some kind of a regression?

Resubmitting sometimes kills the images and sometimes restores them, as far as I can tell.

Re-titling/componenting for the actual issue. Could also make this a phab bug about the fact that commit-sourced versions of DREVs lack the binary files, but I think it's likelier to be handled in Lando sooner if it's made to block landings when the current version of a DREV is sourced from a commit, than waiting for upstream Phab to fix.

(In reply to Emilio Cobos Álvarez (:emilio) from comment #2)

I did not resubmit it, because that revision was totally fine.

Then yeah, the empty binary files are expected. If you compare the last submitted DREV with the version created from the first landing you can see the issue. And the fact that that the first landing was fine.

(In reply to Kagami :saschanaz from comment #3)

https://phabricator.services.mozilla.com/D113359 is also affected, I wonder this is some kind of a regression?

Not a regression, this has always been the case as far as I know.

(In reply to Kagami :saschanaz from comment #4)

Resubmitting sometimes kills the images and sometimes restores them, as far as I can tell.

I'd be very surprised if resubmitting with moz-phab ever kills binary files, do you have an example you can link?

Component: moz-phab → Lando
Flags: needinfo?(krosylight)
Summary: moz-phab uploaded a binary file as empty, somehow → Lando allows re-landing committed versions of DREVs, which may result in empty binary files

I'd be very surprised if resubmitting with moz-phab ever kills binary files, do you have an example you can link?

Sure, this submission did: https://phabricator.services.mozilla.com/D113359?vs=435358&id=435402#change-K6J3tLo3ngge

Flags: needinfo?(krosylight)

(In reply to Kagami :saschanaz from comment #6)

I'd be very surprised if resubmitting with moz-phab ever kills binary files, do you have an example you can link?

Sure, this submission did: https://phabricator.services.mozilla.com/D113359?vs=435358&id=435402#change-K6J3tLo3ngge

That diff, ID 435402, is not a moz-phab submission, that's sourced from a commit. You can tell if you go to the History tab of the Table of Contents and check the "Base" column for text like "rMOZILLACENTRALa3f23a20b5322d0093d9f73189a1f64a9529567e" (or install the extension that'll give you a "Creation Method" column that'll tell you explicitly).

Ah right, I mean pushing a commit, yes. The commit always had the images but pushing it via moz-phab randomly killed them or restored them.

Assignee: nobody → zeid
Keywords: conduit-triaged
Priority: -- → P3
Priority: P3 → P2

A more recent example of this is https://phabricator.services.mozilla.com/D148288. First landed in https://hg.mozilla.org/integration/autoland/rev/59eac90984cb1f248454fc5b87f1dd528547c3f3 (backed out for reasons unrelated to this bug), then relanded in https://hg.mozilla.org/integration/autoland/rev/9651db4a6e3f2dd2a63f6c30d77078f291380a49, which failed to add the binary file correctly (a DLL in this case).

Assignee: zeid → nobody
You need to log in before you can comment on or make changes to this bug.