cmake: Fix version kwarg being added to static_library targets

[RobotLeopard86]

I have encountered an issue where, in some CMake subprojects, the version kwarg gets added to some static_library targets. This then causes the generated meson.build file to fail processing, as version is not a valid kwarg for static targets.

To fix this, I have added a small check around the part where that kwarg is added to ensure the target is not a static library.

Hopefully, since this is a minor bugfix, it can be slotted in for 1.10.0, but I understand if this is not possible since feature freeze has technically passed.

[obackhouse]

any decision on whether this gets into 1.10.0? i'm getting it inside a cmake.subproject and can't think of an obvious (to me) workaround

[RobotLeopard86]

What project are you having it with? For me it was LLVM (which I had to patch the CMake files of to get it down to a minimal build for my use case and get it configuring/compiling in the first place).

I made another PR to fix something I was having issues with and bonzini said it looked good but I haven't heard back from the team in a week about merging.

I get they're busy prepping for release and feature freeze has passed, so I'm being patient, but I was hoping that there would be more concrete action taken.

[eli-schwartz]

I'd be interested to see a test case for this as well. Is this as simple as setting the target property "version" / "soversion" for an add_library static call?

For meson, a library() supports setting sonames regardless of default_library, but I guess CMake is more... loose / relaxer and just allows mixing keywords that don't really go together?

[obackhouse]

What project are you having it with? For me it was LLVM (which I had to patch the CMake files of to get it down to a minimal build for my use case and get it configuring/compiling in the first place).

I made another PR to fix something I was having issues with and bonzini said it looked good but I haven't heard back from the team in a week about merging.

I get they're busy prepping for release and feature freeze has passed, so I'm being patient, but I was hoping that there would be more concrete action taken.

this was buried in a personal project that I wouldn't be able to share right this second, i'm afraid -- just thought it was worth at least mentioning that i have cases where it is blocking.

if this isn't pushed through soon and i get some spare time i can try to isolate a MWE or test case

[RobotLeopard86]

@eli-schwartz I'm not exactly sure what you're saying here. The issue I had was caused by this snippet in LLVM's CMake:

set(LIBCLANG_SOVERSION_ARG)
if(NOT CLANG_FORCE_MATCHING_LIBCLANG_SOVERSION)
  set(LIBCLANG_SOVERSION_ARG SOVERSION 13)
endif()

When I built Clang as a static library, it still had a set soversion property from this CMake bit. Then, Meson would add a kwarg for that in the generated meson.build it makes, but in a static_library call (not library, where this would work). The static_library function has no soversion kwarg, so that would trigger an error during the processing of the generated build file and setup would abort.

My patch fixes this by ensuring that the version and soversion kwargs are not passed to a static_library call.

[eli-schwartz]

@eli-schwartz I'm not exactly sure what you're saying here. The issue I had was caused by this snippet in LLVM's CMake:

set(LIBCLANG_SOVERSION_ARG)
if(NOT CLANG_FORCE_MATCHING_LIBCLANG_SOVERSION)
  set(LIBCLANG_SOVERSION_ARG SOVERSION 13)
endif()

When I built Clang as a static library, it still had a set soversion property from this CMake bit. Then, Meson would [... internal implementation details of meson subprojects ...]

That's exactly what I'm saying -- you should update one of the unittests in this repository to perform the same CMake maneuver (set the soversion property in a CMakeLists.txt which builds a static library).

The point is to have a regression test of this scenario.

[eli-schwartz]

Hopefully, since this is a minor bugfix, it can be slotted in for 1.10.0, but I understand if this is not possible since feature freeze has technically passed.

Oh also to loop back around to this, we are deep into release freeze so for a non-regression I'd rather wait until after the release, and then mark this for backporting into 1.10.1 instead.

[RobotLeopard86]

Hopefully, since this is a minor bugfix, it can be slotted in for 1.10.0, but I understand if this is not possible since feature freeze has technically passed.

Oh also to loop back around to this, we are deep into release freeze so for a non-regression I'd rather wait until after the release, and then mark this for backporting into 1.10.1 instead.

I actually do think this is a regression of some kind (though I don't know how exactly), since this worked before on Windows and I have not changed the CMake files or invocation via the CMake module, and it stopped working semi-randomly. I understand that release freeze is in place, so if you wish to hold this, that is your call and I will respect that.

I will note, though, that I initially opened the issue last week, with no reply, when release freeze was not in effect. I get that you're busy, so if you just didn't see it, it's totally fine. I'm going to guess that this is why my other bugfix PR (#15284) has not been merged despite bonzini giving it the thumbs up a week ago.

Anyways, I hope that the rest of the release cycle goes well for you all; I'll follow procedure for keeping my PRs synced with upstream.

[RobotLeopard86]

@eli-schwartz I haven't worked with regression testing before, so I would like some advice on that.

I'm familiar with how to run tests, so I can try to make a test for this scenario and figure out what the problem is, but if you could help me out with this, it would be appreciated.

I should also note that this problem, as with the other PR I mentioned, only occurs on Windows, which is very odd. I checked this; on both macOS and Linux the snippet causes no issue with an unpatched Meson 1.9.1 installed via pip.

May need to conduct more testing into this. But I agree with your decision to hold this until 1.10.1.