Skip to content

fix(args): resolve --workdir to a repo-relative include path - #1627

Open
Jorge-Polanco-Roque wants to merge 1 commit into
orhun:mainfrom
Jorge-Polanco-Roque:fix/workdir-empty-output
Open

fix(args): resolve --workdir to a repo-relative include path#1627
Jorge-Polanco-Roque wants to merge 1 commit into
orhun:mainfrom
Jorge-Polanco-Roque:fix/workdir-empty-output

Conversation

@Jorge-Polanco-Roque

Copy link
Copy Markdown

Fixes #1369.

Problem

Pointing --workdir at the repository root produces an empty changelog.

Root cause

When --workdir is set, the changelog is scoped by turning the workdir path into an include pattern. That pattern was absolute (built from workdir.join("")), but diff paths are relative to the repo root — so an absolute pattern matches nothing and the changelog comes out empty.

Approach

Resolve --workdir to a repo-relative include pattern inside process_repository. If workdir resolves to the repo root itself, no filter is added so everything is kept. Removed the old absolute-pattern assignment in run_with_changelog_modifier.

Testing

  • Added the test-workdir-repo-root fixture (commit.sh + expected.md) and wired it into test-fixtures.yml with --workdir "$PWD", which exercises the previously-broken absolute-path case. It produces the full changelog with the fix and empty output without it.

Note on formatting: my added lines are within your max_width/comment_width and add no imports, so they match your nightly rustfmt config; I don't have nightly locally, so a quick cargo +nightly fmt on your side is worth a glance.

A note on direction

I noticed you're planning a more holistic rework of the workdir/include-path handling and that @o1x3 offered to help. This is a minimal, test-backed fix for the empty-output regression specifically — happy for it to serve as a stopgap or a reference for the larger refactor, and equally happy to close it if you'd rather fold it into that work. Per CONTRIBUTING I'm flagging it here rather than assuming a direction; let me know how you'd like to proceed.

@codecov-commenter

codecov-commenter commented Sep 1, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 53.22%. Comparing base (4533ff7) to head (571e984).

Files with missing lines Patch % Lines
git-cliff/src/lib.rs 0.00% 8 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1627      +/-   ##
==========================================
- Coverage   53.30%   53.22%   -0.08%     
==========================================
  Files          28       28              
  Lines        2625     2631       +6     
==========================================
+ Hits         1399     1400       +1     
- Misses       1226     1231       +5     
Flag Coverage Δ
unit-tests 53.22% <0.00%> (-0.08%) ⬇️

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.

@Jorge-Polanco-Roque

Copy link
Copy Markdown
Author

Thanks for the CI run — I pushed a fix for the failing test-workdir-repo-root fixture.

Root cause: the commit.sh I added was committed without its executable bit (mode 100644 instead of 100755), so the ./commit.sh step in the fixtures action hit a permission error, created no commits, and git-cliff then errored on an empty repo. Fixed by adding the exec bit — the fixture now passes.

I reproduced the exact CI invocation locally (git init && ./commit.sh, then git cliff --config cliff.toml --workdir "$PWD") and the output matches expected.md.

The remaining Lints and Build Nix flake failures look unrelated to this change: the clippy errors (collapsible_if, missing_docs) are all in git-cliff-core files this PR doesn't touch and appear to come from a newer nightly toolchain flagging pre-existing code — my lib.rs change isn't among them. Happy to open a separate PR for those if it'd help.

And as noted above: I'm glad to have this closed or folded into the larger workdir/include-path rework you mentioned — it's meant only as a minimal stopgap for the empty-output regression.

@Jorge-Polanco-Roque

Copy link
Copy Markdown
Author

Pushed a fix for the failing Lints check — clippy is now green (cargo clippy --tests -- -D warnings passes). The warnings were collapsible_if (collapsed the nested if let/if into let-chains) and missing_docs on the RustEmbed-generated functions. Formatting is green too.

The remaining red checks look unrelated to this change:

  • Build Nix flake: a transient curl 403 while fetching a crate tarball (no Cargo.lock change here).
  • Check NodeJS tarball: looks for an unpublished git-cliff-*@2.14.1 npm package (release-time check).
  • Test suite / test-release-statistics: repo::test::get_latest_tag and a date-based fixture — history/date-dependent tests that don't pass on a fork checkout, independent of this diff.

Happy to rebase or adjust if you'd like.

@Jorge-Polanco-Roque
Jorge-Polanco-Roque force-pushed the fix/workdir-empty-output branch 2 times, most recently from cbdd99d to 217fb07 Compare September 6, 2026 16:44
@orhun

orhun commented Sep 6, 2026

Copy link
Copy Markdown
Owner

Looks good overall, but please fix merge conflicts, drop the unrelated lint changes and add a fixture test to fully cover #1369

@Jorge-Polanco-Roque

Copy link
Copy Markdown
Author

Thanks for the review! Addressed all three:

  • Rebased onto main and resolved the conflicts (kept the let-chain cwd logic from main and wrote the new --workdir block the same way).
  • Dropped the unrelated lint commits, so the PR is now a single commit with just the --workdir fix.
  • Added a second fixture to fully cover Setting --workdir results in an empty output #1369: alongside test-workdir-repo-root (absolute path), test-workdir-empty-commit reproduces the issue's other case, a relative --workdir . on file-less commits, which used to get filtered out to an empty changelog. Both fixtures pass locally.

)

Setting `--workdir` produced an empty changelog because the derived
include pattern was built from the raw workdir (an absolute or
cwd-relative path), while diff paths are matched relative to the
repository root. The pattern therefore matched nothing.

Resolve the workdir against the repository root after discovery and
add a repo-relative include pattern instead. When the workdir is the
repository root, no filter is added so every commit is kept.

Fixes orhun#1369
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.

Setting --workdir results in an empty output

3 participants