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

Replace %2F with / in url for branch #760

Closed

Conversation

matthiasbeyer
Copy link

Motivation

I found that the URL encoding that is put into the Cargo.lock file by cargo does not work in a nix build using crane, because crane does not decode the url in the generated source replacement config.toml.

FWIW, I know that this patch cannot be the actual solution to the problem, but it is a proof of concept that this is the issue.

We (@TheNeikos and I) used this repository:

https://github.com/cloudroots/erased-serde

with this branch: feature/make_make_serializer_public in the repo where we encountered the issue (unfortunately, that repo is still proprietary and closed source).

In the branch setting in the generated source replacement config.toml is branch = "feature%2Fmake_make_serializer_public" - which does not work with cargo and results in cargo failing during buildtime (unfortunately with Temporary failure in name resolution for github.com, which is not really helping here).

Checklist

  • added tests to verify new behavior
  • added an example template or updated an existing one
  • updated docs/API.md (or general documentation) with changes
  • updated CHANGELOG.md

Signed-off-by: Matthias Beyer <mail@beyermatthias.de>
@ipetkov
Copy link
Owner

ipetkov commented Dec 16, 2024

Hi @matthiasbeyer thanks for the PR! I've landed an alternative (and slightly more general) implementation of this in #764. Please try that out and feel free to open a follow up issue if that does not work!

In the mean time will close this out

@ipetkov ipetkov closed this Dec 16, 2024
@matthiasbeyer matthiasbeyer deleted the test-fix-url-encoding branch December 18, 2024 07:48
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