Skip to content

[Fixed][Windows] Layers cache written with \r\r\n row endings breaks cached diffs - #22

Open
mike-shevchenko wants to merge 1 commit into
INTI-CMNB:masterfrom
mike-shevchenko:fix-csv-newline-windows
Open

mike-shevchenko wants to merge 1 commit into
INTI-CMNB:masterfrom
mike-shevchenko:fix-csv-newline-windows

Conversation

@mike-shevchenko

Copy link
Copy Markdown

Problem

On Windows, the second diff of any board fails:

File "kicad-diff.py", line 793, in load_layer_names
    layer_names, name_to_id = load_cached_layers(layers_file)
File "kicad-diff.py", line 704, in load_cached_layers
    ilnum = int(r[0])
IndexError: list index out of range

The first diff of a given board always succeeds, because it is a cache miss and the layers
are read from the PCB. Every later diff that hits the cache fails, which in practice means
any persistent --cache_dir is unusable on Windows.

Cause

csv.writer always terminates rows with \r\n. save_layers_to_cache() opened the stream
in text mode without newline='', so on Windows the \n was translated a second time and
each row landed as \r\r\n:

$ od -c layers.csv | head -2
0000000   L a y e r   I D , L a y e r   n a m e , U s e r   n a m e  \r  \r  \n

Reading that back yields a blank row between each real one, and int(r[0]) raises
IndexError. This is the case the csv docs call out - newline='' is required on both the
read and the write side.

Fix

  • newline='' on both open() calls, so rows are written and read as \r\n.
  • Skip blank rows on read. Strictly this is not needed once the writer is fixed, but without
    it every cache already written by an earlier version keeps crashing until the user finds
    and deletes it. Two lines seemed better than a migration note.

No behaviour change on Linux or macOS, where \n is not translated and the bug never
appeared.

Verification

Same two boards, same cache directory, KiCad 10.0.6 / Python 3.11 on Windows 10:

before after
first diff (cold cache) ok ok
second diff (warm cache) IndexError ok
cache written by old version IndexError ok
layers.csv row ending \r\r\n \r\n

The last row is the backward-compatibility case: I deliberately rewrote a good cache back to
\r\r\n and confirmed the patched script reads it while the unpatched one still raises.

csv.writer emits \r\n, and the stream was opened in text mode without
newline='', so on Windows the \n was translated again and every row of
layers.csv ended \r\r\n. Reading it back produced a blank row between
each real one, so load_cached_layers() raised IndexError on r[0] and the
second diff of any board failed.

Adds newline='' to both open() calls, as the csv module documents, and
skips blank rows on read so caches already written by earlier versions
keep working instead of having to be deleted.
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.

1 participant