1478883 - Tagging operations should use DONTBUILD in commit message

Status: RESOLVED FIXED

(Firefox Build System :: Task Configuration, task)

Description

[Justin Wood (:Callek)]

+++ This bug was initially created as a clone of Bug #1473268 +++

> Our tagging operations are quite frequent now 
> * _RELEASE tags at the end of a release
> * _BUILDN tags early in the release since bug 1301782 
> 
> There's a tag for each product, so  we end up with multiple pushes that cause a
> full set of builds & tests. Three in the case of the betas early in the week
> for Fennec, Dev Edition, and Firefox. 
> 
> The time spent on the pushes doesn't gain us anything so we should add
> DONTBUILD to the commit message [1]. This would be similar to the l10n bumper.
> 
> Also wondering why do we tag on beta when bug 1301782 was about RC releases.
> 
> [1] https://github.com/mozilla-releng/treescript/blob/690f0dfb967cbf214e639a983fd6d25bd3a155ed/treescript/mercurial.py#L17

Over in Bug 1473268 we implemented support for DONTBUILD in treescript itself, so now we need to use it in the taskgraph.

The only spot we need to do DONTBUILD is in the "early tagging" kind, specifically because when all we do is tag we don't need to have builds. The other jobs that use treescript include a version bump, which we always want to see builds on.

How I envision fixing this bug:

https://dxr.mozilla.org/mozilla-central/source/taskcluster/ci/release-early-tagging/kind.yml#23 we add `dontbuild: true` to the worker definition here.

Then over in task.py we add `dontbuild` to the schema https://dxr.mozilla.org/mozilla-central/source/taskcluster/taskgraph/transforms/task.py#576 (as optional)

Finally in https://dxr.mozilla.org/mozilla-central/source/taskcluster/taskgraph/transforms/task.py#1167 which implements the payload information as we expect we check for the existance of 'dontbuild' and pass it to the treescript worker if it is set.

There are no automated tests that are relevant to change for this code.

====

To test locally, at the top directory of your mozilla-central clone:

Running the command `./mach taskgraph full` should run (may take several seconds) and output a list of tasks that it would generate.

If you run `./mach taskgraph full --json` before and after the patch and pipe to different files, you should be able to run a diff program against both to see what you did change, it should change only three tasks (adding dontbuild to the payload) for devedition firefox and fennec.

===

Further Reading, the taskgraph documentation at https://firefox-source-docs.mozilla.org/taskcluster/taskcluster/index.html
How to clone mozilla-central: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Source_Code/Mercurial

Comment 1

[Justin Wood (:Callek)]

Rahul, is this a continuation of work that you would be interested in finishing?

Comment 2

[rahul.shivaprasad]

Sure, I'll get cracking at it. However, you might not have tagged the right Rahul.

Comment 3

[Justin Wood (:Callek)]

Whoops, I did mis click on the person to needinfo here,  thanks.

Comment 4

[rahul.shivaprasad]

Attachment: Bug 1478883 - Tagging operations should use DONTBUILD in commit message

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

Comment 5

[rahul.shivaprasad]

Hi,

I pushed my changes to reviewboard.

I had to add dontbuild to kind.yml in release-version-bump also.

Changes were made to devedition, firefox and fennec in release-early-tagging and release-version-bump on running mach as you said it would.

Kindly review.

Comment 6

[Code Review Bot [:reviewbot]]

Comment on attachment 8995659 [details]
Bug 1478883 - Tagging operations should use DONTBUILD in commit message

https://reviewboard.mozilla.org/r/260050/#review267130


Code analysis found 1 defect in this patch:
 - 1 defect found by mozlint

You can run this analysis locally with:
 - `./mach lint path/to/file` (JS/Python)


If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: taskcluster/taskgraph/transforms/task.py:1218
(Diff revision 1)
>      if worker['push']:
>          task_def['scopes'].append(add_scope_prefix(config, 'treescript:action:push'))
>  
>      if worker.get('force-dry-run'):
>          task_def['payload']['dry_run'] = True
> +    

Error: Blank line contains whitespace [flake8: W293]

Comment 7

[Justin Wood (:Callek)]

Comment on attachment 8995659 [details]
Bug 1478883 - Tagging operations should use DONTBUILD in commit message

https://reviewboard.mozilla.org/r/260050/#review267132

Looks good!

Please fix the mozlint issue and change the if line as I suggested, removing the line from version bumps kind.yml and then we should be good to have this landed.

