* [PATCH v4 05/10] Introduce the discover_git_directory() function
From: Johannes Schindelin @ 2017-03-07 14:33 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Duy Nguyen
In-Reply-To: <cover.1488897111.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 d7af343d14e..486acda2054 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)
{
struct strbuf cwd = STRBUF_INIT, dir = STRBUF_INIT, gitdir = STRBUF_INIT;
--
2.12.0.windows.1.7.g94dafc3b124
^ permalink raw reply related
* [PATCH v4 06/10] Make read_early_config() reusable
From: Johannes Schindelin @ 2017-03-07 14:33 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Duy Nguyen
In-Reply-To: <cover.1488897111.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 v4 08/10] read_early_config(): really discover .git/
From: Johannes Schindelin @ 2017-03-07 14:33 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Duy Nguyen
In-Reply-To: <cover.1488897111.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 v4 10/10] setup_git_directory_gently_1(): avoid die()ing
From: Johannes Schindelin @ 2017-03-07 14:33 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Duy Nguyen
In-Reply-To: <cover.1488897111.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 486acda2054..9118b48590a 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;
}
@@ -993,7 +1003,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
* Re: [PATCH 0/6] deadlock regression in v2.11.0 with failed mkdtemp
From: Jeff King @ 2017-03-07 13:50 UTC (permalink / raw)
To: Horst Schirmeier; +Cc: git
In-Reply-To: <20170307133437.qee2jtynbiwf6uzr@sigill.intra.peff.net>
On Tue, Mar 07, 2017 at 08:34:37AM -0500, Jeff King wrote:
> Yuck. In the original, the error was generated by the child index-pack,
> and we relayed it over the sideband. But we don't even get as far as
> running index-pack in the newer version; we fail trying to make the
> tmpdir. The error ends up in the "unpack" protocol field, but the client
> side does a bad job of showing that. With the rest of the patches, it
> looks like:
>
> $ git push ~/tmp/foo.git HEAD
> Counting objects: 210973, done.
> Delta compression using up to 8 threads.
> Compressing objects: 100% (52799/52799), done.
> error: remote unpack failed: unable to create temporary object directory
> error: failed to push some refs to '/home/peff/tmp/foo.git'
There are two other options here I should mention.
One is that when pack-objects dies, we suppress the ref-status table
entirely. Which is fair, because it doesn't have anything interesting to
say. But for a case like this where the other side stops reading our
pack early but still produces status reports, we could actually read
them all and have a status table like:
$ git push ~/tmp/foo.git HEAD
Counting objects: 209843, done.
Delta compression using up to 8 threads.
Compressing objects: 100% (52186/52186), done.
error: remote unpack failed: unable to create temporary object directory
To /home/peff/tmp/foo.git
! [remote rejected] HEAD -> jk/push-index-pack-deadlock (unpacker error)
error: failed to push some refs to '/home/peff/tmp/foo.git'
We'd have to take some care to make sure we handle the case when the
remote _doesn't_ manage to give us the status (e.g., when there's a
complete hangup). I don't know if it's worth the effort. There's no new
information there.
The second thing is that I think the design of the "unpack <reason>"
report is a bit dated. These days everybody supports the sideband
protocol, so it would probably make more sense to just issue the
"<reason>" report via the sideband (at which point it gets a nice
"remote: " prefix).
We could do something like this:
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index f2c6953a3..6204d3d00 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1670,6 +1670,8 @@ static const char *unpack(int err_fd, struct shallow_info *si)
if (!tmp_objdir) {
if (err_fd > 0)
close(err_fd);
+ rp_error("unable to create temporary object directory: %s",
+ strerror(errno));
return "unable to create temporary object directory";
}
child.env = tmp_objdir_env(tmp_objdir);
and drop the "try to show the unpack failure" parts of my series, and
you'd end up with:
$ git push ~/tmp/foo.git HEAD
Counting objects: 209843, done.
Delta compression using up to 8 threads.
Compressing objects: 100% (52186/52186), done.
remote: error: unable to create temporary object directory: Permission denied
error: failed to push some refs to '/home/peff/tmp/foo.git'
but in cases where pack-objects _doesn't_ fail, existing git versions
do show the "unpack" error. So you'd see it twice.
I don't know if it's worth trying to hack around.
-Peff
^ permalink raw reply related
* [PATCH v4 09/10] Test read_early_config()
From: Johannes Schindelin @ 2017-03-07 14:33 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Duy Nguyen
In-Reply-To: <cover.1488897111.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 v4 07/10] read_early_config(): avoid .git/config hack when unneeded
From: Johannes Schindelin @ 2017-03-07 14:33 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Duy Nguyen
In-Reply-To: <cover.1488897111.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
* Re: [PATCH v3 0/9] Fix the early config
From: Johannes Schindelin @ 2017-03-07 15:18 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jeff King, Duy Nguyen
In-Reply-To: <xmqqpohy6o2a.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Fri, 3 Mar 2017, Junio C Hamano wrote:
> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
>
> > Notable notes:
> >
> > - In contrast to earlier versions, I no longer special-case init and
> > clone. Peff pointed out that this adds technical debt, and that we can
> > actually argue (for consistency's sake) that early config reads the
> > current repository config (if any) even for init and clone.
> >
> > - The read_early_config() function does not cache Git directory
> > discovery nor read values. If needed, this can be implemented later,
> > in a separate patch series.
> >
> > - The alias handling in git.c could possibly benefit from this work, but
> > again, this is a separate topic from the current patch series.
>
> As Peff said in his review, I too find the result of this series a
> more pleasant read than than original.
As do I.
> 2/9 and corresponding 4/9 triggers "ERROR: trailing statements
> should be on next line" from ../linux/scripts/checkpatch.pl because
> of a line inherited from the original; I'll queue them with an
> obvious style fix to work it around.
Thank you. I'll try to pick it up for v3 (which is needed, as I found
another issue that needs to be fixed).
Ciao,
Johannes
^ permalink raw reply
* Re: Reg : GSoC 2017 Microproject
From: Prathamesh Chavan @ 2017-03-07 15:51 UTC (permalink / raw)
To: Vedant Bassi; +Cc: git
In-Reply-To: <CACczA6WCdu0sdd31R2Z6xbr=meo5PTtcOVYCdVHdgZXAfK-3rg@mail.gmail.com>
On Tue, Mar 7, 2017 at 3:52 PM, Vedant Bassi <sharababy.dev@gmail.com> wrote:
> Hi,
>
> I would like to participate in GSoC 2017 and I have chosen the Use
> unsigned integral type for collection of bits , idea from the Micro
> projects list.
>
> I request the help of the community for clarifying a few questions that I have.
>
> 1. Is this Microproject already taken ?
>
> 2. If it is free , I would like to point out one place where a signed
> int is used .
>
> In bisect.h , the structure struct rev_list_info uses flags of
> type signed int but , the value of MSB is not checked as a test case
> for any error checking. Hence it can be of type unsigned int.
> It is only used in rev-list.c for checking cases (BISECT_SHOW_ALL and
> REV_LIST_QUIET ).
>
> Is this a valid case.
>
> Thanks.
You can search your microproject on public inbox of git to check if it
has already
been taken or not.
^ permalink raw reply
* [PATCH] t*: avoid using pipes
From: Prathamesh Chavan @ 2017-03-07 16:10 UTC (permalink / raw)
To: git; +Cc: Pranit Bauva
Hi,
I'm Prathamesh Chavan. As a part of my micropraoject I have been working on
"Avoid pipes for git related commands in test suites". I tried sending the
patch, but it got blocked since the mail contained more than 100 000
characters.
Hence I'll like to attach the link to my branch 'micro-proj', where I did the
required changes.
https://github.com/pratham-pc/git/tree/micro-proj
Thanks.
^ permalink raw reply
* [PATCH] repack: Add options to preserve and prune old pack files
From: James Melvin @ 2017-03-07 16:40 UTC (permalink / raw)
To: git; +Cc: nasserg, mfick, peff, sbeller, James Melvin
The new --preserve-oldpacks option moves old pack files into the
preserved subdirectory instead of deleting them after repacking.
The new --prune-preserved option prunes old pack files from the
preserved subdirectory after repacking, but before potentially
moving the latest old packfiles to this subdirectory.
These options are designed to prevent stale file handle exceptions
during git operations which can happen on users of NFS repos when
repacking is done on them. The strategy is to preserve old pack files
around until the next repack with the hopes that they will become
unreferenced by then and not cause any exceptions to running processes
when they are finally deleted (pruned).
Signed-off-by: James Melvin <jmelvin@codeaurora.org>
---
Documentation/git-repack.txt | 9 +++++++++
builtin/repack.c | 38 ++++++++++++++++++++++++++++++++++++--
2 files changed, 45 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-repack.txt b/Documentation/git-repack.txt
index 26afe6ed5..0b19b761f 100644
--- a/Documentation/git-repack.txt
+++ b/Documentation/git-repack.txt
@@ -143,6 +143,15 @@ other objects in that pack they already have locally.
being removed. In addition, any unreachable loose objects will
be packed (and their loose counterparts removed).
+--preserve-oldpacks::
+ Move old pack files into the preserved subdirectory instead
+ of deleting them after repacking.
+
+--prune-preserved::
+ Prune old pack files from the preserved subdirectory after
+ repacking, but before potentially moving the latest old
+ packfiles to this subdirectory
+
Configuration
-------------
diff --git a/builtin/repack.c b/builtin/repack.c
index 677bc7c81..f1a0c97f3 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -10,8 +10,10 @@
static int delta_base_offset = 1;
static int pack_kept_objects = -1;
+static int preserve_oldpacks = 0;
+static int prune_preserved = 0;
static int write_bitmaps;
-static char *packdir, *packtmp;
+static char *packdir, *packtmp, *preservedir;
static const char *const git_repack_usage[] = {
N_("git repack [<options>]"),
@@ -108,6 +110,27 @@ static void get_non_kept_pack_filenames(struct string_list *fname_list)
closedir(dir);
}
+static void preserve_pack(const char *file_path, const char *file_name, const char *file_ext)
+{
+ char *fname_old;
+
+ if (mkdir(preservedir, 0700) && errno != EEXIST)
+ error(_("failed to create preserve directory"));
+
+ fname_old = mkpathdup("%s/%s.old-%s", preservedir, file_name, ++file_ext);
+ rename(file_path, fname_old);
+
+ free(fname_old);
+}
+
+static void remove_preserved_dir(void) {
+ struct strbuf buf = STRBUF_INIT;
+
+ strbuf_addstr(&buf, preservedir);
+ remove_dir_recursively(&buf, 0);
+ strbuf_release(&buf);
+}
+
static void remove_redundant_pack(const char *dir_name, const char *base_name)
{
const char *exts[] = {".pack", ".idx", ".keep", ".bitmap"};
@@ -121,7 +144,10 @@ static void remove_redundant_pack(const char *dir_name, const char *base_name)
for (i = 0; i < ARRAY_SIZE(exts); i++) {
strbuf_setlen(&buf, plen);
strbuf_addstr(&buf, exts[i]);
- unlink(buf.buf);
+ if (preserve_oldpacks)
+ preserve_pack(buf.buf, base_name, exts[i]);
+ else
+ unlink(buf.buf);
}
strbuf_release(&buf);
}
@@ -194,6 +220,10 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
N_("maximum size of each packfile")),
OPT_BOOL(0, "pack-kept-objects", &pack_kept_objects,
N_("repack objects in packs marked with .keep")),
+ OPT_BOOL(0, "preserve-oldpacks", &preserve_oldpacks,
+ N_("move old pack files into the preserved subdirectory")),
+ OPT_BOOL(0, "prune-preserved", &prune_preserved,
+ N_("prune old pack files from the preserved subdirectory after repacking")),
OPT_END()
};
@@ -217,6 +247,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
packdir = mkpathdup("%s/pack", get_object_directory());
packtmp = mkpathdup("%s/.tmp-%d-pack", packdir, (int)getpid());
+ preservedir = mkpathdup("%s/preserved", packdir);
sigchain_push_common(remove_pack_on_signal);
@@ -404,6 +435,9 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
/* End of pack replacement. */
+ if (prune_preserved)
+ remove_preserved_dir();
+
if (delete_redundant) {
int opts = 0;
string_list_sort(&names);
--
2.12.0.190.gab6997d48.dirty
^ permalink raw reply related
* Re: [PATCH] t*: avoid using pipes
From: Stefan Beller @ 2017-03-07 17:21 UTC (permalink / raw)
To: Prathamesh Chavan; +Cc: git@vger.kernel.org, Pranit Bauva
In-Reply-To: <CAME+mvUe7itzg7JLu9_131smzHHE0JsN-z7q8_dTY1qEdugYWw@mail.gmail.com>
On Tue, Mar 7, 2017 at 8:10 AM, Prathamesh Chavan <pc44800@gmail.com> wrote:
> Hi,
> I'm Prathamesh Chavan. As a part of my micropraoject I have been working on
> "Avoid pipes for git related commands in test suites".
Thanks for working on that microproject!
> I tried sending the
> patch, but it got blocked since the mail contained more than 100 000
> characters.
Yeah, even the github UI seems to have trouble with that commit.
(A bit slow, not showing the full content, but rather I needed to click
on "load diff" for tests files 7000+)
This is a lot of change (in terms of lines) for a micro project. :)
I'd have two competing advices:
* keep it micro first, e.g. just convert one file,
send to list, wait for reviewers feedback and incorporate that
(optional step after having done the full development cycle:
convert all the other files; each as its own patch)
* split up this one patch into multiple patches, e.g. one
file per patch, then send a patch series.
The outcome will be the same, but in the first you get feedback
quicker, such that hopefully you only need to touch the rest of
files after the first file just once.
> Hence I'll like to attach the link to my branch 'micro-proj', where I did the
> required changes.
>
> https://github.com/pratham-pc/git/tree/micro-proj
While I did look at that, not everyone here in the git community
does so. (Also for getting your change in, Junio seems to strongly prefer
patches on list instead of e.g. fetching and cherry-picking from your
github)
When looking at the content, the conversion seems a bit mechanical
(which is fine for a micro project), such as:
...
- test "$(git show --pretty=format:%s | head -n 1)" = "one"
+ test "$(git show --pretty=format:%s >out && head -n 1 <out)" = "one"
...
specifically for the "head" command I don't think it makes a
difference in correctness whether you pipe the file into the tool
or give the filename, i.e. "head -n 1 out" would work just as fine.
There is a difference in readability, though. For consistency I'd
suggest to drop the "<", as the numbers might support:
$ cd t
$ git grep head |wc -l
# This also counts other occurrences of the string,
# not just the invocation of the head tool
2871
$ git grep head |grep "<" |wc -l
# same here
58
Another aspect might be performance at scale as the "<" will
let the shell open the file and pipe the content via stdin to the
head tool, whereas when giving a filename the head tool needs
to open the file. Both times the file doesn't need to be read completely,
but "head -n 1" can close the file handle early in the game.
I dunno.
Thanks,
Stefan
>
> Thanks.
^ permalink raw reply
* [RESEND PATCH] git-gui--askpass: generalize the window title
From: Sebastian Schuberth @ 2017-03-07 15:48 UTC (permalink / raw)
To: git
In-Reply-To: <000001529cbe5436-285f0113-5761-49d8-8961-5a9df0180ed7-000000@eu-west-1.amazonses.com>
git-gui--askpass is not only used for SSH authentication, but also for
HTTPS. In that context it is confusing to have a window title of
"OpenSSH". So generalize the title so that it also says which parent
process, i.e. Git, requires authentication.
Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com>
---
git-gui/git-gui--askpass | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/git-gui/git-gui--askpass b/git-gui/git-gui--askpass
index 4277f30..1e5c3256 100755
--- a/git-gui/git-gui--askpass
+++ b/git-gui/git-gui--askpass
@@ -60,7 +60,7 @@ proc finish {} {
set ::rc 0
}
-wm title . "OpenSSH"
+wm title . "Git Authentication"
tk::PlaceWindow .
vwait rc
exit $rc
--
https://github.com/git/git/pull/195
^ permalink raw reply related
* Re: [RESEND PATCH] git-gui--askpass: generalize the window title
From: Stefan Beller @ 2017-03-07 18:30 UTC (permalink / raw)
To: Sebastian Schuberth, Pat Thoyts; +Cc: git@vger.kernel.org
In-Reply-To: <0102015aa974d7a6-46afa73b-4378-4b01-9db4-723fb9e41d65-000000@eu-west-1.amazonses.com>
https://public-inbox.org/git/xmqq60jz2xry.fsf@gitster.mtv.corp.google.com/
Although the following are included in git.git repository, they have their
own authoritative repository and maintainers:
- git-gui/ comes from git-gui project, maintained by Pat Thoyts:
git://repo.or.cz/git-gui.git
I cc'd Pat.
Thanks,
Stefan
^ permalink raw reply
* Re: fatal error when diffing changed symlinks
From: Junio C Hamano @ 2017-03-07 18:16 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Jeff King, git, Christophe Macabiau
In-Reply-To: <alpine.DEB.2.20.1702251336420.3767@virtualbox>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> > When viewing a working tree file, oid.hash could be 0{40} and
>> > read_sha1_file() is not the right function to use to obtain the
>> > contents.
>> >
>> > Both of these two need to pay attention to 0{40}, I think, as the
>> > user may be running "difftool -R --dir-diff" in which case the
>> > working tree would appear in the left hand side instead.
>>
>> As a side note, I think even outside of 0{40}, this should be checking
>> the return value of read_sha1_file(). A corrupted repo should die(), not
>> segfault.
>
> I agree. I am on it.
Friendly ping, if only to make sure that we can keep a piece of this
thread in the more "recent" pile.
If you have other topics you need to perfect, I think it is OK to
postpone the fix on this topic a bit longer, but I'd hate to ship
two releases with a known breakage without an attempt to fix it, so
if you are otherwise occupied, I may encourage others (including me)
to take a look at this. The new "difftool" also has a reported
regression somebody else expressed willingness to work on, which is
sort of blocked by everybody else not knowing the timeline on this
one. cf. <20170303212836.GB13790@arch-attack.localdomain>
A patch series would be very welcome, but "Please go ahead if
somebody else has time, and I'll help reviewing" would be also
good.
Thanks.
^ permalink raw reply
* [PATCH 3/6] send-pack: use skip_prefix for parsing unpack status
From: Jeff King @ 2017-03-07 13:36 UTC (permalink / raw)
To: Horst Schirmeier; +Cc: git
In-Reply-To: <20170307133437.qee2jtynbiwf6uzr@sigill.intra.peff.net>
This avoids repeating ourselves, and the use of magic
numbers.
Signed-off-by: Jeff King <peff@peff.net>
---
Obviously not necessary, but just a cleanup while I was here.
send-pack.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/send-pack.c b/send-pack.c
index 12e229e44..243633da1 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -133,10 +133,10 @@ static int pack_objects(int fd, struct ref *refs, struct sha1_array *extra, stru
static int receive_unpack_status(int in)
{
const char *line = packet_read_line(in, NULL);
- if (!starts_with(line, "unpack "))
+ if (!skip_prefix(line, "unpack ", &line))
return error("did not receive remote status");
- if (strcmp(line, "unpack ok"))
- return error("unpack failed: %s", line + 7);
+ if (strcmp(line, "ok"))
+ return error("unpack failed: %s", line);
return 0;
}
--
2.12.0.429.gde83c8049
^ permalink raw reply related
* Re: [RFC PATCH] rev-parse: add --show-superproject-working-tree
From: Junio C Hamano @ 2017-03-07 18:44 UTC (permalink / raw)
To: Stefan Beller; +Cc: szeder.dev, email, git, sandals, ville.skytta
In-Reply-To: <20170307034553.10770-1-sbeller@google.com>
Stefan Beller <sbeller@google.com> writes:
> +const char *get_superproject_working_tree()
const char *get_superproject_working_tree(void)
The same for the *.h file declaration.
> +{
> + struct child_process cp = CHILD_PROCESS_INIT;
> + struct strbuf sb = STRBUF_INIT;
> +
> + if (!superproject_exists())
> + return NULL;
> + ...
> + return strbuf_detach(&sb, NULL);
Having reviewed it, I somehow think you do not want to have a
separate superproject_exists() that grabs some part of the
information this caller needs and then discards it.
The helper already does these things:
- xgetcwd(), which may give you "/local/repo/super/sub/dir"
- relative_path() with the result and "..", which may give you
"dir"
- ls-tree HEAD "dir" to see what is in "sub/dir" of the
repository that governs ".."; if "sub/dir" is a gitlink,
you know you started in a working tree of a repository
different from the one that governs "..".
And the caller is trying to figure out where the root of the
superproject is, i.e. "/local/repo/super", and the helper has half
of the answer to that already. If you ask the "ls-tree HEAD" (as I
said, I think it should be "ls-files") to give you not "dir" but
"sub/dir", you can subtract it from the result of xgetcwd() you did
at the beginning of the helper, and that gives what this caller of
the helper wants.
So perhaps your superproject_exists() helper can be eliminated and
instead coded in get_superproject_working_tree() in place to do:
- xgetcwd() to get "/local/repo/super/sub/dir".
- relative_path() to get "dir".
- ask "ls-{tree,files} --full-name HEAD dir" to get "160000"
and "sub/dir".
- subtract "sub/dir" from the tail of the "/local/repo/super/sub/dir"
you got from xgetcwd() earlier.
- return the result.
with a failure/unmet expectations (like not finding 160000) from any
step returning an error, or something like that.
^ permalink raw reply
* Re: [PATCH v5 1/1] config: add conditional include
From: Stefan Beller @ 2017-03-07 18:39 UTC (permalink / raw)
To: Jeff King
Cc: Nguyễn Thái Ngọc Duy, git@vger.kernel.org,
Junio C Hamano, Sebastian Schuberth, Matthieu Moy
In-Reply-To: <20170307084717.i2jru77v3rhd443e@sigill.intra.peff.net>
On Tue, Mar 7, 2017 at 12:47 AM, Jeff King <peff@peff.net> wrote:
> On Mon, Mar 06, 2017 at 02:44:27PM -0800, Stefan Beller wrote:
>
>> > +static int include_condition_is_true(const char *cond, size_t cond_len)
>> > +{
>> ...
>> > +
>> > + error(_("unrecognized include condition: %.*s"), (int)cond_len, cond);
>> > + /* unknown conditionals are always false */
>> > + return 0;
>> > +}
>>
>> Thanks for putting an error message here. I was looking at what
>> is currently queued as origin/nd/conditional-config-include,
>> which doesn't have this error() (yet / not any more?)
>
> It's "not any more". It was in the original and I asked for it to be
> removed during the last review.
Okay. The joys of contradicting opinions on a mailing list. :)
>
>> I'd strongly suggest to keep the error message here as that way
>> a user can diagnose e.g. a typo in the condition easily.
>>
>> If we plan to extend this list of conditions in the future, and a user
>> switches between versions of git, then they may see this message
>> on a regular basis (whenever they use the 'old' version).
>
> That would make it unlike the rest of the config-include mechanism
> (which quietly ignores things it doesn't understand, like include.foo,
> or include.foo.path), as well as the config code in general (which
> ignores misspelt keys).
>
> Some of that "quiet when you don't understand it" is historical
> necessity. Older versions _can't_ complain about not knowing
> include.path, because they don't yet know it's worth complaining about.
agreed
> Likewise here, if this ships in v2.13 and a new condition "foo:" ships
> in v2.14, you get:
>
> v2.12 - no complaint; we don't even understand includeIf at all
> v2.13 - complain; we know includeIf, but not "foo:"
> v2.14 - works as expected
>
> Which is kind of weird and inconsistent. But maybe the typo-detection
> case is more important to get right than consistency across historical
> versions.
Oh, I see. I was contemplating a future in which 2.12 is not used anymore.
When looking at other examples, such as url.<...>.insteadOf we also do not
warn about typos (well we can't actually).
In diff.<driver>.(command/binary/..) we know the limited set of drivers,
which is similar to the situation we have here.
Maybe a compromise between typo checking (edit distance < 2 -> warn;
silent for larger distances) and the consistency over time is desired.
But this is even more code to write.
So for now I retract my strong opinion and be happy with what is
presented as-is for the reasons Peff gave.
Thanks,
Stefan
^ permalink raw reply
* Re: git init --separate-git-dir does not update symbolic .git links for submodules
From: Valery Tolstov @ 2017-03-07 18:59 UTC (permalink / raw)
To: sbeller; +Cc: git, gitster, me, sven
I think we can reuse code from module_clone that writes .git link.
Possibly this code fragment needs to be factored out from module_clone
Also, to fix all the links, we need to obtain the list of submodules
and then iterate over them. module_list command iterates
and prints the result to stdout. Maybe we can reuse this output.
Or create separate function that returns the list of submodules.
Can we call these functions from submodule--helper inside init command,
or run them thorugh internal command interface?
In my opinion, make submodule--helper fully responsible for link fixes
would be a good solution. Then we create two additional function, one
that fixes all submodules on the current level, and another that
fixes individual submodule.
Although it looks good, I'm not quite sure that it's really good.
So, maybe we can do link fixes like this:
1. Start fixing from init command using submodule--helper, with
subcommand that fixes all submodules on current level
2. Each submodule processed with another subcommand/function in
submodule--helper individually
3. Repeat for current submodule recursively
Glad to see your advices.
Regards,
Valery Tolstov
^ permalink raw reply
* Re: RFC: Another proposed hash function transition plan
From: Linus Torvalds @ 2017-03-07 19:15 UTC (permalink / raw)
To: Ian Jackson
Cc: Jonathan Nieder, Git Mailing List, Stefan Beller, bmwill,
Jonathan Tan, Jeff King
In-Reply-To: <22719.680.730866.781688@chiark.greenend.org.uk>
On Tue, Mar 7, 2017 at 10:57 AM, Ian Jackson
<ijackson@chiark.greenend.org.uk> wrote:
>
> Also I think you need to specify how abbreviated object names are
> interpreted.
One option might be to not use hex for the new hash, but base64 encoding.
That would make the full size ASCII hash encoding length roughly
similar (43 base64 characters rather than 40), which would offset some
of the new costs (longer filenames in the loose format, for example).
Also, since 256 isn't evenly divisible by 6, and because you'd want
some way to explictly disambiguate the new hashes, the rule *could* be
that the ASCII representation of a new hash is the base64 encoding of
the 258-bit value that has "10" prepended to it as padding.
That way the first character of the hash would be guaranteed to not be
a hex digit, because it would be in the range [g-v] (indexes 32..47).
Of course, the downside is that base64 encoded hashes can also end up
looking very much like real words, and now case would matter too.
The "use base64 with a "10" two-bit padding prepended" also means that
the natural loose format radix format would remain the first 2
characters of the hash, but due to the first character containing the
padding, it would be a fan-out of 2**10 rather than 2**12.
Of course, having written that, I now realize how it would cause
problems for the usual shit-for-brains case-insensitive filesystems.
So I guess base64 encoding doesn't work well for that reason.
Linus
^ permalink raw reply
* Re: git init --separate-git-dir does not update symbolic .git links for submodules
From: Stefan Beller @ 2017-03-07 19:59 UTC (permalink / raw)
To: CAGZ79kZbc394rmxYDUxCbysKNbEQCB7aLJkf6MGcCeXKAxiKhA
Cc: git@vger.kernel.org, Junio C Hamano, Valery Tolstov, sven
In-Reply-To: <1488913150.8812.0@smtp.yandex.ru>
On Tue, Mar 7, 2017 at 10:59 AM, Valery Tolstov <me@vtolstov.org> wrote:
> I think we can reuse code from module_clone that writes .git link.
> Possibly this code fragment needs to be factored out from module_clone
That fragment already exists, see dir.h:
connect_work_tree_and_git_dir(work_tree, git_dir);
Maybe another good microproject is to use that in module_clone.
>
> Also, to fix all the links, we need to obtain the list of submodules
> and then iterate over them.
Right, but a submodule may have a nested submodule.
So we need to fix each submodule from that list recursively,
i.e. not just the submodule itself, but any potential nested
submodule in that submodule, too.
(the listing doesn't list these nested submodules)
So we would call
fix_gitlink(sub)
{
fix_locally(sub);
// have a child process that calls
// this function on any submodule inside sub.
}
> module_list command iterates
> and prints the result to stdout. Maybe we can reuse this output.
> Or create separate function that returns the list of submodules.
yeah you can make use of module_list_compute to just produce
the list internally.
>
> Can we call these functions from submodule--helper inside init command,
> or run them thorugh internal command interface?
Both sounds fine, though calling internally is preferable for
performance reasons.
>
> In my opinion, make submodule--helper fully responsible for link fixes
> would be a good solution. Then we create two additional function, one
> that fixes all submodules on the current level, and another that
> fixes individual submodule.
> Although it looks good, I'm not quite sure that it's really good.
That sounds good to me.
So "git init --separate-git-dir" calls internally a new function
in the submodule--helper to
>
> So, maybe we can do link fixes like this:
>
> 1. Start fixing from init command using submodule--helper, with
> subcommand that fixes all submodules on current level
for this step we do not change the repository we are in, so there
is no need to call a new process, but we rather want to
call it internally.
> 2. Each submodule processed with another subcommand/function in
> submodule--helper individually
sounds good.
> 3. Repeat for current submodule recursively
and this recursive action needs to have its own process in the submodule,
e.g. "git submodule--helper --recursive fix-git-links"
>
> Glad to see your advices.
>
> Regards,
> Valery Tolstov
Regards,
Stefan
^ permalink raw reply
* Re: git init --separate-git-dir does not update symbolic .git links for submodules
From: Valery Tolstov @ 2017-03-07 19:52 UTC (permalink / raw)
To: sbeller; +Cc: git, gitster, me, sven
Just noticed that there already is function that gives module list
module_list_compute. But purpose of it's arguments is not quite clear
for me at this moment.
^ permalink raw reply
* Re: RFC: Another proposed hash function transition plan
From: Ian Jackson @ 2017-03-07 18:57 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, sbeller, bmwill, jonathantanmy, peff, Linus Torvalds
In-Reply-To: <20170304011251.GA26789@aiede.mtv.corp.google.com>
Jonathan Nieder writes ("RFC: Another proposed hash function transition plan"):
> This past week we came up with this idea for what a transition to a new
> hash function for Git would look like. I'd be interested in your
> thoughts (especially if you can make them as comments on the document,
> which makes it easier to address them and update the document).
Thanks for this.
This is a reasonable plan. It corresponds to approaches (2) and (B)
of my survey mail from the other day. Ie, two parallel homogeneous
hash trees, rather than a unified but heterogeneous hash tree, with
old vs new object names distinguished by length.
I still prefer my proposal with the mixed hash tree, mostly because
the handling of signatures here is very awkward, and because my
proposal does not involve altering object ids stored other than in the
git object graph (eg CI system databases, etc.)
One thing you've missed, I think, is notes: notes have to be dealt
with in a more complicated way. Do you intend to rewrite the tree
objects for notes commits so that the notes are annotations for the
new names for the annotated objects ? And if so, when ?
Also I think you need to specify how abbreviated object names are
interpreted.
Regards,
Ian.
^ permalink raw reply
* Re: git init --separate-git-dir does not update symbolic .git links for submodules
From: Stefan Beller @ 2017-03-07 20:11 UTC (permalink / raw)
To: CAGZ79kZbc394rmxYDUxCbysKNbEQCB7aLJkf6MGcCeXKAxiKhA
Cc: git@vger.kernel.org, Junio C Hamano, Valery Tolstov, sven
In-Reply-To: <1488916365.8812.1@smtp.yandex.ru>
On Tue, Mar 7, 2017 at 11:52 AM, Valery Tolstov <me@vtolstov.org> wrote:
> Just noticed that there already is function that gives module list
> module_list_compute. But purpose of it's arguments is not quite clear
> for me at this moment.
>
static int module_list_compute(int argc, const char **argv,
const char *prefix,
struct pathspec *pathspec,
struct module_list *list)
argc, argv and prefix are just passed through from each caller
argc is the number of arguments on the command line,
argv is an array of said arguments on the command line,
prefix is the position inside the repository.
e.g. If in git.git in Documentation/, you run a git command
"git submodule--helper module_list *"
then first "git" and "submodule--helper" are removed,
and then cmd_submodule__helper is called (at the end of
the submodule helper, which then sees
argc=2, argv=["module_list", "*"],
prefix="Documentation/"
which then proceeds to into module_list().
That parses these arguments, but there is no
argument it knows about, so it does nothing.
Then it just passes these three (argc, argv, prefix)
to module_list_compute, which then makes up the list,
to be stored in the last parameter 'list'.
The 'pathspec' parameter seems weird.
Internally the arguments mentioned above are converted to a pathspec,
such that we can go through all files and call match_pathspec inside
of module_list_compute.
In all but one cases we do not care about the pathspec, but in one
case (in update_clone) we want to issue a warning if the pathspec
was empty (i.e. the user just said "git submodule--helper update_clone"
with no further path arguments)
^ permalink raw reply
* Re: [RESEND PATCH] git-gui--askpass: generalize the window title
From: Sebastian Schuberth @ 2017-03-07 18:40 UTC (permalink / raw)
To: Stefan Beller; +Cc: Pat Thoyts, git@vger.kernel.org
In-Reply-To: <CAGZ79ka_5QogUEwF6SPCwyqSrCNSrtAsqzqJQdXsJkZEAyzDNA@mail.gmail.com>
On Tue, Mar 7, 2017 at 7:30 PM, Stefan Beller <sbeller@google.com> wrote:
> Although the following are included in git.git repository, they have their
> own authoritative repository and maintainers:
Thanks. I continuously get confused by this fact.
--
Sebastian Schuberth
^ 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