Skip to content

Conversation

@vszakats
Copy link
Member

@vszakats vszakats commented Nov 7, 2025

Data files no longer depend on mixed newline styles. Before this
patch the harness still assumed data files to use LF newlines,
ensured by .gitattribute and distributing sources with LF newlines.

To allow using platform native newlines (CRLF on Windows typically),
update the test harness to support data files with any newline style
on disk. And delete .gitattributes.

Fix to:

  • load original data files (from test/data) so that their newline-style
    doesn't matter on the checked out source repo, meaning it works
    when its CRLF on Windows, just like any other file.
    (if a BOM slips in, it's caught by spacecheck.pl as binary content.)
  • do the same in util.py used by smbserver.py (for test 1451).
  • also fix util.py to use us-ascii encoding for data files, replacing utf-8.

Also:

  • runtests: rework the stray CR checker to allow full CRLF data files,
    and keep warning for mixed newlines.

Follow-up to 904e7ec #19347


--- log/5/check-expected
+++ log/5/check-generated
@@ -1,8 +1,7 @@
-HTTP/1.1 200 OK[LF]
-Date: Tue, 09 Nov 2010 14:49:00 GMT[LF]
-Content-Length: 9[LF]
-Connection: close[LF]
-Content-Type: text/plain[LF]
-[LF]
-testdata[LF]
-4
+HTTP/1.1 200 OK[CR][LF]
+Date: Tue, 09 Nov 2010 14:49:00 GMT[CR][LF]
+Content-Length: 9[CR][LF]
+Connection: close[CR][LF]
+Content-Type: text/plain[CR][LF]
+[CR][LF]
+testdata[CR]4

Ref: https://github.com/curl/curl/actions/runs/19209453941/job/54909482696?pr=19398#step:13:3433

@vszakats vszakats marked this pull request as draft November 7, 2025 16:15
@vszakats
Copy link
Member Author

vszakats commented Nov 7, 2025

Oh noes, we need to enforce LF on Windows for git checkouts.

@vszakats vszakats changed the title tests/data: drop .gitattributes tests/data: attempt to drop .gitattributes Nov 7, 2025
@testclutch

This comment was marked as resolved.

@vszakats vszakats force-pushed the tattr branch 2 times, most recently from 59e9b60 to 5ed2b2d Compare November 9, 2025 17:40
vszakats added a commit to vszakats/curl that referenced this pull request Nov 9, 2025
vszakats added a commit that referenced this pull request Nov 9, 2025
Before this patch servers were loading the original data source file
(from `tests/data/test*`) if they failed to open the preprocessed data
file.

It was causing issues in many (most?) tests, because original data files
are not preprocessed, thus may be incomplete and/or come with wrong
newline characters. It's also causing difficult to diagnose issues when
a test accidentally references another test's data, which by chance
makes the test pass initially, until either that or the executed test
data gets an update, and breaking it, as seen in #19329.

Historically, the fallback existed first, then the preprocessed copy.
The fallback is no longer used by tests (except by stray accidents).

Fix it by dropping the fallback logic and relying on the preprocessed
data file saved there by the runtests framework.

Also fix two remaining test data cross-references:
- test1565: reference own server input data instead of test1's.
- test3014: reference own server input data instead of test1439's.
  Ref: #19398

Follow-up to aaf9522 #19329

Closes #19429
@vszakats vszakats marked this pull request as ready for review November 9, 2025 20:22
@vszakats vszakats changed the title tests/data: attempt to drop .gitattributes tests/data: drop .gitattributes, let it use native newlines on disk Nov 9, 2025
@vszakats vszakats changed the title tests/data: drop .gitattributes, let it use native newlines on disk tests/data: drop .gitattributes, support using native newlines on disk Nov 9, 2025
@vszakats vszakats changed the title tests/data: drop .gitattributes, support using native newlines on disk tests/data: support using native newlines on disk, drop .gitattributes Nov 9, 2025
No longer necessary after switching to LF-only newlines for data files.

Follow-up to 904e7ec curl#19347
Also replace the text number with the only test that's actually
read by this code. (via smbserver.py)

Tested with:
```
. ../.venv/bin/activate && TFLAGS='-d -v 1451' ninja tests
```
on Linux:
```
--- log/4/check-expected	2025-11-09 14:44:45.020166727 +0000
+++ log/4/check-generated	2025-11-09 14:44:45.020166727 +0000
@@ -1,8 +1,8 @@
-HTTP/1.1 200 OK[CR][LF]
-Date: Tue, 09 Nov 2010 14:49:00 GMT[CR][LF]
-Content-Length: 9[CR][LF]
-Connection: close[CR][LF]
-Content-Type: text/plain[CR][LF]
-[CR][LF]
+HTTP/1.1 200 OK[LF]
+Date: Tue, 09 Nov 2010 14:49:00 GMT[LF]
+Content-Length: 9[LF]
+Connection: close[LF]
+Content-Type: text/plain[LF]
+[LF]
 testdata[LF]
 4
```
@vszakats vszakats closed this in f477f3e Nov 10, 2025
@vszakats vszakats deleted the tattr branch November 10, 2025 13:22
vszakats added a commit to vszakats/curl that referenced this pull request Nov 13, 2025
vszakats added a commit that referenced this pull request Nov 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

2 participants