Thanks for the fast turnaround here!

::: taskcluster/taskgraph/transforms/task.py:1219
(Diff revision 1)
>          task_def['scopes'].append(add_scope_prefix(config, 'treescript:action:push'))
>  
>      if worker.get('force-dry-run'):
>          task_def['payload']['dry_run'] = True
> +    
> +    if worker['dontbuild']:

if you change this line to `worker.get('dontbuild')` you won't need to modify that extra kind.yml (for release-version-bump)

Comment 8

[rahul.shivaprasad]

Comment on attachment 8995659 [details]
Bug 1478883 - Tagging operations should use DONTBUILD in commit message

https://reviewboard.mozilla.org/r/260050/#review267134

Comment 9

[rahul.shivaprasad]

I fixed the issues.

Not sure what the next step is, do I push again to mozreview or the mercurial repo?

Comment 10

[rahul.shivaprasad]

Attachment: Bug 1478883 - Tagging operations should use DONTBUILD in commit message

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

Comment 11

[Justin Wood (:Callek)]

(In reply to rahul.shivaprasad from comment #9)
> I fixed the issues.
> 
> Not sure what the next step is, do I push again to mozreview or the
> mercurial repo?

In this case, again to mozreview was the right call.

Mozilla-Central generally doesn't do fix-up commits so I'm going ahead and squashing your two commits together.

As far as you pushing to the mercurial repo, the access for that is gated on "Level 3" commit access, which I'm assuming you don't have (totally normal) so I'll land this for you.

Thank you again for this patch!

Comment 12

[rahul.shivaprasad]

Great, thanks. Let me know if there's anything I can work on next.

Comment 13

[Justin Wood (:Callek)]

Attachment: Bug 1478883 - Tagging operations should use DONTBUILD in commit message

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

Comment 14

[Justin Wood (:Callek)]

Comment on attachment 8995732 [details]
Bug 1478883 - Tagging operations should use DONTBUILD in commit message

https://reviewboard.mozilla.org/r/260102/#review267140

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3ba5a7530f2c
Tagging operations should use DONTBUILD in commit message r=Callek

Comment 16

[Justin Wood (:Callek)]

Comment on attachment 8995732 [details]
Bug 1478883 - Tagging operations should use DONTBUILD in commit message

This patch will avoid unnecessary automation churn on beta/esr for releases. This allows us to save sheriff time (for not watching these pushes), and real automation dollars.

Comment 17

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/3ba5a7530f2c

Comment 18

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8995732 [details]
Bug 1478883 - Tagging operations should use DONTBUILD in commit message

NPOTB, Beta62+, ESR60.2+

Comment 19

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/releases/mozilla-beta/rev/75ba8d41ce3b

Comment 20

[Justin Wood (:Callek)]

https://hg.mozilla.org/releases/mozilla-esr60/rev/0d20abc40d2845160ae491fb28e3dc85c54374d3

Comment 21

[Tom Prince [:tomprince]]

Attachment: Bug 1478883: [release] Don't run builds when bumping versions after a release; r?Callek

Comment 22

[Tom Prince [:tomprince]]

Attachment: Bug 1478883: [release] Fix typo in treescript's `dontbuild` parameter.; r?Callek

Comment 23

[Justin Wood (:Callek)]

Comment on attachment 9005743 [details]
Bug 1478883: [release] Don't run builds when bumping versions after a release; r?Callek

Justin Wood (:Callek) has approved the revision.

Comment 24

[Justin Wood (:Callek)]

Comment on attachment 9005744 [details]
Bug 1478883: [release] Fix typo in treescript's `dontbuild` parameter.; r?Callek

Justin Wood (:Callek) has approved the revision.

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5ebac82b70a5
[release] Fix typo in treescript's `dontbuild` parameter.; r=Callek

Comment 26

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/5ebac82b70a5

Comment 27

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9005743 [details]
Bug 1478883: [release] Don't run builds when bumping versions after a release; r?Callek

Ryan VanderMeulen [:RyanVM] has approved the revision.

Comment 28

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/34feae8faf9f
[release] Don't run builds when bumping versions after a release; r=RyanVM,Callek

Comment 29

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/34feae8faf9f

Comment 30

[Tom Prince [:tomprince]]

https://hg.mozilla.org/releases/mozilla-esr60/rev/075eb21ec9a8
https://hg.mozilla.org/releases/mozilla-esr60/rev/2017ce8c7232