Fixed Clang testing in CI, removed override vars in Makefile

Two flags introduced: -fcallgraph-info=su for stack analysis, and -ftrack-macro-expansions=0 for cleaner prettyassert.py warnings, are unfortunately not supported in Clang. The override vars in the Makefile meant it wasn't actually possible to remove these flags for Clang testing, so this commit changes those vars to normal, non-overriding vars. This means `make CFLAGS=-Werror` and `CFLAGS=-Werror make` behave _very_ differently, but this is just an unfortunate quirk of make that needs to be worked around.
2025-11-16 12:34:34 +00:00 · 2022-11-28 23:55:04 -06:00
parent 0c781dd822
commit 9990342440
4 changed files with 133 additions and 128 deletions
--- a/.github/workflows/test.yml
+++ b/.github/workflows/test.yml
@@ -9,6 +9,7 @@ env:
  CFLAGS: -Werror
  MAKEFLAGS: -j
  TESTFLAGS: -k
+  BENCHFLAGS:

 jobs:
  # run tests
@@ -78,12 +79,13 @@ jobs:
        run: |
          make clean
          sed -n '/``` c/,/```/{/```/d; p}' README.md > test.c
-          make all CFLAGS+=" \
+          CFLAGS="$CFLAGS \
            -Duser_provided_block_device_read=NULL \
            -Duser_provided_block_device_prog=NULL \
            -Duser_provided_block_device_erase=NULL \
            -Duser_provided_block_device_sync=NULL \
-            -include stdio.h"
+            -include stdio.h" \
+            make all
          rm test.c

      # run the tests!
@@ -91,7 +93,7 @@ jobs:
        run: |
          make clean
          # TODO include this by default?
-          make test TESTFLAGS+='-Pnone,linear'
+          TESTFLAGS="$TESTFLAGS -Pnone,linear" make test

      # collect coverage info
      #
@@ -110,12 +112,12 @@ jobs:
      - name: sizes
        run: |
          make clean
-          make lfs.code.csv lfs.data.csv lfs.stack.csv lfs.struct.csv \
-            CFLAGS+=" \
-              -DLFS_NO_ASSERT \
-              -DLFS_NO_DEBUG \
-              -DLFS_NO_WARN \
-              -DLFS_NO_ERROR"
+          CFLAGS="$CFLAGS \
+            -DLFS_NO_ASSERT \
+            -DLFS_NO_DEBUG \
+            -DLFS_NO_WARN \
+            -DLFS_NO_ERROR" \
+            make lfs.code.csv lfs.data.csv lfs.stack.csv lfs.struct.csv
          ./scripts/summary.py lfs.struct.csv \
            -bstruct \
            -fsize=struct_size
@@ -133,13 +135,13 @@ jobs:
      - name: sizes-readonly
        run: |
          make clean
-          make lfs.code.csv lfs.data.csv lfs.stack.csv lfs.struct.csv \
-            CFLAGS+=" \
-              -DLFS_NO_ASSERT \
-              -DLFS_NO_DEBUG \
-              -DLFS_NO_WARN \
-              -DLFS_NO_ERROR \
-              -DLFS_READONLY"
+          CFLAGS="$CFLAGS \
+            -DLFS_NO_ASSERT \
+            -DLFS_NO_DEBUG \
+            -DLFS_NO_WARN \
+            -DLFS_NO_ERROR \
+            -DLFS_READONLY" \
+            make lfs.code.csv lfs.data.csv lfs.stack.csv lfs.struct.csv
          ./scripts/summary.py lfs.struct.csv \
            -bstruct \
            -fsize=struct_size
@@ -157,13 +159,13 @@ jobs:
      - name: sizes-threadsafe
        run: |
          make clean
-          make lfs.code.csv lfs.data.csv lfs.stack.csv lfs.struct.csv \
-            CFLAGS+=" \
-              -DLFS_NO_ASSERT \
-              -DLFS_NO_DEBUG \
-              -DLFS_NO_WARN \
-              -DLFS_NO_ERROR \
-              -DLFS_THREADSAFE"
+          CFLAGS="$CFLAGS \
+            -DLFS_NO_ASSERT \
+            -DLFS_NO_DEBUG \
+            -DLFS_NO_WARN \
+            -DLFS_NO_ERROR \
+            -DLFS_THREADSAFE" \
+            make lfs.code.csv lfs.data.csv lfs.stack.csv lfs.struct.csv
          ./scripts/summary.py lfs.struct.csv \
            -bstruct \
            -fsize=struct_size
