Better bevy version/license algorithm for asset generation #648

Selene-Amanita · 2023-06-14T16:50:22Z

Introduction

The current algorithm for generating assets incorrectly guesses the bevy version and license of some crates.
This PR aims at improving this algorithm.
It was tested and currently "should"¹ ² guess a better version for 28 assets, and the same version for all the others.
It guesses a license where one was missing for 44 assets.

This fixes #475 (mainly, it doesn't go to see third parties dependency but it does fix bevy_tileset version in two ways: looking into dev dependencies and other cargo files of the repository)

Current algorithm

The current algorithm tries to guess the version and license depending on the link provided:

for a Github or Gitlab link, it checks the Github or Gitlab repo of the asset, pulls the root cargo manifest, and checks if any crate in the dependency section starts with "bevy". It takes the first one in alphabetical order.
for a crates.io link, it gets the "last" version of the crate (in lexicographical order), and checks if "bevy" is in its dependencies

Problems with current algorithm

For Github/Gitlab, this algorithm can fail in six cases, if the main crate:

doesn't directly depend on a crate starting with bevy, this is the case for bevycheck (and was for hexx until June the 25th)
depends on crates starting with bevy but they're only third-party crates/subcrates which version don't mimic official's bevy version, this is the case for bevy_tileset, see Fix Bevy version detection for assets with Cargo workspaces #475
depends on crates starting with bevy but the first one in alphabetical order is a third-party one like in the previous case
depends on crates starting with bevy but without a version, this is the case for bevy_kamiya, which would be in the same case as bevy_tileset otherwise
bevy dependency is specified in the workspace, this is the case for Desk X, Digital Extinction, Fun Notation, and bevy_assets_bundler
the first bevy dependency found doesn't specify a version (for example it references a path or a workspace dependency).
only the Cargo.toml file at the root of the project is checked, if it exists, but information about version and/or license is in a Cargo.toml file inside the hierarchy of the project this was the case for Cube Collection, Keep Inside, bevy-inspector-egui, bevy-remote-devtools, bevy_assetio_zip, bevy_rapier, bevy_renet⁰, limbo pass, arugio¹, bevy_wasm², naia⁰ ³,
the license field of the github repo isn't checked, so the license wouldn't be picked if it's not in Cargo.toml, this was the case for (I think, I didn't thouroughly check all of them, it might be problem number 7 for some of them) 2048 Game, Battle City, Bevy Shell, Bounce Up!, Cube Collection, Desk X, Digital Extinction, Dodge the Creeps!, Drone Agility Challenge, Dungeon Quest, Fun Notation, Jump Jump, Keep Inside, Miner, Moon creator, Nodus, Not Snake, One-Click Ninja, Robbo, Rubik's Cube, Screen Ball, Tetris, Tetris for Bevy, yet another, Theta Wave, Unbalanced Brawl, Zenith, arugio, bevy-calc, bevy-inspector-egui, bevy_kagiya, bevy-remote-devtools, bevy_assetio_zip, bevy_doryen, bevy_game_template, bevy_rapier, bevy_renet, bevy_sokoban, bevy_wasm, flock-rs, labyrinth-game, limbo pass, miam, snake_bevy and taileater

