rust-mobile / cargo-subcommand

Library for implementing cargo subcommands
6 stars 6 forks source link

Support globs (wildcards) in workspace members #2

Closed PrototypeNM1 closed 4 years ago

PrototypeNM1 commented 4 years ago

Fix issue where e.g. wildcards result in Err that is unwrapped by the calling function, resulting an unnecessary build fail as the target fall through is unused.

This popped up when trying to build Bevy (with winit) for Android. It wasn't clear to me that this is the right place to address the issue, for instance it could instead be handled at the calling function's unwrap. In this case the early return also precludes checking following members, so this felt the the nicer way to cleanup given the crate is using a lot of unwraps and removing one wouldn't have put much a dent in it.

enfipy commented 4 years ago

@dvc94ch Can this be merged and published? cargo-apk still has an issue with that.

dvc94ch commented 4 years ago

Sorry, I haven't fully understood when the error happens and what error happens, that's why the PR wasn't merged. Also procrastination.

dvc94ch commented 4 years ago

I see what the problem is, but it seems that there needs to be a way of handling it other than just skipping?

PrototypeNM1 commented 4 years ago

Agreed, updated PR adds glob as a dependency to handle glob members.

dvc94ch commented 4 years ago

Looks good to me! Can you run rustfmt and add the glob dependency alphabetically to Cargo.toml?

PrototypeNM1 commented 4 years ago

Done.

dvc94ch commented 4 years ago

Thanks!

PrototypeNM1 commented 4 years ago

Awesome!

I just noticed the last commit had something to do with forward slashes on Windows. I'm not sure how glob handles that case so it might be worth checking for a regression; fwiw I was developing on Windows.

Lastly a crates.io release would be appreciated. :)

dvc94ch commented 4 years ago

Released as 0.4.6. Not sure about windows, I don't use it...