Closed Bug 1478883 Opened Last year Closed Last year

Tagging operations should use DONTBUILD in commit message

Categories

(Firefox Build System :: Task Configuration, task)

task
Not set

Tracking

(firefox-esr60 fixed, firefox62 fixed, firefox63 fixed)

RESOLVED FIXED
mozilla63
Tracking Status
firefox-esr60 --- fixed
firefox62 --- fixed
firefox63 --- fixed

People

(Reporter: Callek, Assigned: rahul.shivaprasad, Mentored)

References

Details

(Keywords: good-first-bug, Whiteboard: [good first bug][lang=python])

Attachments

(3 files, 2 obsolete files)

+++ 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
Rahul, is this a continuation of work that you would be interested in finishing?
Flags: needinfo?(rahulch95)
Sure, I'll get cracking at it. However, you might not have tagged the right Rahul.
Whoops, I did mis click on the person to needinfo here,  thanks.
Assignee: nobody → rahul.shivaprasad
Flags: needinfo?(rahulch95)
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 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 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)
Attachment #8995659 - Flags: review?(bugspam.Callek)
Comment on attachment 8995659 [details]
Bug 1478883 - Tagging operations should use DONTBUILD in commit message

https://reviewboard.mozilla.org/r/260050/#review267134
I fixed the issues.

Not sure what the next step is, do I push again to mozreview or the mercurial repo?
(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!
Great, thanks. Let me know if there's anything I can work on next.
Attachment #8995724 - Attachment is obsolete: true
Attachment #8995724 - Flags: review?(bugspam.Callek)
Attachment #8995659 - Attachment is obsolete: true
Comment on attachment 8995732 [details]
Bug 1478883 - Tagging operations should use DONTBUILD in commit message

https://reviewboard.mozilla.org/r/260102/#review267140
Attachment #8995732 - Flags: review?(bugspam.Callek) → review+
Pushed by Callek@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/3ba5a7530f2c
Tagging operations should use DONTBUILD in commit message r=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.
Attachment #8995732 - Flags: approval-mozilla-esr60?
Attachment #8995732 - Flags: approval-mozilla-beta?
https://hg.mozilla.org/mozilla-central/rev/3ba5a7530f2c
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Comment on attachment 8995732 [details]
Bug 1478883 - Tagging operations should use DONTBUILD in commit message

NPOTB, Beta62+, ESR60.2+
Attachment #8995732 - Flags: approval-mozilla-esr60?
Attachment #8995732 - Flags: approval-mozilla-esr60+
Attachment #8995732 - Flags: approval-mozilla-beta?
Attachment #8995732 - Flags: approval-mozilla-beta+
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.
Attachment #9005743 - Flags: review+
Comment on attachment 9005744 [details]
Bug 1478883: [release] Fix typo in treescript's `dontbuild` parameter.; r?Callek

Justin Wood (:Callek) has approved the revision.
Attachment #9005744 - Flags: review+
Pushed by mozilla@hocat.ca:
https://hg.mozilla.org/integration/autoland/rev/5ebac82b70a5
[release] Fix typo in treescript's `dontbuild` parameter.; r=Callek
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.
Attachment #9005743 - Flags: review+
Pushed by mozilla@hocat.ca:
https://hg.mozilla.org/integration/autoland/rev/34feae8faf9f
[release] Don't run builds when bumping versions after a release; r=RyanVM,Callek
See Also: → 1496860
You need to log in before you can comment on or make changes to this bug.