search

rust-mobile / cargo-subcommand

Library for implementing cargo subcommands
6 stars 6 forks source link

Do not call rustc for platform detection; use `current_platform` instead #22

Closed Shnatsel closed 2 years ago

Shnatsel commented 2 years ago

Use the current_platform crate for zero-cost platform detection. Shaves off 90ms off subcommand startup on my machine (invoking rustc is slow when it's managed by rustup).

The Error::RustcNotFound variant is now never constructed, but I didn't remove it because that would be a semver-breaking change. Likewise with the superfluous .to_owned() - not changed to avoid an API break.

Fixes #19

dvc94ch commented 2 years ago

Thanks!

MarijnS95 commented 2 years ago

We were going to remove this field altogether, right? :disappointed:

dvc94ch commented 2 years ago

Ups, forgot about that

Shnatsel commented 2 years ago

That field is a big part of why I was using cargo-subcommand in the first place. I was hoping the platform detection logic, target directory detection logic, etc could all live in a single place rather than having to be reimplemented by every subcommand.

MarijnS95 commented 2 years ago

@Shnatsel Your entire point in #19 was to not resolve "unnecessary" fields up-front (later refined to rustc -vV being the culprit specifically). It's already a separate crate and unused by cargo-subcommand itself and some of its users so I don't see why cargo-subcommand needs to be clobbered with an extra dependency?