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

Support globs (wildcards) in workspace members #2

Merged
merged 2 commits into from
Oct 28, 2020

Conversation

PrototypeNM1
Copy link
Contributor

@PrototypeNM1 PrototypeNM1 commented Aug 16, 2020

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
Copy link

enfipy commented Oct 26, 2020

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

@dvc94ch
Copy link
Collaborator

dvc94ch commented Oct 28, 2020

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
Copy link
Collaborator

dvc94ch commented Oct 28, 2020

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

@PrototypeNM1 PrototypeNM1 force-pushed the fix-wildcard-workspace branch from d1a0639 to 79e5d63 Compare October 28, 2020 21:03
@PrototypeNM1
Copy link
Contributor Author

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

@dvc94ch
Copy link
Collaborator

dvc94ch commented Oct 28, 2020

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

@PrototypeNM1
Copy link
Contributor Author

Done.

@dvc94ch dvc94ch merged commit d2b6472 into rust-mobile:master Oct 28, 2020
@dvc94ch
Copy link
Collaborator

dvc94ch commented Oct 28, 2020

Thanks!

@PrototypeNM1
Copy link
Contributor Author

PrototypeNM1 commented Oct 28, 2020

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
Copy link
Collaborator

dvc94ch commented Oct 28, 2020

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

@MarijnS95 MarijnS95 changed the title Fix build error when wildcard used in workspace. Support globs (wildcards) in workspace members Sep 20, 2021
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.

3 participants