Repository: valohai/valohai-cli

Issue:
Submodules in ad-hoc runs
`git ls-files` does not delve into submodule directories, so when using Git to package files for ad-hoc runs, those files don't get uploaded.

`git ls-files --recurse-submodules` is only supported with `--cached` at present.

A workaround is to hack the packager not to use git for packaging.

Options:

* Expose the `allow_git` flag (e.g. an envvar/global click flag `--no-git`)
* If we'd otherwise use `git ls-files`, detect if submodules are in use and don't use `git ls-files` if they are. (e.g. if the output of `git submodule` is longer than zero).

Discussion:
Kenneth-Schroeder: Thank you, good feedback, I am not familiar with click :)

Patch:
diff --git a/valohai_cli/adhoc.py b/valohai_cli/adhoc.py
index be2a9c2..27264b5 100644
--- a/valohai_cli/adhoc.py
+++ b/valohai_cli/adhoc.py
@@ -14,7 +14,7 @@ from valohai_cli.utils.file_size_format import filesizeformat
 from valohai_cli.utils.hashing import get_fp_sha256
 
 
-def package_adhoc_commit(project: Project, validate: bool = True, yaml_path: Optional[str] = None) -> Dict[str, Any]:
+def package_adhoc_commit(project: Project, validate: bool = True, yaml_path: Optional[str] = None, allow_git: bool = True) -> Dict[str, Any]:
     """
     Create an ad-hoc tarball and commit of the project directory.
 
@@ -38,7 +38,7 @@ def package_adhoc_commit(project: Project, validate: bool = True, yaml_path: Opt
             click.echo(f'Packaging {directory}...')
 
         yaml_path = yaml_path or project.get_yaml_path()
-        tarball = package_directory(directory=directory, progress=True, validate=validate, yaml_path=yaml_path)
+        tarball = package_directory(directory=directory, progress=True, validate=validate, yaml_path=yaml_path, allow_git=allow_git)
         return create_adhoc_commit_from_tarball(project=project, tarball=tarball, yaml_path=yaml_path, description=description)
     finally:
         if tarball:
diff --git a/valohai_cli/commands/deployment/create_version/create_version.py b/valohai_cli/commands/deployment/create_version/create_version.py
index e5ce304..041a261 100644
--- a/valohai_cli/commands/deployment/create_version/create_version.py
+++ b/valohai_cli/commands/deployment/create_version/create_version.py
@@ -25,12 +25,14 @@ from valohai_cli.utils.matching import match_from_list_with_error
 @click.option('--inherit-env-vars/--no-inherit-env-vars', default=True, help='Use project environment variables in deployment.')
 @click.argument('args', nargs=-1, type=click.UNPROCESSED, metavar='ENDPOINT-OPTIONS...')
 @click.option('--adhoc', '-a', is_flag=True, help='Upload the current state of the working directory, then create a deployment version from it.')
+@click.option('--git-packaging/--no-git-packaging', '-g/-G', default=True, is_flag=True, help='When creating ad-hoc executions, whether to allow using Git for packaging directory contents.')
 @click.pass_context
 def create_version(
     ctx: click.Context,
     *,
     args: List[str],
     adhoc: bool,
+    git_packaging: bool = True,
     commit: Optional[str],
     deployment: str,
     environment_variables: List[str],
@@ -47,6 +49,7 @@ def create_version(
         project,
         commit=commit,
         adhoc=adhoc,
+        allow_git_packaging=git_packaging,
         yaml_path=None,
     )
     deployments = request('get', '/api/v0/deployments/', params={'project': project.id}).json()['results']
diff --git a/valohai_cli/commands/execution/run/frontend_command.py b/valohai_cli/commands/execution/run/frontend_command.py
index 0decd30..9a489f4 100644
--- a/valohai_cli/commands/execution/run/frontend_command.py
+++ b/valohai_cli/commands/execution/run/frontend_command.py
@@ -37,6 +37,7 @@ EMPTY_DICT_PLACEHOLDER = object()
 @click.option('--tag', 'tags', multiple=True, help='Tag the execution. May be repeated.')
 @click.option('--sync', '-s', 'download_directory', type=click.Path(file_okay=False), help='Download execution outputs to DIRECTORY.', default=None)
 @click.option('--adhoc', '-a', is_flag=True, help='Upload the current state of the working directory, then run it as an ad-hoc execution.')
+@click.option('--git-packaging/--no-git-packaging', '-g/-G', default=True, is_flag=True, help='When creating ad-hoc tasks, whether to allow using Git for packaging directory contents.')
 @click.option('--validate-adhoc/--no-validate-adhoc', help='Enable or disable validation of adhoc packaged code, on by default', default=True)
 @click.option('--yaml', default=None, help='The path to the configuration YAML (valohai.yaml) file to use.')
 @click.option('--debug-port', type=int)
@@ -55,6 +56,7 @@ def run(
     ctx: click.Context,
     *,
     adhoc: bool,
+    git_packaging: bool = True,
     args: List[str],
     commit: Optional[str],
     yaml: Optional[str],
@@ -110,6 +112,7 @@ def run(
         project,
         commit=commit,
         adhoc=adhoc,
+        allow_git_packaging=git_packaging,
         validate_adhoc_commit=validate_adhoc,
         yaml_path=yaml,
     )
diff --git a/valohai_cli/commands/pipeline/run/run.py b/valohai_cli/commands/pipeline/run/run.py
index af1669a..5a3bdc5 100644
--- a/valohai_cli/commands/pipeline/run/run.py
+++ b/valohai_cli/commands/pipeline/run/run.py
@@ -21,16 +21,19 @@ from valohai_cli.utils.commits import create_or_resolve_commit
 @click.option('--commit', '-c', default=None, metavar='SHA', help='The commit to use. Defaults to the current HEAD.')
 @click.option('--title', '-c', default=None, help='The optional title of the pipeline run.')
 @click.option('--adhoc', '-a', is_flag=True, help='Upload the current state of the working directory, then run it as an ad-hoc execution.')
+@click.option('--git-packaging/--no-git-packaging', '-g/-G', default=True, is_flag=True, help='When creating ad-hoc pipelines, whether to allow using Git for packaging directory contents.')
 @click.option('--yaml', default=None, help='The path to the configuration YAML file (valohai.yaml) file to use.')
 @click.option('--tag', 'tags', multiple=True, help='Tag the pipeline. May be repeated.')
 @click.argument('args', nargs=-1, type=click.UNPROCESSED, metavar='PIPELINE-OPTIONS...')
 @click.pass_context
 def run(
     ctx: Context,
+    *,
     name: Optional[str],
     commit: Optional[str],
     title: Optional[str],
     adhoc: bool,
+    git_packaging: bool = True,
     yaml: Optional[str],
     tags: List[str],
     args: List[str],
@@ -51,7 +54,7 @@ def run(
     if yaml and not adhoc:
         raise click.UsageError('--yaml is only valid with --adhoc')
 
-    commit = create_or_resolve_commit(project, commit=commit, adhoc=adhoc, yaml_path=yaml)
+    commit = create_or_resolve_commit(project, commit=commit, adhoc=adhoc, yaml_path=yaml, allow_git_packaging=git_packaging)
     config = project.get_config(commit_identifier=commit if project.is_remote else None, yaml_path=yaml)
 
     matched_pipeline = match_pipeline(config, name)
diff --git a/valohai_cli/packager.py b/valohai_cli/packager.py
index b7d3bd2..0574e50 100644
--- a/valohai_cli/packager.py
+++ b/valohai_cli/packager.py
@@ -53,8 +53,8 @@ class VhIgnoreUsage(Enum):
     VHIGNORE = 1
 
 
-def package_directory(*, directory: str, yaml_path: str, progress: bool = False, validate: bool = True) -> str:
-    file_stats = get_files_for_package(directory)
+def package_directory(*, directory: str, yaml_path: str, progress: bool = False, validate: bool = True, allow_git: bool = True) -> str:
+    file_stats = get_files_for_package(directory, allow_git=allow_git)
 
     if validate and yaml_path not in file_stats:
         raise ConfigurationError(f'configuration file {yaml_path} missing from {directory}')
diff --git a/valohai_cli/utils/commits.py b/valohai_cli/utils/commits.py
index 3932f56..17304d5 100644
--- a/valohai_cli/utils/commits.py
+++ b/valohai_cli/utils/commits.py
@@ -20,13 +20,14 @@ def create_or_resolve_commit(
     yaml_path: Optional[str],
     adhoc: bool,
     validate_adhoc_commit: bool = True,
+    allow_git_packaging: bool = True,
 ) -> str:
     if adhoc:
         if project.is_remote:
             raise click.UsageError('--adhoc can not be used with remote projects.')
         if commit:
             raise click.UsageError('--commit and --adhoc are mutually exclusive.')
-        commit = str(package_adhoc_commit(project, validate=validate_adhoc_commit, yaml_path=yaml_path)['identifier'])
+        commit = str(package_adhoc_commit(project, validate=validate_adhoc_commit, yaml_path=yaml_path, allow_git=allow_git_packaging)['identifier'])
     elif yaml_path:
         raise click.UsageError('--yaml can only be used with --adhoc.')
 


Test patch:
diff --git a/tests/commands/execution/test_run.py b/tests/commands/execution/test_run.py
index 7aa5a7b..1a8f212 100644
--- a/tests/commands/execution/test_run.py
+++ b/tests/commands/execution/test_run.py
@@ -1,3 +1,6 @@
+import os
+import subprocess
+
 import pytest
 import yaml
 
@@ -406,3 +409,22 @@ def test_kube_no_runtime_config_preset_argument(run_test_setup_kube):
     run_test_setup_kube.run()
 
     assert "runtime_config_preset" not in run_test_setup_kube.run_api_mock.last_create_execution_payload
+
+
+@pytest.mark.parametrize("allow_git_packaging", (True, False))
+def test_no_git_adhoc_packaging(logged_in_and_linked, monkeypatch, allow_git_packaging):
+    proj_dir = os.environ["VALOHAI_PROJECT_DIR"]  # set by `isolate_cli` autouse fixture
+    rts = RunTestSetup(monkeypatch=monkeypatch, adhoc=True)
+    try:
+        subprocess.check_call("git init -b x", cwd=proj_dir, shell=True)
+        subprocess.check_call("git add .", cwd=proj_dir, shell=True)
+        subprocess.check_call("git commit -q -m x", cwd=proj_dir, shell=True)
+    except subprocess.CalledProcessError as e:
+        pytest.skip(f"Failed to set up git repository: {e}")
+    if not allow_git_packaging:
+        rts.args.append("--no-git-packaging")
+    output = rts.run()
+    if allow_git_packaging:
+        assert "Used git to find" in output
+    else:
+        assert "Walked filesystem and found" in output