Closed Bug 1441495 Opened 7 years ago Closed 7 years ago

Remove signing support from mozharness

Categories

(Release Engineering :: Applications: MozharnessCore, enhancement)

Product:
Release Engineering
Component:
Applications: MozharnessCore
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

(firefox61 fixed)

Status:
RESOLVED FIXED
Tracking Flags:
Tracking Status
firefox61 --- fixed

People

(Reporter: catlee, Assigned: vprabhu, Mentored)

References

(Blocks 1 open bug)

Details

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

Attachments

(1 file)

Bug 1441495 - Removed signing support from mozharness.
59 bytes, text/x-review-board-request
catlee
: review+
Details
mozharness includes support for setting up the build environment to communicate with the signing infrastructure. e.g. https://searchfox.org/mozilla-central/search?q=signing_server&path=testing/mozharness Since we're not doing any of our builds in buildbot any more, this is no longer needed. We should remove the code in mozharness that handles MOZ_SIGNING_SERVERS and MOZ_SIGN_CMD.
can i work on this issue ??
(In reply to simtmbd from comment #1) > can i work on this issue ?? Of course! Have you worked with Firefox source code or mozharness before?
(In reply to simtmbd from comment #1) > can i work on this issue ?? Let me know if you are not working on this. I have already worked on my first bug in mozharness. I will be able to take it up.
(In reply to Chris AtLee [:catlee] from comment #2) > (In reply to simtmbd from comment #1) > > can i work on this issue ?? > > Of course! Have you worked with Firefox source code or mozharness before? Hi Chris, Do I have to remove the other helper methods too or just the ones where MOZ_SIGNING_SERVERS and MOZ_SIGN_CMD is used? Also, in https://searchfox.org/mozilla-central/source/testing/mozharness/mozharness/mozilla/signing.py, is the query_moz_sign_cmd method needed? https://searchfox.org/mozilla-central/rev/61d400da1c692453c2dc2c1cf37b616ce13dea5b/testing/mozharness/mozharness/mozilla/signing.py#40
(In reply to Venkatesh Prabhu :vprabhu from comment #4) > (In reply to Chris AtLee [:catlee] from comment #2) > > (In reply to simtmbd from comment #1) > > > can i work on this issue ?? > > > > Of course! Have you worked with Firefox source code or mozharness before? > > Hi Chris, > > Do I have to remove the other helper methods too or just the ones where > MOZ_SIGNING_SERVERS and MOZ_SIGN_CMD is used? > > Also, in > https://searchfox.org/mozilla-central/source/testing/mozharness/mozharness/ > mozilla/signing.py, > > is the query_moz_sign_cmd method needed? > https://searchfox.org/mozilla-central/rev/ > 61d400da1c692453c2dc2c1cf37b616ce13dea5b/testing/mozharness/mozharness/ > mozilla/signing.py#40 That entire file can go away. Both of the mixins and the helper methods they provide aren't used any more.
(In reply to Chris AtLee [:catlee] from comment #5) > (In reply to Venkatesh Prabhu :vprabhu from comment #4) > > (In reply to Chris AtLee [:catlee] from comment #2) > > > (In reply to simtmbd from comment #1) > > > > can i work on this issue ?? > > > > > > Of course! Have you worked with Firefox source code or mozharness before? > > > > Hi Chris, > > > > Do I have to remove the other helper methods too or just the ones where > > MOZ_SIGNING_SERVERS and MOZ_SIGN_CMD is used? > > > > Also, in > > https://searchfox.org/mozilla-central/source/testing/mozharness/mozharness/ > > mozilla/signing.py, > > > > is the query_moz_sign_cmd method needed? > > https://searchfox.org/mozilla-central/rev/ > > 61d400da1c692453c2dc2c1cf37b616ce13dea5b/testing/mozharness/mozharness/ > > mozilla/signing.py#40 > > That entire file can go away. Both of the mixins and the helper methods they > provide aren't used any more. Okay. Everything else looks okay to me except the query_l10n_env method: https://searchfox.org/mozilla-central/rev/61d400da1c692453c2dc2c1cf37b616ce13dea5b/testing/mozharness/scripts/desktop_l10n.py#412 It says 'bootstrap_env is set with query_moz_sign_cmd()' which is defined in signing.py(which goes away as you have mentioned.) Should I remove the call to query_bootstrap_env()? (l10n_env.update(self.query_bootstrap_env())). If I do remove it, will query_l10n_env() get affected? This again has been called a few times in the same file.
I think you can just remove that comment...I believe the code should work still.
(In reply to Chris AtLee [:catlee] from comment #7) > I think you can just remove that comment...I believe the code should work > still. Thanks Chris. Let me know if I should run any tests. I wasn't sure, so directly submitted the changes for review.
(In reply to Venkatesh Prabhu :vprabhu from comment #9) > (In reply to Chris AtLee [:catlee] from comment #7) > > I think you can just remove that comment...I believe the code should work > > still. > > Thanks Chris. Let me know if I should run any tests. I wasn't sure, so > directly submitted the changes for review. Yeah, if you could push your changes to Try, that would be great!
(In reply to Chris AtLee [:catlee] from comment #10) > (In reply to Venkatesh Prabhu :vprabhu from comment #9) > > (In reply to Chris AtLee [:catlee] from comment #7) > > > I think you can just remove that comment...I believe the code should work > > > still. > > > > Thanks Chris. Let me know if I should run any tests. I wasn't sure, so > > directly submitted the changes for review. > > Yeah, if you could push your changes to Try, that would be great! https://treeherder.mozilla.org/#/jobs?repo=try&revision=3168c1b47544714cd286a47fd11519b6e5b81ac8
Looks like you need to clean up at least one import: [task 2018-03-06T04:43:58.926Z] Traceback (most recent call last): [task 2018-03-06T04:43:58.926Z] File "/builds/worker/workspace/build/src/testing/mozharness/scripts/fx_desktop_build.py", line 25, in <module> [task 2018-03-06T04:43:58.926Z] from mozharness.mozilla.building.buildbase import BUILD_BASE_CONFIG_OPTIONS, \ [task 2018-03-06T04:43:58.926Z] File "/builds/worker/workspace/build/src/testing/mozharness/mozharness/mozilla/building/buildbase.py", line 44, in <module> [task 2018-03-06T04:43:58.926Z] from mozharness.mozilla.signing import SigningMixin [task 2018-03-06T04:43:58.926Z] ImportError: No module named signing
(In reply to Chris AtLee [:catlee] from comment #12) > Looks like you need to clean up at least one import: > > [task 2018-03-06T04:43:58.926Z] Traceback (most recent call last): > [task 2018-03-06T04:43:58.926Z] File > "/builds/worker/workspace/build/src/testing/mozharness/scripts/ > fx_desktop_build.py", line 25, in <module> > [task 2018-03-06T04:43:58.926Z] from > mozharness.mozilla.building.buildbase import BUILD_BASE_CONFIG_OPTIONS, \ > [task 2018-03-06T04:43:58.926Z] File > "/builds/worker/workspace/build/src/testing/mozharness/mozharness/mozilla/ > building/buildbase.py", line 44, in <module> > [task 2018-03-06T04:43:58.926Z] from mozharness.mozilla.signing import > SigningMixin > [task 2018-03-06T04:43:58.926Z] ImportError: No module named signing I fixed import error in few files but one of the class params is invoking a method from signing.py in these files. https://searchfox.org/mozilla-central/source/testing/mozharness/mozharness/mozilla/building/buildbase.py#671 Here's the try results for linux platform only. https://treeherder.mozilla.org/#/jobs?repo=try&revision=6b22ca8998e8fe2eeae05ca8c26b16ca7798514c
(In reply to Venkatesh Prabhu :vprabhu from comment #13) > I fixed import error in few files but one of the class params is invoking a > method from signing.py in these files. > > https://searchfox.org/mozilla-central/source/testing/mozharness/mozharness/ > mozilla/building/buildbase.py#671 > > Here's the try results for linux platform only. > https://treeherder.mozilla.org/#/ > jobs?repo=try&revision=6b22ca8998e8fe2eeae05ca8c26b16ca7798514c Ok, great! I think we just need to remove SigningMixin everywhere now.
(In reply to Chris AtLee [:catlee] from comment #14) > (In reply to Venkatesh Prabhu :vprabhu from comment #13) > > I fixed import error in few files but one of the class params is invoking a > > method from signing.py in these files. > > > > https://searchfox.org/mozilla-central/source/testing/mozharness/mozharness/ > > mozilla/building/buildbase.py#671 > > > > Here's the try results for linux platform only. > > https://treeherder.mozilla.org/#/ > > jobs?repo=try&revision=6b22ca8998e8fe2eeae05ca8c26b16ca7798514c > > Ok, great! I think we just need to remove SigningMixin everywhere now. Done. Had to remove MobileSigningMixin too in one file. https://treeherder.mozilla.org/#/jobs?repo=try&revision=dc99ef74404812e9eb29f3c511aad64deeacebbb
Blocks: 1443938
Assignee: nobody → venkateshprabhu2
(In reply to Chris AtLee [:catlee] from comment #14) > (In reply to Venkatesh Prabhu :vprabhu from comment #13) > > I fixed import error in few files but one of the class params is invoking a > > method from signing.py in these files. > > > > https://searchfox.org/mozilla-central/source/testing/mozharness/mozharness/ > > mozilla/building/buildbase.py#671 > > > > Here's the try results for linux platform only. > > https://treeherder.mozilla.org/#/ > > jobs?repo=try&revision=6b22ca8998e8fe2eeae05ca8c26b16ca7798514c > > Ok, great! I think we just need to remove SigningMixin everywhere now. I have added the two query methods in script.py. I imported scriptmixin thinking it might be needed, but wasn't sure. Here are the try results: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f1d3d55c4ce2bd4208037ee394841cf6f4905414
(In reply to Chris AtLee [:catlee] from comment #14) > (In reply to Venkatesh Prabhu :vprabhu from comment #13) > > I fixed import error in few files but one of the class params is invoking a > > method from signing.py in these files. > > > > https://searchfox.org/mozilla-central/source/testing/mozharness/mozharness/ > > mozilla/building/buildbase.py#671 > > > > Here's the try results for linux platform only. > > https://treeherder.mozilla.org/#/ > > jobs?repo=try&revision=6b22ca8998e8fe2eeae05ca8c26b16ca7798514c > > Ok, great! I think we just need to remove SigningMixin everywhere now. I have cleaned it up and removed the import statements. Here are the linux build results: https://treeherder.mozilla.org/#/jobs?repo=try&revision=cf8e3ccf809074270899cb1bdaa5d700b9f01408 Let me know if I should run all the mochitests.
Looks good! I think https://hg.mozilla.org/try/rev/25c64c147662a43f1515127619c037e97d63b15b#l2.12 can still be removed. And yeah, let's do a full try run to make sure we haven't missed any edge cases. `mach try -b do -p all -u all` should do it.
(In reply to Chris AtLee [:catlee] from comment #18) > Looks good! > > I think > https://hg.mozilla.org/try/rev/25c64c147662a43f1515127619c037e97d63b15b#l2. > 12 can still be removed. > > And yeah, let's do a full try run to make sure we haven't missed any edge > cases. `mach try -b do -p all -u all` should do it. Here are the results: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3bdccc1147f74946df779918b8903e6c9243423c
Looks good to me! Can you post the latest version of the patch to mozreview, and then we can get it reviewed and landed!
Comment on attachment 8955741 [details] Bug 1441495 - Removed signing support from mozharness. https://reviewboard.mozilla.org/r/224820/#review233274
Attachment #8955741 - Flags: review?(catlee) → review+
Pushed by catlee@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/86d8aed616d4 Removed signing support from mozharness. r=catlee
https://hg.mozilla.org/mozilla-central/rev/86d8aed616d4
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox61: --- → fixed
Resolution: --- → FIXED
(In reply to Chris AtLee [:catlee] from comment #20) > Looks good to me! > > Can you post the latest version of the patch to mozreview, and then we can > get it reviewed and landed! Can you point to me, similar bugs if and when you come across these so that I can work on them? I will continue looking for unassigned bugs actively.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: