Fix Windows arm64 (C23/clang) compile error in ClustalOmega hsregex.c

[jeroen]

This fixes the build on Windows ARM64 (clang): https://github.com/r-universe/bioc/actions/runs/30352617879/job/90324027659

The arm64 R-devel toolchain (rtools45, clang 19) compiles the ClustalOmega C sources with -std=gnu2x (C23), which no longer supports old-style (K&R) function definitions.

[UBod]

Thanks for pointing me to this issue! However, I will not merge this pull request. You suggested changes in the package's code in a part that has been included from other sources. Our policy is to leave foreign code unchanged as much as possible. I would be grateful if you found a fix that suffices with changing compiler options in some Makevars.win file. In that case, I would gratefully merge an update.

[jeroen]

I don't think that is possible. Your code uses defunct legacy syntax, that you will eventually have to convert.

[UBod]

In the past, issues like this one could be fixed by switches like -std=c11, -std=gnu11, or something like that in PKG_CFLAGS of msaMakefile.win, i.e. reverting to an older standard that still allows for K&R syntax. That is what I meant.

[jeroen]

https://cran.r-project.org/doc/manuals/r-devel/NEWS.html mentions you can put USE_C17 in SystemRequirements. It specifically mentions the case of K&R-style function declarations.

I haven't tried that, you could give it a go.

[UBod]

It seems to me that the windows-arm64 build now runs successfully: https://github.com/r-universe/bioc/actions/runs/30352617879

Please let me know if you can confirm that. If so, I will close this issue. Thanks anyway for keeping your eyes open!

[jeroen]

It runs because I am hotpatching it with the above patch (look at the first lines under the "build package msa" in the windows arm64 log https://github.com/r-universe/bioc/actions/runs/30352617879/job/90422239137

We do this as a temporary solution to help other packages that depend on msa, but I will remove this soon and then it will start failing (and also your dependents) if it isn't fixed.

[UBod]

Ok, I see. Sorry for causing you efforts! Can you try this instead? (sorry, I have no Win* environment to test myself)

diff --git a/src/ClustalOmega/msaMakefile.win b/src/ClustalOmega/msaMakefile.win
index 641988d..460b1f7 100755
--- a/src/ClustalOmega/msaMakefile.win
+++ b/src/ClustalOmega/msaMakefile.win
@@ -39,7 +39,7 @@ clustalomega:
        cp windows/src/clustal-omega-config.h src/; \
        export PKG_LIBS="$(PKG_LIBS) -L"../../gc-8.2.8" -lgccpp82 -lgc82"; \
        export PKG_CXXFLAGS="$(PKG_CXXFLAGS) -std=c++11 -DHAVE_CONFIG_H -I. -DCLUSTALO -DCLUSTALO_NOFILE -DDEFAULT_FILTER=90 -I../../gc-8.2.8/include"; \
-       export PKG_CFLAGS="$(PKG_CFLAGS) -DHAVE_CONFIG_H -I. -DCLUSTALO -DCLUSTALO_NOFILE -DDEFAULT_FILTER=90 -I../../gc-8.2.8/include -lgccpp82 -lgc82"; \
+       export PKG_CFLAGS="$(PKG_CFLAGS)  -std=c11 -DHAVE_CONFIG_H -I. -DCLUSTALO -DCLUSTALO_NOFILE -DDEFAULT_FILTER=90 -I../../gc-8.2.8/include -lgccpp82 -lgc82"; \
        cd src; \
        ${R_HOME}/bin${R_ARCH_BIN}/R.exe CMD SHLIB -o ClustalOmega.dll $(CPPNames) && \
        $(AR) rcs libClustalOmega.a $(OBJNames) && \

[UBod]

BTW, sorry for this ad-hoc approach. I didn't want to create a separate branch (partly because I am anything but a git pro ;-))

[jeroen]

I think usually you are not supposed to add -std=c11 in your CFLAGS because the default CC or CFLAGS may already contain a -std flag, for example my R has this:

 R CMD config CC17
# clang -arch arm64 -std=gnu17

So you would end up with multiple -std= delcarations, and I am not sure what would take precedense.

I think the easiest solution is what the documentation recommends and add USE_C17 to your SystemRequirements here:

msa/DESCRIPTION

Line 33 in 416162a

SystemRequirements: GNU make

Seemingly that will make R automatically use C17.

[UBod]

Thanks for your thoughts! I have used this approach in the msa package on several occasions, and I think - even though I understand your concerns - it has always worked nicely. Your idea of adding USE_C17 to the SystemRequirements has the downside that some other parts of the package (note that it includes four foreign C/C++ libraries from different sources) require other C/C++ standards (I have tried that before without success). Each library has separate Makefile(.win)/Makevars(.win) files that require different standards, which - as said - has worked fine so far. It is just that I had forgotten that in src/ClustalOmega/msaMakefile.win. So please give it a try.

[UBod]

Alternatively, I can also push my new version to the BioC devel repo and we will see if it solves the win/arm64 issue.

[UBod]

I have now pushed a new version 1.45.3 both to GitHub und the BioC git repo. If that works on Win/arm64 now without your patch, we are done. Otherwise, I will give in and merge your pull request. :-)

[jeroen]

I don't think it worked: https://github.com/r-universe/bioc/actions/runs/31162328460/job/92817287259

[UBod]

Ok, I have merged your pull request now. Are you retrieving the source code from GitHub or the BioC repo? I am asking because, in the former case, I would only push the new version to BioC if it works on R-universe. Thanks for your help and your patience!

[jeroen]

We pull from bioconductor. So if possible it would be great to also make the change to the RELEASE_3_23 branch...

[UBod]

Ok, I see. Unfortunately, I am currently moving offices. My Linux desktop, on which I am doing all my dev stuff, will only be available next Wednesday again (after the new office has been set up). I hope that is sufficient.