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

Enable newlib iconv #33

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Enable newlib iconv #33

wants to merge 2 commits into from

Conversation

sajattack
Copy link
Member

We have the headers for this but our libc doesn't actually contain the definitions. This is related to utf-8 string conversions and is built into newlib but we're not taking advantage of it.

@gyrovorbis
Copy link

Hey, dudes, so I'm actually working on doing this as well for our toolchain build scripts in KallistiOS...

But after struggling to make sense of the Newlib documentation for quite awhile, I am kind of left with the impression that this flag alone will simply allow you to use the API, but without specifying any actual encodings to pass to newlib via --newlib-iconv-encodings=ascii,utf8,utf16,etc, the iconv() calls will all fail, since you're essentially building the library without support for any actual encoding formats?

At least I think this is how it works? I need to do more testing, but I wanted to run this by you.

@diamant3
Copy link
Member

According to the browser of my PSP(2000), these are the encodings listed:

  • cyrillic (iso-8859-5)
  • japanese (euc-jp)
  • japanese (shift_jis)
  • korean (euc-kr)
  • simplified chinese (gb18030)
  • traditional chinese (big5)
  • western (iso-8859-1)
  • unicode (utf-8)

I found the encoding by following the guide here: https://manuals.playstation.net/document/en/psp/current/network/browser/encode.html

Is that the right encoding to use/support or idk if i'm correct? hopefully it can help

@gyrovorbis
Copy link

According to the browser of my PSP(2000), these are the encodings listed:

  • cyrillic (iso-8859-5)
  • japanese (euc-jp)
  • japanese (shift_jis)
  • korean (euc-kr)
  • simplified chinese (gb18030)
  • traditional chinese (big5)
  • western (iso-8859-1)
  • unicode (utf-8)

I found the encoding by following the guide here: https://manuals.playstation.net/document/en/psp/current/network/browser/encode.html

Is that the right encoding to use/support or idk if i'm correct? hopefully it can help

Yes, those are what are supported, but are those what are enabled by default? The iconv library is built so that you can pick and choose which encodings get compiled into the binary, so that you aren't automatically including everything and are able to conserve space on embedded platforms.

Notice there's also options for --newlib-iconv-encodings and then setting individual source/dest encodings? It's unclear to me whether just using the flag to enable iconv in general is going to include ALL of the encodings or NONE of the encodings... If I get time, I was going to test on Dreamcast/KOS, but that might not happen for awhile.

@diamant3
Copy link
Member

diamant3 commented Apr 25, 2024

Thanks for the insight!, I think --enable-newlib-iconv is to turn on the libiconv(set to NONE maybe) and after that you can set some specific encoding using that --enable-newlib-iconv-encodings, That's my thinking because there's an option like this on the newlib's source by typing ./configure --help=recursive

--enable-newlib-iconv-from-encodings   enable specific comma-separated list of \"from\" iconv encodings to be built-in
--enable-newlib-iconv-to-encodings   enable specific comma-separated list of \"to\" iconv encodings to be built-in

you can set the from/to(is it src/dest equivalent?) of encodings, hopefully it can help you

i'm currently testing this one too in my free time to merge it but it's much better to look by knowledgeable maintainer ❤️ and maybe this will add to libcglue too

@diamant3
Copy link
Member

yes the to/from encodings configure option is the solution to conserve some space instead of just --enable-newlib-iconv-enodings, please read: https://sourceware.org/newlib/libc.html#iconv-configuration

@gyrovorbis
Copy link

gyrovorbis commented Apr 30, 2024

Okay, guys, I finally had the chance to enable this in the Dreamcast/KallistiOS toolchains, and I have some stuff to report...

  • You need BOTH --enable-newlib-iconv to globally enable the iconv() PLUS --enable-newlib-iconv-encodings to provide a list of encodings.
  • The encoding strings passed to iconv() are very, very picky. They don't work with all of the different encoding aliases given in the Newlib readme, so watch out.

For KallistiOS, we opted to give the user an (optional) list of encodings to enable which is populated with the most common ones that users will need by default (ascii, utf8, utf16, wchar). We basically have a configuration script which allows the user to set variables which then get translated into options and flags to pass to the toolchain...

Here's the PR for the DC: KallistiOS/KallistiOS#532

edit: Btw, don't forget you can also enable multibyte strings in Newlib, which I'm assuming you'll probably want if you're going to go as far as to support iconv() as well!

@sharkwouter
Copy link
Member

Unfortunately, the MacOS build failing is expected. GitHub changed which MacOS achitecture is the default and we don't have full support for ARM on MacOS yet.

@diamant3
Copy link
Member

There's a change in the workflow file, let me try to re-run it if it builds correctly now

@gyrovorbis
Copy link

Btw, I noticed you guys went ahead and pulled in iso_8859_1 as well... Is this used in some common functionality or encoding scheme I wasn't thinking about? Maybe we should do it as well in KOS? Thanks.

@sajattack
Copy link
Member Author

Btw, I noticed you guys went ahead and pulled in iso_8859_1 as well... Is this used in some common functionality or encoding scheme I wasn't thinking about? Maybe we should do it as well in KOS? Thanks.

I think it's the default pre-utf-8 encoding for fat32/windows.

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.

None yet

4 participants