Closed Bug 1379559 Opened 7 years ago Closed 7 years ago

improve the error messages generated by overlay during content mismatch

Tracking

(Not tracked)

Status:

RESOLVED FIXED

People

(Reporter: glob, Assigned: glob)

References

Details

Attachments

(5 files)

overlay: Improve error messages during mismatch (bug 1379559) 7 years ago :glob ✱ 59 bytes, text/x-review-board-request	gps : review+	Details
overlay: add support for passing errors to an external handler (bug 1379559) 7 years ago :glob ✱ 59 bytes, text/x-review-board-request	gps : review+	Details
scripts: add SNS publishing wrapper (bug 1379559) 7 years ago :glob ✱ 59 bytes, text/x-review-board-request	gps : review+	Details
ansible: deploy publish-to-sns to servo-vcs-sync (bug 1379559) 7 years ago :glob ✱ 59 bytes, text/x-review-board-request	gps : review+	Details
servo-vcs-sync: configure overlay to notify via SNS (bug 1379559) 7 years ago :glob ✱ 59 bytes, text/x-review-board-request	gps : review+	Details

:glob ✱

Assignee

Description

•

7 years ago

to allow for stylo devs to resolve servo-vcs-sync issues without our assistance, we need to improve the error messages generated by overlay during content mismatch.

the current output is:

> 2017-04-05T08:14:52.974901 [1] Starting Overlay Servo changesets into a Firefox repository...
> 2017-04-05T08:14:53.786105 [26360] executing: hg strip --no-backup -r 'not public()'
> 2017-04-05T08:14:57.552448 [26360] hg> 494 files updated, 0 files merged, 25 files removed, 0 files unresolved
> 2017-04-05T08:14:58.386849 [26360] resolving destination revision: tip
> 2017-04-05T08:14:58.387036 [26360] executing: hg identify https://hg.mozilla.org/integration/autoland -r tip
> 2017-04-05T08:14:58.827103 [26360] hg> 4305f22e882c
> 2017-04-05T08:14:58.842434 [26360] pulling https://hg.mozilla.org/integration/autoland to obtain 4305f22e882c
> 2017-04-05T08:14:58.842625 [26360] executing: hg pull -r 4305f22e882c https://hg.mozilla.org/integration/autoland
> 2017-04-05T08:14:58.844725 [26360] hg> pulling from https://hg.mozilla.org/integration/autoland
> 2017-04-05T08:14:59.235085 [26360] hg> searching for changes
> 2017-04-05T08:14:59.523209 [26360] hg> adding changesets
> 2017-04-05T08:14:59.537392 [26360] hg> adding manifests
> 2017-04-05T08:14:59.606990 [26360] hg> adding file changes
> 2017-04-05T08:15:00.044167 [26360] hg> added 85 changesets with 570 changes to 497 files
> 2017-04-05T08:15:00.314760 [26360] hg> (run 'hg update' to get a working copy)
> 2017-04-05T08:15:00.347674 [26360] commencing overlay of https://hg.mozilla.org/projects/converted-servo-linear
> 2017-04-05T08:15:00.347856 [26360] executing: hg overlay https://hg.mozilla.org/projects/converted-servo-linear --into servo -d 4305f22e882cb9e769d227e9946ee3a1490bef91
> 2017-04-05T08:15:00.530495 [26360] hg> pulling https://hg.mozilla.org/projects/converted-servo-linear into /home/servo-sync/firefox-overlay/.hg/hg.mozilla.org__projects__converted-servo-linear
> 2017-04-05T08:15:00.618390 [26360] hg> searching for changes
> 2017-04-05T08:15:00.747455 [26360] hg> adding changesets
> 2017-04-05T08:15:00.748378 [26360] hg> adding manifests
> 2017-04-05T08:15:00.749910 [26360] hg> adding file changes
> 2017-04-05T08:15:00.752094 [26360] hg> added 1 changesets with 1 changes to 1 files
> 2017-04-05T08:15:01.494368 [26360] hg> febfe9bf5d1c already processed as 2dbda4bb18ff; skipping 8736/8737 revisions
> 2017-04-05T08:15:02.463585 [26360] hg> abort: content mismatch between source (febfe9bf5d1c) and destination (4305f22e882c) in servo/ports/geckolib/glue.rs
> 2017-04-05T08:15:02.600106 [26360] abort: hg command failed
> 2017-04-05T08:15:02.611109 [26360] mozvcssync.servo abort: Command '['/home/servo-sync/venv/bin/overlay-hg-repos', 'https://hg.mozilla.org/projects/converted-servo-linear', 'https://hg.mozilla.org/integration/autoland', '/home/servo-sync/firefox-overlay', u'--into', 'servo', u'--result-push-url', 'ssh://hg.mozilla.org/integration/autoland', u'--hg', '/home/servo-sync/venv/bin/hg']' returned non-zero exit status 1
> 2017-04-05T08:15:02.620998 [1] servo-overlay.service: main process exited, code=exited, status=1/FAILURE
> 2017-04-05T08:15:02.621217 [1] Failed to start Overlay Servo changesets into a Firefox repository.
> 2017-04-05T08:15:02.621382 [1] Unit servo-overlay.service entered failed state.
> 2017-04-05T08:15:02.621537 [1] Triggering OnFailure= dependencies of servo-overlay.service.
> 2017-04-05T08:15:02.621695 [1] servo-overlay.service failed.

