Skip to content

Conversation

@valtron
Copy link
Contributor

@valtron valtron commented Oct 24, 2025

Ensure that file URLs created from windows paths are valid, i.e. look like file:///C:/foo/bar. Additionally I saw a few places where preexec_fn was being used on windows, so I added guards there.

Test results:

  • Before: 554 failed, 889 passed, 395 skipped, 6 warnings, 107 errors
  • After: 379 failed, 1080 passed, 395 skipped, 8 warnings, 91 errors

Related: #8474

Copy link
Member

@ThomasWaldmann ThomasWaldmann left a comment

Choose a reason for hiding this comment

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

Some feedback. Windows file URLs confuse me. :)

@ThomasWaldmann
Copy link
Member

Linter is unhappy.

Maybe do that to avoid this automatically: https://borgbackup.readthedocs.io/en/master/development.html#building-a-development-environment

@codecov
Copy link

codecov bot commented Oct 27, 2025

Codecov Report

❌ Patch coverage is 77.77778% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.05%. Comparing base (a329890) to head (1f76b6a).
⚠️ Report is 14 commits behind head on master.

Files with missing lines Patch % Lines
src/borg/repository.py 71.42% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #9097      +/-   ##
==========================================
- Coverage   81.07%   81.05%   -0.03%     
==========================================
  Files          77       77              
  Lines       13540    13554      +14     
  Branches     2008     2011       +3     
==========================================
+ Hits        10978    10986       +8     
- Misses       1900     1904       +4     
- Partials      662      664       +2     

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

@ThomasWaldmann ThomasWaldmann merged commit 8c1d919 into borgbackup:master Oct 27, 2025
10 of 13 checks passed
@ThomasWaldmann
Copy link
Member

Thanks!

Are changes in borgstore needed also?

@ThomasWaldmann
Copy link
Member

Hmm, looks like I overlooked another linter fail. I fixed it in #9108.

@valtron
Copy link
Contributor Author

valtron commented Oct 27, 2025

Are changes in borgstore needed also?

I didn't have any, and haven't run into any other issues so far.

@valtron valtron deleted the fix-win-fileurl branch October 27, 2025 23:37
@ThomasWaldmann
Copy link
Member

@valtron can you have a look at #8727?

I think there is a small overlap between what was fixed here and the fixes there, but it could be that we still need some fixes from there.

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.

2 participants