Skip to content

Support shared builds of Chakra.ICU #4737

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
chakrabot merged 1 commit into from
Feb 28, 2018
Merged

Support shared builds of Chakra.ICU #4737

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions Build/Chakra.Build.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,15 @@
%(PreprocessorDefinitions);
INTL_ICU=1
</PreprocessorDefinitions>
<PreprocessorDefinitions Condition="'$(ChakraICU)'=='static'">
%(PreprocessorDefinitions);
U_STATIC_IMPLEMENTATION=1
Copy link
Contributor

@dilijev dilijev Feb 24, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

U_STATIC_IMPLEMENTATION=1 [](start = 8, length = 25)

Ah, I missed this. Thanks

Copy link
Contributor

@dilijev dilijev Feb 24, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not that I expect conflicts on this define name, but this exposes this define to every compilation unit (as opposed to ICU-related-only), no?

In reply to: 170405597 [](ancestors = 170405597)

Copy link
Contributor Author

@jackhorton jackhorton Feb 24, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but I am of the opinion that it is much easier in the long run to not keep ICU integration points separate. While its theoretically possible that we will need to change globalization libraries yet again, in the meantime it is much nicer to be able to use the recycler/common data structures right next to ICU calls. So, in that sense, its good to make the entire codebase aware of ICU in this way.

</PreprocessorDefinitions>
<PreprocessorDefinitions Condition="'$(UseICU)'=='true'">
U_DISABLE_RENAMING=1; <!-- Disable renaming to maintain compatibility with Windows Kit ICU's icuuc/icuin.lib -->
ICU_VERSION=$(IcuVersionMajor);
%(PreprocessorDefinitions)
</PreprocessorDefinitions>
<PreprocessorDefinitions Condition="'$(ForceSWB)'=='true'">
%(PreprocessorDefinitions);
GLOBAL_ENABLE_WRITE_BARRIER=1
Expand Down
2 changes: 1 addition & 1 deletion deps/Chakra.ICU/Chakra.ICU.Build.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
UCONFIG_NO_REGULAR_EXPRESSIONS=1;
UCONFIG_NO_SERVICE=1;
%(PreprocessorDefinitions)
</PreprocessorDefinitions>
</PreprocessorDefinitions>

<!-- Default ICU Configuration (see source\common\common.vcxproj) -->
<!-- Note that we already configure most of what common.vcxproj handles elsewhere -->
Expand Down
3 changes: 3 additions & 0 deletions deps/Chakra.ICU/Chakra.ICU.Common.vcxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@
U_COMMON_IMPLEMENTATION=1
</PreprocessorDefinitions>
</ClCompile>
<Link Condition="'$(ChakraICU)'=='shared'">
<SubSystem>Console</SubSystem>
</Link>
</ItemDefinitionGroup>
<ItemGroup Condition="'$(ChakraICU)'!='false'">
<ClCompile Include="$(IcuCommonSources)" />
Expand Down
6 changes: 6 additions & 0 deletions deps/Chakra.ICU/Chakra.ICU.Data.vcxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,12 @@
<_ProjectFileVersion>10.0.30319.1</_ProjectFileVersion>
<GenCCodePath>$(OutBaseDir)\bin\$(PlatformPathName.ToLower())_$(Configuration.ToLower())\Chakra.ICU.GenCCode.exe</GenCCodePath>
</PropertyGroup>
<ItemDefinitionGroup>
<Link Condition="'$(ChakraICU)'=='shared'">
<SubSystem>Console</SubSystem>
<NoEntryPoint>true</NoEntryPoint>
</Link>
</ItemDefinitionGroup>
<ItemGroup Condition="'$(ChakraICU)'!='false'">
<None Include="$(IntDir)icudt$(IcuVersionMajor)l_dat.obj" /> <!-- forces the automagic build system to create a lib from the obj -->
<CustomBuild Include="$(MSBuildThisFileDirectory)source\data\in\icudt$(IcuVersionMajor)l.dat">
Expand Down
9 changes: 8 additions & 1 deletion deps/Chakra.ICU/Chakra.ICU.Stubdata.vcxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@
<ProjectName>Chakra.ICU.Stubdata</ProjectName>
</PropertyGroup>
<PropertyGroup Label="Configuration">
<ConfigurationType>StaticLibrary</ConfigurationType>
<ConfigurationType Condition="'$(ChakraICU)'!='shared'">StaticLibrary</ConfigurationType>
<ConfigurationType Condition="'$(ChakraICU)'=='shared'">DynamicLibrary</ConfigurationType>
</PropertyGroup>
<Import Project="$(BuildConfigPropsPath)Chakra.Build.Default.props" />
<Import Project="$(VCTargetsPath)\Microsoft.Cpp.Default.props" />
Expand All @@ -31,6 +32,12 @@
$(IcuSourceDirectory)\common
</AdditionalIncludeDirectories>
</ClCompile>
<Link Condition="'$(ChakraICU)'=='shared'">
<SubSystem>Console</SubSystem>
<!-- Make Chakra.ICU.Stubdata pretend like its Chakra.ICU.Data for linking purposes -->
<OutputFile>$(OutDir)\Chakra.ICU.Data.dll</OutputFile>
<ImportLibrary>$(OutDir)\Chakra.ICU.Data.lib</ImportLibrary>
Copy link
Contributor Author

