Add inih dependency to release workflow

[kevinbackhouse]

I forgot to update this workflow in #2443.

[codecov]

Codecov Report

Merging #2457 (072c3ba) into main (842ef05) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #2457   +/-   ##
=======================================
  Coverage   62.08%   62.08%           
=======================================
  Files         121      121           
  Lines       22830    22830           
  Branches    11212    11212           
=======================================
  Hits        14175    14175           
  Misses       6448     6448           
  Partials     2207     2207           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[kevinbackhouse]

I'm puzzled why CIFuzz is still failing. I'll look into it.

[kevinbackhouse]

I'm puzzled why CIFuzz is still failing. I'll look into it.

I think I know what's happening. OSS-Fuzz is using Ubuntu 20.04, which uses an older version of inih that doesn't include the C++ interface.

I'm beginning to think that #2443 was a mistake and I should revert it. There's probably a lot of people still using Ubuntu 20.04 who'll be affected by the same problem.

[hassec]

What about making it a conan dependency -> https://conan.io/center/inih And lettin conan take care of building it?

[piponazo]

What about making it a conan dependency -> https://conan.io/center/inih And lettin conan take care of building it?

Right, inih is an implementation detail for exiv2 and does not need to get exposed to the clients. It should be fine to use inih from conan for the release binaries.

Regarding the problem with the fuzzing, I guess that we could start to use Ubuntu 22.04 for that CI job if it makes our life simpler. Otherwise, we can also use conan there to bring the dependency.

[kevinbackhouse]

The problem with CIFuzz is that it uses a docker image created by the OSS-Fuzz team that's based on Ubuntu 20.04, so there's no way for us to upgrade it to Ubuntu 22.04. I can easily modify the build script to create a workaround for CIFuzz, but I'm worried about this issue being really annoying for users who want to install the latest version (main branch) of Exiv2 on Ubuntu 20.04. Presumably, those people will have to manually build and install inih, as well as Exiv2?

[piponazo]

Presumably, those people will have to manually build and install inih, as well as Exiv2?

The other option they have is to use conan for resolving the obligatory dependencies. I understand that it is not an ideal scenario to force user to install more tools to be able to compile our project. On the other hand, it was not nice either to have the inih library copied inside our repository.

IMHO, I think it is reasonable to ask Exiv2 developers to use conan to ease the management of 3rd party developers. Final users of the exiv2 library & application will not need to compile the project by themselves.

[neheb]

I have a fairly basic(incomplete) meson version of exiv2 which is using INIReader as a subproject if unavailable.

Does cmake have the concept of a subproject?

edit: hmm not as simple it seems: https://stackoverflow.com/questions/57623087/what-is-the-equivelant-to-mesons-subprojects-wrap-files-in-cmake

[kevinbackhouse]

IMHO, I think it is reasonable to ask Exiv2 developers to use conan to ease the management of 3rd party developers. Final users of the exiv2 library & application will not need to compile the project by themselves.

Great, thanks @piponazo! I was having a moment of self-doubt there. I'll go ahead and fix the OSS-Fuzz build script, which will be a PR on the OSS-Fuzz repo, not Exiv2.

[piponazo]

Does cmake have the concept of a subproject?

I am not so familiar with meson, but after reading its documentation, I think that the simil for CMake is this one:
https://cmake.org/cmake/help/latest/module/ExternalProject.html

However (in my experience) it is a pain to manage dependencies with that function when the project is multiplatform. That's why the C++ package managers saw the light few years ago.

[eli-schwartz]

ExternalProject is kind of similar, but there are crucial differences. For example, Meson subprojects are activated as part of dependency resolution, but you need to add a bunch of glue for cmake: think about if find_package(inih) automatically attempted to run an ExternalProject, but only if cmake/Findinih.cmake returned not-found.

It's doesn't really help to remove a vendored copy of inih if you then go and always download and vendor it again, so I'd tend to agree that conan is a better option here.

[kmilos]

I wouldn't like to have yet another tool like conan forced upon me (having to track and bump and take care of minor version numbers like it is needed for CI babystting is fairly annoying), my system package management + CMake is all I need and want to use.

I thought inih is now available on most systems, so we can treat it just like zlib or whatever other dependency, no?

If it's missing somewhere, please request it upstream - I have already packaged it for MSYS2, and requested a package for Cygwin.

[hassec]

IMHO, we should just use conan to retrieve the inih dependency and build it as a static library such that it doesn't become a runtime dependency.
That way it's only a build time dependency and we already use conan for these kinds of things.

I disagree that using the system package manager is a good way to install these dependencies. That's pretty messy, while conan keeps these things quite nicely separated.

But as a first step I'd like to merge this PR to get the release build working again, and then we can discuss how to proceed. WDYT @piponazo @kevinbackhouse @kmilos ?

[piponazo]

@hassec Fine for me, let's fix the current situation and we can always create another PR to improve it :)

[kevinbackhouse]

Sorry for the delay. I've been trying to solve the OSS-Fuzz build failure, but I'll do that separately. The OSS-Fuzz build environment is very weird because it sets lots of non-standard compiler flags, so I've been getting errors when I try to build inih from source. I'm now leaning towards adding a build parameter to Exiv2 that will enable me to build the fuzz target without inih - it isn't relevant for fuzzing anyway.