1473268 - Tagging operations should use DONTBUILD in commit message

Status: RESOLVED FIXED

(Release Engineering :: Release Automation: Other, enhancement)

Description

[Nick Thomas [:nthomas] (UTC+12)]

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

Comment 1

[Rail Aliiev [:rail]]

Just need to be careful when tagging and bumping happen at the same time - the top commit shouldn't use DONTBUILD, so we get builds when we bump the version.

Comment 2

[Nick Thomas [:nthomas] (UTC+12)]

That's a good point! Looks like the ordering is already good though, eg https://hg.mozilla.org/releases/mozilla-release/pushloghtml?fromchange=59666a63d700&tochange=95df3e70835a has the tag earlier than the version bump.

Comment 3

[Justin Wood (:Callek)]

Adding goodfirstbug and mentor=me

https://github.com/mozilla-releng/treescript/

The plan in my mind is to allow the treescript to accept an optional (default to false) boolean param of "dontbuild" which will append to the commit message " DONTBUILD"

Then there will be an in-tree set of code changes that we need to do to take advantage of that, but lets get the treescript changes first then I'll spin out a new bug for the in-tree.

Comment 4

[Justin Wood (:Callek)]

To elaborate a successful patch would modify:

https://github.com/mozilla-releng/treescript/blob/master/treescript/data/treescript_task_schema.json

To add the new parameter, then modify TAG_MSG in https://github.com/mozilla-releng/treescript/blob/61fedcbaeae32abe417ce9824c638c34dae5b619/treescript/mercurial.py to provide DONTBUILD if required.

Similarly should add DONTBUILD in https://github.com/mozilla-releng/treescript/blob/4c8ba8e0373eb241838c73f52d3056c6dbd8f5e1/treescript/versionmanip.py#L90 if required.

The dontbuild functionality can be pulled out to a helper function, at authors discretion.

All modified should have code coverage (tested on PRs) of 100% with pytest, https://github.com/mozilla-releng/treescript/tree/4c8ba8e0373eb241838c73f52d3056c6dbd8f5e1/treescript/test  You can run with tox locally.

Comment 5

[theBugger]

Hi, I am new to this. Can anyone help me in picking up this bug?

Comment 6

[Justin Wood (:Callek)]

(In reply to theBugger from comment #5)
> Hi, I am new to this. Can anyone help me in picking up this bug?

I'm happy to have you do so, (I will assign you once I see the first PR).

What should happen is you should clone treescript (https://github.com/mozilla-releng/treescript)

Once locally cloned you can run python's tox (with python3.6 in PATH) to validate that the tests pass on your local system.

Once they pass the basic information is in comment 3 and comment 4, if you need anything more I can go into more specific on the varying points.

Thank you again for offering to take this!

Comment 7

[Devika Sugathan]

Hi
I would like to be assigned this as my first bug! Could you please point me in the right direction of how to start.

Comment 8

[Devika Sugathan]

Hi
I would like to be assigned this as my first bug! Could you please point me in the right direction of how to start.

Comment 9

[Justin Wood (:Callek)]

Hi Devika, please see the comments above for how to begin (https://bugzilla.mozilla.org/show_bug.cgi?id=1473268#c3 and https://bugzilla.mozilla.org/show_bug.cgi?id=1473268#c6) since there is no Pull Request in github yet, this bug is still up for grabs.

The first person to submit a PR will be assigned this bug.

(Work In progress PR's are ok)

I'm more than willing to provide further guidance and assistance in either this bug or IRC (irc.mozilla.org channel: #ci)

Comment 10

[rahul.shivaprasad]

Hi Justin, I have create a PR. Not sure if my implementation is correct. Let me know if any changes are necessary.

Also please assign this bug to me so that I can contribute further under your guidance.

Thanks

Comment 11

[Justin Wood (:Callek)]

Thank you for the patch. I'll review it within the next 24 hours.

Comment 12

[rahul.shivaprasad]

Hi, I've made the requested changes. Kindly review.

Comment 13

[Justin Wood (:Callek)]

(In reply to rahul.shivaprasad from comment #12)
> Hi, I've made the requested changes. Kindly review.

Thank You, I had reviewed on the github Pull Request, this is very close now... Feel free to reach out to me if there are any issues or confusion.

Comment 14

[rahul.shivaprasad]

Hi, I was ill and couldn't work on it for the past few days. I've tried to add the required tests. Please let me know if any changes are necessary.

Thank you.

Comment 15

[Justin Wood (:Callek)]

(In reply to rahul.shivaprasad from comment #14)
> Hi, I was ill and couldn't work on it for the past few days. I've tried to
> add the required tests. Please let me know if any changes are necessary.
> 
> Thank you.

No worry about being ill (happens to all of us) -- The changes do look good, I commented on a way I would have done differently were I writing it, but I have no issue accepting as-is.

Are you interested in working in mercurial on mozilla-central to add this support to the taskgraph itself (its a different set of work) [docs for the overall taskgraph stuff is at https://firefox-source-docs.mozilla.org/taskcluster/taskcluster/index.html ]

Otherwise I can point you at other treescript stuff I was thinking about (and have to file).

I can also help you find other pieces of work if you have a specific interest, even if I am not the one who will be mentoring you.

Thank you again for your contributions here!

Comment 16

[rahul.shivaprasad]

I thought I implemented it the way you suggested ( I'm a novice at testing and would love to know the right way to have implemented it. ) 

I've been wanting to contribute to open source for a while now and have always been hitting walls; working with you as my mentor has been great. I'd love to add this support to taskgraph or work on any other treescript stuff, I'll go through the link you provided. Just let me know how I can be helpful.

You can email me at [e-mail].

Thank you.

Comment 17

[Justin Wood (:Callek)]

This has merged to `master` on github and I have deployed a 1.1.0 version to production.

(In reply to rahul.shivaprasad from comment #16)
> I thought I implemented it the way you suggested ( I'm a novice at testing
> and would love to know the right way to have implemented it. ) 

I didn't specify anything too specific, so this was perfectly acceptable.

What I would have done was utilize the pytest feature documented at https://docs.pytest.org/en/latest/parametrize.html rather than duplicate the test code. But I had no issue taking your patch as it was, it was still correct.

> I'd love to add this support to taskgraph or work on any other treescript stuff,
> I'll go through the link you provided. Just let me know how I can be helpful.

Awesome! I have just filed Bug 1478883 and sent you off a needinfo there to see if it's one you want to do. Alternatively (before I file them in bugzilla) there is also the list of issues on github https://github.com/mozilla-releng/treescript/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22

These issues are a bit wider of complexity, needing different skills. And I'm sure I have more to specify on how to do some of them, but I'd also be more than happy to have you work on any of them.

> You can email me at <snip>

Similarly you can e-mail me at my personal address (my Callek at gmail) or my work address (callek at mozilla dot com)

> 
> Thank you.

No, indeed thank you, truly a help!

Comment 18

[Mihai Tabara [:mtabara]⌚️GMT]

Hm, seems like this still needs some massaging a bit. 

On 62.0b15 it still didn't have DONTBUILD. Not sure if the treescript was up-to-date or not, but it pushed this[1]. On staging betas testing it's behaving the same[2].

Example task in beta[3] while on maple [4], they are identical.
For reference the `dontbuild` vs `dont_build` in the payload is okay as you set as "dontbuild" in transforms but then overwrite in the task transforms to `dont_build`[5][6]

[1]: https://hg.mozilla.org/releases/mozilla-beta/rev/4ce39b1ba56bd20f42be0962eb0f6e977d7e63e6
[2]: https://hg.mozilla.org/projects/maple/rev/df93015d6408
[3]: https://tools.taskcluster.net/groups/DmKeYx0FSCCAFwAbJA3HBg/tasks/OLdyobECSO-_jNXlE7TDeQ/details
[4]: https://tools.taskcluster.net/groups/McVKblMLQ7GbXFerPIpQdQ/tasks/E8Ut4-v2RMGIlV0sRpTc0A/details
[5]: https://hg.mozilla.org/projects/maple/file/tip/taskcluster/taskgraph/transforms/task.py#l1292
[6]: https://hg.mozilla.org/releases/mozilla-beta/file/tip/taskcluster/taskgraph/transforms/task.py#l1210

Comment 19

[Mihai Tabara [:mtabara]⌚️GMT]

Dropping a NI to [e-mail] as well.

Comment 20

[Justin Wood (:Callek)]

Ok, Rahul, I've changed my thinking about this, which might make this easier...

#1 we *unconditionally* add DONTBUILD to the commit message for all tagging operations (this is ok because we do version bump after it).

#2 we remove the taskgraph in tree code which supports this.

My Reasoning is because when the version was already bumped we still tag but don't do a version bump commit, we also don't need builds on that push either though the dontbuild flag is per-task. And we pretty much never need to build on a tagging-alone push.

Thoughts?

Comment 21

[Agboola Mukhtar <agbo-ola>]

Hi, i'd like to work on this issue if no one is working on it

Comment 22

[Nick Thomas [:nthomas] (UTC+12)]

Thanks for the offer, but this issue was fixed in bug 1478883 a while ago.