@@ -181,13 +183,13 @@ jobs:
      - name: sizes-migrate
        run: |
          make clean
-          make lfs.code.csv lfs.data.csv lfs.stack.csv lfs.struct.csv \
-            CFLAGS+=" \
-              -DLFS_NO_ASSERT \
-              -DLFS_NO_DEBUG \
-              -DLFS_NO_WARN \
-              -DLFS_NO_ERROR \
-              -DLFS_MIGRATE"
+          CFLAGS="$CFLAGS \
+            -DLFS_NO_ASSERT \
+            -DLFS_NO_DEBUG \
+            -DLFS_NO_WARN \
+            -DLFS_NO_ERROR \
+            -DLFS_MIGRATE" \
+            make lfs.code.csv lfs.data.csv lfs.stack.csv lfs.struct.csv
          ./scripts/summary.py lfs.struct.csv \
            -bstruct \
            -fsize=struct_size
@@ -205,12 +207,12 @@ jobs:
      - name: sizes-error-asserts
        run: |
          make clean
-          make lfs.code.csv lfs.data.csv lfs.stack.csv lfs.struct.csv \
-            CFLAGS+=" \
-              -DLFS_NO_DEBUG \
-              -DLFS_NO_WARN \
-              -DLFS_NO_ERROR \
-              -D'LFS_ASSERT(test)=do {if(!(test)) {return -1;}} while(0)'"
+          CFLAGS="$CFLAGS \
+            -DLFS_NO_DEBUG \
+            -DLFS_NO_WARN \
+            -DLFS_NO_ERROR \
+            -D'LFS_ASSERT(test)=do {if(!(test)) {return -1;}} while(0)'" \
+            make lfs.code.csv lfs.data.csv lfs.stack.csv lfs.struct.csv
          ./scripts/summary.py lfs.struct.csv \
            -bstruct \
            -fsize=struct_size
@@ -341,13 +343,14 @@ jobs:
      - name: test-pls
        if: ${{matrix.pls <= 1}}
        run: |
-          make test TESTFLAGS+="-P${{matrix.pls}}"
+          TESTFLAGS="$TESTFLAGS -P${{matrix.pls}}" make test
      # >=2pls takes multiple days to run fully, so we can only
      # run a subset of tests, these are the most important
      - name: test-limited-pls
        if: ${{matrix.pls > 1}}
        run: |
-          make test TESTFLAGS+="-P${{matrix.pls}} test_dirs test_relocations"
+          TESTFLAGS="$TESTFLAGS -P${{matrix.pls}} test_dirs test_relocations" \
+            make test

  # run with LFS_NO_INTRINSICS to make sure that works
  test-no-intrinsics:
@@ -364,7 +367,7 @@ jobs:
          python3 --version
      - name: test-no-intrinsics
        run: |
-          make test CFLAGS+="-DLFS_NO_INTRINSICS"
+          CFLAGS="$CFLAGS -DLFS_NO_INTRINSICS" make test

  # run under Valgrind to check for memory errors
  test-valgrind:
@@ -384,7 +387,29 @@ jobs:
      # on one geometry
      - name: test-valgrind
        run: |
-          make test TESTFLAGS+="-Gdefault --valgrind"
+          TESTFLAGS="$TESTFLAGS -Gdefault --valgrind" make test
+
+  # test that compilation is warning free under clang
+  # run with Clang, mostly to check for Clang-specific warnings
+  test-clang:
+    runs-on: ubuntu-22.04
+    steps:
+      - uses: actions/checkout@v2
+      - name: install
+        run: |
+          # need a few things
+          sudo apt-get install -qq clang python3 python3-pip
+          pip3 install toml
+          clang --version
+          python3 --version
+      - name: test-clang
+        run: |
+          # override CFLAGS since Clang does not support -fcallgraph-info
+          # and -ftrack-macro-expansions
+          make \
+            CC=clang \
+            CFLAGS="$CFLAGS -MMD -g3 -I. -std=c99 -Wall -Wextra -pedantic" \
+            test

  # run benchmarks
  #
@@ -404,7 +429,7 @@ jobs:
          valgrind --version
      - name: bench
        run: |
