Skip to content

Commit f77783a

Browse files
committed
github: fix broken header check
- Move header check from postsubmit to presubmit - Install third_party/getop if needed - Modify check-headers test to use system getopt when it's available.
1 parent 749b03e commit f77783a

File tree

4 files changed

+54
-32
lines changed

4 files changed

+54
-32
lines changed

.github/workflows/postsubmit.yml

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -77,9 +77,6 @@ jobs:
7777
with:
7878
name: filament-mac
7979
path: out/filament-release-darwin.tgz
80-
- name: Check public headers
81-
run: |
82-
test/check-headers/test.sh out/release/filament/include
8380

8481
build-web:
8582
name: build-web

.github/workflows/presubmit.yml

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,9 +64,15 @@ jobs:
6464
- name: Run build script
6565
run: |
6666
cd build/mac && printf "y" | ./build.sh presubmit-with-test
67-
- name: Test material parser
67+
- name: Test - material parser
6868
run: |
6969
out/cmake-release/filament/test/test_material_parser
70+
- name: Test - public headers
71+
run: |
72+
# out/cmake-release should have the artifacts ready for installation. Here we install it
73+
# to test the public headers
74+
ninja -C out/cmake-release install
75+
test/check-headers/test.sh out/release/filament/include
7076
7177
build-desktop-linux:
7278
name: build-linux

test/check-headers/test.sh

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,12 @@ for f in $(find . -name '*.h'); do
5353
done
5454
popd >/dev/null
5555

56+
# Check if the system has getopt
57+
HAS_SYSTEM_GETOPT=0
58+
if echo "#include <getopt.h>" | clang -x c++ -std=c++17 -E - > /dev/null 2>&1; then
59+
HAS_SYSTEM_GETOPT=1
60+
fi
61+
5662
rm -rf out/check-headers
5763
mkdir -p out/check-headers
5864

@@ -61,10 +67,17 @@ echo "Checking that public headers compile independently..."
6167
for include in "${includes[@]}"; do
6268
rm -f ${TMP_FILE}
6369
echo "Checking ${include}"
64-
if [[ "${include}" == "utils/Systrace.h" ]]; then
65-
# A necessary define before we can include utils/Systrace.h
66-
echo "#define SYSTRACE_TAG SYSTRACE_TAG_DISABLED" >> ${TMP_FILE}
67-
fi
70+
case "${include}" in
71+
"utils/Systrace.h")
72+
# A necessary define before we can include utils/Systrace.h
73+
echo "#define SYSTRACE_TAG SYSTRACE_TAG_DISABLED" >> ${TMP_FILE}
74+
;;
75+
"utils/getopt.h")
76+
if [[ $HAS_SYSTEM_GETOPT -eq 1 ]]; then
77+
echo "#define HAS_SYSTEM_GETOPT 1" >> ${TMP_FILE}
78+
fi
79+
;;
80+
esac
6881
echo "#include <${include}>" >> ${TMP_FILE}
6982
# Filament is built internally with C++20, but we maintain C++17 compatibility (for the time
7083
# being) in our public headers to support projects on older toolchains.

third_party/getopt/CMakeLists.txt

Lines changed: 30 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,30 @@
1-
cmake_minimum_required(VERSION 3.10)
2-
project(getopt)
3-
4-
set(TARGET getopt)
5-
set(PUBLIC_HDR_DIR include)
6-
7-
# ==================================================================================================
8-
# Sources and headers
9-
# ==================================================================================================
10-
set(PUBLIC_HDRS include/getopt/getopt.h)
11-
set(PRIVATE_HDRS include/getopt/getopt.h)
12-
13-
set(SRCS
14-
src/getopt.c
15-
src/getopt_long.c)
16-
17-
# ==================================================================================================
18-
# Include and target definitions
19-
# ==================================================================================================
20-
include_directories(${PUBLIC_HDR_DIR})
21-
22-
add_library(${TARGET} STATIC ${PRIVATE_HDRS} ${PUBLIC_HDRS} ${SRCS})
23-
target_include_directories (${TARGET} PUBLIC ${PUBLIC_HDR_DIR})
24-
set_target_properties(${TARGET} PROPERTIES FOLDER ThirdParty)
1+
cmake_minimum_required(VERSION 3.10)
2+
project(getopt)
3+
4+
set(TARGET getopt)
5+
set(PUBLIC_HDR_DIR include)
6+
7+
# ==================================================================================================
8+
# Sources and headers
9+
# ==================================================================================================
10+
set(PUBLIC_HDRS include/getopt/getopt.h)
11+
set(PRIVATE_HDRS include/getopt/getopt.h)
12+
13+
set(SRCS
14+
src/getopt.c
15+
src/getopt_long.c)
16+
17+
# ==================================================================================================
18+
# Include and target definitions
19+
# ==================================================================================================
20+
include_directories(${PUBLIC_HDR_DIR})
21+
22+
add_library(${TARGET} STATIC ${PRIVATE_HDRS} ${PUBLIC_HDRS} ${SRCS})
23+
target_include_directories (${TARGET} PUBLIC ${PUBLIC_HDR_DIR})
24+
set_target_properties(${TARGET} PROPERTIES FOLDER ThirdParty)
25+
26+
# ==================================================================================================
27+
# Installation
28+
# ==================================================================================================
29+
install(TARGETS ${TARGET} ARCHIVE DESTINATION lib/${DIST_DIR})
30+
install(DIRECTORY ${PUBLIC_HDR_DIR}/getopt DESTINATION include)

0 commit comments

Comments
 (0)