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

rust: suggest correct dependency when found in Cargo.lock #13448

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

pbo-linaro
Copy link

As we refer to crates using API version instead of plain version, it can be confusing to user which is left with a simple "Dependency X not found".

Thus, when we detect that a rust dependency is missing, and is not available using a wrap file, if we find something compatible using Cargo.lock import, we just list the correct dependencies names.

@pbo-linaro
Copy link
Author

@xclaesse Followup for conversation on #12945

mesonbuild/interpreter/dependencyfallbacks.py Outdated Show resolved Hide resolved
mesonbuild/interpreter/dependencyfallbacks.py Outdated Show resolved Hide resolved
{
"stdout": [
{
"line": "test cases/rust/26 cargo lock suggest dependency/meson.build:3:0: ERROR: Rust dependency \"bar-1.0-rs\" not found, but following versions were found using Cargo.lock import: \"bar-1-rs\", \"bar-2-rs\", \"bar-0-rs\", \"bar-0.8-rs\""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure whether this really needs a test. All we are testing is the warning message, no? Does that really need an integration test?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If someone changes the cargo.lock import feature later, it would be useful to notice this. But if you don't see any value in this test, I can remove it too.

@pbo-linaro pbo-linaro force-pushed the rust_suggest_dep_from_cargolock branch from 5ce9af5 to f288c1e Compare July 19, 2024 01:39
@pbo-linaro
Copy link
Author

fixed extra parentheses on the if.

@pbo-linaro pbo-linaro force-pushed the rust_suggest_dep_from_cargolock branch from f288c1e to 69f30dc Compare July 19, 2024 02:13
@pbo-linaro
Copy link
Author

Report error only if dependency is required.

s = name.split('-')
if len(s) < 3:
continue
rs_ending = s.pop()

Check notice

Code scanning / CodeQL

Unused local variable Note

Variable rs_ending is not used.
if len(s) < 3:
continue
rs_ending = s.pop()
version = s.pop()

Check notice

Code scanning / CodeQL

Unused local variable Note

Variable version is not used.
s = wrap.name.split('-')
if len(s) < 3:
continue
rs_ending = s.pop()

Check notice

Code scanning / CodeQL

Unused local variable Note

Variable rs_ending is not used.
if len(s) < 3:
continue
rs_ending = s.pop()
version = s.pop()

Check notice

Code scanning / CodeQL

Unused local variable Note

Variable version is not used.
@pbo-linaro pbo-linaro force-pushed the rust_suggest_dep_from_cargolock branch from 69f30dc to f0fed38 Compare July 19, 2024 20:06
@pbo-linaro
Copy link
Author

fixed lint issues

@pbo-linaro
Copy link
Author

@eli-schwartz How can we indicate to meson test infra that the test is supposed to fail?

@pbo-linaro pbo-linaro force-pushed the rust_suggest_dep_from_cargolock branch 2 times, most recently from 83d2050 to 38d4b72 Compare July 21, 2024 18:26
@pbo-linaro
Copy link
Author

More idiomatic python.

mesonbuild/wrap/wrap.py Outdated Show resolved Hide resolved
@pbo-linaro pbo-linaro force-pushed the rust_suggest_dep_from_cargolock branch from 38d4b72 to 5bc92f9 Compare July 21, 2024 22:52
@pbo-linaro
Copy link
Author

Use rsplit to extract crate name + update test to have crate with - in the name.

@pbo-linaro
Copy link
Author

Thanks for your recommendations @dnicolodi @eli-schwartz. I'm still looking for a way to indicate the test is supposed to fail. If you have suggestion, I would be happy to implement it to fix CI.

As we refer to crates using API version instead of plain version, it can
be confusing to user which is left with a simple "Dependency X not
found".

Thus, when we detect that a rust dependency is missing, and is not
available using a wrap file, if we find something compatible using
Cargo.lock import, we just list the correct dependencies names.
@pbo-linaro pbo-linaro force-pushed the rust_suggest_dep_from_cargolock branch from 5bc92f9 to 2272524 Compare July 22, 2024 16:55
@pbo-linaro
Copy link
Author

Found how to implement a "failing" test, and rebased on top of master.

@pbo-linaro
Copy link
Author

Test failure does not seem related to this patch (lto issue on cygwin).

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.

None yet

3 participants