NMake assumed/forced on Windows even though we have Ninja

[ndabas]

A bug has been uncovered in #88: The generator assumes that nmake is available on Windows.

The pico-setup-windows installer bundles Ninja; additionally, we set the CMAKE_GENERATOR environment variable just to nudge CMake in the right direction when detecting which generator to use.

I'd recommend either not specifying a generator at all (so CMake can autodetect as needed) or check if Ninja is available in the PATH. Personally I'd go with not specifying a generator explicitly, that will either pick it up from CMAKE_GENERATOR or let users specify it that way. The build command can also be generalized to cmake --build <buildDir>.

[lurch]

Just as a slight background here: when Pico SDK (and pico-project-generator) were first released, there was a bug in Ninja that meant it would always fail on Windows; and so users would need to explicitly select NMake. Good to hear that this is no longer necessary now 🙂

[ndabas]

Yes I am well aware of the background 🙂 I built pico-setup-windows shortly after the initial release and followed the instructions in the SDK documentation, which specified NMake, so that's what pico-setup-windows did.

At that time it was a fine assumption to make because we had to install Visual Studio Build Tools with C++ support anyway, in order to compile pioasm and elf2uf2. So NMake was going to be present for sure. There were users on the forum who wanted to use the MinGW toolchain instead, so I understand why pico-project-generator ended up with these two code paths.

Anyway, so Ninja got fixed somewhere along the way, and we do not require a host-native compiler on Windows either (we ship pre-compiled pioasm/elf2uf2/picotool etc.), so it would be good to update these assumptions now.

[BitRolher]

Special care has to be taken cause I am not a makefile expert. Because of that I did not create a merge request.
Following code in pico_project.py fixes it for me:
if isWindows:
# Had a special case report, when using MinGW, need to check if using nmake or mingw32-make.
if shutil.which("mingw32-make"):
# Assume MinGW environment
cmakeCmd = 'cmake -DCMAKE_BUILD_TYPE=Debug -G "MinGW Makefiles" ..'
makeCmd = 'mingw32-make '

    else:
	    # Check if the environment variable CMAKE_GENERATOR specifies Ninja; If yes: Don´t specifiy special case to CMake
        if os.environ.get('CMAKE_GENERATOR', 'not_found') == 'Ninja':
            cmakeCmd = 'cmake -DCMAKE_BUILD_TYPE=Debug ..'
            makeCmd = 'ninja '

        else:
            # Everything else assume nmake
            cmakeCmd = 'cmake -DCMAKE_BUILD_TYPE=Debug -G "NMake Makefiles" ..'
            makeCmd = 'nmake '

[BitRolher]

I have pushed the changes to my fork. So anyone can review or use it if it is good enough.

[lurch]

See also 2ab988c (which is still on the update_vscode_json_to_sdk1_5 branch and hasn't yet made its way into master).

[BitRolher]

See also 2ab988c

I see. Ninja gets default. Great. So I suggest to keep my patch just in my repository.
It was just my intention to share it as temporary solution for anybody who is in need of.