Closed Bug 1168612 Opened 9 years ago Closed 9 years ago

prevent mach automation from uploading in taskcluster

Categories

(Release Engineering :: General, defect)

defect
Not set
normal

Tracking

(firefox41 fixed)

RESOLVED FIXED
Tracking Status
firefox41 --- fixed

People

(Reporter: dustin, Assigned: dustin)

References

Details

Attachments

(14 files, 1 obsolete file)

39 bytes, text/x-review-board-request
mrrrgn
: review+
Details
39 bytes, text/x-review-board-request
mrrrgn
: review+
Details
39 bytes, text/x-review-board-request
dustin
: review+
Details
39 bytes, text/x-review-board-request
dustin
: review+
Details
39 bytes, text/x-review-board-request
dustin
: review+
Details
39 bytes, text/x-review-board-request
mrrrgn
: review-
mrrrgn
: review+
Details
39 bytes, text/x-review-board-request
mrrrgn
: review+
Details
39 bytes, text/x-review-board-request
jlund
: review+
Details
39 bytes, text/x-review-board-request
jlund
: review+
Details
39 bytes, text/x-review-board-request
jlund
: review+
Details
570 bytes, patch
dustin
: review+
jlund
: checked-in+
Details | Diff | Splinter Review
784 bytes, patch
dustin
: review+
jlund
: checked-in+
Details | Diff | Splinter Review
40 bytes, text/x-review-board-request
jlund
: review+
Details
40 bytes, text/x-review-board-request
jlund
: review+
Details
I just saw:

21:18:58     INFO -  CHECKSUM FILE END
21:18:58     INFO -  /home/worker/workspace/build/src/obj-firefox/_virtualenv/bin/python /home/worker/workspace/build/src/build/upload.py --base-path ../../../dist \
21:18:58     INFO -  		'../../../dist/fennec-41.0a1.en-US.android-arm.apk'     '../../../dist/fennec-41.0a1.en-US.android-arm.tests.zip' '../../../dist/fennec-41.0a1.en-US.android-arm.crashreporter-symbols.zip'  '../../../dist//fennec-41.0a1.en-US.android-arm.txt' '../../../dist//fennec-41.0a1.en-US.android-arm.json' '../../../dist//fennec-41.0a1.en-US.android-arm.mozinfo.json' '../../../dist//test_packages.json' '../../../dist/jsshell-android-arm.zip'   ../../../dist/gecko-unsigned-unaligned.apk  ../../../dist/robocop.apk  ../../../dist/fennec_ids.txt  ../../../dist/geckoview_library/geckoview_library.zip  ../../../dist/geckoview_library/geckoview_assets.zip  ../../../dist/../embedding/android/geckoview_example/geckoview_example.apk  ../../../dist/geckolibs-20150526204837.aar  ../../../dist/geckolibs-20150526204837.aar.sha1  ../../../dist/geckolibs-20150526204837.pom  ../../../dist/geckolibs-20150526204837.pom.sha1  ../../../dist/ivy-geckolibs-20150526204837.xml  ../../../dist/ivy-geckolibs-20150526204837.xml.sha1  ../../../dist/geckoview-20150526204837.aar  ../../../dist/geckoview-20150526204837.aar.sha1  ../../../dist/geckoview-20150526204837.pom  ../../../dist/geckoview-20150526204837.pom.sha1  ../../../dist/ivy-geckoview-20150526204837.xml  ../../../dist/ivy-geckoview-20150526204837.xml.sha1  ../../../dist/host/bin/mar  ../../../dist/host/bin/mbsdiff \
21:18:58     INFO -  		'../../../dist//fennec-41.0a1.en-US.android-arm.checksums'
21:58:58     INFO - Automation Error: mozprocess timed out after 2400 seconds running ['python2.7', 'mach', '--log-no-times', 'build', '-v']

because the 'upload' mach automation step is running.  I tried removing the upload hostname in bug 1166380 but that crashed due to some hard-coded substituations in c['env'] in a bunch of scripts, and anyway would just have made the upload fail instead of hanging.

It's a bit of a bummer there's no log output here, by the way.

I think the fix is to set the relevant MOZ_AUTOMATION flag in the build shell script; that will pass through mozharness and mach to the mozconfig script, which will not change the existing value.
OK, still not succeeding though, I think because of 

14:52:44  WARNING - Skipping S3 file upload: No taskcluster credentials.
14:52:44    ERROR - could not determine packageUrl property to use against sendchange. Was it set after 'mach build'?
14:52:44    ERROR - could not determine packageUrl property to use against sendchange. Was it set after 'mach build'?
14:52:44  WARNING - signing disabled because MOZ_SIGNING_SERVERS is not set

