diff options
author | Klaus Aehlig <klaus.aehlig@huawei.com> | 2024-02-20 12:00:57 +0100 |
---|---|---|
committer | Klaus Aehlig <klaus.aehlig@huawei.com> | 2024-02-20 17:18:10 +0100 |
commit | a7cc305a3a6e2886a4f7ec7b8fd9943ff45f286d (patch) | |
tree | 544d491293fae297478f959ad84be731f469b02d | |
parent | ca54778751856d77f7da2dba051c473a488f7d1e (diff) | |
download | justbuild-a7cc305a3a6e2886a4f7ec7b8fd9943ff45f286d.tar.gz |
git repo fetch: support "inherit env"
When fetching git repositories, just-mr routinely shells out to
git. In this case, allow the user to specify via "inherit env",
which environment variables from the host environment should be
made available in this action. Typical variables to inherit are
ones providing credentials, like SSH_AUTH_SOCK. As the repository
description specifies the commit that will be taken, and hence the
resulting tree, correctness is not affected by the environement
leaking in here.
-rw-r--r-- | share/man/just-mr-repository-config.5.md | 4 | ||||
-rw-r--r-- | src/other_tools/git_operations/git_repo_remote.cpp | 24 | ||||
-rw-r--r-- | src/other_tools/git_operations/git_repo_remote.hpp | 16 | ||||
-rw-r--r-- | src/other_tools/just_mr/update.cpp | 32 | ||||
-rw-r--r-- | src/other_tools/ops_maps/git_tree_fetch_map.cpp | 1 | ||||
-rw-r--r-- | src/other_tools/ops_maps/git_update_map.cpp | 14 | ||||
-rw-r--r-- | src/other_tools/ops_maps/git_update_map.hpp | 25 | ||||
-rw-r--r-- | src/other_tools/ops_maps/import_to_git_map.cpp | 1 | ||||
-rw-r--r-- | src/other_tools/repo_map/repos_to_setup_map.cpp | 23 | ||||
-rw-r--r-- | src/other_tools/root_maps/commit_git_map.cpp | 5 | ||||
-rw-r--r-- | src/other_tools/root_maps/commit_git_map.hpp | 1 | ||||
-rw-r--r-- | test/other_tools/git_operations/git_repo_remote.test.cpp | 7 |
12 files changed, 122 insertions, 31 deletions
diff --git a/share/man/just-mr-repository-config.5.md b/share/man/just-mr-repository-config.5.md index dcdee02f..169e9a1d 100644 --- a/share/man/just-mr-repository-config.5.md +++ b/share/man/just-mr-repository-config.5.md @@ -98,6 +98,10 @@ The following fields are supported: files. This entry is optional. If missing, the root directory of the Git repository is used. + - *`"inherit env"`* provides a list of variables. When `just-mr` + shells out to `git`, those variables are inherited from the + environment `just-mr` is called within, if set there. + ### *`"git tree"`* It defines as workspace root as a fixed `git` tree, given by the diff --git a/src/other_tools/git_operations/git_repo_remote.cpp b/src/other_tools/git_operations/git_repo_remote.cpp index cb2ff7a0..ac4bc41c 100644 --- a/src/other_tools/git_operations/git_repo_remote.cpp +++ b/src/other_tools/git_operations/git_repo_remote.cpp @@ -395,6 +395,7 @@ auto GitRepoRemote::UpdateCommitViaTmpRepo( std::filesystem::path const& tmp_dir, std::string const& repo_url, std::string const& branch, + std::vector<std::string> const& inherit_env, std::string const& git_bin, std::vector<std::string> const& launcher, anon_logger_ptr const& logger) const noexcept @@ -442,11 +443,18 @@ auto GitRepoRemote::UpdateCommitViaTmpRepo( "Git commit update for remote {} must shell out. Running:\n{}", repo_url, nlohmann::json(cmdline).dump()); + std::map<std::string, std::string> env{}; + for (auto const& k : inherit_env) { + const char* v = std::getenv(k.c_str()); + if (v != nullptr) { + env[k] = std::string(v); + } + } // set up the system command SystemCommand system{repo_url}; auto const command_output = system.Execute(cmdline, - {}, // default env + env, GetGitPath(), // which path is not actually relevant tmp_dir); // output file can be read anyway @@ -510,6 +518,7 @@ auto GitRepoRemote::UpdateCommitViaTmpRepo( auto GitRepoRemote::FetchViaTmpRepo(std::filesystem::path const& tmp_dir, std::string const& repo_url, std::optional<std::string> const& branch, + std::vector<std::string> const& inherit_env, std::string const& git_bin, std::vector<std::string> const& launcher, anon_logger_ptr const& logger) noexcept @@ -580,12 +589,17 @@ auto GitRepoRemote::FetchViaTmpRepo(std::filesystem::path const& tmp_dir, "Git fetch for remote {} must shell out. Running:\n{}", repo_url, nlohmann::json(cmdline).dump()); + std::map<std::string, std::string> env{}; + for (auto const& k : inherit_env) { + const char* v = std::getenv(k.c_str()); + if (v != nullptr) { + env[k] = std::string(v); + } + } // run command SystemCommand system{repo_url}; - auto const command_output = system.Execute(cmdline, - {}, // caller env - GetGitPath(), - tmp_dir); + auto const command_output = + system.Execute(cmdline, env, GetGitPath(), tmp_dir); if (not command_output) { std::string out_str{}; std::string err_str{}; diff --git a/src/other_tools/git_operations/git_repo_remote.hpp b/src/other_tools/git_operations/git_repo_remote.hpp index ce818de6..614e7822 100644 --- a/src/other_tools/git_operations/git_repo_remote.hpp +++ b/src/other_tools/git_operations/git_repo_remote.hpp @@ -93,6 +93,7 @@ class GitRepoRemote : public GitRepo { std::filesystem::path const& tmp_dir, std::string const& repo_url, std::string const& branch, + std::vector<std::string> const& inherit_env, std::string const& git_bin, std::vector<std::string> const& launcher, anon_logger_ptr const& logger) const noexcept @@ -108,13 +109,14 @@ class GitRepoRemote : public GitRepo { /// Uses either a given branch, or fetches all (with base refspecs). /// Returns a success flag. /// It guarantees the logger is called exactly once with fatal if failure. - [[nodiscard]] auto FetchViaTmpRepo(std::filesystem::path const& tmp_dir, - std::string const& repo_url, - std::optional<std::string> const& branch, - std::string const& git_bin, - std::vector<std::string> const& launcher, - anon_logger_ptr const& logger) noexcept - -> bool; + [[nodiscard]] auto FetchViaTmpRepo( + std::filesystem::path const& tmp_dir, + std::string const& repo_url, + std::optional<std::string> const& branch, + std::vector<std::string> const& inherit_env, + std::string const& git_bin, + std::vector<std::string> const& launcher, + anon_logger_ptr const& logger) noexcept -> bool; private: /// \brief Open "fake" repository wrapper for existing CAS. diff --git a/src/other_tools/just_mr/update.cpp b/src/other_tools/just_mr/update.cpp index 885f5421..a768e0e0 100644 --- a/src/other_tools/just_mr/update.cpp +++ b/src/other_tools/just_mr/update.cpp @@ -56,7 +56,7 @@ auto MultiRepoUpdate(std::shared_ptr<Configuration> const& config, return kExitUpdateError; } // gather repos to update - std::vector<std::pair<std::string, std::string>> repos_to_update{}; + std::vector<RepoDescriptionForUpdating> repos_to_update{}; repos_to_update.reserve(update_args.repos_to_update.size()); for (auto const& repo_name : update_args.repos_to_update) { auto repo_desc_parent = repos->At(repo_name); @@ -142,9 +142,33 @@ auto MultiRepoUpdate(std::shared_ptr<Configuration> const& config, nlohmann::json(repo_name).dump()); return kExitUpdateError; } - repos_to_update.emplace_back( - std::make_pair(repo_desc_repository->get()->String(), - repo_desc_branch->get()->String())); + std::vector<std::string> inherit_env{}; + auto repo_desc_inherit_env = + (*resolved_repo_desc) + ->Get("inherit env", Expression::kEmptyList); + if (not repo_desc_inherit_env->IsList()) { + Logger::Log(LogLevel::Error, + "GitCheckout: optional field \"inherit env\" " + "should be a list of strings, but found {}", + repo_desc_inherit_env->ToString()); + return kExitUpdateError; + } + for (auto const& var : repo_desc_inherit_env->List()) { + if (not var->IsString()) { + Logger::Log( + LogLevel::Error, + "GitCheckout: optional field \"inherit env\" " + "should be a list of strings, but found entry {}", + var->ToString()); + return kExitUpdateError; + } + inherit_env.emplace_back(var->String()); + } + + repos_to_update.emplace_back(RepoDescriptionForUpdating{ + .repo = repo_desc_repository->get()->String(), + .branch = repo_desc_branch->get()->String(), + .inherit_env = inherit_env}); } else { Logger::Log(LogLevel::Error, diff --git a/src/other_tools/ops_maps/git_tree_fetch_map.cpp b/src/other_tools/ops_maps/git_tree_fetch_map.cpp index 3dbb557a..ac76155b 100644 --- a/src/other_tools/ops_maps/git_tree_fetch_map.cpp +++ b/src/other_tools/ops_maps/git_tree_fetch_map.cpp @@ -423,6 +423,7 @@ auto CreateGitTreeFetchMap( tmp_dir->GetPath(), target_path.string(), std::nullopt, + key.inherit_env, git_bin, launcher, wrapped_logger)) { diff --git a/src/other_tools/ops_maps/git_update_map.cpp b/src/other_tools/ops_maps/git_update_map.cpp index 157d13bb..0ce9de49 100644 --- a/src/other_tools/ops_maps/git_update_map.cpp +++ b/src/other_tools/ops_maps/git_update_map.cpp @@ -35,7 +35,7 @@ auto CreateGitUpdateMap(GitCASPtr const& git_cas, if (not git_repo) { (*logger)( fmt::format("Failed to open tmp Git repository for remote {}", - key.first), + key.repo), /*fatal=*/true); return; } @@ -43,7 +43,7 @@ auto CreateGitUpdateMap(GitCASPtr const& git_cas, if (not tmp_dir) { (*logger)(fmt::format("Failed to create commit update tmp dir for " "remote {}", - key.first), + key.repo), /*fatal=*/true); return; } @@ -55,11 +55,12 @@ auto CreateGitUpdateMap(GitCASPtr const& git_cas, fatal); }); // update commit - auto id = fmt::format("{}:{}", key.first, key.second); + auto id = fmt::format("{}:{}", key.repo, key.branch); JustMRProgress::Instance().TaskTracker().Start(id); auto new_commit = git_repo->UpdateCommitViaTmpRepo(tmp_dir->GetPath(), - key.first, - key.second, + key.repo, + key.branch, + key.inherit_env, git_bin, launcher, wrapped_logger); @@ -70,5 +71,6 @@ auto CreateGitUpdateMap(GitCASPtr const& git_cas, JustMRStatistics::Instance().IncrementExecutedCounter(); (*setter)(new_commit->c_str()); }; - return AsyncMapConsumer<StringPair, std::string>(update_commits, jobs); + return AsyncMapConsumer<RepoDescriptionForUpdating, std::string>( + update_commits, jobs); } diff --git a/src/other_tools/ops_maps/git_update_map.hpp b/src/other_tools/ops_maps/git_update_map.hpp index 1978069a..e1aac2e3 100644 --- a/src/other_tools/ops_maps/git_update_map.hpp +++ b/src/other_tools/ops_maps/git_update_map.hpp @@ -23,19 +23,28 @@ #include "src/other_tools/git_operations/git_repo_remote.hpp" #include "src/utils/cpp/hash_combine.hpp" -using StringPair = std::pair<std::string, std::string>; +struct RepoDescriptionForUpdating { + std::string repo{}; + std::string branch{}; + std::vector<std::string> inherit_env{}; /*non-key!*/ + + [[nodiscard]] auto operator==(const RepoDescriptionForUpdating& other) const + -> bool { + return repo == other.repo and branch == other.branch; + } +}; /// \brief Maps a pair of repository url and branch to an updated commit hash. -using GitUpdateMap = AsyncMapConsumer<StringPair, std::string>; +using GitUpdateMap = AsyncMapConsumer<RepoDescriptionForUpdating, std::string>; namespace std { template <> -struct hash<StringPair> { - [[nodiscard]] auto operator()(StringPair const& ct) const noexcept - -> std::size_t { +struct hash<RepoDescriptionForUpdating> { + [[nodiscard]] auto operator()( + RepoDescriptionForUpdating const& ct) const noexcept -> std::size_t { size_t seed{}; - hash_combine<std::string>(&seed, ct.first); - hash_combine<std::string>(&seed, ct.second); + hash_combine<std::string>(&seed, ct.repo); + hash_combine<std::string>(&seed, ct.branch); return seed; } }; @@ -46,4 +55,4 @@ struct hash<StringPair> { std::vector<std::string> const& launcher, std::size_t jobs) -> GitUpdateMap; -#endif // INCLUDED_SRC_OTHER_TOOLS_OPS_MAPS_GIT_UPDATE_MAP_HPP
\ No newline at end of file +#endif // INCLUDED_SRC_OTHER_TOOLS_OPS_MAPS_GIT_UPDATE_MAP_HPP diff --git a/src/other_tools/ops_maps/import_to_git_map.cpp b/src/other_tools/ops_maps/import_to_git_map.cpp index 1de1ef15..4051cb01 100644 --- a/src/other_tools/ops_maps/import_to_git_map.cpp +++ b/src/other_tools/ops_maps/import_to_git_map.cpp @@ -193,6 +193,7 @@ auto CreateImportToGitMap( tmp_dir->GetPath(), target_path.string(), std::nullopt, + std::vector<std::string>{} /* XXX */, git_bin, launcher, wrapped_logger)) { diff --git a/src/other_tools/repo_map/repos_to_setup_map.cpp b/src/other_tools/repo_map/repos_to_setup_map.cpp index 3bad60f4..aec57868 100644 --- a/src/other_tools/repo_map/repos_to_setup_map.cpp +++ b/src/other_tools/repo_map/repos_to_setup_map.cpp @@ -137,12 +137,35 @@ void GitCheckout(ExpressionPtr const& repo_desc, auto pragma_absent_value = pragma_absent and pragma_absent->get()->IsBool() and pragma_absent->get()->Bool(); + std::vector<std::string> inherit_env{}; + auto repo_desc_inherit_env = + repo_desc->Get("inherit env", Expression::kEmptyList); + if (not repo_desc_inherit_env->IsList()) { + (*logger)(fmt::format("GitCheckout: optional field \"inherit env\" " + "should be a list of strings, but found {}", + repo_desc_inherit_env->ToString()), + true); + return; + } + for (auto const& var : repo_desc_inherit_env->List()) { + if (not var->IsString()) { + (*logger)( + fmt::format("GitCheckout: optional field \"inherit env\" " + "should be a list of strings, but found entry {}", + var->ToString()), + true); + return; + } + inherit_env.emplace_back(var->String()); + } + // populate struct GitRepoInfo git_repo_info = { .hash = repo_desc_commit->get()->String(), .repo_url = repo_desc_repository->get()->String(), .branch = repo_desc_branch->get()->String(), .subdir = subdir.empty() ? "." : subdir.string(), + .inherit_env = inherit_env, .mirrors = std::move(mirrors), .origin = repo_name, .ignore_special = pragma_special_value == PragmaSpecial::Ignore, diff --git a/src/other_tools/root_maps/commit_git_map.cpp b/src/other_tools/root_maps/commit_git_map.cpp index 08680531..3cf53cb4 100644 --- a/src/other_tools/root_maps/commit_git_map.cpp +++ b/src/other_tools/root_maps/commit_git_map.cpp @@ -511,6 +511,7 @@ void EnsureCommit( if (git_repo->FetchViaTmpRepo(tmp_dir->GetPath(), mirror, repo_info.branch, + repo_info.inherit_env, git_bin, launcher, wrapped_logger)) { @@ -543,6 +544,7 @@ void EnsureCommit( if (git_repo->FetchViaTmpRepo(tmp_dir->GetPath(), *preferred_url, repo_info.branch, + repo_info.inherit_env, git_bin, launcher, wrapped_logger)) { @@ -575,6 +577,7 @@ void EnsureCommit( if (git_repo->FetchViaTmpRepo(tmp_dir->GetPath(), fetch_repo, repo_info.branch, + repo_info.inherit_env, git_bin, launcher, wrapped_logger)) { @@ -610,6 +613,7 @@ void EnsureCommit( tmp_dir->GetPath(), *preferred_mirror, repo_info.branch, + repo_info.inherit_env, git_bin, launcher, wrapped_logger)) { @@ -645,6 +649,7 @@ void EnsureCommit( if (git_repo->FetchViaTmpRepo(tmp_dir->GetPath(), mirror, repo_info.branch, + repo_info.inherit_env, git_bin, launcher, wrapped_logger)) { diff --git a/src/other_tools/root_maps/commit_git_map.hpp b/src/other_tools/root_maps/commit_git_map.hpp index fd1530af..423393fb 100644 --- a/src/other_tools/root_maps/commit_git_map.hpp +++ b/src/other_tools/root_maps/commit_git_map.hpp @@ -35,6 +35,7 @@ struct GitRepoInfo { std::string repo_url{}; std::string branch{}; std::string subdir{}; /* key */ + std::vector<std::string> inherit_env{}; std::vector<std::string> mirrors{}; // name of repository for which work is done; used in progress reporting std::string origin{}; diff --git a/test/other_tools/git_operations/git_repo_remote.test.cpp b/test/other_tools/git_operations/git_repo_remote.test.cpp index 37be3737..03df8553 100644 --- a/test/other_tools/git_operations/git_repo_remote.test.cpp +++ b/test/other_tools/git_operations/git_repo_remote.test.cpp @@ -247,6 +247,7 @@ TEST_CASE("Single-threaded fake repository operations", "[git_repo_remote]") { REQUIRE(repo_fetch_all->FetchViaTmpRepo(tmp_path_fetch_all, *repo_path, std::nullopt, + {}, "git", {}, logger)); @@ -275,6 +276,7 @@ TEST_CASE("Single-threaded fake repository operations", "[git_repo_remote]") { repo_fetch_wRefspec->FetchViaTmpRepo(tmp_path_fetch_wRefspec, *repo_path, "master", + {}, "git", {}, logger)); @@ -295,7 +297,7 @@ TEST_CASE("Single-threaded fake repository operations", "[git_repo_remote]") { REQUIRE(FileSystemManager::CreateDirectory(tmp_path_commit_upd)); // do remote ls auto fetched_commit = repo_commit_upd->UpdateCommitViaTmpRepo( - tmp_path_commit_upd, *repo_path, "master", "git", {}, logger); + tmp_path_commit_upd, *repo_path, "master", {}, "git", {}, logger); REQUIRE(fetched_commit); CHECK(*fetched_commit == kRootCommit); @@ -363,6 +365,7 @@ TEST_CASE("Multi-threaded fake repository operations", "[git_repo_remote]") { target_repo->FetchViaTmpRepo(tmp_path_fetch_all, *remote_repo_path, std::nullopt, + {}, "git", {}, logger)); @@ -378,6 +381,7 @@ TEST_CASE("Multi-threaded fake repository operations", "[git_repo_remote]") { tmp_path_fetch_wRefspec, *remote_repo_path, "master", + {}, "git", {}, logger)); @@ -393,6 +397,7 @@ TEST_CASE("Multi-threaded fake repository operations", "[git_repo_remote]") { tmp_path_commit_upd, *remote_repo_path, "master", + {}, "git", {}, logger); |