downloadCargoPackageFromGit fails on Cargo-invalid filenames

[simonzkl]

Describe the bug
downloadCargoPackageFromGit fails when a git dependency contains a filename that Cargo refuses to package, even if the file is unrelated to the build.

This appears to be caused by #976, which changed git vendoring to call cargo package -l. Cargo rejects package filenames containing characters such as :.

Error:

error: cannot package a filename with a special character `:`: fixtures/US:TN.md

Expected: crane vendors the git dependency successfully.

I suppose the repo should fix this so it doesn't contain any files names that are invalid on windows. Though on the other hand, a regular cargo build by itself doesn't choke on these files.

I've worked around it by running this in downloadCargoPackageFromGit prePatch.

find . -depth -name '*[\\<>:"|?*]*' -print -exec rm -rf {} +

Not sure if this is a good idea to generalize though. If there's no reasonable way to handle this, then maybe documenting it with a manual workaround could be the way to go.

Reproduction
Minimal repro:

https://github.com/simonzkl/crane-download-git-invalid-filename-repro

Run:

nix build github:simonzkl/crane-download-git-invalid-filename-repro#

[ipetkov]

Hi @simonzkl thanks for the report! Unfortunately it looks like this hits a bit of an edge case in how we're trying to leverage cargo package -l to respect exclusions in Cargo.toml (as previously reported by others who expected this behavior). It does look like cargo vendor does not fall victim to this check since running the following seems to build fine:

cd $(mktemp)
cargo init --lib
cargo add --git https://github.com/simonzkl/crane-download-git-invalid-filename-repro
mkdir .cargo
cargo vendor >>.cargo/config.toml
cargo build

Unfortunately I don't see an easy way to fix this (without rewriting all the cargo inclusion/exclusion logic) since cargo package -l doesn't seem to have a way to opt out of this (and dropping this feature would break other code already expecting to have their git dependencies' exclusions respected).

Besides patching the sources, as you've already discovered, the only other advice I'd give is either rename the file upstream, or make sure it is listed as part of the crate's file exclusions (since publishing such a crate would fail in the first place, maybe this is a "repo issue")

[simonzkl]

It's interesting that cargo vendor doesn't run into this. Would that not be a valid option for vendoring git dependencies? In any case, I just wanted to highlight the issue if anyone else ever runs into it. A wontfix is completely valid from my pov.

[ipetkov]

I'm not entirely certain if cargo vendor honors inclusions/exclusions, but if it does it may be able to reuse cargo's own internals which aren't accessible from the CLI.

I haven't been brave enough to try to extract the logic from the cargo sources (nor keep up with potential upstream changes). Happy to leave this open for anyone else who may run into it though!