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

Using iconv for Utf8Converter on Apple device #15599

Closed
wants to merge 16 commits into from
Closed

Conversation

jchen351
Copy link
Contributor

Description

using iconv for Utf8Converter on Apple device

Motivation and Context

Because wstring_convert will be deprecated in macOS SDK 13.3 C++17

snnn and others added 4 commits April 19, 2023 23:38

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@jchen351 jchen351 changed the title Cjian/iconv mac Using iconv for Utf8Converter on Apple device Apr 20, 2023
@jchen351 jchen351 marked this pull request as ready for review April 20, 2023 06:46
snnn
snnn previously approved these changes Apr 25, 2023
@@ -46,6 +47,7 @@
DBDB57DB2603211A004F16BE /* Info.plist */ = {isa = PBXFileReference; lastKnownFileType = text.plist.xml; path = Info.plist; sourceTree = "<group>"; };
DBDB588A2609B18F004F16BE /* Resources */ = {isa = PBXFileReference; lastKnownFileType = folder; name = Resources; path = OnnxruntimeModuleTest/Resources; sourceTree = "<group>"; };
DBDB58AF262A92D6004F16BE /* OnnxruntimeModuleTest.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = OnnxruntimeModuleTest.mm; sourceTree = "<group>"; };
E3D7A60D29F8D87D00FF5265 /* libiconv.tbd */ = {isa = PBXFileReference; lastKnownFileType = "sourcecode.text-based-dylib-definition"; name = libiconv.tbd; path = Platforms/MacOSX.platform/Developer/SDKs/MacOSX13.3.sdk/usr/lib/libiconv.tbd; sourceTree = DEVELOPER_DIR; };
Copy link
Member

Choose a reason for hiding this comment

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

Where can we find the license of this file: Platforms/MacOSX.platform/Developer/SDKs/MacOSX13.3.sdk/usr/lib/libiconv.tbd ?

Would it be bad to hardcode a XCode version here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This file was generated from XCode GUI Application. I have to link the library for these 2 xcode projects from the Xcode GUI. The library is part of Apple SDK.

I can try to remove the 13.3 from the MacOSX13.3.sdk, and it should works.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Screenshot 2023-04-25 at 9 25 57 PM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Screenshot 2023-04-25 at 9 26 48 PM

Copy link
Member

Choose a reason for hiding this comment

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

The library must come with some headers files. One of them should be iconv.h. Could you please open the file and inspect its license?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

/* Copyright (C) 1999-2003, 2005-2006 Free Software Foundation, Inc.
This file is part of the GNU LIBICONV Library.

The GNU LIBICONV Library is free software; you can redistribute it
and/or modify it under the terms of the GNU Library General Public
License as published by the Free Software Foundation; either version 2
of the License, or (at your option) any later version.

The GNU LIBICONV Library is distributed in the hope that it will be
useful, but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
Library General Public License for more details.

You should have received a copy of the GNU Library General Public
License along with the GNU LIBICONV Library; see the file COPYING.LIB.
If not, write to the Free Software Foundation, Inc., 51 Franklin Street,
Fifth Floor, Boston, MA 02110-1301, USA. */

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is GNU, but is the copyright expired?

Copy link
Member

Choose a reason for hiding this comment

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

No. I don't think so. We should avoid using it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Then we have to find another alternative before we adapt MacOS SDK 13.3.

Copy link
Member

Choose a reason for hiding this comment

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

@snnn snnn requested a review from skottmckay April 26, 2023 04:02
@snnn
Copy link
Member

snnn commented Apr 26, 2023

@skottmckay, what do you think?

@skottmckay
Copy link
Contributor

Probably safest to ask CELA.

I don't recall having to worry about licenses for any other libraries that are part of the OS (if we're using the /usr/lib version). Is there any reason why the /usr/lib version would not be sufficient?

Also not clear to me why it would be okay to use libiconv on Linux but not on mac, but if we can stick to the /usr/lib version maybe that question doesn't matter.