⁰(these ones are a special case because the link points inside the hierarchy where the "good" Cargo.toml is but the algorithm ignores it and starts at the root anyway)
¹(not fixed in the diff bellow because of 403 error on search API because of too many requests, I tested separately it's fixed, see #648 (comment))
²(not fixed in the diff bellow because the repo was not indexed for search, I tested after indexing it's fixed)
³(not fixed in the diff bellow because the repository was renamed, see #648 (comment))

For crates.io, this algorithm can fail in two cases:

the crate has a version number >=10 (for example "0.16.0"), which in lexicographical order is less than 9, so the actual last version wouldn't be picked (in the previous example, "0.9.0" would be picked instead), this was the case for bevy-ui-navigation, bevy_asset_loader, bevy_easings, bevy_fly_camera, bevy_framepace, bevy_ninepatch and bevy_water, some of those were following the bevy version number and got wrong when switching to "0.10.0"
the crate doesn't depend on "bevy" but on its subcrates (bevy_ecs, ...), this was the case for bevy_prototype_inline_assets and bevy_system_graph

Proposed solution for Github/Gitlab

Searching recursively for the actual bevy version in subcrates/third party crates would be a bit complex for the bevy website, but there is often a workaround.

Usually crates which reference a subcrate/third party crate would still depend directly on bevy for their examples/tests/benchmark.

In this PR's algorithm, when we check a Cargo.toml, we:

search only the official bevy crates in the dependencies first,
then in the dev dependencies,
then in the workspace dependencies,
(Edit: I deleted this step I think it's not relevant and can fetch a wrong version number) and then only we search any crate starting with bevy hopping we either missed an official crate (past/future versions of bevy) or we find a third-party crate with a version number mimicking official bevy (which was already the case in the previous algorithm)
if we find any bevy dependency along the way, we make sure that we can get a version out of it, or we continue searching

Also, for Github only, we:

check the Cargo.toml at the root of the project first, we continue on error because there could be no Cargo.toml at the root of the project,
if we didn't get a license, we check it on the github repo,
if we are still missing a license or a version, we search the repo for the path of all Cargo.toml files, if this fails we stop here and return what we already have, if this succeed we remove the root Cargo.toml because we already searched in it,
then, we iterate over all the Cargo.toml file and check them for a version a license, until it fails (we then return what we have), we explored all of them, or we found both a version and a license (we consider that there's no need to go further
if we have duplicated/potentially conflicting information (like, two licenses), we try to "merge" it (if we have "MIT" somewhere and "Apache" somewhere else we would put "MIT Apache"), but we don't necessarily explore all the Cargo.toml files to merge everything though, also versions can't be merged because they're not normalized.

Proposed solution for crates.io

I changed the SQLite query to correctly parse major/minor/patch and compare major and minor as integers.
Note: I didn't compare patch as integer and it technically can still have the "over 10" problem, because I didn't want to handle the case where there is a string at the end like "-dev", and I think it wouldn't happen often anyway the patch is usually 0 or 1, sometimes up to 3 but rarely more, and a patch change would normally not change bevy's dependency
I check that the dependencies I filter are not only on "bevy" but on all its subcrates
I did a big SQLite query to do all of that put I store and reuse the prepared statement so I don't think it's a very big deal

Test method

After generating, I used this javascript code to scrape each (asset_title, asset_bevy_version, asset_license) tuples sorted y title on the Asset webpage:

var assetCards = document.getElementsByClassName("asset-card");
var assetList = [...assetCards].map((card) => {
	let title = card.getElementsByClassName("asset-card__title")[0].innerHTML;

	let versionNodes = card.getElementsByClassName("asset-card__bevy-versions");
	let version = null;
	if (versionNodes.length > 0) {
		version = versionNodes[0].getElementsByClassName("asset-card__tag")[0].innerHTML;
	}

	let licenseNodes = card.getElementsByClassName("asset-card__licenses");
	let license = null;
	if (licenseNodes.length > 0) {
		license = [...licenseNodes[0].getElementsByClassName("asset-card__tag")]
			.reduce((acc, v) => {return acc +" "+ v.innerHTML}, "");
  }

	return [title, version, license];
}).sort((a,b) => {
	if(a[0] < b[0]) {
		return -1;
	}
	return 1;
}).reduce((acc, v) => {return acc + v[0] +" | "+ v[1] +" | "+ v[2] +"\n"}, "");
assetList

I then did a simple diff before and after modifications:

Result of the big diff

(you can ignore `Lavagna`, `bevy_scriptum` and `bevy_xpbd`, they are new assets, and `bevy_transform_gizmo`, it changed version recently)


< 2048 Game | 0.9 | null
---
> 2048 Game | 0.9 |  Apache-2.0
4c4
< Battle City | 0.10.1 | null
---
> Battle City | 0.10.1 |  MIT
13c13
< Bevy Shell | 0.9.1 | null
---
> Bevy Shell | 0.9.1 |  Apache-2.0
17c17
< Bounce Up! | 0.9 | null
---
> Bounce Up! | 0.9 |  Apache-2.0
21,26c21,26
< Cube Collection | null | null
< Desk X | null | null
< Digital Extinction | null | null
< Dodge the Creeps! | 0.8 | null
< Drone Agility Challenge | 0.10.1 | null
< Dungeon Quest | 0.7.0 | null
---
> Cube Collection | 0.10.0 |  LGPL-3.0
> Desk X | 0.10 |  MIT Apache-2.0
> Digital Extinction | 0.10 |  GPL-3.0
> Dodge the Creeps! | 0.8 |  CC0-1.0
> Drone Agility Challenge | 0.10.1 |  Apache-2.0
> Dungeon Quest | 0.7.0 |  MPL-2.0
30c30
< Fun Notation | null | null
---
> Fun Notation | 0.10 |  MIT Apache-2.0
32c32
< Jump Jump | 0.9.1 | null
---
> Jump Jump | 0.9.1 |  MIT
34c34
< Keep Inside | null | null
---
> Keep Inside | 0.4 |  CC0-1.0
36d35
< Lavagna | 0.10 |  MIT/Apache-2.0
45c44
< Miner | 0.6 | null
---
> Miner | 0.6 |  Apache-2.0
47c46
< Moon creator | 0.6.1 | null
---
> Moon creator | 0.6.1 |  Apache-2.0
49,50c48,49
< Nodus | null | null
< Not Snake | 0.7.0 | null
---
> Nodus | null |  MIT
> Not Snake | 0.7.0 |  Apache-2.0
54c53
< One-Click Ninja | 0.5.0 | null
---
> One-Click Ninja | 0.5.0 |  MIT
58,60c57,59
< Robbo | 0.4.0 | null
< Rubik's Cube | 0.9 | null
< Screen Ball | 0.10 | null
---
> Robbo | 0.4.0 |  GPL-2.0
> Rubik's Cube | 0.9 |  MIT
> Screen Ball | 0.10 |  MIT
62c61
< Tetris | 0.10 | null
---
> Tetris | 0.10 |  MIT
64c63
< Tetris for Bevy, yet another | 0.10 | null
---
> Tetris for Bevy, yet another | 0.10 |  GPL-3.0
66c65
< Theta Wave | 0.10.1 | null
---
> Theta Wave | 0.10.1 |  MIT
69c68
< Unbalanced Brawl | 0.5.0 | null
---
> Unbalanced Brawl | 0.5.0 |  CC0-1.0
74,75c73,74
< Zenith | 0.5 | null
< arugio | null | null
---
> Zenith | 0.5 |  MIT
> arugio | null |  MIT
78c77
< bevy-calc | 0.5.0 | null
---
> bevy-calc | 0.5.0 |  MIT
83,84c82,83
< bevy-inspector-egui | null | null
< bevy-kajiya | null | null
---
> bevy-inspector-egui | 0.10 |  MIT Apache-2.0
> bevy-kajiya | 0.8.0 |  Apache-2.0
89c88
< bevy-remote-devtools | null | null
---
> bevy-remote-devtools | 0.7 |  MIT
98c97
< bevy-ui-navigation | ^0.6.0 |  MIT Apache-2.0
---
> bevy-ui-navigation | ^0.10 |  MIT Apache-2.0
105c104
< bevy_asset_loader | ^0.6 |  MIT Apache-2.0
---
> bevy_asset_loader | ^0.10 |  MIT Apache-2.0
107,108c106,107
< bevy_assetio_zip | null | null
< bevy_assets_bundler | null |  MIT
---
> bevy_assetio_zip | 0.4 |  non-standard
> bevy_assets_bundler | 0.10 |  MIT
119,120c118,119
< bevy_doryen | git | null
< bevy_easings | ^0.9 |  MIT Apache-2.0
---
> bevy_doryen | git |  MIT
> bevy_easings | ^0.10 |  MIT Apache-2.0
129c128
< bevy_fly_camera | ^0.6.0 |  MIT
---
> bevy_fly_camera | ^0.10.0 |  MIT
132c131
< bevy_framepace | ^0.9 |  MIT Apache-2.0
---
> bevy_framepace | ^0.10 |  MIT Apache-2.0
134c133
< bevy_game_template | 0.10 | null
---
> bevy_game_template | 0.10 |  CC0-1.0
166c165
< bevy_ninepatch | ^0.9 |  MIT Apache-2.0
---
> bevy_ninepatch | ^0.10 |  MIT Apache-2.0
177c176
< bevy_prototype_inline_assets | null |  MIT
---
> bevy_prototype_inline_assets | ^0.3.0 |  MIT
185c184
< bevy_rapier | null | null
---
> bevy_rapier | 0.10 |  Apache-2.0
187c186
< bevy_renet | null | null
---
> bevy_renet | 0.10.1 |  MIT Apache-2.0
194d192
< bevy_scriptum | 0.10.1 |  MIT Apache-2.0
200c198
< bevy_sokoban | 0.8.1 | null
---
> bevy_sokoban | 0.8.1 |  MIT
208c206
< bevy_system_graph | null |  MIT Apache-2.0
---
> bevy_system_graph | ^0.9 |  MIT Apache-2.0
212c210
< bevy_tileset | 0.7 |  MIT Apache-2.0
---
> bevy_tileset | 0.10 |  MIT Apache-2.0
215c213
< bevy_transform_gizmo | 0.10.0 |  MIT Apache-2.0
---
> bevy_transform_gizmo | 0.10.1 |  MIT Apache-2.0
224,225c222,223
< bevy_wasm | null | null
< bevy_water | ^0.9.1 |  MIT/Apache-2.0
---
> bevy_wasm | null |  Apache-2.0
> bevy_water | ^0.10 |  MIT/Apache-2.0
229,230c227
< bevy_xpbd | null | null
< bevycheck | null |  MIT
---
> bevycheck | 0.10 |  MIT
237c234
< flock-rs | 0.4 | null
---
> flock-rs | 0.4 |  MIT
243c240
< labyrinth-game | git | null
---
> labyrinth-game | git |  Apache-2.0
245,247c242,244
< limbo pass | null | null
< miam | 0.8 | null
< naia | null | null
---
> limbo pass | 0.10.0 |  MIT
> miam | 0.8 |  CC0-1.0
> naia | null |  Apache-2.0
263c260
< snake_bevy | 0.3.0 | null
---
> snake_bevy | 0.3.0 |  MIT
265c262
< taileater | 0.5 | null
---
> taileater | 0.5 |  Apache-2.0

I also measured the time of execution pre- and post-PR and it stays around 180s.

Comments on the code

In the first commit of this PR, I changed get_bevy_dependency_version's place in the code, it makes more sense where it is now in my opinion, I changed only the comment so you don't have to check the rest for this commit. I slightly changed the code of it in a later commit though.
I am aware search_bevy_in_dependencies is not very rust-idiomatic, but even though we have two sorted iterators, there's no way to my knowledge to have this optimised "find the first intersecting element" algorithm. I was also limited by what is permitted by const.

alice-i-cecile · 2023-06-14T16:58:40Z

generate-assets/src/lib.rs

@@ -10,6 +10,53 @@ pub mod gitlab_client;

 type CratesIoDb = cratesio_dbdump_csvtab::rusqlite::Connection;

+const OFFICIAL_BEVY_CRATES: &'static [&'static str] = &[


Hmm, I wonder if there's a way to verify that this is kept up to date 🤔

@mockersf mentionned using https://github.com/bevyengine/bevy/blob/main/tools/publish.sh here: #475 (comment)

I didn't use it because I didn't want to spend too long on this, but I agree that it creates a "we need to permanently update a list somewhere in the code" task which isn't great.

Also it simplifies the code that it's a constant, otherwise you would have to fetch it at a higher level and pass and clone the iterator, I guess?

Also btw now that you point that out I forgot: in the list of the link I see that there is no bevy_a11y for example. I simply did a ls of the crates folder of the bevy repo, which does contain bevy_a11y, and this crate exists on crates.io: https://crates.io/crates/bevy_a11y, so I'm not sure which list is correct here?

Another potential source of Bevy crates is https://crates.io/users/cart, this info can probably be retrieved from crates.io dump.

But it seems good enough as a first step to use a static list

Another potential source of Bevy crates is https://crates.io/users/cart, this info can probably be retrieved from crates.io dump.

Just implemented this in a new commit.
I tested it nothing changed in the website diff.

I copied code from https://github.com/alyti/cratesio-dbdump-lookup/blob/main/src/lib.rs#L341 (which is private and I modified it anyway, the crate haven't been updated since 2021, so? Should I make a PR there to put it there instead?)

I didn't use https://crates.io/users/cart I used homepage = "https://bevyengine.org" and repository = "https://github.com/bevyengine/bevy", it should be enough.

The list it pulls btw:

"bevy", "bevy_a11y", "bevy_animation", "bevy_app", "bevy_asset", "bevy_audio", "bevy_core", "bevy_core_pipeline", "bevy_derive", "bevy_diagnostic", "bevy_dylib", "bevy_dynamic_plugin", "bevy_ecs", "bevy_encase_derive", "bevy_gilrs", "bevy_gltf", "bevy_hierarchy", "bevy_input", "bevy_internal", "bevy_log", "bevy_macro_utils", "bevy_math", "bevy_mikktspace", "bevy_pbr", "bevy_property", "bevy_property_derive", "bevy_ptr", "bevy_reflect", "bevy_reflect_derive", "bevy_render", "bevy_render_macros", "bevy_scene", "bevy_sprite", "bevy_tasks", "bevy_text", "bevy_time", "bevy_transform", "bevy_type_registry", "bevy_ui", "bevy_utils", "bevy_wgpu", "bevy_window", "bevy_winit",

I added two new tests to test what happen if there's a problem retrieving the list.

The list seems to contain past bevy crates too and would stay up to date so maybe we could even get rid of the "try to get a crate that starts with bevy as a last resort" part?
It would prevent us from having wrong version number at all like what happened with Nodus, worse case scenario we just don't have a number.
My only "fear" is that past crates would somehow not be pulled anymore (updated from another repo?) and we wouldn't have a failsafe anymore.

You could potentially also check for the list at https://github.com/bevyengine/bevy-crate-reservations/blob/main/reserved_crates which should all be reserved by bevy already

@IceSentry

I think the current list is better since:

I already have the crates.io db dump anyway

it contains only crates that are or were actually used, reserved_crates have a repository of https://github.com/bevyengine/bevy-crate-reservations instead

nobody should depend on reserved crates, and even if they do, I shouldn't pick a version number from it, I would get version 0.0.1 since it's the version of reserved crates, which is wrong

The only problem I see with the current list is if some bevy crates are moved to another repo at some point, but even then we could just modify the SQLite query to reflect that.

Other (very specific) problem I see: a project depending on an official subcrate on its main branch, but that subcrate is a new yet-unrealised one. The "main" version wouldn't be picked then.

alice-i-cecile

This is definitely an improvement, and I'm fine with the clunky search strategy for now.

Tests would be nice if you can think of a good way to mock them up.

Selene-Amanita · 2023-06-14T17:22:21Z

Tests would be nice if you can think of a good way to mock them up.

I can try, but last time I tried generating a manifest with cargo_toml it was painful, I'll see, no promises. Where should I put it, in the same lib.rs?

Also, should I assume the toml crate correctly parses the manifests (like for example if the dependencies are mentionned in a separated section or not etc) or should I start from a string manifest? I feel like I should start with a cargo_toml manifest struct and assume it's correctly parsed by toml, but it might be less clear for people who would read the test?

Edit: I made tests, and modifying the code to handle even more cases, editing the PR right now, I'll make the commit just after

Selene-Amanita · 2023-06-14T21:33:18Z

I added tests, and modified the code to handle more cases.
I edited the first message of the PR to reflect that.

I added a search in the workspace dependencies if dev dependencies fail. I had to update cargo_toml and toml dependencies for that. (btw, should we search in workspace dependencies before dev ones?)
I made sure to continue to search if the dependency we found doesn't specify a version.

Before this PR, those tests failed (I think):

from_dev_dependencies (case 1*, sometimes)
from_workspace_dependencies (case 5*)
from_dependencies_ignore_third_party (case 3*)
from_dev_dependencies_ignore_third_party (case 2*, sometimes)
from_workspace_dependencies_ignore_third_party (case 5*)
from_dev_dependencies_with_path_dependency (case 6*)
from_third_party_crate_with_path_dependency (case 6*)

(* in the "Problems with current algorithm" section)

Before dce0d55, those tests failed (I think):

from_workspace_dependencies (case 5)
from_workspace_dependencies_ignore_third_party (case 5)
from_dev_dependencies_with_path_dependency (case 6)
from_third_party_crate_with_path_dependency (case 6)

Note that those changes make Nodus have an incorrect version because its cargo.toml is actually not valid grammar. But 4 new assets have a correct version!

Also, information: assets bevy-remote-devtools and Keep Inside don't have a version because their Cargo.toml is not at the root of the project :(

mockersf · 2023-06-14T23:08:53Z

generate-assets/src/lib.rs

+                if detail.branch == Some(String::from("main")) {
+                    Some(String::from("main"))


I think I would like to remove this case. Targeting the "main" branch is not really a guarantee that it's tracking main.
A plugin targeting main but that wasn't updated in 10 months won't work again current main.

I wanted to make another PR to fix #477 (and/or address bevyengine/bevy-assets#297 ) so maybe it would be possible to use the last update date in this algorithm at the same time (with a threshold like "it needs to be updated at least less than one month ago to qualify to the main branch tag)?

It kinda make sense to at least display some version in the case where the asset is actually updated regularly to follow main.

I can remove it for now if you want.

alice-i-cecile

Great; I'm glad that wasn't too painful to test in the end :)

Selene-Amanita · 2023-06-15T08:32:51Z

Great; I'm glad that wasn't too painful to test in the end :)

Yeah it was okay, I think what I remembered was generating a Cargo.toml, here for the tests I create a dummy one that probably wouldn't work but it does for the purpose of the test.

I changed the official bevy crates list from static to pulled from crates.io db dump: #648 (comment)

CI failed, and I'm really not sure why (no error message?), I can try to look into it but I would appreciate some pointers if you have any idea because I'm not sure where to start. Maybe updating shalzz/zola-deploy-action to v0.17.2 could fix it magically? I used the last version of Zola and it runs on my machine, and I didn't fundamentally change anything on the website itself??

alice-i-cecile · 2023-06-15T13:50:51Z

Error: Reason: Function call 'resize_image' failed
Error: Reason: resize_image: Cannot find file: ../static/screenshots/UI (User Interface)/borders.png

Clicking through to the failed build-website job (scroll down), this is the error message I'm seeing. Doesn't appear to be your fault: this is likely due to bevyengine/bevy#7795 by @ickshonpe. Perhaps @mockersf can comment on what exactly is going on though.

Selene-Amanita · 2023-06-15T13:57:54Z

Oh okay, good to know!

I just realized the reason I didn't see the error message is because my UMatrix (firefox extension) was blocking the actions.githubusercontent.com api call to fetch it 🤦‍♀️ I had "Error:" with nothing after instead and was very confused hahaha

I still need opinion on at least two points before merging:

should I remove the "main" tag for now, with the possibility of putting it back in another PR: Better bevy version/license algorithm for asset generation #648 (comment) ?
should I remove the part of the algorithm searching for "bevy" prefixed third party crates, resulting in some wrong version assumption: Better bevy version/license algorithm for asset generation #648 (comment) ?
(also very minor: should I check the workspace dependencies before the dev ones?)

IceSentry · 2023-06-15T21:57:42Z

Thanks for working on this! When I made the original algorithm I was mostly focused on getting something working quickly before the release, but the direction you are going with this is much nicer. I'll try to leave a full review soon.

mockersf · 2023-06-25T21:24:11Z

Error: Reason: Function call 'resize_image' failed
Error: Reason: resize_image: Cannot find file: ../static/screenshots/UI (User Interface)/borders.png

Clicking through to the failed build-website job (scroll down), this is the error message I'm seeing. Doesn't appear to be your fault: this is likely due to bevyengine/bevy#7795 by @ickshonpe. Perhaps @mockersf can comment on what exactly is going on though.

There are new examples added to the Bevy main repo, their screenshots need to be added here, I'm doing it from time to time. This will get more stable once the 0.12 is released on webgpu support can use the latest branch.

@Selene-Amanita could you check out bevyengine/bevy-assets#333 and how to get the correct version for bevy_asset_loader?

NiklasEi · 2023-06-25T21:26:02Z

I've been trying to get the correct Bevy version to display for bevy_asset_loader. No combination of [current website main, this PR] x [crates.io, github workspace, github crate directory] results in the correct bevy dependency to be shown.

As a stopgap solution for the "hard cases": could we have an optional bevy version field in the assets data that takes precedence?

Otherwise I guess I am left with trying to change the order of my dependencies or adding a Bevy dependency to the workspace?

Selene-Amanita · 2023-06-26T00:40:49Z

There are new examples added to the Bevy main repo, their screenshots need to be added here, I'm doing it from time to time. This will get more stable once the 0.12 is released on webgpu support can use the latest branch.

@mockersf Okay so I just rebase and I should be good for CI, right?

@Selene-Amanita could you check out bevyengine/bevy-assets#333 and how to get the correct version for bevy_asset_loader?

I've been trying to get the correct Bevy version to display for bevy_asset_loader. No combination of [current website main, this PR] x [crates.io, github workspace, github crate directory] results in the correct bevy dependency to be shown.

@NiklasEi @mockersf After investigating a tiny bit, I think that's what is happening:

putting the github link doesn't work because it only looks at the Cargo.toml that is at the root of the project, and this doesn't contain a bevy dependency (before and after this PR)
putting the cargo.io link doesn't work because it uses a different logic to guess the version, and I didn't update this logic in this PR. From my limited understanding, this algorithm actually takes all the released versions of the crate, and picks the last one, and considers that's the Bevy version. This algorithm seems very wrong, but I'm not sure that's what's actually happening because yeah for "bevy_asset_loader" it would make more sense that it returns "^0.6.0" by using its dependency to "bevy_common_assets". I need to investigate further.

As a stopgap solution for the "hard cases": could we have an optional bevy version field in the assets data that takes precedence?
Otherwise I guess I am left with trying to change the order of my dependencies or adding a Bevy dependency to the workspace?

@NiklasEi @mockersf I see several ways to solve this:

adding bevy -or any subcrate-, as a dependency in the workspace of the github root Cargo.toml would fix the problem, but it's a workaround more than a proper solution.
somehow check all the Cargo.toml in github and gitlab links, not only the root one, but starting with the root one. Will add complexity but will cover more edge cases. I think to do that we need to remove the fallback "pick a dependency that starts with "bevy" which I think we should do anyway because it returns false positives. I can try to do that. Didn't do it to keep the PR simple and not increase generation time and github/gitlab API calls too much (not sure if there's a limit on it/if that's a concern).
Fix the logic to guess the version from crates.io link, I can try to do that too, I didn't do it to keep the PR simple and be able to build on it later.
Add a field to specify a version in the "asset" toml file. I think this should be done, the way I see it is that it would take the max version between the "specified" one and "guessed" one, to avoid toml files not being updated, just in case. I actually wanted to do that, but in another PR, alongside a change to "normalize" versions to not only display them in a coherent way but also be able to compare them, which would allow them to be sorted too, which is another PR I wanted to do, but I was waiting for this one to be merged first. I think to do that we'll need to remove the prefix fallback too.

Tell me which one of those solutions seem reasonable or not.

I'm pretty tired now (late + lack of sleep + "drugged" by meds, apologies if I made mistakes) but I'll investigate more tomorrow!

Selene-Amanita · 2023-06-26T14:51:20Z

@NiklasEi @mockersf

After further investigation, I'm pretty sure the reason crates.io fails is because it takes the "last" version of the crate, except that "last" is defined by lexicographic order, and "0.9" > "0.16". bevy_asset_loader's version 0.9 dependency on Bevy is "^0.6".

I'll try to fix this in this PR as well even thought it's a different part of the algorithm because it's seriously wrong.

mockersf · 2023-06-26T15:17:02Z

oh that's both a 🤣 and a 🤦 bug at the same time. thanks for the investigation!

Selene-Amanita · 2023-06-27T11:35:02Z

I fixed the crates.io algorithm too.

To note: I deleted the "fallback to any dependency whose name starts with bevy" in the github/gitlab algorithm, I really think it's not relevant, prone to error, and it does return an invalid version for Nodus and doesn't improve any other version guess.

I edited the first comment on this PR, some highlights:

a better bevy version is guessed for 9 (!) assets
I also measured the time of execution pre- and post-PR and it stays around 180s
there was another "problem" in the crates.io algorithm: it was only searching for "bevy", not the subcrates like "bevy_ecs"
the "lexicographical" bug still exists for the "patch" of the version (for example if a crate has version 0.1.16), but I think it doesn't matter, see my explanation in the "Note" of the first post

Selene-Amanita · 2023-06-27T23:22:34Z

I added a new commit, this time I changed the algorithm for github, to check the other Cargo.toml files in the repository when needed.

I updated the first post with details, in brief.

11 assets "should" (see 403 and naia bellow) have a better bevy version after this commit, which brings us to a total of 28 (! counting naia and arugio) counting all the improvments of this PR,
44 (!) assets have a better license,
I made a call to Github API to check the license when we don't find it in the root Cargo.toml,
I made a call to Github's search API to search other Cargo.toml files hen we don't have version+license with the root Cargo.toml and the point above

Important:

The algorithm takes 40s more (understandable, it makes more API calls), 220s in total (note: with the 403 errors I'm not confident in that number though it can be a bit more I'll try again tomorrow)
At the end of the algorithm, calls to the "search" API of github would start yielding 403 errors on my computer, I think it's because I make too many of them in too short of a time (me testing my algorithm a lot probably didn't help), that is unfortunate and lead to arugio not having a version guessed, but it doesn't affect the other API calls beside search, so all the assets with a root Cargo.toml will be fine like before, this commit is still a net improvement even if this 403 thing is bothersome
Because the repository was renamed from naia-rs/naia to naia-lib/naia, but not in the assets link, searching Cargo.toml files in naia's repo yield no result so my algorithm can't guess its bevy version

Edit: tested again today, I still have 403 errors at the end, and it's 235s.

Edit2: found the reason for naia not working, I don't think this is a case we should handle.

Selene-Amanita added 2 commits June 14, 2023 17:45

Better Bevy version algorithm for assets

d95f81f

rustfmt

384e6bb

Selene-Amanita changed the title ~~Better bevy version algorithm~~ Better bevy version algorithm for asset generation Jun 14, 2023

alice-i-cecile added C-Bug A problem with the code that runs the site A-Assets The collection of ecosystem crates found on the Bevy Assets page labels Jun 14, 2023

alice-i-cecile reviewed Jun 14, 2023

View reviewed changes

alice-i-cecile approved these changes Jun 14, 2023

View reviewed changes

Add tests and handle more cases

dce0d55

Selene-Amanita added 2 commits June 14, 2023 22:45

Add from_third_party_crate_with_path_dependency test

e8122af

Delete dev comment and modify from_other_dependencies

0ea3c59

mockersf reviewed Jun 14, 2023

View reviewed changes

alice-i-cecile approved these changes Jun 15, 2023

View reviewed changes

Remove static official bevy crates and pull it from crates.io db dump

fcd9b3a

Fix comments

e618f8f

IceSentry self-requested a review June 15, 2023 21:51

Selene-Amanita requested a review from mockersf June 21, 2023 10:57

mockersf mentioned this pull request Jun 25, 2023

Point to bevy_asset_loader on crates.io bevyengine/bevy-assets#333

Merged

Selene-Amanita and others added 2 commits June 27, 2023 12:28

Better Bevy version algorithm for assets with crates.io

6c5b0eb

Merge branch 'bevyengine:main' into better-bevy-version-algorithm

6a73443

Check all cargo.toml files in github repository

f825919

Selene-Amanita changed the title ~~Better bevy version algorithm for asset generation~~ Better bevy version/license algorithm for asset generation Jun 28, 2023

This was referenced Jun 29, 2023

Search all Cargo.toml files from Gitlab project in asset generation #660

Open

Get metadata from Codeberg for asset generation #661

Open

mockersf approved these changes Jul 9, 2023

View reviewed changes

mockersf added the S-Ready-For-Final-Review Ready for a maintainer to consider for merging label Jul 9, 2023

alice-i-cecile added this pull request to the merge queue Jul 10, 2023

Merged via the queue into bevyengine:main with commit 19b289b Jul 10, 2023

Selene-Amanita deleted the better-bevy-version-algorithm branch July 10, 2023 00:52

Selene-Amanita mentioned this pull request Jul 14, 2023

Comment and reorganize the asset generation algorithm #716

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Better bevy version/license algorithm for asset generation #648

Better bevy version/license algorithm for asset generation #648

Selene-Amanita commented Jun 14, 2023 •

edited

Loading

alice-i-cecile Jun 14, 2023

Selene-Amanita Jun 14, 2023

mockersf Jun 14, 2023

Selene-Amanita Jun 15, 2023 •

edited

Loading

IceSentry Jun 15, 2023

Selene-Amanita Jun 15, 2023 •

edited

Loading

Selene-Amanita Jun 18, 2023

alice-i-cecile left a comment

Selene-Amanita commented Jun 14, 2023 •

edited

Loading

Selene-Amanita commented Jun 14, 2023 •

edited

Loading

mockersf Jun 14, 2023

Selene-Amanita Jun 14, 2023

alice-i-cecile left a comment

Selene-Amanita commented Jun 15, 2023 •

edited

Loading

alice-i-cecile commented Jun 15, 2023

Selene-Amanita commented Jun 15, 2023 •

edited

Loading

IceSentry commented Jun 15, 2023

mockersf commented Jun 25, 2023

NiklasEi commented Jun 25, 2023

Selene-Amanita commented Jun 26, 2023 •

edited

Loading

Selene-Amanita commented Jun 26, 2023

mockersf commented Jun 26, 2023

Selene-Amanita commented Jun 27, 2023 •

edited

Loading

Selene-Amanita commented Jun 27, 2023 •

edited

Loading

		@@ -10,6 +10,53 @@ pub mod gitlab_client;

		type CratesIoDb = cratesio_dbdump_csvtab::rusqlite::Connection;

		const OFFICIAL_BEVY_CRATES: &'static [&'static str] = &[

		if detail.branch == Some(String::from("main")) {
		Some(String::from("main"))

Better bevy version/license algorithm for asset generation #648

Better bevy version/license algorithm for asset generation #648

Conversation

Selene-Amanita commented Jun 14, 2023 • edited Loading

Introduction

Current algorithm

Problems with current algorithm

Proposed solution for Github/Gitlab

Proposed solution for crates.io

Test method

Comments on the code

alice-i-cecile Jun 14, 2023

Choose a reason for hiding this comment

Selene-Amanita Jun 14, 2023

Choose a reason for hiding this comment

mockersf Jun 14, 2023

Choose a reason for hiding this comment

Selene-Amanita Jun 15, 2023 • edited Loading

Choose a reason for hiding this comment

IceSentry Jun 15, 2023

Choose a reason for hiding this comment

Selene-Amanita Jun 15, 2023 • edited Loading

Choose a reason for hiding this comment

Selene-Amanita Jun 18, 2023

Choose a reason for hiding this comment

alice-i-cecile left a comment

Choose a reason for hiding this comment

Selene-Amanita commented Jun 14, 2023 • edited Loading

Selene-Amanita commented Jun 14, 2023 • edited Loading

mockersf Jun 14, 2023

Choose a reason for hiding this comment

Selene-Amanita Jun 14, 2023

Choose a reason for hiding this comment

alice-i-cecile left a comment

Choose a reason for hiding this comment

Selene-Amanita commented Jun 15, 2023 • edited Loading

alice-i-cecile commented Jun 15, 2023

Selene-Amanita commented Jun 15, 2023 • edited Loading

IceSentry commented Jun 15, 2023

mockersf commented Jun 25, 2023

NiklasEi commented Jun 25, 2023

Selene-Amanita commented Jun 26, 2023 • edited Loading

Selene-Amanita commented Jun 26, 2023

mockersf commented Jun 26, 2023

Selene-Amanita commented Jun 27, 2023 • edited Loading

Selene-Amanita commented Jun 27, 2023 • edited Loading

Selene-Amanita commented Jun 14, 2023 •

edited

Loading

Selene-Amanita Jun 15, 2023 •

edited

Loading

Selene-Amanita Jun 15, 2023 •

edited

Loading

Selene-Amanita commented Jun 14, 2023 •

edited

Loading

Selene-Amanita commented Jun 14, 2023 •

edited

Loading

Selene-Amanita commented Jun 15, 2023 •

edited

Loading

Selene-Amanita commented Jun 15, 2023 •

edited

Loading

Selene-Amanita commented Jun 26, 2023 •

edited

Loading

Selene-Amanita commented Jun 27, 2023 •

edited

Loading

Selene-Amanita commented Jun 27, 2023 •

edited

Loading