Skip to content

Fix Saigo SDK-related merge mishaps - #2032

Open
slipher wants to merge 4 commits into
DaemonEngine:masterfrom
slipher:saigosdk-merge
Open

slipher wants to merge 4 commits into
DaemonEngine:masterfrom
slipher:saigosdk-merge

Conversation

@slipher

@slipher slipher commented Nov 4, 2026

Copy link
Copy Markdown
Member

Some fix-ups to get my work from #2010 to its true form.

slipher and others added 4 commits November 4, 2026 03:30
Come on, why do I have to put the .exe?

There was supposed to be a commit doing this in DaemonEngine#2010 but then I
accidentally removed it.
The version of the Saigo SDK download merged as part of DaemonEngine#2018 had some
regressions compared to the original:
- The path cache variable was not presented as a configurable option to
  the user on first configure.
- If download failed, it could not be retried.
- Disabling USE_EXTERNAL_DEPS did not stop external_deps from being
  automatically used.

I do think it was a good idea to set SAIGOSDK_PREFIX in the cache upon
downloading it. So I rewrote things a bit from the original to do this.
It's important to set the cache only after the download succeeds to
avoid the 2nd regression.
Make possible to cross-compile a Windows engine from Linux
while running a native Linux SDK to build the NaCl game.
I originally wanted to use the name SAIGISDK_ROOT, but looking at the
CMake cache for an Unvanquished build, DIR is by far the most common
suffix for directories that pertain to a single dependency. (Sometimes
ROOT_DIR is additionally used but we don't have any of those.) PREFIX is
used more to refer to a Unix-style set up where multiple softwares may
be mingled in the same directories. It's true that the Saigo SDK was
built that way, installing several programs together, but that's not
relevant for a CMake toolchain consumer. Anyway I think PREFIX is also
not good because it's not too comprehensible to people without much
Unixy background. I went many years in coding before understanding what
"prefix" refers to.

@illwieckz illwieckz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM.

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.

2 participants