Unknown extra binary listed for data-encoding-bin

[ia0]

Current Behavior

I've read about #5882 in the news and wanted to check it on my binary crates:

• For wasefire-cli it's all good. The text displays Running the above command will globally install the wasefire binary. which is correct.
• However for data-encoding-bin it's wrong. The text displays Running the above command will globally install the data-encoding and data-encoding-bin binaries. while to my understanding there should only be a single build-target, named data-encoding.

Expected Behavior

For data-encoding-bin, the text displays Running the above command will globally install the data-encoding binary..

Steps To Reproduce

1. Go to https://crates.io/crates/data-encoding-bin
2. Look at the install section on the right
3. cargo install data-encoding-bin
4. ls ~/.cargo/bin/data-encoding* lists only data-encoding binary

Environment

I hope it's irrelevant, but Chrome on Linux.

Anything else?

No response

[Turbo87]

hmm, from what I can see in the Cargo.toml file that indeed appears to be incorrect. I'll take a look once I'm back from vacation :)

[ia0]

Thanks for the quick feedback! No rush :-) enjoy your vacation!

[eth3lbert]

I think the culprit is the following:

https://github.com/LukeMathWalker/cargo-manifest/blob/00bef139e1a40b468bfb4ef5965d4eb488a8da1e/src/lib.rs#L338-L347

            let has_main_rs = src.contains("main.rs");
            if has_main_rs {
                let target = DiscoveredTarget {
                    name: package.name.clone(),
                    path: "src/main.rs".to_string(),
                };
                discovered_targets.push(target);
            }

            process_discovered_targets(&mut self.bin, discovered_targets, autobins)?;

It adds a target with the package name (data-ecoding-bin in this case), but there's already a [[bin]] with the name data-encoding declared in the manifest¹ that also has the same path src/main.rs. I think it should only add new DiscoveredTarget if there are no declared bin with the same path.

Footnotes

1. manifest https://docs.rs/crate/data-encoding-bin/latest/source/Cargo.toml ↩

[ia0]

Maybe that's enough to fix the issue, but it looks to me that the situation is more complicated than that. To the best of my understanding, the wasefire-cli scenario and the data-encoding-bin scenario are very similar. They only expose a single build-target with a different name than the crate name and with src/main.rs as their path. However, one is working correctly (a single build-target shown in crates.io) and one is not (2 build-targets showing up). The only difference I can think of (given I've read there was some form of a backfill), is that data-encoding-bin was published 3 months ago while wasefire-cli was only published 13 days ago. Thus data-encoding-bin might have been through the "backfill" path, while wasefire-cli went through the "new" path. Is the code snippet you showed for both paths or only one (logically the "backfill" one)?

[eth3lbert]

Maybe that's enough to fix the issue, but it looks to me that the situation is more complicated than that. To the best of my understanding, the wasefire-cli scenario and the data-encoding-bin scenario are very similar. They only expose a single build-target with a different name than the crate name and with src/main.rs as their path. However, one is working correctly (a single build-target shown in crates.io) and one is not (2 build-targets showing up). The only difference I can think of (given I've read there was some form of a backfill), is that data-encoding-bin was published 3 months ago while wasefire-cli was only published 13 days ago. Thus data-encoding-bin might have been through the "backfill" path, while wasefire-cli went through the "new" path. Is the code snippet you showed for both paths or only one (logically the "backfill" one)?

Oh, there's autobins = false in the manifest of wasefire-cli, which is different from data-encoding-bin. And if autobins is false, according to https://github.com/LukeMathWalker/cargo-manifest/blob/00bef139e1a40b468bfb4ef5965d4eb488a8da1e/src/lib.rs#L455, wasefire-cli won't be added.

[ia0]

Thanks for the debugging! I think it makes sense now. For wasefire-cli I used a version of cargo that had rust-lang/cargo#13713, while I couldn't have for data-encoding-bin since that PR was merged after the publication of that crate.

[Turbo87]

LukeMathWalker/cargo-manifest#51 should hopefully fix the issue

[Turbo87]

fix is deployed and I've rerun the backfilling script for crate versions with more than one bin (about 47k versions). I think we're done here :)

let me know if I missed anything!

[ia0]

Looks good to me, thanks!