1302774 - Rename the "cross-opt" macosx config variant to "cross-opt-st-an"

Status: RESOLVED FIXED

(Release Engineering :: Applications: MozharnessCore, defect)

Description

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

Despite the name, this config variant runs a static analysis build.

We refer to this variant in the mac os X static analysis task description, so fixing this will require renaming that reference as well as renaming the config variant file.

Following the threads that connect those two things is part of what makes this a good-first-bug :)

Comment 1

[mistry.1]

Hello Dustin,
I would like to take this as my first bug. So can you please tell me the bug in detail and what I have to do.

Thank You!

Comment 2

[mistry.1]

(In reply to Dustin J. Mitchell [:dustin] from comment #0)
> Despite the name, this config variant runs a static analysis build.
> 
> We refer to this variant in the mac os X static analysis task description,
> so fixing this will require renaming that reference as well as renaming the
> config variant file.
> 
> Following the threads that connect those two things is part of what makes
> this a good-first-bug :)
Hello Dustin,
I would like to take this as my first bug. So can you please tell me the bug in detail and what I have to do.

Thank You!

Comment 3

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

Welcome!

As I said in comment 0, part of what makes this a good first bug is that it requires a lot of connecting things together.  Once you've made those connections, you'll have a good understanding of how task-graph generation works.

So, to get started, there's a macosx64 static anlaysis job defined here;

https://dxr.mozilla.org/mozilla-central/source/taskcluster/ci/static-analysis/kind.yml

and on line 40 it specifies:
https://dxr.mozilla.org/mozilla-central/source/taskcluster/ci/static-analysis/kind.yml#40
            custom-build-variant-cfg: cross-opt

This generates a task like this one
  https://tools.taskcluster.net/task-inspector/#b1c5XzTPTMO4FE2teghMKQ/
Which eventually runs Mozharness with --custom-build-variant-cfg=cross-opt; seen in the task run log at
  https://public-artifacts.taskcluster.net/b1c5XzTPTMO4FE2teghMKQ/0/public/logs/live_backing.log
Which mozharness uses to find
  https://dxr.mozilla.org/mozilla-central/source/testing/mozharness/configs/builds/releng_sub_mac_configs/64_cross_opt.py

which includes
    'src_mozconfig': 'browser/config/mozconfigs/macosx64/opt-static-analysis',

The confusing bit here is that the name "cross-opt" suggests an optimized cross compile, but in fact it's a static analysis build (because of that src_mozconfig).  So the fix is to rename the variant config to cross-opt-st-an.

Hopefully that gives you enough to start your explorations.  It might be helpful for you to explore how all of this works, bit by bit, and write your findings down in this bug.  If you get stuck, let me know and I can offer some help.

Comment 4

[Steven Ellul]

Attachment: Patch file

Hi,

I think this is the fix you are looking for. Let me know if I've misunderstood.

Thanks

Comment 5

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

Comment on attachment 8812883 [details] [diff] [review]
Patch file

Review of attachment 8812883 [details] [diff] [review]:
-----------------------------------------------------------------

That's a start!  However, there is no `https://dxr.mozilla.org/mozilla-central/source/testing/mozharness/configs/builds/releng_sub_mac_configs/64_cross_opt_st_an.py` so the task will fail.  You'll need to make some changes under `testing/mozharness`, too.  Keep digging :)

::: taskcluster/ci/static-analysis/kind.yml
@@ +39,1 @@
>              # enables static analysis.

You can remove this comment now, since "st-an" is at least a reasonable name for something that enables static analysis.

Comment 6

[Steven Ellul]

