Fix line-ending related errors on MacOS (ARM64)

smartsim/_core/_cli/build.py

@@ -26,6 +26,7 @@
 
 import argparse
 import os
+import platform
 import sys
 import typing as t
 from pathlib import Path
@@ -113,7 +114,12 @@ def build_database(
     # check database installation
     database_name = "KeyDB" if keydb else "Redis"
     database_builder = builder.DatabaseBuilder(
-        build_env(), build_env.MALLOC, build_env.JOBS, verbose
+        build_env(),
+        jobs=build_env.JOBS,
+        _os=builder.OperatingSystem.from_str(platform.system()),
+        architecture=builder.Architecture.from_str(platform.machine()),
+        malloc=build_env.MALLOC,
+        verbose=verbose,
     )
     if not database_builder.is_built:
         logger.info(
@@ -173,12 +179,14 @@ def build_redis_ai(
 
     rai_builder = builder.RedisAIBuilder(
         build_env=build_env_dict,
+        jobs=build_env.JOBS,
+        _os=builder.OperatingSystem.from_str(platform.system()),
+        architecture=builder.Architecture.from_str(platform.machine()),
         torch_dir=str(torch_dir) if torch_dir else "",
         libtf_dir=str(libtf_dir) if libtf_dir else "",
         build_torch=use_torch,
         build_tf=use_tf,
         build_onnx=use_onnx,
-        jobs=build_env.JOBS,
         verbose=verbose,
     )
 

smartsim/_core/_install/builder.py

@@ -24,6 +24,8 @@
 # OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
+# pylint: disable=too-many-lines
+
 import concurrent.futures
 import enum
 import itertools
@@ -107,6 +109,11 @@ def from_str(cls, string: str, /) -> "OperatingSystem":
         raise BuildError(f"Unrecognized or unsupported operating system: {string}")
 
 
+class Platform(t.NamedTuple):
+    os: OperatingSystem
+    architecture: Architecture
+
+
 class Builder:
     """Base class for building third-party libraries"""
 
@@ -121,10 +128,16 @@ class Builder:
     )
 
     def __init__(
-        self, env: t.Dict[str, t.Any], jobs: t.Optional[int] = 1, verbose: bool = False
+        self,
+        env: t.Dict[str, str],
+        jobs: int = 1,
+        _os: OperatingSystem = OperatingSystem.from_str(platform.system()),
+        architecture: Architecture = Architecture.from_str(platform.machine()),
+        verbose: bool = False,
     ) -> None:
         # build environment from buildenv
         self.env = env
+        self._platform = Platform(_os, architecture)
 
         # Find _core directory and set up paths
         _core_dir = Path(os.path.abspath(__file__)).parent.parent
@@ -236,12 +249,20 @@ class DatabaseBuilder(Builder):
 
     def __init__(
         self,
-        build_env: t.Optional[t.Dict[str, t.Any]] = None,
+        build_env: t.Optional[t.Dict[str, str]] = None,
         malloc: str = "libc",
-        jobs: t.Optional[int] = None,
+        jobs: int = 1,
+        _os: OperatingSystem = OperatingSystem.from_str(platform.system()),
+        architecture: Architecture = Architecture.from_str(platform.machine()),
         verbose: bool = False,
     ) -> None:
-        super().__init__(build_env or {}, jobs=jobs, verbose=verbose)
+        super().__init__(
+            build_env or {},
+            jobs=jobs,
+            _os=_os,
+            architecture=architecture,
+            verbose=verbose,
+        )
         self.malloc = malloc
 
     @property
@@ -278,17 +299,21 @@ def build_from_git(
         if not self.is_valid_url(git_url):
             raise BuildError(f"Malformed {database_name} URL: {git_url}")
 
+        clone_cmd = config_git_command(
+            self._platform,
+            [
+                self.binary_path("git"),
+                "clone",
+                git_url,
+                "--branch",
+                branch,
+                "--depth",
+                "1",
+                database_name,
+            ],
+        )
+
         # clone Redis
-        clone_cmd = [
-            self.binary_path("git"),
-            "clone",
-            git_url,
-            "--branch",
-            branch,
-            "--depth",
-            "1",
-            database_name,
-        ]
         self.run_command(clone_cmd, cwd=self.build_dir)
 
         # build Redis
@@ -372,20 +397,24 @@ def __init__(
         self,
         _os: OperatingSystem = OperatingSystem.from_str(platform.system()),
         architecture: Architecture = Architecture.from_str(platform.machine()),
-        build_env: t.Optional[t.Dict[str, t.Any]] = None,
+        build_env: t.Optional[t.Dict[str, str]] = None,
         torch_dir: str = "",
         libtf_dir: str = "",
         build_torch: bool = True,
         build_tf: bool = True,
         build_onnx: bool = False,
-        jobs: t.Optional[int] = None,
+        jobs: int = 1,
         verbose: bool = False,
     ) -> None:
-        super().__init__(build_env or {}, jobs=jobs, verbose=verbose)
+        super().__init__(
+            build_env or {},
+            jobs=jobs,
+            _os=_os,
+            architecture=architecture,
+            verbose=verbose,
+        )
 
         self.rai_install_path: t.Optional[Path] = None
-        self._os = _os
-        self._architecture = architecture
 
         # convert to int for RAI build script
         self._torch = build_torch
@@ -398,20 +427,23 @@ def __init__(
         self._validate_platform()
 
     def _validate_platform(self) -> None:
-        platform_ = (self._os, self._architecture)
         unsupported = []
-        if platform_ not in _DLPackRepository.supported_platforms():
+        if self._platform not in _DLPackRepository.supported_platforms():
             unsupported.append("DLPack")
-        if self.fetch_tf and (platform_ not in _TFArchive.supported_platforms()):
+        if self.fetch_tf and (self._platform not in _TFArchive.supported_platforms()):
             unsupported.append("Tensorflow")
-        if self.fetch_onnx and (platform_ not in _ORTArchive.supported_platforms()):
+        if self.fetch_onnx and (
+            self._platform not in _ORTArchive.supported_platforms()
+        ):
             unsupported.append("ONNX")
-        if self.fetch_torch and (platform_ not in _PTArchive.supported_platforms()):
+        if self.fetch_torch and (
+            self._platform not in _PTArchive.supported_platforms()
+        ):
             unsupported.append("PyTorch")
         if unsupported:
             raise BuildError(
-                f"The {', '.join(unsupported)} backend(s) are not "
-                f"supported on {self._os} with {self._architecture}"
+                f"The {', '.join(unsupported)} backend(s) are not supported "
+                f"on {self._platform.os} with {self._platform.architecture}"
             )
 
     @property
@@ -452,25 +484,25 @@ def get_deps_dir_path_for(self, device: TDeviceStr) -> Path:
         def fail_to_format(reason: str) -> BuildError:  # pragma: no cover
             return BuildError(f"Failed to format RedisAI dependency path: {reason}")
 
-        if self._os == OperatingSystem.DARWIN:
+        _os, architecture = self._platform
+        if _os == OperatingSystem.DARWIN:
             os_ = "macos"
-        elif self._os == OperatingSystem.LINUX:
+        elif _os == OperatingSystem.LINUX:
             os_ = "linux"
         else:  # pragma: no cover
-            raise fail_to_format(f"Unknown operating system: {self._os}")
-        if self._architecture == Architecture.X64:
+            raise fail_to_format(f"Unknown operating system: {_os}")
+        if architecture == Architecture.X64:
             arch = "x64"
-        elif self._architecture == Architecture.ARM64:
+        elif architecture == Architecture.ARM64:
             arch = "arm64v8"
         else:  # pragma: no cover
-            raise fail_to_format(f"Unknown architecture: {self._architecture}")
+            raise fail_to_format(f"Unknown architecture: {architecture}")
         return self.rai_build_path / f"deps/{os_}-{arch}-{device}"
 
     def _get_deps_to_fetch_for(
         self, device: TDeviceStr
     ) -> t.Tuple[_RAIBuildDependency, ...]:
-        os_ = self._os
-        arch = self._architecture
+        os_, arch = self._platform
         # TODO: It would be nice if the backend version numbers were declared
         #       alongside the python package version numbers so that all of the
         #       dependency versions were declared in single location.
@@ -565,18 +597,21 @@ def build_from_git(
             raise BuildError(f"Malformed RedisAI URL: {git_url}")
 
         # clone RedisAI
-        clone_cmd = [
-            self.binary_path("env"),
-            "GIT_LFS_SKIP_SMUDGE=1",
-            "git",
-            "clone",
-            "--recursive",
-            git_url,
-            "--branch",
-            branch,
-            "--depth=1",
-            os.fspath(self.rai_build_path),
-        ]
+        clone_cmd = config_git_command(
+            self._platform,
+            [
+                self.binary_path("env"),
+                "GIT_LFS_SKIP_SMUDGE=1",
+                "git",
+                "clone",
+                "--recursive",
+                git_url,
+                "--branch",
+                branch,
+                "--depth=1",
+                os.fspath(self.rai_build_path),
+            ],
+        )
 
         self.run_command(clone_cmd, out=subprocess.DEVNULL, cwd=self.build_dir)
         self._fetch_deps_for(device)
@@ -873,7 +908,6 @@ def url(self) -> str:
 def _choose_pt_variant(
     os_: OperatingSystem,
 ) -> t.Union[t.Type[_PTArchiveLinux], t.Type[_PTArchiveMacOSX]]:
-
     if os_ == OperatingSystem.DARWIN:
         return _PTArchiveMacOSX
     if os_ == OperatingSystem.LINUX:
@@ -995,3 +1029,20 @@ def _git(*args: str) -> None:
             raise BuildError(
                 f"Command `{' '.join(cmd)}` failed with exit code {proc.returncode}"
             )
+
+
+def config_git_command(plat: Platform, cmd: t.Sequence[str]) -> t.List[str]:
+    """Modify git commands to include autocrlf when on a platform that needs
+    autocrlf enabled to behave correctly
+    """
+    cmd = list(cmd)
+    where = next((i for i, tok in enumerate(cmd) if tok.endswith("git")), len(cmd)) + 2
+    if where >= len(cmd):
+        raise ValueError(f"Failed to locate git command in '{' '.join(cmd)}'")
+    if plat == Platform(OperatingSystem.DARWIN, Architecture.ARM64):
+        cmd = (
+            cmd[:where]
+            + ["--config", "core.autocrlf=false", "--config", "core.eol=lf"]
+            + cmd[where:]
+        )
+    return cmd

tests/install/test_builder.py

@@ -287,3 +287,78 @@ def test_valid_platforms():
         build_torch=True,
         build_onnx=False,
     )
+
+
+@pytest.mark.parametrize(
+    "plat,cmd,expected_cmd",
+    [
+        # Bare Word
+        pytest.param(
+            build.Platform(build.OperatingSystem.LINUX, build.Architecture.X64),
+            ["git", "clone", "my-repo"],
+            ["git", "clone", "my-repo"],
+            id="git-Linux-X64",
+        ),
+        pytest.param(
+            build.Platform(build.OperatingSystem.LINUX, build.Architecture.ARM64),
+            ["git", "clone", "my-repo"],
+            ["git", "clone", "my-repo"],
+            id="git-Linux-Arm64",
+        ),
+        pytest.param(
+            build.Platform(build.OperatingSystem.DARWIN, build.Architecture.X64),
+            ["git", "clone", "my-repo"],
+            ["git", "clone", "my-repo"],
+            id="git-Darwin-X64",
+        ),
+        pytest.param(
+            build.Platform(build.OperatingSystem.DARWIN, build.Architecture.ARM64),
+            ["git", "clone", "my-repo"],
+            [
+                "git",
+                "clone",
+                "--config",
+                "core.autocrlf=false",
+                "--config",
+                "core.eol=lf",
+                "my-repo",
+            ],
+            id="git-Darwin-Arm64",
+        ),
+        # Abs path
+        pytest.param(
+            build.Platform(build.OperatingSystem.LINUX, build.Architecture.X64),
+            ["/path/to/git", "clone", "my-repo"],
+            ["/path/to/git", "clone", "my-repo"],
+            id="Abs-Linux-X64",
+        ),
+        pytest.param(
+            build.Platform(build.OperatingSystem.LINUX, build.Architecture.ARM64),
+            ["/path/to/git", "clone", "my-repo"],
+            ["/path/to/git", "clone", "my-repo"],
+            id="Abs-Linux-Arm64",
+        ),
+        pytest.param(
+            build.Platform(build.OperatingSystem.DARWIN, build.Architecture.X64),
+            ["/path/to/git", "clone", "my-repo"],
+            ["/path/to/git", "clone", "my-repo"],
+            id="Abs-Darwin-X64",
+        ),
+        pytest.param(
+            build.Platform(build.OperatingSystem.DARWIN, build.Architecture.ARM64),
+            ["/path/to/git", "clone", "my-repo"],
+            [
+                "/path/to/git",
+                "clone",
+                "--config",
+                "core.autocrlf=false",
+                "--config",
+                "core.eol=lf",
+                "my-repo",
+            ],
+            id="Abs-Darwin-Arm64",
+        ),
+    ],
+)
+def test_git_commands_are_configered_correctly_for_platforms(plat, cmd, expected_cmd):
+    assert build.config_git_command(plat, cmd) == expected_cmd