-          make bench BENCHFLAGS+="-o lfs.bench.csv"
+          BENCHFLAGS="$BENCHFLAGS -o lfs.bench.csv" make bench

          # find bench results
          ./scripts/summary.py lfs.bench.csv \
@@ -463,27 +488,6 @@ jobs:
          path: status
          retention-days: 1

-  # test that compilation is warning free under clang
-  clang:
-    runs-on: ubuntu-20.04
-    steps:
-      - uses: actions/checkout@v2
-      - name: install
-        run: |
-          # need toml, also pip3 isn't installed by default?
-          sudo apt-get update -qq
-          sudo apt-get install -qq python3 python3-pip
-          sudo pip3 install toml
-      - name: install-clang
-        run: |
-          sudo apt-get update -qq
-          sudo apt-get install -qq clang
-          echo "CC=clang" >> $GITHUB_ENV
-          clang --version
-      # no reason to not test again
-      - name: test-clang
-        run: make test TESTFLAGS+="-k"
-
  # self-host with littlefs-fuse for a fuzz-like test
  fuse:
    runs-on: ubuntu-22.04
--- a/109
+++ b/109
@@ -67,91 +67,92 @@ BENCH_GCDA  := $(BENCH_BAC:%.b.a.c=%.b.a.gcda)
 BENCH_PERF  := $(BENCH_RUNNER:%=%.perf)
 BENCH_TRACE := $(BENCH_RUNNER:%=%.trace)

+CFLAGS += -fcallgraph-info=su
+CFLAGS += -g3
+CFLAGS += -I.
+CFLAGS += -std=c99 -Wall -Wextra -pedantic
+CFLAGS += -ftrack-macro-expansion=0
 ifdef DEBUG
-override CFLAGS += -O0
+CFLAGS += -O0
 else
-override CFLAGS += -Os
+CFLAGS += -Os
 endif
 ifdef TRACE
-override CFLAGS += -DLFS_YES_TRACE
+CFLAGS += -DLFS_YES_TRACE
 endif
-override CFLAGS += -g3
-override CFLAGS += -I.
-override CFLAGS += -std=c99 -Wall -Wextra -pedantic
-override CFLAGS += -ftrack-macro-expansion=0
 ifdef YES_COV
-override CFLAGS += --coverage
+CFLAGS += --coverage
 endif
 ifdef YES_PERF
-override CFLAGS += -fno-omit-frame-pointer
+CFLAGS += -fno-omit-frame-pointer
 endif
 ifdef YES_PERFBD
-override CFLAGS += -fno-omit-frame-pointer
+CFLAGS += -fno-omit-frame-pointer
 endif

 ifdef VERBOSE
-override CODEFLAGS   += -v
-override DATAFLAGS   += -v
-override STACKFLAGS  += -v
-override STRUCTFLAGS += -v
-override COVFLAGS    += -v
-override PERFFLAGS   += -v
-override PERFBDFLAGS += -v
+CODEFLAGS   += -v
+DATAFLAGS   += -v
+STACKFLAGS  += -v
+STRUCTFLAGS += -v
+COVFLAGS    += -v
+PERFFLAGS   += -v
+PERFBDFLAGS += -v
 endif
 # forward -j flag
-override PERFFLAGS   += $(filter -j%,$(MAKEFLAGS))
-override PERFBDFLAGS += $(filter -j%,$(MAKEFLAGS))
+PERFFLAGS   += $(filter -j%,$(MAKEFLAGS))
+PERFBDFLAGS += $(filter -j%,$(MAKEFLAGS))
 ifneq ($(NM),nm)
-override CODEFLAGS += --nm-path="$(NM)"
-override DATAFLAGS += --nm-path="$(NM)"
+CODEFLAGS += --nm-path="$(NM)"
+DATAFLAGS += --nm-path="$(NM)"
 endif
 ifneq ($(OBJDUMP),objdump)
-override CODEFLAGS   += --objdump-path="$(OBJDUMP)"
-override DATAFLAGS   += --objdump-path="$(OBJDUMP)"
-override STRUCTFLAGS += --objdump-path="$(OBJDUMP)"
-override PERFFLAGS   += --objdump-path="$(OBJDUMP)"
-override PERFBDFLAGS += --objdump-path="$(OBJDUMP)"
+CODEFLAGS   += --objdump-path="$(OBJDUMP)"
+DATAFLAGS   += --objdump-path="$(OBJDUMP)"
+STRUCTFLAGS += --objdump-path="$(OBJDUMP)"
+PERFFLAGS   += --objdump-path="$(OBJDUMP)"
+PERFBDFLAGS += --objdump-path="$(OBJDUMP)"
 endif
 ifneq ($(PERF),perf)