@jackhorton jackhorton Feb 23, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@curtisman is there any way around this? At runtime, we want Common.dll to auto-load Data.dll, but to create Data.dll we need Common. Thus, Stubdata is built and common links against Stubdata.lib... normally. Except then Common will try to auto-load Stubdata.dll rather than Data.dll, which isn't what we want. I couldn't find a way to work around this, and having the stubdata project output misnamed files like this is what stubdata.vcxproj in upstream ICU does.

</Link>
</ItemDefinitionGroup>
<ItemGroup Condition="'$(ChakraICU)'!='false'">
<ClCompile Include="$(IcuStubdataSources)" />
Expand Down
7 changes: 6 additions & 1 deletion deps/Chakra.ICU/Chakra.ICU.i18n.vcxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,18 @@
<!-- Some ICU files use embedded UTF-8 -->
<AdditionalOptions>/utf-8 %(AdditionalOptions)</AdditionalOptions>
</ClCompile>
<Link Condition="'$(ChakraICU)'=='shared'">
<SubSystem>Console</SubSystem>
</Link>
</ItemDefinitionGroup>
<ItemGroup Condition="'$(ChakraICU)'!='false'">
<ClCompile Include="$(IcuI18nSources)" />
<ClInclude Include="$(IcuI18nHeaders)" />
<ProjectReference Include="$(MSBuildThisFileDirectory)Chakra.ICU.Stubdata.vcxproj">
<Project>{E14F373D-05A0-4259-A5E9-AFE8405FB847}</Project>
<ReferenceOutputAssembly>false</ReferenceOutputAssembly>
</ProjectReference>
<ProjectReference Include="$(MSBuildThisFileDirectory)Chakra.ICU.Common.vcxproj">
<Project>{EE2A3111-4D85-427C-B0AB-E6B0EA7FFB44}</Project>
</ProjectReference>
</ItemGroup>
<Import Project="$(BuildConfigPropsPath)Chakra.Build.targets" Condition="exists('$(BuildConfigPropsPath)Chakra.Build.targets')" />
Expand Down
13 changes: 13 additions & 0 deletions lib/Common/CommonDefines.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,19 @@
#define ENABLE_UNICODE_API 1 // Enable use of Unicode-related APIs
#endif

// ICU Configuration
#if defined(HAS_ICU) || defined(HAS_REAL_ICU)
#define U_SHOW_CPLUSPLUS_API 0
// ICU 55 (Ubuntu 16.04 system default) has uloc_toUnicodeLocale* marked as draft, which is required for Intl
#if ICU_VERSION > 56
#define U_DEFAULT_SHOW_DRAFT 0
#define U_HIDE_DRAFT_API 1
#endif
#define U_HIDE_DEPRECATED_API 1
#define U_HIDE_OBSOLETE_API 1
#define U_HIDE_INTERNAL_API 1
#endif

// Language features

#if !defined(CHAKRACORE_LITE) && (defined(_WIN32) || defined(INTL_ICU))
Expand Down
2 changes: 0 additions & 2 deletions lib/Runtime/PlatformAgnostic/ICU.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@
#ifdef WINDOWS10_ICU
#include <icu.h>
#else
#define U_STATIC_IMPLEMENTATION 1
Copy link
Contributor

@dilijev dilijev Feb 24, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this not still needed if building non-shared? (I didn't see this added to CommonDefines.h or anywhere else)

Copy link
Collaborator

@jefgen jefgen Feb 24, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like Chakra.Build.props sets the define, but I don't know if you build with both static and dynamic linking of ICU or not..?

Copy link
Contributor Author

@jackhorton jackhorton Feb 24, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We build with one or the other at any given time. So, the build files set this if ChakraICU=static now.

#define U_SHOW_CPLUSPLUS_API 0
#include "unicode/ucal.h"
#include "unicode/ucol.h"
#include "unicode/udat.h"
Expand Down