aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorKaren Arutyunov <karen@codesynthesis.com>2020-09-22 22:18:18 +0300
committerKaren Arutyunov <karen@codesynthesis.com>2020-09-29 15:01:07 +0300
commitb937ec2bf461ec06bf601e854f694e86060eba59 (patch)
tree133cbb173399bddce1b35041cccd1ec67cd46deb
parent39f6d06c729fc1b1ffdeda67daa0ddc1d6baadb0 (diff)
Use system git rather than bundled one for some operations on Windows
-rw-r--r--bdep/ci.cxx4
-rw-r--r--bdep/git.cxx134
-rw-r--r--bdep/git.hxx43
-rw-r--r--bdep/git.ixx6
-rw-r--r--bdep/git.txx79
-rw-r--r--bdep/project-author.cxx1
-rw-r--r--bdep/publish.cxx30
-rw-r--r--bdep/release.cxx24
-rw-r--r--bdep/types.hxx1
9 files changed, 261 insertions, 61 deletions
diff --git a/bdep/ci.cxx b/bdep/ci.cxx
index 081529a..797725f 100644
--- a/bdep/ci.cxx
+++ b/bdep/ci.cxx
@@ -81,7 +81,11 @@ namespace bdep
<< s.branch << "'" <<
info << "run 'git push --set-upstream' to set";
+ // It's unlikely that the branch remote is configured globally, so we
+ // use the bundled git.
+ //
optional<string> rem (git_line (git_ver,
+ false /* system */,
prj,
false /* ignore_error */,
"config",
diff --git a/bdep/git.cxx b/bdep/git.cxx
index 1c672c7..1749efe 100644
--- a/bdep/git.cxx
+++ b/bdep/git.cxx
@@ -11,38 +11,139 @@ using namespace butl;
namespace bdep
{
- static optional<semantic_version> git_ver;
+ static pair<process_path, optional<string>> sys_git;
+ static optional<semantic_version> sys_git_ver;
+
+// On POSIX the system git is always assumed.
+//
+#ifndef _WIN32
+ static pair<process_path, optional<string>>& bun_git (sys_git);
+ static optional<semantic_version>& bun_git_ver (sys_git_ver);
+#else
+ static pair<process_path, optional<string>> bun_git;
+ static optional<semantic_version> bun_git_ver;
+#endif
// Check that git is at least of the specified minimum supported version.
//
void
- git_check_version (const semantic_version& min_ver)
+ git_check_version (const semantic_version& min_ver, bool system)
{
// Query and cache git version on the first call.
//
- if (!git_ver)
+ optional<semantic_version>& gv (system ? sys_git_ver : bun_git_ver);
+
+ if (!gv)
{
// Make sure that the getline() function call doesn't end up with an
// infinite recursion.
//
- git_ver = semantic_version ();
+ gv = semantic_version ();
- optional<string> s (git_line (*git_ver,
- false /* ignore_error */,
- "--version"));
+ optional<string> s (
+ git_line (*gv, system, false /* ignore_error */, "--version"));
- if (!s || !(git_ver = git_version (*s)))
+ if (!s || !(gv = git_version (*s)))
fail << "unable to obtain git version";
}
// Note that we don't expect the min_ver to contain the build component,
// that doesn't matter functionality-wise for git.
//
- if (*git_ver < min_ver)
- fail << "unsupported git version " << *git_ver <<
+ if (*gv < min_ver)
+ fail << "unsupported git version " << *gv <<
info << "minimum supported version is " << min_ver << endf;
}
+ // Return git process path and the --exec-path option, if it is required for
+ // the execution.
+ //
+ const pair<process_path, optional<string>>&
+ git_search (bool system)
+ {
+ tracer trace ("git_search");
+
+ // On the first call search for the git process path, calculate the
+ // --exec-path option value required for its execution, and cache them for
+ // subsequent use.
+ //
+ pair<process_path, optional<string>>& git (system ? sys_git : bun_git);
+
+ if (git.first.empty ())
+ {
+ // On startup git prepends the PATH environment variable value with the
+ // computed directory path where its sub-programs are supposedly located
+ // (--exec-path option, GIT_EXEC_PATH environment variable, etc; see
+ // cmd_main() in git's git.c for details).
+ //
+ // Then, when git needs to run itself or one of its components as a
+ // child process, it resolves the full executable path searching in
+ // directories listed in PATH (see locate_in_PATH() in git's
+ // run-command.c for details).
+ //
+ // On Windows we install git and its components into a place where it is
+ // not expected to be, which results in the wrong path in PATH as set by
+ // git (for example, c:/build2/libexec/git-core) which in turn may lead
+ // to running some other git that appears in the PATH variable. To
+ // prevent this we pass the git's exec directory via the --exec-path
+ // option explicitly.
+ //
+ process_path pp;
+
+#ifdef _WIN32
+
+ // Figure out the bundle absolute directory path based on our own
+ // process path and realize it for comparison. Though, we only need it
+ // when searching for the system git.
+ //
+ dir_path bd;
+
+ if (system)
+ {
+ // We should be able to construct/realize our process path so we don't
+ // handle invalid_path.
+ //
+ bd = path (process::path_search (argv0, true /* init */).
+ effect_string ()).directory ().realize ();
+
+ // Search in PATH first and fallback to the bundle directory.
+ //
+ pp = process::path_search ("git",
+ true /* init */,
+ bd,
+ true /* path_only */);
+ }
+ else
+#endif
+ pp = process::path_search ("git", true /* init */);
+
+ optional<string> ep;
+#ifdef _WIN32
+
+ // Note that we need to add --exec-path not only if the bundled git is
+ // requested but also if we ended up with the bundled git while
+ // searching for the system one.
+
+ // Realize for comparison.
+ //
+ // We should be able to realize the existing program parent directory
+ // path so we don't handle invalid_path.
+ //
+ dir_path d (pp.effect.directory ().realize ());
+
+ if (!system || d == bd)
+ ep = "--exec-path=" + d.string ();
+
+ l4 ([&]{trace << (system ? "system: '" : "bundled: '") << pp.effect
+ << "'" << (ep ? " " + *ep : "");});
+#endif
+
+ git = make_pair (move (pp), move (ep));
+ }
+
+ return git;
+ }
+
optional<string>
git_line (process&& pr, fdpipe&& pipe, bool ie, char delim)
{
@@ -93,9 +194,13 @@ namespace bdep
{
auto git_config = [&repo] (const char* name) -> optional<string>
{
+ // It's unlikely that the remote URL is configured globally, thus we use
+ // the bundled git.
+ //
return git_line (semantic_version {2, 1, 0},
+ false /* system */,
repo,
- true /* ignore_error */,
+ true /* ignore_error */,
"config",
"--get",
name);
@@ -259,10 +364,11 @@ namespace bdep
fdpipe pipe (open_pipe ()); // Text mode seems appropriate.
process pr (start_git (semantic_version {2, 11, 0},
+ false /* system */,
repo,
- 0 /* stdin */,
- pipe /* stdout */,
- 2 /* stderr */,
+ 0 /* stdin */,
+ pipe /* stdout */,
+ 2 /* stderr */,
"status",
"--porcelain=2",
"--branch"));
diff --git a/bdep/git.hxx b/bdep/git.hxx
index dcf3407..bfc5cd2 100644
--- a/bdep/git.hxx
+++ b/bdep/git.hxx
@@ -17,15 +17,30 @@ namespace bdep
// All functions that start git process take the minimum supported git
// version as an argument.
//
+ // They also take the system flag (only considered on Windows) that if true
+ // causes these functions to prefer the system git program (determined via
+ // PATH) over the bundled one (the bundled git is still used as a fallback).
+ // Normally, the caller should use the system git when operating on behalf
+ // of the user (commits, pushes, etc), so that the user interacts with the
+ // git program they expect. For the query requests (git repository status,
+ // etc) the bundled git should be used instead to avoid compatibility issues
+ // (e.g., symlink handling, etc; but when querying for potentially global
+ // configuration values such as author, system git should be used). Note
+ // that on POSIX the system git is used for everything.
+ //
// Start git process.
//
template <typename I, typename O, typename E, typename... A>
process
- start_git (const semantic_version&, I&& in, O&& out, E&& err, A&&... args);
+ start_git (const semantic_version&,
+ bool system,
+ I&& in, O&& out, E&& err,
+ A&&... args);
template <typename I, typename O, typename E, typename... A>
process
start_git (const semantic_version&,
+ bool system,
const dir_path& repo,
I&& in, O&& out, E&& err,
A&&... args);
@@ -33,11 +48,13 @@ namespace bdep
template <typename I, typename O, typename E, typename... A>
inline process
start_git (const semantic_version& min_ver,
+ bool system,
dir_path& repo,
I&& in, O&& out, E&& err,
A&&... args)
{
return start_git (min_ver,
+ system,
const_cast<const dir_path&> (repo),
forward<I> (in), forward<O> (out), forward<E> (err),
forward<A> (args)...);
@@ -53,15 +70,23 @@ namespace bdep
template <typename... A>
void
run_git (const semantic_version&,
+ bool system,
bool progress,
const dir_path& repo,
A&&... args);
template <typename... A>
inline void
- run_git (const semantic_version& min_ver, const dir_path& repo, A&&... args)
+ run_git (const semantic_version& min_ver,
+ bool system,
+ const dir_path& repo,
+ A&&... args)
{
- run_git (min_ver, true /* progress */, repo, forward<A> (args)...);
+ run_git (min_ver,
+ system,
+ true /* progress */,
+ repo,
+ forward<A> (args)...);
}
// Return the first line of the git output. If ignore_error is true, then
@@ -69,11 +94,15 @@ namespace bdep
//
template <typename... A>
optional<string>
- git_line (const semantic_version&, bool ignore_error, A&&... args);
+ git_line (const semantic_version&,
+ bool system,
+ bool ignore_error,
+ A&&... args);
template <typename... A>
optional<string>
git_line (const semantic_version&,
+ bool system,
const dir_path& repo,
bool ignore_error,
A&&... args);
@@ -88,11 +117,15 @@ namespace bdep
//
template <typename... A>
optional<string>
- git_string (const semantic_version&, bool ignore_error, A&&... args);
+ git_string (const semantic_version&,
+ bool system,
+ bool ignore_error,
+ A&&... args);
template <typename... A>
optional<string>
git_string (const semantic_version&,
+ bool system,
const dir_path& repo,
bool ignore_error,
A&&... args);
diff --git a/bdep/git.ixx b/bdep/git.ixx
index 8289544..2913cc6 100644
--- a/bdep/git.ixx
+++ b/bdep/git.ixx
@@ -6,11 +6,13 @@ namespace bdep
template <typename I, typename O, typename E, typename... A>
inline process
start_git (const semantic_version& min_ver,
+ bool system,
const dir_path& repo,
I&& in, O&& out, E&& err,
A&&... args)
{
return start_git (min_ver,
+ system,
forward<I> (in), forward<O> (out), forward<E> (err),
"-C", repo,
forward<A> (args)...);
@@ -25,11 +27,13 @@ namespace bdep
template <typename... A>
inline optional<string>
git_line (const semantic_version& min_ver,
+ bool system,
const dir_path& repo,
bool ie,
A&&... args)
{
return git_line (min_ver,
+ system,
ie,
"-C", repo,
forward<A> (args)...);
@@ -38,11 +42,13 @@ namespace bdep
template <typename... A>
inline optional<string>
git_string (const semantic_version& min_ver,
+ bool system,
const dir_path& repo,
bool ie,
A&&... args)
{
return git_string (min_ver,
+ system,
ie,
"-C", repo,
forward<A> (args)...);
diff --git a/bdep/git.txx b/bdep/git.txx
index 858c452..47705df 100644
--- a/bdep/git.txx
+++ b/bdep/git.txx
@@ -6,6 +6,7 @@ namespace bdep
template <typename... A>
void
run_git (const semantic_version& min_ver,
+ bool system,
bool progress,
const dir_path& repo,
A&&... args)
@@ -28,6 +29,7 @@ namespace bdep
// messages to stdout.
//
process pr (start_git (min_ver,
+ system,
repo,
0 /* stdin */,
err /* stdout */,
@@ -71,43 +73,38 @@ namespace bdep
}
void
- git_check_version (const semantic_version& min_ver);
+ git_check_version (const semantic_version& min_ver, bool system);
+
+ const pair<process_path, optional<string>>&
+ git_search (bool system);
template <typename I, typename O, typename E, typename... A>
process
start_git (const semantic_version& min_ver,
+ bool system,
I&& in, O&& out, E&& err,
A&&... args)
{
- git_check_version (min_ver);
+ git_check_version (min_ver, system);
try
{
- using namespace butl;
+ const pair<process_path, optional<string>>& git (git_search (system));
- // On startup git prepends the PATH environment variable value with the
- // computed directory path where its sub-programs are supposedly located
- // (--exec-path option, GIT_EXEC_PATH environment variable, etc; see
- // cmd_main() in git's git.c for details).
- //
- // Then, when git needs to run itself or one of its components as a
- // child process, it resolves the full executable path searching in
- // directories listed in PATH (see locate_in_PATH() in git's
- // run-command.c for details).
- //
- // On Windows we install git and its components into a place where it is
- // not expected to be, which results in the wrong path in PATH as set by
- // git (for example, c:/build2/libexec/git-core) which in turn may lead
- // to running some other git that appear in the PATH variable. To
- // prevent this we pass the git's exec directory via the --exec-path
- // option explicitly.
+ const optional<string>& ep (git.second); // --exec-path
+
+ // Note that if we are running the bundled git (on Windows) and there is
+ // no vi editor in PATH, then some commands (e.g. commit) may fail with
+ // the 'cannot spawn vi' error. To avoid this error, we will pass the
+ // EDITOR=notepad environment variable as a fallback for the git's
+ // editor search sequence (GIT_EDITOR, core.editor, VISUAL, EDITOR, vi),
+ // unless it is already set.
//
- string ep;
- process_path pp (process::path_search ("git", true /* init */));
+ const char* vars[] = {nullptr, nullptr};
+ process_env pe (git.first, vars);
-#ifdef _WIN32
- ep = "--exec-path=" + pp.effect.directory ().string ();
-#endif
+ if (ep && !getenv ("EDITOR"))
+ vars[0] = "EDITOR=notepad";
return process_start_callback (
[] (const char* const args[], size_t n)
@@ -116,8 +113,8 @@ namespace bdep
print_process (args, n);
},
forward<I> (in), forward<O> (out), forward<E> (err),
- pp,
- !ep.empty () ? ep.c_str () : nullptr,
+ pe,
+ ep,
forward<A> (args)...);
}
catch (const process_error& e)
@@ -128,12 +125,17 @@ namespace bdep
template <typename... A>
optional<string>
- git_line (const semantic_version& min_ver, bool ie, char delim, A&&... args)
+ git_line (const semantic_version& min_ver,
+ bool system,
+ bool ie,
+ char delim,
+ A&&... args)
{
fdpipe pipe (open_pipe ());
auto_fd null (ie ? open_null () : auto_fd ());
process pr (start_git (min_ver,
+ system,
0 /* stdin */,
pipe /* stdout */,
ie ? null.get () : 2 /* stderr */,
@@ -144,16 +146,30 @@ namespace bdep
template <typename... A>
inline optional<string>
- git_line (const semantic_version& min_ver, bool ie, A&&... args)
+ git_line (const semantic_version& min_ver,
+ bool system,
+ bool ie,
+ A&&... args)
{
- return git_line (min_ver, ie, '\n' /* delim */, forward<A> (args)...);
+ return git_line (min_ver,
+ system,
+ ie,
+ '\n' /* delim */,
+ forward<A> (args)...);
}
template <typename... A>
inline optional<string>
- git_string (const semantic_version& min_ver, bool ie, A&&... args)
+ git_string (const semantic_version& min_ver,
+ bool system,
+ bool ie,
+ A&&... args)
{
- return git_line (min_ver, ie, '\0' /* delim */, forward<A> (args)...);
+ return git_line (min_ver,
+ system,
+ ie,
+ '\0' /* delim */,
+ forward<A> (args)...);
}
template <typename... A>
@@ -184,6 +200,7 @@ namespace bdep
v.push_back ("-v");
run_git (semantic_version {2, 1, 0},
+ true /* system */,
progress,
repo,
"push",
diff --git a/bdep/project-author.cxx b/bdep/project-author.cxx
index 723dbe9..e4aae46 100644
--- a/bdep/project-author.cxx
+++ b/bdep/project-author.cxx
@@ -37,6 +37,7 @@ namespace bdep
// be queried with the GIT_AUTHOR_IDENT logical variable.
//
if (optional<string> l = git_line (semantic_version {2, 1, 0},
+ true /* system */,
prj,
true /* ignore_error */,
"var", "GIT_AUTHOR_IDENT"))
diff --git a/bdep/publish.cxx b/bdep/publish.cxx
index c97bdd5..e5e6f26 100644
--- a/bdep/publish.cxx
+++ b/bdep/publish.cxx
@@ -405,6 +405,7 @@ namespace bdep
auto_fd null (q ? open_null () : auto_fd ());
process pr (start_git (git_ver,
+ true /* system */,
prj,
0 /* stdin */,
q ? null.get () : 1 /* stdout */,
@@ -433,7 +434,13 @@ namespace bdep
auto worktree_prune = [&prj] ()
{
- run_git (git_ver, prj, "worktree", "prune", verb > 2 ? "-v" : nullptr);
+ run_git (git_ver,
+ true /* system */,
+ prj,
+ "worktree",
+ "prune",
+ verb > 2 ? "-v" :
+ nullptr);
};
// Create the build2-control branch if it doesn't exist, from scratch if
@@ -449,6 +456,7 @@ namespace bdep
// fetch command and re-try.
//
bool local_exists (git_line (git_ver,
+ false /* system */,
prj,
false /* ignore_error */,
"branch",
@@ -459,6 +467,7 @@ namespace bdep
// everywhere) via the --remote option or smth? Maybe later.
//
bool remote_exists (git_line (git_ver,
+ false /* system */,
prj,
false /* ignore_error */,
"branch",
@@ -492,10 +501,11 @@ namespace bdep
fdpipe pipe (open_pipe ());
process pr (start_git (git_ver,
+ true /* system */,
prj,
- null /* stdin */,
- pipe /* stdout */,
- 2 /* stderr */,
+ null /* stdin */,
+ pipe /* stdout */,
+ 2 /* stderr */,
"hash-object",
"-wt", "tree",
"--stdin"));
@@ -508,6 +518,7 @@ namespace bdep
// Create the (empty) root commit.
//
optional<string> commit (git_line (git_ver,
+ true /* system */,
prj,
false,
"commit-tree",
@@ -524,11 +535,12 @@ namespace bdep
// tighten things up a bit.
//
run_git (git_ver,
+ true /* system */,
prj,
"update-ref",
"refs/heads/build2-control",
*commit,
- "" /* oldvalue */);
+ "" /* oldvalue */);
// Checkout the branch. Note that the upstream branch is not setup
// for it yet. This will be done by the push operation.
@@ -547,6 +559,7 @@ namespace bdep
// branch.
//
run_git (git_ver,
+ true /* system */,
prj,
"branch",
verb < 2 ? "-q" : nullptr,
@@ -618,10 +631,11 @@ namespace bdep
// Note that fast-forwarding can potentially fail. That will mean the
// local branch has diverged from the remote one for some reason
// (e.g., inability to revert the commit, etc.). We again leave it to
- // the use to deal with.
+ // the user to deal with.
//
if (local_exists && remote_exists)
run_git (git_ver,
+ true /* system */,
wd,
"merge",
verb < 2 ? "-q" : verb > 2 ? "-v" : nullptr,
@@ -662,6 +676,7 @@ namespace bdep
}
run_git (git_ver,
+ true /* system */,
wd,
"add",
verb > 2 ? "-v" : nullptr,
@@ -700,6 +715,7 @@ namespace bdep
}
run_git (git_ver,
+ true /* system */,
wd,
"commit",
verb < 2 ? "-q" : verb > 2 ? "-v" : nullptr,
@@ -730,6 +746,7 @@ namespace bdep
worktree_remove (); // Release the branch before removal.
run_git (git_ver,
+ true /* system */,
prj,
"branch",
verb < 2 ? "-q" : nullptr,
@@ -738,6 +755,7 @@ namespace bdep
}
else
run_git (git_ver,
+ true /* system */,
wd,
"reset",
verb < 2 ? "-q" : nullptr,
diff --git a/bdep/release.cxx b/bdep/release.cxx
index 66b4a30..3f13427 100644
--- a/bdep/release.cxx
+++ b/bdep/release.cxx
@@ -934,10 +934,11 @@ namespace bdep
//
process pr (
start_git (git_ver,
+ true /* system */,
prj.path,
- 0 /* stdin */,
- 2 /* stdout */,
- 2 /* stderr */,
+ 0 /* stdin */,
+ 2 /* stdout */,
+ 2 /* stderr */,
"commit",
verb < 1 ? "-q" : verb >= 2 ? "-v" : nullptr,
amend ? "--amend" : nullptr,
@@ -1040,8 +1041,9 @@ namespace bdep
//
optional<string> ms (
git_string (git_ver,
+ false /* system */,
prj.path,
- false /* ignore_error */,
+ false /* ignore_error */,
"log",
"--format=%B",
"HEAD~" + to_string (o.squash ()) + "..HEAD"));
@@ -1062,6 +1064,7 @@ namespace bdep
// Squash commits, reverting them into the index.
//
run_git (git_ver,
+ true /* system */,
prj.path,
"reset",
verb < 1 ? "-q" : nullptr,
@@ -1085,6 +1088,7 @@ namespace bdep
if (e.normal ())
{
run_git (git_ver,
+ true /* system */,
prj.path,
"reset",
verb < 1 ? "-q" : nullptr,
@@ -1131,6 +1135,7 @@ namespace bdep
const optional<project::current_tag>& ct (prj.cur_tag);
run_git (git_ver,
+ true /* system */,
prj.path,
"tag",
(ct && ct->action == cmd_release_current_tag::update
@@ -1141,7 +1146,12 @@ namespace bdep
*prj.tag);
if (ct && ct->action == cmd_release_current_tag::remove)
- run_git (git_ver, prj.path, "tag", "--delete", ct->name);
+ run_git (git_ver,
+ true /* system */,
+ prj.path,
+ "tag",
+ "--delete",
+ ct->name);
}
// Open.
@@ -1197,7 +1207,11 @@ namespace bdep
string remote;
string brspec;
{
+ // It's unlikely that the branch remote is configured globally, so we
+ // use the bundled git.
+ //
optional<string> rem (git_line (git_ver,
+ false /* system */,
prj.path,
false /* ignore_error */,
"config",
diff --git a/bdep/types.hxx b/bdep/types.hxx
index d2a0355..2564851 100644
--- a/bdep/types.hxx
+++ b/bdep/types.hxx
@@ -99,6 +99,7 @@ namespace bdep
// <libbutl/process.mxx>
//
using butl::process;
+ using butl::process_env;
using butl::process_path;
using butl::process_exit;
using butl::process_error;