From e785e898a6a32fc63b35615b55a147d309082f3d Mon Sep 17 00:00:00 2001 From: Viktor Szakats Date: Sat, 17 May 2025 02:02:23 +0200 Subject: [PATCH] checksrc: avoid extra runs in CI, enable more check locally, fix fallouts To avoid redundant work in CI and to avoid a single checksrc issue make all autotools jobs fail. After this patch checksrc issues make fail the checksrc job, the `dist / verify-out-of-tree-autotools-debug`, `dist / maketgz-and-verify-in-tree` jobs and the fuzzer job (if run). Of these, the `dist` jobs replicate local builds, also testing the build logic. Also add a script to check the complete local repository, optionally with the build tree to verify generated C files. Also: - automatically run checksrc in subdirectories having a `checksrc` target. (examples, OS400, tests http/client, unit and tunit) - tests/libtest: make sure to run `checksrc` on generated `lib1521.c`. (requires in-tree autotools build.) - tests: run `checksrc` on targets also for non-`DEBUGBUILD` builds. It ensures to check `lib1521.c` in CI via job `dist / maketgz-and-verify-in-tree`. - src: drop redundant `$(builddir)` in autotools builds. - scripts: add `checksrc-all.sh` script to check all C sources and the build directory as an option. - use the above from CI, also make it verify all generated sources. - silence `checksrc` issues in generated C sources. - checksrc: add `-v` option to enable verbose mode. - checksrc: make verbose mode show checked filename and fix to only return error on failure. - make sure that generated C files pass `checksrc`. Assisted-by: Daniel Stenberg Closes #17376 --- .github/workflows/checksrc.yml | 2 +- .github/workflows/distcheck.yml | 2 ++ .github/workflows/http3-linux.yml | 2 +- .github/workflows/linux-old.yml | 1 + .github/workflows/linux.yml | 2 +- .github/workflows/macos.yml | 2 +- .github/workflows/non-native.yml | 5 +++++ .github/workflows/windows.yml | 3 +++ configure.ac | 3 +++ docs/cmdline-opts/pinnedpubkey.md | 5 +++-- docs/examples/Makefile.am | 7 +++++++ include/curl/Makefile.am | 2 ++ lib/Makefile.am | 2 ++ packages/Makefile.am | 7 +++++++ scripts/Makefile.am | 6 +++--- scripts/checksrc-all.sh | 23 +++++++++++++++++++++++ scripts/checksrc.pl | 17 +++++++++++++++-- src/CMakeLists.txt | 3 +++ src/Makefile.am | 28 +++++++++++++++++++--------- src/mk-file-embed.pl | 1 + tests/http/clients/Makefile.am | 4 ++++ tests/libtest/Makefile.am | 5 ++--- tests/mk-bundle.pl | 1 + tests/server/Makefile.am | 3 +-- tests/tunit/Makefile.am | 4 ++++ tests/unit/Makefile.am | 4 ++++ 26 files changed, 119 insertions(+), 25 deletions(-) create mode 100755 scripts/checksrc-all.sh diff --git a/.github/workflows/checksrc.yml b/.github/workflows/checksrc.yml index 4083cc3c56..990551aeab 100644 --- a/.github/workflows/checksrc.yml +++ b/.github/workflows/checksrc.yml @@ -41,7 +41,7 @@ jobs: name: checkout - name: check - run: git ls-files -z "*.[ch]" | xargs -0 -n1 ./scripts/checksrc.pl + run: scripts/checksrc-all.sh codespell-cmakelint-pytype-ruff: runs-on: ubuntu-latest diff --git a/.github/workflows/distcheck.yml b/.github/workflows/distcheck.yml index 20d4a7aa3d..c851de0cd3 100644 --- a/.github/workflows/distcheck.yml +++ b/.github/workflows/distcheck.yml @@ -109,6 +109,8 @@ jobs: make make test-ci make install + popd + scripts/checksrc-all.sh verify-out-of-tree-cmake: runs-on: ubuntu-latest diff --git a/.github/workflows/http3-linux.yml b/.github/workflows/http3-linux.yml index 80cae92d43..fd1ecbcd54 100644 --- a/.github/workflows/http3-linux.yml +++ b/.github/workflows/http3-linux.yml @@ -38,6 +38,7 @@ permissions: {} env: MAKEFLAGS: -j 5 + CURL_CI: github # handled in renovate.json openssl-version: 3.5.0 # handled in renovate.json @@ -527,7 +528,6 @@ jobs: - name: 'run pytest event based' env: CURL_TEST_EVENT: 1 - CURL_CI: github PYTEST_ADDOPTS: '--color=yes' PYTEST_XDIST_AUTO_NUM_WORKERS: 4 run: | diff --git a/.github/workflows/linux-old.yml b/.github/workflows/linux-old.yml index 8ad3bc00d3..6d7c6a8b39 100644 --- a/.github/workflows/linux-old.yml +++ b/.github/workflows/linux-old.yml @@ -47,6 +47,7 @@ permissions: {} env: MAKEFLAGS: -j 5 + CURL_CI: github DEBIAN_FRONTEND: noninteractive jobs: diff --git a/.github/workflows/linux.yml b/.github/workflows/linux.yml index 52328ba478..295d8a6378 100644 --- a/.github/workflows/linux.yml +++ b/.github/workflows/linux.yml @@ -37,6 +37,7 @@ permissions: {} env: MAKEFLAGS: -j 5 + CURL_CI: github CURL_CLANG_TIDYFLAGS: '-checks=-clang-analyzer-security.insecureAPI.strcpy,-clang-analyzer-optin.performance.Padding,-clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling,-clang-analyzer-valist.Uninitialized' # unhandled bearssl-version: 0.6 @@ -698,7 +699,6 @@ jobs: - name: 'run pytest' if: contains(matrix.build.install_steps, 'pytest') env: - CURL_CI: github PYTEST_ADDOPTS: '--color=yes' PYTEST_XDIST_AUTO_NUM_WORKERS: 4 run: | diff --git a/.github/workflows/macos.yml b/.github/workflows/macos.yml index 0f619cee92..c4e558bb11 100644 --- a/.github/workflows/macos.yml +++ b/.github/workflows/macos.yml @@ -47,6 +47,7 @@ permissions: {} # newer than the 10.8 required by `CFURLCreateDataAndPropertiesFromResource`. env: + CURL_CI: github MAKEFLAGS: -j 4 LDFLAGS: -w # suppress 'object file was built for newer macOS version than being linked' warnings @@ -338,7 +339,6 @@ jobs: - name: 'run pytest' if: ${{ !matrix.build.clang-tidy && contains(matrix.build.install_steps, 'pytest') }} env: - CURL_CI: github PYTEST_ADDOPTS: '--color=yes' PYTEST_XDIST_AUTO_NUM_WORKERS: 4 run: | diff --git a/.github/workflows/non-native.yml b/.github/workflows/non-native.yml index 64dde730d5..eb49a87010 100644 --- a/.github/workflows/non-native.yml +++ b/.github/workflows/non-native.yml @@ -35,6 +35,9 @@ concurrency: permissions: {} +env: + CURL_CI: github + jobs: netbsd: name: 'NetBSD, CM clang openssl ${{ matrix.arch }}' @@ -141,6 +144,7 @@ jobs: architecture: ${{ matrix.arch }} run: | export MAKEFLAGS=-j3 + export CURL_CI=github # https://ports.freebsd.org/ time sudo pkg install -y autoconf automake libtool \ pkgconf brotli openldap26-client libidn2 libnghttp2 stunnel py311-impacket @@ -229,6 +233,7 @@ jobs: set -e ln -s /usr/bin/gcpp /usr/bin/cpp # Some tests expect `cpp`, which is named `gcpp` in this env. export MAKEFLAGS=-j3 + export CURL_CI=github time autoreconf -fi mkdir bld && cd bld && time ../configure --enable-unity --enable-test-bundles --enable-debug --enable-warnings --enable-werror \ --prefix="${HOME}"/install \ diff --git a/.github/workflows/windows.yml b/.github/workflows/windows.yml index 0a8b813025..f4b2ad50e4 100644 --- a/.github/workflows/windows.yml +++ b/.github/workflows/windows.yml @@ -35,6 +35,9 @@ concurrency: permissions: {} +env: + CURL_CI: github + jobs: cygwin: name: "cygwin, ${{ matrix.build == 'cmake' && 'CM' || 'AM' }} ${{ matrix.platform }} ${{ matrix.name }}" diff --git a/configure.ac b/configure.ac index dd88ea5e30..8a91594a5c 100644 --- a/configure.ac +++ b/configure.ac @@ -39,6 +39,7 @@ terms of the curl license; see COPYING for more details]) AC_CONFIG_SRCDIR([lib/urldata.h]) AC_CONFIG_HEADERS(lib/curl_config.h) +AH_TOP([/* !checksrc! disable COPYRIGHT all */]) AC_CONFIG_MACRO_DIR([m4]) AM_MAINTAINER_MODE m4_ifdef([AM_SILENT_RULES], [AM_SILENT_RULES([yes])]) @@ -453,6 +454,8 @@ XC_LIBTOOL LT_LANG([Windows Resource]) +AM_CONDITIONAL(NOT_CURL_CI, test -z "$CURL_CI") + # # Automake conditionals based on libtool related checks # diff --git a/docs/cmdline-opts/pinnedpubkey.md b/docs/cmdline-opts/pinnedpubkey.md index d21a18f693..fab43456c5 100644 --- a/docs/cmdline-opts/pinnedpubkey.md +++ b/docs/cmdline-opts/pinnedpubkey.md @@ -32,8 +32,9 @@ together then the peer is still verified by public key. PEM/DER support: -OpenSSL and GnuTLS (added in 7.39.0), wolfSSL (added in 7.43.0), mbedTLS -(added in 7.47.0), Secure Transport macOS 10.7+/iOS 10+ (added in 7.54.1), +OpenSSL and GnuTLS (added in 7.39.0), wolfSSL (added in 7.43.0), +mbedTLS (added in 7.47.0), +Secure Transport macOS 10.7+/iOS 10+ (added in 7.54.1), Schannel (added in 7.58.1) sha256 support: diff --git a/docs/examples/Makefile.am b/docs/examples/Makefile.am index 02ba20b99f..042922f1c4 100644 --- a/docs/examples/Makefile.am +++ b/docs/examples/Makefile.am @@ -69,3 +69,10 @@ CS_ = $(CS_0) checksrc: $(CHECKSRC)(@PERL@ $(top_srcdir)/scripts/checksrc.pl -D$(srcdir) $(srcdir)/*.c) + +if NOT_CURL_CI +if DEBUGBUILD +# for debug builds, we scan the sources on all regular make invokes +all-local: checksrc +endif +endif diff --git a/include/curl/Makefile.am b/include/curl/Makefile.am index 8864d60e1f..f664f6ab7e 100644 --- a/include/curl/Makefile.am +++ b/include/curl/Makefile.am @@ -35,7 +35,9 @@ CS_ = $(CS_0) checksrc: $(CHECKSRC)@PERL@ $(top_srcdir)/scripts/checksrc.pl -D$(top_srcdir)/include/curl $(pkginclude_HEADERS) +if NOT_CURL_CI if DEBUGBUILD # for debug builds, we scan the sources on all regular make invokes all-local: checksrc endif +endif diff --git a/lib/Makefile.am b/lib/Makefile.am index 9c26d4d667..8dac236606 100644 --- a/lib/Makefile.am +++ b/lib/Makefile.am @@ -163,10 +163,12 @@ checksrc: $(CHECKSRC)(@PERL@ $(top_srcdir)/scripts/checksrc.pl -D$(srcdir) \ $(CSOURCES) $(HHEADERS)) +if NOT_CURL_CI if DEBUGBUILD # for debug builds, we scan the sources on all regular make invokes all-local: checksrc endif +endif # disable the tests that are mostly causing false positives TIDYFLAGS := -checks=-clang-analyzer-security.insecureAPI.strcpy,-clang-analyzer-optin.performance.Padding,-clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling -quiet diff --git a/packages/Makefile.am b/packages/Makefile.am index 96f52bdb4c..43e544a019 100644 --- a/packages/Makefile.am +++ b/packages/Makefile.am @@ -50,3 +50,10 @@ CS_ = $(CS_0) checksrc: $(CHECKSRC)(@PERL@ $(top_srcdir)/scripts/checksrc.pl -D$(srcdir) $(srcdir)/OS400/*.[ch]) + +if NOT_CURL_CI +if DEBUGBUILD +# for debug builds, we scan the sources on all regular make invokes +all-local: checksrc +endif +endif diff --git a/scripts/Makefile.am b/scripts/Makefile.am index 1ad250ae31..03eb8044ad 100644 --- a/scripts/Makefile.am +++ b/scripts/Makefile.am @@ -22,9 +22,9 @@ # ########################################################################### -EXTRA_DIST = coverage.sh completion.pl firefox-db2pem.sh checksrc.pl \ - mk-ca-bundle.pl mk-unity.pl schemetable.c cd2nroff nroff2cd cdall cd2cd managen \ - dmaketgz maketgz release-tools.sh verify-release cmakelint.sh mdlinkcheck \ +EXTRA_DIST = coverage.sh completion.pl firefox-db2pem.sh checksrc.pl checksrc-all.sh \ + mk-ca-bundle.pl mk-unity.pl schemetable.c cd2nroff nroff2cd cdall cd2cd managen \ + dmaketgz maketgz release-tools.sh verify-release cmakelint.sh mdlinkcheck \ CMakeLists.txt pythonlint.sh randdisable wcurl dist_bin_SCRIPTS = wcurl diff --git a/scripts/checksrc-all.sh b/scripts/checksrc-all.sh new file mode 100755 index 0000000000..2c4b3c5e28 --- /dev/null +++ b/scripts/checksrc-all.sh @@ -0,0 +1,23 @@ +#!/bin/sh +# Copyright (C) Viktor Szakats +# +# SPDX-License-Identifier: curl + +set -eu + +anyfailed=0 + +for dir in $({ + if git rev-parse --is-inside-work-tree >/dev/null 2>&1; then + git ls-files '*.[ch]' + else + find . -name '*.[ch]' + fi + [ -n "${1:-}" ] && find "$@" -name '*.[ch]' + } | grep -v -F '/CMakeFiles/' | sed -E 's|/[^/]+$||' | sort -u); do + if ! ./scripts/checksrc.pl "${dir}"/*.[ch]; then + anyfailed=1 + fi +done + +exit "${anyfailed}" diff --git a/scripts/checksrc.pl b/scripts/checksrc.pl index 68189b376f..f62962ff1a 100755 --- a/scripts/checksrc.pl +++ b/scripts/checksrc.pl @@ -39,7 +39,7 @@ my $dir="."; my $wlist=""; my @alist; my $windows_os = $^O eq 'MSWin32' || $^O eq 'cygwin' || $^O eq 'msys'; -my $verbose; +my $verbose = 0; my %skiplist; my %ignore; @@ -288,6 +288,11 @@ while(defined $file) { $file = shift @ARGV; next; } + elsif($file =~ /^-v/) { + $verbose = 1; + $file = shift @ARGV; + next; + } elsif($file =~ /^(-h|--help)/) { undef $file; last; @@ -307,6 +312,7 @@ if(!$file) { print " -W[file] Skip the given file - ignore all its flaws\n"; print " -i Indent spaces. Default: 2\n"; print " -m Maximum line length. Default: 79\n"; + print " -v Verbose\n"; print "\nDetects and warns for these problems:\n"; my @allw = keys %warnings; push @allw, keys %warnings_extended; @@ -448,6 +454,11 @@ sub scanfile { my $l = ""; my $prep = 0; my $prevp = 0; + + if($verbose) { + printf "Checking file: $file\n"; + } + open(my $R, '<', $file) || die "failed to open $file"; my $incomment=0; @@ -1123,5 +1134,7 @@ if($errors || $warnings || $verbose) { $serrors, $swarnings; } - exit 5; # return failure + if($errors || $warnings) { + exit 5; # return failure + } } diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 469df6d761..9cae87b01d 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -33,6 +33,9 @@ if(ENABLE_CURL_MANUAL AND HAVE_MANUAL_TOOLS) add_custom_command( OUTPUT "tool_hugehelp.c" COMMAND ${CMAKE_COMMAND} -E echo "#include \"tool_setup.h\"" > "tool_hugehelp.c" + COMMAND ${CMAKE_COMMAND} -E echo "/* !checksrc! disable COPYRIGHT all */" >> "tool_hugehelp.c" + COMMAND ${CMAKE_COMMAND} -E echo "/* !checksrc! disable INCLUDEDUP all */" >> "tool_hugehelp.c" + COMMAND ${CMAKE_COMMAND} -E echo "/* !checksrc! disable LONGLINE all */" >> "tool_hugehelp.c" COMMAND ${CMAKE_COMMAND} -E echo "#ifndef HAVE_LIBZ" >> "tool_hugehelp.c" COMMAND "${PERL_EXECUTABLE}" "${CMAKE_CURRENT_SOURCE_DIR}/mkhelp.pl" < "${CURL_ASCIIPAGE}" >> "tool_hugehelp.c" COMMAND ${CMAKE_COMMAND} -E echo "#else" >> "tool_hugehelp.c" diff --git a/src/Makefile.am b/src/Makefile.am index 5e84a20b14..2df95a0f94 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -125,7 +125,7 @@ endif # Use absolute directory to disable VPATH ASCIIPAGE=$(top_builddir)/docs/cmdline-opts/curl.txt MKHELP=$(top_srcdir)/src/mkhelp.pl -HUGE=$(builddir)/tool_hugehelp.c +HUGE=tool_hugehelp.c HUGECMD = $(HUGEIT_$(V)) HUGEIT_0 = @echo " HUGE " $@; @@ -145,22 +145,29 @@ if HAVE_LIBZ # This generates the tool_hugehelp.c file in both uncompressed and # compressed formats. $(HUGE): $(ASCIIPAGE) $(MKHELP) - $(HUGECMD) (echo '#include "tool_setup.h"' > $(HUGE); \ - echo '#ifndef HAVE_LIBZ' >> $(HUGE); \ - $(PERL) $(MKHELP) < $(ASCIIPAGE) >> $(HUGE); \ - echo '#else' >> $(HUGE); \ - $(PERL) $(MKHELP) -c < $(ASCIIPAGE) >> $(HUGE); \ + $(HUGECMD)( \ + echo '/* !checksrc! disable COPYRIGHT all */' > $(HUGE); \ + echo '/* !checksrc! disable INCLUDEDUP all */' >> $(HUGE); \ + echo '/* !checksrc! disable LONGLINE all */' >> $(HUGE); \ + echo '#include "tool_setup.h"' >> $(HUGE); \ + echo '#ifndef HAVE_LIBZ' >> $(HUGE); \ + $(PERL) $(MKHELP) < $(ASCIIPAGE) >> $(HUGE); \ + echo '#else' >> $(HUGE); \ + $(PERL) $(MKHELP) -c < $(ASCIIPAGE) >> $(HUGE); \ echo '#endif /* HAVE_LIBZ */' >> $(HUGE) ) else # HAVE_LIBZ # This generates the tool_hugehelp.c file uncompressed only $(HUGE): $(ASCIIPAGE) $(MKHELP) - $(HUGECMD)(echo '#include "tool_setup.h"' > $(HUGE); \ + $(HUGECMD)( \ + echo '/* !checksrc! disable COPYRIGHT all */' > $(HUGE); \ + echo '#include "tool_setup.h"' >> $(HUGE); \ $(PERL) $(MKHELP) < $(ASCIIPAGE) >> $(HUGE) ) endif else # USE_MANUAL # built-in manual has been disabled, make a blank file $(HUGE): + echo '/* !checksrc! disable COPYRIGHT all */' > $(HUGE); \ echo '#include "tool_hugehelp.h"' >> $(HUGE) endif @@ -168,7 +175,7 @@ curl_cfiles_gen += $(HUGE) curl_hfiles_gen += tool_hugehelp.h CLEANFILES += $(HUGE) -CA_EMBED_CSOURCE = $(builddir)/tool_ca_embed.c +CA_EMBED_CSOURCE = tool_ca_embed.c curl_cfiles_gen += $(CA_EMBED_CSOURCE) CLEANFILES += $(CA_EMBED_CSOURCE) if CURL_CA_EMBED_SET @@ -178,7 +185,8 @@ $(CA_EMBED_CSOURCE): $(MK_FILE_EMBED) $(CURL_CA_EMBED) $(PERL) $(MK_FILE_EMBED) --var curl_ca_embed < $(CURL_CA_EMBED) > $(CA_EMBED_CSOURCE) else $(CA_EMBED_CSOURCE): - echo 'extern const void *curl_ca_embed; const void *curl_ca_embed;' > $(CA_EMBED_CSOURCE) + echo '/* !checksrc! disable COPYRIGHT all */' > $(CA_EMBED_CSOURCE) + echo 'extern const void *curl_ca_embed; const void *curl_ca_embed;' >> $(CA_EMBED_CSOURCE) endif CHECKSRC = $(CS_$(V)) @@ -190,10 +198,12 @@ CS_ = $(CS_0) checksrc: $(CHECKSRC)(@PERL@ $(top_srcdir)/scripts/checksrc.pl -D$(srcdir) $(CURL_CFILES) $(CURL_HFILES)) +if NOT_CURL_CI if DEBUGBUILD # for debug builds, we scan the sources on all regular make invokes all-local: checksrc endif +endif # disable the tests that are mostly causing false positives TIDYFLAGS := -checks=-clang-analyzer-security.insecureAPI.strcpy,-clang-analyzer-optin.performance.Padding,-clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling -quiet diff --git a/src/mk-file-embed.pl b/src/mk-file-embed.pl index e4dbe35de1..9daa519e89 100755 --- a/src/mk-file-embed.pl +++ b/src/mk-file-embed.pl @@ -35,6 +35,7 @@ print <