feat: Display warning when codeql.cmd is used

[aeisenberg]

The old launcher has been deprecated and codeql.exe is
recommended.

Fixes #287.

[jcreedcmu]

@github/product-docs-dsp does this need docs?

[hmakholm]

Sorry, a late observation that I've only just now connected the dots for:

For customers who need to upload databases to LGTM Enterprise, we recommend they stay with release 2.0.1 of the CLI in order to have compatible extractors. It doesn't have codeql.exe. We probably ought to make an explicit decision whether those customers should get warnings from their extension, or perhaps advised to let the extension use its own managed CLI.

[aeisenberg]

Right...they will get a warning, which should be ignored.

If these users use the extension-managed CLI, would that be compatible with databases downloaded from LGTM built with an old extractor?

I could update the warning message to explicitly say "Don't update if you are using LGTM enterprise", but that's not a great experience.

[hmakholm]

If these users use the extension-managed CLI, would that be compatible with databases downloaded from LGTM built with an old extractor?

Yes, old and new CLI are basically equivalent once a database has been created. Databases from old extractors need to be upgraded before they can be queried with the bleeding edge of Semmle/ql, but all CLI version should be able to do that equally well.

I could update the warning message to explicitly say "Don't update if you are using LGTM enterprise", but that's not a great experience.

I don't have strong opinions about what the outcome should be here -- just pointing out that there's a distinct use case that we ought to think explicitly about (even if we end up explicitly deciding to do nothing).

A possibility would be to put a notice into the release notes for the extension stating that if you're in such and such situation you need to (and have our blessing to) ignore that warning.

[aeisenberg]

@sj do you have any thoughts on this? I'm not sure how confusing this will be for our enterprise customers.

[sj]

I'm honestly a little bit confused myself about this. Why did we switch to an .exe and which group of users is affected by this? Aren't most users letting the VS Code extension manage which CLI is installed?

[hmakholm]

Why did we switch to an .exe and which group of users is affected by this?

The main rationale, as I recall, was that it turned out to be difficult-to-impossible for the .cmd wrapper to preserve correct argument quoting as it passed the command line onwards to the JVM. We had a growing list of "oh, by the way, this option value cannot reliably contain spaces" type exceptions. Switching to an .exe gave us access to much saner string processing for this task.

The directly affected users would be customers who
a) want to use the CodeQL CLI to created databases that they upload to LGTM-e 1.23, and therefore can't upgrade their CLI past release 2.0.1 because that would give them too new extractors, and who also
b) have configured their VSCode extension with the exact path to the CLI they downloaded, including the .cmd suffix.

I haven't followed along closely enough to be able to explain why it is desirable to display the warnings, though.

Hmm, @aeisenberg, would it be feasible to only display the warning after checking that there is in fact an .exe installed that the user can upgrade to?

[aeisenberg]

I think that all sounds reasonable. It shouldn't be a difficult change. If someone is pointing directly to an external instance of the cli, there's likely a reason for that and they won't want to upgrade.

My understanding for the warning is that we want to give users ample warning to switch to the exe in case a future version of the cli removes the cmd.