1561293 - [mozinfo] "get_stack_fixer_function()" fails with "ImportError: No module named fix_macosx_stack" for android debug targets

Status: RESOLVED WORKSFORME

(Testing :: Mozbase, defect, P3)

Description

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Running wpt tests for the x86_64-linux-android debug target always fails with:

Traceback (most recent call last):
File "/usr/local/Cellar/python@2/2.7.16/Frameworks/Python.framework/Versions/2.7/lib/python2.7/threading.py", line 801, in __bootstrap_inner
self.run()
File "/Users/henrik/code/gecko/testing/web-platform/tests/tools/wptrunner/wptrunner/testrunner.py", line 337, in run
with self.browser_cls(self.logger, **self.browser_kwargs) as browser:
File "/Users/henrik/code/gecko/testing/web-platform/tests/tools/wptrunner/wptrunner/browsers/fennec.py", line 89, in init
FirefoxBrowser.init(self, logger, None, prefs_root, test_type, **kwargs)
File "/Users/henrik/code/gecko/testing/web-platform/tests/tools/wptrunner/wptrunner/browsers/firefox.py", line 212, in init
self.symbols_path)
File "/Users/henrik/code/gecko/testing/mozbase/mozrunner/mozrunner/utils.py", line 277, in get_stack_fixer_function
'fix_macosx_stack')
File "/Users/henrik/code/gecko/testing/mozbase/mozrunner/mozrunner/utils.py", line 257, in import_stack_fixer_module
module = import(module_name, globals(), locals(), [])
File "/Users/henrik/code/gecko/build/mach_bootstrap.py", line 400, in call
module = self._original_import(name, globals, locals, fromlist, level)
ImportError: No module named fix_macosx_stack

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+2]]

This is actually a mozrunner problem because the method doesn't actually take the target's platform into account but only the local one. That means when you work on MacOS you will get macosx instead of linux (which is the same for Android builds). As such getting the path will fail because it doesn't exist:

https://searchfox.org/mozilla-central/rev/928742d3ea30e0eb4a8622d260041564d81a8468/testing/mozbase/mozrunner/mozrunner/utils.py#243

What would be needed is to get mozinfo updated with the data from the mozinfo.json from the objdir.

This can be done by checking the value of mozinfo.info.get("os") instead.

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+2]]

So further investigation has shown that this is a problem with mozinfo. Running wpt tests, the update() method gets called, so that the info dictionary gets updated:

{u'bin_suffix': u'', u'official': False, u'allow_legacy_extensions': True, u'pgo': False, u'sync': False, u'buildapp': u'mobile/android', u'crashreporter': False, u'devedition': False, u'require_signing': False, u'platform_guess': u'android-arm', u'appname': u'fennec', u'stylo': True, u'mozconfig': u'/Users/henrik/.mozbuild/mozconfigs/mobile-debug', u'topsrcdir': u'/Users/henrik/code/gecko', u'ubsan': False, 'os_version': StringVersion ('10.14'), 'version': 'OS X 10.14.5', 'webrender': False, u'toolkit': u'android', u'buildtype_guess': u'debug', 'bits': 64, 'has_sandbox': True, u'datareporting': True, u'artifact': True, 'automation': False, u'healthreport': True, u'updater': False, u'cc_type': None, u'asan': False, u'release_or_beta': False, u'ccov': False, u'android_min_sdk': u'16', u'tests_enabled': True, u'telemetry': False, u'tsan': False, u'nightly_build': True, u'debug': True, 'os': u'android', 'processor': u'x86_64'}

Also the global boolean values (eg. isMac) are getting updated, but the next time when mozinfo.isMac is accessed the flag has been reset. Checking mozinfo.info["os"] shows the correct information.

Usually a module gets loaded only once for a process and as such the global values should persist.

Note that the list of default platforms in choices also misses android.

Geoff, do you have any idea why the global is* variables don't persist their value? I'm a bit out of ideas.

Comment 3

[Geoff Brown [:gbrown]]

When mozinfo is initialized, I expect it to have generic information about the host os:

>>> import mozinfo
>>> mozinfo.info
{'has_sandbox': True, 'linux_distro': 'Ubuntu', 'os': 'linux', 'automation': False, 'os_version': StringVersion ('18.04'), 'version': 'Ubuntu 18.04', 'webrender': False, 'bits': 64, 'processor': 'x86_64'}
>>> mozinfo.info["os"]
'linux'
>>> mozinfo.isLinux
True
>>> mozinfo.isMac
False

