Simplify Build Process

[jcreedcmu]

Following https://github.com/github/codeql-coreql-team/issues/194, we will not need many of the separate modules in the lib directory. @aeisenberg and I have experienced some friction using rush to manage dependencies and builds of separate modules relative to just gulp-based builds of the extension itself. @dbartol says that he still does find being able to build and/or update dependencies with a single command useful. We should try to find a solution that makes everybody happy.

[dbartol]

I'd like to understand the sources of the friction you've encountered here. In my work on the extension, I pretty much just use rush update, rush build, rush format, and occasionally rush add to add a new dependency. When I do that, the build just works, the dependency updating just works, and formatting of the all of source files just works. Is the friction that one or more of those commands doesn't really just work? Or are there other operations that Rush is making more difficult?

There is a place in the build process where the way we use Rush is overcomplicating things. When setting it up, I chose pnpm as the underlying package manager, based on its ability to solve some of the uglier consequences of npm, like "phantom dependencies" and "doppelgangers". What I did not realize at the time was that when we actually package the extension into a .vsix, the vsce tool pretty much expects the node_modules layout to conform to the typical npm layout, forcing us to have a "deploy" step in the build that more or less manually copies the dependencies into the form that npm would have in the first place. I believe that if we just used npm (or maybe Yarn) as the underlying package manager, we could avoid this step, but I haven't tried it yet.

To summarize a much longer discussion I had in Slack with @jcreedcmu, here's my thought process for why the system we have today is our best option:

Do we really need multiple packages?
Yes, but not as many as we have. We need one package for the extension, and we should have one package for the API that the extension exposes to other extensions, to enable other extensions to run queries (including context-sensitive ones) and help examine results. The API package would be small, consisting mostly of TypeScript declarations for the interfaces that would actually be implemented by the extension package itself. For an example of how other extensions do this, see the vscode-test-explorer extension that our extension depends on at runtime, and its corresponding vscode-test-adapter-api package that our extension depends on at build time for the API declaration.
I also believe that we should keep semmle-vscode-utils separate to make development of internal extensions a bit easier, but that's not as compelling of an argument on its own.

How should we deal with having multiple packages?
The obvious choices here are either a monorepo or a separate repo per package. I'm firmly in the monorepo camp. For whatever friction Rush brings, handling dependencies between simultaneously-developed packages across multiple repos is only going to be worse, whether we update the multiple repos manually, via an internally-published npm package, or via a submodule.

How should we manage our monorepo?
I still think Rush seems to be an entirely reasonable solution to monorepo management, although if I were starting from scratch I would look closer at both Lerna and Yarn workspaces. If moving to either of those solutions would let us continue having a monorepo while removing some real, identifiable friction, without requiring too much work to transition from Rush, I don't have a problem with moving. However, I still don't see the Rush-imposed friction that others do, so I'd be very surprised if such a move could actually reduce the friction further, let alone be worth the effort.

[jcreedcmu]

I think my main difference of opinion is that I really would specifically like to not keep semmle-vscode-utils (and tools/build-tasks and configs/typescript-configs) as separate modules. Already just trying to explore what it would take to fold them into the main body of code I've discovered that our lint rules aren't applying equally to the typescript code in there, and there were a number of typescript-compiler imposed lint-esque errors that weren't detected because of absent/inconsistent tsconfig.json files.

The 'real, identifiable friction' in this case is exactly what I'm experiencing now: trying to make any changes to the build process is quite complicated, and I find it hard to keep all the moving pieces in my head at once. I keep making errors with rush in terms of not being entirely sure what the minimal set of rush unlink and rush purge and rush update is sufficient to get an actually reproducible build that doesn't secretly use some cached state.

[jcreedcmu]

To put it another way: I feel like I'm coming around to believing that rush is a reasonable solution if we are committed to having the extension divided up into a half-dozen different modules, but that division is starting to feel like overkill. The only source of external reusability that I know of is your dependency (somewhere? in an unpublished project?) on semmle-vscode-utils, and I think the consensus between @aeisenberg and I at least is that's a small enough amount of code that simply actually duplicating the contents of it into the extension itself wouldn't feel too terrible to us, and you can continue depending on your own copy of it if you want to maintain that as a module yourself.

[jcreedcmu]

@adityasharad you mentioned having some less-than-positive experiences with rush on other projects --- can you summarize here for the sake of having a more complete picture?

[aeisenberg]

In general, once you understand a few rush commands, it's not difficult to use properly. Here are my issues with rush as it is:

1. We're stuck on old version of pnpm. It might be possible to change to yarn or npm, but then we'd need to put some effort into adapting our build process.
2. Rush is friction for any potential contributors. Maybe we shouldn't be expecting many OSS contributors for this project, but it would be nice.
3. I haven't been able to get incremental builds working well. Any code change invokes a full gulp build, which takes ~25s. It's likely that with some work, I can figure out a way so that gulp watch will be sufficient for most code changes, but so far rush has made this more difficult to figure out.

