Repository: conan-io/conan

Issue:
Memory error when download very big packages
`conan install node/6.1.0@silkedit/stable -s compiler="Visual Studio" -s compiler.version=14`

Downloader.download method keeps in memory too much data.


```
DEBUG :uploader_downloader.py[74]: <type 'exceptions.MemoryError'> [2016-09-23 15:15:02,983]
DEBUG :uploader_downloader.py[75]: Traceback (most recent call last):
                                     File "c:\python27\lib\site-packages\conans\client\rest\uploader_downloader.py", line 62, in download
                                       ret.extend(data)
                                   MemoryError
```

Discussion:

Patch:
diff --git a/conans/client/deps_builder.py b/conans/client/deps_builder.py
index 341d1c41a..83c9c8821 100644
--- a/conans/client/deps_builder.py
+++ b/conans/client/deps_builder.py
@@ -416,7 +416,7 @@ class DepsBuilder(object):
     def _create_new_node(self, current_node, dep_graph, requirement, public_deps, name_req):
         """ creates and adds a new node to the dependency graph
         """
-        conanfile_path = self._retriever.get_conanfile(requirement.conan_reference)
+        conanfile_path = self._retriever.get_recipe(requirement.conan_reference)
         output = ScopedOutput(str(requirement.conan_reference), self._output)
         dep_conanfile = self._loader.load_conan(conanfile_path, output)
         if dep_conanfile:
diff --git a/conans/client/proxy.py b/conans/client/proxy.py
index 57ee121b9..cdecc8855 100644
--- a/conans/client/proxy.py
+++ b/conans/client/proxy.py
@@ -70,17 +70,17 @@ class ConanProxy(object):
             remote = self._registry.get_ref(package_reference.conan)
             self._manifest_manager.check_package(package_reference, remote)
 
-    def get_conanfile(self, conan_reference):
+    def get_recipe(self, conan_reference):
         output = ScopedOutput(str(conan_reference), self._out)
 
         def _refresh():
-            conan_dir_path = self._client_cache.export(conan_reference)
-            rmdir(conan_dir_path)
+            export_path = self._client_cache.export(conan_reference)
+            rmdir(export_path)
             # It might need to remove shortpath
             rmdir(self._client_cache.source(conan_reference), True)
             current_remote, _ = self._get_remote(conan_reference)
             output.info("Retrieving from remote '%s'..." % current_remote.name)
-            self._remote_manager.get_conanfile(conan_reference, current_remote)
+            self._remote_manager.get_recipe(conan_reference, export_path, current_remote)
             if self._update:
                 output.info("Updated!")
             else:
@@ -88,7 +88,6 @@ class ConanProxy(object):
 
         # check if it is in disk
         conanfile_path = self._client_cache.conanfile(conan_reference)
-
         path_exist = path_exists(conanfile_path, self._client_cache.store)
 
         if path_exist:
@@ -122,7 +121,7 @@ class ConanProxy(object):
                                          "to replace it." % (remote.name, conan_reference))
 
         else:
-            self._retrieve_conanfile(conan_reference, output)
+            self._retrieve_recipe(conan_reference, output)
 
         if self._manifest_manager:
             remote = self._registry.get_ref(conan_reference)
@@ -146,13 +145,14 @@ class ConanProxy(object):
 
         return 0
 
-    def _retrieve_conanfile(self, conan_reference, output):
+    def _retrieve_recipe(self, conan_reference, output):
         """ returns the requested conanfile object, retrieving it from
         remotes if necessary. Can raise NotFoundException
         """
         def _retrieve_from_remote(remote):
             output.info("Trying with '%s'..." % remote.name)
-            result = self._remote_manager.get_conanfile(conan_reference, remote)
+            export_path = self._client_cache.export(conan_reference)
+            result = self._remote_manager.get_recipe(conan_reference, export_path, remote)
             self._registry.set_ref(conan_reference, remote)
             return result
 
@@ -261,7 +261,8 @@ class ConanProxy(object):
     def download_packages(self, reference, package_ids):
         assert(isinstance(package_ids, list))
         remote, _ = self._get_remote(reference)
-        self._remote_manager.get_conanfile(reference, remote)
+        export_path = self._client_cache.export(reference)
+        self._remote_manager.get_recipe(reference, export_path, remote)
         self._registry.set_ref(reference, remote)
         output = ScopedOutput(str(reference), self._out)
         for package_id in package_ids:
@@ -280,7 +281,8 @@ class ConanProxy(object):
         try:
             output.info("Looking for package %s in remote '%s' " % (package_id, remote.name))
             # Will raise if not found NotFoundException
-            self._remote_manager.get_package(package_reference, remote)
+            package_path = self._client_cache.package(package_reference)
+            self._remote_manager.get_package(package_reference, package_path, remote)
             output.success('Package installed %s' % package_id)
             return True
         except ConanConnectionError:
diff --git a/conans/client/remote_manager.py b/conans/client/remote_manager.py
index 819cfa96c..1d716f8cb 100644
--- a/conans/client/remote_manager.py
+++ b/conans/client/remote_manager.py
@@ -1,16 +1,17 @@
-from conans.errors import ConanException, ConanConnectionError
+import os
+import shutil
+import tarfile
+import time
+import traceback
+
 from requests.exceptions import ConnectionError
-from conans.util.files import save, tar_extract, rmdir
+
+from conans.errors import ConanException, ConanConnectionError
+from conans.util.files import tar_extract, rmdir, relative_dirs, mkdir
 from conans.util.log import logger
-import traceback
-import os
 from conans.paths import PACKAGE_TGZ_NAME, CONANINFO, CONAN_MANIFEST, CONANFILE, EXPORT_TGZ_NAME
-from io import BytesIO
-import tarfile
 from conans.util.files import gzopen_without_timestamps
 from conans.util.files import touch
-import shutil
-import time
 
 
 class RemoteManager(object):
@@ -85,35 +86,35 @@ class RemoteManager(object):
         returns (ConanDigest, remote_name)"""
         return self._call_remote(remote, "get_package_digest", package_reference)
 
-    def get_conanfile(self, conan_reference, remote):
+    def get_recipe(self, conan_reference, dest_folder, remote):
         """
         Read the conans from remotes
         Will iterate the remotes to find the conans unless remote was specified
 
-        returns (dict relative_filepath:content , remote_name)"""
-        export_files = self._call_remote(remote, "get_conanfile", conan_reference)
-        export_folder = self._client_cache.export(conan_reference)
-        uncompress_files(export_files, export_folder, EXPORT_TGZ_NAME)
+        returns (dict relative_filepath:abs_path , remote_name)"""
+        zipped_files = self._call_remote(remote, "get_recipe", conan_reference, dest_folder)
+        files = unzip_and_get_files(zipped_files, dest_folder, EXPORT_TGZ_NAME)
         # Make sure that the source dir is deleted
         rmdir(self._client_cache.source(conan_reference), True)
 #       TODO: Download only the CONANFILE file and only download the rest of files
 #       in install if needed (not found remote package)
+        return files
 
-    def get_package(self, package_reference, remote):
+    def get_package(self, package_reference, dest_folder, remote):
         """
         Read the conans package from remotes
         Will iterate the remotes to find the conans unless remote was specified
 
-        returns (dict relative_filepath:content , remote_name)"""
-        package_files = self._call_remote(remote, "get_package", package_reference)
-        destination_dir = self._client_cache.package(package_reference)
-        uncompress_files(package_files, destination_dir, PACKAGE_TGZ_NAME)
-
+        returns (dict relative_filepath:abs_path , remote_name)"""
+        zipped_files = self._call_remote(remote, "get_package", package_reference, dest_folder)
+        files = unzip_and_get_files(zipped_files, dest_folder, PACKAGE_TGZ_NAME)
         # Issue #214 https://github.com/conan-io/conan/issues/214
-        for dirname, _, files in os.walk(destination_dir):
+        for dirname, _, files in os.walk(dest_folder):
             for fname in files:
                 touch(os.path.join(dirname, fname))
 
+        return files
+
     def search(self, remote, pattern=None, ignorecase=True):
         """
         Search exported conans information from remotes
@@ -211,20 +212,27 @@ def compress_files(files, name, excluded, dest_dir):
     return ret
 
 
-def uncompress_files(files, folder, name):
+def unzip_and_get_files(files, destination_dir, tgz_name):
+    '''Moves all files from package_files, {relative_name: tmp_abs_path}
+    to destination_dir, unzipping the "tgz_name" if found'''
+
+    tgz_file = files.pop(tgz_name, None)
+    if tgz_file:
+        uncompress_file(tgz_file, destination_dir)
+
+    return relative_dirs(destination_dir)
+
+
+def uncompress_file(src_path, dest_folder):
     try:
-        for file_name, content in files:
-            if os.path.basename(file_name) == name:
-                #  Unzip the file and not keep the tgz
-                tar_extract(BytesIO(content), folder)
-            else:
-                save(os.path.join(folder, file_name), content)
+        with open(src_path, 'rb') as file_handler:
+            tar_extract(file_handler, dest_folder)
     except Exception as e:
-        error_msg = "Error while downloading/extracting files to %s\n%s\n" % (folder, str(e))
+        error_msg = "Error while downloading/extracting files to %s\n%s\n" % (dest_folder, str(e))
         # try to remove the files
         try:
-            if os.path.exists(folder):
-                shutil.rmtree(folder)
+            if os.path.exists(dest_folder):
+                shutil.rmtree(dest_folder)
                 error_msg += "Folder removed"
         except Exception as e:
             error_msg += "Folder not removed, files/package might be damaged, remove manually"
diff --git a/conans/client/rest/auth_manager.py b/conans/client/rest/auth_manager.py
index 966a2773d..24cb1f43a 100644
--- a/conans/client/rest/auth_manager.py
+++ b/conans/client/rest/auth_manager.py
@@ -142,12 +142,12 @@ class ConanApiAuthManager(object):
         return self._rest_client.get_package_digest(package_reference)
 
     @input_credentials_if_unauthorized
-    def get_conanfile(self, conan_reference):
-        return self._rest_client.get_conanfile(conan_reference)
+    def get_recipe(self, conan_reference, dest_folder):
+        return self._rest_client.get_recipe(conan_reference, dest_folder)
 
     @input_credentials_if_unauthorized
-    def get_package(self, package_reference):
-        return self._rest_client.get_package(package_reference)
+    def get_package(self, package_reference, dest_folder):
+        return self._rest_client.get_package(package_reference, dest_folder)
 
     @input_credentials_if_unauthorized
     def search(self, pattern, ignorecase):
diff --git a/conans/client/rest/rest_client.py b/conans/client/rest/rest_client.py
index 68e28d8e7..fcf6affe2 100644
--- a/conans/client/rest/rest_client.py
+++ b/conans/client/rest/rest_client.py
@@ -12,6 +12,7 @@ from conans.model.manifest import FileTreeManifest
 from conans.client.rest.uploader_downloader import Uploader, Downloader
 from conans.model.ref import ConanFileReference
 from six.moves.urllib.parse import urlsplit, parse_qs
+import tempfile
 
 
 def handle_return_deserializer(deserializer=None):
@@ -107,7 +108,7 @@ class RestApiClient(object):
         contents = {key: decode_text(value) for key, value in dict(contents).items()}
         return FileTreeManifest.loads(contents[CONAN_MANIFEST])
 
-    def get_conanfile(self, conan_reference):
+    def get_recipe(self, conan_reference, dest_folder):
         """Gets a dict of filename:contents from conans"""
         # Get the conanfile snapshot first
         url = "%s/conans/%s/download_urls" % (self._remote_api_url, "/".join(conan_reference))
@@ -117,12 +118,10 @@ class RestApiClient(object):
             raise NotFoundException("Conan '%s' doesn't have a %s!" % (conan_reference, CONANFILE))
 
         # TODO: Get fist an snapshot and compare files and download only required?
+        file_paths = self.download_files_to_folder(urls, dest_folder, self._output)
+        return file_paths
 
-        # Download the resources
-        contents = self.download_files(urls, self._output)
-        return contents
-
-    def get_package(self, package_reference):
+    def get_package(self, package_reference, dest_folder):
         """Gets a dict of filename:contents from package"""
         url = "%s/conans/%s/packages/%s/download_urls" % (self._remote_api_url,
                                                           "/".join(package_reference.conan),
@@ -133,8 +132,8 @@ class RestApiClient(object):
         # TODO: Get fist an snapshot and compare files and download only required?
 
         # Download the resources
-        contents = self.download_files(urls, self._output)
-        return contents
+        file_paths = self.download_files_to_folder(urls, dest_folder, self._output)
+        return file_paths
 
     def upload_conan(self, conan_reference, the_files):
         """
@@ -361,6 +360,25 @@ class RestApiClient(object):
                 output.writeln("")
             yield os.path.normpath(filename), contents
 
+    def download_files_to_folder(self, file_urls, to_folder, output=None):
+        """
+        :param: file_urls is a dict with {filename: abs_path}
+
+        It writes downloaded files to disk (appending to file, only keeps chunks in memory)
+        """
+        downloader = Downloader(self.requester, output, self.VERIFY_SSL)
+        ret = {}
+        for filename, resource_url in file_urls.items():
+            if output:
+                output.writeln("Downloading %s" % filename)
+            auth, _ = self._file_server_capabilities(resource_url)
+            abs_path = os.path.join(to_folder, filename)
+            downloader.download(resource_url, abs_path, auth=auth)
+            if output:
+                output.writeln("")
+            ret[filename] = abs_path
+        return ret
+
     def upload_files(self, file_urls, files, output):
         t1 = time.time()
         failed = {}
diff --git a/conans/client/rest/uploader_downloader.py b/conans/client/rest/uploader_downloader.py
index eec4d8e84..475dfb450 100644
--- a/conans/client/rest/uploader_downloader.py
+++ b/conans/client/rest/uploader_downloader.py
@@ -125,8 +125,10 @@ class Downloader(object):
                         if self.output:
                             print_progress(self.output, units)
                         last_progress = units
-
-            return bytes(ret)
+            if not file_path:
+                return bytes(ret)
+            else:
+                return
         except Exception as e:
             logger.debug(e.__class__)
             logger.debug(traceback.format_exc())
diff --git a/conans/server/store/file_manager.py b/conans/server/store/file_manager.py
index a7f95ab65..6b6d4295b 100644
--- a/conans/server/store/file_manager.py
+++ b/conans/server/store/file_manager.py
@@ -17,7 +17,7 @@ class FileManager(object):
         self._storage_adapter = storage_adapter
 
     # ############ SNAPSHOTS
-    def get_conanfile(self, conan_reference):
+    def get_recipe(self, conan_reference):
         conanfile_path = self.paths.conanfile(conan_reference)
         return self._storage_adapter.get_file(conanfile_path)
 


Test patch:
diff --git a/conans/test/download_test.py b/conans/test/download_test.py
index e1ed5a986..e41c6e29a 100644
--- a/conans/test/download_test.py
+++ b/conans/test/download_test.py
@@ -69,7 +69,7 @@ class DownloadTest(unittest.TestCase):
                                      client2.remote_manager,
                                      "default")
 
-        installer.get_conanfile(conan_ref)
+        installer.get_recipe(conan_ref)
         installer.get_package(package_ref, force_build=False)
 
         reg_path = client2.paths.export(ConanFileReference.loads("Hello/1.2.1/frodo/stable"))
diff --git a/conans/test/model/order_libs_test.py b/conans/test/model/order_libs_test.py
index 5e70ef0cc..99dfa93aa 100644
--- a/conans/test/model/order_libs_test.py
+++ b/conans/test/model/order_libs_test.py
@@ -48,7 +48,7 @@ class Retriever(object):
         content = base_content % (name, self._reqs(requires), name, self._libs(name))
         save(conan_path, content)
 
-    def get_conanfile(self, conan_ref):
+    def get_recipe(self, conan_ref):
         conan_path = os.path.join(self.folder, "/".join(conan_ref), CONANFILE)
         return conan_path
 
diff --git a/conans/test/model/transitive_reqs_test.py b/conans/test/model/transitive_reqs_test.py
index 441b7f8f5..a8ec49e2e 100644
--- a/conans/test/model/transitive_reqs_test.py
+++ b/conans/test/model/transitive_reqs_test.py
@@ -36,7 +36,7 @@ class Retriever(object):
         conan_path = os.path.join(self.folder, "/".join(conan_ref), CONANFILE)
         save(conan_path, content)
 
-    def get_conanfile(self, conan_ref):
+    def get_recipe(self, conan_ref):
         conan_path = os.path.join(self.folder, "/".join(conan_ref), CONANFILE)
         return conan_path
 
diff --git a/conans/test/remote_manager_test.py b/conans/test/remote_manager_test.py
index 8e02eaf41..3362245b2 100644
--- a/conans/test/remote_manager_test.py
+++ b/conans/test/remote_manager_test.py
@@ -1,17 +1,20 @@
+import os
+import tempfile
 import unittest
-from conans.client.remote_manager import RemoteManager
+
 from mock import Mock
+
+from conans.client.client_cache import ClientCache
+from conans.client.remote_manager import RemoteManager
+from conans.client.remote_registry import Remote
 from conans.errors import NotFoundException
 from conans.model.ref import ConanFileReference, PackageReference
+from conans.model.manifest import FileTreeManifest
+from conans.paths import CONANFILE, CONAN_MANIFEST, CONANINFO
 from conans.test.tools import TestBufferConanOutput, TestClient
 from conans.test.utils.test_files import temp_folder
 from conans.test.utils.cpp_test_files import cpp_hello_conan_files
-from conans.client.remote_registry import Remote
-from conans.client.client_cache import ClientCache
 from conans.util.files import save
-from conans.paths import CONANFILE, CONAN_MANIFEST, CONANINFO
-import os
-from conans.model.manifest import FileTreeManifest
 
 
 class MockRemoteClient(object):
@@ -19,8 +22,13 @@ class MockRemoteClient(object):
     def __init__(self):
         self.upload_package = Mock()
         self.get_conan_digest = Mock()
-        self.get_conanfile = Mock(return_value=[("one.txt", "ONE")])
-        self.get_package = Mock(return_value=[("one.txt", "ONE")])
+        tmp_folder = tempfile.mkdtemp(suffix='conan_download')
+        save(os.path.join(tmp_folder, "one.txt"), "ONE")
+        self.get_recipe = Mock(return_value={"one.txt": os.path.join(tmp_folder, "one.txt")})
+
+        tmp_folder = tempfile.mkdtemp(suffix='conan_download')
+        save(os.path.join(tmp_folder, "one.txt"), "ONE")
+        self.get_package = Mock(return_value={"one.txt":  os.path.join(tmp_folder, "one.txt")})
         self.remote_url = None
 
         self.raise_count = 0
@@ -78,10 +86,10 @@ class RemoteManagerTest(unittest.TestCase):
         self.manager.get_conan_digest(self.conan_reference, Remote("other", "url"))
         self.assertTrue(self.remote_client.get_conan_digest.called)
 
-        self.assertFalse(self.remote_client.get_conanfile.called)
-        self.manager.get_conanfile(self.conan_reference, Remote("other", "url"))
-        self.assertTrue(self.remote_client.get_conanfile.called)
+        self.assertFalse(self.remote_client.get_recipe.called)
+        self.manager.get_recipe(self.conan_reference, temp_folder(), Remote("other", "url"))
+        self.assertTrue(self.remote_client.get_recipe.called)
 
         self.assertFalse(self.remote_client.get_package.called)
-        self.manager.get_package(self.package_reference, Remote("other", "url"))
+        self.manager.get_package(self.package_reference, temp_folder(), Remote("other", "url"))
         self.assertTrue(self.remote_client.get_package.called)