@snnn
Copy link
Member

snnn commented May 16, 2023

We do not use libiconv on Linux. libiconv is from GNU. https://www.gnu.org/software/libiconv/ . We use glibc on Linux. glibc is not licensed under GPL.

@skottmckay
Copy link
Contributor

skottmckay commented May 17, 2023

The stackoverflow article made it sound like the default iconv in macos was not the GNU libiconv one, but the license in the SDK for MacOSX (/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/iconv.h) is the GNU license.

I can only find iconv.h in the Xcode SDKs. No other locations.

// find . -name iconv.h -type f 2>&/dev/null
./System/Volumes/Data/Applications/Xcode.app/Contents/Developer/Platforms/AppleTVOS.platform/Developer/SDKs/AppleTVOS.sdk/usr/include/iconv.h
./System/Volumes/Data/Applications/Xcode.app/Contents/Developer/Platforms/iPhoneOS.platform/Developer/SDKs/iPhoneOS.sdk/usr/include/iconv.h
./System/Volumes/Data/Applications/Xcode.app/Contents/Developer/Platforms/WatchOS.platform/Developer/SDKs/WatchOS.sdk/usr/include/iconv.h
./System/Volumes/Data/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/iconv.h
./System/Volumes/Data/Applications/Xcode.app/Contents/Developer/Platforms/WatchSimulator.platform/Developer/SDKs/WatchSimulator.sdk/usr/include/iconv.h
./System/Volumes/Data/Applications/Xcode.app/Contents/Developer/Platforms/AppleTVSimulator.platform/Developer/SDKs/AppleTVSimulator.sdk/usr/include/iconv.h
./System/Volumes/Data/Applications/Xcode.app/Contents/Developer/Platforms/iPhoneSimulator.platform/Developer/SDKs/iPhoneSimulator.sdk/usr/include/iconv.h
./Applications/Xcode.app/Contents/Developer/Platforms/AppleTVOS.platform/Developer/SDKs/AppleTVOS.sdk/usr/include/iconv.h
./Applications/Xcode.app/Contents/Developer/Platforms/iPhoneOS.platform/Developer/SDKs/iPhoneOS.sdk/usr/include/iconv.h
./Applications/Xcode.app/Contents/Developer/Platforms/WatchOS.platform/Developer/SDKs/WatchOS.sdk/usr/include/iconv.h
./Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/iconv.h
./Applications/Xcode.app/Contents/Developer/Platforms/WatchSimulator.platform/Developer/SDKs/WatchSimulator.sdk/usr/include/iconv.h
./Applications/Xcode.app/Contents/Developer/Platforms/AppleTVSimulator.platform/Developer/SDKs/AppleTVSimulator.sdk/usr/include/iconv.h
./Applications/Xcode.app/Contents/Developer/Platforms/iPhoneSimulator.platform/Developer/SDKs/iPhoneSimulator.sdk/usr/include/iconv.h

https://opensource.apple.com/source/libiconv/libiconv-9/libiconv/include/iconv.h.in.auto.html

Not sure if macos doesn't use libiconv so doesn't care about the GPL license or what.

Can we use the source from the gnu libc version or are there too many files to copy?

Other option might be boost::locale

@edgchen1
Copy link
Contributor

from here: https://www.gnu.org/software/libiconv/

Copyright
The libiconv and libcharset libraries and their header files are under LGPL.
The iconv program is under GPL.

is LGPL ok for us to use?

@snnn
Copy link
Member

snnn commented May 19, 2023

is LGPL ok for us to use?

If we dynamically link to it, not much obligation. Just make sure to not include it in our packages.

Copy link
Member

@snnn snnn left a comment

Choose a reason for hiding this comment

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

Based on an internal discussion, we have a better choice than this library. We can use the system's builtin APIs.

# Conflicts:
#	onnxruntime/core/providers/cpu/nn/string_normalizer.cc
@jchen351 jchen351 closed this Nov 17, 2023
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