I like the idea of extracting an API so that downstream extensions can run queries, though this is the first I've heard of it (maybe this was the plan since before I started on this project). With our current breakdown of packages we'd need a lot of work to get there. And, exactly what the API would be is dependent on what sorts of downstream extensions we want to support. I agree that an API package should be part of the same repo as the main extension.

I don't want to derail this conversation, but what sort of extensions do you have in mind?

[aeisenberg]

@adityasharad and I worked on that same project. See https://github.com/covid-modeling/model-runner. The problem main problems we had with rush were:

• its dependency on an old version of pnpm
• the difficulty of migrating a single package repo to a multi-package repo.
• there was lots of configuration we needed to write. This was doable, but I didn't spend the time to read all the docs.

When we tried to use lerna, there was very little friction (though we still have some problems).

[dbartol]

its dependency on an old version of pnpm

Using pnpm was a mistake on my part. I'll give plain old npm a try locally and see if I can get rid of the whole complex deploy process.

the difficulty of migrating a single package repo to a multi-package repo.

We already have a multi-package repo, so that doesn't apply here. Probably true for Rush in general, though.

there was lots of configuration we needed to write. This was doable, but I didn't spend the time to read all the docs.

This is already done in our repo. And mostly it was a matter of taking the initial rush.json generated by rush init and adding the declaration of each package.

[dbartol]

Rush is friction for any potential contributors. Maybe we shouldn't be expecting many OSS contributors for this project, but it would be nice.

Any monorepo system is friction on potential contributors. Is learning three commands (rush update, rush build, and maybe rush format) really that difficult? Especially when contributors who already use VS Code have rush build wired up as the default build command, and the other two tasks right there at the top of the Tasks menu?

I haven't been able to get incremental builds working well. Any code change invokes a full gulp build, which takes ~25s. It's likely that with some work, I can figure out a way so that gulp watch will be sufficient for most code changes, but so far rush has made this more difficult to figure out.

When you say "full gulp build", that's still just a full gulp build of the extension package, right? I expect that most of the time there is spent in the deployment step, which copies all the dependencies and invokes vsce to create the package. I'd be open to changing the default build to not create the package, and the default debug target to run the extension from its own directory rather than the dist directory. I was bitten several times early on by subtle differences between the files that would up getting packaged and files that only existed in the original extension directory, so I made the default as close to production as possible. Now that our CI is much improved, we can probably make the dev inner loop shorter and let the CI catch problems like the ones I hit.
In any case, I don't see how Rush is getting in the way here. rush build literally just calls the build script in each project in order, which just calls gulp build.

[dbartol]

I think my main difference of opinion is that I really would specifically like to not keep semmle-vscode-utils (and tools/build-tasks and configs/typescript-configs) as separate modules. Already just trying to explore what it would take to fold them into the main body of code I've discovered that our lint rules aren't applying equally to the typescript code in there, and there were a number of typescript-compiler imposed lint-esque errors that weren't detected because of absent/inconsistent tsconfig.json files.

The build utils need to be built somehow, before we build the actual extension. Having them in a separate package actually sounds easier to me than complicating the Gulp build within a single package to build the tools first.

I'm fine with tsconfig.json files being shared via some mechanism other than packages. Which tsconfig.json files were missing or inconsistent?

[jcreedcmu]

The build utils need to be built somehow, before we build the actual extension. Having them in a separate package actually
sounds easier to me than complicating the Gulp build within a single package to build the tools first.

Do they? I seem to be experiencing success with naming a directory gulpfile.ts as long as ts-node is installed, cf. https://stackoverflow.com/questions/49847926/how-can-i-run-gulp-with-a-typescript-file

[dbartol]

True, ts-node works.

[dbartol]

I tried some experiments, and my conclusion is that it's impossible for anything involving computers to work correctly.

Rush's support for package managers other than pnpm is basically busted. Like it doesn't even populate your node_modules with all the right packages. Even if we ditched Rush, vsce's support for Yarn is also busted. So, whatever we did would either involve using only npm, or working around vsce's limitations one way or another.

I did try removing the packaging step from the default gulp build for the extension, and that cuts build time roughly in half. The extension still seems to work when launching it under the debugger directly from the extension directory.

[jcreedcmu]

I tried some experiments, and my conclusion is that it's impossible for anything involving computers to work correctly.

On that I can enthusiastically agree :)

Separately I think I'm going to spend a little bit more time experimenting with whether my imagined 'simplifications' actually do simplify things and don't run into any further unexpected obstacles. Definitely I want to say again that I really appreciate your input no matter what, even if I appear to be constantly pushing back on your preferred tool choices.