Add versions to link properties in manifest #1104
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1104 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 71 71
Lines 11949 11949
=========================================
Hits 11949 11949 ☔ View full report in Codecov by Sentry. |
BenFordTytherington
force-pushed
the
version-links
branch
from
502b361
to
40229d6
Compare
BenFordTytherington
force-pushed
the
version-links
branch
from
40229d6
to
270648d
Compare
| @@ -46,7 +46,7 @@ fn main() { | |||
| // Disable exporting the standard include directory, as we are exporting custom headers | |||
| .export_include_prefixes([]) | |||
| .export_include_directory(header_dir(), "cxx-qt-lib-extras") | |||
| .reexport_dependency("cxx-qt-lib"); | |||
| .reexport_dependency("cxx-qt-lib-0-6-1"); | |||
There was a problem hiding this comment.
Ah, this is a problem!
With Cargos dependency resolver, we may end up with cxx-qt-lib 0.6.2 as the actual dependency here, as it can choose another compatible version.
In which case, reexport_dependency would no longer work!
And we have the same issue with future minor versions, where Cargo may choose a newer version...
So maybe we only include the part of the version in the links key that actually includes a "breaking change".
E.g. cxx-qt-lib-0.6.
However, that will lead to issues if two downstream dependencies enforce exact minor versions that then will have conflicting links keys...
The other route in which we could solve this issue is to use another name in the reexport_dependency.
I guess we could use the name from the manifest instead 🤔
Any opinions @BenFordTytherington @ahayzen-kdab ?
There was a problem hiding this comment.
@LeonMatthesKDAB Hmm so maybe we reexplore this with the build system refactor in 0.8?
Fix for #1103