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

Libiconv dependency handling #11

Open
justinmichaelvieira opened this issue Feb 23, 2023 · 4 comments · May be fixed by #14
Open

Libiconv dependency handling #11

justinmichaelvieira opened this issue Feb 23, 2023 · 4 comments · May be fixed by #14

Comments

@justinmichaelvieira
Copy link

justinmichaelvieira commented Feb 23, 2023

I recently added this library in to a project that requires CGO (due to cross compilation needs), and because of that I compiled iconv as a static library (alternatively, I believe providing an install of the dynamic linked iconv library for the target machine would have also worked - however, I chose to compile and include iconv statically), as github.com/qiniu/iconv, required by this library, has the C "iconv" library as a dependency.

I think some mention of this requirement on the "target" system may be useful in the README; Would instructions for linking iconv statically and/or dynamically using CGO, and/or instructions on how to provide the proper iconv DLL on the target machine, also be something that would be useful to have in the README for this project? I am acutely aware that compile options and procedure for go code is hard to come by, so wanted to get your opinion on if adding something about how to do this in the README would be useful (in your opinion); If so, I can put together a PR with what I have found regarding installing iconv on the target system, and bundling it using CGO (I've been working on cross compiling and including iconv for an app I'm working on, so would be more than happy to share my notes for the README, if you think it'll be of use for others).

Also, thank you for providing this great escpos library; It helped me quite a bit recently with a few label formats I needed to print via escpos on a newer golang project.

@hennedo
Copy link
Owner

hennedo commented Feb 25, 2023

Hey Justin,
you are absolutely right, it should be documented, but:
The dependency is only there because I copied it over from the seer-robotics version. When I got time to take a closer look the library seemed to be already in use by other people, so at this point I didn't want do create breaking changes. If I remember correctly it's only important for the GBK encoding, which I personally don't need so if it wasn't already being used by others I would simply had removed it.

Meaning: I'd rather get rid of the CGO dependency so it can be compiled without CGO. That said my (brief) tests of other ways to encode the texts yielded incorrect results and I didn't have time to resolve this.

If it's not that much of work for you I'd gladly accept a PR improving the documentation as I don't know when i'll get to exchanging the iconv dependency for a native go variant.

@justinmichaelvieira
Copy link
Author

Awesome; I will try and put together a PR describing the iconv dependency and ways to compile for it so things deploy correctly.

That said my (brief) tests of other ways to encode the texts yielded incorrect results and I didn't have time to resolve this.

Do you remember what library you tried, and what results were incorrect? I was hoping to research how to remove the dependency as well (if I find the time...); Although I think at this juncture documenting the dependency is most useful until there is a clear path to removing it.

Thanks again :)

@justinmichaelvieira
Copy link
Author

I decided to try removing the dependency to iconv instead; #12 is the change for that.

Has been working for me well; Please let me know what you think, when you get a chance :)

@justinmichaelvieira
Copy link
Author

This issue should be closed when #14 is merged.

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