after updating with a json file, I expect the json values to take precedence, but values which had been initialized and not over-written should persist:

>>> mozinfo.find_and_update_from_json("/home/gbrown/objdirs/x86_64")
'/home/gbrown/objdirs/x86_64/mozinfo.json'
>>> mozinfo.info
{u'bin_suffix': u'', u'official': False, u'allow_legacy_extensions': True, u'pgo': False, u'sync': False, u'buildapp': u'mobile/android', u'crashreporter': True, u'devedition': False, u'require_signing': False, u'platform_guess': u'android-arm', u'appname': u'fennec', u'stylo': True, u'mozconfig': u'/home/gbrown/mozconfig-x86', u'topsrcdir': u'/home/gbrown/src', u'ubsan': False, 'os_version': StringVersion ('18.04'), 'version': 'Ubuntu 18.04', 'webrender': False, u'toolkit': u'android', u'buildtype_guess': u'opt', 'bits': 64, 'has_sandbox': True, u'datareporting': True, u'artifact': False, 'automation': False, u'healthreport': True, u'updater': False, u'cc_type': u'clang', u'asan': False, u'release_or_beta': False, u'ccov': False, u'android_min_sdk': u'16', u'tests_enabled': True, 'linux_distro': 'Ubuntu', u'telemetry': False, u'tsan': False, u'nightly_build': True, u'debug': False, 'os': u'android', 'processor': u'x86_64'}
>>> mozinfo.info["os"]
u'android'
>>> mozinfo.info["os_version"]
StringVersion ('18.04')
>>> mozinfo.isLinux
True
>>> mozinfo.isMac
False

I had previously noted that isLinux is true for android environments; I assumed it was simply a reflection of the host environment, and worked around it (do not rely on mozinfo.is* for android environments, instead check mozinfo.info["os"]).

But now I see what you mean: The code at
https://searchfox.org/mozilla-central/rev/06bd14ced96f25ff1dbd5352cb985fc0fa12a64e/testing/mozbase/mozinfo/mozinfo/mozinfo.py#215
should update isLinux/isMac/etc to False once the android json is read -- but instead, those items appear to remain as they were previously. How strange!

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+2]]

So the problem here is that we are importing all is* globals by value but not by reference. As such their value outside of the module never change. It also explains why it works for info which is a dictionary and exported by reference.

I tried different solutions including module reloading to get this fixed but didn't find any way which is backward compatible. As such fixing this problem most likely would involve a backward-incompatible change. Means we should export them as functions like isMac(). There doesn't seem to be any way around it.

Geoff, let me know what you think.

Comment 5

[Geoff Brown [:gbrown]]

Would it be possible to export both isMac and isMac(), to avoid breaking existing callers?

Another approach would be to accept the existing behavior of mozinfo and instead change the conditions used in get_stack_fixer_function, something like:

if mozinfo.info.get("os") == "android":
  ...
elif mozinfo.isMac:
  ...
elif ...

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+2]]

This wouldn't stop us to ship a totally broken API here. And I think that we have to get this fixed with a new major release. I know that a lot of internal code is using it, but we should at best get it updated. External consumers most likely have the correct dependencies setup, and can be updated once upgrading to the new major release.

If you are ok with that I could do that given that I have to wait for Android emulator try builds close to half a day anyway the next days.

Comment 7

[Geoff Brown [:gbrown]]

OK. Thanks!

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+2]]

So my approach here would be to move everything into a class, and create a singleton called mozinfo, which then will be exported. Which means that for every consumer of mozinfo the following change will have to be made:

• import mozinfo

• from mozinfo import mozinfo

With that approach no other changes will be necessary to the code at all.

As Andrew mentioned to me on IRC we will have to be very careful about the import costs. Given that this module is used thousands of times in tree, we cannot accept performance loss. So performance tests will have to be made to ensure that.

Beside that we also have the attrs package vendored in tree. Based on that the to be created class could be made depend on it, and using slots might give us an additional extra performance (even it might not be that much).

Comment 9

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Reverting the summary to keep this specific failure, and I will file a new bug for the mozinfo refactoring.

Comment 10

[Geoff Brown [:gbrown]]

Removed in https://hg.mozilla.org/mozilla-central/rev/a780a89ed13f78cd89b3c200f6a5d91723a04f73