Closed Bug 1285608 Opened 8 years ago Closed 8 years ago

Support Android artifact builds on automation

Categories

(Testing :: General, defect, P2)

Product:
Testing
Component:
General
Type:
defect

Tracking

(firefox53 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla53
Tracking Flags:
Tracking Status
firefox53 --- fixed

People

(Reporter: armenzg, Assigned: nalexander)

References

(Blocks 1 open bug)

Details

Attachments

(3 files, 2 obsolete files)

Bug 1285608 - Part 1: Add Android moz.build and Gradle artifact build configs.
58 bytes, text/x-review-board-request
impossibus
: review+
Details
Bug 1285608 - Part 2: Make --artifact handle Android builds.
58 bytes, text/x-review-board-request
chmanchester
: review+
impossibus
: review+
Details
Bug 1285608 - Part 3: Make Android "build-like" jobs "other" or "test" in TH.
58 bytes, text/x-review-board-request
dustin
: review+
Details 
I spoke with jgriffin and we will not be commiting to this on Q3 since it is lower priority for the org.

We will mark it as P2 (we will tackle if we have time).
Attachment #8797213 - Flags: review?(mjzffr)
maja: this doesn't work, because the handling of _artifact.py doesn't handle my case (where there is 64_prefix and 64_other_prefix).  But these configurations should be correct.  Can you pick these up and make them work with --artifact?  Or, can you give me detailed instructions about how to do it myself?  It's not clear how our configuration matrix (opt vs. debug, Gradle vs. moz.build) is supposed to work with the --artifact code.
Flags: needinfo?(mjzffr)
Hey, thanks getting started with this. Forgive the delay -- I will reply later today or tomorrow.
I'm thinking we can check |self.config['build_variant']| (which might be something like api-15 or api-15-gradle) and adjust the args to |_update_build_variant| to use that as a prefix for the artifact variant [1]. For example, we want to end up calling |self._update_build_variant(rw_config, variant='api_15_artifact')|. Then we also need to add entries for 'api_15_artifact' etc to the list of build_variants [2].


[1] https://dxr.mozilla.org/mozilla-central/rev/4b9944879c9a60a9aba4a744a7401bc38e0f39c4/testing/mozharness/scripts/fx_desktop_build.py#127-129
[2] https://dxr.mozilla.org/mozilla-central/rev/da986c9f1f723af1e0c44f4ccd4cddd5fb6084e8/testing/mozharness/mozharness/mozilla/building/buildbase.py#363
Flags: needinfo?(mjzffr)
Comment on attachment 8797213 [details]
Bug 1285608 - Add Android moz.build and Gradle artifact build configs.

https://reviewboard.mozilla.org/r/82816/#review82864
Attachment #8797213 - Flags: review?(mjzffr)
Attachment #8797213 - Attachment is obsolete: true
Here's a try build showing that the Android bits are doing good things:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=732c7d1dc8e12e164413c6bae46ea7ccae47e510

Note the three build jobs -- B (buildbot), B (tc), Bg (tc) -- are all green in under 15 minutes.

Here's a try build (hopefully!) showing that the Linux bits are not broken:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=40bc384193d9d6282f2371a5bd279e7fdccde3bc
Assignee: nobody → nalexander
Status: NEW → ASSIGNED
Comment on attachment 8812518 [details]
Bug 1285608 - Part 2: Make --artifact handle Android builds.

https://reviewboard.mozilla.org/r/94228/#review94706

This looks reasonable to me. Maja may have more comments.

::: testing/mozharness/scripts/fx_desktop_build.py:143
(Diff revision 1)
> +            # We use True as a non-falsy identity placeholder.
> +            variant = c.get('artifact_flag_build_variant_in_try', True)
> +            if not variant:
> +                self.info('Artifact build requested in try syntax, but build '

This might be a bit clearer if we had an explicit `if 'artifact_flag_build_variant_in_try' in c:` that we used somewhere rather using `True` as a sentinel value.
Attachment #8812518 - Flags: review?(cmanchester) → review+
Comment on attachment 8812519 [details]
Bug 1285608 - Part 3a: Don't overload "api-15-opt" build_type.

https://reviewboard.mozilla.org/r/94230/#review94736

I'm not sure I'm the right one to review this -- I don't know what `build_type` means in this context (it's not "opt" or "debug" like in the taskgraph stuff).
Attachment #8812519 - Flags: review?(dustin)
Comment on attachment 8812520 [details]
Bug 1285608 - Part 3: Make Android "build-like" jobs "other" or "test" in TH.

https://reviewboard.mozilla.org/r/94232/#review94740
Attachment #8812520 - Flags: review?(dustin) → review+
(In reply to Chris Manchester (Offline Nov 22-28 :chmanchester) from comment #11)
> Comment on attachment 8812518 [details]
> Bug 1285608 - Part 2: Make --artifact handle Android builds.
> 
> https://reviewboard.mozilla.org/r/94228/#review94706
> 
> This looks reasonable to me. Maja may have more comments.
> 
> ::: testing/mozharness/scripts/fx_desktop_build.py:143
> (Diff revision 1)
> > +            # We use True as a non-falsy identity placeholder.
> > +            variant = c.get('artifact_flag_build_variant_in_try', True)
> > +            if not variant:
> > +                self.info('Artifact build requested in try syntax, but build '
> 
> This might be a bit clearer if we had an explicit `if
> 'artifact_flag_build_variant_in_try' in c:` that we used somewhere rather
> using `True` as a sentinel value.

This is a good suggestion.  I'll try to integrate it and see if it's better.
(In reply to Dustin J. Mitchell [:dustin] from comment #12)
> Comment on attachment 8812519 [details]
> Bug 1285608 - Part 3a: Don't overload "api-15-opt" build_type.
> 
> https://reviewboard.mozilla.org/r/94230/#review94736
> 
> I'm not sure I'm the right one to review this -- I don't know what
> `build_type` means in this context (it's not "opt" or "debug" like in the
> taskgraph stuff).

My final patch doesn't actually depend on this.  I find this whole `build_type` thing almost random -- there's opt/debug, and then some SAN-specific stuff.  Who knows where it's documented or what it means.  I can just drop this, it's not important.
I'm glad to hear I'm not the only one :)
Comment on attachment 8812517 [details]
Bug 1285608 - Part 1: Add Android moz.build and Gradle artifact build configs.

https://reviewboard.mozilla.org/r/94226/#review94924

Thanks for working on this. lgtm + a couple of suggestions

::: mobile/android/config/mozconfigs/android-api-15/artifact
(Diff revision 1)
> -# This is useful for profiling with eideticker. See bug 788680
> -STRIP_FLAGS="--strip-debug"
> +unset CXX
> +unset HOST_CC
> +unset HOST_CXX
>  
> -export MOZILLA_OFFICIAL=1
> +ac_add_options --enable-artifact-builds
> -export MOZ_TELEMETRY_REPORTING=1

You may want to try adding `ac_add_options --enable-artifact-build-symbols` to both mozconfigs. This will make the crashreporter symbols available to test jobs in case mozcrash needs to download them due to a crash. (I don't know about Android, but that's what I expect on all other platforms.)

::: testing/mozharness/mozharness/mozilla/building/buildbase.py:362
(Diff revision 1)
>          'code-coverage': 'builds/releng_sub_%s_configs/%s_code_coverage.py',
>          'source': 'builds/releng_sub_%s_configs/%s_source.py',
>          'api-15-gradle-dependencies': 'builds/releng_sub_%s_configs/%s_api_15_gradle_dependencies.py',
>          'api-15': 'builds/releng_sub_%s_configs/%s_api_15.py',
> +        'api-15-artifact': 'builds/releng_sub_%s_configs/%s_api_15_artifact.py',
>          'api-15-debug': 'builds/releng_sub_%s_configs/%s_api_15_debug.py',

Would you like to add support for api-15-debug-artifact as well? It should be the same pattern of changes as for the others.
Attachment #8812517 - Flags: review?(mjzffr) → review+
Comment on attachment 8812518 [details]
Bug 1285608 - Part 2: Make --artifact handle Android builds.

https://reviewboard.mozilla.org/r/94228/#review94940

::: testing/mozharness/scripts/fx_desktop_build.py:146
(Diff revision 1)
> -                variant = 'debug-artifact'
> +                default = 'debug-artifact'
> +
> +            # We use True as a non-falsy identity placeholder.
> +            variant = c.get('artifact_flag_build_variant_in_try', True)
> +            if not variant:
> +                self.info('Artifact build requested in try syntax, but build '

Also reorganize the three log messages to minimize repetition. Something like "Artifact build requested", a conditional "build variant has...", and "Using build variant [variant]"
Attachment #8812518 - Flags: review?(mjzffr) → review+
Attachment #8812519 - Attachment is obsolete: true
(In reply to Maja Frydrychowicz (:maja_zf) from comment #17)
> Comment on attachment 8812517 [details]
> Bug 1285608 - Part 1: Add Android moz.build and Gradle artifact build
> configs.
> 
> https://reviewboard.mozilla.org/r/94226/#review94924
> 
> Thanks for working on this. lgtm + a couple of suggestions
> 
> ::: mobile/android/config/mozconfigs/android-api-15/artifact
> (Diff revision 1)
> > -# This is useful for profiling with eideticker. See bug 788680
> > -STRIP_FLAGS="--strip-debug"
> > +unset CXX
> > +unset HOST_CC
> > +unset HOST_CXX
> >  
> > -export MOZILLA_OFFICIAL=1
> > +ac_add_options --enable-artifact-builds
> > -export MOZ_TELEMETRY_REPORTING=1
> 
> You may want to try adding `ac_add_options --enable-artifact-build-symbols`
> to both mozconfigs. This will make the crashreporter symbols available to
> test jobs in case mozcrash needs to download them due to a crash. (I don't
> know about Android, but that's what I expect on all other platforms.)

Not supported for Android.  Crash symbols are ... interesting ... on Android.  See https://treeherder.mozilla.org/logviewer.html#?job_id=32064589&repo=try#L1494.

> ::: testing/mozharness/mozharness/mozilla/building/buildbase.py:362
> (Diff revision 1)
> >          'code-coverage': 'builds/releng_sub_%s_configs/%s_code_coverage.py',
> >          'source': 'builds/releng_sub_%s_configs/%s_source.py',
> >          'api-15-gradle-dependencies': 'builds/releng_sub_%s_configs/%s_api_15_gradle_dependencies.py',
> >          'api-15': 'builds/releng_sub_%s_configs/%s_api_15.py',
> > +        'api-15-artifact': 'builds/releng_sub_%s_configs/%s_api_15_artifact.py',
> >          'api-15-debug': 'builds/releng_sub_%s_configs/%s_api_15_debug.py',
> 
> Would you like to add support for api-15-debug-artifact as well? It should
> be the same pattern of changes as for the others.

I did this for this iteration.
Comment on attachment 8812518 [details]
Bug 1285608 - Part 2: Make --artifact handle Android builds.

https://reviewboard.mozilla.org/r/94228/#review94940

> Also reorganize the three log messages to minimize repetition. Something like "Artifact build requested", a conditional "build variant has...", and "Using build variant [variant]"

I did this.
Pushed by gszorc@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7ba1a4d296a1
Part 1: Add Android moz.build and Gradle artifact build configs. r=maja_zf
https://hg.mozilla.org/integration/autoland/rev/f72e2fa55bf5
Part 2: Make --artifact handle Android builds. r=chmanchester,maja_zf
https://hg.mozilla.org/integration/autoland/rev/e26b6b6d7d98
Part 3: Make Android "build-like" jobs "other" or "test" in TH. r=dustin
https://hg.mozilla.org/mozilla-central/rev/7ba1a4d296a1
https://hg.mozilla.org/mozilla-central/rev/f72e2fa55bf5
https://hg.mozilla.org/mozilla-central/rev/e26b6b6d7d98
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox53: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: