Repository: conan-io/conan
Pull request: raise error instead of skipping files if short_paths
Changelog:  Bugfix: Raise an error if source files cannot be correctly copied to build folder because of long paths in Windows.
Docs: omit

Close #4484


Problem statement:
Conan should throw an exception instead of a warning when it can't copy a source file because the path is too long on Windows
I just spent a lot of time debugging an issue happening only on Windows. Basically, the build would fail to link when building with conan. But it worked when I built it manually. I eventually figured out that conan was skipping files while copying the sources to the build directory warning me like this:

WARN: Filename too long, file excluded:

I believe this should throw an error instead of a warning. I can't see a case where omitting source files would lead to good results. Enabling short_paths fixed the issue in the end. But I think conan should have stopped executing instead of moving on knowing full well it skipped some source files.

Hints from issue discussion:
Interesting thing: this issue was implemented this way, long, long time ago, and the offending package was failing to unzip some .html of the documentation. It was totally ok to ignore them.

I think it might be changed, but for that it should be considered a bug, because otherwise it might be breaking, and we cannot break existing users (unless until 2.0). Lets discuss it, thanks very much for raising this issue.
Yes, this is totally a bug, it doesn't make sense that a unzip operation will be discarding files.

Commit message: raise error instead of skipping files if short_paths

Patch to review:
diff --git a/conans/client/installer.py b/conans/client/installer.py
index 5a81447dfbd..8c1b2511c23 100644
--- a/conans/client/installer.py
+++ b/conans/client/installer.py
@@ -1,5 +1,4 @@
 import os
-import platform
 import shutil
 import time
 
@@ -90,13 +89,12 @@ def prepare_build(self):
             mkdir(self.build_folder)
             self._conan_file.source_folder = self.source_folder
         else:
-            if platform.system() == "Windows" and os.getenv("CONAN_USER_HOME_SHORT") != "None":
-                from conans.util.windows import ignore_long_path_files
-                ignore = ignore_long_path_files(self.source_folder, self.build_folder, self._out)
-            else:
-                ignore = None
-
-            shutil.copytree(self.source_folder, self.build_folder, symlinks=True, ignore=ignore)
+            try:
+                shutil.copytree(self.source_folder, self.build_folder, symlinks=True)
+            except Exception as e:
+                msg = str(e)
+                msg += "\nConsider using short_paths=True if paths too long" if "206" in msg else ""
+                raise ConanException("%s\nError copying sources to build folder" % msg)
             logger.debug("BUILD: Copied to %s", self.build_folder)
             logger.debug("BUILD: Files copied %s", ",".join(os.listdir(self.build_folder)))
             self._conan_file.source_folder = self.build_folder
diff --git a/conans/client/tools/files.py b/conans/client/tools/files.py
index bfb9680013b..40495f11484 100644
--- a/conans/client/tools/files.py
+++ b/conans/client/tools/files.py
@@ -262,7 +262,8 @@ def normalized_text(text):
     normalized_search = normalized_text(search)
     index = normalized_content.find(normalized_search)
     if index == -1:
-        return _manage_text_not_found(search, file_path, strict, "replace_path_in_file", output=output)
+        return _manage_text_not_found(search, file_path, strict, "replace_path_in_file",
+                                      output=output)
 
     while index != -1:
         content = content[:index] + replace + content[index + len(search):]
@@ -334,7 +335,8 @@ def verify(filepath):
         return None
 
     def _get_possible_filenames(filename):
-        extensions_win = os.getenv("PATHEXT", ".COM;.EXE;.BAT;.CMD").split(";") if "." not in filename else []
+        extensions_win = (os.getenv("PATHEXT", ".COM;.EXE;.BAT;.CMD").split(";")
+                          if "." not in filename else [])
         extensions = [".sh"] if platform.system() != "Windows" else extensions_win
         extensions.insert(1, "")  # No extension
         return ["%s%s" % (filename, entry.lower()) for entry in extensions]
