Simplify build procedure

[saccarosium]

Hi,
this is my attempt to simplify the build procedure so you can just go to the root of the folder and execute the build script and you are good to go.

In this PR I would also like to reduce the volume of the 4ed_build.cpp so it can be a little more manageable. We can also talk about the custom_layer proposal made in the discord some time ago.

[mrmixer]

It's mostly OK, there are a few details that I think need to be addressed.

command_metadata.h: There seems to be a small mix of two version of the file. Mostly word_complete_prev doesn't appear in the fcoder_metacmd_table array in the last version. If you rebuild it should fix itself I believe.

build.bat:

• doesn't setup MSVC anymore. The old script called call %custom_bin%\setup_cl_x64.bat. Unless you've got a reason, I think we should still call it.
• This is just a note, but I'd like to document it here: previous scripts set FirstError to 1 whenever ERRORLEVEL was set buy the build script. This PR removes that, but the only use I could find for FirstError was to pass it to CTIME (tool for measuring build times) to indicate the build had failed. And it seems to be in all test code we don't use anymore.
• When calling the build executable we do %build_root%\build %*. I think it'd be better to add the extension just in case there is a build.bat or build.com file in the directory. %build_root%\build.exe %*

build.sh:

• You added -Wno-null-dereference -Wno-logical-op-parentheses -Wno-switch to the linux build. Those flags were only on Mac. Removing them on my linux setup doesn't show any warnings, so it's not an issue. I wonder if we could fix the null dereference and logical op parentheses. The null dereference is probably for the assert code, and we should replace that with __debugbreak() on MSVC, __builtin_debugtrap( ) for Clang and __asm__( "int $3" ); for GCC. The logical op parentheses would be nice to fix, as that often causes bugs in my experience. But it can stay for the moment. I also don't know what the default warning level is in Clang and GCC but maybe we should try -Wall at some point (the windows build uses -W4).
• Does c++ calls a specific compiler or does it use whatever is configured ? Previously the script used g++, so is there a reason for the change (I'm not super familiar with linux) ? Also in 4ed_build.cpp we are still calling g++ and clang++ explicitly (if there is an issue with that).

4ed_build.cpp:

• I would always add a long name for the parameters, and long name should use double dash (I believe this is standard, but I can be wrong). So we would have -m, --mode, -b, --backend, -h, --help.
• I also would prefer to use debug instead of dev for the mode.
• In the help, I'd specify that backend is only for Windows.
• In the help, I'd use a capital letter and a period in the description.

Maybe the following can be in a different PR.

• It was discussed previously, but it would be nice to be able to have a way to explicitly ask for debug symbols (even in release mode); Similarly it would be nice to be able to ask for optimization without setting SHIP_MODE. We could have -o, --optimization, -s, --symbols. Using "debug mode" or "release mode" would use the default flags, that could be overwritten with -o or -s. Parameters order needs to be handled in that case.
• Another helpful thing would be to have a flag to display the build commands generated (-p, --print_command). That can help whith debug and create a variation of the command manually (printing the command just before the systemf calls).

build.yml:

• We need to figure out why the CI builds on Mac and Linux fail.
• When passing parameters in that file, use the long names to be more explicit. (e.g. build_cmd: .\build.bat --backend=DX11 ) assuming we go with that idea.

README.MD:

• You removed the setup for older version of Mac. Is not an issue anymore ?
• It's not from your PR, but what does that mean ?

4coder targets x86_64. If you are using a M1+ ARM CPU you need to prefix the build scripts commands with: arch -arch x86_64

Does it mean modifying the shell script ? Does it work with the new script ?

• I think it would still be useful to have the build options listed and explained in the README.
• If we go with the double dashes, we need to change -help to --help.

[saccarosium]

It was discussed previously, but it would be nice to be able to have a way to explicitly ask for debug symbols (even in release mode); Similarly it would be nice to be able to ask for optimization without setting SHIP_MODE. We could have -o, --optimization, -s, --symbols. Using "debug mode" or "release mode" would use the default flags, that could be overwritten with -o or -s. Parameters order needs to be handled in that case. Another helpful thing would be to have a flag to display the build commands generated (-p, --print_command). That can help whith debug and create a variation of the command manually (printing the command just before the systemf calls).

I had the same idea actually, but I was hesitant because the added complexity of the parsing, but I should do it.

In the help, I'd use a capital letter and a period in the description.

I've used the style of the help of the core utils. But I'm ok either way.

Does c++ calls a specific compiler or does it use whatever is configured ? Previously the script used g++, so is there a reason for the change (I'm not super familiar with linux) ?

Calling c++ or cc on a UNIX system invokes the default toolchain used. This is done for compatibility I suppose (I'm not sure is a POSIX standard or something). On MacOS it uses clang on linux usually is gcc but it depends on the distribution. This is probably the most portable way to do it.

Also note that the user can custumized it with the CC environment variable. It is a convention in POSIX systems but I should probably document it.

Also in 4ed_build.cpp we are still calling g++ and clang++ explicitly (if there is an issue with that).

Yes, I think I should refactor it a little bit more. In the current moment 4ed_build.cpp assumes a specific compiler on a specific platform but since some compilers are actually cross platform this restriction should be removed I think.

We need to figure out why the CI builds on Mac and Linux fail.

It is simply because they aren't set as executable, I'm currently on Windows so I can't fix it directly but we could just call the script bash <script>

[mrmixer]

Hi, sorry for the delay. There are still a few things to address and somethings we'll need to discuss with other maintainers.

• Could you add those lines to setup_cl_generic.bat to add support for Visual Studio 2026:

SET VC_PATH=C:\Program Files\Microsoft Visual Studio\18\Community
IF NOT DEFINED LIB (IF EXIST "%VC_PATH%" (call "%VC_PATH%\VC\Auxiliary\Build\vcvarsall.bat" %1))

SET VC_PATH=C:\Program Files\Microsoft Visual Studio\18\Professional
IF NOT DEFINED LIB (IF EXIST "%VC_PATH%" (call "%VC_PATH%\VC\Auxiliary\Build\vcvarsall.bat" %1))

In build.bat and build.sh:

• One thing that I don't like is that, if there is any error and you pushed a directory, the directory isn't poped before exiting the script, which often result in being in the build folder instead of the original folder you were in. I think the easier fix would be to to save the working directory at the start, and always set it back before exiting the script. I would also move the error handling after the popd.

@echo off

set start_directory=%cd%
...
if %ERRORLEVEL% neq 0 (goto END)
...
:END
cd /D "%start_directory%"

• I think we could remove the pushd %src_root%, because build.exe will search the root folder, and then push the build directory, so I don't think it's necessary to move to the source directory.

In 4coder_base_types.cpp : I will ask the other maintainers, but I would prefer not to have operator overloading. In a more general way, I would prefer the code to be more C than CPP.

In 4ed_build.cpp:

• The copy of the custom layer files isn't correct. It copies the files in the build folder instead of copying them in build/custom.
• I would prefer that all if and loops have curly brackets even if there is only one line inside them (I'll ask other maintainers about it).
• I would prefer not to have return or exit statements in the middle of functions (I'll ask other maintainers about it).
• set_common_dirs if we don't do the pushd in the source directory, we need to remove the parent folder part.
• parse_flags: the mode string for release + debug info doesn't match the one in the usage function.
• parse_flags: I'm not sure about the 3rd mode. I think I'd prefer to have just release mode which set optimization and ship flags, the debug mode which sets symbols and internal flags, and have two other flags to be able to manually ask for optimizations (-o, --optimizations) and symbols (-s, --symbols). I think that would be a better user experience.
• parse_flags: we probably should use string_match_insensitive function to make sure casse doesn't matter when comparing the strings.
• parse_flags: should we have a short version of --gl-api, like -gl or -g ?
• I think we could remove SUPER and OPTS in the options enum. SUPER isn't used any more, and OPTS is always used so we can just add those without checking the flag.
• I don't like that we rely on prev_error as it's not clear where that comes from (but I understand that it's the API we have at the moment, and I think you talked about that file on Discord). So I would add a comment to indicates it comes from 4coder_file_moving.h. But I think it might also be better to use error_state instead ? Maybe ?
• You removed -m64 and -m32 flags on the Mac build. What were those flag for if they are not needed ?

[saccarosium]

One thing that I don't like is that, if there is any error and you pushed a directory, the directory isn't poped before exiting the script, which often result in being in the build folder instead of the original folder you were in. I think the easier fix would be to to save the working directory at the start, and always set it back before exiting the script.

It is only a problem when dealing with cmd on windows if you are using either powershell or a POSIX shell changing the directory doesn't change it in the top process. But I'll fix it for those that still use cmd.exe.

I don't like that we rely on prev_error as it's not clear where that comes from (but I understand that it's the API we have at the moment, and I think you talked about that file on Discord). So I would add a comment to indicates it comes from 4coder_file_moving.h. But I think it might also be better to use error_state instead ? Maybe

Yes, I was saying in the discord that the 4coder_file_moving.h is a bit crapy and should be refactored. But I wasn't sure if anybody is relying on the API. In my opinion should be completely be refactored.

parse_flags: I'm not sure about the 3rd mode. I think I'd prefer to have just release mode which set optimization and ship flags, the debug mode which sets symbols and internal flags, and have two other flags to be able to manually ask for optimizations (-o, --optimizations) and symbols (-s, --symbols). I think that would be a better user experience.

Not sure if it is a good idea seems but I can do it.

[Jack-Punter]

Looks pretty good on the whole, got a few comments here and there but nothing super major I dont think.

[mrmixer]

Almost good to go on my side.
I'm sorry if there is a comment that just say "test", but github didn't show the comment above without it (or more likely I messed up something).

• In previous commits, you removed the -m64 and -m32 flags from linux and Mac builds. I couldn't find what those flags are for. Aren't they necessary any more ?
• You also removed the part about realpath for older mac version from the README. Isn't that an issue ?
• I didn't check the linux and Mac bash issues from Jack as I don't know anything about that.
• Like Jack, I'd prefer to have the README contain the parameters for the build. I understand that they can go out of sync, but I don't think that's hard to fix or a big issue.

[mrmixer]

root is not used in the script.
The changed I was asking for is a bit different that what you changed.
What I'd like is that when you run that script, if it succeed of fail, you stay in the folder you were when you ran it.
For example if I'm in C:\temp and 4cc is in C:\4cc, after executing the script I want to still be in C:\temp.

So I'd do something like this.

@echo off

set directory_to_restore=%cd%

rem Make sure we are at the root of the project
cd /D "%~dp0"

set src_root=%cd%\code
set custom_root=%src_root%\custom
set build_root=%cd%\build

rem If the user want's 32 bits they need to setup MSVC before calling the script.
call %custom_root%\bin\setup_cl_x64.bat

set opts=/nologo /FC /Zi /I%src_root% /I%custom_root%

if not exist "%build_root%" mkdir %build_root%
pushd %build_root%
call cl %opts% %src_root%\4ed_build.cpp /Febuild
popd

if %ERRORLEVEL% neq 0 (goto END)

%build_root%\build.exe %*

:END
cd /D "%directory_to_restore%"

[mrmixer]

%root% is never used, we can remove it.

[saccarosium]

In previous commits, you removed the -m64 and -m32 flags from linux and Mac builds. I couldn't find what those flags are for. Aren't they necessary any more ?

m64 sets the architecture to x64 witch is the default. I can see the case for m32 that it is the equivalent for x86 but it should be set automatically if compiled on a 32bit system.

You also removed the part about realpath for older mac version from the README. Isn't that an issue ?

No it isn't. It will work on macos no problem

About the executable bit @Jack-Punter I don't have a linux machine right now so I can't set the executable bit myself.

[Jack-Punter]

About the executable bit @Jack-Punter I don't have a linux machine right now so I can't set the executable bit myself.

I've just pushed this and reverted the explicit bash calls in the workflow file. I'll try and take a look at the other changes later.

As a quick side note, can you avoid force-pushing when updating a PR, it makes it harder to link up old comments with new changes (and forces us to delete our checkouts and re-pull as you overwrite the history)

[mrmixer]

%root% is never used, we can remove it.

[mrmixer]

Any particular reason you added NDEBUG ?
I'm fine with it (it's a probably a good thing to have it).

[saccarosium]

It is only a convention some STD function check for it I think assert.h is one of them. But other than that doesn't do anything else.

[mrmixer]

Everything looks good to me. I only tested on Windows.

[Jack-Punter]

Im happy with these changes and it seems to work fine on my linux machine, however either because i've commited to the PR, or because the commit i requested changes on no longer exists in the tree (due to force push) i can't update my review to be approved. If we're all happy with this i have the power to bypass our rules and merge regardless of the open requested changes

[Jack-Punter]

NVM It was hidden in a wierd place