Skip to content

Commit fcbc80b

Browse files
bolinfestfacebook-github-bot
authored andcommitted
remove cargo fetch calls
Summary: Currently, the open source build contains this line in a `Cargo.toml` file: https://github.com/facebook/sapling-staging/blob/c403b32adb27eb0c2d8a6a63261b9bf8942138ca/eden/scm/Cargo.toml#L7 For posterity, the contents of the line are: ``` graphql-parser = { git = "https://github.com/vmagro/graphql-parser", rev = "1d155d96e6052767380ab5e67c57e3d6608a31ac" } ``` The `cargo fetch` calls I am removing in this diff *used to* work. But today when I ran the workflow that used this `Dockerfile`, they failed with: ``` #13 4.589 error: failed to resolve patches for `https://github.com/rust-lang/crates.io-index` #13 4.589 #13 4.589 Caused by: #13 4.589 failed to load source for dependency `graphql-parser` #13 4.589 #13 4.589 Caused by: #13 4.589 Unable to update https://github.com/vmagro/graphql-parser?rev=1d155d96e6052767380ab5e67c57e3d6608a31ac #13 4.589 #13 4.589 Caused by: #13 4.589 revspec '1d155d96e6052767380ab5e67c57e3d6608a31ac' not found; class=Reference (4); code=NotFound (-3) #13 ERROR: process "/bin/sh -c cargo fetch --manifest-path eden/scm/exec/hgmain/Cargo.toml" did not complete successfully: exit code: 101 ------ > [ 8/17] RUN cargo fetch --manifest-path eden/scm/exec/hgmain/Cargo.toml: #13 4.589 error: failed to resolve patches for `https://github.com/rust-lang/crates.io-index` #13 4.589 #13 4.589 Caused by: #13 4.589 failed to load source for dependency `graphql-parser` #13 4.589 #13 4.589 Caused by: #13 4.589 Unable to update https://github.com/vmagro/graphql-parser?rev=1d155d96e6052767380ab5e67c57e3d6608a31ac #13 4.589 #13 4.589 Caused by: #13 4.589 revspec '1d155d96e6052767380ab5e67c57e3d6608a31ac' not found; class=Reference (4); code=NotFound (-3) ``` What happened? Here's what it looks like: - After discovering that our `Cargo.toml` files created by `autocargo` contained massive `[patch.crates-io]` sections that appeared to be slowing down our open source build, I went and posted about it in the Source Control Team group: https://fb.workplace.com/groups/sourcecontrolteam/posts/5310340079087295 - One way to improve the situation is to eliminate these patches from `//third-party/rust`, so I spot-checked a number of the patches, tracking down the diff that introduced them, and attempted to follow-up with the author to upstream the patch. - D37532244 (199330f) appeared to be the diff that pulled in the patch for `graphql-parser`. In the comments, I pinged the author (vmagro) to see if he could upstream his patch. - Vinnie agreed and updated his PR: graphql-rust/graphql-parser#66 - *what appears to have happened* is that Vinnie updated the PR by doing a "force push" where the previous head was `1d155d96e6052767380ab5e67c57e3d6608a31ac` and the new head is `c778917f57f6b2c26d9291819b9bb341a2f5948a`. - GitHub...does not like force pushes. Indeed, if you visit graphql-rust/graphql-parser@1d155d9 you see a yellow warning banner at the top that says **This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.** I don't know what call `cargo` makes to resolve these `git` dependencies, but I assume GitHub is refusing to respond as it normally would for orphaned commits such as these. D39190591 (f500204) is the for fbsource overall, but based on this experience, I still think it is best to remove these `cargo fetch` calls. Of note, **even though the `cargo fetch` calls failed, the GitHub action to build Sapling still succeeds** because Sapling does not depend on `graphql-parser`, so `cargo build` only builds what it needs whereas `cargo fetch` fetches everything in the `Cargo.toml` whether it needs it or not. In sum, given the current state of `autocargo` and the `[patch.crates-io]` section it generates, `cargo fetch` just seems like a giant liability, so we are better off without it. Reviewed By: fanzeyi Differential Revision: D39189595 fbshipit-source-id: cbdf3c51e39b7ed2e49a4373e7f9fc66337766cc
1 parent a47d7c2 commit fcbc80b

File tree

1 file changed

+1
-17
lines changed

1 file changed

+1
-17
lines changed

ci/gen_workflows.py

Lines changed: 1 addition & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,6 @@
1313

1414
DOCKERFILE_DIR = ".github/workflows"
1515

16-
CARGO_FETCH_PATHS = [
17-
"eden/scm/exec/hgmain/Cargo.toml",
18-
"eden/scm/edenscmnative/conch_parser/Cargo.toml",
19-
"eden/scm/exec/scratch/Cargo.toml",
20-
"eden/scm/exec/scm_daemon/Cargo.toml",
21-
]
22-
2316
UBUNTU_VERSIONS = ["20.04", "22.04"]
2417

2518
UBUNTU_VERSION_DEPS = {
@@ -80,15 +73,6 @@ def __init__(self, out_dir: Path):
8073
def gen_build_ubuntu_image(self, *, ubuntu_version: str) -> None:
8174
image_name = f"ubuntu:{ubuntu_version}"
8275
dockerfile_name = f"sapling-cli-ubuntu-{ubuntu_version}.Dockerfile"
83-
cargo_prefetch_commands = "".join(
84-
[f"RUN cargo fetch --manifest-path {p}\n" for p in CARGO_FETCH_PATHS]
85-
)
86-
if cargo_prefetch_commands:
87-
cargo_prefetch_commands = (
88-
"\n# Run `cargo fetch` on the crates we plan to build.\n"
89-
+ cargo_prefetch_commands
90-
)
91-
9276
full_deps = UBUNTU_DEPS + UBUNTU_VERSION_DEPS[ubuntu_version]
9377
dockerfile = f"""\
9478
FROM {image_name}
@@ -113,7 +97,7 @@ def gen_build_ubuntu_image(self, *, ubuntu_version: str) -> None:
11397
# so assume it needs everything.
11498
COPY . /tmp/repo
11599
WORKDIR /tmp/repo
116-
{cargo_prefetch_commands}
100+
117101
# Create and populate a Yarn offline mirror by running `yarn install`
118102
# in the addons/ folder that contains yarn.lock, package.json, and the
119103
# package.json file for each entry in the Yarn workspace.

0 commit comments

Comments
 (0)