Repository navigation
intl: VS2015: workaround ICU VS2015 failure? #2279
Description
Activity
argh, 2015 is live now, it'd be a shame to have this as the only blocker.
is it possible to say something like "to build with VS2015 you must disable icu completely" or is it broken no matter how we look at it?
to build with VS2015 you must disable icu completely
This is true at the moment. Sorry, I missed the scope of this issue before.
Working on a repro. Is the icu lib itself that fails to build or does it fail at link time? We could change the download process to overwrite ufile.c from with a version from trunk (but at that point it would be better to commit it in our repo somewhere), or we could hack-define those missing functions somewhere else either in node or with an additional file for the icu library.
Getting this error:
..\..\deps\icu\source\io\ufile.c(70): error C2109: subscript requires array or pointer type..which is at compile time, so my idea of defining the missing functions somewhere else won't help.
We should probably hack the build to use a different version of ufile.c then.
@orangemocha yes, that's the error. The particular function gets treated as "undefined assuming function returning int", and then somehow has a problem subscripting an int.
I/we/you can snag the
ufile.cfrom basically here and overwrite it in the situation ofvariables.icu_ver_major < 56and optionally&& MSVC 2015or justMSVC. I think thisufile.cwill work for ICU 54 and 55. And it's already checked in to the 56 stream.@orangemocha thanks for the discussion. I will open a PR here for a workaround.
oops, was looking for the Intl label!
- added a commit that references this issue
on Aug 14, 2015 Fixed by 4c06515.
- added a commit that references this issue
on Aug 24, 2015 - added a commit that references this issue
on Nov 4, 2016
ICU (which provides
Intl) failed under VS2015 in the same way and for the same reason as openssl in #478. Should we try to workaround?I already committed a fix to IcuBug:11689, scheduled for release in a couple of months with ICU 56. If we hit this issue earlier, it looks like we could just pull a copy of
ufile.cfrom ICU trunk on top of the downloaded ICU.Just filing this for reference.
@rvagg @orangemocha