From 23ae85751966c47935a793ba1b49d738058f1a05 Mon Sep 17 00:00:00 2001 From: Naomi Kirby Date: Thu, 23 May 2024 09:40:33 -0700 Subject: [PATCH 1/9] Add submodules to run-task testing --- test/test_scripts_run_task.py | 68 +++++++++++++++++++++++++---------- 1 file changed, 49 insertions(+), 19 deletions(-) diff --git a/test/test_scripts_run_task.py b/test/test_scripts_run_task.py index f0ed878c1..6577ceac4 100644 --- a/test/test_scripts_run_task.py +++ b/test/test_scripts_run_task.py @@ -282,33 +282,61 @@ def git_current_rev(cwd): universal_newlines=True, ).strip() +@pytest.fixture() +def mock_homedir(monkeypatch): + "Mock home directory to isolate git config changes" + with tempfile.TemporaryDirectory() as fakehome: + env = os.environ.copy() + env['HOME'] = str(fakehome) + monkeypatch.setattr(os, "environ", env) + + # Enable the git file protocol for testing. + subprocess.check_call( + ["git", "config", "--global", "protocol.file.allow", "always"], env=env) + yield str(fakehome) @pytest.fixture(scope="session") # Tests shouldn't change this repo def mock_git_repo(): "Mock repository with files, commits and branches for using as source" - with tempfile.TemporaryDirectory() as repo: - repo_path = str(repo) - # Init git repo and setup user config - subprocess.check_call(["git", "init", "-b", "main"], cwd=repo_path) - subprocess.check_call(["git", "config", "user.name", "pytest"], cwd=repo_path) + def _create_empty_repo(path): + subprocess.check_call(["git", "init", "-b", "main", path]) + subprocess.check_call(["git", "config", "user.name", "pytest"], cwd=path) subprocess.check_call( - ["git", "config", "user.email", "py@tes.t"], cwd=repo_path + ["git", "config", "user.email", "py@tes.t"], cwd=path ) - def _commit_file(message, filename): - with open(os.path.join(repo, filename), "w") as fout: - fout.write("test file content") - subprocess.check_call(["git", "add", filename], cwd=repo_path) - subprocess.check_call(["git", "commit", "-m", message], cwd=repo_path) - return git_current_rev(repo_path) + def _commit_file(message, filename, path): + with open(os.path.join(path, filename), "w") as fout: + fout.write("test file content") + subprocess.check_call(["git", "add", filename], cwd=path) + subprocess.check_call(["git", "commit", "-m", message], cwd=path) + return git_current_rev(path) + + with tempfile.TemporaryDirectory() as repo: + # Create the submodule repository. + submod_path = os.path.join(str(repo), "submodule") + _create_empty_repo(submod_path) + _commit_file("Submodule content", "content", submod_path) + + # Create the testing repository. + repo_path = os.path.join(str(repo), "repository") + _create_empty_repo(repo_path) + + # Add submodule (to main branch) + subprocess.check_call( + ["git", "-c", "protocol.file.allow=always", + "submodule", "add", f"file://{os.path.abspath(submod_path)}", "submodule"], + cwd=repo_path + ) + subprocess.check_call(["git", "add", "submodule"], cwd=repo_path) # Commit mainfile (to main branch) - main_commit = _commit_file("Initial commit", "mainfile") + main_commit = _commit_file("Initial commit", "mainfile", repo_path) # New branch mybranch subprocess.check_call(["git", "checkout", "-b", "mybranch"], cwd=repo_path) # Commit branchfile to mybranch branch - branch_commit = _commit_file("File in mybranch", "branchfile") + branch_commit = _commit_file("File in mybranch", "branchfile", repo_path) # Set current branch back to main subprocess.check_call(["git", "checkout", "main"], cwd=repo_path) @@ -318,15 +346,16 @@ def _commit_file(message, filename): @pytest.mark.parametrize( "base_ref,ref,files,hash_key", [ - (None, None, ["mainfile"], "main"), - (None, "main", ["mainfile"], "main"), - (None, "mybranch", ["mainfile", "branchfile"], "branch"), - ("main", "main", ["mainfile"], "main"), - ("main", "mybranch", ["mainfile", "branchfile"], "branch"), + (None, None, ["mainfile", "submodule/content"], "main"), + (None, "main", ["mainfile", "submodule/content"], "main"), + (None, "mybranch", ["mainfile", "submodule/content", "branchfile"], "branch"), + ("main", "main", ["mainfile", "submodule/content"], "main"), + ("main", "mybranch", ["mainfile", "submodule/content", "branchfile"], "branch"), ], ) def test_git_checkout( mock_stdin, + mock_homedir, run_task_mod, mock_git_repo, base_ref, @@ -367,6 +396,7 @@ def test_git_checkout( def test_git_checkout_with_commit( mock_stdin, + mock_homedir, run_task_mod, mock_git_repo, ): From b2c88d34cf846265e62e5a028d4a3a92cf276b05 Mon Sep 17 00:00:00 2001 From: Naomi Kirby Date: Thu, 23 May 2024 17:05:03 -0700 Subject: [PATCH 2/9] Add support for submodule selection in run-task --- src/taskgraph/run-task/run-task | 56 +++++++++++++++++++++-------- test/test_scripts_run_task.py | 63 +++++++++++++++++++++++++++++---- 2 files changed, 97 insertions(+), 22 deletions(-) diff --git a/src/taskgraph/run-task/run-task b/src/taskgraph/run-task/run-task index f3a343de3..622ec8f28 100755 --- a/src/taskgraph/run-task/run-task +++ b/src/taskgraph/run-task/run-task @@ -598,6 +598,7 @@ def git_checkout( commit: Optional[str], ssh_key_file: Optional[Path], ssh_known_hosts_file: Optional[Path], + submodules: Optional[str | bool], ): env = { # abort if transfer speed is lower than 1kB/s for 1 minute @@ -705,23 +706,40 @@ def git_checkout( run_required_command(b"vcs", args, cwd=destination_path) - if os.path.exists(os.path.join(destination_path, ".gitmodules")): - args = [ - "git", - "submodule", - "init", - ] - - run_required_command(b"vcs", args, cwd=destination_path) - - args = [ - "git", - "submodule", - "update", - "--force", # Overrides any potential local changes + if submodules is not None: + def _filter_submodule(status): + # A boolean value selects/disables all submodules. + if isinstance(submodules, bool): + return submodules + + # Otherwise, a colon-separated stringlist of selected submodules. + smpath = status.split()[1] + return smpath in submodules.split(':') + + submodpaths = [ + status.split()[1] + for status in subprocess.check_output(["git", "submodule", "status"], + cwd=destination_path, universal_newlines=True).splitlines() + if _filter_submodule(status) ] - run_required_command(b"vcs", args, cwd=destination_path) + for p in submodpaths: + args = [ + "git", + "submodule", + "init", + p + ] + run_required_command(b"vcs", args, cwd=destination_path) + + args = [ + "git", + "submodule", + "update", + "--force", # Overrides any potential local changes + p + ] + run_required_command(b"vcs", args, cwd=destination_path) _clean_git_checkout(destination_path) @@ -894,6 +912,12 @@ def collect_vcs_options(args, project, name): ref = os.environ.get("%s_HEAD_REF" % env_prefix) pip_requirements = os.environ.get("%s_PIP_REQUIREMENTS" % env_prefix) private_key_secret = os.environ.get("%s_SSH_SECRET_NAME" % env_prefix) + submodules = os.environ.get("%s_SUBMODULES" % env_prefix) + + # Some special values can be used to request all submodules. + if submodules is not None: + if submodules.lower() in ["auto", "true", "yes"]: + submodules = True store_path = os.environ.get("HG_STORE_PATH") @@ -928,6 +952,7 @@ def collect_vcs_options(args, project, name): "repo-type": repo_type, "ssh-secret-name": private_key_secret, "pip-requirements": pip_requirements, + "submodules": submodules, } @@ -976,6 +1001,7 @@ def vcs_checkout_from_args(options): revision, ssh_key_file, ssh_known_hosts_file, + options["submodules"], ) elif options["repo-type"] == "hg": if not revision and not ref: diff --git a/test/test_scripts_run_task.py b/test/test_scripts_run_task.py index 6577ceac4..59139ed28 100644 --- a/test/test_scripts_run_task.py +++ b/test/test_scripts_run_task.py @@ -126,7 +126,41 @@ def test_install_pip_requirements( { "base-repo": "https://hg.mozilla.org/mozilla-unified", }, - ) + ), + pytest.param( + { + "REPOSITORY_TYPE": "git", + "BASE_REPOSITORY": "https://github.com/mozilla-mobile/mozilla-vpn-client.git", + "HEAD_REPOSITORY": "https://github.com/mozilla-mobile/mozilla-vpn-client.git", + "HEAD_REV": "abcdef", + "SUBMODULES": "3rdparty/i18n", + }, + {}, + ), + pytest.param( + { + "REPOSITORY_TYPE": "git", + "BASE_REPOSITORY": "https://github.com/mozilla-mobile/mozilla-vpn-client.git", + "HEAD_REPOSITORY": "https://github.com/mozilla-mobile/mozilla-vpn-client.git", + "HEAD_REV": "abcdef", + "SUBMODULES": "auto", + }, + { + "submodules": True, + }, + ), + pytest.param( + { + "REPOSITORY_TYPE": "git", + "BASE_REPOSITORY": "https://github.com/mozilla-mobile/mozilla-vpn-client.git", + "HEAD_REPOSITORY": "https://github.com/mozilla-mobile/mozilla-vpn-client.git", + "HEAD_REV": "abcdef", + "SUBMODULES": "yes", + }, + { + "submodules": True, + }, + ), ], ) def test_collect_vcs_options(monkeypatch, run_task_mod, env, extra_expected): @@ -159,6 +193,7 @@ def test_collect_vcs_options(monkeypatch, run_task_mod, env, extra_expected): "ssh-secret-name": env.get("SSH_SECRET_NAME"), "sparse-profile": False, "store-path": env.get("HG_STORE_PATH"), + "submodules": env.get("SUBMODULES"), } if "PIP_REQUIREMENTS" in env: expected["pip-requirements"] = os.path.join( @@ -344,13 +379,24 @@ def _commit_file(message, filename, path): @pytest.mark.parametrize( - "base_ref,ref,files,hash_key", + "base_ref,ref,submodules,files,hash_key", [ - (None, None, ["mainfile", "submodule/content"], "main"), - (None, "main", ["mainfile", "submodule/content"], "main"), - (None, "mybranch", ["mainfile", "submodule/content", "branchfile"], "branch"), - ("main", "main", ["mainfile", "submodule/content"], "main"), - ("main", "mybranch", ["mainfile", "submodule/content", "branchfile"], "branch"), + # Check out the repository in a bunch of states. + (None, None, None, ["mainfile"], "main"), + (None, "main", None, ["mainfile"], "main"), + (None, "mybranch", None, ["mainfile", "branchfile"], "branch"), + ("main", "main", None, ["mainfile"], "main"), + ("main", "mybranch", None, ["mainfile", "branchfile"], "branch"), + # Same tests again - but with submodules. + (None, None, True, ["mainfile", "submodule/content"], "main"), + (None, "main", True, ["mainfile", "submodule/content"], "main"), + (None, "mybranch", True, ["mainfile", "branchfile", "submodule/content"], "branch"), + ("main", "main", True, ["mainfile", "submodule/content"], "main"), + ("main", "mybranch", True, ["mainfile", "branchfile", "submodule/content"], "branch"), + # Tests for submodule matching rules. + (None, "main", "submodule", ["mainfile", "submodule/content"], "main"), + (None, "main", "one:two:three", ["mainfile"], "main"), + (None, "main", "one:two:submodule", ["mainfile", "submodule/content"], "main"), ], ) def test_git_checkout( @@ -360,6 +406,7 @@ def test_git_checkout( mock_git_repo, base_ref, ref, + submodules, files, hash_key, ): @@ -375,6 +422,7 @@ def test_git_checkout( commit=None, ssh_key_file=None, ssh_known_hosts_file=None, + submodules=submodules, ) # Check desired files exist @@ -412,6 +460,7 @@ def test_git_checkout_with_commit( commit=mock_git_repo["branch"], ssh_key_file=None, ssh_known_hosts_file=None, + submodules=None, ) From f66a8251380e7ce8a471b41188ed4aee6767a5be Mon Sep 17 00:00:00 2001 From: Naomi Kirby Date: Thu, 23 May 2024 17:13:34 -0700 Subject: [PATCH 3/9] Test checkouts with only a subset of submodules --- test/test_scripts_run_task.py | 40 +++++++++++++++++++++++------------ 1 file changed, 26 insertions(+), 14 deletions(-) diff --git a/test/test_scripts_run_task.py b/test/test_scripts_run_task.py index 59139ed28..037580441 100644 --- a/test/test_scripts_run_task.py +++ b/test/test_scripts_run_task.py @@ -348,22 +348,33 @@ def _commit_file(message, filename, path): return git_current_rev(path) with tempfile.TemporaryDirectory() as repo: - # Create the submodule repository. - submod_path = os.path.join(str(repo), "submodule") - _create_empty_repo(submod_path) - _commit_file("Submodule content", "content", submod_path) + # Create the submodule repositories + sm1_path = os.path.join(str(repo), "submodule1") + _create_empty_repo(sm1_path) + _commit_file("The first submodule", "first", sm1_path) + + sm2_path = os.path.join(str(repo), "submodule2") + _create_empty_repo(sm2_path) + _commit_file("The second submodule", "second", sm2_path) # Create the testing repository. repo_path = os.path.join(str(repo), "repository") _create_empty_repo(repo_path) - # Add submodule (to main branch) + # Add submodules (to main branch) + subprocess.check_call( + ["git", "-c", "protocol.file.allow=always", + "submodule", "add", f"file://{os.path.abspath(sm1_path)}", "sm1"], + cwd=repo_path + ) subprocess.check_call( ["git", "-c", "protocol.file.allow=always", - "submodule", "add", f"file://{os.path.abspath(submod_path)}", "submodule"], + "submodule", "add", f"file://{os.path.abspath(sm2_path)}", "sm2"], cwd=repo_path ) - subprocess.check_call(["git", "add", "submodule"], cwd=repo_path) + subprocess.check_call(["git", "add", "sm1"], cwd=repo_path) + subprocess.check_call(["git", "add", "sm2"], cwd=repo_path) + subprocess.check_call(["git", "commit", "-m", "Add submodules"], cwd=repo_path) # Commit mainfile (to main branch) main_commit = _commit_file("Initial commit", "mainfile", repo_path) @@ -388,15 +399,16 @@ def _commit_file(message, filename, path): ("main", "main", None, ["mainfile"], "main"), ("main", "mybranch", None, ["mainfile", "branchfile"], "branch"), # Same tests again - but with submodules. - (None, None, True, ["mainfile", "submodule/content"], "main"), - (None, "main", True, ["mainfile", "submodule/content"], "main"), - (None, "mybranch", True, ["mainfile", "branchfile", "submodule/content"], "branch"), - ("main", "main", True, ["mainfile", "submodule/content"], "main"), - ("main", "mybranch", True, ["mainfile", "branchfile", "submodule/content"], "branch"), + (None, None, True, ["mainfile", "sm1/first", "sm2/second"], "main"), + (None, "main", True, ["mainfile", "sm1/first", "sm2/second"], "main"), + (None, "mybranch", True, ["mainfile", "branchfile", "sm1/first", "sm2/second"], "branch"), + ("main", "main", True, ["mainfile", "sm1/first", "sm2/second"], "main"), + ("main", "mybranch", True, ["mainfile", "branchfile", "sm1/first", "sm2/second"], "branch"), # Tests for submodule matching rules. - (None, "main", "submodule", ["mainfile", "submodule/content"], "main"), + (None, "main", "sm1", ["mainfile", "sm1/first"], "main"), + (None, "main", "sm2", ["mainfile", "sm2/second"], "main"), (None, "main", "one:two:three", ["mainfile"], "main"), - (None, "main", "one:two:submodule", ["mainfile", "submodule/content"], "main"), + (None, "main", "one:two:sm1", ["mainfile", "sm1/first"], "main"), ], ) def test_git_checkout( From 89d7fe5b854e1163959cff6f206317b875b2a0c1 Mon Sep 17 00:00:00 2001 From: Naomi Kirby Date: Thu, 23 May 2024 18:34:51 -0700 Subject: [PATCH 4/9] Add tests to verify checkout behaviour in run_task transform --- test/test_transforms_run_run_task.py | 46 ++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/test/test_transforms_run_run_task.py b/test/test_transforms_run_run_task.py index 027732971..35cebe669 100644 --- a/test/test_transforms_run_run_task.py +++ b/test/test_transforms_run_run_task.py @@ -168,6 +168,31 @@ def assert_run_task_command_generic_worker(task): ] +def assert_no_checkouts(task): + assert task["worker"]["env"] == { + "MOZ_SCM_LEVEL": "1", + } + assert task["worker"]["command"] == [ + "/usr/local/bin/run-task", + "--", + "bash", + "-cx", + "echo hello world", + ] + +def assert_change_head(task): + assert task["worker"]["env"] == { + "CI_BASE_REPOSITORY": "http://hg.example.com", + "CI_HEAD_REF": "default", + "CI_HEAD_REPOSITORY": "http://hg.somewhere.com", + "CI_HEAD_REV": "an-awesome-branch", + "CI_REPOSITORY_TYPE": "hg", + "HG_STORE_PATH": "/builds/worker/checkouts/hg-store", + "MOZ_SCM_LEVEL": "1", + "REPOSITORIES": '{"ci": "Taskgraph"}', + "VCS_PATH": "/builds/worker/checkouts/vcs", + } + @pytest.mark.parametrize( "task", ( @@ -208,6 +233,27 @@ def assert_run_task_command_generic_worker(task): }, id="run_task_command_generic_worker", ), + pytest.param( + { + "run": { + "checkout": False, + }, + }, + id="no_checkouts", + ), + pytest.param( + { + "run": { + "checkout": { + "ci": { + "head_repository": "http://hg.somewhere.com", + "head_rev": "an-awesome-branch", + }, + }, + }, + }, + id="change_head", + ), ), ) def test_run_task(request, run_task_using, task): From 07caf67872ffb1c03545f9cade0327496bbccc8a Mon Sep 17 00:00:00 2001 From: Naomi Kirby Date: Thu, 23 May 2024 18:57:41 -0700 Subject: [PATCH 5/9] Add submodules to RepoConfig and handle them in base transforms --- src/taskgraph/transforms/base.py | 1 + src/taskgraph/transforms/run/common.py | 5 +++ test/test_transforms_run_run_task.py | 56 ++++++++++++++++++++++++++ 3 files changed, 62 insertions(+) diff --git a/src/taskgraph/transforms/base.py b/src/taskgraph/transforms/base.py index fda0c584f..90624986a 100644 --- a/src/taskgraph/transforms/base.py +++ b/src/taskgraph/transforms/base.py @@ -26,6 +26,7 @@ class RepoConfig: path: str = "" head_rev: Union[str, None] = None ssh_secret_name: Union[str, None] = None + submodules: Union[bool, str, None] = None @dataclass(frozen=True, eq=False) diff --git a/src/taskgraph/transforms/run/common.py b/src/taskgraph/transforms/run/common.py index 66466bc5f..2c80a5407 100644 --- a/src/taskgraph/transforms/run/common.py +++ b/src/taskgraph/transforms/run/common.py @@ -143,6 +143,10 @@ def support_vcs_checkout(config, task, taskdesc, repo_configs, sparse=False): } ) for repo_config in repo_configs.values(): + repo_submods = repo_config.submodules + if not isinstance(repo_config.submodules, str): + repo_submods = "auto" if repo_config.submodules else None + env.update( { f"{repo_config.prefix.upper()}_{key}": value @@ -153,6 +157,7 @@ def support_vcs_checkout(config, task, taskdesc, repo_configs, sparse=False): "HEAD_REF": repo_config.head_ref, "REPOSITORY_TYPE": repo_config.type, "SSH_SECRET_NAME": repo_config.ssh_secret_name, + "SUBMODULES": repo_submods }.items() if value is not None } diff --git a/test/test_transforms_run_run_task.py b/test/test_transforms_run_run_task.py index 35cebe669..e497c3af3 100644 --- a/test/test_transforms_run_run_task.py +++ b/test/test_transforms_run_run_task.py @@ -180,6 +180,37 @@ def assert_no_checkouts(task): "echo hello world", ] + +def assert_with_all_submodules(task): + assert task["worker"]["env"] == { + "CI_BASE_REPOSITORY": "http://hg.example.com", + "CI_HEAD_REF": "default", + "CI_HEAD_REPOSITORY": "http://hg.example.com", + "CI_HEAD_REV": "abcdef", + "CI_REPOSITORY_TYPE": "hg", + "CI_SUBMODULES": "auto", + "HG_STORE_PATH": "/builds/worker/checkouts/hg-store", + "MOZ_SCM_LEVEL": "1", + "REPOSITORIES": '{"ci": "Taskgraph"}', + "VCS_PATH": "/builds/worker/checkouts/vcs", + } + + +def assert_with_one_submodule(task): + assert task["worker"]["env"] == { + "CI_BASE_REPOSITORY": "http://hg.example.com", + "CI_HEAD_REF": "default", + "CI_HEAD_REPOSITORY": "http://hg.example.com", + "CI_HEAD_REV": "abcdef", + "CI_REPOSITORY_TYPE": "hg", + "CI_SUBMODULES": "xyz", + "HG_STORE_PATH": "/builds/worker/checkouts/hg-store", + "MOZ_SCM_LEVEL": "1", + "REPOSITORIES": '{"ci": "Taskgraph"}', + "VCS_PATH": "/builds/worker/checkouts/vcs", + } + + def assert_change_head(task): assert task["worker"]["env"] == { "CI_BASE_REPOSITORY": "http://hg.example.com", @@ -193,6 +224,7 @@ def assert_change_head(task): "VCS_PATH": "/builds/worker/checkouts/vcs", } + @pytest.mark.parametrize( "task", ( @@ -241,6 +273,30 @@ def assert_change_head(task): }, id="no_checkouts", ), + pytest.param( + { + "run": { + "checkout": { + "ci": { + "submodules": True, + } + }, + }, + }, + id="with_all_submodules", + ), + pytest.param( + { + "run": { + "checkout": { + "ci": { + "submodules": "xyz", + } + }, + }, + }, + id="with_one_submodule", + ), pytest.param( { "run": { From a9c38498a9d0e32fb01c297ae610cf959ff10987 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Fri, 24 May 2024 02:44:04 +0000 Subject: [PATCH 6/9] style: pre-commit.ci auto fixes [...] --- src/taskgraph/transforms/run/common.py | 2 +- test/test_scripts_run_task.py | 56 +++++++++++++++++++------- 2 files changed, 43 insertions(+), 15 deletions(-) diff --git a/src/taskgraph/transforms/run/common.py b/src/taskgraph/transforms/run/common.py index 2c80a5407..9b90ce3bf 100644 --- a/src/taskgraph/transforms/run/common.py +++ b/src/taskgraph/transforms/run/common.py @@ -157,7 +157,7 @@ def support_vcs_checkout(config, task, taskdesc, repo_configs, sparse=False): "HEAD_REF": repo_config.head_ref, "REPOSITORY_TYPE": repo_config.type, "SSH_SECRET_NAME": repo_config.ssh_secret_name, - "SUBMODULES": repo_submods + "SUBMODULES": repo_submods, }.items() if value is not None } diff --git a/test/test_scripts_run_task.py b/test/test_scripts_run_task.py index 037580441..15cc86e09 100644 --- a/test/test_scripts_run_task.py +++ b/test/test_scripts_run_task.py @@ -317,28 +317,30 @@ def git_current_rev(cwd): universal_newlines=True, ).strip() + @pytest.fixture() def mock_homedir(monkeypatch): "Mock home directory to isolate git config changes" with tempfile.TemporaryDirectory() as fakehome: env = os.environ.copy() - env['HOME'] = str(fakehome) + env["HOME"] = str(fakehome) monkeypatch.setattr(os, "environ", env) # Enable the git file protocol for testing. subprocess.check_call( - ["git", "config", "--global", "protocol.file.allow", "always"], env=env) + ["git", "config", "--global", "protocol.file.allow", "always"], env=env + ) yield str(fakehome) + @pytest.fixture(scope="session") # Tests shouldn't change this repo def mock_git_repo(): "Mock repository with files, commits and branches for using as source" + def _create_empty_repo(path): subprocess.check_call(["git", "init", "-b", "main", path]) subprocess.check_call(["git", "config", "user.name", "pytest"], cwd=path) - subprocess.check_call( - ["git", "config", "user.email", "py@tes.t"], cwd=path - ) + subprocess.check_call(["git", "config", "user.email", "py@tes.t"], cwd=path) def _commit_file(message, filename, path): with open(os.path.join(path, filename), "w") as fout: @@ -346,7 +348,7 @@ def _commit_file(message, filename, path): subprocess.check_call(["git", "add", filename], cwd=path) subprocess.check_call(["git", "commit", "-m", message], cwd=path) return git_current_rev(path) - + with tempfile.TemporaryDirectory() as repo: # Create the submodule repositories sm1_path = os.path.join(str(repo), "submodule1") @@ -363,14 +365,28 @@ def _commit_file(message, filename, path): # Add submodules (to main branch) subprocess.check_call( - ["git", "-c", "protocol.file.allow=always", - "submodule", "add", f"file://{os.path.abspath(sm1_path)}", "sm1"], - cwd=repo_path + [ + "git", + "-c", + "protocol.file.allow=always", + "submodule", + "add", + f"file://{os.path.abspath(sm1_path)}", + "sm1", + ], + cwd=repo_path, ) subprocess.check_call( - ["git", "-c", "protocol.file.allow=always", - "submodule", "add", f"file://{os.path.abspath(sm2_path)}", "sm2"], - cwd=repo_path + [ + "git", + "-c", + "protocol.file.allow=always", + "submodule", + "add", + f"file://{os.path.abspath(sm2_path)}", + "sm2", + ], + cwd=repo_path, ) subprocess.check_call(["git", "add", "sm1"], cwd=repo_path) subprocess.check_call(["git", "add", "sm2"], cwd=repo_path) @@ -401,9 +417,21 @@ def _commit_file(message, filename, path): # Same tests again - but with submodules. (None, None, True, ["mainfile", "sm1/first", "sm2/second"], "main"), (None, "main", True, ["mainfile", "sm1/first", "sm2/second"], "main"), - (None, "mybranch", True, ["mainfile", "branchfile", "sm1/first", "sm2/second"], "branch"), + ( + None, + "mybranch", + True, + ["mainfile", "branchfile", "sm1/first", "sm2/second"], + "branch", + ), ("main", "main", True, ["mainfile", "sm1/first", "sm2/second"], "main"), - ("main", "mybranch", True, ["mainfile", "branchfile", "sm1/first", "sm2/second"], "branch"), + ( + "main", + "mybranch", + True, + ["mainfile", "branchfile", "sm1/first", "sm2/second"], + "branch", + ), # Tests for submodule matching rules. (None, "main", "sm1", ["mainfile", "sm1/first"], "main"), (None, "main", "sm2", ["mainfile", "sm2/second"], "main"), From 1a79daa7f6ee06e3e5fd107c04e182c0ed0165e5 Mon Sep 17 00:00:00 2001 From: Naomi Kirby Date: Thu, 23 May 2024 19:39:37 -0700 Subject: [PATCH 7/9] Task description submodules as a list instead of delimiting --- src/taskgraph/transforms/base.py | 2 +- src/taskgraph/transforms/run/common.py | 5 +++-- test/test_transforms_run_run_task.py | 31 ++++++++++++++++++++++++-- 3 files changed, 33 insertions(+), 5 deletions(-) diff --git a/src/taskgraph/transforms/base.py b/src/taskgraph/transforms/base.py index 90624986a..82205b8ca 100644 --- a/src/taskgraph/transforms/base.py +++ b/src/taskgraph/transforms/base.py @@ -26,7 +26,7 @@ class RepoConfig: path: str = "" head_rev: Union[str, None] = None ssh_secret_name: Union[str, None] = None - submodules: Union[bool, str, None] = None + submodules: Union[bool, List[str], None] = None @dataclass(frozen=True, eq=False) diff --git a/src/taskgraph/transforms/run/common.py b/src/taskgraph/transforms/run/common.py index 9b90ce3bf..48873e560 100644 --- a/src/taskgraph/transforms/run/common.py +++ b/src/taskgraph/transforms/run/common.py @@ -143,8 +143,9 @@ def support_vcs_checkout(config, task, taskdesc, repo_configs, sparse=False): } ) for repo_config in repo_configs.values(): - repo_submods = repo_config.submodules - if not isinstance(repo_config.submodules, str): + if isinstance(repo_config.submodules, list): + repo_submods = ':'.join(repo_config.submodules) + else: repo_submods = "auto" if repo_config.submodules else None env.update( diff --git a/test/test_transforms_run_run_task.py b/test/test_transforms_run_run_task.py index e497c3af3..2a963b7fa 100644 --- a/test/test_transforms_run_run_task.py +++ b/test/test_transforms_run_run_task.py @@ -203,7 +203,22 @@ def assert_with_one_submodule(task): "CI_HEAD_REPOSITORY": "http://hg.example.com", "CI_HEAD_REV": "abcdef", "CI_REPOSITORY_TYPE": "hg", - "CI_SUBMODULES": "xyz", + "CI_SUBMODULES": "apple", + "HG_STORE_PATH": "/builds/worker/checkouts/hg-store", + "MOZ_SCM_LEVEL": "1", + "REPOSITORIES": '{"ci": "Taskgraph"}', + "VCS_PATH": "/builds/worker/checkouts/vcs", + } + + +def assert_with_two_submodules(task): + assert task["worker"]["env"] == { + "CI_BASE_REPOSITORY": "http://hg.example.com", + "CI_HEAD_REF": "default", + "CI_HEAD_REPOSITORY": "http://hg.example.com", + "CI_HEAD_REV": "abcdef", + "CI_REPOSITORY_TYPE": "hg", + "CI_SUBMODULES": "orange:banana", "HG_STORE_PATH": "/builds/worker/checkouts/hg-store", "MOZ_SCM_LEVEL": "1", "REPOSITORIES": '{"ci": "Taskgraph"}', @@ -290,13 +305,25 @@ def assert_change_head(task): "run": { "checkout": { "ci": { - "submodules": "xyz", + "submodules": ["apple"], } }, }, }, id="with_one_submodule", ), + pytest.param( + { + "run": { + "checkout": { + "ci": { + "submodules": ["orange", "banana"], + } + }, + }, + }, + id="with_two_submodules", + ), pytest.param( { "run": { From 018b4dfa54e39f522f5707e10cce75d49fd1235f Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Fri, 24 May 2024 02:51:21 +0000 Subject: [PATCH 8/9] style: pre-commit.ci auto fixes [...] --- src/taskgraph/transforms/run/common.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/taskgraph/transforms/run/common.py b/src/taskgraph/transforms/run/common.py index 48873e560..7802b1062 100644 --- a/src/taskgraph/transforms/run/common.py +++ b/src/taskgraph/transforms/run/common.py @@ -144,7 +144,7 @@ def support_vcs_checkout(config, task, taskdesc, repo_configs, sparse=False): ) for repo_config in repo_configs.values(): if isinstance(repo_config.submodules, list): - repo_submods = ':'.join(repo_config.submodules) + repo_submods = ":".join(repo_config.submodules) else: repo_submods = "auto" if repo_config.submodules else None From 3612b26467ee222651d430e4bae8ab8ea3709def Mon Sep 17 00:00:00 2001 From: Naomi Kirby Date: Fri, 24 May 2024 10:51:11 -0700 Subject: [PATCH 9/9] Fix type annotations for python <= 3.9 --- src/taskgraph/run-task/run-task | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/taskgraph/run-task/run-task b/src/taskgraph/run-task/run-task index 622ec8f28..d07122032 100755 --- a/src/taskgraph/run-task/run-task +++ b/src/taskgraph/run-task/run-task @@ -37,7 +37,7 @@ import urllib.error import urllib.request from pathlib import Path from threading import Thread -from typing import Optional +from typing import Optional, Union SECRET_BASEURL_TPL = "http://taskcluster/secrets/v1/secret/{}" @@ -598,7 +598,7 @@ def git_checkout( commit: Optional[str], ssh_key_file: Optional[Path], ssh_known_hosts_file: Optional[Path], - submodules: Optional[str | bool], + submodules: Union[str, bool, None], ): env = { # abort if transfer speed is lower than 1kB/s for 1 minute