Repository: iterative/dvc

Issue:
alias `list` as `ls`?
I have been reaching for `dvc ls` out of habit instead of `dvc list`. Should we introduce an alias for `dvc list`?

Discussion:
skshetry: @ap-kulkarni, could you please create a docs PR on the `dvc.org` side? 
ap-kulkarni: > @ap-kulkarni, could you please create a docs PR on the `dvc.org` side?

Sure.. Should I quote this ticket number to raise the PR there or a separate ticket should be created?
efiop: @ap-kulkarni Linking this ticket is enough, no need to create new issue. Thank you! 🙏 

Patch:
diff --git a/dvc/command/ls/__init__.py b/dvc/command/ls/__init__.py
index a44877f03..1b2317c6f 100644
--- a/dvc/command/ls/__init__.py
+++ b/dvc/command/ls/__init__.py
@@ -54,6 +54,7 @@ def add_parser(subparsers, parent_parser):
     )
     list_parser = subparsers.add_parser(
         "list",
+        aliases=["ls"],
         parents=[parent_parser],
         description=append_doc_link(LIST_HELP, "list"),
         help=LIST_HELP,
diff --git a/dvc/config_schema.py b/dvc/config_schema.py
index c3aa29d32..13d5a73ae 100644
--- a/dvc/config_schema.py
+++ b/dvc/config_schema.py
@@ -156,6 +156,8 @@ SCHEMA = {
                     "grant_write_acp": str,
                     "grant_full_control": str,
                     "cache_regions": bool,
+                    "read_timeout": Coerce(int),
+                    "connect_timeout": Coerce(int),
                     **REMOTE_COMMON,
                 },
                 "gs": {
diff --git a/dvc/fs/http.py b/dvc/fs/http.py
index 0f533bcf0..ccfe4b326 100644
--- a/dvc/fs/http.py
+++ b/dvc/fs/http.py
@@ -87,6 +87,9 @@ class HTTPFileSystem(NoDirectoriesMixin, FSSpecWrapper):
                     ssl=make_context(config["ssl_verify"])
                 )
 
+        # Allow reading proxy configurations from the environment.
+        client_kwargs["trust_env"] = True
+
         credentials["get_client"] = self.get_client
         self.upload_method = config.get("method", "POST")
         return credentials
diff --git a/dvc/fs/s3.py b/dvc/fs/s3.py
index 91295a791..e22854179 100644
--- a/dvc/fs/s3.py
+++ b/dvc/fs/s3.py
@@ -99,6 +99,11 @@ class BaseS3FileSystem(ObjectFSWrapper):
         client["endpoint_url"] = config.get("endpointurl")
         client["verify"] = config.get("ssl_verify")
 
+        # timeout configuration
+        config_kwargs = login_info["config_kwargs"]
+        config_kwargs["read_timeout"] = config.get("read_timeout")
+        config_kwargs["connect_timeout"] = config.get("connect_timeout")
+
         # encryptions
         additional = login_info["s3_additional_kwargs"]
         additional["ServerSideEncryption"] = config.get("sse")
diff --git a/dvc/ignore.py b/dvc/ignore.py
index 141a482fa..30d86e6e5 100644
--- a/dvc/ignore.py
+++ b/dvc/ignore.py
@@ -11,7 +11,6 @@ from dvc.fs.base import BaseFileSystem
 from dvc.path_info import PathInfo
 from dvc.pathspec_math import PatternInfo, merge_patterns
 from dvc.scheme import Schemes
-from dvc.system import System
 from dvc.types import AnyPath, List, Optional
 from dvc.utils import relpath
 from dvc.utils.collections import PathStringTrie
@@ -84,7 +83,7 @@ class DvcIgnorePatterns(DvcIgnore):
         else:
             return False
 
-        if not System.is_unix():
+        if os.name == "nt":
             path = normalize_file(path)
         return path
 
diff --git a/dvc/state.py b/dvc/state.py
index 4881a62ef..ffdb051e3 100644
--- a/dvc/state.py
+++ b/dvc/state.py
@@ -79,11 +79,6 @@ class State(StateBase):  # pylint: disable=too-many-instance-attributes
         if not isinstance(fs, LocalFileSystem):
             return
 
-        assert isinstance(path_info, str) or path_info.scheme == "local"
-        assert hash_info
-        assert isinstance(hash_info, HashInfo)
-        assert os.path.exists(path_info)
-
         mtime, size = get_mtime_and_size(path_info, fs, self.dvcignore)
         inode = get_inode(path_info)
 
@@ -111,17 +106,12 @@ class State(StateBase):  # pylint: disable=too-many-instance-attributes
         if not isinstance(fs, LocalFileSystem):
             return None
 
-        assert isinstance(path_info, str) or path_info.scheme == "local"
-        path = os.fspath(path_info)
-
-        # NOTE: use os.path.exists instead of LocalFileSystem.exists
-        # because it uses lexists() and will return True for broken
-        # symlinks that we cannot stat() in get_mtime_and_size
-        if not os.path.exists(path):
+        try:
+            mtime, size = get_mtime_and_size(path_info, fs, self.dvcignore)
+        except FileNotFoundError:
             return None
 
-        mtime, size = get_mtime_and_size(path, fs, self.dvcignore)
-        inode = get_inode(path)
+        inode = get_inode(path_info)
 
         value = self.md5s.get(inode)
 
@@ -140,12 +130,11 @@ class State(StateBase):  # pylint: disable=too-many-instance-attributes
         if not isinstance(fs, LocalFileSystem):
             return
 
-        assert isinstance(path_info, str) or path_info.scheme == "local"
-
-        if not fs.exists(path_info):
+        try:
+            mtime, _ = get_mtime_and_size(path_info, fs, self.dvcignore)
+        except FileNotFoundError:
             return
 
-        mtime, _ = get_mtime_and_size(path_info, fs, self.dvcignore)
         inode = get_inode(path_info)
         relative_path = relpath(path_info, self.root_dir)
 
diff --git a/dvc/system.py b/dvc/system.py
index a7c27f62b..0387bc8b5 100644
--- a/dvc/system.py
+++ b/dvc/system.py
@@ -2,30 +2,13 @@ import errno
 import logging
 import os
 import platform
-import sys
 
 from dvc.exceptions import DvcException
 
 logger = logging.getLogger(__name__)
 
-if (
-    sys.platform == "win32"
-    and sys.version_info < (3, 8)
-    and sys.getwindowsversion() >= (6, 2)
-):
-    try:
-        import speedcopy
-
-        speedcopy.patch_copyfile()
-    except ImportError:
-        pass
-
 
 class System:
-    @staticmethod
-    def is_unix():
-        return os.name != "nt"
-
     @staticmethod
     def hardlink(source, link_name):
         try:
@@ -117,99 +100,14 @@ class System:
         # so we need to chmod it to look like a normal copy.
         os.chmod(link_name, 0o666 & ~umask)
 
-    @staticmethod
-    def _getdirinfo(path):
-        from collections import namedtuple
-
-        from win32file import (  # pylint: disable=import-error
-            FILE_FLAG_BACKUP_SEMANTICS,
-            FILE_FLAG_OPEN_REPARSE_POINT,
-            FILE_SHARE_READ,
-            OPEN_EXISTING,
-            CreateFileW,
-            GetFileInformationByHandle,
-        )
-
-        # NOTE: use FILE_FLAG_OPEN_REPARSE_POINT to open symlink itself and not
-        # the target See https://docs.microsoft.com/en-us/windows/desktop/api/
-        # fileapi/nf-fileapi-createfilew#symbolic-link-behavior
-        flags = FILE_FLAG_BACKUP_SEMANTICS | FILE_FLAG_OPEN_REPARSE_POINT
-
-        hfile = CreateFileW(
-            path, 0, FILE_SHARE_READ, None, OPEN_EXISTING, flags, None
-        )
-
-        # See BY_HANDLE_FILE_INFORMATION structure from fileapi.h
-        Info = namedtuple(
-            "BY_HANDLE_FILE_INFORMATION",
-            [
-                "dwFileAttributes",
-                "ftCreationTime",
-                "ftLastAccessTime",
-                "ftLastWriteTime",
-                "dwVolumeSerialNumber",
-                "nFileSizeHigh",
-                "nFileSizeLow",
-                "nNumberOfLinks",
-                "nFileIndexHigh",
-                "nFileIndexLow",
-            ],
-        )
-
-        return Info(*GetFileInformationByHandle(hfile))
-
     @staticmethod
     def inode(path):
-        path = os.fspath(path)
-
-        if System.is_unix():
-            import ctypes
-
-            inode = os.lstat(path).st_ino
-            # NOTE: See https://bugs.python.org/issue29619 and
-            # https://stackoverflow.com/questions/34643289/
-            # pythons-os-stat-is-returning-wrong-inode-value
-            inode = ctypes.c_ulong(inode).value
-        else:
-            # getdirinfo from ntfsutils works on both files and dirs
-            info = System._getdirinfo(path)
-            inode = abs(
-                hash(
-                    (
-                        info.dwVolumeSerialNumber,
-                        info.nFileIndexHigh,
-                        info.nFileIndexLow,
-                    )
-                )
-            )
-        assert inode >= 0
-        assert inode < 2 ** 64
-        return inode
+        return os.lstat(path).st_ino
 
     @staticmethod
     def is_symlink(path):
-        path = os.fspath(path)
-
-        if System.is_unix():
-            return os.path.islink(path)
-
-        # https://docs.microsoft.com/en-us/windows/desktop/fileio/
-        # file-attribute-constants
-        from winnt import (  # pylint: disable=import-error
-            FILE_ATTRIBUTE_REPARSE_POINT,
-        )
-
-        if os.path.lexists(path):
-            info = System._getdirinfo(path)
-            return info.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT
-        return False
+        return os.path.islink(path)
 
     @staticmethod
     def is_hardlink(path):
-        path = os.fspath(path)
-
-        if System.is_unix():
-            return os.stat(path).st_nlink > 1
-
-        info = System._getdirinfo(path)
-        return info.nNumberOfLinks > 1
+        return os.stat(path).st_nlink > 1
diff --git a/requirements/default.txt b/requirements/default.txt
index b97a691a8..609856256 100644
--- a/requirements/default.txt
+++ b/requirements/default.txt
@@ -24,11 +24,9 @@ packaging>=19.0
 zc.lockfile>=1.2.1
 flufl.lock>=3.2,<4
 win-unicode-console>=0.5; sys_platform == 'win32'
-pywin32>=225; sys_platform == 'win32' and python_version < '3.10'
 networkx>=2.5
 psutil>=5.8.0
 pydot>=1.2.4
-speedcopy>=2.0.1; python_version < '3.8' and sys_platform == 'win32'
 dataclasses==0.7; python_version < '3.7'
 importlib-metadata>=1.4; python_version < '3.8'
 flatten_dict>=0.4.1,<1
diff --git a/scripts/pyinstaller/hooks/hook-win32file.py b/scripts/pyinstaller/hooks/hook-win32file.py
deleted file mode 100644
index daee85e09..000000000
--- a/scripts/pyinstaller/hooks/hook-win32file.py
+++ /dev/null
@@ -1,1 +0,0 @@
-hiddenimports = ["win32timezone"]


Test patch:
diff --git a/requirements/tests.txt b/requirements/tests.txt
index 155f54a5e..17915135a 100644
--- a/requirements/tests.txt
+++ b/requirements/tests.txt
@@ -21,6 +21,7 @@ google-compute-engine==2.8.13
 google-cloud-storage==1.42.1
 urllib3==1.26.6
 dvclive>=0.2.1
+pywin32>=225; sys_platform == 'win32' and python_version < '3.10'
 
 # required by collective.checkdocs
 Pygments==2.10.0
diff --git a/tests/func/test_system.py b/tests/func/test_system.py
deleted file mode 100644
index 12c42b6d1..000000000
--- a/tests/func/test_system.py
+++ /dev/null
@@ -1,17 +0,0 @@
-import os
-
-import pytest
-
-from dvc.system import System
-
-
-@pytest.mark.skipif(os.name != "nt", reason="Windows specific")
-def test_getdirinfo(tmp_path):
-    # simulate someone holding an exclusive access
-    locked = str(tmp_path / "locked")
-    fd = os.open(locked, os.O_WRONLY | os.O_CREAT | os.O_EXCL)
-    try:
-        # should not raise WinError
-        System._getdirinfo(locked)
-    finally:
-        os.close(fd)
diff --git a/tests/unit/command/ls/test_ls.py b/tests/unit/command/ls/test_ls.py
index c690d0ff5..7e7970c01 100644
--- a/tests/unit/command/ls/test_ls.py
+++ b/tests/unit/command/ls/test_ls.py
@@ -109,3 +109,8 @@ def test_show_colors(mocker, capsys, monkeypatch):
         "\x1b[01;34msrc\x1b[0m",
         "\x1b[01;32mrun.sh\x1b[0m",
     ]
+
+
+def test_list_alias():
+    cli_args = parse_args(["ls", "local_dir"])
+    assert cli_args.func == CmdList