Mop up #1084 breakage

[botovq]

Revert the minimum cmake version change since that isn't viable for major linux distros. Also, it led to weird compilation failures due to some undesirable side effects when compiling assembly files:

/__w/portable/portable/crypto/aes/vpaes-elf-x86_64.S:8:1: error: expected identifier or '(' before '.' token
    8 | .text
      | ^

[vszakats]

@botovq Thanks for the quick update!

Are you sure the if() works inside argument lists as expected?

This isn't documented (AFAICF) and compiles silently by CMake, but
doesn't actually seem to enable the thing enclosed inside the condition.
It also breaks when swapping the condition to, say, UNIX, or when
adding an else() branch.

There is a way to add options with inline conditions in certain cases,
with a different syntax (haven't tried this):
https://cmake.org/cmake/help/v3.0/manual/cmake-generator-expressions.7.html

Or with additional calls to set_target_properties(ssl PROPERTIES ...). Or with
set_property(TARGET ssl APPEND PROPERTY ...).

[botovq]

Are you sure the if() works inside arguments lists as expected?

No, of course not :)

Now that you point it out, I see the problem...

I don't have access to a Windows machine to check. Does #1103 work?

[vszakats]

@botovq #1103 LGTM, thanks! I also don't have a Windows machine to test, and my cross-builds are without DLLs. Reproducing a fitting build locally would be a bit tedious.

Within LibreSSL's Windows CI workflow, maybe adding a post-build step doing:

find . -name '*.exe' -o -name '*.dll'

would add a way to verify this in the logs.
https://github.com/curl/curl/blob/0acf0bcedad9208703cdbe91b06cacb1065a6e53/.github/workflows/windows.yml#L162

[botovq]

@vszakats There's an upload build artifact stage in our Windows CI. This one from #1103 shows that the major is actually appended:

$ find . -name '*.dll'
./crypto/Release/crypto-55.dll
./ssl/Release/ssl-58.dll
./tests/crypto-55.dll
./tests/ssl-58.dll
./tests/tls-31.dll
./tls/Release/tls-31.dll

whereas on HEAD the majors are absent.

Thanks for the pointers!

https://github.com/libressl/portable/actions/runs/11142580203/artifacts/2005442115

[vszakats]

@botovq Nice, that also does the job! It's looking good.

[busterb]

Good catch @botovq, sorry about that. I'm a little baffled how changing the minimum CMake check could actually impact how assembly code is generated though.

[botovq]

@busterb I suspect it has to do with implicitly enabling more cmake policies. Not sure what it could be though.

[botovq]

@busterb It compiles the assembly files with cc -x c, which of course cannot work. I suspect it has to do with https://cmake.org/cmake/help/latest/policy/CMP0056.html

Not sure how to fix this. Presumably we need to label the .S files as assembly files rather than C files somehow.

[botovq]

@busterb It's actually CMP0119 and we should probably look into using enable_language(ASM) rather than forcing the use of the C-compiler for some of the .S files.

[busterb]

Experimented with this, but couldn't find a combination that gets things building when CMP0119 is set to the new behavior.

[busterb]

Oh, figured out what you meant, testing in #1104

[botovq]

On Tue, Oct 08, 2024 at 02:52:22AM -0700, Brent Cook wrote: Oh, figured out what you meant, testing in #1104

Ah great. Was just about to send a very similar PR. Thanks!