Skip to content
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.

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

Support custom encodings on Windows through GNU libiconv #11480

Merged

Conversation

HertzDevil
Copy link
Contributor

@HertzDevil HertzDevil commented Nov 20, 2021

This PR adds support for IOs with custom encodings on Windows, using https://github.com/pffang/libiconv-for-Windows, which wraps GNU libiconv in an MSVC project.

To make things work more generically across platforms, there is now a new flag -Dwithout_iconv which disables both libiconv and the built-in iconv_* functions from the C library, plus all the associated tests; any attempt to use custom encodings will raise during runtime, similar to Windows before this PR. This is useful when the target platform has a poor iconv implementation (e.g. Android NDK) and libiconv is not yet available on that platform. The code below is all that is needed to select between libiconv and iconv at the moment:

# src/crystal/iconv.cr
{% if flag?(:win32) %}
  require "./lib_iconv"
  private USE_LIBICONV = true
{% else %}
  require "c/iconv"
  private USE_LIBICONV = false
{% end %}

Thus Windows is hardcoded to use libiconv, and other platforms will continue to use iconv, but it is easy to change this in the future. If -Dwithout_iconv is provided then src/crystal/iconv.cr itself would not be required.

The decision to distinguish LibIconv from LibC is intentional, because they are not strictly API-compatible (e.g. FreeBSD has __iconv, GNU libiconv has iconvctl).

Related: #5430

@HertzDevil HertzDevil added kind:feature platform:windows Windows support based on the MSVC toolchain / Win32 API topic:stdlib:text labels Nov 20, 2021
.github/workflows/win.yml Outdated Show resolved Hide resolved
require "c/stddef"

{% if flag?(:without_iconv) %}
{% raise "The `without_iconv` flag is preventing you to use the LibIconv module" %}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
{% raise "The `without_iconv` flag is preventing you to use the LibIconv module" %}
{% raise "The `without_iconv` flag is preventing you from using the LibIconv module" %}

@straight-shoota straight-shoota merged commit c88be0c into crystal-lang:master Nov 26, 2021
@oprypin
Copy link
Member

oprypin commented Nov 26, 2021

Every invocation of Crystal now gives a warning

iconv.lib(iconv.obj) : warning LNK4099: PDB 'libiconvStatic.pdb' was not found with 'iconv.lib(iconv.obj)' or at 'D:\a\crystal\crystal\libiconvStatic.pdb'; linking object as if no debug info
iconv.lib(localcharset.obj) : warning LNK4099: PDB 'libiconvStatic.pdb' was not found with 'iconv.lib(localcharset.obj)' or at 'D:\a\crystal\crystal\libiconvStatic.pdb'; linking object as if no debug info

https://github.com/crystal-lang/crystal/runs/4338209730?check_suite_focus=true#step:37:34

Currently that disrupts compilation of Shards
https://github.com/crystal-lang/install-crystal/runs/4338333591?check_suite_focus=true#step:4:29
https://github.com/crystal-lang/shards/blob/v0.16.0/Makefile#L27

@HertzDevil HertzDevil deleted the feature/windows-libiconv branch November 29, 2021 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:feature platform:windows Windows support based on the MSVC toolchain / Win32 API topic:stdlib:text
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants