1323633 - Ensure that task graphs do not contain duplicate Treeherder symbols, also consider treeherder.collection.keys()

Status: RESOLVED FIXED

(Firefox Build System :: Task Configuration, task)

Description

[HAMMAD AKHTAR]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_1) AppleWebKit/602.2.14 (KHTML, like Gecko) Version/10.0.1 Safari/602.2.14

Comment 1

[HAMMAD AKHTAR]

(In reply to HAMMAD AKHTAR from comment #0)
> User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_1)
> AppleWebKit/602.2.14 (KHTML, like Gecko) Version/10.0.1 Safari/602.2.14

Comment 2

[HAMMAD AKHTAR]

Treeherder decides what symbol to print, and where, based on
  task.extra.treeherder.collection.keys()
  task.extra.treeherder.machine.platform
  task.extra.treeherder.groupSymbol
  task.extra.treeherder.symbol

We should never generate two tasks with the same symbol.  The decision task should verify this and fail if we do.

Thanks Philor for pointing this out :)

I'll take up this bug.

Comment 3

[HAMMAD AKHTAR]

Attachment: Bug 1323633 - Ensure that task graphs do not contain duplicate Treeherder symbols, also consider treeherder.collection.keys();

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

Comment 4

[HAMMAD AKHTAR]

Comment on attachment 8818804 [details]
Bug 1323633 - Ensure that task graphs do not contain duplicate Treeherder symbols, also consider treeherder.collection.keys();

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

Comment 6

[HAMMAD AKHTAR]

Comment on attachment 8818804 [details]
Bug 1323633 - Ensure that task graphs do not contain duplicate Treeherder symbols, also consider treeherder.collection.keys();

Hey Dustin, patch is ready for review. It is failing on try because I am still working on an older version of the repository.

Comment 7

[Dustin J. Mitchell [:dustin] (he/him)]

Comment on attachment 8818804 [details]
Bug 1323633 - Ensure that task graphs do not contain duplicate Treeherder symbols, also consider treeherder.collection.keys();

https://reviewboard.mozilla.org/r/98748/#review99976

This looks good.  However, as you mention, it needs to be rebased.  It also seems to have "adopted" `verify_docs` which doesn't belong in this commit.  Can you rebase on top of the latest and push for review again, and I'll land it?

This kind of thing happens often.  It's always tricky working on a few bugs at the same time that touch the same files.

Comment 8

[HAMMAD AKHTAR]

Comment on attachment 8818804 [details]
Bug 1323633 - Ensure that task graphs do not contain duplicate Treeherder symbols, also consider treeherder.collection.keys();

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

Comment 9

[HAMMAD AKHTAR]

(In reply to Dustin J. Mitchell [:dustin] from comment #7)
> Comment on attachment 8818804 [details]
> Bug 1323633 - Ensure that task graphs do not contain duplicate Treeherder
> symbols, also consider treeherder.collection.keys();
> 
> https://reviewboard.mozilla.org/r/98748/#review99976
> 
> This looks good.  However, as you mention, it needs to be rebased.  It also
> seems to have "adopted" `verify_docs` which doesn't belong in this commit. 
> Can you rebase on top of the latest and push for review again, and I'll land
> it?

When I rename the verifydoc.py to verify.py so as to have a common verification module, all the tracking history for verifydoc.py is lost. It is as if content of verify.py are created in this patch only.

If above is fine then commit is ready for review :)

Comment 10

[Dustin J. Mitchell [:dustin] (he/him)]

Ah, I see.  If you use `hg mv` then hg will track the change as a rename.  Thanks, too, for running another try job.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8e51ea2602c4
Ensure that task graphs do not contain duplicate Treeherder symbols, also consider treeherder.collection.keys(); r=dustin

Comment 12

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/8e51ea2602c4