-
Notifications
You must be signed in to change notification settings - Fork 119
Refine build system for efficient configurations #619
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
base: master
Are you sure you want to change the base?
Conversation
b2178db
to
7928db7
Compare
This addresses build system inefficiencies and compatibility issues: 1. Configuration change detection: Use cmp-based comparison to avoid unnecessary rebuilds when configuration hasn't actually changed. Previously, every make run would touch .config and trigger full rebuilds. 2. Directory creation optimization: Replace runtime mkdir -p $(shell dirname) calls with order-only prerequisites (|). Add mkdir -p $(dir $@) to compilation rules to ensure subdirectories exist for nested object files (build/dtc/libfdt/*.o, build/devices/*.o). 3. Configuration dependency tracking: Add $(CONFIG_FILE) as explicit prerequisite to all compilation rules. This ensures object files rebuild when feature flags change (e.g., ENABLE_JIT=1 -> 0). 4. Emscripten SDL2_mixer fix: Add -sSTRICT=0 to CFLAGS_emcc to disable STRICT mode. The emscripten-ports/SDL2_mixer repository was archived in Jan 2024 and has warnings in music_modplug.c that become fatal errors under STRICT mode's -Werror flag. 5. CI scan-build fix: Set LATEST_RELEASE=dummy environment variable in scan-build steps. This prevents network-dependent prebuilt file downloads while avoiding 32-bit compilation requirements (ENABLE_PREBUILT=0 triggers -m32 flag which requires 32-bit development libraries).
Introduce ENABLE_SDL_MIXER feature flag to conditionally compile SDL2_mixer-dependent audio code. This allows SDL2 graphics support without the problematic SDL2_mixer library in emscripten builds. emscripten-ports/SDL2_mixer (archived in 2024) has unfixable compilation warnings in music_modplug.c. The port system enforces -sSTRICT -Werror which cannot be overridden.
This addresses multiple CI failures related to Architecture tests and SDL_MIXER installation across different platforms.
7928db7
to
3f46e82
Compare
@ChinYikMing, Although we are experiencing intermittent CI regressions on macOS-arm64 targets, it remains essential to refine the build system. Please review the changes, especially those related to WebAssembly-specific rules. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jserv , could you point out where is the warnings of music_modplug.c
under emcc
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Running make doom ENABLE_SDL_MIXER=0 -j8
or make system ENABLE_SYSTEM=1 ENABLE_SDL_MIXER=0 INITRD_SIZE=32 -j8
with the changes gives errors:
CC build/syscall_sdl.o
src/syscall_sdl.c:108:12: error: ‘chan’ defined but not used [-Werror=unused-variable]
108 | static int chan;
| ^~~~
src/syscall_sdl.c:107:17: error: ‘nr_sfx_samples’ defined but not used [-Werror=unused-variable]
107 | static uint32_t nr_sfx_samples;
| ^~~~~~~~~~~~~~
src/syscall_sdl.c:98:17: error: ‘music_midi_data’ defined but not used [-Werror=unused-variable]
98 | static uint8_t *music_midi_data;
| ^~~~~~~~~~~~~~~
src/syscall_sdl.c:97:18: error: ‘music_thread’ defined but not used [-Werror=unused-variable]
97 | static pthread_t music_thread;
| ^~~~~~~~~~~~
cc1: all warnings being treated as errors
make: *** [Makefile:395: build/syscall_sdl.o] Error 1
Consider this patch for better conditional compile for ENABLE_SDL_MIXER
:
diff --git a/src/syscall_sdl.c b/src/syscall_sdl.c
index 32a3397..4f38a1a 100644
--- a/src/syscall_sdl.c
+++ b/src/syscall_sdl.c
@@ -94,15 +94,14 @@ typedef struct sound {
} sound_t;
/* SDL-mixer-related and music-related variables */
+#if RV32_HAS(SDL_MIXER)
static pthread_t music_thread;
static uint8_t *music_midi_data;
-#if RV32_HAS(SDL_MIXER)
static Mix_Music *mid;
/* SDL-mixer-related and sfx-related variables */
static pthread_t sfx_thread;
static Mix_Chunk *sfx_chunk;
-#endif
static uint8_t *sfx_samples;
static uint32_t nr_sfx_samples;
static int chan;
@@ -111,6 +110,7 @@ static int chan;
static bool audio_init = false;
static bool sfx_thread_init = false;
static bool music_thread_init = false;
+#endif
typedef struct {
void *data;
@@ -841,10 +841,8 @@ static void play_sfx(riscv_t *rv)
.size = sfx_data_size,
.volume = volume,
};
-#if RV32_HAS(SDL_MIXER)
pthread_create(&sfx_thread, NULL, sfx_handler, &sfx);
sfx_thread_init = true;
-#endif
/* FIXME: In web browser runtime, web workers in thread pool do not reap
* after sfx_handler return, thus we have to join them. sfx_handler does not
* contain infinite loop,so do not worry to be stalled by it */
@@ -900,10 +898,8 @@ static void play_music(riscv_t *rv)
.looping = looping,
.volume = volume,
};
-#if RV32_HAS(SDL_MIXER)
pthread_create(&music_thread, NULL, music_handler, &music);
music_thread_init = true;
-#endif
/* FIXME: In web browser runtime, web workers in thread pool do not reap
* after music_handler return, thus we have to join them. music_handler does
* not contain infinite loop,so do not worry to be stalled by it */
@@ -927,6 +923,7 @@ static void set_music_volume(riscv_t *rv)
}
#endif /* RV32_HAS(SDL_MIXER) */
+#if RV32_HAS(SDL_MIXER)
static void init_audio(void)
{
if (!(SDL_WasInit(-1) & SDL_INIT_AUDIO)) {
@@ -943,7 +940,6 @@ static void init_audio(void)
exit(EXIT_FAILURE);
}
-#if RV32_HAS(SDL_MIXER)
/* Initialize SDL2 Mixer */
if (Mix_Init(MIX_INIT_MID) != MIX_INIT_MID) {
rv_log_fatal("Mix_Init failed: %s", Mix_GetError());
@@ -954,10 +950,11 @@ static void init_audio(void)
Mix_Quit();
exit(EXIT_FAILURE);
}
-#endif
audio_init = true;
}
+#endif
+#if RV32_HAS(SDL_MIXER)
static void shutdown_audio()
{
/*
@@ -968,7 +965,6 @@ static void shutdown_audio()
* on a valid pthread_t identifier.
*/
-#if RV32_HAS(SDL_MIXER)
if (music_thread_init) {
stop_music();
pthread_join(music_thread, NULL);
@@ -987,10 +983,10 @@ static void shutdown_audio()
Mix_CloseAudio();
Mix_Quit();
-#endif
audio_init = sfx_thread_init = music_thread_init = false;
}
+#endif
void sdl_video_audio_cleanup()
{
@@ -998,6 +994,8 @@ void sdl_video_audio_cleanup()
SDL_DestroyWindow(window);
window = NULL;
}
+
+#if RV32_HAS(SDL_MIXER)
/*
* The sfx_or_music_thread_init flag might not be set if a quick ctrl-c
* occurs while the audio configuration is being initialized. Therefore,
@@ -1006,6 +1004,7 @@ void sdl_video_audio_cleanup()
bool sfx_or_music_thread_init = sfx_thread_init | music_thread_init;
if (sfx_or_music_thread_init || (!sfx_or_music_thread_init && audio_init))
shutdown_audio();
+#endif
SDL_Quit();
}
@@ -1016,10 +1015,14 @@ void syscall_setup_audio(riscv_t *rv)
switch (request) {
case INIT_AUDIO:
+#if RV32_HAS(SDL_MIXER)
init_audio();
+#endif
break;
case SHUTDOWN_AUDIO:
+#if RV32_HAS(SDL_MIXER)
shutdown_audio();
+#endif
break;
default:
rv_log_error("Unknown sound request: %d", request);
This addresses build system inefficiencies and compatibility issues:
Summary by cubic
Speeds up and stabilizes the Make build by only updating .config when values change and rebuilding objects when feature flags change. Also replaces ad‑hoc mkdir calls with proper directory prerequisites and adds clearer config warnings.
Refactors
Migration