Skip to content
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

Switch .gitmodules to use https #435

Closed
wants to merge 1 commit into from
Closed

Conversation

rajivr
Copy link
Contributor

@rajivr rajivr commented Oct 23, 2023

Issue #, if available:

Description of changes:

I would like to be able to use PartiQL crates directly from git. When I tried to do the following in my Cargo.toml

[dependencies]
partiql-parser = { git = "https://github.com/partiql/partiql-lang-rust.git", rev = "fbaac0fe03"}
partiql-catalog = { git = "https://github.com/partiql/partiql-lang-rust.git", rev = "fbaac0fe03"}
partiql-source-map = { git = "https://github.com/partiql/partiql-lang-rust.git", rev = "fbaac0fe03"}
partiql-ast = { git = "https://github.com/partiql/partiql-lang-rust.git", rev = "fbaac0fe03"}
partiql-logical-planner = { git = "https://github.com/partiql/partiql-lang-rust.git", rev = "fbaac0fe03"}
partiql-logical = { git = "https://github.com/partiql/partiql-lang-rust.git", rev = "fbaac0fe03"}
partiql-value = { git = "https://github.com/partiql/partiql-lang-rust.git", rev = "fbaac0fe03"}
partiql-eval = { git = "https://github.com/partiql/partiql-lang-rust.git", rev = "fbaac0fe03"}

cargo update gives the following error.

$ cargo update
    Updating crates.io index
    Updating git repository `https://github.com/partiql/partiql-lang-rust.git`
error: failed to get `partiql-ast` as a dependency of package `fdb-rl v0.1.0 (/home/rajiv/zfs/foundation-db/dev-env/fdb-rl/fdb-rl)`

Caused by:
  failed to load source for dependency `partiql-ast`

Caused by:
  Unable to update https://github.com/partiql/partiql-lang-rust.git?rev=fbaac0fe03

Caused by:
  failed to update submodule `partiql-conformance-tests/partiql-tests`

Caused by:
  failed to parse url for submodule `partiql-conformance-tests/partiql-tests`: `[email protected]:partiql/partiql-tests.git`

Caused by:
  relative URL without a base

By switching the git protocol to use https this error can be resolved. Here is a working version using a repository where this commit is available.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Signed-off-by: Rajiv Ranganath <[email protected]>
Copy link
Member

@alancai98 alancai98 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your interest in the project! I don't believe it's an issue with our repo and perhaps with the cargo version you're using. Feel free to let me know otherwise.

Regarding the GitHub actions CI issues, think that's an issue on our end related to GitHub actions running on a fork.

@@ -1,3 +1,3 @@
[submodule "partiql-conformance-tests/partiql-tests"]
path = partiql-conformance-tests/partiql-tests
url = git@github.com:partiql/partiql-tests.git
url = https://github.com/partiql/partiql-tests.git
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't quite remember the context but we purposely chose to use git@ rather than use https in #313.

I'm not able to reproduce your error on my end through importing. Perhaps your import error is related to rust-lang/cargo#12295? From that issue, it looks like there was a bug when a submodule used [email protected]:... for some Rust/cargo versions. Maybe you'll need to update your Rust/cargo version?

Copy link
Contributor Author

@rajivr rajivr Oct 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the reply and the pointer to the Cargo issue.

I updated to latest rust toolchain (rustc 1.75.0-nightly (df871fbf0 2023-10-24) and cargo 1.75.0-nightly (d2f6a0485 2023-10-20)). While the issue still remains, the error is different.

I suspect the reason why it might be working for you could be because you have privileged access to partiql/partiql-tests.git repository (see below).

After updating to latest rust toolchain, when I did cargo update, it nudged me to set net.git-fetch-with-cli=true.

$ cargo update
    Updating crates.io index
    Updating git repository `https://github.com/partiql/partiql-lang-rust.git`
    Updating git submodule `[email protected]:partiql/partiql-tests.git`
