darcs

Patch 1664 make readTextFile strict in the content by splitting it into lines

Title make readTextFile strict in the content by splitting it into lines
Superseder Nosy List bf
Related Issues
Status accepted Assigned To
Milestone

Created on 2018-03-18.22:02:14 by bf, last changed 2018-03-28.12:42:48 by gh.

Files
File name Status Uploaded Type Edit Remove
make-readtextfile-strict-in-the-content-by-splitting-it-into-lines.dpatch bf, 2018-03-18.22:02:13 application/x-darcs-patch
really-make-readtextfile-strict.dpatch bf, 2018-03-25.13:51:45 application/x-darcs-patch
use-darcs_util_lock_readtextfile-for-_authorspellings.dpatch bf, 2018-03-28.06:54:45 application/x-darcs-patch
See mailing list archives for discussion on individual patches.
Messages
msg19987 (view) Author: bf Date: 2018-03-18.22:02:13
1 patch for repository http://darcs.net/screened:

patch ffe1ef35050722f3716909e9033d60b15f9e3916
Author: Ben Franksen <ben.franksen@online.de>
Date:   Tue Mar 13 12:06:33 CET 2018
  * make readTextFile strict in the content by splitting it into lines
  
  We want to be able to write a changed version of the file without worrying
  about failures due to semiopen files. (I have observed occasional but hard
  to reproduce test failures with the previous lazy version.) Since text
files
  are processed line-wise anyway (in the few places where we use
  readTextFile), this is a cheap and reliable solution.
Attachments
msg20008 (view) Author: gh Date: 2018-03-23.16:47:49
The changes are correct and the code looks better, I just do not see how
splitting file contents into lines makes file reading strict?
msg20018 (view) Author: bf Date: 2018-03-24.18:03:04
Oops. Good catch. Indeed, the new readTextFile isn't any stricter than
the old version. Reasoning about strictness is... difficult.

Why did the patch seem solve the problem I observed? I have no idea.

Attached is a follow-up patch that really makes it strict this time. I
hope ;-)

1 patch for repository http://darcs.net/screened:

patch 1f3bd44d17c27deffcdb45a7b2ac353d70e24da0
Author: Ben Franksen <ben.franksen@online.de>
Date:   Sat Mar 24 18:58:18 CET 2018
  * really make readTextFile strict
Attachments
msg20025 (view) Author: bf Date: 2018-03-25.11:34:54
I should refrain from sending untested patch bundles. In the last one I
forgot to take into account that 'last' is a partial function! Oh my...
msg20030 (view) Author: bf Date: 2018-03-25.13:51:45
Okay, here's another bundle. Tested it this time.
Attachments
msg20051 (view) Author: gh Date: 2018-03-27.14:01:25
I was about to ask you why not use something like System.IO.Strict, but
then I investigated a little and found that that library uses sep +
length anyway. I'm accepting the bundle.
msg20054 (view) Author: bf Date: 2018-03-27.15:57:46
Yes, I looked at the strict package and found the same as you did...
msg20058 (view) Author: gh Date: 2018-03-27.20:23:46
Can the same function be used in Darcs.UI.Commands.ShowAuthor instead of
Ratified.readFile ?

Also I would just go ahead and nuke that Ratified module, along with
contrib/darcs-errors.hlint . The idea is to suppress hlint Errors like
"avoid hGetContents" and "avoid Prelude.readFilen", but hlint is
currently throwing 10.000 warnings, and there is probably no general
rule of which ones to silent.

Maybe it would be better to comment the places where we use these
functions and say why file contents is guaranteed to be consumed in each
case.
msg20061 (view) Author: ganesh Date: 2018-03-28.05:35:51
Please don't nuke Ratified for now, I have a pending patch where I want 
to use it.
msg20063 (view) Author: bf Date: 2018-03-28.06:54:45
> Can the same function be used in Darcs.UI.Commands.ShowAuthor instead
> of Ratified.readFile ?

Done, see attached follow-up bundle:

1 patch for repository http://darcs.net/screened:

patch fa2350acc56b76be6994487dd4c8cce1793fd5f3
Author: Ben Franksen <ben.franksen@online.de>
Date:   Wed Mar 28 08:12:59 CEST 2018
  * use Darcs.Util.Lock.readTextFile for .authorspellings
  
  Also, use catchIOError to catch IO errors when reading the file.
Attachments
msg20065 (view) Author: gh Date: 2018-03-28.12:42:48
OK good, accepted.

> Please don't nuke Ratified for now, I have a pending patch where I want
> to use it.

Alright :)
History
Date User Action Args
2018-03-18 22:02:14bfcreate
2018-03-18 22:05:26bfsetstatus: needs-screening -> needs-review
2018-03-23 16:47:50ghsetmessages: + msg20008
2018-03-24 18:03:04bfsetfiles: + really-make-readtextfile-strict.dpatch
messages: + msg20018
2018-03-25 11:34:54bfsetmessages: + msg20025
2018-03-25 13:49:56bfsetfiles: - really-make-readtextfile-strict.dpatch
2018-03-25 13:51:45bfsetfiles: + really-make-readtextfile-strict.dpatch
messages: + msg20030
2018-03-27 14:01:25ghsetstatus: needs-review -> accepted
messages: + msg20051
2018-03-27 15:57:46bfsetmessages: + msg20054
2018-03-27 20:23:46ghsetmessages: + msg20058
2018-03-28 05:35:51ganeshsetmessages: + msg20061
2018-03-28 06:54:45bfsetfiles: + use-darcs_util_lock_readtextfile-for-_authorspellings.dpatch
messages: + msg20063
2018-03-28 12:42:48ghsetmessages: + msg20065