Skip to content
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

CI integration of NUT for Windows builds - go (semi-)native with MSYS2 provided clang on Windows #1651

Open
jimklimov opened this issue Sep 5, 2022 · 5 comments
Labels
CI Entries related to continuous integration infrastructure (historically also recipes like Makefiles) cross-builds refactor/fightwarn PR or issue proposal to improve code maintainability without functional changes, or to fix warnings Windows

Comments

@jimklimov
Copy link
Member

jimklimov commented Sep 5, 2022

For kicks, finally got around to try "standard-issue" pre-set environment "MSYS2 MinGW Clang x64" in the semi-native build environment, with current master branch (same as v2.8.0-Windows tag at the moment):

Prepare filenames (and ccache):

:; ( cd /usr/bin ; ln -fs clang-11 clang++-11 ; ln -fs clang-11 clang-cpp-11 ; )
:; ( cd /usr/lib/ccache ; ln -fsr /usr/bin/ccache clang-11 ; ln -fsr /usr/bin/ccache clang++-11 )

Then go NUT:

:; export CC=clang-11
:; export CXX=clang++-11
:; export CPP=clang-cpp-11
:; ./ci_build.sh

Let's see what pops out...

For related efforts and discussions, see also:

@jimklimov jimklimov added Windows CI Entries related to continuous integration infrastructure (historically also recipes like Makefiles) cross-builds labels Sep 5, 2022
@jimklimov
Copy link
Member Author

It compiled stuff almost well, except the C++ code where it failed to #include <string> :\ but otherwise without fatal complaints for Windows codebase.

However for linking it calls gcc and passes it CFLAGS arguments intended for clang, so linking fails, e.g.

libtool: link: clang-11 -I../include -IC:/msys64/clang64/include/libusb-compat -IC:/msys64/clang64/include/libusb-1.0 -IC:/msys64/mingw64/bin/../include/neon -IC:/msys64/clang64/include/modbus -I../clients -g -O2 -Qunused-arguments -fdiagnostics-color=always -Wno-unknown-warning-option -std=gnu99 -ferror-limit=0 -Wno-system-headers -Wall -Wextra -Weverything -Wno-disabled-macro-expansion -Wno-unused-macros -Wno-reserved-id-macro -Wno-padded -Wno-documentation -Wno-cast-qual -pedantic -Wno-float-conversion -Wno-double-promotion -Wno-implicit-float-conversion -Wno-conversion -Wno-incompatible-pointer-types-discards-qualifiers -Wno-format -Wno-error -o .libs/dummy-ups.exe dummy_ups-dummy-ups.o  ./.libs/libdummy.a ../common/.libs/libcommon.a ../common/.libs/libparseconf.a ../clients/.libs/libupsclient.dll.a -L/clang64/lib
x86_64-pc-msys-gcc: error: unrecognized command-line option '-ferror-limit=0'
clang-11: error: linker (via gcc) command failed with exit code 1 (use -v to see invocation)
make[1]: *** [Makefile:1498: dummy-ups.exe] Error 1

@jimklimov
Copy link
Member Author

Retrying with a hack in local workspace to just avoid that option so far (needed for CLANG to report all issues and not bail out after 10 or 20), and to keep reporting all warnings as errors in the quick ci_build.sh scenario:

diff --git a/ci_build.sh b/ci_build.sh
index 111a4d7af..193506ed2 100755
--- a/ci_build.sh
+++ b/ci_build.sh
@@ -1774,6 +1774,7 @@ bindings)
     # enable whatever is auto-detectable (except docs), and highlight
     # any warnings if we can.
     ${CONFIGURE_SCRIPT} --enable-Wcolor \
+           --enable-Werror \
         --with-all=auto --with-cgi=auto --with-serial=auto \
         --with-dev=auto --with-doc=skip \
         --with-nut_monitor=auto --with-pynut=auto \