(In reply to Dustin J. Mitchell [:dustin] from comment #5)
> Comment on attachment 8812883 [details] [diff] [review]
> Patch file
> 
> Review of attachment 8812883 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> That's a start!  However, there is no
> `https://dxr.mozilla.org/mozilla-central/source/testing/mozharness/configs/
> builds/releng_sub_mac_configs/64_cross_opt_st_an.py` so the task will fail. 
> You'll need to make some changes under `testing/mozharness`, too.  Keep
> digging :)
> 
> ::: taskcluster/ci/static-analysis/kind.yml
> @@ +39,1 @@
> >              # enables static analysis.
> 
> You can remove this comment now, since "st-an" is at least a reasonable name
> for something that enables static analysis.

Thanks for quick feedback, will have a look

Comment 7

[Steven Ellul]

Attachment: Patch attempt 2

Used ack-grep to search for any other uses of this to be more thorough

Comment 8

[Steven Ellul]

(In reply to Steven Ellul from comment #7)
> Created attachment 8812984 [details] [diff] [review]
> Patch attempt 2
> 
> Used ack-grep to search for any other uses of this to be more thorough

I cannot see the change in diff but have also changed the filename you mentioned in your feedback

Comment 9

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

Comment on attachment 8812984 [details] [diff] [review]
Patch attempt 2

Did you use `hg mv` to rename the file?  I can do that locally, too.  The relevant lines in my `hg export` are:

diff --git a/testing/mozharness/configs/builds/releng_sub_mac_configs/64_cross_opt.py b/testing/mozharness/configs/builds/releng_sub_mac_configs/64_cross_opt_st_an.py
rename from testing/mozharness/configs/builds/releng_sub_mac_configs/64_cross_opt.py
rename to testing/mozharness/configs/builds/releng_sub_mac_configs/64_cross_opt_st_an.py

By the way, you may want to look into using mozreview -- it's a much nicer way of requesting review:

  https://mozilla-version-control-tools.readthedocs.io/en/latest/mozreview-user.html

I'll push the result to try to see how it works and grant r+ if that's good.

Comment 10

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

https://treeherder.mozilla.org/#/jobs?repo=try&revision=2d68f8762a5a

Comment 11

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

looks good!!

Comment 12

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/d9206ed568ce93d5d8505b594011f1c69047e730
Bug 1302774: rename cross-opt to cross-opt-st-an; r=dustin

Comment 13

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

Awesome work -- what would you like to hack on now?

Comment 14

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/d9206ed568ce

Comment 15

[Steven Ellul]

(In reply to Dustin J. Mitchell [:dustin] from comment #9)
> Comment on attachment 8812984 [details] [diff] [review]
> Patch attempt 2
> 
> Did you use `hg mv` to rename the file?  I can do that locally, too.  The
> relevant lines in my `hg export` are:
> 
> diff --git
> a/testing/mozharness/configs/builds/releng_sub_mac_configs/64_cross_opt.py
> b/testing/mozharness/configs/builds/releng_sub_mac_configs/
> 64_cross_opt_st_an.py
> rename from
> testing/mozharness/configs/builds/releng_sub_mac_configs/64_cross_opt.py
> rename to
> testing/mozharness/configs/builds/releng_sub_mac_configs/64_cross_opt_st_an.
> py
> 
> By the way, you may want to look into using mozreview -- it's a much nicer
> way of requesting review:
> 
>  
> https://mozilla-version-control-tools.readthedocs.io/en/latest/mozreview-
> user.html
> 
> I'll push the result to try to see how it works and grant r+ if that's good.

Thanks for the heads up,  it does look like a much nicer way to push changes, only issue is that I am doing this for a final year Computer Science module and have been told I have to submit through bugzilla to make marking simpler for them.

Comment 16

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

If you wouldn't mind putting the instructors in touch with me, that would be great.

Comment 17

[Steven Ellul]

(In reply to Dustin J. Mitchell [:dustin] from comment #16)
> If you wouldn't mind putting the instructors in touch with me, that would be
> great.

I spoke to my tutor and what I said wasn't quite correct. It wasn't the case that we can't use MozReview, just that they were unaware we could use it and do not know how (I think). I will email a link of this page to my tutor so he can look into it.