fix(cargo): Treat projects outside the analyer root as packages #8574
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #8574 +/- ##
=========================================
Coverage 67.70% 67.70%
Complexity 1007 1007
=========================================
Files 246 246
Lines 7924 7924
Branches 883 883
=========================================
Hits 5365 5365
Misses 2176 2176
Partials 383 383
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
sschuberth
force-pushed
the
cargo-oot-packages
branch
from
1955e0f to
c41c4c4
Compare
plugins/package-managers/cargo/src/funTest/assets/projects/synthetic/cargo-expected-output.yml
Outdated
Show resolved
Hide resolved
sschuberth
force-pushed
the
cargo-oot-packages
branch
2 times, most recently
from
a18952e to
92a8819
Compare
| } | ||
|
|
||
| return listOf(ProjectAnalyzerResult(project, nonProjectPackages)) | ||
| } | ||
| } | ||
|
|
||
| private fun CargoMetadata.Package.isProject() = source == null | ||
| /** | ||
| * Return the local path for this Cargo package if applicable, or null if the Cargo package is not local. |
There was a problem hiding this comment.
Is this always an absolute path and should it be mentioned in the docs or even checked with an assertion?
(I'm asking this because the startWith() check within isProject() seems to rely on that)
There was a problem hiding this comment.
Is this always an absolute path
Yes, because it's derived from definitionFile which is guaranteed to be absolute.
should it be mentioned in the docs or even checked with an assertion?
I'm simply calling absoluteFile now to make it work in any case.
There was a problem hiding this comment.
Yes, because it's derived from
definitionFilewhich is guaranteed to be absolute.
My question was about the File returned by getLocalPath() not about the parent of the definitionFile.
So, should the absoluteFile call be moved into getLocalPath() ?
There was a problem hiding this comment.
Yes, that's also guaranteed to be absolute (as it's derived from a URI).
There was a problem hiding this comment.
Would a require check make sense / mention in the function docs of getLocalPath() ? ..ort rather .also {checkState(it.isAbsolute) } ?
sschuberth
force-pushed
the
cargo-oot-packages
branch
from
92a8819 to
d32f7dc
Compare
Signed-off-by: Sebastian Schuberth <[email protected]>
Signed-off-by: Sebastian Schuberth <[email protected]>
sschuberth
force-pushed
the
cargo-oot-packages
branch
from
d32f7dc to
2d2fc45
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
E.g. when analyzing a single Cargo project in a sub-directory with a `cargo.toml` file that points to "path dependencies" in a parent directory, these dependencies should not be seen as projects but as packages. This restores the behavior from before 7522a0c. Resolves #8571. Signed-off-by: Sebastian Schuberth <[email protected]>
sschuberth
force-pushed
the
cargo-oot-packages
branch
from
2d2fc45 to
83c7d62
Compare
E.g. when analyzing a single Cargo project in a sub-directory with a
cargo.tomlfile that points to "path dependencies" in a parent directory, these dependencies should not be seen as projects but as packages. This restores the behavior from before 7522a0c.Resolves #8571.