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

ConvertUTF.{cpp,h} are non-free #12

Open
cstrahan opened this issue Apr 10, 2018 · 17 comments
Open

ConvertUTF.{cpp,h} are non-free #12

cstrahan opened this issue Apr 10, 2018 · 17 comments

Comments

@cstrahan
Copy link

The ConvertUTF.{cpp,h} are non-free (and buggy/unmaintained), as discussed on the Debian mailing list:

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=823100

This poses problems for packaging/redistribution. Would it be possible to, perhaps, support linking with libicu for this purpose?

We're currently using linenoise-ng in Nix, and this is same issue is blocking an effort to package Nix for Debian. replxx looks nice, and if we could get this issue resolved here, it's possible we'd switch away from linenoise-ng.

Thanks!

@AmokHuginnsson
Copy link
Owner

AmokHuginnsson commented Apr 10, 2018

I am quite open to use alternative to ConvertUTF.{cpp,h}, it can be ICU or some other conversion library, as long as the choice of the conversion library is preserved for users (and preferable handled by cmake).
What I mean is that I would like for the users to still use ConvertUTF.{cpp,h} if they prefer to. That would mean abstracting out conversion interface.
I will happily accept pull request implementing this change having all conversion backends working, I mean both ICU and ConvertUTF.{cpp,h}.
Debian people could simply ignore ConvertUTF.{cpp,h} files and use ICU for their build.
I still want to have ConvertUTF.{cpp,h} available for MSVCXX users sake as bringing ICU as hard dependency could significantly increase MSVCXX build difficulty.

@dothebart
Copy link

We @arangodb have similar opinions on that. If you propose a patch to replace this by ICU (which is already one of the other arangodb dependencies) we could drop ConvertUTF.{CPP,}. However, we currently don't have the time to work on this.

@doug-moen
Copy link
Contributor

If Debian is correct, and this code isn't open source, then I'll need to fix the problem before I can use replxx in my open source project. However, I notice that LLVM contains the same source file, and Debian contains an LLVM package.

I used apt-get source llvm-6.0-dev to get the latest version of the Debian source package for LLVM. The file lib/Support/ConvertUTF.cpp in LLVM is recognizably the same code, with the same original licence text. One difference is that the LLVM version of ConvertUTF.cpp has the LLVM licence pasted at the top of the file, before the Unicode licence.

So maybe this isn't actually a problem. Or maybe Debian can be made happy by copying the LLVM version of ConvertUTF.cpp, even though it's really the same code with the same licence. I doubt that there is any actual legal risk of "Unicode, Inc" suing users of replxx for licence violation. Why wouldn't they go after bigger targets like LLVM or Apple first?

@AmokHuginnsson
Copy link
Owner

@doug-moen
Hello, thank you for taking interest in replxx.

I am not a lawyer. I could use llvm's version of ConvertUTF.{cpp,h} if some Debian maintainer confirmed that is would be sufficient. Using libicu is another option.

Unfortunatelly right now I do not have the time to take care of it.
Fixing how special keys are handled (#6) is higher on my TODO list anyway.

@doug-moen
Copy link
Contributor

Okay, thanks. I was trying to figure out if there is actually an issue that needs to be fixed. I'd volunteer to fix it myself if there was an actual problem. However, I wouldn't consider LLVM non-free or refuse to link to it due to its use of ConvertUTF, and for consistency I should treat replxx the same way.

So I decided there's no issue, and I'm now using replxx in my project, replacing GNU readline. Thanks for making the project.

@olegator77
Copy link
Contributor

Hi!
I can suggest switch to utf8cpp library - It has native C++ API, and free license.

BTW, i had replaced ConvertUTF by utf8cpp in my projects. It works fine :)

@LoganDark
Copy link

@olegator77 what about normal C?

@olegator77
Copy link
Contributor

@LoganDark replxx is C++ library, so actually it seems - normal: add C++ library instead of plain C library

@LoganDark
Copy link

Okay, I guess C++ and C are completely interoperable then.

@sbadia
Copy link

sbadia commented Apr 2, 2020

Hi here,

