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

[build.zig] Override config.h definitions #4193

Merged
merged 2 commits into from
Aug 4, 2024

Conversation

lnc3l0t
Copy link
Contributor

@lnc3l0t lnc3l0t commented Jul 29, 2024

Overview

This PR introduces a new option allowing build.zig to override config.h settings using a simple string:

pub fn build(b: *std.Build) !void {
// ...
const config: []const u8 =
    "-DSUPPORT_FILEFORMAT_JPG -DSUPPORT_FILEFORMAT_PNG=0";

const raylib_dep = b.dependency("raylib", .{ .config = config });
// ...
}

This example enables support for the JPG format (disabled by default) and disables support for the PNG (enabled by default).

Two commits are present in the PR:

  • the first allows for all definitions in config.h to be overriden, it has more cumbersome parsing since some definitions have different data types
  • the second restricts the user to override only SUPPORT_* flags (which simplifies the implementation and you could possibly merge, given my assumptions are correct [below]).

Thoughts

  • Issue [build] Support config header in build.zig #3516 suggested the addConfigHeader function, but I haven't found a way to use it. My logic mimics what has been done for CMake: parse config.h to retrieve default values and override them with the ones provided by user, then inhibit config.h by supplying -DEXTERNAL_CONFIG_FLAGS
  • In CMake for what I can gather only the SUPPORT flags are handled, therefore my second commit (99307a4). I was thinking that after -DEXTERNAL_CONFIG_FLAGS all the other flags wouldn't be set, but it seems everything is redefined if needed in the various source file (is this assumption correct?).
I used a command to find references to all NON-SUPPORT flags (all are redefined elsewhere except CONFIG_H)
> cat src/config.h | grep -Eo "^#define \w+" | cut -d" " -f2 | grep --invert SUPPORT | parallel --tag grep -Erni \"#define {}\" --exclude=config.h --before-context=1 \| wc -l | column -t
CONFIG_H                                     0
MAX_FILEPATH_CAPACITY                        2
MAX_FILEPATH_LENGTH                          4
MAX_KEYBOARD_KEYS                            2
MAX_MOUSE_BUTTONS                            2
MAX_GAMEPADS                                 2
MAX_GAMEPAD_BUTTONS                          2
MAX_GAMEPAD_AXIS                             2
MAX_GAMEPAD_VIBRATION_TIME                   2
MAX_CHAR_PRESSED_QUEUE                       2
MAX_TOUCH_POINTS                             8
MAX_KEY_PRESSED_QUEUE                        2
MAX_AUTOMATION_EVENTS                        2
MAX_DECOMPRESSION_SIZE                       2
RL_DEFAULT_BATCH_BUFFERS                     8
RL_DEFAULT_BATCH_DRAWCALLS                   8
RL_DEFAULT_BATCH_MAX_TEXTURE_UNITS           8
RL_MAX_MATRIX_STACK_SIZE                     8
RL_MAX_SHADER_LOCATIONS                      8
RL_CULL_DISTANCE_NEAR                        8
RL_CULL_DISTANCE_FAR                         8
RL_DEFAULT_SHADER_ATTRIB_LOCATION_POSITION   2
RL_DEFAULT_SHADER_ATTRIB_LOCATION_TEXCOORD   5
RL_DEFAULT_SHADER_ATTRIB_LOCATION_NORMAL     2
RL_DEFAULT_SHADER_ATTRIB_LOCATION_COLOR      2
RL_DEFAULT_SHADER_ATTRIB_LOCATION_TEXCOORD2  2
RL_DEFAULT_SHADER_ATTRIB_LOCATION_TANGENT    2
RL_DEFAULT_SHADER_ATTRIB_NAME_POSITION       5
RL_DEFAULT_SHADER_ATTRIB_NAME_TEXCOORD       11
RL_DEFAULT_SHADER_ATTRIB_NAME_NORMAL         5
RL_DEFAULT_SHADER_ATTRIB_NAME_COLOR          5
RL_DEFAULT_SHADER_ATTRIB_NAME_TANGENT        5
RL_DEFAULT_SHADER_ATTRIB_NAME_TEXCOORD2      5
RL_DEFAULT_SHADER_UNIFORM_NAME_VIEW          5
RL_DEFAULT_SHADER_UNIFORM_NAME_PROJECTION    5
RL_DEFAULT_SHADER_UNIFORM_NAME_MVP           5
RL_DEFAULT_SHADER_UNIFORM_NAME_NORMAL        5
RL_DEFAULT_SHADER_UNIFORM_NAME_MODEL         5
RL_DEFAULT_SHADER_UNIFORM_NAME_COLOR         5
RL_DEFAULT_SHADER_SAMPLER2D_NAME_TEXTURE0    5
RL_DEFAULT_SHADER_SAMPLER2D_NAME_TEXTURE1    5
RL_DEFAULT_SHADER_SAMPLER2D_NAME_TEXTURE2    5
MAX_TEXT_BUFFER_LENGTH                       5
SPLINE_SEGMENT_DIVISIONS                     2
MAX_TEXTSPLIT_COUNT                          5
MAX_MATERIAL_MAPS                            2
MAX_MESH_VERTEX_BUFFERS                      2
AUDIO_DEVICE_FORMAT                          2
AUDIO_DEVICE_SAMPLE_RATE                     2
AUDIO_DEVICE_CHANNELS                        2
MAX_AUDIO_BUFFER_POOL_CHANNELS               2
MAX_TRACELOG_MSG_LENGTH                      2
  • Zig doesn't have a standard regex support, the parsing is done manually in my code
  • In the second commit I added lines to trim whitespace, they are not really needed given the current state of config.h should I remove them? and assume the formatting will always be the same
  • Should I remove the comments?
  • I think after we decide the correct implementation I will have to rebase the commits to have a single one

Thank you for considering my PR :)

lnc3l0t added 2 commits July 29, 2024 10:08
The new Options field "config" holds a string the user can set in the
format "-Dflag_a=1 -Dflag_b=0 ..." to override the values set in
`config.h`.
The file is parsed and the default values are appended to the
compilation flags, if the user doesn't override them.
The user string is appended to the compilation flags.
The "-DEXTERNAL_CONFIG_FLAGS" is added to prevent "config.h" inclusion.

Note: a certain format is assumed for the formatting of config.h
Note: this commit references the closed issue raysan5#3516
Lines from `config.h` which contains "SUPPORT" are added to compilation after being parsed:
- remove whitespace
- format to preprocessor option https://gcc.gnu.org/onlinedocs/gcc/Preprocessor-Options.html

The user supplied flags have priority over the ones read from the file.

NOTE: extension to commit 4da7f82, the logic is simplified
because the SUPPORT flags only have binary values, which makes them easier to parse.
@lnc3l0t lnc3l0t changed the title Zig override config h [build.zig] Override config.h definitions Jul 29, 2024
@Gamer-Kold
Copy link
Contributor

Looks good to me

@Gamer-Kold
Copy link
Contributor

As for the questions about the implementation that you have I am not the person to answer them.

But I can confirm that the implementation is doing what you're saying its doing. I think as a user of the build.zig; I'd like some comments explaining that it only works for the SUPPORT flags.

@lnc3l0t
Copy link
Contributor Author

lnc3l0t commented Jul 30, 2024

First commit (4da7f82) makes it possible to configure everything, not just the SUPPORT flags.
I don't know if you'd want to test it, but thanks for the feedback @Gamer-Kold

@raysan5 raysan5 merged commit b2d48ff into raysan5:master Aug 4, 2024
@raysan5
Copy link
Owner

raysan5 commented Aug 4, 2024

@lnc3l0t I'm merging it but note that I can not verify this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants