Skip to content

Commit a2518b1

Browse files
committed
Merge remote-tracking branch 'origin/gitster-rn4-clean-smudge-review'
2 parents dac3c4d + bc93d2f commit a2518b1

File tree

1 file changed

+23
-14
lines changed

1 file changed

+23
-14
lines changed

‎rev_news/drafts/edition-4.md‎

Lines changed: 23 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -112,21 +112,30 @@ It it now safe to use this service, and aspiring contributors are encouraged to
112112

113113
### Reviews
114114

115-
*[sha1_file: pass empty buffer to index empty file](http://thread.gmane.org/gmane.comp.version-control.git/269050)
116-
117-
Jim Hill posted a bugfix patch along with a test case. The bug was
118-
that a NULL pointer was passed instead of an empty string when
119-
filtering an empty file. This generated an error on stderr but `git
120-
add` succeeded anyway.
121-
122-
Junio Hamano acknowledged that it was a good fix, and that by the way
123-
the fact that `git add` succeeded despite the error was another bug.
115+
*[clean/smudge empty contents](http://thread.gmane.org/gmane.comp.version-control.git/269050)
116+
117+
Jim Hill noticed that Git issues an error message saying that copy_fd() was given a bad
118+
file descriptor when clean/smudge filters is fed an file with empty contents, found that
119+
the problem was caused because an in-memory contents that was empty was passed (by mistake)
120+
as `NULL`, instead of an empty string `""` in this codepath, but the `NULL` was used as a
121+
signal to tell Git to instead read from a given file descriptor. The fix was trivially
122+
correct and was applied.
123+
124+
The new test script, however, exhibited a flaky behaviour. Sometimes it passed, sometimes
125+
it saw EPIPE. Peff observed:
126+
127+
> Hmm, I thought we turned off SIGPIPE when writing to filters these days.
128+
> Looks like we still complain if we get EPIPE, though. I feel like it
129+
> should be the filter's business whether it wants to consume all of the
130+
> input or not[1], and we should only be checking its exit status.
131+
>
132+
> [1] As a practical example, consider a file format that has a lot of
133+
> cruft at the end. The clean filter would want to read only to the
134+
> start of the cruft, and then stop for reasons of efficiency.
124135
125-
Jim replied that he has found more than one other bug and provided
126-
more test cases. Junio reviewed the new tests along with Jeff King,
127-
who suggested that clean/smudge filters could be allowed to quit
128-
before reading their input fully. Junio then decided to
129-
[implement this suggestion](http://thread.gmane.org/gmane.comp.version-control.git/269050/focus=269383).
136+
The discussion lead to
137+
an [enhancement](http://thread.gmane.org/gmane.comp.version-control.git/269050/focus=269383)
138+
to allow clean/smudge filters to quit before reading their input fully.
130139

131140
### Support
132141

0 commit comments

Comments
(0)