also, on error we're not uploading artifacts, which is kinda lame.
I disabled sendchanges using morgan's `configs/disable_sendchanges.py`.  I'm going to work on moving all of the `_postflight_build` stuff into its own actions, though, so that we can just disable these in the task's build.sh with --no-upload, etc.
With the new actions in a pinned mozharness version

remote:   https://treeherder.mozilla.org/#/jobs?repo=try&revision=0e0454785bef
Woo

22:11:03     INFO - #####
22:11:03     INFO - ##### Skipping upload-files step.
22:11:03     INFO - #####
22:11:03     INFO - #####
22:11:03     INFO - ##### Skipping sendchange step.
22:11:03     INFO - #####

I added a little bit more to also try to disable signing (via mozharness config files)

remote:   https://treeherder.mozilla.org/#/jobs?repo=try&revision=bcb8adf9f6ac
23:12:33     INFO - # TBPL SUCCESS #
23:12:33     INFO - #####
23:12:33     INFO - ##### FxDesktopBuild summary:
23:12:33     INFO - #####
23:12:33     INFO - # TBPL SUCCESS #
23:12:33     INFO - Copying logs to upload dir...
23:12:33     INFO - mkdir: /home/worker/workspace/build/upload/logs
23:12:33    DEBUG - Copying /home/worker/logs/localconfig.json to /home/worker/workspace/build/upload/logs/localconfig.json
23:12:33    DEBUG - mkdir_p: /home/worker/workspace/build/upload/logs Already exists.
23:12:33    DEBUG - Copying /home/worker/logs/log_info.log to /home/worker/workspace/build/upload/logs/log_info.log
23:12:33    DEBUG - mkdir_p: /home/worker/workspace/build/upload/logs Already exists.
23:12:33    DEBUG - Copying /home/worker/logs/log_raw.log to /home/worker/workspace/build/upload/logs/log_raw.log
23:12:33    DEBUG - mkdir_p: /home/worker/workspace/build/upload/logs Already exists.
23:12:33    DEBUG - Copying /home/worker/logs/log_warning.log to /home/worker/workspace/build/upload/logs/log_warning.log
23:12:33    DEBUG - mkdir_p: /home/worker/workspace/build/upload/logs Already exists.
23:12:33    DEBUG - Copying /home/worker/logs/log_critical.log to /home/worker/workspace/build/upload/logs/log_critical.log
23:12:33    DEBUG - mkdir_p: /home/worker/workspace/build/upload/logs Already exists.
23:12:33    DEBUG - Copying /home/worker/logs/log_error.log to /home/worker/workspace/build/upload/logs/log_error.log
23:12:33    DEBUG - mkdir_p: /home/worker/workspace/build/upload/logs Already exists.
23:12:33    DEBUG - Copying /home/worker/logs/log_debug.log to /home/worker/workspace/build/upload/logs/log_debug.log
23:12:33    DEBUG - mkdir_p: /home/worker/workspace/build/upload/logs Already exists.
23:12:33    DEBUG - Copying /home/worker/logs/log_fatal.log to /home/worker/workspace/build/upload/logs/log_fatal.log
cleanup
+ cleanup
+ '[' -n '' ']'
[taskcluster] Artifact "public/build" not found at path
"/home/worker/artifacts/" skipping.
[taskcluster] Unsuccessful task run with exit code: 1 completed in 1891.399
seconds

I'm really not sure why the exit code is 1, but at least it says TBPL SUCCESS.
Attached patch bug1168612-mozharness.patch (obsolete) — Splinter Review
Attachment #8612894 - Flags: review?(jlund)
Bug 1168612: set MOZ_AUTOMATION_UPLOAD=0 in taskcluster; r=mrrrgn
Attachment #8612897 - Flags: review?(winter2718)
Bug 1168612: link uploads to artifacts so that we get logs even on failure; r=mrrrgn
Attachment #8612898 - Flags: review?(winter2718)
Bug 1168612: remove CCACHE variables from build.sh, as mozharness overrides them; r=mrrrgn
Attachment #8612899 - Flags: review?(winter2718)
Bug 1168612: disable sendchanges from taskcluster; r=mrrrgn
Attachment #8612900 - Flags: review?(winter2718)
Bug 1168612: disable signing and platform_supports_post_upload_to_latest via mozharness configuration files; r=mrrrgn
Attachment #8612901 - Flags: review?(winter2718)
Bug 1168612: remove unused oauth.txt; r=mrrrgn
Attachment #8612902 - Flags: review?(winter2718)
Comment on attachment 8612898 [details]
MozReview Request: Bug 1168612: link uploads to artifacts so that we get logs even on failure; r=mrrrgn