diff --git a/conans/test/functional/old/path_limit_test.py b/conans/test/functional/old/path_limit_test.py
index 51b0184a154..7449d640a68 100644
--- a/conans/test/functional/old/path_limit_test.py
+++ b/conans/test/functional/old/path_limit_test.py
@@ -8,6 +8,7 @@
 from conans.test import CONAN_TEST_FOLDER
 from conans.test.utils.tools import NO_SETTINGS_PACKAGE_ID, TestClient, TestServer
 from conans.util.files import load
+from textwrap import dedent
 
 base = '''
 from conans import ConanFile
@@ -39,6 +40,28 @@ def package(self):
 
 
 class PathLengthLimitTest(unittest.TestCase):
+    @unittest.skipUnless(platform.system() == "Windows", "requires Win")
+    def failure_copy_test(self):
+        client = TestClient()
+        conanfile = dedent("""
+            from conans import ConanFile
+            from conans.tools import save
+            import os
+
+            class ConanLib(ConanFile):
+                def source(self):
+                    cwd = os.getcwd()
+                    size = len(os.getcwd())
+                    sub = "a/"*((240-size)/2)
+                    path = os.path.join(cwd, sub, "file.txt")
+                    path = os.path.normpath(path)
+                    save(path, "contents")
+
+            """)
+        client.save({"conanfile.py": conanfile})
+        client.run("create . pkg/1.0@user/testing", assert_error=True)
+        self.assertIn("Consider using short_paths=True if paths too long", client.out)
+        self.assertIn("Error copying sources to build folder", client.out)
 
     def remove_test(self):
         short_home = tempfile.mkdtemp(dir=CONAN_TEST_FOLDER)
diff --git a/conans/util/windows.py b/conans/util/windows.py
index 1b95ffe8fd1..76a7525ab57 100644
--- a/conans/util/windows.py
+++ b/conans/util/windows.py
@@ -89,23 +89,6 @@ def path_shortener(path, short_paths):
     return redirect
 
 
-def ignore_long_path_files(src_folder, build_folder, output):
-    def _filter(src, files):
-        filtered_files = []
-        for the_file in files:
-            source_path = os.path.join(src, the_file)
-            # Without storage path, just relative
-            rel_path = os.path.relpath(source_path, src_folder)
-            dest_path = os.path.normpath(os.path.join(build_folder, rel_path))
-            # it is NOT that "/" is counted as "\\" so it counts double
-            # seems a bug in python, overflows paths near the limit of 260,
-            if len(dest_path) >= 249:
-                filtered_files.append(the_file)
-                output.warn("Filename too long, file excluded: %s" % dest_path)
-        return filtered_files
-    return _filter
-
-
 def rm_conandir(path):
     """removal of a directory that might contain a link to a short path"""
     link = os.path.join(path, CONAN_LINK)


Code review comments:
diff_hunk: @@ -90,13 +89,12 @@ def prepare_build(self):
             mkdir(self.build_folder)
             self._conan_file.source_folder = self.source_folder
         else:
-            if platform.system() == "Windows" and os.getenv("CONAN_USER_HOME_SHORT") != "None":
-                from conans.util.windows import ignore_long_path_files
-                ignore = ignore_long_path_files(self.source_folder, self.build_folder, self._out)
-            else:
-                ignore = None
-
-            shutil.copytree(self.source_folder, self.build_folder, symlinks=True, ignore=ignore)
+            try:
+                shutil.copytree(self.source_folder, self.build_folder, symlinks=True)
+            except Exception as e:
+                msg = str(e)
+                msg += "\nConsider using short_paths=True if paths too long" if "206" in msg else ""
path: conans/client/installer.py
text: @user1:
The `206` looks like some kind of magic number, I would add a comment with a link or maybe check for `WindowsError: [Error 206]` to make it clearer (maybe the full error message).

Also, can the message be assertive? if the Windows error is `WindowsError: [Error 206] The filename or extension is too long` probably we can tell the user without doubts that **the error is because of long paths** and recommend the `short_paths` solution.

@author:
Fun fact it is not a ``WindowsError``. It is a ``shutil.Error`` wrapping a system error, with no further metadata, because it comes from the system. The only thing that seems to be useful is the system error number, cause I guess the error string might be localized.