diff --git a/configure.ac b/configure.ac
index db1f33c90..5e2cf14c2 100644
--- a/configure.ac
+++ b/configure.ac
@@ -3234,16 +3234,16 @@ AS_CASE(["${nut_enable_warnings}"],
         CXXFLAGS="${CXXFLAGS} -Wall"
         ],
     [clang-hard], [
-        CFLAGS="${CFLAGS} -ferror-limit=0 -Wno-system-headers -Wall -Wextra -Weverything -Wno-disabled-macro-expansion -Wno-unused-macros -Wno-reserved-id-macro -Wno-padded -Wno-documentation -Wno-cast-qual -pedantic"
-        CXXFLAGS="${CXXFLAGS} -ferror-limit=0 -Wno-system-headers -Wall -Wextra -Weverything -Wno-disabled-macro-expansion -Wno-unused-macros -Wno-reserved-id-macro -Wno-padded -Wno-documentation -Wno-cast-qual -Wno-c++98-compat-pedantic -Wno-c++98-compat"
+        CFLAGS="${CFLAGS} -Wno-system-headers -Wall -Wextra -Weverything -Wno-disabled-macro-expansion -Wno-unused-macros -Wno-reserved-id-macro -Wno-padded -Wno-documentation -Wno-cast-qual -pedantic"
+        CXXFLAGS="${CXXFLAGS} -Wno-system-headers -Wall -Wextra -Weverything -Wno-disabled-macro-expansion -Wno-unused-macros -Wno-reserved-id-macro -Wno-padded -Wno-documentation -Wno-cast-qual -Wno-c++98-compat-pedantic -Wno-c++98-compat"
         ],
     [clang-medium], [
-        CFLAGS="${CFLAGS} -ferror-limit=0 -Wno-system-headers -Wall -Wextra -Weverything -Wno-disabled-macro-expansion -Wno-unused-macros -Wno-reserved-id-macro -Wno-padded -Wno-documentation -Wno-cast-qual -pedantic -Wno-float-conversion -Wno-double-promotion -Wno-implicit-float-conversion -Wno-conversion -Wno-incompatible-pointer-types-discards-qualifiers"
-        CXXFLAGS="${CXXFLAGS} -ferror-limit=0 -Wno-system-headers -Wall -Wextra -Weverything -Wno-disabled-macro-expansion -Wno-unused-macros -Wno-reserved-id-macro -Wno-padded -Wno-documentation -Wno-cast-qual -Wno-c++98-compat-pedantic -Wno-c++98-compat"
+        CFLAGS="${CFLAGS} -Wno-system-headers -Wall -Wextra -Weverything -Wno-disabled-macro-expansion -Wno-unused-macros -Wno-reserved-id-macro -Wno-padded -Wno-documentation -Wno-cast-qual -pedantic -Wno-float-conversion -Wno-double-promotion -Wno-implicit-float-conversion -Wno-conversion -Wno-incompatible-pointer-types-discards-qualifiers"
+        CXXFLAGS="${CXXFLAGS} -Wno-system-headers -Wall -Wextra -Weverything -Wno-disabled-macro-expansion -Wno-unused-macros -Wno-reserved-id-macro -Wno-padded -Wno-documentation -Wno-cast-qual -Wno-c++98-compat-pedantic -Wno-c++98-compat"
         ],
     [clang-minimal], [
-        CFLAGS="${CFLAGS} -ferror-limit=0 -Wall -Wextra -Wno-documentation"
-        CXXFLAGS="${CXXFLAGS} -ferror-limit=0 -Wall -Wextra -Wno-documentation"
+        CFLAGS="${CFLAGS} -Wall -Wextra -Wno-documentation"
+        CXXFLAGS="${CXXFLAGS} -Wall -Wextra -Wno-documentation"
         ],
     [gcc-legacy], [CFLAGS="${CFLAGS} -Wall -Wsign-compare"],
     [gcc-minimal], [

...and waiting for results...

@jimklimov jimklimov added the refactor/fightwarn PR or issue proposal to improve code maintainability without functional changes, or to fix warnings label Sep 5, 2022
@jimklimov
Copy link
Member Author

jimklimov commented Sep 5, 2022

Verdicts seen so far:

  • C++ include <string> for some reason fails here; perhaps configure script should check for it as a factor whether the platform is able to build C++(11) code.
    • UPDATE: The file actually seems to be there, with FS structured similar to GCC builds (but very differently sized, 4Kb vs 168Kb), so possibly the location is just not exposed/used by compiler as its system-include path:
$ find /clang64 -name string
/clang64/include/c++/v1/experimental/string
/clang64/include/c++/v1/string

$ find /mingw64 -name string
/mingw64/include/c++/12.1.0/debug/string
/mingw64/include/c++/12.1.0/experimental/string
/mingw64/include/c++/12.1.0/string
  • genericups: use of serial macros not known to the OS (we had fallbacks in common.h IIRC):
In file included from genericups.c:30:
./genericups.h:176:4: error: use of undeclared identifier 'TIOCM_ST'
          TIOCM_ST                      /* shutdown: ST                 */
          ^
./genericups.h:200:4: error: use of undeclared identifier 'TIOCM_ST'
          TIOCM_ST                      /* shutdown: TX BREA            */
          ^
./genericups.h:224:4: error: use of undeclared identifier 'TIOCM_ST'
          TIOCM_ST                      /* shutdown: ST (break)         */
          ^
./genericups.h:285:4: error: use of undeclared identifier 'TIOCM_ST'
          TIOCM_ST                      /* shutdown: ST (break)         */
          ^
genericups.c:73:12: error: use of undeclared identifier 'TIOCM_ST'
                *line |= TIOCM_ST;
                         ^
genericups.c:325:15: error: use of undeclared identifier 'TIOCM_ST'
        if (flags == TIOCM_ST) {
                     ^
6 errors generated.
  • Lots of noise hides "our" errors due to libtool wrappers (./.libs/lt-*.c) that seem poorly generated - not our bug, but maybe something to hack warnings for (and report to upstream) in our recipes:
    • some vars not declared static;
    • use of _spawnv() not exposed by headers used:
./.libs/lt-upssched.c:319:16: error: implicit declaration of function '_spawnv' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
  rval = (int) _spawnv (_P_WAIT, lt_argv_zero, (const char * const *) newargz);
* use of non-literal format strings (check, cover with pragmas to ignore `-Wformat-nonliteral` if this is intentional)

Typically the same stack is seen for the wrappers:

libtool: link: clang-11 -I../include -IC:/msys64/clang64/include/libusb-compat -IC:/msys64/clang64/include/libusb-1.0 -IC:/msys64/mingw64/bin/../include/neon -IC:/msys64/clang64/include/modbus -g -O2 -Qunused-arguments -fdiagnostics-color=always -Wno-unknown-warning-option -std=gnu99 -Wno-system-headers -Wall -Wextra -Weverything -Wno-disabled-macro-expansion -Wno-unused-macros -Wno-reserved-id-macro -Wno-padded -Wno-documentation -Wno-cast-qual -pedantic -Wno-float-conversion -Wno-double-promotion -Wno-implicit-float-conversion -Wno-conversion -Wno-incompatible-pointer-types-discards-qualifiers -Wno-format -Werror -o .libs/skel.exe skel.o  ./.libs/libdummy.a ../common/.libs/libcommon.a ../common/.libs/libparseconf.a
./.libs/lt-skel.c:137:13: error: no previous extern declaration for non-static variable 'program_name' [-Werror,-Wmissing-variable-declarations]
const char *program_name = "libtool-wrapper"; /* in case xstrdup fails */
            ^
./.libs/lt-skel.c:137:7: note: declare 'static' if the variable is not intended to be used outside of this translation unit
const char *program_name = "libtool-wrapper"; /* in case xstrdup fails */
      ^
./.libs/lt-skel.c:162:33: error: no previous extern declaration for non-static variable 'MAGIC_EXE' [-Werror,-Wmissing-variable-declarations]
externally_visible const char * MAGIC_EXE = "%%%MAGIC EXE variable%%%";
                                ^
./.libs/lt-skel.c:162:26: note: declare 'static' if the variable is not intended to be used outside of this translation unit
externally_visible const char * MAGIC_EXE = "%%%MAGIC EXE variable%%%";
                         ^
./.libs/lt-skel.c:163:14: error: no previous extern declaration for non-static variable 'LIB_PATH_VARNAME' [-Werror,-Wmissing-variable-declarations]
const char * LIB_PATH_VARNAME = "PATH";
             ^
./.libs/lt-skel.c:163:7: note: declare 'static' if the variable is not intended to be used outside of this translation unit
const char * LIB_PATH_VARNAME = "PATH";
      ^
./.libs/lt-skel.c:164:14: error: no previous extern declaration for non-static variable 'LIB_PATH_VALUE' [-Werror,-Wmissing-variable-declarations]
const char * LIB_PATH_VALUE   = "";
             ^
./.libs/lt-skel.c:164:7: note: declare 'static' if the variable is not intended to be used outside of this translation unit
const char * LIB_PATH_VALUE   = "";
      ^
./.libs/lt-skel.c:165:14: error: no previous extern declaration for non-static variable 'EXE_PATH_VARNAME' [-Werror,-Wmissing-variable-declarations]
const char * EXE_PATH_VARNAME = "";
             ^
./.libs/lt-skel.c:165:7: note: declare 'static' if the variable is not intended to be used outside of this translation unit
const char * EXE_PATH_VARNAME = "";
      ^
./.libs/lt-skel.c:166:14: error: no previous extern declaration for non-static variable 'EXE_PATH_VALUE' [-Werror,-Wmissing-variable-declarations]
const char * EXE_PATH_VALUE   = "";
             ^
./.libs/lt-skel.c:166:7: note: declare 'static' if the variable is not intended to be used outside of this translation unit
const char * EXE_PATH_VALUE   = "";
      ^
./.libs/lt-skel.c:167:14: error: no previous extern declaration for non-static variable 'TARGET_PROGRAM_NAME' [-Werror,-Wmissing-variable-declarations]
const char * TARGET_PROGRAM_NAME = "skel.exe"; /* hopefully, no .exe */
             ^
./.libs/lt-skel.c:167:7: note: declare 'static' if the variable is not intended to be used outside of this translation unit
const char * TARGET_PROGRAM_NAME = "skel.exe"; /* hopefully, no .exe */
      ^
./.libs/lt-skel.c:319:16: error: implicit declaration of function '_spawnv' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
  rval = (int) _spawnv (_P_WAIT, lt_argv_zero, (const char * const *) newargz);
               ^
./.libs/lt-skel.c:319:16: note: did you mean 'spawnv'?
/usr/include/process.h:24:5: note: 'spawnv' declared here
int spawnv(int mode, const char *path, const char * const *argv);
    ^
./.libs/lt-skel.c:596:32: error: format string is not a string literal [-Werror,-Wformat-nonliteral]
      (void) vfprintf (stderr, fmt, args);
                               ^~~
./.libs/lt-skel.c:607:21: error: format string is not a string literal [-Werror,-Wformat-nonliteral]
  vfprintf (stderr, message, ap);
                    ^~~~~~~
10 errors generated.
C:\msys64\mingw64\bin\strip.exe: './skel.exe': No such file
../libtool: line 11142: ./skel.exe: No such file or directory

@jimklimov
Copy link
Member Author

jimklimov commented Sep 5, 2022

We probably need a relaxed LT_CFLAGS (to retain CFLAGS other than those set for --enable-warnings and --enable-Werror) in our makefiles.

At least hacking into generated libtool script to neuter -Wno-error gets it a bit further, to fail with linker:

/usr/lib/gcc/x86_64-pc-msys/11.3.0/../../../../x86_64-pc-msys/bin/ld: /tmp/lt-upsc-3b5979.o: in function `main':
/home/klimov/nut-win-clang/clients/./.libs/lt-upsc.c:322: undefined reference to `_spawnv'
collect2: error: ld returned 1 exit status
clang-11: error: linker (via gcc) command failed with exit code 1 (use -v to see invocation)
C:\msys64\mingw64\bin\strip.exe: './upsc.exe': No such file

Further replacing _spawnv with spawn and including <process.h> let the build pass this hiccup with just warnings (fixable by prefixing static to vars noted above - just not sure about MAGIC_EXE which is externally_visible so maybe needs extern beside attributes used now) -- all probably bugs for autotools and/or msys2 projects to address. I did not track down where vprintf was involved.

Finally a re-run with make -ks should provide issues actually relevant to NUT codebase, confirming that the two above (C++ and genericups) are the only ones this clang-11 disliked. Which is even a bit disappointing, given how much llvm-mingw clang-15 complains about (see #1650).

@jimklimov
Copy link
Member Author

jimklimov commented Sep 6, 2022

For now throwing in a towel at this on my side, it was not a low hanging fruit and I have some higher priorities at the time.

A quick fix attempt by setting and AC_SUBST'ing either LTCFLAGS or LT_CFLAGS in the configure.ac (conserving the CFLAGS value before we add warnings flags) did not propagate into the generated libtool script - it still defines LTCFLAGS as the full CFLAGS determined by the configure script. Maybe some macro similar to AM_CONDITIONAL is needed to inject/override it into the Makefile.am processing (assuming this is where libtool takes the string from eventually)?..

Not sure how to easily detect that clang uses gcc as the linker (other than trying to link in configure and looking at success and/or stderr text?) so as to not use flags incompatible with GCC in this scenario. Note that a similar investigation in #1647/#1650 uses (I think) a complete llvm-mingw clang stack including the lld so should not suffer from this nuance. And this is not a problem I've ever seen before with "purely POSIX" builds.

FWIW the anti-warning C code fixes above (adding static, headers, spawnv, etc.) belong in lt_main.sh and then applied by make libtool.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Entries related to continuous integration infrastructure (historically also recipes like Makefiles) cross-builds refactor/fightwarn PR or issue proposal to improve code maintainability without functional changes, or to fix warnings Windows
Projects
Status: Todo
Development

No branches or pull requests

1 participant