k icon indicating copy to clipboard operation
k copied to clipboard

Support nalgebra 0.30 and 0.31

Open AndrewJSchoen opened this issue 3 years ago • 4 comments

Updated the cargo.toml file to support both versions. Running cargo build with 0.31 seems to build without any issue on my machine.

AndrewJSchoen avatar May 26 '22 22:05 AndrewJSchoen

Codecov Report

Merging #91 (2023f12) into main (442f6cb) will not change coverage. The diff coverage is n/a.

@@           Coverage Diff           @@
##             main      #91   +/-   ##
=======================================
  Coverage   68.53%   68.53%           
=======================================
  Files          14       14           
  Lines        1570     1570           
=======================================
  Hits         1076     1076           
  Misses        494      494           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

codecov[bot] avatar May 26 '22 22:05 codecov[bot]

Seems like there may be some issues with kiss3d (which would presumably have to have the same update) based on the tests.

AndrewJSchoen avatar May 26 '22 22:05 AndrewJSchoen

@OTL given that this would require a corresponding fix in Kiss3d, how would you prefer we proceed?

AndrewJSchoen avatar Jun 08 '22 02:06 AndrewJSchoen

Thanks for the PR! The situation is almost the same as before.

There are three major options we can choose from:

The first (regular) way is to update ncollide to use the latest nalgebra (https://github.com/dimforge/ncollide/pull/379), then update kiss3d to use the latest nalgebra and ncollide, and finally update this crate. This way may take longer as we need to wait for ncollide and kiss3d to be updated.

The second way is to replace the use of ncollide in kiss3d with another library (perhaps parry). I'm not the maintainer of kiss3d, so I don't know if this is possible or if this would be acceptable. If this way is accepted, then we need to wait only for kiss3d.

The third way is to replace the use of kiss3d and ncollide in all repositories under github.com/openrr with other libraries. We are considering moving from kiss3d to bevy, ncollide to parry, but don't have the bandwidth to do it right now (the maintainers would accept this approach if it works). In this way, there is no need to wait for other libraries updates. Of course, this way is the hardest.

For now, we are choosing the first way until we have the bandwidth to do the third.

taiki-e avatar Jun 08 '22 05:06 taiki-e

Just to clarify, I think this change should be inconsequential for kiss3d, since if kiss3d has a dependency on an earlier version of nalgebra, the earlier version would be selected in the dependencies. nalgebra = ">= 0.30, >= 0.31" as opposed to simply nalgebra = "0.31" allows both, but if kiss3d is not being used, and a newer version is needed in another library, 0.31 would be selected.

AndrewJSchoen avatar Jan 09 '23 16:01 AndrewJSchoen

I think that will work for private dependencies, but for public dependencies, I think one of the builds will break when both crate using k and nalgebra 0.31 and crate using k and nalgebra 0.30 appear in the dependency graph.

taiki-e avatar Jan 10 '23 04:01 taiki-e

I'm not sure I follow. Is that something we could test locally? I would be happy to try it out. For example, right now, I have another project that also uses/exports their own version of nalgebra (0.31) which means my build always breaks right now unless I use my own fork (which has the above dependency setting).

AndrewJSchoen avatar Jan 10 '23 16:01 AndrewJSchoen

It does not seem to work as you said in https://github.com/openrr/k/pull/91#issuecomment-1375921174. For example, the following will cause a "mismatched types" error due to version mismatch:

[package]
name = "a"
version = "0.1.0"
edition = "2021"

[workspace]

[dependencies]
urdf-viz = { version = "0.41", default-features = false }

[patch.crates-io]
k = { git = "https://github.com/AndrewJSchoen/k.git" }

cargo tree output for the above is:

$ cargo tree -i -p [email protected] -p [email protected]
nalgebra v0.30.1
├── kiss3d v0.35.0
│   └── urdf-viz v0.41.0
│       └── c v0.1.0 (/Users/taiki/projects/tmp/a/c)
└── ncollide3d v0.33.0
    └── kiss3d v0.35.0 (*)

nalgebra v0.31.4
└── k v0.29.1 (https://github.com/AndrewJSchoen/k.git#2023f12c)
    └── urdf-viz v0.41.0 (*)

(My original concern in https://github.com/openrr/k/pull/91#issuecomment-1376732024 was the case where crate like the above example and nalgebra 0.31 are both included in the dependency tree.)

For example, right now, I have another project that also uses/exports their own version of nalgebra (0.31) which means my build always breaks right now unless I use my own fork (which has the above dependency setting).

In this case, you are using a version of nalgebra that is incompatible with the version of nalgebra that k uses, so this is expected behavior. And patching until upstream is updated is also the correct workaround.

taiki-e avatar Jan 11 '23 06:01 taiki-e