:+1
Attachment #8612898 - Flags: review?(winter2718) → review+
Comment on attachment 8612897 [details]
MozReview Request: Bug 1168612: set MOZ_AUTOMATION_UPLOAD=0 in taskcluster; r=mrrrgn

https://reviewboard.mozilla.org/r/9651/#review8413

Ship It!
Attachment #8612897 - Flags: review?(winter2718) → review+
Comment on attachment 8612902 [details]
MozReview Request: Bug 1168612: update comment on unused oauth.txt; r=mrrrgn

I'm going to run my jobs without an oauth.txt to make sure this doesn't fail them....
Comment on attachment 8612900 [details]
MozReview Request: Bug 1168612: disable sendchanges from taskcluster; r=mrrrgn

https://reviewboard.mozilla.org/r/9657/#review8419
Attachment #8612900 - Flags: review?(winter2718) → review+
Attachment #8612901 - Flags: review?(winter2718) → review+
Attachment #8612899 - Flags: review?(winter2718) → review+
Comment on attachment 8612894 [details] [diff] [review]
bug1168612-mozharness.patch

I'm running against this patch with a full-build try:
  https://treeherder.mozilla.org/#/jobs?repo=try&revision=8ed6bb6fd991
https://reviewboard.mozilla.org/r/9661/#review8433

I updated this commit to just change the comments
Comment on attachment 8612902 [details]
MozReview Request: Bug 1168612: update comment on unused oauth.txt; r=mrrrgn

Bug 1168612: update comment on unused oauth.txt; r=mrrrgn
Attachment #8612902 - Attachment description: MozReview Request: Bug 1168612: remove unused oauth.txt; r=mrrrgn → MozReview Request: Bug 1168612: update comment on unused oauth.txt; r=mrrrgn
https://reviewboard.mozilla.org/r/9653/#review8443

Updated to use the correct path (`$WORKSPACE/build/upload` instead of `$HOME/build/upload`)
Attachment #8612898 - Flags: review+ → review?(winter2718)
Comment on attachment 8612898 [details]
MozReview Request: Bug 1168612: link uploads to artifacts so that we get logs even on failure; r=mrrrgn

Bug 1168612: link uploads to artifacts so that we get logs even on failure; r=mrrrgn
Comment on attachment 8612899 [details]
MozReview Request: Bug 1168612: remove CCACHE variables from build.sh, as mozharness overrides them; r=mrrrgn

Bug 1168612: remove CCACHE variables from build.sh, as mozharness overrides them; r=mrrrgn
Attachment #8612899 - Flags: review+ → review?(winter2718)
Comment on attachment 8612900 [details]
MozReview Request: Bug 1168612: disable sendchanges from taskcluster; r=mrrrgn

Bug 1168612: disable sendchanges from taskcluster; r=mrrrgn
Attachment #8612900 - Flags: review+ → review?(winter2718)
Attachment #8612901 - Flags: review+ → review?(winter2718)
Comment on attachment 8612901 [details]
MozReview Request: Bug 1168612: disable signing and platform_supports_post_upload_to_latest via mozharness configuration files; r=mrrrgn

Bug 1168612: disable signing and platform_supports_post_upload_to_latest via mozharness configuration files; r=mrrrgn
Comment on attachment 8612902 [details]
MozReview Request: Bug 1168612: update comment on unused oauth.txt; r=mrrrgn

Bug 1168612: update comment on unused oauth.txt; r=mrrrgn
Bug 1168612: don't spoil the exit status with a trap function; r=mrrrgn
Attachment #8612963 - Flags: review?(winter2718)
Attachment #8612899 - Flags: review?(winter2718) → review+
Attachment #8612900 - Flags: review?(winter2718) → review+
Attachment #8612901 - Flags: review?(winter2718) → review+
Ugh, sorry, I was hoping mozreview wouldn't flag all of the unchanged commits as r?.  I manually carried forward three r+'s -- the other r?'s are legit.
Attachment #8612963 - Flags: review?(winter2718) → review+
Comment on attachment 8612898 [details]
MozReview Request: Bug 1168612: link uploads to artifacts so that we get logs even on failure; r=mrrrgn

C'est logique.
Attachment #8612898 - Flags: review?(winter2718) → review+
Bug 1168612: move upload-files, sendchange, and check-test to independent actions; r=jlund
Attachment #8612990 - Flags: review?(jlund)
Attachment #8612894 - Attachment is obsolete: true
Attachment #8612894 - Flags: review?(jlund)
Comment on attachment 8612902 [details]
MozReview Request: Bug 1168612: update comment on unused oauth.txt; r=mrrrgn

