Skip to content

CI validation: --filelist CRLF fix - #2

Closed
jaypatrickhoward wants to merge 1 commit into
devfrom
filelist-crlf
Closed

CI validation: --filelist CRLF fix#2
jaypatrickhoward wants to merge 1 commit into
devfrom
filelist-crlf

Conversation

@jaypatrickhoward

Copy link
Copy Markdown
Owner

Fork-only PR to run CI on the --filelist CRLF fix before filing upstream. Do not merge.

`--filelist` stopped accepting Windows CRLF line endings in 165e52c
("first implementation supporting Process Substitution", facebook#4349). That
rewrite reads the list into a buffer opened in binary mode and splits it
on '\n' alone. Previously the list was opened in text mode, where the
Windows CRT translated CRLF to LF before the parser saw it, so stripping
'\n' was enough. In binary mode the CR survives into every path:
zstd: can't stat a.txt : No such file or directory -- ignored
The '\r' is invisible in that message, which makes it awkward to
diagnose. Drop a trailing '\r' when the line pointers are built, and add
a playTests case that fails without the fix.
@jaypatrickhoward

Copy link
Copy Markdown
OwnerAuthor

Upstream PR filed as facebook#4766; fork CI was 103/103 green.

Sign up for freeto 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

@jaypatrickhoward