Add VARIABLE_DEFINITION directive location#66
Merged
LegNeato merged 1 commit intographql-rust:masterfrom Jan 30, 2023
vmagro:variable-def-directive
Merged
Add VARIABLE_DEFINITION directive location#66LegNeato merged 1 commit intographql-rust:masterfrom vmagro:variable-def-directive
LegNeato merged 1 commit intographql-rust:masterfrom
vmagro:variable-def-directive
Conversation
facebook-github-bot
pushed a commit
to facebookexperimental/rust-shed
that referenced
this pull request
Jun 28, 2022
Summary: graphql-rust/graphql-parser#66 Reviewed By: zertosh Differential Revision: D37455822 fbshipit-source-id: f69d459c11d3e44166fbd72e70294adfc39ca4aa
facebook-github-bot
pushed a commit
to facebook/sapling
that referenced
this pull request
Jun 28, 2022
Summary: graphql-rust/graphql-parser#66 Reviewed By: zertosh Differential Revision: D37455822 fbshipit-source-id: f69d459c11d3e44166fbd72e70294adfc39ca4aa
facebook-github-bot
pushed a commit
to facebook/fb303
that referenced
this pull request
Jun 28, 2022
Summary: graphql-rust/graphql-parser#66 Reviewed By: zertosh Differential Revision: D37455822 fbshipit-source-id: f69d459c11d3e44166fbd72e70294adfc39ca4aa
facebook-github-bot
pushed a commit
to facebook/fb303
that referenced
this pull request
Jun 30, 2022
Summary: My previous attempt D37455822 (21c109c) apparently broke reindeer, trying again graphql-rust/graphql-parser#66 Reviewed By: zertosh Differential Revision: D37532244 fbshipit-source-id: 755f48e91ce9a72acd2a782a12ee1f1e6ede254b
facebook-github-bot
pushed a commit
to facebook/sapling
that referenced
this pull request
Jun 30, 2022
Summary: My previous attempt D37455822 (6a141e0) apparently broke reindeer, trying again graphql-rust/graphql-parser#66 Reviewed By: zertosh Differential Revision: D37532244 fbshipit-source-id: 755f48e91ce9a72acd2a782a12ee1f1e6ede254b
facebook-github-bot
pushed a commit
to facebookexperimental/rust-shed
that referenced
this pull request
Jun 30, 2022
Summary: My previous attempt D37455822 (5526e05) apparently broke reindeer, trying again graphql-rust/graphql-parser#66 Reviewed By: zertosh Differential Revision: D37532244 fbshipit-source-id: 755f48e91ce9a72acd2a782a12ee1f1e6ede254b
dotansimha
approved these changes
Jul 3, 2022
LegNeato
requested changes
Aug 4, 2022
Member
LegNeato
left a comment
LegNeato
left a comment
There was a problem hiding this comment.
Thank you! Can we get a test to make sure this doesn't regress?
Member
|
Perhaps this and #61 can be merged |
I have some schemas that have this directive location, and this crate currently crashes while trying to parse it.
Contributor
Author
|
@LegNeato I added a test that is similar to other directive parsers, let me know if you had something else in mind |
|
@LegNeato once this lands, any chance we could get a new version of this crate pushed to crates.io along with a new version of |
facebook-github-bot
pushed a commit
to facebook/fb303
that referenced
this pull request
Aug 31, 2022
Summary: Currently, `Cargo.toml` files generated by `autocargo` contain the following line: https://www.internalfb.com/code/fbsource/[159701a8d991]/fbcode/eden/scm/public_autocargo/Cargo.toml?lines=7 At the time of this writing, GitHub still appears to know about `1d155d96e6052767380ab5e67c57e3d6608a31ac`, as it was recently "orphaned" (i.e., it does not belong to a branch), but because it is "orphaned," it appears that GitHub refuses to tell `cargo` about it when it requests it, which breaks builds. To create this diff, I updated the `[patch.crates-io]` section in `third-party/rust/Cargo.toml` and then ran `fbcode/common/rust/cargo_from_buck/bin/autocargo`. I replaced `1d155d96e6052767380ab5e67c57e3d6608a31ac` with the new head of graphql-rust/graphql-parser#66, which is `c778917f57f6b2c26d9291819b9bb341a2f5948a`. 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 (c7c08d9) 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. Reviewed By: fanzeyi Differential Revision: D39190591 fbshipit-source-id: b8d1187cdec9312e9215b28020a735058faeb2d7
facebook-github-bot
pushed a commit
to facebookexperimental/rust-shed
that referenced
this pull request
Aug 31, 2022
Summary: Currently, `Cargo.toml` files generated by `autocargo` contain the following line: https://www.internalfb.com/code/fbsource/[159701a8d991]/fbcode/eden/scm/public_autocargo/Cargo.toml?lines=7 At the time of this writing, GitHub still appears to know about `1d155d96e6052767380ab5e67c57e3d6608a31ac`, as it was recently "orphaned" (i.e., it does not belong to a branch), but because it is "orphaned," it appears that GitHub refuses to tell `cargo` about it when it requests it, which breaks builds. To create this diff, I updated the `[patch.crates-io]` section in `third-party/rust/Cargo.toml` and then ran `fbcode/common/rust/cargo_from_buck/bin/autocargo`. I replaced `1d155d96e6052767380ab5e67c57e3d6608a31ac` with the new head of graphql-rust/graphql-parser#66, which is `c778917f57f6b2c26d9291819b9bb341a2f5948a`. 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 (8a740fa) 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. Reviewed By: fanzeyi Differential Revision: D39190591 fbshipit-source-id: b8d1187cdec9312e9215b28020a735058faeb2d7
facebook-github-bot
pushed a commit
to facebook/sapling
that referenced
this pull request
Aug 31, 2022
Summary: Currently, `Cargo.toml` files generated by `autocargo` contain the following line: https://www.internalfb.com/code/fbsource/[159701a8d991]/fbcode/eden/scm/public_autocargo/Cargo.toml?lines=7 At the time of this writing, GitHub still appears to know about `1d155d96e6052767380ab5e67c57e3d6608a31ac`, as it was recently "orphaned" (i.e., it does not belong to a branch), but because it is "orphaned," it appears that GitHub refuses to tell `cargo` about it when it requests it, which breaks builds. To create this diff, I updated the `[patch.crates-io]` section in `third-party/rust/Cargo.toml` and then ran `fbcode/common/rust/cargo_from_buck/bin/autocargo`. I replaced `1d155d96e6052767380ab5e67c57e3d6608a31ac` with the new head of graphql-rust/graphql-parser#66, which is `c778917f57f6b2c26d9291819b9bb341a2f5948a`. 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. Reviewed By: fanzeyi Differential Revision: D39190591 fbshipit-source-id: b8d1187cdec9312e9215b28020a735058faeb2d7
facebook-github-bot
pushed a commit
to facebook/hhvm
that referenced
this pull request
Aug 31, 2022
Summary: Currently, `Cargo.toml` files generated by `autocargo` contain the following line: https://www.internalfb.com/code/fbsource/[159701a8d991]/fbcode/eden/scm/public_autocargo/Cargo.toml?lines=7 At the time of this writing, GitHub still appears to know about `1d155d96e6052767380ab5e67c57e3d6608a31ac`, as it was recently "orphaned" (i.e., it does not belong to a branch), but because it is "orphaned," it appears that GitHub refuses to tell `cargo` about it when it requests it, which breaks builds. To create this diff, I updated the `[patch.crates-io]` section in `third-party/rust/Cargo.toml` and then ran `fbcode/common/rust/cargo_from_buck/bin/autocargo`. I replaced `1d155d96e6052767380ab5e67c57e3d6608a31ac` with the new head of graphql-rust/graphql-parser#66, which is `c778917f57f6b2c26d9291819b9bb341a2f5948a`. 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 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. Reviewed By: fanzeyi Differential Revision: D39190591 fbshipit-source-id: b8d1187cdec9312e9215b28020a735058faeb2d7
facebook-github-bot
pushed a commit
to facebook/sapling
that referenced
this pull request
Sep 1, 2022
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
LegNeato
approved these changes
Jan 30, 2023
facebook-github-bot
pushed a commit
to facebookexperimental/rust-shed
that referenced
this pull request
Mar 18, 2023
Summary: vmagro deleted his forked and now broke Reindeer importing. Please be more careful. graphql-rust/graphql-parser#66 has been merged but not released. Reviewed By: dtolnay Differential Revision: D44198593 fbshipit-source-id: 287f0f3f6860cdf16deb7be80448c2962a9f10a6
facebook-github-bot
pushed a commit
to facebook/fb303
that referenced
this pull request
Mar 18, 2023
Summary: vmagro deleted his forked and now broke Reindeer importing. Please be more careful. graphql-rust/graphql-parser#66 has been merged but not released. Reviewed By: dtolnay Differential Revision: D44198593 fbshipit-source-id: 287f0f3f6860cdf16deb7be80448c2962a9f10a6
facebook-github-bot
pushed a commit
to facebook/sapling
that referenced
this pull request
Mar 18, 2023
Summary: vmagro deleted his forked and now broke Reindeer importing. Please be more careful. graphql-rust/graphql-parser#66 has been merged but not released. Reviewed By: dtolnay Differential Revision: D44198593 fbshipit-source-id: 287f0f3f6860cdf16deb7be80448c2962a9f10a6
|
@LegNeato I think this PR was never included in a release? any chance we can get a release for that fix? |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
I have some schemas that have this directive location, and this crate
currently crashes while trying to parse it.