Fix diff-file being created when not configured

[flyinghyrax]

This fixes path handling in the mirror subcommand that caused a diff file to be written to the current directory if diff-file wasn't set in the configuration file. This is the issue where Path("") becomes Path(".") and boolean-checks as truthy.

I encountered what might be an unexpected behavior when diff-file is set to an existing directory and diff-append-epoch is enabled. I've added a unit test to capture the current behavior, but it might be worth changing depending on how you feel about the compatibility break.

Split into 3 commits:

1. Removes a diff_file argument and attribute from BandersnatchMirror. The initializer accepted both diff_file and diff_full_path, and diff_file doesn't appear to be used anywhere. I went ahead and removed it while I was around then extracted this change into its own commit so it can be accepted/removed on its own.

2. Adds 2 tests for the mirror subcommand:

   • check whether a diff file is being created when not configured
   • capture the interaction between the diff-file and diff-append-epoch options when diff-file is set to an existing directory

   If you check out this commit, the first test is expected to fail.

3. Changing the path handling in the mirror subcommand for diff-file.

[codecov]

Codecov Report

Attention: Patch coverage is 92.18750% with 5 lines in your changes are missing coverage. Please review.

Project coverage is 83.92%. Comparing base (4d020e8) to head (6cbab83).
Report is 74 commits behind head on main.

Files	Patch %	Lines
src/bandersnatch/mirror.py	82.60%	2 Missing and 2 partials ⚠️
src/bandersnatch/tests/test_mirror.py	97.56%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1716      +/-   ##
==========================================
+ Coverage   79.69%   83.92%   +4.23%     
==========================================
  Files          31       32       +1     
  Lines        4324     4405      +81     
  Branches      780      791      +11     
==========================================
+ Hits         3446     3697     +251     
+ Misses        721      526     -195     
- Partials      157      182      +25     

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

[cooperlees]

LGTM - Just a couple NITs and questions ... If I misunderstood just explain so and we can look at accept and merge.

[cooperlees]

Suggested change

	assert len(lines) > 0
	assert len(lines) == 3

This should always be the same number of lines right? Or did I count wrong / miss why > 0 is better here?

[flyinghyrax]

You didn’t miss anything, I think this was just laziness 😆 And after already mocking synchronize it’s kind of silly.

I’ll double-check this and update it.

[flyinghyrax]

Okay so something interesting happened here after all, on my Windows host anyway, here's my longwinded over explanation. 😬

When I changed the assertion to len(lines) == 3, it failed on 5 != 3. Stepping through the test in the debugger, sure enough lines had 2 extra blank lines for some reason:

[
'C:\\Users\\mrsei\\AppData\\Local\\Temp\\pytest-of-mrsei\\pytest-160\\test_mirror_subcommand_diff_fi0\\mirror\\AB\\C1\\ABC123\\black-123-py3-non-any.whl', 
'', 
'C:\\Users\\mrsei\\AppData\\Local\\Temp\\pytest-of-mrsei\\pytest-160\\test_mirror_subcommand_diff_fi0\\mirror\\98\\7D\\987DEF\\ruff-123.tar.gz', 
'', 
'C:\\Users\\mrsei\\AppData\\Local\\Temp\\pytest-of-mrsei\\pytest-160\\test_mirror_subcommand_diff_fi0\\mirror\\98\\7D\\987DEF\\ruff-123-py3-none-any.whl'
]

And opening the file in a text editor it appears that way as well.

But if break before the text gets written to the file and look at diff_text:

'C:\\Users\\mrsei\\AppData\\Local\\Temp\\pytest-of-mrsei\\pytest-161\\test_mirror_subcommand_diff_fi0\\mirror\\AB\\C1\\ABC123\\black-123-py3-non-any.whl\r\nC:\\Users\\mrsei\\AppData\\Local\\Temp\\pytest-of-mrsei\\pytest-161\\test_mirror_subcommand_diff_fi0\\mirror\\98\\7D\\987DEF\\ruff-123-py3-none-any.whl\r\nC:\\Users\\mrsei\\AppData\\Local\\Temp\\pytest-of-mrsei\\pytest-161\\test_mirror_subcommand_diff_fi0\\mirror\\98\\7D\\987DEF\\ruff-123.tar.gz'

Since that's escaped by the debugger it looks like hot garbage, but you can see the \r\ns in there and running it through print shows exactly three lines:

C:\Users\mrsei\AppData\Local\Temp\pytest-of-mrsei\pytest-161\test_mirror_subcommand_diff_fi0\mirror\AB\C1\ABC123\black-123-py3-non-any.whl
C:\Users\mrsei\AppData\Local\Temp\pytest-of-mrsei\pytest-161\test_mirror_subcommand_diff_fi0\mirror\98\7D\987DEF\ruff-123-py3-none-any.whl
C:\Users\mrsei\AppData\Local\Temp\pytest-of-mrsei\pytest-161\test_mirror_subcommand_diff_fi0\mirror\98\7D\987DEF\ruff-123.tar.gz

The real weirdness (and hint) was when I looked at the diff file in a hex editor:

The paths are separated by \r\r\n. And after staring a while I remembered universal newlines.

---

This is easy enough to fix, but do you think this may impact anyway scripting against the diff file output? I'm guessing if I were to bash such a script, I'd be stripping the blank lines out of the file, and that would fail gracefully because there just wouldn't be any blank lines to remove. 🤷

[cooperlees]

Good old Windows. If nothing goes wrong here on Linux and if your fix doesn't impact Linux/MacOS (POSIX filesystems) then I am cool for the fix to come in this PR.

[flyinghyrax]

I can do a before/after check in a docker container this weekend to make certain, since if it did affect Linux I'm not sure there's an existing unit test that would catch the regression in CI.

[cooperlees]

Looks good to me - Lets fix other nits in follow up diffs!