-override PERFFLAGS += --perf-path="$(PERF)"
+PERFFLAGS += --perf-path="$(PERF)"
 endif

-override TESTFLAGS  += -b
-override BENCHFLAGS += -b
+TESTFLAGS  += -b
+BENCHFLAGS += -b
 # forward -j flag
-override TESTFLAGS  += $(filter -j%,$(MAKEFLAGS))
-override BENCHFLAGS += $(filter -j%,$(MAKEFLAGS))
+TESTFLAGS  += $(filter -j%,$(MAKEFLAGS))
+BENCHFLAGS += $(filter -j%,$(MAKEFLAGS))
 ifdef YES_PERF
-override TESTFLAGS  += -p$(TEST_PERF)
-override BENCHFLAGS += -p$(BENCH_PERF)
+TESTFLAGS  += -p$(TEST_PERF)
+BENCHFLAGS += -p$(BENCH_PERF)
 endif
 ifdef YES_PERFBD
-override TESTFLAGS += -t$(TEST_TRACE) --trace-backtrace --trace-freq=100
+TESTFLAGS += -t$(TEST_TRACE) --trace-backtrace --trace-freq=100
 endif
 ifndef NO_PERFBD
-override BENCHFLAGS += -t$(BENCH_TRACE) --trace-backtrace --trace-freq=100
+BENCHFLAGS += -t$(BENCH_TRACE) --trace-backtrace --trace-freq=100
 endif
 ifdef VERBOSE
-override TESTFLAGS   += -v
-override TESTCFLAGS  += -v
-override BENCHFLAGS  += -v
-override BENCHCFLAGS += -v
+TESTFLAGS   += -v
+TESTCFLAGS  += -v
+BENCHFLAGS  += -v
+BENCHCFLAGS += -v
 endif
 ifdef EXEC
-override TESTFLAGS  += --exec="$(EXEC)"
-override BENCHFLAGS += --exec="$(EXEC)"
+TESTFLAGS  += --exec="$(EXEC)"
+BENCHFLAGS += --exec="$(EXEC)"
 endif
 ifneq ($(GDB),gdb)
-override TESTFLAGS  += --gdb-path="$(GDB)"
-override BENCHFLAGS += --gdb-path="$(GDB)"
+TESTFLAGS  += --gdb-path="$(GDB)"
+BENCHFLAGS += --gdb-path="$(GDB)"
 endif
 ifneq ($(VALGRIND),valgrind)
-override TESTFLAGS  += --valgrind-path="$(VALGRIND)"
-override BENCHFLAGS += --valgrind-path="$(VALGRIND)"
+TESTFLAGS  += --valgrind-path="$(VALGRIND)"
+BENCHFLAGS += --valgrind-path="$(VALGRIND)"
 endif
 ifneq ($(PERF),perf)
-override TESTFLAGS  += --perf-path="$(PERF)"
-override BENCHFLAGS += --perf-path="$(PERF)"
+TESTFLAGS  += --perf-path="$(PERF)"
+BENCHFLAGS += --perf-path="$(PERF)"
 endif


@@ -189,13 +190,13 @@ help:
 ## Build the test-runner
 .PHONY: test-runner build-test
 ifndef NO_COV
-test-runner build-test: override CFLAGS+=--coverage
+test-runner build-test: CFLAGS+=--coverage
 endif
 ifdef YES_PERF
-test-runner build-test: override CFLAGS+=-fno-omit-frame-pointer
+test-runner build-test: CFLAGS+=-fno-omit-frame-pointer
 endif
 ifdef YES_PERFBD
-test-runner build-test: override CFLAGS+=-fno-omit-frame-pointer
+test-runner build-test: CFLAGS+=-fno-omit-frame-pointer
 endif
 # note we remove some binary dependent files during compilation,
 # otherwise it's way to easy to end up with outdated results
@@ -223,13 +224,13 @@ test-list: test-runner
 ## Build the bench-runner
 .PHONY: bench-runner build-bench
 ifdef YES_COV