in SF we spoke about sending an augmented error message to an sns topic, which stylo devs could subscribe to.  this would be in addition to the full logs we generate now.


the important line from the above example being:
> content mismatch between source (febfe9bf5d1c) and destination (4305f22e882c) in servo/ports/geckolib/glue.rs

in the above case i would expect something like:

> Synchronising of github.com/servo/servo to hg.mozilla.org/integration/autoland failed
> due to unexpected changes.
> 
> File:
>    servo/ports/geckolib/glue.rs
> Changed by:
>   https://hg.mozilla.org/integration/autoland/rev/aa46d54ab74b
>   Bug 1321754 - Add an enum value to SheetParsingMode for agent sheets that use no unsafe rules. r=heycam
> 
> Full error:
>  abort: content mismatch between source (febfe9bf5d1c) and destination (4305f22e882c) in servo/ports/geckolib/glue.rs

to work out what commit made the problematic change, i grabbed the last revision on the source (converted-servo-linear) that touched the file:
> $ hg log ports/geckolib/glue.rs -r 'reverse(:febfe9bf5d1c)' -l 1
> changeset:   8718:135f4003dbf4
> user:        Xidorn Quan <me@upsuper.org>
> date:        Mon Apr 03 07:05:42 2017 -0500
> summary:     servo: Merge #16241 - Stop passing url as string into Servo side (from upsuper:bug1352763-bug1352025); r=Manishearth,emilio

and listed all revisions on integration-autoland that touched that file after the merge:

> $ hg log servo/ports/geckolib/glue.rs -r 'reverse(:4305f22e882c)'
> changeset:   351225:aa46d54ab74b
> user:        Ting-Yu Lin <tlin@mozilla.com>
> date:        Tue Mar 28 18:06:26 2017 +0800
> summary:     Bug 1321754 - Add an enum value to SheetParsingMode for agent sheets that use no unsafe rules. r=heycam
> 
> changeset:   350928:13eb16dd02f4
> user:        Xidorn Quan <me@upsuper.org>
> date:        Mon Apr 03 07:05:42 2017 -0500
> summary:     servo: Merge #16241 - Stop passing url as string into Servo side (from upsuper:bug1352763-bug1352025); r=Manishearth,emilio

:glob ✱

Assignee

Comment 1

•

7 years ago

after a chatting with smacleod we came the the conclusion it may be easier to just list the commits on both the source and destination repos that were unexpected, instead of matching by file.

:glob ✱

Assignee

Comment 2

•

7 years ago

here's the output i current have for when a file was unexpectedly added to the destination repo.  in this case there were other servo changes that are not overlaid; only the problematic commit is identified in the error report.

> files mismatch between source and destination: this.file.should.not.be.here
> 
> Destination Repository:
> 
> Last overlaid revision:
> 
> changeset: 7ceb17fb947d0e299901c4690a58d476a7ddd165
> user:      Hiroyuki Ikezoe <hikezoe@mozilla.com>
> date:      Mon Jul 10 02:31:35 2017 -0700
> summary:   servo: Merge #17648 - Do not fill computed values in missing 0%/100% keyframes in CSS Animation (from hiikezoe:missing-keyframes); r=birtles
> 
> Revisions that require investigation:
> 
> changeset: 5bebe77c723ff431d3db4839364917b93dfda6f8
> user:      byron jones <glob@mozilla.com>
> date:      Tue Jul 11 22:16:37 2017 +0800
> summary:   rogue commit

now working on hooking up more errors to it and sns integration:

:glob ✱

Assignee

Updated

•

7 years ago

Depends on: 1381391

Comment hidden (mozreview-request)

Generate an error that shows the last overlaid revision and any commits that
touch mismatch files.

For now these error messages are not used, that's addressed in the next commit.

Review commit: https://reviewboard.mozilla.org/r/158148/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/158148/

Comment hidden (mozreview-request)

In order to deliver informative errors to those who can resolve overlay issues,
add support for specifying an external command to deliver notifications.

Review commit: https://reviewboard.mozilla.org/r/158150/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/158150/

Comment hidden (mozreview-request)

Add a wrapper around aws-cli that publishes to SNS.

