To follow on from my comment above, looking at how it works in runner.ts currently, the option is used to control where the repository being analyzed is checked out to, but we haven't done it properly. It seems wrong to me that we currently use cmd.checkoutPath in initConfig but don't use it when initializing the codeql databases. So to a large degree the changes in this PR are just fixing a bug that was present already 👍

I'm worried there are two usages changed within the runner but only one in the init-action. It looks like there's a use of getRequiredEnvParam("GITHUB_WORKSPACE") passed to initConfig in init-action.ts that also needs changing.

Although, I'm actually not sure that changing the calls to initConfig is the right thing to do. This will change how local queries are resolved and I'm not sure if that is intended or not.

I think we should not change initConfig here. The source-root property is the equivalent of the --source-root flag of the CodeQL CLI and its only purpose it to inform CodeQL what the path to the "source files" is so it can produce the correct relative paths in the SARIF output.

I think we should not change initConfig here.

I've now come around to the other way of thinking. I think we should change initConfig (in the sense that we should pass the checkout path to it) because that's what we're already doing in the runnner. You can see we pass cmd.checkoutPath to initConfig and that's currently the only place we use it in the runner command.

The source-root property is the equivalent of the --source-root flag of the CodeQL CLI and its only purpose it to inform CodeQL what the path to the "source files" is so it can produce the correct relative paths in the SARIF output.

What does "path to the source files" mean? Is that the root of the github repository, or a subdirectory within a repository? When talking about relative paths in the SARIF output we need those to be relative to the root of the repository, don't we? If not then showing files in the code scanning UI will not work.

What does "path to the source files" mean? Is that the root of the github repository, or a subdirectory within a repository? When talking about relative paths in the SARIF output we need those to be relative to the root of the repository, don't we? If not then showing files in the code scanning UI will not work.

Yes that's right. It should be the path to the root of the github repository being analysed, or perhaps more precisely the repository to which the SARIF file will be uploaded. The use-case for this feature is for complex build setups that for example require a bunch of repositories (the repo to be analysed an its dependencies) to be checked out in a particular folder hierarchy that does not match the actions/checkout's defaults .

I've now come around to the other way of thinking. I think we should change initConfig (in the sense that we should pass the checkout path to it) because that's what we're already doing in the runnner. You can see we pass cmd.checkoutPath to initConfig and that's currently the only place we use it in the runner command.

I think there is no such thing as "the checkout path". A user can organise the workspace folder structure in any way they like either by configuring actions/checkout or even cloning repos "manually". I think a user could store the config file in another repo that is checked out somewhere in the workspace, or perhaps even have it in a fixed place on a self-hosted worker.

Although, I'm actually not sure that changing the calls to initConfig is the right thing to do. This will change how local queries are resolved and I'm not sure if that is intended or not.

Yes, I deliberately did not change initConfig because I believe that when someone specifies a local query in the config file, ./ has the connotation of the root of the repository. It would be very confusing to find that it changes based on the source-root specified elsewhere in the workflow file, in my opinion.

Is it for when you want to analyze the whole repository but the repository is checked out to a non-standard location?

Yes, exactly, which, admittedly, is a minor use-case, but one that I have seen already and one which is supported in the CLI with the --source-root and --working-dir flags.

If instead the intention is to be able to analyze only a subdirectory of a repository, then I think this may need deeper thought to know if this will work or not.

My intention was not to replace paths/paths-ignore. Although, it seems it could somewhat be used to that effect.

Finally, what manual testing has been done of this? Have you tried running it in a workflow.

Yes, you can find the workflow here and the results there.

@mario-campos, sorry for the confusing comments above. You can likely ignore my comments and go with what @aibaars says. We had quite a long chat on this and I think we're now on the same page with this, so you can ignore anything contradictory that I said earlier.

Yes, I can add tests. Where would I do that, though? I don't see a test file for init.ts.

Hmm good point, we don't seem to have unit tests. Perhaps another test job in pr-checks.yml, that exercises the source-root field in the init step. I think you can copy the test-packaging-javascript-config job as a template, and remove the packs inputs there. Those tests use a simple multilanguage test folder checked into the repo, so you could point explicitly to that path as the source root.

Huh. Any idea why a source root would be considered invalid (aside from it not existing)?

A fatal error occurred: Invalid source root: /home/runner/work/codeql-action/codeql-action/tests/multi-language-repo.
Error: Failure invoking /opt/hostedtoolcache/CodeQL/0.0.0-20210622/x64/codeql/codeql with arguments database,init,/home/runner/work/_temp/codeql_databases/javascript,--language=javascript,--source-root=/home/runner/work/codeql-action/codeql-action/tests/multi-language-repo.

Exit code 2 and error was:

A fatal error occurred: Invalid source root: /home/runner/work/codeql-action/codeql-action/tests/multi-language-repo.