-bench-runner build-bench: override CFLAGS+=--coverage
+bench-runner build-bench: CFLAGS+=--coverage
 endif
 ifdef YES_PERF
-bench-runner build-bench: override CFLAGS+=-fno-omit-frame-pointer
+bench-runner build-bench: CFLAGS+=-fno-omit-frame-pointer
 endif
 ifndef NO_PERFBD
-bench-runner build-bench: override CFLAGS+=-fno-omit-frame-pointer
+bench-runner build-bench: CFLAGS+=-fno-omit-frame-pointer
 endif
 # note we remove some binary dependent files during compilation,
 # otherwise it's way to easy to end up with outdated results
@@ -361,7 +362,7 @@ $(BUILDDIR)/runners/bench_runner: $(BENCH_OBJ)
 # our main build rule generates .o, .d, and .ci files, the latter
 # used for stack analysis
 $(BUILDDIR)/%.o $(BUILDDIR)/%.ci: %.c
-	$(CC) -c -MMD -fcallgraph-info=su $(CFLAGS) $< -o $(BUILDDIR)/$*.o
+	$(CC) -c -MMD $(CFLAGS) $< -o $(BUILDDIR)/$*.o

 $(BUILDDIR)/%.s: %.c
 	$(CC) -S $(CFLAGS) $< -o $@
--- a/runners/bench_runner.c
+++ b/runners/bench_runner.c
@@ -1201,12 +1201,12 @@ static void list_implicit_defines(void) {
 // geometries to bench

 const bench_geometry_t builtin_geometries[] = {
-    {"default", {{NULL}, BENCH_CONST(16),   BENCH_CONST(512),   {NULL}}},
-    {"eeprom",  {{NULL}, BENCH_CONST(1),    BENCH_CONST(512),   {NULL}}},
-    {"emmc",    {{NULL}, {NULL},            BENCH_CONST(512),   {NULL}}},
-    {"nor",     {{NULL}, BENCH_CONST(1),    BENCH_CONST(4096),  {NULL}}},
-    {"nand",    {{NULL}, BENCH_CONST(4096), BENCH_CONST(32768), {NULL}}},
-    {NULL, {{NULL}, {NULL}, {NULL}, {NULL}}},
+    {"default", {{0}, BENCH_CONST(16),   BENCH_CONST(512),   {0}}},
+    {"eeprom",  {{0}, BENCH_CONST(1),    BENCH_CONST(512),   {0}}},
+    {"emmc",    {{0}, {0},               BENCH_CONST(512),   {0}}},
+    {"nor",     {{0}, BENCH_CONST(1),    BENCH_CONST(4096),  {0}}},
+    {"nand",    {{0}, BENCH_CONST(4096), BENCH_CONST(32768), {0}}},
+    {NULL, {{0}, {0}, {0}, {0}}},
 };

 const bench_geometry_t *bench_geometries = builtin_geometries;
--- a/runners/test_runner.c
+++ b/runners/test_runner.c
@@ -1241,12 +1241,12 @@ static void list_implicit_defines(void) {
 // geometries to test

 const test_geometry_t builtin_geometries[] = {
-    {"default", {{NULL}, TEST_CONST(16),   TEST_CONST(512),   {NULL}}},
-    {"eeprom",  {{NULL}, TEST_CONST(1),    TEST_CONST(512),   {NULL}}},
-    {"emmc",    {{NULL}, {NULL},           TEST_CONST(512),   {NULL}}},
-    {"nor",     {{NULL}, TEST_CONST(1),    TEST_CONST(4096),  {NULL}}},
-    {"nand",    {{NULL}, TEST_CONST(4096), TEST_CONST(32768), {NULL}}},
-    {NULL, {{NULL}, {NULL}, {NULL}, {NULL}}},
+    {"default", {{0}, TEST_CONST(16),   TEST_CONST(512),   {0}}},
+    {"eeprom",  {{0}, TEST_CONST(1),    TEST_CONST(512),   {0}}},
+    {"emmc",    {{0}, {0},              TEST_CONST(512),   {0}}},
+    {"nor",     {{0}, TEST_CONST(1),    TEST_CONST(4096),  {0}}},
+    {"nand",    {{0}, TEST_CONST(4096), TEST_CONST(32768), {0}}},
+    {NULL, {{0}, {0}, {0}, {0}}},
 };

 const test_geometry_t *test_geometries = builtin_geometries;