error: failed to get `partiql-ast` as a dependency of package `fdb-rl v0.1.0 (/home/rajiv/zfs/foundation-db/dev-env/fdb-rl/fdb-rl)`

Caused by:
  failed to load source for dependency `partiql-ast`

Caused by:
  Unable to update https://github.com/partiql/partiql-lang-rust.git?rev=fbaac0fe03

Caused by:
  failed to update submodule `partiql-conformance-tests/partiql-tests`

Caused by:
  failed to fetch submodule `partiql-conformance-tests/partiql-tests` from [email protected]:partiql/partiql-tests.git

Caused by:
  failed to authenticate when downloading repository

  * attempted ssh-agent authentication, but no usernames succeeded: `git`

  if the git CLI succeeds then `net.git-fetch-with-cli` may help here
  https://doc.rust-lang.org/cargo/reference/config.html#netgit-fetch-with-cli

Caused by:
  no authentication methods succeeded

Once net.git-fetch-with-cli=true was set, the error seems to be more precise. My GitHub public key cannot be used for the repository - [email protected]: Permission denied (publickey). Please make sure you have the correct access rights and the repository exists..

$ cargo update
    Updating crates.io index
    Updating git repository `https://github.com/partiql/partiql-lang-rust.git`
    Updating git submodule `[email protected]:partiql/partiql-tests.git`
[email protected]: Permission denied (publickey).
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.
error: failed to get `partiql-ast` as a dependency of package `fdb-rl v0.1.0 (/home/rajiv/zfs/foundation-db/dev-env/fdb-rl/fdb-rl)`

Caused by:
  failed to load source for dependency `partiql-ast`

Caused by:
  Unable to update https://github.com/partiql/partiql-lang-rust.git?rev=fbaac0fe03

Caused by:
  failed to update submodule `partiql-conformance-tests/partiql-tests`

Caused by:
  failed to fetch submodule `partiql-conformance-tests/partiql-tests` from [email protected]:partiql/partiql-tests.git

Caused by:
  process didn't exit successfully: `git fetch --tags --force --update-head-ok '[email protected]:partiql/partiql-tests.git' '+refs/heads/*:refs/remotes/origin/*' '+HEAD:refs/remotes/origin/HEAD'` (exit status: 128)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect the reason why it might be working for you could be because you have privileged access to partiql/partiql-tests.git repository (see below).

Hmm partiql/partiql-tests is a public repo so I don't believe that should cause the permission issue you're seeing. Could you verify that you have an ssh-agent running (relevant cargo doc)? And that the ssh key on your computer (running ssh-add -l) matches up with what's displayed on https://github.com/settings/keys? This was an issue I faced when GitHub updated the SSH host keys earlier this year.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI the above cargo doc link states

Note: Cargo does not support git’s shorthand SSH URLs like [email protected]:user/repo.git. Use a full SSH URL like ssh://[email protected]/user/repo.git.

I believe this note only applies to shorthand URLs specified in the Cargo.toml files and not in the .gitmodules submodule file.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm partiql/partiql-tests is a public repo so I don't believe that should cause the permission issue you're seeing. Could you verify that you have an ssh-agent running (relevant cargo doc)? And that the ssh key on your computer (running ssh-add -l) matches up with what's displayed on https://github.com/settings/keys? This was an issue I faced when GitHub updated the SSH host keys earlier this year.

You are right! Turns out this was an "authentication" issue. When cloning using [email protected]:..., it seems authentication is mandatory, and that extends even to git submodules via cargo.

I use [email protected]:... URIs only for the repositories that I own and store my SSH key in a hardware token. Since my hardware token was not available when doing cargo update, and because cargo and git were trying to authenticate, and this was failing.

Inserting my hardware token and doing a cargo update work. Thanks for the help and the pointers.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, glad we were able to find the issue! I'll close this PR then.

Feel free to add to add comments or reopen if you face further issues w/ the git submodule.

@alancai98 alancai98 closed this Oct 27, 2023
@rajivr rajivr deleted the gitmodules branch January 16, 2024 13:44
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