Node: Preprocessing step for ICU 59.1 on z (was: “some platforms”)

[srl295]

Subsystem: deps

ICU 59.1 ( per https://github.com/nodejs/node/pull/12486 ) needs a preprocessing step ( see here to convert u'文'; to u'\u6587';, because ICU's _source code_ is now UTF-8.

The escaper itself is (currently) an ICU C++ tool, written using a subset of ICU's source code (only header files) that does not need any escaping or data. So it is a single .cpp file built into a single executable.

Platforms this is known to be needed on are:

• IBM: AIX , z/OS (using xlC++)
• Oracle: Solaris (using Oracle Studio 12.5 which is now the minimum supported version )
  edit this is only for z, see discussion

(if gcc or clang are used, there’s no issue.)

ICU4C handles all of this by fun with makefiles. I think for node’s build, it might be best done at configure time:

• Actually compile the single binary
• run all source code through to create the normal temporary node/deps/icu4c directory but with preprocessed source as well as original source
• the temporary icu source is now compilable directly by the gyp-based toolchain without any additional issues.

the preprocessed source has #line directives in it so that compile errors, etc, show the original source names.

The escaper steps are basically:

• escapesrc < somefile.cpp > _somefile.cpp
• $(CC) -c _somefile.cpp …  (the usual build)

[bnoordhuis]

Groan... the ICU build is already a twisty little maze of scripts and hacks all alike and now you want to add another step?

Platforms this is known to be needed on are:

IBM: AIX, z/OS (using xlC++)
Oracle: Solaris (using Oracle Studio 12.5 which is now the minimum supported version ).

Easy workaround: let them eat cake^W^Wuse gcc. We don't support those compilers.

I think for node’s build, it might be best done at configure time:

Strongly disagree. It _might_ be acceptable when updating the in-tree copy but I'm not convinced it's necessary in the first place.

[refack]

😭 _{https://github.com/nodejs/node/pull/12218}

It might be acceptable when updating the in-tree copy but I'm not convinced it's necessary in the first place.

👍 for floating this as a patch to the in tree copy (if necessary)

[gibfahn]

IBM: AIX

We use gcc on AIX (see the setup instructions).

Oracle: Solaris

Not a supported platform, and I'm pretty sure SmartOS uses gcc (cc/ @misterdjules)

IBM: z/OS (using xlC++)

cc/ @joransiu @jBarz @jbajwa re/ z/OS for whether the C++ compiler used for Node requires this.

[sam-github]

I'm having trouble finding the escapesrc cpp source, couldn't it be done with a couple lines of python (already a build dep) instead of c++? And its not clear to me why the fixup isn't done before committing to github rather than on every single build, this is what @refack and others call "floating a patch", right?

[misterdjules]

@gibfahn

and I'm pretty sure SmartOS uses gcc (cc/ @misterdjules)

That's correct, GCC is used to build node on SmartOS.

[refack]

I'm having trouble finding the escapesrc cpp source, couldn't it be done with a couple lines of python (already a build dep) instead of c++? And its not clear to me why the fixup isn't done before committing to github rather than on every single build, this is what @refack and others call "floating a patch", right?

👍 on both points.

[srl295]

Groan... the ICU build is already a twisty little maze of scripts and hacks all alike and now you want to add another step?

At least they are more alike now… but yeah. 🙁

• Re @misterdjules and @gibfahn : Glad to hear that Solaris and AIX should be good to go. 😌

• that leaves z/OS?

Source code: escapesrc/

• escapesrc/escapesrc.cpp including escapesrc /cptbl.h
• sample files: before and after

per @sam-github

And its not clear to me why the fixup isn't done before committing to github rather than on every single build

Because for _most_ users, the code does not need to change. It seems like the fewer files that change between deps/ and upstream, the better. But i could go either way (if this is even needed).

In my implementation, I don't change any files in-place, but only cache altered copies right at the point of compilation. That won't go over well with gyp, perhaps.

couldn't it be done with a couple lines of python (already a build dep) instead of c++?

Yes, technically. That's an option.

And all, sorry to not put more context around this (and then to go on leave right after!) — I mainly wanted to make sure this was captured as a possible issue.

[joransiu]

Re: z/OS -- There is no GCC on the platform. XL C/C++ compiler does not currently support unicode source or actual unicode in the source. To build ICU component on z/OS, we will need the preprocessor step that @srl295 mentioned.

[Trott]

@srl295 Should this remain open? If so, has anything changed that's worth noting?

[apapirovski]

I'm going to close this out given the lack of a follow up since the last time @Trott resurrected this. Feel free to reopen if you've got something more current.