This breaks builds. See this task, which completes successfully until oauth is removed: https://tools.taskcluster.net/task-inspector/#lpw5-FFUTtOQBvt8pv2-vA/0 

We'll need to modify mozharness, or make a custom config instead.
Attachment #8612902 - Flags: review?(winter2718) → review-
Comment on attachment 8612902 [details]
MozReview Request: Bug 1168612: update comment on unused oauth.txt; r=mrrrgn

https://reviewboard.mozilla.org/r/9661/#review8463

Ship It!
Attachment #8612902 - Flags: review+
Regarding "don't spoil the exit status with a trap function":
  [taskcluster] Successful task run with exit code: 0 completed in 1857.481
https://reviewboard.mozilla.org/r/9693/#review8493

nice. I think it's better than these are ripped out and made into their own actions. I've added some comments below that we'll need to discuss before I'll r+

::: mozharness/mozilla/building/buildbase.py:1679
(Diff revision 1)
> +        if not self.config.get('enable_check_test'):

if we are moving this step outside of build and into its own action, we should be able to remove the 'enable_check_test' config item.

Instead, we just define the 'check-test' action to be run in the respective config files that have enable_check_test==True

::: scripts/fx_desktop_build.py:38
(Diff revision 1)
> +                'check-test',

adding these actions here tells mozharness that this script is capable of running these three. To actually avail of them, we will need to add them where we want in each platform+variant config file:

http://mxr.mozilla.org/build/search?string=default_actions&find=mozharness%2Fconfigs%2Fbuilds&findi=&filter=%5E%5B%5E%5C0%5D*%24&hitlimit=&tree=build
Comment on attachment 8612990 [details]
MozReview Request: Bug 1168612: move upload-files, sendchange, and check-test to independent actions; r=jlund

clearing my queue. r- mainly because I believe we will need to tell the configs to use the new actions.
Attachment #8612990 - Flags: review?(jlund) → review-
https://reviewboard.mozilla.org/r/9693/#review8541

> adding these actions here tells mozharness that this script is capable of running these three. To actually avail of them, we will need to add them where we want in each platform+variant config file:
> 
> http://mxr.mozilla.org/build/search?string=default_actions&find=mozharness%2Fconfigs%2Fbuilds&findi=&filter=%5E%5B%5E%5C0%5D*%24&hitlimit=&tree=build

Ah, good point.  So I'll add `upload-files`, `sendchange`, and `check-test` everywhere I see `build` that looks like it's using buildbase (and not buildb2gbase)
Bug 1168612: add new actions to all buildbase-based configs; r=jlund
Attachment #8613493 - Flags: review?(jlund)
Bug 1168612: remove `check-test` action where enable_check_test=False; r=jlund
Attachment #8613494 - Flags: review?(jlund)
Comment on attachment 8612990 [details]
MozReview Request: Bug 1168612: move upload-files, sendchange, and check-test to independent actions; r=jlund

Comments addressed in subsequent patches
Attachment #8612990 - Flags: review- → review?(jlund)
Comment on attachment 8613493 [details]
MozReview Request: Bug 1168612: add new actions to all buildbase-based configs; r=jlund

https://reviewboard.mozilla.org/r/9751/#review8623

pretty much looks good. I see some failures[1]. IIUC the configs/b2g/releng-* configs are used with b2g_build.py and is independant from BuildScript. For example, b2g/releng-emulator.py[2]

So we will need to remove those and then I think we are good to ship it.

[1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=146071751b1e&filter-resultStatus=busted
[2] http://mxr.mozilla.org/build/source/buildbot-configs/mozilla/b2g_config.py#701
Attachment #8613493 - Flags: review?(jlund)
Ah, I thought the `releng-` indicated it was ours :)
Comment on attachment 8613493 [details]
MozReview Request: Bug 1168612: add new actions to all buildbase-based configs; r=jlund

Bug 1168612: add new actions to all buildbase-based configs; r=jlund
Attachment #8613493 - Flags: review?(jlund)
Comment on attachment 8613494 [details]
MozReview Request: Bug 1168612: remove `check-test` action where enable_check_test=False; r=jlund

Bug 1168612: remove `check-test` action where enable_check_test=False; r=jlund
(In reply to Dustin J. Mitchell [:dustin] from comment #47)
> Ah, I thought the `releng-` indicated it was ours :)

