Many thanks for your work on this; glad to see it finally made it into a PR! Before I start reviewing this, can you quickly outline how one enables build tracing?

I've enabled the changes required to make it work globally and not added a flag, since I thought it might be superior in most cases:

• Input packages (i.e. the packages that are passed to the extractor explicitly) are always extracted
• If a dependency of that package is in the same module as the input package, or if it is contained in the source directory of an input package, it is also extracted.

I've enabled the changes required to make it work globally and not added a flag

As discussed previously, I would strongly prefer build tracing to be opt-in, since it is a new feature. Also, doesn't it currently only work for module-based code?

As discussed previously, I would strongly prefer build tracing to be opt-in, since it is a new feature.

Build tracing itself is opt-in, because the autobuilder is run by default, right? The behavior for extraction via autobuilder hasn't changed too much, the only change is some more dependencies may be extracted, but I think they should all be part of the project.

I wouldn't mind adding a flag to make this behavior opt-in, though.

Also, doesn't it currently only work for module-based code?

It falls back on package directory, which is basically what the old code did except in the case that there's a package nested under a package passed to the extractor explicitly.

Actually, before review I'm going to run a dist-compare with a query that selects files; does the below query seem sensible?

import go

from PackageName n
select n.getFile()

I'll mark this as a draft in the interim.

I'm afraid you have lost me. Perhaps I should just look at the code before asking any more questions.

As discussed offline, I would favour breaking this PR up into two parts, one being the general improvements to package handling, and one being the build-tracing specific bits.

I've subverted this PR for extracting more dependencies because most of the comments were related to it.

Comments addressed.

* [ ]  do a dist-compare with `--no-cleanup` and sanity check the results;

Will do. Should I just run a few of the standard queries and a few checks that AST has actually been extracted?

* [ ]  run an ad-hoc query on the databases produced by the dist-compare to get the list of files and compare those before and after the change;

This should now theoretically be in progress, though I've run it wrong about eight times now...

* [ ]  run a Go-Differences job to validate extractor performance.

I'm not entirely sure how to do this; do I fill out the gitCommits parameter or the gitBranch parameter?

Also, the test failures seem to me to be Actions failures, so I've rerun them (the tests pass on my local machine).

Will do. Should I just run a few of the standard queries and a few checks that AST has actually been extracted?

If the changes aren't too voluminous, a standard dist-compare should do the trick, I think.

I'm not entirely sure how to do this; do I fill out the gitCommits parameter or the gitBranch parameter?

Both. gitBranch is where the scripts are taken from; it doesn't matter too much, but I generally use the base commit. Note that all commits have to be commits on git.semmle.com.

How is the evaluation of this PR coming along?

Ah, I'd forgotten to go through and post this.

Dist-compare now shows no results changes and a rather large slowdown in cadence evaluation times, which lines up with this report that says that 22k files were added to the cadence snapshot.

The last Go-Differences job I ran seems to have failed, so I've started a new one.

The last Go-Differences job I ran seems to have failed, so I've started a new one.

That job's failed because it apparently tries to checkout the sub-sha in the ql directory instead of the Go one, so I'm not entirely sure how I should be doing this.

Yes, you cannot use a sub-sha with the Go-Differences job, you have to make a branch in the main repo that points the submodule to the right sha.

• Is it normal in this project/org to keep "review comments" commits or to squash them?

The latter; see the newly added internal Wiki article on the subject.

• Do we get feedback on the command-line indicating what it decided to extract and what it didn't?

The extractor prints out the path of each file it extracts. (This is suppressed by codeql test run, but during normal database builds it ends up in the logs.)

@sauyon, any news on the evaluation?

Sorry, I'd forgotten to review the job. Go-Differences seems to suggest no time differences.

I was looking at the analysis time numbers, whoops. I would have expected cockroach to take more time as it now contains significantly more (generated) files. I'll take a look at concourse too, but I suspect that's a similar story.

I'm not able to reproduce the difference in concourse build time locally. Also, the actual extraction portion for concourse seems to take something like 20s for me, so I think it's probably something to do with the dependency installation or build, which shouldn't have been changed by this PR.

OK, thanks for checking. In that case I'll merge.