Ruby: Change how we pull in shared/tree-sitter-extractor dependency
#16585
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
criemen
force-pushed
the
criemen/ruby-codeql-extractor-hack
branch
2 times, most recently
from
2aa4561 to
f6a3106
Compare
Previously, we pulled in the shared tree-sitter extractor via a `git` dependency in `Cargo.toml` to address a `rules_rust` limitation (no `path` dependencies outside of the cargo workspace)). This was a problem, as that means we're cloning `github/codeql` _again_ for the build, which is quite slow. I found another way that is faster, and still produces correct builds for both `cargo`` and `rules_rust`: * Cargo depends on a fake crate that has the same dependencies as the real crate (thanks to `sync-files.py`). Therefore, cargo pulls in the right dependencies into the lockfile, which bazel targets * For local builds, we override the path to that dependency in a cargo config, so we're pulling in the correct code * rules_rust only uses `path` dependencies for collecting transitive dependencies, it never pulls in the code from there. So far that, we manually provide a `BUILD.bazel` file for the shared extractor, and depend on that.
criemen
force-pushed
the
criemen/ruby-codeql-extractor-hack
branch
from
f6a3106 to
8c46b61
Compare
|
Ah, one more note: If this weren't a public repository/windows support wouldn't be a concern, I'd have symlinked the files, or even the project directory, but that then creates problems for people checking out the repo on Windows that don't have symlinks enabled. All our developers need to have those enabled anyways for bazel to work, but external contributors probably haven't, so that's a non-starter. |
criemen
force-pushed
the
criemen/ruby-codeql-extractor-hack
branch
from
23b7645 to
6c88b95
Compare
We've removed cross from the internal build when converting to bazel, mirror that here.
criemen
force-pushed
the
criemen/ruby-codeql-extractor-hack
branch
from
6c88b95 to
b09f3c1
Compare
aibaars
reviewed
May 24, 2024
|
|
||
| [patch.crates-io] | ||
| tree-sitter = {git = "https://github.com/redsun82/tree-sitter.git", rev = "1f5c1112ceaa8fc6aff61d1852690407670d2a96"} | ||
| tree-sitter = { git = "https://github.com/redsun82/tree-sitter.git", rev = "1f5c1112ceaa8fc6aff61d1852690407670d2a96" } |
There was a problem hiding this comment.
Why do we have our own fork of tree-sitter?
There was a problem hiding this comment.
That's unrelated to this PR, and is I believe no longer necessary:
tree-sitter/tree-sitter#3352 was merged (which broke our build otherwise), and we're now waiting on tree-sitter cutting a new release for us to pull in that fix via crates.io instead of git.
hvitved
approved these changes
May 27, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously, we pulled in the shared tree-sitter extractor via a
gitdependency inCargo.tomlto address arules_rustlimitation (nopathdependencies outside of the cargo workspace)). This was a problem, as that means we're cloninggithub/codeqlagain for the build, which is quite slow.I found another way that is faster, and still produces correct builds for both
cargo`` andrules_rust`:sync-files.py). Therefore, cargo pulls in the right dependencies into the lockfile, which bazel targetspathdependencies for collecting transitive dependencies, it never pulls in the code from there. So far that, we manually provide aBUILD.bazelfile for the shared extractor, and depend on that.The git dependency was also a major source of slowdown for the upcoming bzlmod/rules_rust conversion, so we needed to get rid of it for that reason, too.