Skip to content

cli-helper: Respect VCS paths in package configs - #12374

Open
CAOShurong wants to merge 1 commit into
oss-review-toolkit:mainfrom
CAOShurong:codex/ort-8713-vcs-path
Open

cli-helper: Respect VCS paths in package configs#12374
CAOShurong wants to merge 1 commit into
oss-review-toolkit:mainfrom
CAOShurong:codex/ort-8713-vcs-path

Conversation

@CAOShurong

Copy link
Copy Markdown
Contributor

Summary

  • add an optional --ort-file input to package-configuration create
  • filter stored repository scan results by the package's processed VCS path
  • preserve the existing behavior when no ORT result is provided

Reproduction

With the evaluation result attached to #8713, current main generated nine path excludes, including paths from unrelated Logback modules. Passing the same result with this change generates only the two expected logback-classic excludes.

The orthw-shell wrapper already retains the scan result used to initialize a workspace. A follow-up there can pass that file through once this CLI option is available.

Verification

  • ./gradlew --no-configuration-cache :cli-helper:test :cli-helper:funTest :cli-helper:detekt
  • added a functional test with a stored full-repository scan and a later vcsProcessed.path curation
  • reran the issue attachment end to end: nine excludes before, two after
  • reran without --ort-file: the original nine-exclude behavior is unchanged

Fixes #8713.

AI assistance was used to inspect the code and draft the patch. The reproduction, implementation, and tests were reviewed and run locally.

@CAOShurong
CAOShurong requested a review from a team as a code owner August 29, 2026 22:43
@CAOShurong
CAOShurong force-pushed the codex/ort-8713-vcs-path branch 2 times, most recently from 7b9114b to c6e7244 Compare August 29, 2026 22:47
@codecov

codecov Bot commented Aug 29, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 77.77778% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 59.32%. Comparing base (dd533ee) to head (c6e7244).

Files with missing lines Patch % Lines
...ain/kotlin/commands/packageconfig/CreateCommand.kt 77.77% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main   #12374      +/-   ##
============================================
+ Coverage     59.12%   59.32%   +0.20%     
- Complexity     1880     1894      +14     
============================================
  Files           365      365              
  Lines         13803    13811       +8     
  Branches       1440     1442       +2     
============================================
+ Hits           8161     8194      +33     
+ Misses         5112     5073      -39     
- Partials        530      544      +14     
Flag Coverage Δ
funTest-no-external-tools 32.03% <77.77%> (+2.56%) ⬆️
test-ubuntu-26.04 42.66% <0.00%> (-0.04%) ⬇️
test-windows-2025 42.64% <0.00%> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread cli-helper/src/funTest/kotlin/commands/packageconfig/CreateCommandFunTest.kt Outdated
Comment thread cli-helper/src/funTest/kotlin/commands/packageconfig/CreateCommandFunTest.kt Outdated
@sschuberth
sschuberth requested a review from a team August 30, 2026 07:16
Filter stored scan results by the package's processed VCS path before
generating path excludes so curations added after scanning are respected.
Preserve existing behavior when no ORT result is provided.

Fixes oss-review-toolkit#8713.

Signed-off-by: CAOShurong <170531907+CAOShurong@users.noreply.github.com>
@CAOShurong
CAOShurong force-pushed the codex/ort-8713-vcs-path branch from c6e7244 to 8dc81fe Compare August 30, 2026 08:58
@sschuberth
sschuberth enabled auto-merge (rebase) August 30, 2026 09:07
).convert { Identifier(it) }
.required()

private val ortFile by option(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

There was a reason to not read an entire OrtFile, when this command was implemented, which still is valid, which is execution performance. E.g. I believe generating a package config should be fast.
Reading an OrtFile makes it much slower.

Question: Could the VCS path also be obtained from the scan results inside scanResultsStorageDir ?

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.

Generated package configuration path excludes does not respect vcs path curations

3 participants