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

AlpineLinux and musl-libc fixes #3986

Merged
merged 2 commits into from Feb 6, 2017

Conversation

ysbaddaden
Copy link
Contributor

fixes #3974
fixes #3980

@@ -8,7 +8,7 @@ struct Iconv
original_from, original_to = from, to

@skip_invalid = invalid == :skip
{% unless flag?(:freebsd) %}
{% unless flag?(:freebsd) || flag?(:musl) %}
Copy link
Contributor

Choose a reason for hiding this comment

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

This is out of the scope of the PR but is failing silently here a good idea?

Copy link
Contributor Author

@ysbaddaden ysbaddaden Feb 3, 2017

Choose a reason for hiding this comment

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

It's not out of scope, but iconv in musl-libc doesn't provide any solution for ignoring invalid bytes —FreeBSD provides __iconv to solve this.

I'm not sure what I can do. I could raise, but we couldn't compile anything then, because ignore_invalid is the default. Or maybe I should let it choke on invalid bytes, check Errno for EILSEQ, and replace the invalid bytes manually? Something akin to http://git.alpinelinux.org/cgit/aports/tree/main/musl/iconv.c?h=3.5-stable#n76 I suppose.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah yes, I didn't see the __iconv usage for freebsd. I guess silently failing makes sense here.

@ysbaddaden
Copy link
Contributor Author

macOS builds on Travis look to be dead :/

@RX14
Copy link
Contributor

RX14 commented Feb 3, 2017

@ysbaddaden I think maybe removing libevent from this line would fix it: https://github.com/crystal-lang/crystal/blob/master/bin/ci#L152.

@ysbaddaden
Copy link
Contributor Author

After spending some time on the issue there is nothing to do, but avoid to set the libiconv //IGNORE extension. musl-libc will fail and set errno to EILSEQ, then if we passed invalid: :skip we'll skip a byte and continue the convertion. Done.

@ysbaddaden
Copy link
Contributor Author

Travis is happy and so am I.

@ysbaddaden ysbaddaden merged commit 2b57a98 into crystal-lang:master Feb 6, 2017
@ysbaddaden ysbaddaden deleted the core-iconv-musl-libc branch February 6, 2017 15:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

process_spec fails because of missing /usr/bin/touch Iconv does not work on alpine linux
2 participants