* [PATCH v5 02/11] setup_git_directory(): use is_dir_sep() helper
From: Johannes Schindelin @ 2017-03-09 22:23 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Duy Nguyen, Brandon Williams
In-Reply-To: <cover.1489098170.git.johannes.schindelin@gmx.de>
It is okay in practice to test for forward slashes in the output of
getcwd(), because we go out of our way to convert backslashes to forward
slashes in getcwd()'s output on Windows.
Still, the correct way to test for a dir separator is by using the
helper function we introduced for that very purpose. It also serves as a
good documentation what the code tries to do (not "how").
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
setup.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/setup.c b/setup.c
index 967f289f1ef..4a15b105676 100644
--- a/setup.c
+++ b/setup.c
@@ -910,7 +910,9 @@ static const char *setup_git_directory_gently_1(int *nongit_ok)
return setup_bare_git_dir(&cwd, offset, nongit_ok);
offset_parent = offset;
- while (--offset_parent > ceil_offset && cwd.buf[offset_parent] != '/');
+ while (--offset_parent > ceil_offset &&
+ !is_dir_sep(cwd.buf[offset_parent]))
+ ; /* continue */
if (offset_parent <= ceil_offset)
return setup_nongit(cwd.buf, nongit_ok);
if (one_filesystem) {
--
2.12.0.windows.1.7.g94dafc3b124
^ permalink raw reply related
* [PATCH v5 03/11] Prepare setup_discovered_git_directory() the root directory
From: Johannes Schindelin @ 2017-03-09 22:23 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Duy Nguyen, Brandon Williams
In-Reply-To: <cover.1489098170.git.johannes.schindelin@gmx.de>
Currently, the offset parameter (indicating what part of the cwd
parameter corresponds to the current directory after discovering the
.git/ directory) is set to 0 when we are running in the root directory.
However, in the next patches we will avoid changing the current working
directory while searching for the .git/ directory, meaning that the
offset corresponding to the root directory will have to be 1 to reflect
that this directory is characterized by the path "/" (and not "").
So let's make sure that setup_discovered_git_directory() only tries to
append the trailing slash to non-root directories.
Note: the setup_bare_git_directory() does not need a corresponding
change, as it does not want to return a prefix.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
setup.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/setup.c b/setup.c
index 4a15b105676..20a1f0f870e 100644
--- a/setup.c
+++ b/setup.c
@@ -721,8 +721,10 @@ static const char *setup_discovered_git_dir(const char *gitdir,
if (offset == cwd->len)
return NULL;
- /* Make "offset" point to past the '/', and add a '/' at the end */
- offset++;
+ /* Make "offset" point past the '/' (already the case for root dirs) */
+ if (offset != offset_1st_component(cwd->buf))
+ offset++;
+ /* Add a '/' at the end */
strbuf_addch(cwd, '/');
return cwd->buf + offset;
}
--
2.12.0.windows.1.7.g94dafc3b124
^ permalink raw reply related
* [PATCH v5 04/11] setup_git_directory_1(): avoid changing global state
From: Johannes Schindelin @ 2017-03-09 22:24 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Duy Nguyen, Brandon Williams
In-Reply-To: <cover.1489098170.git.johannes.schindelin@gmx.de>
For historical reasons, Git searches for the .git/ directory (or the
.git file) by changing the working directory successively to the parent
directory of the current directory, until either anything was found or
until a ceiling or a mount point is hit.
Further global state may be changed in case a .git/ directory was found.
We do have a use case, though, where we would like to find the .git/
directory without having any global state touched, though: when we read
the early config e.g. for the pager or for alias expansion.
Let's just move all of code that changes any global state out of the
function `setup_git_directory_gently_1()` into
`setup_git_directory_gently()`.
In subsequent patches, we will use the _1() function in a new
`discover_git_directory()` function that we will then use for the early
config code.
Note: the new loop is a *little* tricky, as we have to handle the root
directory specially: we cannot simply strip away the last component
including the slash, as the root directory only has that slash. To remedy
that, we introduce the `min_offset` variable that holds the minimal length
of an absolute path, and using that to special-case the root directory,
including an early exit before trying to find the parent of the root
directory.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
setup.c | 193 +++++++++++++++++++++++++++++++++++++++-------------------------
1 file changed, 118 insertions(+), 75 deletions(-)
diff --git a/setup.c b/setup.c
index 20a1f0f870e..a7bb09608c0 100644
--- a/setup.c
+++ b/setup.c
@@ -818,50 +818,49 @@ static int canonicalize_ceiling_entry(struct string_list_item *item,
}
}
+enum discovery_result {
+ GIT_DIR_NONE = 0,
+ GIT_DIR_EXPLICIT,
+ GIT_DIR_DISCOVERED,
+ GIT_DIR_BARE,
+ /* these are errors */
+ GIT_DIR_HIT_CEILING = -1,
+ GIT_DIR_HIT_MOUNT_POINT = -2
+};
+
/*
* We cannot decide in this function whether we are in the work tree or
* not, since the config can only be read _after_ this function was called.
+ *
+ * Also, we avoid changing any global state (such as the current working
+ * directory) to allow early callers.
+ *
+ * The directory where the search should start needs to be passed in via the
+ * `dir` parameter; upon return, the `dir` buffer will contain the path of
+ * the directory where the search ended, and `gitdir` will contain the path of
+ * the discovered .git/ directory, if any. This path may be relative against
+ * `dir` (i.e. *not* necessarily the cwd).
*/
-static const char *setup_git_directory_gently_1(int *nongit_ok)
+static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,
+ struct strbuf *gitdir)
{
const char *env_ceiling_dirs = getenv(CEILING_DIRECTORIES_ENVIRONMENT);
struct string_list ceiling_dirs = STRING_LIST_INIT_DUP;
- static struct strbuf cwd = STRBUF_INIT;
- const char *gitdirenv, *ret;
- char *gitfile;
- int offset, offset_parent, ceil_offset = -1;
+ const char *gitdirenv;
+ int ceil_offset = -1, min_offset = has_dos_drive_prefix(dir->buf) ? 3 : 1;
dev_t current_device = 0;
int one_filesystem = 1;
/*
- * We may have read an incomplete configuration before
- * setting-up the git directory. If so, clear the cache so
- * that the next queries to the configuration reload complete
- * configuration (including the per-repo config file that we
- * ignored previously).
- */
- git_config_clear();
-
- /*
- * Let's assume that we are in a git repository.
- * If it turns out later that we are somewhere else, the value will be
- * updated accordingly.
- */
- if (nongit_ok)
- *nongit_ok = 0;
-
- if (strbuf_getcwd(&cwd))
- die_errno(_("Unable to read current working directory"));
- offset = cwd.len;
-
- /*
* If GIT_DIR is set explicitly, we're not going
* to do any discovery, but we still do repository
* validation.
*/
gitdirenv = getenv(GIT_DIR_ENVIRONMENT);
- if (gitdirenv)
- return setup_explicit_git_dir(gitdirenv, &cwd, nongit_ok);
+ if (gitdirenv) {
+ strbuf_addstr(gitdir, gitdirenv);
+ return GIT_DIR_EXPLICIT;
+ }
if (env_ceiling_dirs) {
int empty_entry_found = 0;
@@ -869,15 +868,15 @@ static const char *setup_git_directory_gently_1(int *nongit_ok)
string_list_split(&ceiling_dirs, env_ceiling_dirs, PATH_SEP, -1);
filter_string_list(&ceiling_dirs, 0,
canonicalize_ceiling_entry, &empty_entry_found);
- ceil_offset = longest_ancestor_length(cwd.buf, &ceiling_dirs);
+ ceil_offset = longest_ancestor_length(dir->buf, &ceiling_dirs);
string_list_clear(&ceiling_dirs, 0);
}
- if (ceil_offset < 0 && has_dos_drive_prefix(cwd.buf))
- ceil_offset = 1;
+ if (ceil_offset < 0)
+ ceil_offset = min_offset - 2;
/*
- * Test in the following order (relative to the cwd):
+ * Test in the following order (relative to the dir):
* - .git (file containing "gitdir: <path>")
* - .git/
* - ./ (bare)
@@ -889,63 +888,104 @@ static const char *setup_git_directory_gently_1(int *nongit_ok)
*/
one_filesystem = !git_env_bool("GIT_DISCOVERY_ACROSS_FILESYSTEM", 0);
if (one_filesystem)
- current_device = get_device_or_die(".", NULL, 0);
+ current_device = get_device_or_die(dir->buf, NULL, 0);
for (;;) {
- gitfile = (char*)read_gitfile(DEFAULT_GIT_DIR_ENVIRONMENT);
- if (gitfile)
- gitdirenv = gitfile = xstrdup(gitfile);
- else {
- if (is_git_directory(DEFAULT_GIT_DIR_ENVIRONMENT))
- gitdirenv = DEFAULT_GIT_DIR_ENVIRONMENT;
+ int offset = dir->len;
+
+ if (offset > min_offset)
+ strbuf_addch(dir, '/');
+ strbuf_addstr(dir, DEFAULT_GIT_DIR_ENVIRONMENT);
+ gitdirenv = read_gitfile(dir->buf);
+ if (!gitdirenv && is_git_directory(dir->buf))
+ gitdirenv = DEFAULT_GIT_DIR_ENVIRONMENT;
+ strbuf_setlen(dir, offset);
+ if (gitdirenv) {
+ strbuf_addstr(gitdir, gitdirenv);
+ return GIT_DIR_DISCOVERED;
}
- if (gitdirenv) {
- ret = setup_discovered_git_dir(gitdirenv,
- &cwd, offset,
- nongit_ok);
- free(gitfile);
- return ret;
+ if (is_git_directory(dir->buf)) {
+ strbuf_addstr(gitdir, ".");
+ return GIT_DIR_BARE;
}
- free(gitfile);
- if (is_git_directory("."))
- return setup_bare_git_dir(&cwd, offset, nongit_ok);
+ if (offset <= min_offset)
+ return GIT_DIR_HIT_CEILING;
- offset_parent = offset;
- while (--offset_parent > ceil_offset &&
- !is_dir_sep(cwd.buf[offset_parent]))
+ while (--offset > ceil_offset && !is_dir_sep(dir->buf[offset]))
; /* continue */
- if (offset_parent <= ceil_offset)
- return setup_nongit(cwd.buf, nongit_ok);
- if (one_filesystem) {
- dev_t parent_device = get_device_or_die("..", cwd.buf,
- offset);
- if (parent_device != current_device) {
- if (nongit_ok) {
- if (chdir(cwd.buf))
- die_errno(_("Cannot come back to cwd"));
- *nongit_ok = 1;
- return NULL;
- }
- strbuf_setlen(&cwd, offset);
- die(_("Not a git repository (or any parent up to mount point %s)\n"
- "Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set)."),
- cwd.buf);
- }
- }
- if (chdir("..")) {
- strbuf_setlen(&cwd, offset);
- die_errno(_("Cannot change to '%s/..'"), cwd.buf);
- }
- offset = offset_parent;
+ if (offset <= ceil_offset)
+ return GIT_DIR_HIT_CEILING;
+
+ strbuf_setlen(dir, offset > min_offset ? offset : min_offset);
+ if (one_filesystem &&
+ current_device != get_device_or_die(dir->buf, NULL, offset))
+ return GIT_DIR_HIT_MOUNT_POINT;
}
}
const char *setup_git_directory_gently(int *nongit_ok)
{
+ static struct strbuf cwd = STRBUF_INIT;
+ struct strbuf dir = STRBUF_INIT, gitdir = STRBUF_INIT;
const char *prefix;
- prefix = setup_git_directory_gently_1(nongit_ok);
+ /*
+ * We may have read an incomplete configuration before
+ * setting-up the git directory. If so, clear the cache so
+ * that the next queries to the configuration reload complete
+ * configuration (including the per-repo config file that we
+ * ignored previously).
+ */
+ git_config_clear();
+
+ /*
+ * Let's assume that we are in a git repository.
+ * If it turns out later that we are somewhere else, the value will be
+ * updated accordingly.
+ */
+ if (nongit_ok)
+ *nongit_ok = 0;
+
+ if (strbuf_getcwd(&cwd))
+ die_errno(_("Unable to read current working directory"));
+ strbuf_addbuf(&dir, &cwd);
+
+ switch (setup_git_directory_gently_1(&dir, &gitdir)) {
+ case GIT_DIR_NONE:
+ prefix = NULL;
+ break;
+ case GIT_DIR_EXPLICIT:
+ prefix = setup_explicit_git_dir(gitdir.buf, &cwd, nongit_ok);
+ break;
+ case GIT_DIR_DISCOVERED:
+ if (dir.len < cwd.len && chdir(dir.buf))
+ die(_("Cannot change to '%s'"), dir.buf);
+ prefix = setup_discovered_git_dir(gitdir.buf, &cwd, dir.len,
+ nongit_ok);
+ break;
+ case GIT_DIR_BARE:
+ if (dir.len < cwd.len && chdir(dir.buf))
+ die(_("Cannot change to '%s'"), dir.buf);
+ prefix = setup_bare_git_dir(&cwd, dir.len, nongit_ok);
+ break;
+ case GIT_DIR_HIT_CEILING:
+ prefix = setup_nongit(cwd.buf, nongit_ok);
+ break;
+ case GIT_DIR_HIT_MOUNT_POINT:
+ if (nongit_ok) {
+ *nongit_ok = 1;
+ strbuf_release(&cwd);
+ strbuf_release(&dir);
+ return NULL;
+ }
+ die(_("Not a git repository (or any parent up to mount point %s)\n"
+ "Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set)."),
+ dir.buf);
+ default:
+ die("BUG: unhandled setup_git_directory_1() result");
+ }
+
if (prefix)
setenv(GIT_PREFIX_ENVIRONMENT, prefix, 1);
else
@@ -954,6 +994,9 @@ const char *setup_git_directory_gently(int *nongit_ok)
startup_info->have_repository = !nongit_ok || !*nongit_ok;
startup_info->prefix = prefix;
+ strbuf_release(&dir);
+ strbuf_release(&gitdir);
+
return prefix;
}
--
2.12.0.windows.1.7.g94dafc3b124
^ permalink raw reply related
* [PATCH v5 05/11] Introduce the discover_git_directory() function
From: Johannes Schindelin @ 2017-03-09 22:24 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Duy Nguyen, Brandon Williams
In-Reply-To: <cover.1489098170.git.johannes.schindelin@gmx.de>
We modified the setup_git_directory_gently_1() function earlier to make
it possible to discover the GIT_DIR without changing global state.
However, it is still a bit cumbersome to use if you only need to figure
out the (possibly absolute) path of the .git/ directory. Let's just
provide a convenient wrapper function with an easier signature that
*just* discovers the .git/ directory.
We will use it in a subsequent patch to fix the early config.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
cache.h | 7 +++++++
setup.c | 43 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 50 insertions(+)
diff --git a/cache.h b/cache.h
index 80b6372cf76..5218726cf88 100644
--- a/cache.h
+++ b/cache.h
@@ -518,6 +518,13 @@ extern void set_git_work_tree(const char *tree);
#define ALTERNATE_DB_ENVIRONMENT "GIT_ALTERNATE_OBJECT_DIRECTORIES"
extern void setup_work_tree(void);
+/*
+ * Find GIT_DIR of the repository that contains the current working directory,
+ * without changing the working directory or other global state. The result is
+ * appended to gitdir. The return value is either NULL if no repository was
+ * found, or pointing to the path inside gitdir's buffer.
+ */
+extern const char *discover_git_directory(struct strbuf *gitdir);
extern const char *setup_git_directory_gently(int *);
extern const char *setup_git_directory(void);
extern char *prefix_path(const char *prefix, int len, const char *path);
diff --git a/setup.c b/setup.c
index a7bb09608c0..43f522fa996 100644
--- a/setup.c
+++ b/setup.c
@@ -924,6 +924,49 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,
}
}
+const char *discover_git_directory(struct strbuf *gitdir)
+{
+ struct strbuf dir = STRBUF_INIT, err = STRBUF_INIT;
+ size_t gitdir_offset = gitdir->len, cwd_len;
+ struct repository_format candidate;
+
+ if (strbuf_getcwd(&dir))
+ return NULL;
+
+ cwd_len = dir.len;
+ if (setup_git_directory_gently_1(&dir, gitdir) <= 0) {
+ strbuf_release(&dir);
+ return NULL;
+ }
+
+ /*
+ * The returned gitdir is relative to dir, and if dir does not reflect
+ * the current working directory, we simply make the gitdir absolute.
+ */
+ if (dir.len < cwd_len && !is_absolute_path(gitdir->buf + gitdir_offset)) {
+ /* Avoid a trailing "/." */
+ if (!strcmp(".", gitdir->buf + gitdir_offset))
+ strbuf_setlen(gitdir, gitdir_offset);
+ else
+ strbuf_addch(&dir, '/');
+ strbuf_insert(gitdir, gitdir_offset, dir.buf, dir.len);
+ }
+
+ strbuf_reset(&dir);
+ strbuf_addf(&dir, "%s/config", gitdir->buf + gitdir_offset);
+ read_repository_format(&candidate, dir.buf);
+ strbuf_release(&dir);
+
+ if (verify_repository_format(&candidate, &err) < 0) {
+ warning("ignoring git dir '%s': %s",
+ gitdir->buf + gitdir_offset, err.buf);
+ strbuf_release(&err);
+ return NULL;
+ }
+
+ return gitdir->buf + gitdir_offset;
+}
+
const char *setup_git_directory_gently(int *nongit_ok)
{
static struct strbuf cwd = STRBUF_INIT;
--
2.12.0.windows.1.7.g94dafc3b124
^ permalink raw reply related
* [PATCH v5 07/11] read_early_config(): avoid .git/config hack when unneeded
From: Johannes Schindelin @ 2017-03-09 22:24 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Duy Nguyen, Brandon Williams
In-Reply-To: <cover.1489098170.git.johannes.schindelin@gmx.de>
So far, we only look whether the startup_info claims to have seen a
git_dir.
However, do_git_config_sequence() (and consequently the
git_config_with_options() call used by read_early_config() asks the
have_git_dir() function whether we have a .git/ directory, which in turn
also looks at git_dir and at the environment variable GIT_DIR. And when
this is the case, the repository config is handled already, so we do not
have to do that again explicitly.
Let's just use the same function, have_git_dir(), to determine whether we
have to handle .git/config explicitly.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
config.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/config.c b/config.c
index 9cfbeafd04c..068fa4dcfa6 100644
--- a/config.c
+++ b/config.c
@@ -1427,14 +1427,15 @@ void read_early_config(config_fn_t cb, void *data)
* core.repositoryformatversion), we'll read whatever is in .git/config
* blindly. Similarly, if we _are_ in a repository, but not at the
* root, we'll fail to find .git/config (because it's really
- * ../.git/config, etc). See t7006 for a complete set of failures.
+ * ../.git/config, etc), unless setup_git_directory() was already called.
+ * See t7006 for a complete set of failures.
*
* However, we have historically provided this hack because it does
* work some of the time (namely when you are at the top-level of a
* valid repository), and would rarely make things worse (i.e., you do
* not generally have a .git/config file sitting around).
*/
- if (!startup_info->have_repository) {
+ if (!have_git_dir()) {
struct git_config_source repo_config;
memset(&repo_config, 0, sizeof(repo_config));
--
2.12.0.windows.1.7.g94dafc3b124
^ permalink raw reply related
* [PATCH v5 08/11] read_early_config(): really discover .git/
From: Johannes Schindelin @ 2017-03-09 22:25 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Duy Nguyen, Brandon Williams
In-Reply-To: <cover.1489098170.git.johannes.schindelin@gmx.de>
Earlier, we punted and simply assumed that we are in the top-level
directory of the project, and that there is no .git file but a .git/
directory so that we can read directly from .git/config.
However, that is not necessarily true. We may be in a subdirectory. Or
.git may be a gitfile. Or the environment variable GIT_DIR may be set.
To remedy this situation, we just refactored the way
setup_git_directory() discovers the .git/ directory, to make it
reusable, and more importantly, to leave all global variables and the
current working directory alone.
Let's discover the .git/ directory correctly in read_early_config() by
using that new function.
This fixes 4 known breakages in t7006.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
config.c | 31 ++++++++++++-------------------
| 8 ++++----
2 files changed, 16 insertions(+), 23 deletions(-)
diff --git a/config.c b/config.c
index 068fa4dcfa6..a88df53fdbc 100644
--- a/config.c
+++ b/config.c
@@ -1414,34 +1414,27 @@ static void configset_iter(struct config_set *cs, config_fn_t fn, void *data)
void read_early_config(config_fn_t cb, void *data)
{
+ struct strbuf buf = STRBUF_INIT;
+
git_config_with_options(cb, data, NULL, 1);
/*
- * Note that this is a really dirty hack that does the wrong thing in
- * many cases. The crux of the problem is that we cannot run
- * setup_git_directory() early on in git's setup, so we have no idea if
- * we are in a repository or not, and therefore are not sure whether
- * and how to read repository-local config.
- *
- * So if we _aren't_ in a repository (or we are but we would reject its
- * core.repositoryformatversion), we'll read whatever is in .git/config
- * blindly. Similarly, if we _are_ in a repository, but not at the
- * root, we'll fail to find .git/config (because it's really
- * ../.git/config, etc), unless setup_git_directory() was already called.
- * See t7006 for a complete set of failures.
- *
- * However, we have historically provided this hack because it does
- * work some of the time (namely when you are at the top-level of a
- * valid repository), and would rarely make things worse (i.e., you do
- * not generally have a .git/config file sitting around).
+ * When setup_git_directory() was not yet asked to discover the
+ * GIT_DIR, we ask discover_git_directory() to figure out whether there
+ * is any repository config we should use (but unlike
+ * setup_git_directory_gently(), no global state is changed, most
+ * notably, the current working directory is still the same after the
+ * call).
*/
- if (!have_git_dir()) {
+ if (!have_git_dir() && discover_git_directory(&buf)) {
struct git_config_source repo_config;
memset(&repo_config, 0, sizeof(repo_config));
- repo_config.file = ".git/config";
+ strbuf_addstr(&buf, "/config");
+ repo_config.file = buf.buf;
git_config_with_options(cb, data, &repo_config, 1);
}
+ strbuf_release(&buf);
}
static void git_config_check_init(void);
--git a/t/t7006-pager.sh b/t/t7006-pager.sh
index 304ae06c600..4f3794d415e 100755
--- a/t/t7006-pager.sh
+++ b/t/t7006-pager.sh
@@ -360,19 +360,19 @@ test_pager_choices 'git aliasedlog'
test_default_pager expect_success 'git -p aliasedlog'
test_PAGER_overrides expect_success 'git -p aliasedlog'
test_core_pager_overrides expect_success 'git -p aliasedlog'
-test_core_pager_subdir expect_failure 'git -p aliasedlog'
+test_core_pager_subdir expect_success 'git -p aliasedlog'
test_GIT_PAGER_overrides expect_success 'git -p aliasedlog'
test_default_pager expect_success 'git -p true'
test_PAGER_overrides expect_success 'git -p true'
test_core_pager_overrides expect_success 'git -p true'
-test_core_pager_subdir expect_failure 'git -p true'
+test_core_pager_subdir expect_success 'git -p true'
test_GIT_PAGER_overrides expect_success 'git -p true'
test_default_pager expect_success test_must_fail 'git -p request-pull'
test_PAGER_overrides expect_success test_must_fail 'git -p request-pull'
test_core_pager_overrides expect_success test_must_fail 'git -p request-pull'
-test_core_pager_subdir expect_failure test_must_fail 'git -p request-pull'
+test_core_pager_subdir expect_success test_must_fail 'git -p request-pull'
test_GIT_PAGER_overrides expect_success test_must_fail 'git -p request-pull'
test_default_pager expect_success test_must_fail 'git -p'
@@ -380,7 +380,7 @@ test_PAGER_overrides expect_success test_must_fail 'git -p'
test_local_config_ignored expect_failure test_must_fail 'git -p'
test_GIT_PAGER_overrides expect_success test_must_fail 'git -p'
-test_expect_failure TTY 'core.pager in repo config works and retains cwd' '
+test_expect_success TTY 'core.pager in repo config works and retains cwd' '
sane_unset GIT_PAGER &&
test_config core.pager "cat >cwd-retained" &&
(
--
2.12.0.windows.1.7.g94dafc3b124
^ permalink raw reply related
* [PATCH v5 06/11] Make read_early_config() reusable
From: Johannes Schindelin @ 2017-03-09 22:24 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Duy Nguyen, Brandon Williams
In-Reply-To: <cover.1489098170.git.johannes.schindelin@gmx.de>
The pager configuration needs to be read early, possibly before
discovering any .git/ directory.
Let's not hide this function in pager.c, but make it available to other
callers.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
cache.h | 1 +
config.c | 31 +++++++++++++++++++++++++++++++
| 31 -------------------------------
3 files changed, 32 insertions(+), 31 deletions(-)
diff --git a/cache.h b/cache.h
index 5218726cf88..e7b57457e73 100644
--- a/cache.h
+++ b/cache.h
@@ -1804,6 +1804,7 @@ extern int git_config_from_blob_sha1(config_fn_t fn, const char *name,
const unsigned char *sha1, void *data);
extern void git_config_push_parameter(const char *text);
extern int git_config_from_parameters(config_fn_t fn, void *data);
+extern void read_early_config(config_fn_t cb, void *data);
extern void git_config(config_fn_t fn, void *);
extern int git_config_with_options(config_fn_t fn, void *,
struct git_config_source *config_source,
diff --git a/config.c b/config.c
index c6b874a7bf7..9cfbeafd04c 100644
--- a/config.c
+++ b/config.c
@@ -1412,6 +1412,37 @@ static void configset_iter(struct config_set *cs, config_fn_t fn, void *data)
}
}
+void read_early_config(config_fn_t cb, void *data)
+{
+ git_config_with_options(cb, data, NULL, 1);
+
+ /*
+ * Note that this is a really dirty hack that does the wrong thing in
+ * many cases. The crux of the problem is that we cannot run
+ * setup_git_directory() early on in git's setup, so we have no idea if
+ * we are in a repository or not, and therefore are not sure whether
+ * and how to read repository-local config.
+ *
+ * So if we _aren't_ in a repository (or we are but we would reject its
+ * core.repositoryformatversion), we'll read whatever is in .git/config
+ * blindly. Similarly, if we _are_ in a repository, but not at the
+ * root, we'll fail to find .git/config (because it's really
+ * ../.git/config, etc). See t7006 for a complete set of failures.
+ *
+ * However, we have historically provided this hack because it does
+ * work some of the time (namely when you are at the top-level of a
+ * valid repository), and would rarely make things worse (i.e., you do
+ * not generally have a .git/config file sitting around).
+ */
+ if (!startup_info->have_repository) {
+ struct git_config_source repo_config;
+
+ memset(&repo_config, 0, sizeof(repo_config));
+ repo_config.file = ".git/config";
+ git_config_with_options(cb, data, &repo_config, 1);
+ }
+}
+
static void git_config_check_init(void);
void git_config(config_fn_t fn, void *data)
--git a/pager.c b/pager.c
index ae796433630..73ca8bc3b17 100644
--- a/pager.c
+++ b/pager.c
@@ -43,37 +43,6 @@ static int core_pager_config(const char *var, const char *value, void *data)
return 0;
}
-static void read_early_config(config_fn_t cb, void *data)
-{
- git_config_with_options(cb, data, NULL, 1);
-
- /*
- * Note that this is a really dirty hack that does the wrong thing in
- * many cases. The crux of the problem is that we cannot run
- * setup_git_directory() early on in git's setup, so we have no idea if
- * we are in a repository or not, and therefore are not sure whether
- * and how to read repository-local config.
- *
- * So if we _aren't_ in a repository (or we are but we would reject its
- * core.repositoryformatversion), we'll read whatever is in .git/config
- * blindly. Similarly, if we _are_ in a repository, but not at the
- * root, we'll fail to find .git/config (because it's really
- * ../.git/config, etc). See t7006 for a complete set of failures.
- *
- * However, we have historically provided this hack because it does
- * work some of the time (namely when you are at the top-level of a
- * valid repository), and would rarely make things worse (i.e., you do
- * not generally have a .git/config file sitting around).
- */
- if (!startup_info->have_repository) {
- struct git_config_source repo_config;
-
- memset(&repo_config, 0, sizeof(repo_config));
- repo_config.file = ".git/config";
- git_config_with_options(cb, data, &repo_config, 1);
- }
-}
-
const char *git_pager(int stdout_is_tty)
{
const char *pager;
--
2.12.0.windows.1.7.g94dafc3b124
^ permalink raw reply related
* [PATCH v5 09/11] Test read_early_config()
From: Johannes Schindelin @ 2017-03-09 22:25 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Duy Nguyen, Brandon Williams
In-Reply-To: <cover.1489098170.git.johannes.schindelin@gmx.de>
So far, we had no explicit tests of that function.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
t/helper/test-config.c | 15 +++++++++++++++
t/t1309-early-config.sh | 50 +++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 65 insertions(+)
create mode 100755 t/t1309-early-config.sh
diff --git a/t/helper/test-config.c b/t/helper/test-config.c
index 83a4f2ab869..8e3ed6a76cb 100644
--- a/t/helper/test-config.c
+++ b/t/helper/test-config.c
@@ -66,6 +66,16 @@ static int iterate_cb(const char *var, const char *value, void *data)
return 0;
}
+static int early_config_cb(const char *var, const char *value, void *vdata)
+{
+ const char *key = vdata;
+
+ if (!strcmp(key, var))
+ printf("%s\n", value);
+
+ return 0;
+}
+
int cmd_main(int argc, const char **argv)
{
int i, val;
@@ -73,6 +83,11 @@ int cmd_main(int argc, const char **argv)
const struct string_list *strptr;
struct config_set cs;
+ if (argc == 3 && !strcmp(argv[1], "read_early_config")) {
+ read_early_config(early_config_cb, (void *)argv[2]);
+ return 0;
+ }
+
setup_git_directory();
git_configset_init(&cs);
diff --git a/t/t1309-early-config.sh b/t/t1309-early-config.sh
new file mode 100755
index 00000000000..0c55dee514c
--- /dev/null
+++ b/t/t1309-early-config.sh
@@ -0,0 +1,50 @@
+#!/bin/sh
+
+test_description='Test read_early_config()'
+
+. ./test-lib.sh
+
+test_expect_success 'read early config' '
+ test_config early.config correct &&
+ test-config read_early_config early.config >output &&
+ test correct = "$(cat output)"
+'
+
+test_expect_success 'in a sub-directory' '
+ test_config early.config sub &&
+ mkdir -p sub &&
+ (
+ cd sub &&
+ test-config read_early_config early.config
+ ) >output &&
+ test sub = "$(cat output)"
+'
+
+test_expect_success 'ceiling' '
+ test_config early.config ceiling &&
+ mkdir -p sub &&
+ (
+ GIT_CEILING_DIRECTORIES="$PWD" &&
+ export GIT_CEILING_DIRECTORIES &&
+ cd sub &&
+ test-config read_early_config early.config
+ ) >output &&
+ test -z "$(cat output)"
+'
+
+test_expect_success 'ceiling #2' '
+ mkdir -p xdg/git &&
+ git config -f xdg/git/config early.config xdg &&
+ test_config early.config ceiling &&
+ mkdir -p sub &&
+ (
+ XDG_CONFIG_HOME="$PWD"/xdg &&
+ GIT_CEILING_DIRECTORIES="$PWD" &&
+ export GIT_CEILING_DIRECTORIES XDG_CONFIG_HOME &&
+ cd sub &&
+ test-config read_early_config early.config
+ ) >output &&
+ test xdg = "$(cat output)"
+'
+
+test_done
--
2.12.0.windows.1.7.g94dafc3b124
^ permalink raw reply related
* [PATCH v5 10/11] setup_git_directory_gently_1(): avoid die()ing
From: Johannes Schindelin @ 2017-03-09 22:25 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Duy Nguyen, Brandon Williams
In-Reply-To: <cover.1489098170.git.johannes.schindelin@gmx.de>
This function now has a new caller in addition to setup_git_directory():
the newly introduced discover_git_directory(). That function wants to
discover the current .git/ directory, and in case of a corrupted one
simply pretend that there is none to be found.
Example: if a stale .git file exists in the parent directory, and the
user calls `git -p init`, we want Git to simply *not* read any
repository config for the pager (instead of aborting with a message that
the .git file is corrupt).
Let's actually pretend that there was no GIT_DIR to be found in that case
when being called from discover_git_directory(), but keep the previous
behavior (i.e. to die()) for the setup_git_directory() case.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
setup.c | 26 ++++++++++++++++++--------
1 file changed, 18 insertions(+), 8 deletions(-)
diff --git a/setup.c b/setup.c
index 43f522fa996..b0a28f609e2 100644
--- a/setup.c
+++ b/setup.c
@@ -825,7 +825,8 @@ enum discovery_result {
GIT_DIR_BARE,
/* these are errors */
GIT_DIR_HIT_CEILING = -1,
- GIT_DIR_HIT_MOUNT_POINT = -2
+ GIT_DIR_HIT_MOUNT_POINT = -2,
+ GIT_DIR_INVALID_GITFILE = -3
};
/*
@@ -842,7 +843,8 @@ enum discovery_result {
* `dir` (i.e. *not* necessarily the cwd).
*/
static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,
- struct strbuf *gitdir)
+ struct strbuf *gitdir,
+ int die_on_error)
{
const char *env_ceiling_dirs = getenv(CEILING_DIRECTORIES_ENVIRONMENT);
struct string_list ceiling_dirs = STRING_LIST_INIT_DUP;
@@ -890,14 +892,22 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,
if (one_filesystem)
current_device = get_device_or_die(dir->buf, NULL, 0);
for (;;) {
- int offset = dir->len;
+ int offset = dir->len, error_code = 0;
if (offset > min_offset)
strbuf_addch(dir, '/');
strbuf_addstr(dir, DEFAULT_GIT_DIR_ENVIRONMENT);
- gitdirenv = read_gitfile(dir->buf);
- if (!gitdirenv && is_git_directory(dir->buf))
- gitdirenv = DEFAULT_GIT_DIR_ENVIRONMENT;
+ gitdirenv = read_gitfile_gently(dir->buf, die_on_error ?
+ NULL : &error_code);
+ if (!gitdirenv) {
+ if (die_on_error ||
+ error_code == READ_GITFILE_ERR_NOT_A_FILE) {
+ if (is_git_directory(dir->buf))
+ gitdirenv = DEFAULT_GIT_DIR_ENVIRONMENT;
+ } else if (error_code &&
+ error_code != READ_GITFILE_ERR_STAT_FAILED)
+ return GIT_DIR_INVALID_GITFILE;
+ }
strbuf_setlen(dir, offset);
if (gitdirenv) {
strbuf_addstr(gitdir, gitdirenv);
@@ -934,7 +944,7 @@ const char *discover_git_directory(struct strbuf *gitdir)
return NULL;
cwd_len = dir.len;
- if (setup_git_directory_gently_1(&dir, gitdir) <= 0) {
+ if (setup_git_directory_gently_1(&dir, gitdir, 0) <= 0) {
strbuf_release(&dir);
return NULL;
}
@@ -994,7 +1004,7 @@ const char *setup_git_directory_gently(int *nongit_ok)
die_errno(_("Unable to read current working directory"));
strbuf_addbuf(&dir, &cwd);
- switch (setup_git_directory_gently_1(&dir, &gitdir)) {
+ switch (setup_git_directory_gently_1(&dir, &gitdir, 1)) {
case GIT_DIR_NONE:
prefix = NULL;
break;
--
2.12.0.windows.1.7.g94dafc3b124
^ permalink raw reply related
* [PATCH v5 11/11] t1309: document cases where we would want early config not to die()
From: Johannes Schindelin @ 2017-03-09 22:25 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Duy Nguyen, Brandon Williams
In-Reply-To: <cover.1489098170.git.johannes.schindelin@gmx.de>
Jeff King came up with a couple examples that demonstrate how the new
read_early_config() that looks harder for the current .git/ directory
could die() in an undesirable way.
Let's add those cases to the test script, to document what we would like
to happen when early config encounters problems.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
t/t1309-early-config.sh | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/t/t1309-early-config.sh b/t/t1309-early-config.sh
index 0c55dee514c..027eca63a3c 100755
--- a/t/t1309-early-config.sh
+++ b/t/t1309-early-config.sh
@@ -47,4 +47,29 @@ test_expect_success 'ceiling #2' '
test xdg = "$(cat output)"
'
+test_with_config ()
+{
+ rm -rf throwaway &&
+ git init throwaway &&
+ (
+ cd throwaway &&
+ echo "$*" >.git/config &&
+ test-config read_early_config early.config
+ )
+}
+
+test_expect_success 'ignore .git/ with incompatible repository version' '
+ test_with_config "[core]repositoryformatversion = 999999" 2>err &&
+ grep "warning:.* Expected git repo version <= [1-9]" err
+'
+
+test_expect_failure 'ignore .git/ with invalid repository version' '
+ test_with_config "[core]repositoryformatversion = invalid"
+'
+
+
+test_expect_failure 'ignore .git/ with invalid config' '
+ test_with_config "["
+'
+
test_done
--
2.12.0.windows.1.7.g94dafc3b124
^ permalink raw reply related
* Re: [PATCH 2/2] pathspec: allow escaped query values
From: Jonathan Tan @ 2017-03-09 22:31 UTC (permalink / raw)
To: Brandon Williams, git; +Cc: sbeller, pclouds
In-Reply-To: <20170309210756.105566-3-bmwill@google.com>
On 03/09/2017 01:07 PM, Brandon Williams wrote:
> diff --git a/t/t6135-pathspec-with-attrs.sh b/t/t6135-pathspec-with-attrs.sh
> index b5e5a0607..585d17bad 100755
> --- a/t/t6135-pathspec-with-attrs.sh
> +++ b/t/t6135-pathspec-with-attrs.sh
> @@ -178,4 +178,13 @@ test_expect_success 'abort on asking for wrong magic' '
> test_must_fail git ls-files . ":(attr:!label=foo)"
> '
>
> +test_expect_success 'check attribute list' '
> + cat <<-EOF >>.gitattributes &&
> + * whitespace=indent,trail,space
> + EOF
> + git ls-files ":(attr:whitespace=indent\,trail\,space)" >actual &&
> + git ls-files >expect &&
> + test_cmp expect actual
> +'
> +
> test_done
Is there a way to verify that `\,` is not escaped by the shell into `,`?
Maybe also add tests that show \ as the last character and \ escaping
another \.
^ permalink raw reply
* Re: [PATCH v3 2/2] submodule--helper.c: remove duplicate code
From: Valery Tolstov @ 2017-03-09 22:54 UTC (permalink / raw)
To: git; +Cc: sbeller, me, gitster, bmwill
In-Reply-To: <20170309012734.21541-3-me@vtolstov.org>
As remainder. It is better if only [PATCH v3 2/2] is taken,
on top the patch from here
https://public-inbox.org/git/20170309221543.15897-8-sbeller@google.com/
The [PATCH v3 1/2] is just a copy of latter.
^ permalink raw reply
* Re: [PATCH 07/17] connect_work_tree_and_git_dir: safely create leading directories
From: Brandon Williams @ 2017-03-09 23:29 UTC (permalink / raw)
To: Stefan Beller; +Cc: gitster, git, novalis, sandals, hvoigt, jrnieder, ramsay
In-Reply-To: <20170309221543.15897-8-sbeller@google.com>
On 03/09, Stefan Beller wrote:
> In a later patch we'll use connect_work_tree_and_git_dir when the
> directory for the gitlink file doesn't exist yet. This patch makes
> connect_work_tree_and_git_dir safe to use for both cases of
> either the git dir or the working dir missing.
>
> To do so, we need to call safe_create_leading_directories[_const]
> on both directories. However this has to happen before we construct
> the absolute paths as real_pathdup assumes the directories to
> be there already.
>
> So for both the config file in the git dir as well as the .git link
> file we need to
> a) construct the name
> b) call SCLD
> c) get the absolute path
> d) once a-c is done for both we can consume the absolute path
> to compute the relative path to each other and store those
> relative paths.
>
> The implementation provided here puts a) and b) for both cases first,
> and then performs c and d after.
>
> One of the two users of 'connect_work_tree_and_git_dir' already checked
> for the directory being there, so we can loose that check as
> connect_work_tree_and_git_dir handles this functionality now.
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
> dir.c | 32 +++++++++++++++++++++-----------
> submodule.c | 11 ++---------
> 2 files changed, 23 insertions(+), 20 deletions(-)
>
> diff --git a/dir.c b/dir.c
> index 4541f9e146..6f52af7abb 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -2728,23 +2728,33 @@ void untracked_cache_add_to_index(struct index_state *istate,
> /* Update gitfile and core.worktree setting to connect work tree and git dir */
> void connect_work_tree_and_git_dir(const char *work_tree_, const char *git_dir_)
> {
> - struct strbuf file_name = STRBUF_INIT;
> + struct strbuf gitfile_sb = STRBUF_INIT;
> + struct strbuf cfg_sb = STRBUF_INIT;
> struct strbuf rel_path = STRBUF_INIT;
> - char *git_dir = real_pathdup(git_dir_);
> - char *work_tree = real_pathdup(work_tree_);
> + char *git_dir, *work_tree;
>
> - /* Update gitfile */
> - strbuf_addf(&file_name, "%s/.git", work_tree);
> - write_file(file_name.buf, "gitdir: %s",
> - relative_path(git_dir, work_tree, &rel_path));
> + /* Prepare .git file */
> + strbuf_addf(&gitfile_sb, "%s/.git", work_tree_);
> + if (safe_create_leading_directories_const(gitfile_sb.buf))
> + die(_("could not create directories for %s"), gitfile_sb.buf);
> +
> + /* Prepare config file */
> + strbuf_addf(&cfg_sb, "%s/config", git_dir_);
> + if (safe_create_leading_directories_const(cfg_sb.buf))
> + die(_("could not create directories for %s"), cfg_sb.buf);
>
> + git_dir = real_pathdup(git_dir_);
> + work_tree = real_pathdup(work_tree_);
Just a note that this is a spot that'll be affected by the change to
real_pathdup() which adds a 'die_on_error' parameter to correct bad
behaviour I introduced.
> +
> + /* Write .git file */
> + write_file(gitfile_sb.buf, "gitdir: %s",
> + relative_path(git_dir, work_tree, &rel_path));
> /* Update core.worktree setting */
> - strbuf_reset(&file_name);
> - strbuf_addf(&file_name, "%s/config", git_dir);
> - git_config_set_in_file(file_name.buf, "core.worktree",
> + git_config_set_in_file(cfg_sb.buf, "core.worktree",
> relative_path(work_tree, git_dir, &rel_path));
>
> - strbuf_release(&file_name);
> + strbuf_release(&gitfile_sb);
> + strbuf_release(&cfg_sb);
> strbuf_release(&rel_path);
> free(work_tree);
> free(git_dir);
> diff --git a/submodule.c b/submodule.c
> index 0e55372f37..04d185738f 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -1442,8 +1442,6 @@ void absorb_git_dir_into_superproject(const char *prefix,
>
> /* Not populated? */
> if (!sub_git_dir) {
> - char *real_new_git_dir;
> - const char *new_git_dir;
> const struct submodule *sub;
>
> if (err_code == READ_GITFILE_ERR_STAT_FAILED) {
> @@ -1466,13 +1464,8 @@ void absorb_git_dir_into_superproject(const char *prefix,
> sub = submodule_from_path(null_sha1, path);
> if (!sub)
> die(_("could not lookup name for submodule '%s'"), path);
> - new_git_dir = git_path("modules/%s", sub->name);
> - if (safe_create_leading_directories_const(new_git_dir) < 0)
> - die(_("could not create directory '%s'"), new_git_dir);
> - real_new_git_dir = real_pathdup(new_git_dir);
> - connect_work_tree_and_git_dir(path, real_new_git_dir);
> -
> - free(real_new_git_dir);
> + connect_work_tree_and_git_dir(path,
> + git_path("modules/%s", sub->name));
> } else {
> /* Is it already absorbed into the superprojects git dir? */
> char *real_sub_git_dir = real_pathdup(sub_git_dir);
> --
> 2.12.0.rc1.45.g207f5fbb2b
>
--
Brandon Williams
^ permalink raw reply
* Re: [PATCH 12/17] update submodules: add submodule_move_head
From: Brandon Williams @ 2017-03-09 23:37 UTC (permalink / raw)
To: Stefan Beller; +Cc: gitster, git, novalis, sandals, hvoigt, jrnieder, ramsay
In-Reply-To: <20170309221543.15897-13-sbeller@google.com>
On 03/09, Stefan Beller wrote:
> In later patches we introduce the options and flag for commands
> that modify the working directory, e.g. git-checkout.
>
> This piece of code will be used universally for
> all these working tree modifications as it
> * supports dry run to answer the question:
> "Is it safe to change the submodule to this new state?"
> e.g. is it overwriting untracked files or are there local
> changes that would be overwritten?
> * supports a force flag that can be used for resetting
> the tree.
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
> submodule.c | 135 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> submodule.h | 7 ++++
> 2 files changed, 142 insertions(+)
>
> diff --git a/submodule.c b/submodule.c
> index 0b2596e88a..bc5fecf8c5 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -1239,6 +1239,141 @@ int bad_to_remove_submodule(const char *path, unsigned flags)
> return ret;
> }
>
> +static int submodule_has_dirty_index(const struct submodule *sub)
> +{
> + struct child_process cp = CHILD_PROCESS_INIT;
> +
> + prepare_submodule_repo_env_no_git_dir(&cp.env_array);
> +
> + cp.git_cmd = 1;
> + argv_array_pushl(&cp.args, "diff-index", "--quiet", \
> + "--cached", "HEAD", NULL);
The formatting of this line is a little odd. Also you can drop the
backslash.
Mostly because I haven't dug too deep yet/can't remember: Does
diff-index do the correct thing with nested submodules? If I remember
correctly it does so this shouldn't be a problem.
> + cp.no_stdin = 1;
> + cp.no_stdout = 1;
> + cp.dir = sub->path;
> + if (start_command(&cp))
> + die("could not recurse into submodule '%s'", sub->path);
> +
> + return finish_command(&cp);
> +}
> +
> +static void submodule_reset_index(const char *path)
> +{
> + struct child_process cp = CHILD_PROCESS_INIT;
> + prepare_submodule_repo_env_no_git_dir(&cp.env_array);
> +
> + cp.git_cmd = 1;
> + cp.no_stdin = 1;
> + cp.dir = path;
> +
> + argv_array_pushf(&cp.args, "--super-prefix=%s/", path);
> + argv_array_pushl(&cp.args, "read-tree", "-u", "--reset", NULL);
> +
> + argv_array_push(&cp.args, EMPTY_TREE_SHA1_HEX);
> +
> + if (run_command(&cp))
> + die("could not reset submodule index");
> +}
> +
> +/**
> + * Moves a submodule at a given path from a given head to another new head.
> + * For edge cases (a submodule coming into existence or removing a submodule)
> + * pass NULL for old or new respectively.
> + */
> +int submodule_move_head(const char *path,
> + const char *old,
> + const char *new,
> + unsigned flags)
> +{
> + int ret = 0;
> + struct child_process cp = CHILD_PROCESS_INIT;
> + const struct submodule *sub;
> +
> + sub = submodule_from_path(null_sha1, path);
> +
> + if (!sub)
> + die("BUG: could not get submodule information for '%s'", path);
> +
> + if (old && !(flags & SUBMODULE_MOVE_HEAD_FORCE)) {
> + /* Check if the submodule has a dirty index. */
> + if (submodule_has_dirty_index(sub))
> + return error(_("submodule '%s' has dirty index"), path);
> + }
> +
> + if (!(flags & SUBMODULE_MOVE_HEAD_DRY_RUN)) {
> + if (old) {
> + if (!submodule_uses_gitfile(path))
> + absorb_git_dir_into_superproject("", path,
> + ABSORB_GITDIR_RECURSE_SUBMODULES);
> + } else {
> + struct strbuf sb = STRBUF_INIT;
> + strbuf_addf(&sb, "%s/modules/%s",
> + get_git_common_dir(), sub->name);
> + connect_work_tree_and_git_dir(path, sb.buf);
> + strbuf_release(&sb);
> +
> + /* make sure the index is clean as well */
> + submodule_reset_index(path);
> + }
> + }
> +
> + prepare_submodule_repo_env_no_git_dir(&cp.env_array);
> +
> + cp.git_cmd = 1;
> + cp.no_stdin = 1;
> + cp.dir = path;
> +
> + argv_array_pushf(&cp.args, "--super-prefix=%s/", path);
> + argv_array_pushl(&cp.args, "read-tree", NULL);
> +
> + if (flags & SUBMODULE_MOVE_HEAD_DRY_RUN)
> + argv_array_push(&cp.args, "-n");
> + else
> + argv_array_push(&cp.args, "-u");
> +
> + if (flags & SUBMODULE_MOVE_HEAD_FORCE)
> + argv_array_push(&cp.args, "--reset");
> + else
> + argv_array_push(&cp.args, "-m");
> +
> + argv_array_push(&cp.args, old ? old : EMPTY_TREE_SHA1_HEX);
> + argv_array_push(&cp.args, new ? new : EMPTY_TREE_SHA1_HEX);
> +
> + if (run_command(&cp)) {
> + ret = -1;
> + goto out;
> + }
> +
> + if (!(flags & SUBMODULE_MOVE_HEAD_DRY_RUN)) {
> + if (new) {
> + struct child_process cp1 = CHILD_PROCESS_INIT;
> + /* also set the HEAD accordingly */
> + cp1.git_cmd = 1;
> + cp1.no_stdin = 1;
> + cp1.dir = path;
> +
> + argv_array_pushl(&cp1.args, "update-ref", "HEAD",
> + new ? new : EMPTY_TREE_SHA1_HEX, NULL);
> +
> + if (run_command(&cp1)) {
> + ret = -1;
> + goto out;
> + }
> + } else {
> + struct strbuf sb = STRBUF_INIT;
> +
> + strbuf_addf(&sb, "%s/.git", path);
> + unlink_or_warn(sb.buf);
> + strbuf_release(&sb);
> +
> + if (is_empty_dir(path))
> + rmdir_or_warn(path);
> + }
> + }
> +out:
> + return ret;
> +}
> +
> static int find_first_merges(struct object_array *result, const char *path,
> struct commit *a, struct commit *b)
> {
> diff --git a/submodule.h b/submodule.h
> index 6f3fe85c7c..4cdf6445f7 100644
> --- a/submodule.h
> +++ b/submodule.h
> @@ -96,6 +96,13 @@ extern int push_unpushed_submodules(struct sha1_array *commits,
> extern void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir);
> extern int parallel_submodules(void);
>
> +#define SUBMODULE_MOVE_HEAD_DRY_RUN (1<<0)
> +#define SUBMODULE_MOVE_HEAD_FORCE (1<<1)
> +extern int submodule_move_head(const char *path,
> + const char *old,
> + const char *new,
> + unsigned flags);
> +
> /*
> * Prepare the "env_array" parameter of a "struct child_process" for executing
> * a submodule by clearing any repo-specific envirionment variables, but
> --
> 2.12.0.rc1.45.g207f5fbb2b
>
--
Brandon Williams
^ permalink raw reply
* Re: [PATCH 12/17] update submodules: add submodule_move_head
From: Brandon Williams @ 2017-03-09 23:43 UTC (permalink / raw)
To: Stefan Beller; +Cc: gitster, git, novalis, sandals, hvoigt, jrnieder, ramsay
In-Reply-To: <20170309221543.15897-13-sbeller@google.com>
On 03/09, Stefan Beller wrote:
> +/**
> + * Moves a submodule at a given path from a given head to another new head.
> + * For edge cases (a submodule coming into existence or removing a submodule)
> + * pass NULL for old or new respectively.
> + */
> +int submodule_move_head(const char *path,
> + const char *old,
> + const char *new,
> + unsigned flags)
> +{
> + int ret = 0;
> + struct child_process cp = CHILD_PROCESS_INIT;
> + const struct submodule *sub;
> +
> + sub = submodule_from_path(null_sha1, path);
> +
> + if (!sub)
> + die("BUG: could not get submodule information for '%s'", path);
> +
> + if (old && !(flags & SUBMODULE_MOVE_HEAD_FORCE)) {
> + /* Check if the submodule has a dirty index. */
> + if (submodule_has_dirty_index(sub))
> + return error(_("submodule '%s' has dirty index"), path);
> + }
> +
> + if (!(flags & SUBMODULE_MOVE_HEAD_DRY_RUN)) {
> + if (old) {
> + if (!submodule_uses_gitfile(path))
> + absorb_git_dir_into_superproject("", path,
> + ABSORB_GITDIR_RECURSE_SUBMODULES);
> + } else {
> + struct strbuf sb = STRBUF_INIT;
> + strbuf_addf(&sb, "%s/modules/%s",
> + get_git_common_dir(), sub->name);
> + connect_work_tree_and_git_dir(path, sb.buf);
> + strbuf_release(&sb);
> +
> + /* make sure the index is clean as well */
> + submodule_reset_index(path);
> + }
> + }
> +
> + prepare_submodule_repo_env_no_git_dir(&cp.env_array);
> +
> + cp.git_cmd = 1;
> + cp.no_stdin = 1;
> + cp.dir = path;
> +
> + argv_array_pushf(&cp.args, "--super-prefix=%s/", path);
Missed this one too. Same question as the other spot.
--
Brandon Williams
^ permalink raw reply
* Re: [PATCH 12/17] update submodules: add submodule_move_head
From: Brandon Williams @ 2017-03-09 23:40 UTC (permalink / raw)
To: Stefan Beller; +Cc: gitster, git, novalis, sandals, hvoigt, jrnieder, ramsay
In-Reply-To: <20170309221543.15897-13-sbeller@google.com>
On 03/09, Stefan Beller wrote:
> +static void submodule_reset_index(const char *path)
> +{
> + struct child_process cp = CHILD_PROCESS_INIT;
> + prepare_submodule_repo_env_no_git_dir(&cp.env_array);
> +
> + cp.git_cmd = 1;
> + cp.no_stdin = 1;
> + cp.dir = path;
> +
> + argv_array_pushf(&cp.args, "--super-prefix=%s/", path);
What happens with nested submodules? As in can we reach this code path
with super_prefix already being set? If so we would need to include
that as part of the super_prefix being passed to the child. Just
looking for some clarification.
> + argv_array_pushl(&cp.args, "read-tree", "-u", "--reset", NULL);
> +
> + argv_array_push(&cp.args, EMPTY_TREE_SHA1_HEX);
> +
> + if (run_command(&cp))
> + die("could not reset submodule index");
> +}
> +
--
Brandon Williams
^ permalink raw reply
* [PATCH] blame: move blame_entry duplication to add_blame_entry()
From: René Scharfe @ 2017-03-10 0:12 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Jeff King
All callers of add_blame_entry() allocate and copy the second argument.
Let the function do it for them, reducing code duplication.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
builtin/blame.c | 25 ++++++++-----------------
1 file changed, 8 insertions(+), 17 deletions(-)
diff --git a/builtin/blame.c b/builtin/blame.c
index cffc626540..f7aa95f4ba 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -658,8 +658,11 @@ static struct origin *find_rename(struct scoreboard *sb,
/*
* Append a new blame entry to a given output queue.
*/
-static void add_blame_entry(struct blame_entry ***queue, struct blame_entry *e)
+static void add_blame_entry(struct blame_entry ***queue,
+ const struct blame_entry *src)
{
+ struct blame_entry *e = xmalloc(sizeof(*e));
+ memcpy(e, src, sizeof(*e));
origin_incref(e->suspect);
e->next = **queue;
@@ -760,21 +763,15 @@ static void split_blame(struct blame_entry ***blamed,
struct blame_entry *split,
struct blame_entry *e)
{
- struct blame_entry *new_entry;
-
if (split[0].suspect && split[2].suspect) {
/* The first part (reuse storage for the existing entry e) */
dup_entry(unblamed, e, &split[0]);
/* The last part -- me */
- new_entry = xmalloc(sizeof(*new_entry));
- memcpy(new_entry, &(split[2]), sizeof(struct blame_entry));
- add_blame_entry(unblamed, new_entry);
+ add_blame_entry(unblamed, &split[2]);
/* ... and the middle part -- parent */
- new_entry = xmalloc(sizeof(*new_entry));
- memcpy(new_entry, &(split[1]), sizeof(struct blame_entry));
- add_blame_entry(blamed, new_entry);
+ add_blame_entry(blamed, &split[1]);
}
else if (!split[0].suspect && !split[2].suspect)
/*
@@ -785,18 +782,12 @@ static void split_blame(struct blame_entry ***blamed,
else if (split[0].suspect) {
/* me and then parent */
dup_entry(unblamed, e, &split[0]);
-
- new_entry = xmalloc(sizeof(*new_entry));
- memcpy(new_entry, &(split[1]), sizeof(struct blame_entry));
- add_blame_entry(blamed, new_entry);
+ add_blame_entry(blamed, &split[1]);
}
else {
/* parent and then me */
dup_entry(blamed, e, &split[1]);
-
- new_entry = xmalloc(sizeof(*new_entry));
- memcpy(new_entry, &(split[2]), sizeof(struct blame_entry));
- add_blame_entry(unblamed, new_entry);
+ add_blame_entry(unblamed, &split[2]);
}
}
--
2.12.0
^ permalink raw reply related
* Re: [PATCH v1] Travis: also test on 32-bit Linux
From: René Scharfe @ 2017-03-10 0:14 UTC (permalink / raw)
To: Jeff King, Vegard Nossum
Cc: Lars Schneider, Junio C Hamano, allan.x.xavier,
Johannes Schindelin, Git Mailing List
In-Reply-To: <20170305113618.ko2jymle4n5f2b5l@sigill.intra.peff.net>
Am 05.03.2017 um 12:36 schrieb Jeff King:
> I grepped for 'memcpy.*sizeof' and found one other case that's not a
> bug, but is questionable.
>
> Of the "good" cases, I think most of them could be converted into
> something more obviously-correct, which would make auditing easier. The
> three main cases I saw were:
> 3. There were a number of alloc-and-copy instances. The copy part is
> the same as (2) above, but you have to repeat the size, which is
> potentially error-prone. I wonder if we would want something like:
>
> #define ALLOC_COPY(dst, src) do { \
> (dst) = xmalloc(sizeof(*(dst))); \
> COPY_ARRAY(dst, src, 1); \
> while(0)
>
> That avoids having to specify the size at all, and triggers a
> compile-time error if "src" and "dst" point to objects of different
> sizes.
Or you could call it DUP or similar. And you could use ALLOC_ARRAY in
its definition and let it infer the size implicitly (don't worry too
much about the multiplication with one):
#define DUPLICATE_ARRAY(dst, src, n) do { \
ALLOC_ARRAY((dst), (n)); \
COPY_ARRAY((dst), (src), (n)); \
} while (0)
#define DUPLICATE(dst, src) DUPLICATE_ARRAY((dst), (src), 1)
But do we even want such a thing? Duplicating objects should be rare,
and keeping allocation and assignment/copying separate makes for more
flexible building blocks. Adding ALLOC (and CALLOC) for single objects
could be more widely useful, I think.
René
^ permalink raw reply
* Re: [PATCH v1] Travis: also test on 32-bit Linux
From: René Scharfe @ 2017-03-10 0:14 UTC (permalink / raw)
To: Jeff King, Vegard Nossum
Cc: Lars Schneider, Junio C Hamano, allan.x.xavier,
Johannes Schindelin, Git List
In-Reply-To: <20170305113618.ko2jymle4n5f2b5l@sigill.intra.peff.net>
Am 05.03.2017 um 12:36 schrieb Jeff King:
> I grepped for 'memcpy.*sizeof' and found one other case that's not a
> bug, but is questionable.
>
> Of the "good" cases, I think most of them could be converted into
> something more obviously-correct, which would make auditing easier. The
> three main cases I saw were:
> 2. Ones which just copy a single object, like:
>
> memcpy(&dst, &src, sizeof(dst));
>
> Perhaps we should be using struct assignment like:
>
> dst = src;
>
> here. It's safer and it should give the compiler more room to
> optimize. The only downside is that if you have pointers, it is
> easy to write "dst = src" when you meant "*dst = *src".
Compilers can usually inline memcpy(3) calls, but assignments are
shorter and more pleasing to the eye, and we get a type check for
free. How about this?
-- >8 --
Subject: [PATCH] cocci: use assignment operator to copy structs
Add a semantic patch for converting memcpy(3) calls targeting
addresses of variables (i.e., variables preceded by &) -- which are
basically always structs -- to simple assignments, and apply it to
the current tree. The resulting code is shorter, simpler and its
type safety is checked by the compiler.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
builtin/log.c | 2 +-
contrib/coccinelle/copy.cocci | 31 +++++++++++++++++++++++++++++++
convert.c | 2 +-
credential-cache--daemon.c | 2 +-
daemon.c | 2 +-
line-log.c | 2 +-
revision.c | 2 +-
7 files changed, 37 insertions(+), 6 deletions(-)
create mode 100644 contrib/coccinelle/copy.cocci
diff --git a/builtin/log.c b/builtin/log.c
index 55d20cc2d8..23bb9a9e76 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1030,7 +1030,7 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,
if (!origin)
return;
- memcpy(&opts, &rev->diffopt, sizeof(opts));
+ opts = rev->diffopt;
opts.output_format = DIFF_FORMAT_SUMMARY | DIFF_FORMAT_DIFFSTAT;
diff_setup_done(&opts);
diff --git a/contrib/coccinelle/copy.cocci b/contrib/coccinelle/copy.cocci
new file mode 100644
index 0000000000..f0d883932a
--- /dev/null
+++ b/contrib/coccinelle/copy.cocci
@@ -0,0 +1,31 @@
+@@
+type T;
+T dst;
+T src;
+@@
+(
+- memcpy(&dst, &src, sizeof(dst));
++ dst = src;
+|
+- memcpy(&dst, &src, sizeof(src));
++ dst = src;
+|
+- memcpy(&dst, &src, sizeof(T));
++ dst = src;
+)
+
+@@
+type T;
+T dst;
+T *src;
+@@
+(
+- memcpy(&dst, src, sizeof(dst));
++ dst = *src;
+|
+- memcpy(&dst, src, sizeof(*src));
++ dst = *src;
+|
+- memcpy(&dst, src, sizeof(T));
++ dst = *src;
+)
diff --git a/convert.c b/convert.c
index 8d652bf27c..4bae12be6b 100644
--- a/convert.c
+++ b/convert.c
@@ -290,7 +290,7 @@ static int crlf_to_git(const char *path, const char *src, size_t len,
if ((checksafe == SAFE_CRLF_WARN ||
(checksafe == SAFE_CRLF_FAIL)) && len) {
struct text_stat new_stats;
- memcpy(&new_stats, &stats, sizeof(new_stats));
+ new_stats = stats;
/* simulate "git add" */
if (convert_crlf_into_lf) {
new_stats.lonelf += new_stats.crlf;
diff --git a/credential-cache--daemon.c b/credential-cache--daemon.c
index 46c5937526..798cf33c3a 100644
--- a/credential-cache--daemon.c
+++ b/credential-cache--daemon.c
@@ -22,7 +22,7 @@ static void cache_credential(struct credential *c, int timeout)
e = &entries[entries_nr++];
/* take ownership of pointers */
- memcpy(&e->item, c, sizeof(*c));
+ e->item = *c;
memset(c, 0, sizeof(*c));
e->expiration = time(NULL) + timeout;
}
diff --git a/daemon.c b/daemon.c
index 473e6b6b63..f891398aad 100644
--- a/daemon.c
+++ b/daemon.c
@@ -785,7 +785,7 @@ static void add_child(struct child_process *cld, struct sockaddr *addr, socklen_
newborn = xcalloc(1, sizeof(*newborn));
live_children++;
- memcpy(&newborn->cld, cld, sizeof(*cld));
+ newborn->cld = *cld;
memcpy(&newborn->address, addr, addrlen);
for (cradle = &firstborn; *cradle; cradle = &(*cradle)->next)
if (!addrcmp(&(*cradle)->address, &newborn->address))
diff --git a/line-log.c b/line-log.c
index 65f3558b3b..64f141e200 100644
--- a/line-log.c
+++ b/line-log.c
@@ -1093,7 +1093,7 @@ static int process_all_files(struct line_log_data **range_out,
rg = rg->next;
assert(rg);
rg->pair = diff_filepair_dup(queue->queue[i]);
- memcpy(&rg->diff, pairdiff, sizeof(struct diff_ranges));
+ rg->diff = *pairdiff;
}
free(pairdiff);
}
diff --git a/revision.c b/revision.c
index b37dbec378..289977c796 100644
--- a/revision.c
+++ b/revision.c
@@ -2738,7 +2738,7 @@ int prepare_revision_walk(struct rev_info *revs)
struct object_array old_pending;
struct commit_list **next = &revs->commits;
- memcpy(&old_pending, &revs->pending, sizeof(old_pending));
+ old_pending = revs->pending;
revs->pending.nr = 0;
revs->pending.alloc = 0;
revs->pending.objects = NULL;
--
2.12.0
^ permalink raw reply related
* Re: Possible bug: git pull --rebase discards local commits
From: Igor Djordjevic @ 2017-03-10 0:23 UTC (permalink / raw)
To: Junio C Hamano, Joshua Phillips; +Cc: git, Laszlo Kiss
In-Reply-To: <xmqqvayruvwk.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On 23/08/2016 21:28, Junio C Hamano wrote:
> Joshua Phillips <jphillips@imap.cc> writes:
> > I've found a case where git pull --rebase discards commits in my branch
> > if the remote-tracking branch was rewound (and the remote tracking
> > branch's reflog contains my branch's latest commit). This is due to
> > git-pull's usage of git merge-base --fork-point.
> >
> > On one hand, this behaviour might be correct since the remote repository
> > essentially removed that commit from master by 'reset --hard'. On the
> > other hand, I was surprised that git pull --rebase discarded a commit in
> > my branch.
>
> Yup, that sounds like a bad way to handle the situation. After all,
> the upstream may have first accepted your first attempt, and then
> decided that it was premature and rewound it, expecting you to give
> an improved reroll. But I also agree with you that it may be
> correct to drop it because the upstream already rejected it.
>
> Since Git cannot tell between these two cases, we should play safer
> than what the current code does, I would think.
Were there any news in this regards so far? Would it be (more)
sensible/safe to report the dropped commits, too? Something like:
Dropping: Commit B
Applying: Commit X
Applying: Commit Y
... where "Applying: *" are standard "git rebase" output lines, and
"Dropping: *" a newly proposed one (example graphs (1.1) to (1.3)
shown below[1]).
That said, applied commits might even be considered pretty
uninteresting here (as they`re kept/transferred over anyway), a noise
drowning what otherwise might be really important - the dropped/lost
ones...?
It does feel a bit scary learning that you may _silently_ lose
commits, especially as --fork-point is used by default for both
vanilla `git rebase` and `git pull --rebase`.
P.S. As a relatively new user, I actually just got aware of this
behavior from another, recently posted e-mail[2], having me
investigate further, yet thought replying here might be better as it
got some attention already (adding author of that other e-mail,
Laszlo Kiss, to Cc).
Regards,
Buga
[1] Example graphs:
(1.1) ---A---B (master)
\
X---Y (topic)
(1.2) C---D (master)
/
---A---B
\
X---Y (topic)
(1.3) X'---Y' (topic)
/
C---D (master)
/
---A
Note that I didn`t use "origin/master" but just "master" on purpose,
as the branch being tracked doesn`t have to be a remote one, making
the lost local commits confusion even greater.
[2] https://public-inbox.org/git/CAO0LFki4PN8zz2xpoSpjTHJGS=NG_suQYR27EcmzEMiaCw9kuA@mail.gmail.com/
^ permalink raw reply
* Re: [media] omap3isp: Correctly set IO_OUT_SEL and VP_CLK_POL for CCP2 mode
From: Fengguang Wu @ 2017-03-10 2:49 UTC (permalink / raw)
To: Pavel Machek
Cc: kbuild-all, Laurent Pinchart, Sakari Ailus, mchehab, kernel list,
ivo.g.dimitrov.75, sre, pali.rohar, linux-media, git, Ye Xiaolong
In-Reply-To: <20170303214838.GA26826@amd>
On Fri, Mar 03, 2017 at 10:48:38PM +0100, Pavel Machek wrote:
>Hi!
>
>> [auto build test ERROR on linuxtv-media/master]
>> [also build test ERROR on v4.10 next-20170303]
>> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>>
>
>Yes, the patch is against Sakari's ccp2 branch. It should work ok there.
>
>I don't think you can do much to fix the automated system....
We could, if "git format-patch" can be setup to auto append lines
parent-commit: X
parent-patch-id: Y
With that information, as long as the parent commit/patch is public --
either by "git push" or posting patch to mailing lists -- we'll have
good chance to find and use it as the base for "git am".
Currently "git format-patch" already has the option "--base=auto" to
auto append the more accurate lines
base-commit: P
prerequisite-patch-id: X
prerequisite-patch-id: Y
prerequisite-patch-id: Z
That's the best information git can offer. Unfortunately it cannot
ALWAYS work without human aid. What's worse, when it cannot figure out
the base-commit, the whole "git format-patch" command will abort like
this
$ git format-patch -1
fatal: base commit shouldn't be in revision list
That fatal error makes it not a viable option to always turn on
"--base=auto" in .gitconfig.
Without a fully-automated solution, I don't think many people will
bother or remember to manually specify base-commit before sending
patches out.
To effectively save the robot from "base commit" guessing works, what
we can do is to
1) append "parent-commit"/"parent-patch-id" lines when git cannot
figure out and append the "base-commit"/"prerequisite-patch-id"
lines. So that the test robot always get the information to do
its job.
2) advise kernel developers to run this once
git config format.useAutoBase yes
to configure "--base=auto" as the default behavior.
Thanks,
Fengguang
^ permalink raw reply
* Re: [PATCH GSoC] Allow "-" as a short-hand for "@{-1}" in branch deletions
From: Siddharth Kannan @ 2017-03-10 3:30 UTC (permalink / raw)
To: Shuyang Shi; +Cc: Stefan Beller, Matthieu Moy, git@vger.kernel.org
In-Reply-To: <0102015ab11ee091-f9f11bb5-559a-4c92-b5f6-9f7755e8f4b9-000000@eu-west-1.amazonses.com>
Hey Shuyang,
On Thu, Mar 09, 2017 at 09:47:12AM -0800, Stefan Beller wrote:
> > The "-" shorthand that stands for "the branch we were previously on",
> > like we did for "git merge -" sometime after we introduced "git checkout -".
> > Now I am introducing this shorthand to branch delete, i.e.
> > "git branch -d -".
> >
> > More reference:
> > https://public-inbox.org/git/7vppuewl6h.fsf@alter.siamese.dyndns.org/
>
1. I have already worked on this project, and my patch is in the
"Needs review" section in "What's cooking". It implements this change
inside sha1_name.c and doesn't touch git branch. So, your patch is
mutually exclusive to my previous patch.
2. Matthieu made an argument against enabling commands like "git
branch -D -" even by mistake [1]. The way that I have implemented
ensured that not a lot of "rm"-like commands were enabled.
My patch that would enable this shorthand for other projects is
here[2].
[1]: http://public-inbox.org/git/vpqh944eof7.fsf@anie.imag.fr/
[2]: http://public-inbox.org/git/1488007487-12965-5-git-send-email-kannan.siddharth12@gmail.com/
Thanks,
Siddharth.
^ permalink raw reply
* RE: [PATCH v2 GSoC RFC] diff: allow "-" as a short-hand for "last branch"
From: mash @ 2017-03-10 4:52 UTC (permalink / raw)
To: Siddharth Kannan
Cc: Junio C Hamano, Vegard Nossum, stepnem, Stefan Beller,
Vedant Bassi, Prathamesh Chavan, Matthieu Moy, Git Mailing List
In-Reply-To: <1cm4dm-0007OE-MZ@crossperf.com>
> From the discussion over the different versions of my patch, I get
> the feeling that enabling this shorthand for all the commands is the
> direction that git wants to move in.
Interesting.
> Sorry about the time you spent on this patch.
Don't worry about it. I just seem to be too stupid to search through the
mailing list archive properly.
Maybe you can reuse the diff tests. I'll do another microproject then.
mash
The original message doesn't seem to cc the mailing list:
> Hey, I have already worked on this, and I made the change inside
> sha1_name.c.
> The final version of my patch is here[1].
> > Handling the dash in sha1_name:get_sha1_basic is not an issue but git
> > was designed with the dash in mind for options not for this weird
> > short-hand so as long as there's no decision made that git should
> > actually have this short-hand everywhere it does not seem like a good
> > idea to change anything in there because it would probably have
> > unwanted side-effects.
> Actually, this was discussed even when I was working on this patch.
> I said [2]
> > Making a change in sha1_name.c will touch a lot of commands
> > (setup_revisions is called from everywhere in the codebase), so, I am
> > still trying to figure out how to do this such that the rest of the
> > codepath remains unchanged.
> Matthieu replied to this [3]
> > I don't have strong opinion on this: I tend to favor consistency and
> > supporting "-" everywhere goes in this direction, but I think the
> > downsides should be considered too. A large part of the exercice here
> > is to write a good commit message!
> From the discussion over the different versions of my patch, I get
> the feeling that enabling this shorthand for all the commands is the
> direction that git wants to move in.
> Sorry about the time you spent on this patch.
> [1]: http://public-inbox.org/git/1488007487-12965-1-git-send-email-kannan.siddharth12@gmail.com/
> [2]: https://public-inbox.org/git/20170207191450.GA5569@ubuntu-512mb-blr1-01.localdomain/
> [3]: https://public-inbox.org/git/vpqh944eof7.fsf@anie.imag.fr/
> Thanks,
> Siddharth.
^ permalink raw reply
* [PATCH v2 GSoC RFC] diff: allow "-" as a short-hand for "last branch"
From: Siddharth Kannan @ 2017-03-10 4:59 UTC (permalink / raw)
To: mash
Cc: Git Mailing List, Junio C Hamano, Vegard Nossum, stepnem,
Stefan Beller, Vedant Bassi, Prathamesh Chavan, Matthieu Moy
Hey, I have already worked on this, and I made the change inside
sha1_name.c.
The final version of my patch is here[1].
> Handling the dash in sha1_name:get_sha1_basic is not an issue but
> git
> was designed with the dash in mind for options not for this weird
> short-hand so as long as there's no decision made that git should
> actually have this short-hand everywhere it does not seem like a
> good
> idea to change anything in there because it would probably have
> unwanted side-effects.
Actually, this was discussed even when I was working on this patch.
I said [2]
> Making a change in sha1_name.c will touch a lot of commands
> (setup_revisions is called from everywhere in the codebase), so, I
> am
> still trying to figure out how to do this such that the rest of the
> codepath remains unchanged.
Matthieu replied to this [3]
> I don't have strong opinion on this: I tend to favor consistency and
> supporting "-" everywhere goes in this direction, but I think the
> downsides should be considered too. A large part of the exercice
> here
> is to write a good commit message!
From the discussion over the different versions of my patch, I get
the feeling that enabling this shorthand for all the commands is the
direction that git wants to move in.
Sorry about the time you spent on this patch.
[1]: http://public-inbox.org/git/1488007487-12965-1-git-send-email-kannan.siddharth12@gmail.com/
[2]: https://public-inbox.org/git/20170207191450.GA5569@ubuntu-512mb-blr1-01.localdomain/
[3]: https://public-inbox.org/git/vpqh944eof7.fsf@anie.imag.fr/
Thanks,
Siddharth.
P.S. This message was sent _before_ 1cmCXH-0000ND-9K@crossperf.com but
I didn't CC The mailing list in that message. I am sending it with the
mailing list cc-ed to ensure that the conversation makes sense.
^ permalink raw reply
* Re: [PATCH v2 GSoC RFC] diff: allow "-" as a short-hand for "last branch"
From: Siddharth Kannan @ 2017-03-10 5:00 UTC (permalink / raw)
To: mash
Cc: Junio C Hamano, Vegard Nossum, stepnem, Stefan Beller,
Vedant Bassi, Prathamesh Chavan, Matthieu Moy, Git Mailing List
In-Reply-To: <1cmCXH-0000ND-9K@crossperf.com>
On Fri, Mar 10, 2017 at 04:52:07AM +0000, mash wrote:
> Maybe you can reuse the diff tests. I'll do another microproject then.
Yeah, definitely. If there are more tests required, then I will reuse
your ones!
>
> mash
>
> The original message doesn't seem to cc the mailing list:
Thanks! It was rather daft of me to not realise this. I was waiting
for it to appear on public-inbox.
I re-sent it with the CC. The timestamp is a little bit
skewed, but I think it should make sense.
Thanks,
Siddharth.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox