Skip to content

build(docker): Set correct targets in docker-build.sh#2678

Merged
xezon merged 2 commits intoTheSuperHackers:mainfrom
bjoernfi:fix-docker-build-targets
May 3, 2026
Merged

build(docker): Set correct targets in docker-build.sh#2678
xezon merged 2 commits intoTheSuperHackers:mainfrom
bjoernfi:fix-docker-build-targets

Conversation

@bjoernfi
Copy link
Copy Markdown

@bjoernfi bjoernfi commented May 3, 2026

Relates to #2085

In the script comments and usage section, it says that ./scripts/docker-build.sh --target generalszh should be used to build Generals ZH. As no such target exists, the build fails with ninja: error: unknown target 'generalszh'. Moreover, the script allows to build both generalszh.exe and the ZH tools via --game zh instead of --target but it derives the wrong targets if that option is used. Thus, this PR fixes the script to set the correct targets and updates the comment and usage sections accordingly.

I also want to note that there is a wiki page describing how to use the docker-build.sh script. This will need to be updated as well. From what I can see, it would be sufficient to replace --target generalszh with --target z_generals. However, I would suggest removing these instructions from the wiki page and instead referring to the usage section of the script to improve maintainability.

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 3, 2026

Greptile Summary

This PR fixes the docker-build.sh script by replacing incorrect CMake target names (generalszh, generalszh_tools, generalsv_tools) with the actual targets defined in the CMakeLists (z_generals, g_generals, plus the full list of tool targets). The fix aligns the --game derived targets, inline help text, and usage examples with the real build system. The previously flagged stale generalszh example in the -t/--target option description (line 62) has been corrected in this PR.

Confidence Score: 5/5

Safe to merge — all changes are documentation and target-name corrections in a build helper script.

No logic errors introduced; the new target names are confirmed valid against the CMakeLists files in the repo. The previously reported P1 (stale generalszh in the -t/--target description) is already resolved in this diff.

No files require special attention.

Important Files Changed

Filename Overview
scripts/docker-build.sh Corrects all CMake target references: updates usage comments, help text examples, and the derived target lists for --game zh and --game generals to match actual CMakeLists-defined targets; g_wdump is correctly absent (no such target exists in the Generals subdirectory).

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[docker-build.sh invoked] --> B{--target set?}
    B -- Yes --> E[Use explicit TARGET]
    B -- No --> C{--game value}
    C -- zh/zerohour --> D1["TARGET = z_generals z_worldbuilder\ncore_particleeditor core_debugwindow\nz_guiedit z_imagepacker\nz_mapcachebuilder z_w3dview z_wdump"]
    C -- generals --> D2["TARGET = g_generals g_worldbuilder\ncore_particleeditor core_debugwindow\ng_guiedit g_imagepacker\ng_mapcachebuilder g_w3dview"]
    C -- all --> D3["TARGET = (empty = build all)"]
    D1 --> E
    D2 --> E
    D3 --> E
    E --> F[docker run zerohour-build with MAKE_TARGET]
Loading

Reviews (2): Last reviewed commit: "fix: update examples for targets in usag..." | Re-trigger Greptile

Copy link
Copy Markdown

@Skyaero42 Skyaero42 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks fine.

@xezon xezon added Build Anything related to building, compiling Platform Work towards platform support, such as Linux, MacOS labels May 3, 2026
@xezon xezon changed the title fix(docker): Set correct targets in docker-build.sh build(docker): Set correct targets in docker-build.sh May 3, 2026
@xezon xezon merged commit f1341ec into TheSuperHackers:main May 3, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Build Anything related to building, compiling Platform Work towards platform support, such as Linux, MacOS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants