Skip to content

bevy_reflect incorrectly looks for bevy in dev-deps#1182

Merged
cart merged 2 commits intobevyengine:masterfrom
Moxinilian:reflect-deps-fix
Jan 3, 2021
Merged

bevy_reflect incorrectly looks for bevy in dev-deps#1182
cart merged 2 commits intobevyengine:masterfrom
Moxinilian:reflect-deps-fix

Conversation

@Moxinilian
Copy link
Member

@Moxinilian Moxinilian commented Jan 1, 2021

When trying to determine how it should mention itself in derive macros, the crate finder incorrectly assumes it can use bevy::reflect if bevy is available as a dev dependency. This PR fixes this behavior by ignoring dev-dependencies.

@Moxinilian Moxinilian added C-Bug An unexpected or incorrect behavior core labels Jan 1, 2021
@Moxinilian
Copy link
Member Author

Moxinilian commented Jan 1, 2021

After giving it some thought I realized this means bevy_reflect will no longer work in situations in which examples need bevy_reflect only as a dev dependency. I need to change this.
Implemented in following commit.

@cart
Copy link
Member

cart commented Jan 1, 2021

Hmm isn't this effectively a no-op change?

How is accepting both dev and normal deps different from first accepting normal, then falling back to dev?

@Moxinilian
Copy link
Member Author

Moxinilian commented Jan 2, 2021

Consider the following:

[dependencies]
bevy_reflect = "*"

[dev-dependencies]
bevy = "*"

Originally, the first case would incorrectly use bevy::reflect in the main crate code (as bevy is found, which has priority), whereas here it will correctly pick up bevy_reflect in all cases (as it is always available). This was incorrect because dev-dependencies are not available to main crate code.

Now consider the following:

[dependencies]
# nothing

[dev-dependencies]
bevy = "*"

Without the dev-deps fallback, bevy would not be located if the examples use bevy::reflect.

@cart
Copy link
Member

cart commented Jan 3, 2021

Ah right. I wasn't considering the case where you have the full bevy crate and a subcrate.

@cart cart merged commit 804c068 into bevyengine:master Jan 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-Bug An unexpected or incorrect behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants