1645779 - intl/icu_sources_data.py broken after switching "mach python" to python3

Status: RESOLVED FIXED

(Core :: JavaScript: Internationalization API, defect)

Description

[Dan Horák]

After changes from bug 1621960 the intl/icu_sources_data.py script fails to run with

Updating ICU sources lists...
Traceback (most recent call last):
  File "intl/icu_sources_data.py", line 291, in <module>
    main()
  File "intl/icu_sources_data.py", line 284, in main
    update_sources(topsrcdir)
  File "intl/icu_sources_data.py", line 181, in update_sources
    for s in get_sources_from_makefile(makefile)]
  File "intl/icu_sources_data.py", line 139, in get_sources_from_makefile
    import pymake.parser
  File "/home/sharkcz/projects/firefox/build/mach_bootstrap.py", line 476, in __call__
    module = self._original_import(name, globals, locals, fromlist, level)
  File "/home/sharkcz/projects/firefox/build/pymake/pymake/parser.py", line 37, in <module>
    import data, functions, util, parserdata
  File "/home/sharkcz/projects/firefox/build/mach_bootstrap.py", line 476, in __call__
    module = self._original_import(name, globals, locals, fromlist, level)
ModuleNotFoundError: No module named 'data'

We need to run the script on big endian platforms to generate the right data file (see bug 1264836 and bug 1642184).

Comment 1

[Mike Hommey [:glandium]]

You can run python2 mach python intl/icu_sources_data.py in the meantime.

Comment 2

[Dan Horák]

(In reply to Mike Hommey [:glandium] from comment #1)

You can run python2 mach python intl/icu_sources_data.py in the meantime.

Thanks, it helps.

Comment 3

[Jeff Walden [:Waldo]]

FYI, we have documentation on updating ICU now. We discussed in bug 1642505 whether to document the use of icu_sources_data.py and largely decided to keep it simple -- invoking that subscript directly (and therefore having to think about Python 2 versus Python 3) is largely a power-user obscure feature.

It's best if you follow the steps in that file when doing updates/rebuilds of this stuff, because that document -- now that it exists -- is the source of truth on how to do things. And I for one intend to lean heavily on it so that I can forget everything I had to relearn in order to write it. :-)

Comment 4

[Axel Hecht [:Pike]]

Side note, each time I run icu_sources_data.py, I have to hack away the GNU-ism of --output-sync, 'cause on a mac, it seems to want to find bsd make instead of gnu make.

Comment 5

[Jeff Walden [:Waldo]]

I'm happy to take a patch to fix that in some sensible fashion. :-) (I think someone complained about this in another bug as well, maybe even it was you, but I don't know the platform distinctions well enough to write a patch that preserves the current behavior as far as that's possible.)

Comment 6

[Ricky Stewart]

I think there's a quick fix, testing now, with any luck I'll have a patch shortly.

Comment 7

[Ricky Stewart]

Attachment: Bug 1645779 - Make icu_sources_data.py Python 3-compliant

This removes a dependency on pymake, which is Python 2-only, and thoroughly unnecessary since we just use it to find assignments of the form OBJECTS = .... We can replicate this logic by just isolating lines that begin with that literal string, and everything else can stay the same. This is definitionally more brittle than actually using a parser, but it works fine for now, and the original implementation wasn't significantly better (it didn't handle any form of dynamism, anything more complicated than a single unconditional assignment with a space-separated list of literal strings representing outputs, etc.)

Comment 8

[Dan Horák]

My quick test shows that the change from comment 7 works fine (Fedora 32 on s390x, without any Python2).

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d6b0857e2fa1
Make icu_sources_data.py Python 3-compliant r=jwalden,anba

Comment 10

[Dorel Luca [:dluca]]

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

Comment 11

[Mathew Hodson]

This should be marked Regressed By bug 1621960. It's backwards.

Comment 12

[Dan Horák]

link updated

Comment 13

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1621960