they are. but they just use a different script with it's own set of actions because b2g device builds are 'built' very differently than ff.
Comment on attachment 8613493 [details]
MozReview Request: Bug 1168612: add new actions to all buildbase-based configs; r=jlund

Bug 1168612: add new actions to all buildbase-based configs; r=jlund
Comment on attachment 8613494 [details]
MozReview Request: Bug 1168612: remove `check-test` action where enable_check_test=False; r=jlund

Bug 1168612: remove `check-test` action where enable_check_test=False; r=jlund
That try run was green (well, Windows isn't done, but I'm confident)
Comment on attachment 8612990 [details]
MozReview Request: Bug 1168612: move upload-files, sendchange, and check-test to independent actions; r=jlund

https://reviewboard.mozilla.org/r/9695/#review8745

Ship It!
Attachment #8612990 - Flags: review?(jlund) → review+
Comment on attachment 8613493 [details]
MozReview Request: Bug 1168612: add new actions to all buildbase-based configs; r=jlund

https://reviewboard.mozilla.org/r/9751/#review8747

Ship It!
Attachment #8613493 - Flags: review?(jlund) → review+
Attachment #8613494 - Flags: review?(jlund) → review+
Comment on attachment 8613494 [details]
MozReview Request: Bug 1168612: remove `check-test` action where enable_check_test=False; r=jlund

https://reviewboard.mozilla.org/r/9753/#review8749

Ship It!
(In reply to Dustin J. Mitchell [:dustin] from comment #55)
> That try run was green (well, Windows isn't done, but I'm confident)

lgtm :)
08:49 <chmanchester> jlund: it looks like we turned off make check, how often are we intending to run it?
08:50 <chmanchester> jlund: my recent try builds have "12:44:55     INFO - 'enable_check_test' is false; skipping"
08:50 <jlund> which platform(s)?
08:50 <chmanchester> same on trunk trees
08:53 <chmanchester> I think https://bugzilla.mozilla.org/show_bug.cgi?id=1168612 turned them off inadvertently
08:54 <chmanchester> with http://hg.mozilla.org/build/mozharness/rev/341cc9614896
08:57 <jlund> chmanchester: yes, looks like you're right
08:57 <jlund> dustin: ^ looks like when we moved build steps into their own mozharness action we forgot to remove this part: http://mxr.mozilla.org/build/source/mozharness/mozharness/mozilla/building/buildbase.py#1716
09:00 <dustin> oh, that patch removed enable_check_tests, but didn't remove the check for it?
09:00 <dustin> indeed
Attachment #8623737 - Flags: review?(dustin)
Comment on attachment 8623737 [details] [diff] [review]
150617_remove_check_test_config_item.patch

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

__  _
       .-.'  `; `-._  __  _
      (_,         .-:'  `; `-._
    ,'o"(        (_,           )
   (__,-'      ,'o"(            )>
      (       (__,-'            )
       `-'._.--._(             )
          |||  |||`-'._.--._.-'
                     |||  |||
Attachment #8623737 - Flags: review?(dustin) → review+
Comment on attachment 8623737 [details] [diff] [review]
150617_remove_check_test_config_item.patch

remote:   https://hg.mozilla.org/build/mozharness/rev/776dda3defa1
Attachment #8623737 - Flags: checked-in+
Blocks: 1175746
I'd like to blame missing this on account of my mozreview noobness but I shouldn't. /me fails

context: https://bugzil.la/1175762
Attachment #8624004 - Flags: review?(dustin)
Attachment #8624004 - Flags: review?(dustin) → review+
Comment on attachment 8624004 [details] [diff] [review]
150617_disable_check_test_for_android.patch

remote:   https://hg.mozilla.org/build/mozharness/rev/707a511a48e2
Attachment #8624004 - Flags: checked-in+
Bug 1168612 - Temporarily disable TestMP4Reader.cpp so we can re-enable the rest of make check. r=jlund
Attachment #8624332 - Flags: review?(jlund)
Comment on attachment 8624332 [details]
MozReview Request: Bug 1168612 - Temporarily disable TestMP4Reader.cpp so we can re-enable the rest of make check. r=jlund

https://reviewboard.mozilla.org/r/11669/#review10127

Ship It!
Attachment #8624332 - Flags: review?(jlund) → review+
Comment on attachment 8624333 [details]
MozReview Request: Bug 1168612 - Bump mozharness to get check tests back. r=jlund

https://reviewboard.mozilla.org/r/11671/#review10129

Ship It!
Attachment #8624333 - Flags: review?(jlund) → review+
Component: General Automation → General
You need to log in before you can comment on or make changes to this bug.