Skip to content

Commit

Permalink
Fix remote download
Browse files Browse the repository at this point in the history
  • Loading branch information
@forki
forki committed Apr 12, 2016
1 parent bc73939 commit 1c2250e
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 4 deletions.
3 changes: 3 additions & 0 deletions RELEASE_NOTES.md
View file
@@ -1,3 +1,6 @@
#### 2.58.16 - 12.04.2016
* BUGFIX: Create folder for all source file dependencies

#### 2.58.15 - 12.04.2016 #### 2.58.15 - 12.04.2016
* BUGFIX: Remove process should remove packages from specified groups - https://github.com/fsprojects/Paket/issues/1596 * BUGFIX: Remove process should remove packages from specified groups - https://github.com/fsprojects/Paket/issues/1596


Expand Down
9 changes: 5 additions & 4 deletions src/Paket.Core/RemoteDownload.fs
View file
Expand Up @@ -128,7 +128,11 @@ let rec DirectoryCopy(sourceDirName, destDirName, copySubDirs) =
DirectoryCopy(subdir.FullName, Path.Combine(destDirName, subdir.Name), copySubDirs) DirectoryCopy(subdir.FullName, Path.Combine(destDirName, subdir.Name), copySubDirs)


/// Gets a single file from github. /// Gets a single file from github.
let downloadRemoteFiles(remoteFile:ResolvedSourceFile,destination) = async { let downloadRemoteFiles(remoteFile:ResolvedSourceFile,destination) = async {
let targetFolder = FileInfo(destination).Directory
if not targetFolder.Exists then
targetFolder.Create()

match remoteFile.Origin, remoteFile.Name with match remoteFile.Origin, remoteFile.Name with
| SingleSourceFileOrigin.GistLink, Constants.FullProjectSourceFileName -> | SingleSourceFileOrigin.GistLink, Constants.FullProjectSourceFileName ->
let fi = FileInfo(destination) let fi = FileInfo(destination)
Expand Down Expand Up @@ -172,9 +176,6 @@ let downloadRemoteFiles(remoteFile:ResolvedSourceFile,destination) = async {
| SingleSourceFileOrigin.HttpLink(origin), _ -> | SingleSourceFileOrigin.HttpLink(origin), _ ->
let url = origin + remoteFile.Commit let url = origin + remoteFile.Commit
let authentication = auth remoteFile.AuthKey url let authentication = auth remoteFile.AuthKey url
let targetFolder = FileInfo(destination).Directory
if not targetFolder.Exists then
targetFolder.Create()
match Path.GetExtension(destination).ToLowerInvariant() with match Path.GetExtension(destination).ToLowerInvariant() with
| ".zip" -> | ".zip" ->
do! downloadFromUrl(authentication, url) destination do! downloadFromUrl(authentication, url) destination
Expand Down

6 comments on commit 1c2250e

@haraldsteinlechner
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@forki i'm a little confused by your recent commits. 65e554e broke github downloads. After that there seems to be some commits fixing the issue for specific cases (including this one).
So the question is: should ClearDir recreate the directory or not. If the current implementation is specification we need to change the comment (at least it confused me when tracking down broken github download)

@forki
Copy link
Member Author

@forki forki commented on 1c2250e Apr 12, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought it's not needed to create the dir if it already exists. What did I broke in CleanDir!?

@haraldsteinlechner
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think RemoteDownload.fs 162-164 cleans the directory and immediately afterwards downloads a zip file into the cleaned dir. in that case downloadFromUrl fails with error message:
Message: One or more errors occurred.
Details: An exception occurred during a WebClient request.
The inner.inner exception unveils directorynotfound exn.

@forki
Copy link
Member Author

@forki forki commented on 1c2250e Apr 12, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah I found the issue. thanks

@forki
Copy link
Member Author

@forki forki commented on 1c2250e Apr 12, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about 3089df6?

@haraldsteinlechner
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

works.

Please sign in to comment.