This wrapper exists to support passing in the message via STDIN, and removes
the requirement to specify the AWS region explicitly (as it can be extracted
from the topic's ARN).

Review commit: https://reviewboard.mozilla.org/r/158152/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/158152/

Comment hidden (mozreview-request)

Review commit: https://reviewboard.mozilla.org/r/158154/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/158154/

Comment hidden (mozreview-request)

Review commit: https://reviewboard.mozilla.org/r/158156/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/158156/

Gregory Szorc [:gps]

Comment 8

•

7 years ago

mozreview-review

Comment on attachment 8887325 [details]
overlay: Improve error messages during mismatch (bug 1379559)

https://reviewboard.mozilla.org/r/158148/#review163730

It's weird to see the new code introduced before it is used. But OK.

::: hgext/overlay/__init__.py:58
(Diff revision 1)
> +
> +    # Find revisions newer than the last overlaid.
> +    dest_revs = scmutil.revrange(
> +        repo, ['%s:: and file("path:%s")' % (last_ctx.hex(), prefix)])
> +    for rev in dest_revs:
> +        node = repo[rev]

Nit: the variable name should have "ctx" in it per convention.

::: hgext/overlay/__init__.py:90
(Diff revision 1)
> +
> +
> +def _report_mismatch(ui, sourcerepo, lastsourcectx, destrepo, lastdestctx,
> +                     prefix, files, error_message, hint=None):
> +    if files:
> +        prefixed_file_set = set(['%s%s' % (prefix, f) for f in files])

Nit: you don't need the [] here since set() accepts an iterable and comprehension statements are iterable.

Attachment #8887325 - Flags: review?(gps) → review+

Gregory Szorc [:gps]

Comment 9

•

7 years ago

mozreview-review

Comment on attachment 8887326 [details]
overlay: add support for passing errors to an external handler (bug 1379559)

https://reviewboard.mozilla.org/r/158150/#review163734

Attachment #8887326 - Flags: review?(gps) → review+

Gregory Szorc [:gps]

Comment 10

•

7 years ago

mozreview-review

Comment on attachment 8887327 [details]
scripts: add SNS publishing wrapper (bug 1379559)

https://reviewboard.mozilla.org/r/158152/#review163740

Attachment #8887327 - Flags: review?(gps) → review+

Gregory Szorc [:gps]

Comment 11

•

7 years ago

mozreview-review

Comment on attachment 8887328 [details]
ansible: deploy publish-to-sns to servo-vcs-sync (bug 1379559)

https://reviewboard.mozilla.org/r/158154/#review163742

Attachment #8887328 - Flags: review?(gps) → review+

Gregory Szorc [:gps]

Comment 12

•

7 years ago

mozreview-review

Comment on attachment 8887329 [details]
servo-vcs-sync: configure overlay to notify via SNS (bug 1379559)

https://reviewboard.mozilla.org/r/158156/#review163746

::: ansible/roles/vcs-sync/files/servo-overlay.service:21
(Diff revision 1)
>    --into servo \
>    --result-push-url $OVERLAY_RESULT_PUSH_URL \
>    --push-tree $OVERLAY_PUSH_TREE \
>    --hg /home/servo-sync/venv/bin/hg \
> -  --overlay-hg-repos /home/servo-sync/venv/bin/overlay-hg-repos
> +  --overlay-hg-repos /home/servo-sync/venv/bin/overlay-hg-repos \
> +  --notify '~/publish-to-sns --subject "Servo Overlay Failure" --topic-arn $OVERLAY_ERROR_TOPIC'

The use of ~ here is slightly wonky, as that's a shell-ism. But you handle it properly in the overlay extension, so I suppose it is OK.

Attachment #8887329 - Flags: review?(gps) → review+

Comment hidden (mozreview-request)

Comment on attachment 8887325 [details]
overlay: Improve error messages during mismatch (bug 1379559)

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/158148/diff/1-2/

Comment hidden (mozreview-request)

Comment on attachment 8887326 [details]
overlay: add support for passing errors to an external handler (bug 1379559)

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/158150/diff/1-2/

Comment hidden (mozreview-request)

Comment on attachment 8887327 [details]
scripts: add SNS publishing wrapper (bug 1379559)

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/158152/diff/1-2/

Comment hidden (mozreview-request)

Comment on attachment 8887328 [details]
ansible: deploy publish-to-sns to servo-vcs-sync (bug 1379559)

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/158154/diff/1-2/

Comment hidden (mozreview-request)

Comment on attachment 8887329 [details]
servo-vcs-sync: configure overlay to notify via SNS (bug 1379559)

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/158156/diff/1-2/

Pulsebot

Comment 18

•

7 years ago

Pushed by bjones@mozilla.com:
https://hg.mozilla.org/hgcustom/version-control-tools/rev/f66e6aa660e0
overlay: Improve error messages during mismatch r=gps
https://hg.mozilla.org/hgcustom/version-control-tools/rev/59cc790df27d
overlay: add support for passing errors to an external handler r=gps
https://hg.mozilla.org/hgcustom/version-control-tools/rev/5fa269023123
scripts: add SNS publishing wrapper r=gps
https://hg.mozilla.org/hgcustom/version-control-tools/rev/755fd7ffdf99
ansible: deploy publish-to-sns to servo-vcs-sync r=gps
https://hg.mozilla.org/hgcustom/version-control-tools/rev/9095b4202cdf
servo-vcs-sync: configure overlay to notify via SNS r=gps

Status: NEW → RESOLVED

Closed: 7 years ago

Resolution: --- → FIXED

You need to log in before you can comment on or make changes to this bug.