diff --git a/doc/download.xml b/doc/download.xml
index cf8548c..ea3f4bc 100644
--- a/doc/download.xml
+++ b/doc/download.xml
@@ -69,7 +69,10 @@ The following components are supported.
and the function writes the downloaded contents to this file;
the returned record does not have a result component in this case.
- If the download fails then this file is not left behind.
+ If the download fails then a file created by the attempt is not left
+ behind. A file that was already there before the call is not removed,
+ since it is not ours to delete – but note that a failed attempt may
+ well have overwritten it, so its old contents are not guaranteed either.
verifyCert
-
diff --git a/lib/download.gi b/lib/download.gi
index 4b9a6ce..eecbda0 100644
--- a/lib/download.gi
+++ b/lib/download.gi
@@ -173,11 +173,6 @@ Add( Download_Methods, rec(
code:= Process( DirectoryCurrent(), exec, InputTextNone(), outstream, args );
CloseStream( outstream );
if code <> 0 then
- # wget may have created the target file; try to remove it
- if IsBound( opt.target ) and IsString( opt.target ) and
- IsExistingFile( opt.target ) and RemoveFile( opt.target ) <> true then
- Error( "Download cannot remove unwanted file ", opt.target );
- fi;
return rec( success:= false,
error:= Concatenation( "Process returned ", String( code ) ) );
elif not ( IsBound( opt.target ) and IsString( opt.target ) ) then
@@ -245,7 +240,7 @@ InstallMethod( Download,
InstallMethod( Download,
[ "IsString", "IsRecord" ],
function( url, opt )
- local timeout, errors, r, res;
+ local timeout, hadTarget, errors, r, res;
# Do not modify the caller's record when filling in the defaults below.
opt:= ShallowCopy( opt );
@@ -264,6 +259,11 @@ InstallMethod( Download,
fi;
fi;
+ # Whether the caller brought the target file, as opposed to a method
+ # creating it below. We remove only what we created.
+ hadTarget:= IsBound( opt.target ) and IsString( opt.target ) and
+ IsExistingFile( opt.target );
+
# Run over the methods.
errors:= [];
for r in Download_Methods do
@@ -275,9 +275,11 @@ InstallMethod( Download,
fi;
# A failed method may have left a partial or bogus target file behind.
# Remove it here, so that the guarantee holds for every method,
- # including ones added to 'Download_Methods' from outside.
- if IsBound( opt.target ) and IsString( opt.target ) and
- IsExistingFile( opt.target ) then
+ # including ones added to 'Download_Methods' from outside -- but never
+ # remove a file the caller had before the call, which is not ours to
+ # delete.
+ if not hadTarget and IsBound( opt.target ) and IsString( opt.target )
+ and IsExistingFile( opt.target ) then
RemoveFile( opt.target );
fi;
Info( InfoUtils, 2, "Download method ", r.name, " failed with\n",
diff --git a/tst/download.tst b/tst/download.tst
index 02856d3..2a18cec 100644
--- a/tst/download.tst
+++ b/tst/download.tst
@@ -164,6 +164,17 @@ true
gap> IsExistingFile( file );
false
+## A file that was already there is not removed by a failed download:
+## 'Download' only cleans up after itself.
+gap> FileString( file, "mine" );;
+gap> res1:= Download( Concatenation( baseurl, "/missing" ),
+> rec( target:= file ) );;
+gap> res1.success;
+false
+gap> IsExistingFile( file );
+true
+gap> RemoveFile( file );;
+
## test errors and redirects
gap> res1:= Download( Concatenation( baseurl, "/missing" ) );;
gap> res1.success = false;