-
Notifications
You must be signed in to change notification settings - Fork 63
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
missing iniparser include directory when compiling #96
Comments
iniparser doesn't have a pkg-config file, so we can't just use that. On my system iniparser just installs |
Just installing right now, I needed to run these commands because on my ubuntu 20.04 it's in
I don't know meson nor ninja, and they seem to sometimes use $CFLAGS (and maybe $INCLUDE ?), sometimes not. Is there a way to add a |
Found another project that is including iniparser with a fallback... https://github.com/nnstreamer/nntrainer/blob/main/meson.build#L138-L147 Maybe we could do something like this? I don't think the maintainer of iniparser is interested in build system related PRs. |
the problem with this method is the hardcoded include directory. If someone have iniparser installed in /usr/local/include/... Maybe the correct way is to make a PR to iniparser to add a pkg-config file.. |
A pkg-config file would be very nice indeed. |
I know that we concluded there was no .pc file during this PR, but I think the linked issue is old. Looks like they've had one since 2018, but the Arch package still isn't shipping it. Edit: I've requested that the Arch package issue be reopened in light of the Edit 2: |
It seems like there's no iniparser release which contains the |
As a workaround, you can |
I'm discussing with upstream iniparser by email and they indicate that iniparser was never meant to be installed system-wide, it was meant to be embedded in projects instead. That probably explains the lack of We may want to consider dropping the iniparser dep and include our own parser instead. rootston has a 200 line parser for instance: https://github.com/emersion/rootston/blob/master/ini.c |
Eh, it seems rootston is using inih, also available in Arch repos. So switching to that would be an option too (not sure it's worth it for 200 lines). |
Huh, thanks for looking into this. That does make a lot of sense. I'm fine with either approach. The only benefit I see to depending on the system package is not having to manage updates unless they include breaking changes. |
Reads like it has shipping by distros in mind and pkgs.org shows wide support. Therefore i would link against the distros package instead of including its source. |
I still prefer using a dedicated lib instead of shipping an maintaining our own iniparser, even if it is only 200 lines of sth. which probably won't change and we could use those lines when the lib includes breaking changes. But I'm not opposed to using them either. |
Hi,
When I compile the rev 323d89e (current master), I got the following error:
Adding the include directory for lib iniparser in build.ninja for core/config.c correct the problem.
Since I do not know how to make meson discover and add the correct path, I corrected manually the build.ninja
Best,
The text was updated successfully, but these errors were encountered: