-
-
Notifications
You must be signed in to change notification settings - Fork 613
Fixes #4581: Failure to scan cargo #4582
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
base: develop
Are you sure you want to change the base?
Conversation
|
@AyanSinhaMahapatra review this please !! |
|
@AyanSinhaMahapatra sure I ll add the required test in the test suite !! |
|
@AyanSinhaMahapatra i added the test please review !!
here i used the same cargo.toml file this https://raw.githubusercontent.com/cesarb/constant_time_eq/refs/heads/main/Cargo.toml mentioned in the issue |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| packages_data = cargo.CargoTomlHandler.parse(test_file) | ||
| self.check_packages_data(packages_data, expected_loc, regen=REGEN_TEST_FIXTURES) | ||
|
|
||
| def test_parse_cargo_toml_single_file_no_crash(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@omsuneri the fixes you applied in #4582 are inthe assemble function for cargo, but you are only testing cargo.CargoTomlHandler.parse for a single cargo file which was not what was failing before. You need to add a test which was failing before your changes and passes with your fixes. It is best practice to write the test before you add the fixes anyway.
Create a test with a full --package scan like the test_scan_works_on_cargo_workspace_tauri test below, as assembly functions are called in the package plugin along with the package parsers. See https://github.com/aboutcode-org/scancode-toolkit/blob/develop/src/packagedcode/plugin_package.py for more details.
| [dev-dependencies] | ||
| criterion = { version = "0.5.1", features = ["cargo_bench_support", "html_reports"] } | ||
| count_instructions = "0.2.0" | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you shorten this test file by getting rid of everything below the dev-dependencies as we ignore this anyway as they are not useful info? We keep test files as small as possible to reduce repo size
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@AyanSinhaMahapatra yaa actually i followed the same cargo.toml which is in the issue also before adding i looked in other cargo.toml and there is features and everything so i just added this one without any type of annotation
will be making this shorter !!
Signed-off-by: Om Santosh Suneri <[email protected]>
Signed-off-by: Om Santosh Suneri <[email protected]>
Signed-off-by: Om Santosh Suneri <[email protected]>
b755a2f to
9c3b77f
Compare

Issue :
Fixes #4581
Changes made :
cargo.pyadded proper null check before accessing
.pathon parent resourceadded fallback to use
os.path.dirname(resource.path)when no parent existsnpm.pycode was calling .parent() twice instead of reusing the result
after making changes i passed the same cargo.toml file in the scancode and the error is resolved.
before changes result.json:
after changes result.json: