darcs

Patch 2027 fix creation of temporary directories

Title fix creation of temporary directories
Superseder Nosy List bf
Related Issues
Status accepted-pending-tests Assigned To
Milestone

Created on 2020-06-20.08:06:42 by bf, last changed 2020-07-11.14:40:25 by ganesh.

Files
File name Status Uploaded Type Edit Remove
diff-command_-withtempdir__withdelayeddir.dpatch bf, 2020-06-24.13:32:54 application/x-darcs-patch
diff.txt ganesh, 2020-07-04.15:25:37 text/plain
fix-creation-of-temporary-directories.dpatch bf, 2020-06-20.08:06:42 application/x-darcs-patch
fix-in-diff-command.dpatch bf, 2020-07-10.11:11:59 application/x-darcs-patch
patch-preview.txt bf, 2020-06-20.08:06:42 text/x-darcs-patch
patch-preview.txt bf, 2020-06-24.13:32:53 text/x-darcs-patch
patch-preview.txt bf, 2020-07-10.11:11:59 text/x-darcs-patch
unnamed bf, 2020-06-20.08:06:42 text/plain
unnamed bf, 2020-06-24.13:32:54 text/plain
unnamed bf, 2020-07-10.11:11:59 text/plain
See mailing list archives for discussion on individual patches.
Messages
msg22039 (view) Author: bf Date: 2020-06-20.08:06:42
1 patch for repository http://darcs.net/screened:

patch 31f2953b33a4a713544c22eac8c09b3446f9e91a
Author: Ben Franksen <ben.franksen@online.de>
Date:   Mon May 18 10:58:02 CEST 2020
  * fix creation of temporary directories
  
  The algorithm we used to generate a unique name is prone to race conditions
  and I occasionally got random test failures because of this. The new
  implementation uses the temporary package.
Attachments
msg22068 (view) Author: ganesh Date: 2020-06-20.22:26:30
Looks like a good simplification
msg22116 (view) Author: ganesh Date: 2020-06-24.00:12:08
Unfortunately this is causing a test failure on Windows in diff.sh:

| diff: darcs-diff-f5276236205c89d1\old-temp1: No such file or directory
| diff: darcs-diff-f5276236205c89d1\new-temp1: No such file or directory

I haven't investigated the details at all yet.
msg22117 (view) Author: bf Date: 2020-06-24.13:32:54
I think this change should fix the problem you noticed on Windows. I am
pretty sure this is not Windows specific per se, rather I suspect that
timing differences have uncovered a bug here.

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

patch 97dfa3830cac2b55f8745b9c35321fd512c73762
Author: Ben Franksen <ben.franksen@online.de>
Date:   Wed Jun 24 15:36:07 CEST 2020
  * diff command: withTempDir->withDelayedDir
Attachments
msg22118 (view) Author: bf Date: 2020-06-24.13:52:09
As processes that run darcs commands are supposed to be short-lived, 
perhaps we should make withDelayedDir the standard way to handle 
temporary directories. The problem here is that we usually read hashed 
files lazily, which makes it pretty hard to reason about when it is 
safe to finally remove the directory and its content.
msg22126 (view) Author: bf Date: 2020-07-03.16:19:56
I have pushed the (hopefully) fix for your test failure to screened.
msg22129 (view) Author: ganesh Date: 2020-07-04.15:25:37
Unfortunately it's still failing (I double-checked I have the
"diff command: withTempDir->withDelayedDir" patch) - full output
attached.
Attachments
msg22132 (view) Author: bf Date: 2020-07-04.20:57:29
> Unfortunately it's still failing (I double-checked I have the
> "diff command: withTempDir->withDelayedDir" patch) - full output
> attached.

Hmm. The output says

| diff: darcs-diff-e7b8195ee68f0c35\old-temp1: No such file or directory

Note the Windows-like '\' path separator here. Could it be that this is
a similar problem to one we had previously where a native Windows
version of an external command is used instead of the cygwin one?
msg22137 (view) Author: bf Date: 2020-07-07.10:35:59
Can you add --debug to the first 'darcs diff' command in the test and 
place an 'exit 1' after that, then attach the output?
msg22142 (view) Author: bf Date: 2020-07-10.11:12:00
Hi Ganesh, I think this patch should fix the problem on Windows.

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

patch 5a047f6168e6c682bbf49609d9bd5d6aa0c77da8
Author: Ben Franksen <ben.franksen@online.de>
Date:   Fri Jul 10 13:11:04 CEST 2020
  * fix in diff command
  
  We must not use the native System.FilePath's </> operator and then call
  System.FilePath.Posix.takeFileName on the result.
Attachments
msg22143 (view) Author: ganesh Date: 2020-07-11.14:40:24
That works. Thanks a lot for fixing this even when I have been so
unresponsive about debugging it.
History
Date User Action Args
2020-06-20 08:06:42bfcreate
2020-06-20 08:06:58bfsetstatus: needs-screening -> needs-review
2020-06-20 22:26:31ganeshsetstatus: needs-review -> accepted-pending-tests
messages: + msg22068
2020-06-24 00:12:08ganeshsetstatus: accepted-pending-tests -> review-in-progress
messages: + msg22116
2020-06-24 13:32:55bfsetfiles: + patch-preview.txt, diff-command_-withtempdir__withdelayeddir.dpatch, unnamed
messages: + msg22117
2020-06-24 13:52:10bfsetmessages: + msg22118
2020-07-03 16:19:56bfsetmessages: + msg22126
2020-07-04 15:25:37ganeshsetfiles: + diff.txt
messages: + msg22129
2020-07-04 20:57:29bfsetmessages: + msg22132
2020-07-07 10:35:59bfsetmessages: + msg22137
2020-07-10 11:12:01bfsetfiles: + patch-preview.txt, fix-in-diff-command.dpatch, unnamed
messages: + msg22142
2020-07-11 14:40:25ganeshsetstatus: review-in-progress -> accepted-pending-tests
messages: + msg22143