As said @olegator77, utf8cpp library seems a good candidate.
taglib project has encounter the same issue with ConvertUTF.{ccp,h} (see taglib/taglib#793) and has migrated to utf8cpp lib (see this pull-request: taglib/taglib#794)

@minfrin
Copy link
Contributor

minfrin commented Nov 17, 2021

Confirmation that this issue blocks inclusion to Fedora:

https://bugzilla.redhat.com/show_bug.cgi?id=2017179#c8

@minfrin
Copy link
Contributor

minfrin commented Nov 17, 2021

Suggestion to use code used by LLVM project, which has acceptable license:

#125 (comment)

@minfrin
Copy link
Contributor

minfrin commented Nov 22, 2021

From https://bugzilla.redhat.com/show_bug.cgi?id=2017179#c9

--- Comment #9 from Zbigniew Jędrzejewski-Szmek [email protected] ---
%{_libdir}/.so.

Please specify the SO VERSION here, don't use a glob. See
https://docs.fedoraproject.org/en-US/packaging-guidelines/#_listing_shared_library_files

I don't think Unicode Strict can be accepted into Fedora, though. So we will have to wait on that issue. I have subscribed as well.

After looking at the original file, the license is problematic. The proposed
solution
of using the file in LLVM does not be a solution: I think LLVM is in
"violation" of the literal
license terms too, and they should not say that this file is available under
the Apache2 license.
I used quotes because it seems that the actual legal risk is negligible.
Nevertheless, we want to
keep everything kosher in Fedora, so we don't want to accept this minor
violation.

utf8cpp seems like a better approach. [1] does a similar conversion, and it
seems very
straightforward. utf8cpp is in Fedora, so we'd want to just use the system
library.

Note that this license was briefly discussed on legal@, but the question was
sidestepped because
the files weren't actually used [2].

[1] https://github.com/taglib/taglib/pull/794/files
[2]
https://lists.fedoraproject.org/archives/list/[email protected]/thread/KQEAGD4UCDRO2X5P4WYOOT5FUFWXJ4NP/#KQEAGD4UCDRO2X5P4WYOOT5FUFWXJ4NP

GerHobbelt pushed a commit to GerHobbelt/replxx that referenced this issue Sep 14, 2022
You may get uncaugh exception (in case of i.e. broken pipe):

    terminating with uncaught exception of type std::runtime_error: write failed

On destroy:

    (lldb) target create "clickhouse-22.8-release" --core "core.clickhouse-clie.402986-642410"
    bt
    Core file '/wrk/core.clickhouse-clie.402986-642410' (x86_64) was loaded.
    (lldb) bt
    * thread AmokHuginnsson#1, name = 'clickhouse-clie', stop reason = signal SIGABRT
      * frame #0: 0x00007f03fb5c900b libc.so.6`raise + 203
        frame AmokHuginnsson#1: 0x00007f03fb5a8859 libc.so.6`abort + 299
        frame AmokHuginnsson#2: 0x000000001b703f44 clickhouse-22.8-release`::abort_message(format=<unavailable>) at abort_message.cpp:78:5
        frame AmokHuginnsson#3: 0x000000001b703dd4 clickhouse-22.8-release`demangling_terminate_handler() at cxa_default_handlers.cpp:67:21
        frame AmokHuginnsson#4: 0x000000001b721063 clickhouse-22.8-release`std::__terminate(func=<unavailable>)()) at cxa_handlers.cpp:59:9
        frame AmokHuginnsson#5: 0x000000001b720fce clickhouse-22.8-release`std::terminate() at cxa_handlers.cpp:88:17
        frame AmokHuginnsson#6: 0x000000000a3b21db clickhouse-22.8-release`__clang_call_terminate + 11
        frame AmokHuginnsson#7: 0x00000000189b1bfc clickhouse-22.8-release`replxx::Replxx::ReplxxImpl::~ReplxxImpl(this=0x00007f03fa945308) at replxx_impl.cxx:336:1
        frame AmokHuginnsson#8: 0x00000000189b1ce9 clickhouse-22.8-release`replxx::Replxx::ReplxxImpl::~ReplxxImpl(this=0x00007f03fa945300) at replxx_impl.cxx:334:41
        frame AmokHuginnsson#9: 0x00000000188b0644 clickhouse-22.8-release`ReplxxLineReader::~ReplxxLineReader() [inlined] std::__1::unique_ptr<replxx::Replxx::ReplxxImpl, void (*)(replxx::Replxx::ReplxxImpl*)>::reset(this=<unavailable>, __p=<unavailable>) at unique_ptr.h:315:7
        frame AmokHuginnsson#10: 0x00000000188b0626 clickhouse-22.8-release`ReplxxLineReader::~ReplxxLineReader() [inlined] std::__1::unique_ptr<replxx::Replxx::ReplxxImpl, void (*)(replxx::Replxx::ReplxxImpl*)>::~unique_ptr(this=<unavailable>) at unique_ptr.h:269
        frame AmokHuginnsson#11: 0x00000000188b0626 clickhouse-22.8-release`ReplxxLineReader::~ReplxxLineReader() [inlined] replxx::Replxx::~Replxx(this=<unavailable>) at replxx.hxx:76
        frame AmokHuginnsson#12: 0x00000000188b0626 clickhouse-22.8-release`ReplxxLineReader::~ReplxxLineReader(this=0x00007ffd10038440) at ReplxxLineReader.cpp:229
        frame AmokHuginnsson#13: 0x00000000158b2100 clickhouse-22.8-release`DB::ClientBase::runInteractive(this=0x00007ffd10038620) at ClientBase.cpp:2067:1
        frame AmokHuginnsson#14: 0x000000000a4de372 clickhouse-22.8-release`DB::Client::main(this=0x00007ffd10038620, (null)=<unavailable>) at Client.cpp:261:9
        frame AmokHuginnsson#15: 0x0000000018923a86 clickhouse-22.8-release`Poco::Util::Application::run(this=0x00007ffd10038620) at Application.cpp:334:8
        frame AmokHuginnsson#16: 0x000000000a4ed341 clickhouse-22.8-release`mainEntryClickHouseClient(argc=39, argv=0x00007f03fa8201c0) at Client.cpp:1220:23
        frame AmokHuginnsson#17: 0x000000000a3b17ab clickhouse-22.8-release`main(argc_=<unavailable>, argv_=<unavailable>) at main.cpp:449:12
        frame AmokHuginnsson#18: 0x00007f03fb5aa083 libc.so.6`__libc_start_main + 243
        frame AmokHuginnsson#19: 0x000000000a17032e clickhouse-22.8-release`_start + 46
GerHobbelt pushed a commit to GerHobbelt/replxx that referenced this issue Sep 14, 2022
You may get uncaugh exception (in case of i.e. broken pipe):

    terminating with uncaught exception of type std::runtime_error: write failed

On destroy:

    (lldb) target create "clickhouse-22.8-release" --core "core.clickhouse-clie.402986-642410"
    bt
    Core file '/wrk/core.clickhouse-clie.402986-642410' (x86_64) was loaded.
    (lldb) bt
    * thread AmokHuginnsson#1, name = 'clickhouse-clie', stop reason = signal SIGABRT
      * frame #0: 0x00007f03fb5c900b libc.so.6`raise + 203
        frame AmokHuginnsson#1: 0x00007f03fb5a8859 libc.so.6`abort + 299
        frame AmokHuginnsson#2: 0x000000001b703f44 clickhouse-22.8-release`::abort_message(format=<unavailable>) at abort_message.cpp:78:5
        frame AmokHuginnsson#3: 0x000000001b703dd4 clickhouse-22.8-release`demangling_terminate_handler() at cxa_default_handlers.cpp:67:21
        frame AmokHuginnsson#4: 0x000000001b721063 clickhouse-22.8-release`std::__terminate(func=<unavailable>)()) at cxa_handlers.cpp:59:9
        frame AmokHuginnsson#5: 0x000000001b720fce clickhouse-22.8-release`std::terminate() at cxa_handlers.cpp:88:17
        frame AmokHuginnsson#6: 0x000000000a3b21db clickhouse-22.8-release`__clang_call_terminate + 11
        frame AmokHuginnsson#7: 0x00000000189b1bfc clickhouse-22.8-release`replxx::Replxx::ReplxxImpl::~ReplxxImpl(this=0x00007f03fa945308) at replxx_impl.cxx:336:1
        frame AmokHuginnsson#8: 0x00000000189b1ce9 clickhouse-22.8-release`replxx::Replxx::ReplxxImpl::~ReplxxImpl(this=0x00007f03fa945300) at replxx_impl.cxx:334:41
        frame AmokHuginnsson#9: 0x00000000188b0644 clickhouse-22.8-release`ReplxxLineReader::~ReplxxLineReader() [inlined] std::__1::unique_ptr<replxx::Replxx::ReplxxImpl, void (*)(replxx::Replxx::ReplxxImpl*)>::reset(this=<unavailable>, __p=<unavailable>) at unique_ptr.h:315:7
        frame AmokHuginnsson#10: 0x00000000188b0626 clickhouse-22.8-release`ReplxxLineReader::~ReplxxLineReader() [inlined] std::__1::unique_ptr<replxx::Replxx::ReplxxImpl, void (*)(replxx::Replxx::ReplxxImpl*)>::~unique_ptr(this=<unavailable>) at unique_ptr.h:269
        frame AmokHuginnsson#11: 0x00000000188b0626 clickhouse-22.8-release`ReplxxLineReader::~ReplxxLineReader() [inlined] replxx::Replxx::~Replxx(this=<unavailable>) at replxx.hxx:76
        frame AmokHuginnsson#12: 0x00000000188b0626 clickhouse-22.8-release`ReplxxLineReader::~ReplxxLineReader(this=0x00007ffd10038440) at ReplxxLineReader.cpp:229
        frame AmokHuginnsson#13: 0x00000000158b2100 clickhouse-22.8-release`DB::ClientBase::runInteractive(this=0x00007ffd10038620) at ClientBase.cpp:2067:1
        frame AmokHuginnsson#14: 0x000000000a4de372 clickhouse-22.8-release`DB::Client::main(this=0x00007ffd10038620, (null)=<unavailable>) at Client.cpp:261:9
        frame AmokHuginnsson#15: 0x0000000018923a86 clickhouse-22.8-release`Poco::Util::Application::run(this=0x00007ffd10038620) at Application.cpp:334:8
        frame AmokHuginnsson#16: 0x000000000a4ed341 clickhouse-22.8-release`mainEntryClickHouseClient(argc=39, argv=0x00007f03fa8201c0) at Client.cpp:1220:23
        frame AmokHuginnsson#17: 0x000000000a3b17ab clickhouse-22.8-release`main(argc_=<unavailable>, argv_=<unavailable>) at main.cpp:449:12
        frame AmokHuginnsson#18: 0x00007f03fb5aa083 libc.so.6`__libc_start_main + 243
        frame AmokHuginnsson#19: 0x000000000a17032e clickhouse-22.8-release`_start + 46
@siliconja
Copy link

I created a Ubuntu PPA to build a 'libreplxx-dev' package (sudo apt install libreplxx-dev) and encountered this licensing issue. The debuild tool runs lintian on source packages and throws an error "E: replxx source: license-problem-convert-utf-code".

To address this I made patches that adopt the ConvertUTF.h and ConvertUTF.cpp files from the llvm-project commit 20451cb (llvm/llvm-project@20451cb) which have an updated license as described in previous comments.

Ubuntu PPA is here: https://launchpad.net/~siliconja/+archive/ubuntu/replxx/

@minfrin
Copy link
Contributor

minfrin commented Dec 17, 2022

Based on @siliconja's recommendation above, here is a PR: #142

@minfrin
Copy link
Contributor

minfrin commented Jun 20, 2023

Quick ping, any news on the PR above? Looks like lots of packaging pain gets unblocked when this is completed.

@doug-moen
Copy link
Contributor

The PR is safe to merge without testing because the only change is to comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants