8000 make dependencies in `common/` finer-grained by froydnj · Pull Request #6528 · sorbet/sorbet · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content
8000

make dependencies in common/ finer-grained #6528

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Nov 1, 2022
Merged

Conversation

froydnj
Copy link
Contributor
@froydnj froydnj commented Nov 1, 2022

Motivation

I want to parallelize file-finding during option parsing, partly for speed reasons and partly because other LSP-related changes might wind up having to entirely rescan for files, and it would be nicer if the latter made a passing attempt to be fast.

The first blocker here is that we have two Bazel components:

  • //common
  • //common/concurrency

Parallelizing file-finding requires making the first depend on the second, but the second already depends on the first, and Bazel won't let us introduce cycles in our dependency graph.

Therefore, we need to split out some components that just //common/concurrency can depend on without pulling in all of //common. In a future PR, then, we can add the dependency on //common/concurrency to //common without introducing cycles.

This is all relatively straightforward, if a little tedious. The weirdest part of the change is "untangle common/os/os.cc from exception infrastructure with a hammer", which is mostly me being lazy about not splitting out more specific OS-related dependencies (debugging primitives, threading primitives, etc.) so that we could use common/exception/Exception.h etc. in common/os/. If we would like to avoid introducing bare throw logic_error, I can work on splitting things out.

Test plan

Ideally, if this builds, everything should Just Work.

@froydnj froydnj requested a review from a team as a code owner November 1, 2022 15:06
@froydnj froydnj requested review from jez and removed request for a team November 1, 2022 15:06
@froydnj froydnj merged commit 6171e01 into master Nov 1, 2022
@froydnj froydnj deleted the froydnj-os-build-component branch November 1, 2022 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
0