* Re: [PATCH v3 0/9] Fix the early config
From: Jeff King @ 2017-03-08 17:42 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, git, Duy Nguyen
In-Reply-To: <xmqqy3wf3dbo.fsf@gitster.mtv.corp.google.com>
On Wed, Mar 08, 2017 at 09:09:31AM -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > Good catch. Another "non-gentle" thing I noticed here while looking at
> > another thread: the repository-format version check uses the config
> > parser, which will die() in certain circumstances. So for instance:
> >
> > $ git init
> > $ git rev-parse && echo ok
> > ok
> >
> > $ echo '[core]repositoryformatversion = 10' >.git/config
> > $ git rev-parse && echo ok
> > fatal: Expected git repo version <= 1, found 10
>
> Just to set my expectation straight. Do you expect/wish this not to
> fail because of this in cmd_rev_parse()?
No, I was just using "rev-parse" as a sample command that tried to do
repo setup. I meant the above snippet that you quoted to both be fine
and expected outputs. The problem is the _other_ two cases where the
config code dies before we even get to the version-number check.
> Or are you discussing a more general issue, iow, anything that can
> work without repository (i.e. those who do _gently version of the
> setup and act on *nongit_ok) should pretend as if there were no
> (broken) repository and take the "no we are not in a repository"
> codepath?
Yes, exactly. It would have been less confusing if I picked something
that passed nongit_ok. Like hash-object:
$ git init
$ echo content >file
$ git hash-object file
d95f3ad14dee633a758d2e331151e950dd13e4ed
$ echo '[core]repositoryformatversion = 10' >.git/config
$ git hash-object file
warning: Expected git repo version <= 1, found 10
d95f3ad14dee633a758d2e331151e950dd13e4ed
The warning is fine and reasonable here. But then:
$ echo '[core]repositoryformatversion = foobar' >.git/config
$ git hash-object file
fatal: bad numeric config value 'foobar' for 'core.repositoryformatversion' in file .git/config: invalid unit
That's wrong. We're supposed to be gentle. And ditto:
$ echo '[co' >.git/config
$ git hash-object file
fatal: bad config line 1 in file .git/config
Those last two should issue a warning at most, and then let the command
continue.
-Peff
^ permalink raw reply
* [PATCH] submodule--helper.c: remove duplicate code
From: me @ 2017-03-08 17:44 UTC (permalink / raw)
To: git; +Cc: Valery Tolstov
From: Valery Tolstov <me@vtolstov.org>
Remove code fragment from module_clone that duplicates functionality
of connect_work_tree_and_git_dir in dir.c
Signed-off-by: Valery Tolstov <me@vtolstov.org>
---
>> 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.
By suggestion of Stefan Beller I would like to make this micro
improvement as my microproject for GSoc.
builtin/submodule--helper.c | 22 +++-------------------
1 file changed, 3 insertions(+), 19 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 899dc334e..cda8a3bc1 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -579,7 +579,6 @@ static int module_clone(int argc, const char **argv, const char *prefix)
const char *name = NULL, *url = NULL, *depth = NULL;
int quiet = 0;
int progress = 0;
- FILE *submodule_dot_git;
char *p, *path = NULL, *sm_gitdir;
struct strbuf rel_path = STRBUF_INIT;
struct strbuf sb = STRBUF_INIT;
@@ -653,27 +652,12 @@ static int module_clone(int argc, const char **argv, const char *prefix)
strbuf_reset(&sb);
}
- /* Write a .git file in the submodule to redirect to the superproject. */
- strbuf_addf(&sb, "%s/.git", path);
- if (safe_create_leading_directories_const(sb.buf) < 0)
- die(_("could not create leading directories of '%s'"), sb.buf);
- submodule_dot_git = fopen(sb.buf, "w");
- if (!submodule_dot_git)
- die_errno(_("cannot open file '%s'"), sb.buf);
-
- fprintf_or_die(submodule_dot_git, "gitdir: %s\n",
- relative_path(sm_gitdir, path, &rel_path));
- if (fclose(submodule_dot_git))
- die(_("could not close file %s"), sb.buf);
- strbuf_reset(&sb);
- strbuf_reset(&rel_path);
-
- /* Redirect the worktree of the submodule in the superproject's config */
p = git_pathdup_submodule(path, "config");
if (!p)
die(_("could not get submodule directory for '%s'"), path);
- git_config_set_in_file(p, "core.worktree",
- relative_path(path, sm_gitdir, &rel_path));
+
+ /* Connect module worktree and git dir */
+ connect_work_tree_and_git_dir(path, sm_gitdir);
/* setup alternateLocation and alternateErrorStrategy in the cloned submodule if needed */
git_config_get_string("submodule.alternateLocation", &sm_alternate);
--
2.12.0.190.g250ed7eaf
^ permalink raw reply related
* Re: fatal: Could not get current working directory: Permission denied | affected 2.10,2.11,2.12, but not 1.9.5 |
From: René Scharfe @ 2017-03-08 17:36 UTC (permalink / raw)
To: Zenobiusz Kunegunda; +Cc: git
In-Reply-To: <elvahoiwfqayelbskykd@qjih>
Am 06.03.2017 um 17:10 schrieb Zenobiusz Kunegunda:
> OS: FreeBSD 10.3-STABLE
>
> Story: I was trying to install openproject using this manual
> https://www.openproject.org/open-source/download/manual-installation-guide/
>
> Everything was fine till command $ bundle install --deployment
> --without postgres sqlite development test therubyracer docker
>
> works witg git version: 1.9.5 ( branch from repo ) does not work with
> git version: 2.10 ( branch from from repo ) 2.11 ( both from FreeBSD
> and from git repository) 2.12 ( branch from repo )
>
> On another server that passed but there was npm problem.
>
> This is error for $ bundle install --deployment --without postgres
> sqlite development test therubyracer docker
I suspect you might get better responses from the makers of bundler
(http://bundler.io/, http://groups.google.com/group/ruby-bundler).
> Fetching dependency metadata from https://rubygems.org/. fatal: Could
> not get current working directory: Permission denied
>
> Retrying `git fetch --force --quiet --tags
> "/usr/home/USER/openproject/vendor/bundle/ruby/2.2.0/cache/bundler/git/awesome_nested_set-209215f38dc7f6765d32201897f8688e973f4de7"`
> due to error (2/4): Bundler::Source::Git::GitCommandError Git error:
> command `git fetch --force --quiet --tags
> "/usr/home/USER/openproject/vendor/bundle/ruby/2.2.0/cache/bundler/git/awesome_nested_set-209215f38dc7f6765d32201897f8688e973f4de7"`
> in directory
> /usr/home/USER/openproject/vendor/bundle/ruby/2.2.0/bundler/gems/awesome_nested_set-7bd473e845e2
> has failed. If this error persists you could try removing the cache
> directory
> '/usr/home/USER/openproject/vendor/bundle/ruby/2.2.0/cache/bundler/git/awesome_nested_set-209215f38dc7f6765d32201897f8688e973f4de7'fatal:
> Could not get current working directory: Permission denied
These long and repetitive messages make me feel dizzy. So git fetch was
executed in
"/usr/home/USER/openproject/vendor/bundle/ruby/2.2.0/bundler/gems/awesome_nested_set-7bd473e845e2"
and reported that it was not allowed to get the current working
directory, right? That's odd.
I suspect that older versions of git ignored the error, used an empty
string and went with that instead of an absolute path, but that's just a
guess.
Was git perhaps started by bundler under a different user ID? You could
check that e.g. by printing the return value of geteuid(2).
"/bin/pwd -P" executed in the same directory under the same user ID as
git should error out and report "Permission denied" as well. Are the
permissions bits and ACLs for that directory and its parents OK?
Thanks,
René
^ permalink raw reply
* [PATCH 10/10] ls-partial: created command to list missing blobs
From: Jeff Hostetler @ 2017-03-08 17:38 UTC (permalink / raw)
To: git
Cc: jeffhost, peff, gitster, markbt, benpeart, jonathantanmy,
Jeff Hostetler
In-Reply-To: <1488994685-37403-1-git-send-email-jeffhost@microsoft.com>
From: Jeff Hostetler <git@jeffhostetler.com>
Added a command to list the missing blobs for a commit.
This can be used after a partial clone or fetch to list
the omitted blobs that the client would need to checkout
the given commit/branch. Optionally respecting or ignoring
the current sparse-checkout definition.
This command prints a simple list of blob SHAs. It is
expected that this would be piped into another command
with knowledge of the transport and/or blob store.
Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
---
Makefile | 2 +
builtin.h | 1 +
builtin/ls-partial.c | 110 ++++++++++++++++++++
git.c | 1 +
partial-utils.c | 279 +++++++++++++++++++++++++++++++++++++++++++++++++++
partial-utils.h | 93 +++++++++++++++++
6 files changed, 486 insertions(+)
create mode 100644 builtin/ls-partial.c
create mode 100644 partial-utils.c
create mode 100644 partial-utils.h
diff --git a/Makefile b/Makefile
index 9ec6065..96e9e1e 100644
--- a/Makefile
+++ b/Makefile
@@ -791,6 +791,7 @@ LIB_OBJS += pack-write.o
LIB_OBJS += pager.o
LIB_OBJS += parse-options.o
LIB_OBJS += parse-options-cb.o
+LIB_OBJS += partial-utils.o
LIB_OBJS += patch-delta.o
LIB_OBJS += patch-ids.o
LIB_OBJS += path.o
@@ -908,6 +909,7 @@ BUILTIN_OBJS += builtin/init-db.o
BUILTIN_OBJS += builtin/interpret-trailers.o
BUILTIN_OBJS += builtin/log.o
BUILTIN_OBJS += builtin/ls-files.o
+BUILTIN_OBJS += builtin/ls-partial.o
BUILTIN_OBJS += builtin/ls-remote.o
BUILTIN_OBJS += builtin/ls-tree.o
BUILTIN_OBJS += builtin/mailinfo.o
diff --git a/builtin.h b/builtin.h
index 9e4a898..df00c4b 100644
--- a/builtin.h
+++ b/builtin.h
@@ -79,6 +79,7 @@ extern int cmd_interpret_trailers(int argc, const char **argv, const char *prefi
extern int cmd_log(int argc, const char **argv, const char *prefix);
extern int cmd_log_reflog(int argc, const char **argv, const char *prefix);
extern int cmd_ls_files(int argc, const char **argv, const char *prefix);
+extern int cmd_ls_partial(int argc, const char **argv, const char *prefix);
extern int cmd_ls_tree(int argc, const char **argv, const char *prefix);
extern int cmd_ls_remote(int argc, const char **argv, const char *prefix);
extern int cmd_mailinfo(int argc, const char **argv, const char *prefix);
diff --git a/builtin/ls-partial.c b/builtin/ls-partial.c
new file mode 100644
index 0000000..8ebf045
--- /dev/null
+++ b/builtin/ls-partial.c
@@ -0,0 +1,110 @@
+#include "cache.h"
+#include "blob.h"
+#include "tree.h"
+#include "commit.h"
+#include "quote.h"
+#include "builtin.h"
+#include "parse-options.h"
+#include "pathspec.h"
+#include "dir.h"
+#include "partial-utils.h"
+
+static struct trace_key trace_partial = TRACE_KEY_INIT(PARTIAL);
+
+static int verbose;
+static int ignore_sparse;
+struct exclude_list el;
+
+static const char * const ls_partial_usage[] = {
+ N_("git ls-partial [<options>] <tree-ish>"),
+ NULL
+};
+
+/*
+ * map <tree-ish> arg into SHA1 and get the root treenode.
+ */
+static struct tree *lookup_tree_from_treeish(const char *arg)
+{
+ unsigned char sha1[20];
+ struct tree *tree;
+
+ if (get_sha1(arg, sha1))
+ die("not a valid object name '%s'", arg);
+
+ trace_printf_key(
+ &trace_partial,
+ "ls-partial: treeish '%s' '%s'\n",
+ arg, sha1_to_hex(sha1));
+
+ if (verbose) {
+ printf("commit\t%s\n", sha1_to_hex(sha1));
+ printf("branch\t%s\n", arg);
+ }
+
+ tree = parse_tree_indirect(sha1);
+ if (!tree)
+ die("not a tree object '%s'", arg);
+
+ return tree;
+}
+
+static void print_results(const struct pu_vec *vec)
+{
+ int k;
+
+ for (k = 0; k < vec->data_nr; k++)
+ printf("%s\n", oid_to_hex(&vec->data[k]->oid));
+}
+
+static void print_results_verbose(const struct pu_vec *vec)
+{
+ int k;
+
+ /* TODO Consider -z version */
+
+ for (k = 0; k < vec->data_nr; k++)
+ printf("%s\t%s\n", oid_to_hex(&vec->data[k]->oid), vec->data[k]->fullpath.buf);
+}
+
+int cmd_ls_partial(int argc, const char **argv, const char *prefix)
+{
+ struct exclude_list el;
+ struct tree *tree;
+ struct pu_vec *vec;
+ struct pu_vec *vec_all = NULL;
+ struct pu_vec *vec_sparse = NULL;
+ struct pu_vec *vec_missing = NULL;
+
+ const struct option ls_partial_options[] = {
+ OPT__VERBOSE(&verbose, N_("show verbose blob details")),
+ OPT_BOOL(0, "ignore-sparse", &ignore_sparse,
+ N_("ignore sparse-checkout settings (scan whole tree)")),
+ OPT_END()
+ };
+
+ git_config(git_default_config, NULL);
+ argc = parse_options(argc, argv, prefix,
+ ls_partial_options, ls_partial_usage, 0);
+ if (argc < 1)
+ usage_with_options(ls_partial_usage, ls_partial_options);
+
+ tree = lookup_tree_from_treeish(argv[0]);
+
+ vec_all = pu_vec_ls_tree(tree, prefix, argv + 1);
+ if (ignore_sparse || pu_load_sparse_definitions("info/sparse-checkout", &el) < 0)
+ vec = vec_all;
+ else {
+ vec_sparse = pu_vec_filter_sparse(vec_all, &el);
+ vec = vec_sparse;
+ }
+
+ vec_missing = pu_vec_filter_missing(vec);
+ vec = vec_missing;
+
+ if (verbose)
+ print_results_verbose(vec);
+ else
+ print_results(vec);
+
+ return 0;
+}
diff --git a/git.c b/git.c
index 33f52ac..ef1e019 100644
--- a/git.c
+++ b/git.c
@@ -444,6 +444,7 @@ static struct cmd_struct commands[] = {
{ "interpret-trailers", cmd_interpret_trailers, RUN_SETUP_GENTLY },
{ "log", cmd_log, RUN_SETUP },
{ "ls-files", cmd_ls_files, RUN_SETUP | SUPPORT_SUPER_PREFIX },
+ { "ls-partial", cmd_ls_partial, RUN_SETUP },
{ "ls-remote", cmd_ls_remote, RUN_SETUP_GENTLY },
{ "ls-tree", cmd_ls_tree, RUN_SETUP },
{ "mailinfo", cmd_mailinfo, RUN_SETUP_GENTLY },
diff --git a/partial-utils.c b/partial-utils.c
new file mode 100644
index 0000000..b75e91e
--- /dev/null
+++ b/partial-utils.c
@@ -0,0 +1,279 @@
+#include "cache.h"
+#include "blob.h"
+#include "tree.h"
+#include "commit.h"
+#include "quote.h"
+#include "builtin.h"
+#include "parse-options.h"
+#include "pathspec.h"
+#include "dir.h"
+#include "partial-utils.h"
+
+static struct trace_key trace_partial_utils = TRACE_KEY_INIT(PARTIAL_UTILS);
+
+void pu_row_trace(
+ const struct pu_row *row,
+ const char *label)
+{
+ trace_printf_key(
+ &trace_partial_utils,
+ "%s: %06o %s %.*s\n",
+ label,
+ row->mode,
+ oid_to_hex(&row->oid),
+ (int)row->fullpath.len,
+ row->fullpath.buf);
+}
+
+struct pu_row *pu_row_alloc(
+ const unsigned char *sha1,
+ const struct strbuf *base,
+ const char *entryname,
+ unsigned mode)
+{
+ struct pu_row *row = xcalloc(1, sizeof(struct pu_row));
+
+ hashcpy(row->oid.hash, sha1);
+ strbuf_init(&row->fullpath, base->len + strlen(entryname) + 1);
+ if (base->len)
+ strbuf_addbuf(&row->fullpath, base);
+ strbuf_addstr(&row->fullpath, entryname);
+ row->mode = mode;
+ row->entryname_offset = base->len;
+
+ pu_row_trace(row, "alloc");
+
+ return row;
+}
+
+struct pu_vec *pu_vec_alloc(
+ unsigned int nr_pre_alloc)
+{
+ struct pu_vec *vec = xcalloc(1, sizeof(struct pu_vec));
+
+ vec->data = xcalloc(nr_pre_alloc, sizeof(struct pu_row *));
+ vec->data_alloc = nr_pre_alloc;
+
+ return vec;
+}
+
+void pu_vec_append(
+ struct pu_vec *vec,
+ struct pu_row *row)
+{
+ ALLOC_GROW(vec->data, vec->data_nr + 1, vec->data_alloc);
+ vec->data[vec->data_nr++] = row;
+}
+
+static int ls_tree_cb(
+ const unsigned char *sha1,
+ struct strbuf *base,
+ const char *pathname,
+ unsigned mode,
+ int stage,
+ void *context)
+{
+ struct pu_vec *vec = (struct pu_vec *)context;
+
+ /* omit submodules */
+ if (S_ISGITLINK(mode))
+ return 0;
+
+ pu_vec_append(vec, pu_row_alloc(sha1, base, pathname, mode));
+
+ if (S_ISDIR(mode))
+ return READ_TREE_RECURSIVE;
+
+ return 0;
+}
+
+struct pu_vec *pu_vec_ls_tree(
+ struct tree *tree,
+ const char *prefix,
+ const char **argv)
+{
+ struct pu_vec *vec;
+ struct pathspec pathspec;
+ int k;
+
+ vec = pu_vec_alloc(PU_VEC_DEFAULT_SIZE);
+
+ parse_pathspec(
+ &pathspec, PATHSPEC_GLOB | PATHSPEC_ICASE | PATHSPEC_EXCLUDE,
+ PATHSPEC_PREFER_CWD, prefix, argv);
+ for (k = 0; k < pathspec.nr; k++)
+ pathspec.items[k].nowildcard_len = pathspec.items[k].len;
+ pathspec.has_wildcard = 0;
+
+ if (read_tree_recursive(tree, "", 0, 0, &pathspec, ls_tree_cb, vec) != 0)
+ die("Could not read tree");
+
+ return vec;
+}
+
+int pu_load_sparse_definitions(
+ const char *path,
+ struct exclude_list *pel)
+{
+ int result;
+ char *sparse = git_pathdup("info/sparse-checkout");
+ memset(pel, 0, sizeof(*pel));
+ result = add_excludes_from_file_to_list(sparse, "", 0, pel, 0);
+ free(sparse);
+ return result;
+}
+
+static int mode_to_dtype(unsigned mode)
+{
+ if (S_ISREG(mode))
+ return DT_REG;
+ if (S_ISDIR(mode) || S_ISGITLINK(mode))
+ return DT_DIR;
+ if (S_ISLNK(mode))
+ return DT_LNK;
+ return DT_UNKNOWN;
+}
+
+static int apply_excludes_1(
+ struct pu_row **subset,
+ unsigned int nr,
+ struct strbuf *prefix,
+ struct exclude_list *pel,
+ int defval,
+ struct pu_vec *vec_out);
+
+/* apply directory rules. based on clear_ce_flags_dir() */
+static int apply_excludes_dir(
+ struct pu_row **subset,
+ unsigned int nr,
+ struct strbuf *prefix,
+ char *basename,
+ struct exclude_list *pel,
+ int defval,
+ struct pu_vec *vec_out)
+{
+ struct pu_row **subset_end;
+ int dtype = DT_DIR;
+ int ret = is_excluded_from_list(
+ prefix->buf, prefix->len, basename, &dtype, pel);
+ int rc;
+
+ strbuf_addch(prefix, '/');
+
+ if (ret < 0)
+ ret = defval;
+
+ for (subset_end = subset; subset_end != subset + nr; subset_end++) {
+ struct pu_row *row = *subset_end;
+ if (strncmp(row->fullpath.buf, prefix->buf, prefix->len))
+ break;
+ }
+
+ rc = apply_excludes_1(
+ subset, subset_end - subset,
+ prefix, pel, ret,
+ vec_out);
+ strbuf_setlen(prefix, prefix->len - 1);
+ return rc;
+}
+
+/* apply sparse rules to subset[0..nr). based on clear_ce_flags_1() */
+static int apply_excludes_1(
+ struct pu_row **subset,
+ unsigned int nr,
+ struct strbuf *prefix,
+ struct exclude_list *pel,
+ int defval,
+ struct pu_vec *vec_out)
+{
+ struct pu_row **subset_end = subset + nr;
+
+ while (subset != subset_end) {
+ struct pu_row *row = *subset;
+ const char *name, *slash;
+ int len, dtype, val;
+
+ if (prefix->len && strncmp(row->fullpath.buf, prefix->buf, prefix->len))
+ break;
+
+ name = row->fullpath.buf + prefix->len;
+ slash = strchr(name, '/');
+
+ if (slash) {
+ int processed;
+
+ len = slash - name;
+ strbuf_add(prefix, name, len);
+
+ processed = apply_excludes_dir(
+ subset, subset_end - subset,
+ prefix, prefix->buf + prefix->len - len,
+ pel, defval,
+ vec_out);
+
+ if (processed) {
+ subset += processed;
+ strbuf_setlen(prefix, prefix->len - len);
+ continue;
+ }
+
+ strbuf_addch(prefix, '/');
+ subset += apply_excludes_1(
+ subset, subset_end - subset,
+ prefix, pel, defval,
+ vec_out);
+ strbuf_setlen(prefix, prefix->len - len - 1);
+ continue;
+ }
+
+ dtype = mode_to_dtype(row->mode);
+ val = is_excluded_from_list(
+ row->fullpath.buf, row->fullpath.len, name, &dtype, pel);
+ if (val < 0)
+ val = defval;
+ if (val > 0) {
+ pu_row_trace(row, "sparse");
+ pu_vec_append(vec_out, row);
+ }
+ subset++;
+ }
+
+ return nr - (subset_end - subset);
+}
+
+struct pu_vec *pu_vec_filter_sparse(
+ const struct pu_vec *vec_in,
+ struct exclude_list *pel)
+{
+ struct pu_vec *vec_out;
+ struct strbuf prefix = STRBUF_INIT;
+ int defval = 0;
+
+ vec_out = pu_vec_alloc(vec_in->data_nr);
+
+ apply_excludes_1(
+ vec_in->data, vec_in->data_nr,
+ &prefix, pel, defval,
+ vec_out);
+
+ return vec_out;
+}
+
+struct pu_vec *pu_vec_filter_missing(
+ const struct pu_vec *vec_in)
+{
+ struct pu_vec *vec_out;
+ int k;
+
+ vec_out = pu_vec_alloc(vec_in->data_nr);
+
+ for (k = 0; k < vec_in->data_nr; k++) {
+ struct pu_row *row = vec_in->data[k];
+ if (!has_sha1_file(row->oid.hash)) {
+ pu_row_trace(row, "missing");
+ pu_vec_append(vec_out, row);
+ }
+ }
+
+ return vec_out;
+}
diff --git a/partial-utils.h b/partial-utils.h
new file mode 100644
index 0000000..3bdf2e4
--- /dev/null
+++ b/partial-utils.h
@@ -0,0 +1,93 @@
+#ifndef PARTIAL_UTILS_H
+#define PARTIAL_UTILS_H
+
+/*
+ * A 'partial-utils row' represents a single item in the tree.
+ * This is conceptually equivalent to a cache_entry, but does
+ * not require an index_state and lets us operate on any commit
+ * and not be tied to the current worktree.
+ */
+struct pu_row
+{
+ struct strbuf fullpath;
+ struct object_id oid;
+ unsigned mode;
+ unsigned entryname_offset;
+};
+
+/*
+ * A 'partial-utils vec' represents a vector of 'pu row'
+ * values using the normal vector machinery.
+ */
+struct pu_vec
+{
+ struct pu_row **data;
+ unsigned int data_nr;
+ unsigned int data_alloc;
+};
+
+#define PU_VEC_DEFAULT_SIZE (1024*1024)
+
+
+void pu_row_trace(
+ const struct pu_row *row,
+ const char *label);
+
+struct pu_row *pu_row_alloc(
+ const unsigned char *sha1,
+ const struct strbuf *base,
+ const char *entryname,
+ unsigned mode);
+
+struct pu_vec *pu_vec_alloc(
+ unsigned int nr_pre_alloc);
+
+/*
+ * Append the given row onto the vector WITHOUT
+ * assuming ownership of the pointer.
+ */
+void pu_vec_append(
+ struct pu_vec *vec,
+ struct pu_row *row);
+
+/*
+ * Enumerate the contents of the tree (recursively) into
+ * a vector of rows. This is essentially "ls-tree -r -t"
+ * into a vector.
+ */
+struct pu_vec *pu_vec_ls_tree(
+ struct tree *tree,
+ const char *prefix,
+ const char **argv);
+
+/*
+ * Load a sparse-checkout file into (*pel).
+ * Returns -1 if none or error.
+ */
+int pu_load_sparse_definitions(
+ const char *path,
+ struct exclude_list *pel);
+
+/*
+ * Filter the given vector using the sparse-checkout
+ * definitions and return new vector of just the paths
+ * that WOULD BE populated.
+ *
+ * The returned vector BORROWS rows from the input vector.
+ *
+ * This is loosely based upon clear_ce_flags() in unpack-trees.c
+ */
+struct pu_vec *pu_vec_filter_sparse(
+ const struct pu_vec *vec_in,
+ struct exclude_list *pel);
+
+/*
+ * Filter the given vector and return the list of blobs
+ * missing from the local ODB.
+ *
+ * The returned vector BORROWS rows from the input vector.
+ */
+struct pu_vec *pu_vec_filter_missing(
+ const struct pu_vec *vec_in);
+
+#endif /* PARTIAL_UTILS_H */
--
2.7.4
^ permalink raw reply related
* [PATCH 09/10] clone: add partial-by-size and partial-special arguments
From: Jeff Hostetler @ 2017-03-08 17:38 UTC (permalink / raw)
To: git
Cc: jeffhost, peff, gitster, markbt, benpeart, jonathantanmy,
Jeff Hostetler
In-Reply-To: <1488994685-37403-1-git-send-email-jeffhost@microsoft.com>
From: Jeff Hostetler <git@jeffhostetler.com>
Teach clone to accept --partial-by-size=n and --partial-special
arguments to request that the server omit certain blobs from
the generated packfile.
Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
---
builtin/clone.c | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/builtin/clone.c b/builtin/clone.c
index 3f63edb..e5a5904 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -56,6 +56,8 @@ static struct string_list option_required_reference = STRING_LIST_INIT_NODUP;
static struct string_list option_optional_reference = STRING_LIST_INIT_NODUP;
static int option_dissociate;
static int max_jobs = -1;
+static const char *partial_by_size;
+static int partial_special;
static struct option builtin_clone_options[] = {
OPT__VERBOSITY(&option_verbosity),
@@ -112,6 +114,11 @@ static struct option builtin_clone_options[] = {
TRANSPORT_FAMILY_IPV4),
OPT_SET_INT('6', "ipv6", &family, N_("use IPv6 addresses only"),
TRANSPORT_FAMILY_IPV6),
+ OPT_STRING(0, "partial-by-size", &partial_by_size,
+ N_("size"),
+ N_("only include blobs smaller than this")),
+ OPT_BOOL(0, "partial-special", &partial_special,
+ N_("only include blobs for git special files")),
OPT_END()
};
@@ -625,6 +632,9 @@ static void update_remote_refs(const struct ref *refs,
if (check_connectivity) {
struct check_connected_options opt = CHECK_CONNECTED_INIT;
+ if (partial_by_size || partial_special)
+ opt.allow_partial = 1;
+
opt.transport = transport;
opt.progress = transport->progress;
@@ -1021,6 +1031,10 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
warning(_("--shallow-since is ignored in local clones; use file:// instead."));
if (option_not.nr)
warning(_("--shallow-exclude is ignored in local clones; use file:// instead."));
+ if (partial_by_size)
+ warning(_("--partial-by-size is ignored in local clones; use file:// instead."));
+ if (partial_special)
+ warning(_("--partial-special is ignored in local clones; use file:// instead."));
if (!access(mkpath("%s/shallow", path), F_OK)) {
if (option_local > 0)
warning(_("source repository is shallow, ignoring --local"));
@@ -1052,6 +1066,18 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
transport_set_option(transport, TRANS_OPT_UPLOADPACK,
option_upload_pack);
+ if (partial_by_size) {
+ transport_set_option(transport, TRANS_OPT_PARTIAL_BY_SIZE,
+ partial_by_size);
+ if (transport->smart_options)
+ transport->smart_options->partial_by_size = partial_by_size;
+ }
+ if (partial_special) {
+ transport_set_option(transport, TRANS_OPT_PARTIAL_SPECIAL, "yes");
+ if (transport->smart_options)
+ transport->smart_options->partial_special = 1;
+ }
+
if (transport->smart_options && !deepen)
transport->smart_options->check_self_contained_and_connected = 1;
--
2.7.4
^ permalink raw reply related
* [PATCH 01/10] pack-objects: eat CR in addition to LF after fgets.
From: Jeff Hostetler @ 2017-03-08 17:37 UTC (permalink / raw)
To: git
Cc: jeffhost, peff, gitster, markbt, benpeart, jonathantanmy,
Jeff Hostetler
In-Reply-To: <1488994685-37403-1-git-send-email-jeffhost@microsoft.com>
From: Jeff Hostetler <git@jeffhostetler.com>
Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
---
builtin/pack-objects.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index f294dcf..7e052bb 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -2764,6 +2764,8 @@ static void get_object_list(int ac, const char **av)
int len = strlen(line);
if (len && line[len - 1] == '\n')
line[--len] = 0;
+ if (len && line[len - 1] == '\r')
+ line[--len] = 0;
if (!len)
break;
if (*line == '-') {
--
2.7.4
^ permalink raw reply related
* [PATCH 08/10] fetch: add partial-by-size and partial-special arguments
From: Jeff Hostetler @ 2017-03-08 17:38 UTC (permalink / raw)
To: git
Cc: jeffhost, peff, gitster, markbt, benpeart, jonathantanmy,
Jeff Hostetler
In-Reply-To: <1488994685-37403-1-git-send-email-jeffhost@microsoft.com>
From: Jeff Hostetler <git@jeffhostetler.com>
Teach fetch to accept --partial-by-size=n and --partial-special
arguments and pass them to fetch-patch to request that the
server certain blobs from the generated packfile.
Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
---
builtin/fetch.c | 26 +++++++++++++++++++++++++-
connected.c | 3 +++
connected.h | 3 +++
3 files changed, 31 insertions(+), 1 deletion(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index b5ad09d..3d47107 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -52,6 +52,8 @@ static const char *recurse_submodules_default;
static int shown_url = 0;
static int refmap_alloc, refmap_nr;
static const char **refmap_array;
+static const char *partial_by_size;
+static int partial_special;
static int option_parse_recurse_submodules(const struct option *opt,
const char *arg, int unset)
@@ -141,6 +143,11 @@ static struct option builtin_fetch_options[] = {
TRANSPORT_FAMILY_IPV4),
OPT_SET_INT('6', "ipv6", &family, N_("use IPv6 addresses only"),
TRANSPORT_FAMILY_IPV6),
+ OPT_STRING(0, "partial-by-size", &partial_by_size,
+ N_("size"),
+ N_("only include blobs smaller than this")),
+ OPT_BOOL(0, "partial-special", &partial_special,
+ N_("only include blobs for git special files")),
OPT_END()
};
@@ -731,6 +738,10 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
const char *filename = dry_run ? "/dev/null" : git_path_fetch_head();
int want_status;
int summary_width = transport_summary_width(ref_map);
+ struct check_connected_options opt = CHECK_CONNECTED_INIT;
+
+ if (partial_by_size || partial_special)
+ opt.allow_partial = 1;
fp = fopen(filename, "a");
if (!fp)
@@ -742,7 +753,7 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
url = xstrdup("foreign");
rm = ref_map;
- if (check_connected(iterate_ref_map, &rm, NULL)) {
+ if (check_connected(iterate_ref_map, &rm, &opt)) {
rc = error(_("%s did not send all necessary objects\n"), url);
goto abort;
}
@@ -882,6 +893,9 @@ static int quickfetch(struct ref *ref_map)
struct ref *rm = ref_map;
struct check_connected_options opt = CHECK_CONNECTED_INIT;
+ if (partial_by_size || partial_special)
+ opt.allow_partial = 1;
+
/*
* If we are deepening a shallow clone we already have these
* objects reachable. Running rev-list here will return with
@@ -1020,6 +1034,10 @@ static struct transport *prepare_transport(struct remote *remote, int deepen)
set_option(transport, TRANS_OPT_DEEPEN_RELATIVE, "yes");
if (update_shallow)
set_option(transport, TRANS_OPT_UPDATE_SHALLOW, "yes");
+ if (partial_by_size)
+ set_option(transport, TRANS_OPT_PARTIAL_BY_SIZE, partial_by_size);
+ if (partial_special)
+ set_option(transport, TRANS_OPT_PARTIAL_SPECIAL, "yes");
return transport;
}
@@ -1314,6 +1332,12 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
argc = parse_options(argc, argv, prefix,
builtin_fetch_options, builtin_fetch_usage, 0);
+ if (partial_by_size) {
+ unsigned long s;
+ if (!git_parse_ulong(partial_by_size, &s))
+ die(_("invalid partial-by-size value"));
+ }
+
if (deepen_relative) {
if (deepen_relative < 0)
die(_("Negative depth in --deepen is not supported"));
diff --git a/connected.c b/connected.c
index 136c2ac..b07cbb5 100644
--- a/connected.c
+++ b/connected.c
@@ -62,6 +62,9 @@ int check_connected(sha1_iterate_fn fn, void *cb_data,
argv_array_pushf(&rev_list.args, "--progress=%s",
_("Checking connectivity"));
+ if (opt->allow_partial)
+ argv_array_push(&rev_list.args, "--allow-partial");
+
rev_list.git_cmd = 1;
rev_list.env = opt->env;
rev_list.in = -1;
diff --git a/connected.h b/connected.h
index 4ca325f..756259e 100644
--- a/connected.h
+++ b/connected.h
@@ -34,6 +34,9 @@ struct check_connected_options {
/* If non-zero, show progress as we traverse the objects. */
int progress;
+ /* A previous partial clone/fetch may have omitted some blobs. */
+ int allow_partial;
+
/*
* Insert these variables into the environment of the child process.
*/
--
2.7.4
^ permalink raw reply related
* [PATCH 03/10] pack-objects: test for --partial-by-size --partial-special
From: Jeff Hostetler @ 2017-03-08 17:37 UTC (permalink / raw)
To: git
Cc: jeffhost, peff, gitster, markbt, benpeart, jonathantanmy,
Jeff Hostetler
In-Reply-To: <1488994685-37403-1-git-send-email-jeffhost@microsoft.com>
From: Jeff Hostetler <git@jeffhostetler.com>
Some simple tests for pack-objects with the new --partial-by-size
and --partial-special options.
Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
---
t/5316-pack-objects-partial.sh | 72 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 72 insertions(+)
create mode 100644 t/5316-pack-objects-partial.sh
diff --git a/t/5316-pack-objects-partial.sh b/t/5316-pack-objects-partial.sh
new file mode 100644
index 0000000..352de34
--- /dev/null
+++ b/t/5316-pack-objects-partial.sh
@@ -0,0 +1,72 @@
+#!/bin/sh
+
+test_description='pack-object partial'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+ perl -e "print \"a\" x 11;" > a &&
+ perl -e "print \"a\" x 1100;" > b &&
+ perl -e "print \"a\" x 1100000;" > c &&
+ echo "ignored" > .gitignore &&
+ git add a b c .gitignore &&
+ git commit -m test
+ '
+
+test_expect_success 'all blobs' '
+ git pack-objects --revs --thin --stdout >all.pack <<-EOF &&
+ master
+
+ EOF
+ git index-pack all.pack &&
+ test 4 = $(git verify-pack -v all.pack | grep blob | wc -l)
+ '
+
+test_expect_success 'no blobs' '
+ git pack-objects --revs --thin --stdout --partial-by-size=0 >none.pack <<-EOF &&
+ master
+
+ EOF
+ git index-pack none.pack &&
+ test 0 = $(git verify-pack -v none.pack | grep blob | wc -l)
+ '
+
+test_expect_success 'small blobs' '
+ git pack-objects --revs --thin --stdout --partial-by-size=1M >small.pack <<-EOF &&
+ master
+
+ EOF
+ git index-pack small.pack &&
+ test 3 = $(git verify-pack -v small.pack | grep blob | wc -l)
+ '
+
+test_expect_success 'tiny blobs' '
+ git pack-objects --revs --thin --stdout --partial-by-size=100 >tiny.pack <<-EOF &&
+ master
+
+ EOF
+ git index-pack tiny.pack &&
+ test 2 = $(git verify-pack -v tiny.pack | grep blob | wc -l)
+ '
+
+test_expect_success 'special' '
+ git pack-objects --revs --thin --stdout --partial-special >spec.pack <<-EOF &&
+ master
+
+ EOF
+ git index-pack spec.pack &&
+ test 1 = $(git verify-pack -v spec.pack | grep blob | wc -l)
+ '
+
+test_expect_success 'union' '
+ git pack-objects --revs --thin --stdout --partial-by-size=0 --partial-special >union.pack <<-EOF &&
+ master
+
+ EOF
+ git index-pack union.pack &&
+ test 1 = $(git verify-pack -v union.pack | grep blob | wc -l)
+ '
+
+test_done
+
+
--
2.7.4
^ permalink raw reply related
* [PATCH 00/10] RFC Partial Clone and Fetch
From: Jeff Hostetler @ 2017-03-08 17:37 UTC (permalink / raw)
To: git
Cc: jeffhost, peff, gitster, markbt, benpeart, jonathantanmy,
Jeff Hostetler
From: Jeff Hostetler <git@jeffhostetler.com>
[RFC] Partial Clone and Fetch
=============================
This is a WIP RFC for a partial clone and fetch feature wherein the client
can request that the server omit various blobs from the packfile during
clone and fetch. Clients can later request omitted blobs (either from a
modified upload-pack-like request to the server or via a completely
independent mechanism).
The purpose here is to reduce the size of packfile downloads and help
git scale to extremely large repos.
I use the term "partial" here to refer to a portion of one or more commits
and to avoid use of loaded terms like "sparse", "lazy", "narrow", and "skeleton".
The concept of a partial clone/fetch is independent of and can complement
the existing shallow-clone, refspec, and limited-ref filtering mechanisms
since these all filter at the DAG level whereas the work described here
works *within* the set of commits already chosen for download.
A. Requesting a Partial Clone/Fetch
===================================
Clone, fetch, and fetch-pack will accept one or more new "partial"
command line arguments as described below. The fetch-pack/upload-pack
protocol will be extended to include these new arguments. Upload-pack
and pack-objects will be updated accordingly. Pack-objects will filter
out the unwanted blobs as it is building the packfile. Rev-list and
index-pack will be updated to not complain when missing blobs are
detected in the received packfile.
[1] "--partial-by-size=<n>[kmg]"
Where <n> is a non-negative integer with an optional unit.
Request that only blobs smaller than this be included in the packfile.
The client might use this to implement an alternate LFS or ODB mechanism
for large blobs, such as suggested in:
https://public-inbox.org/git/20161130210420.15982-1-chriscool@tuxfamily.org/
A special case of size zero would omit all blobs and is similar to the
commits-and-trees-only feature described in:
https://public-inbox.org/git/20170113155253.1644-1-benpeart@microsoft.com/
[2] "--partial-special"
Request that special files, such as ".gitignore" and .gitattributes",
be included.
[3] *TODO* "--partial-by-profile=<sparse-checkout-path>"
Where <sparse-checkout-path> is verson-controlled file in the repository
(either present in the requested commit or the default HEAD on the server).
[I envision a ".gitsparse/<path>" hierarchy where teams can store
common sparse-checkout profiles. And then they can reference
them from their private ".git/info/sparse-checkout" files.]
Pack-objects will use this file and the sparse-checkout rules to only
include blobs in the packfile that would be needed to do the corresponding
sparse-checkout (and let the client avoid having to demand-load their
entire enlistment).
When multiple "partial" options are given, they are treated as a simple OR
giving the union of the blobs selected.
The patch series describes the changes to the fetch-pack/upload-pack
protocol:
Documentation/technical/pack-protocol.txt
Documentation/technical/protocol-capabilities.txt
B. Issues Backfilling Omitted Blobs
===================================
Ideally, if the client only does "--partial-by-profile" fetches, it
should not need to fetch individual missing blobs, but we have to allow
for it to handle the other commands and other unexpected issues.
There are 3 orthogonal concepts here: when, how and where?
[1] When:
(1a) a pre-command or hook to identify needed blobs and pre-fetch them
before allowing the actual command to start;
(1b) a dry-run mode for the command to likewise pre-fetch them; or
(1c) "fault" them in as necessary in read_object() while the command is
running and without any pre-fetch (either synchronously or asynchronously
and with/without a helper process).
Ideas for (1c) are being addressed in the following threads:
https://public-inbox.org/git/20170113155253.1644-1-benpeart@microsoft.com/
https://public-inbox.org/git/20170117184258.sd7h2hkv27w52gzt@sigill.intra.peff.net/
https://public-inbox.org/git/20161130210420.15982-1-chriscool@tuxfamily.org/
so I won't consider them here.
Ideas (1a) and (1b) have the advantage that they try to obtain all
required blobs before allowing an operation to start, so there is
less opportunity to leave the user in a weird state.
The best solution may be a combination of (1a) and (1b) and may depend
on the individual command. However, (1b) will further complicate the
source in the existing commands, so in some cases it may be simpler to
just take the ideas and implement stand-alone pre-commands.
For now, I'm going to limit this RFC to (1a).
[2] How:
(2a) augment the existing git protocols to include blob requests;
(2b) local external process (such as a database client or a local bulk
fetch daemon);
Ideas for (2b) are being addressed in the above threads, so I won't
consider them here.
So I'm going to limit this RFC to (2a).
[3] Where:
(3a) the same remote server used for the partial clone/fetch;
(3b) anywhere else, such as a proxy server or Azure or S3 blob store.
There's no reason that the client should be limited to going back to
the same server, but I'm not going to consider it here, so I'm going
to limit this RFC to (3a).
C. New Blob-Fetch Protocol (2a)
===============================
*TODO* A new pair of commands, such as fetch-blob-pack and upload-blob-pack,
will be created to let the client request a batch of blobs and receive a
packfile. A protocol similar to the fetch-pack/upload-pack will be spoken
between them. (This avoids complicating the existing protocol and the work
of enumerating the refs.) Upload-blob-pack will use pack-objects to build
the packfile.
It is also more efficient than requesting a single blob at a time using
the existing fetch-pack/upload-pack mechanism (with the various allow
unreachable options).
*TODO* The new request protocol will be defined in the patch series.
It will include: a list of the desired blob SHAs. Possibly also the commit
SHA, branch name, and pathname of each blob (or whatever is necessary to let
the server address the reachability concerns). Possibly also the last
known SHA for each blob to allow for deltafication in the packfile.
D. Pre-fetching Blobs (1a)
==========================
On the client side, one or more special commands will be created to assemble
the list of blobs needed for an operation and passed to fetch-blob-pack.
Checkout Example: After running a command like:
'clone --partial-by-size=1m --no-checkout'
and before doing an actual checkout, we need a command to essentially do:
(1) "ls-tree -r <tree-ish>",
(2) filter that by the sparse-checkout currently in effect,
(3) filter that for missing blobs,
(4) and pass the resulting list to fetch-blob-pack.
Afterwards, checkout should complete without faulting.
A new "git ls-partial <treeish>" command has been created to do
steps 1 thru 3 and print the resulting list of SHAs on stdout.
E. Unresolved Thoughts
======================
*TODO* The server should optionally return (in a side-band?) a list
of the blobs that it omitted from the packfile (and possibly the sizes
or sha1_object_info() data for them) during the fetch-pack/upload-pack
operation. This would allow the client to distinguish from invalid
SHAs and missing ones. Size information would allow the client to
maybe choose between various servers.
*TODO* The partial clone arguments should be recorded in ".git/info/"
so that subsequent fetch commands can inherit them and rev-list/index-pack
know to not complain by default.
*TODO* Update GC like rev-list to not complain when there are missing blobs.
*TODO* Extend ls-partial to include the "-m" and 3 tree-ish arguments
like read-tree, so we can pre-fetch for merges that may require file
merges (that may or may not be within our sparse-checkout).
*TODO* I also need to review the RFC that Mark Thomas submitted over
the weekend:
https://public-inbox.org/git/20170304191901.9622-1-markbt%40efaref.net/t
Jeff Hostetler (10):
pack-objects: eat CR in addition to LF after fgets.
pack-objects: add --partial-by-size=n --partial-special
pack-objects: test for --partial-by-size --partial-special
upload-pack: add partial (sparse) fetch
fetch-pack: add partial-by-size and partial-special
rev-list: add --allow-partial option to relax connectivity checks
index-pack: add --allow-partial option to relax blob existence checks
fetch: add partial-by-size and partial-special arguments
clone: add partial-by-size and partial-special arguments
ls-partial: created command to list missing blobs
Documentation/technical/pack-protocol.txt | 14 ++
Documentation/technical/protocol-capabilities.txt | 7 +
Makefile | 2 +
builtin.h | 1 +
builtin/clone.c | 26 ++
builtin/fetch-pack.c | 9 +
builtin/fetch.c | 26 +-
builtin/index-pack.c | 20 +-
builtin/ls-partial.c | 110 +++++++++
builtin/pack-objects.c | 64 ++++-
builtin/rev-list.c | 22 +-
connected.c | 3 +
connected.h | 3 +
fetch-pack.c | 17 ++
fetch-pack.h | 2 +
git.c | 1 +
partial-utils.c | 279 ++++++++++++++++++++++
partial-utils.h | 93 ++++++++
t/5316-pack-objects-partial.sh | 72 ++++++
transport.c | 8 +
transport.h | 8 +
upload-pack.c | 32 ++-
22 files changed, 813 insertions(+), 6 deletions(-)
create mode 100644 builtin/ls-partial.c
create mode 100644 partial-utils.c
create mode 100644 partial-utils.h
create mode 100644 t/5316-pack-objects-partial.sh
--
2.7.4
^ permalink raw reply
* [PATCH 06/10] rev-list: add --allow-partial option to relax connectivity checks
From: Jeff Hostetler @ 2017-03-08 17:38 UTC (permalink / raw)
To: git
Cc: jeffhost, peff, gitster, markbt, benpeart, jonathantanmy,
Jeff Hostetler
In-Reply-To: <1488994685-37403-1-git-send-email-jeffhost@microsoft.com>
From: Jeff Hostetler <git@jeffhostetler.com>
Teach rev-list to optionally not complain when there are missing
blobs. This is for use following a partial clone or fetch when
the server omitted certain blobs.
Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
---
builtin/rev-list.c | 22 +++++++++++++++++++++-
1 file changed, 21 insertions(+), 1 deletion(-)
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 0aa93d5..50c49ba 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -45,6 +45,7 @@ static const char rev_list_usage[] =
" --left-right\n"
" --count\n"
" special purpose:\n"
+" --allow-partial\n"
" --bisect\n"
" --bisect-vars\n"
" --bisect-all"
@@ -53,6 +54,9 @@ static const char rev_list_usage[] =
static struct progress *progress;
static unsigned progress_counter;
+static int allow_partial;
+static struct trace_key trace_partial = TRACE_KEY_INIT(PARTIAL);
+
static void finish_commit(struct commit *commit, void *data);
static void show_commit(struct commit *commit, void *data)
{
@@ -178,8 +182,16 @@ static void finish_commit(struct commit *commit, void *data)
static void finish_object(struct object *obj, const char *name, void *cb_data)
{
struct rev_list_info *info = cb_data;
- if (obj->type == OBJ_BLOB && !has_object_file(&obj->oid))
+ if (obj->type == OBJ_BLOB && !has_object_file(&obj->oid)) {
+ if (allow_partial) {
+ /* Assume a previous partial clone/fetch omitted it. */
+ trace_printf_key(
+ &trace_partial, "omitted blob '%s' '%s'\n",
+ oid_to_hex(&obj->oid), name);
+ return;
+ }
die("missing blob object '%s'", oid_to_hex(&obj->oid));
+ }
if (info->revs->verify_objects && !obj->parsed && obj->type != OBJ_COMMIT)
parse_object(obj->oid.hash);
}
@@ -329,6 +341,14 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
show_progress = arg;
continue;
}
+ if (!strcmp(arg, "--allow-partial")) {
+ allow_partial = 1;
+ continue;
+ }
+ if (!strcmp(arg, "--no-allow-partial")) {
+ allow_partial = 0;
+ continue;
+ }
usage(rev_list_usage);
}
--
2.7.4
^ permalink raw reply related
* [PATCH 04/10] upload-pack: add partial (sparse) fetch
From: Jeff Hostetler @ 2017-03-08 17:37 UTC (permalink / raw)
To: git
Cc: jeffhost, peff, gitster, markbt, benpeart, jonathantanmy,
Jeff Hostetler
In-Reply-To: <1488994685-37403-1-git-send-email-jeffhost@microsoft.com>
From: Jeff Hostetler <git@jeffhostetler.com>
Teach upload-pack to advertise the "partial" capability
in the fetch-pack/upload-pack protocol header and to pass
the value of partial-by-size and partial-special on to
pack-objects.
Update protocol documentation.
This might be used in conjunction with a partial (sparse) clone
or fetch to omit various blobs from the generated packfile.
Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
---
Documentation/technical/pack-protocol.txt | 14 ++++++++++
Documentation/technical/protocol-capabilities.txt | 7 +++++
upload-pack.c | 32 ++++++++++++++++++++++-
3 files changed, 52 insertions(+), 1 deletion(-)
diff --git a/Documentation/technical/pack-protocol.txt b/Documentation/technical/pack-protocol.txt
index c59ac99..0032729 100644
--- a/Documentation/technical/pack-protocol.txt
+++ b/Documentation/technical/pack-protocol.txt
@@ -212,6 +212,7 @@ out of what the server said it could do with the first 'want' line.
upload-request = want-list
*shallow-line
*1depth-request
+ *partial
flush-pkt
want-list = first-want
@@ -223,10 +224,15 @@ out of what the server said it could do with the first 'want' line.
PKT-LINE("deepen-since" SP timestamp) /
PKT-LINE("deepen-not" SP ref)
+ partial = PKT-LINE("partial-by-size" SP magnitude) /
+ PKT-LINE("partial-special)
+
first-want = PKT-LINE("want" SP obj-id SP capability-list)
additional-want = PKT-LINE("want" SP obj-id)
depth = 1*DIGIT
+
+ magnitude = 1*DIGIT [ "k" | "m" | "g" ]
----
Clients MUST send all the obj-ids it wants from the reference
@@ -249,6 +255,14 @@ complete those commits. Commits whose parents are not received as a
result are defined as shallow and marked as such in the server. This
information is sent back to the client in the next step.
+The client can optionally request a partial packfile that omits
+various blobs. The value of "partial-by-size" is a non-negative
+integer with optional units and requests blobs smaller than this
+value. The "partial-special" command requests git-special files,
+such as ".gitignore". Using both requests the union of the two.
+These requests are only valid if the server advertises the "partial"
+capability.
+
Once all the 'want's and 'shallow's (and optional 'deepen') are
transferred, clients MUST send a flush-pkt, to tell the server side
that it is done sending the list.
diff --git a/Documentation/technical/protocol-capabilities.txt b/Documentation/technical/protocol-capabilities.txt
index 26dcc6f..9aa2123 100644
--- a/Documentation/technical/protocol-capabilities.txt
+++ b/Documentation/technical/protocol-capabilities.txt
@@ -309,3 +309,10 @@ to accept a signed push certificate, and asks the <nonce> to be
included in the push certificate. A send-pack client MUST NOT
send a push-cert packet unless the receive-pack server advertises
this capability.
+
+partial
+-------
+
+If the upload-pack server advertises this capability, fetch-pack
+may send various "partial-*" commands to request a partial clone
+or fetch where the server omits certain blobs from the packfile.
diff --git a/upload-pack.c b/upload-pack.c
index 7597ba3..74f9dfa 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -63,6 +63,11 @@ static int advertise_refs;
static int stateless_rpc;
static const char *pack_objects_hook;
+static struct strbuf partial_by_size = STRBUF_INIT;
+static int client_requested_partial_capability;
+static int have_partial_by_size;
+static int have_partial_special;
+
static void reset_timeout(void)
{
alarm(timeout);
@@ -130,6 +135,10 @@ static void create_pack_file(void)
argv_array_push(&pack_objects.args, "--delta-base-offset");
if (use_include_tag)
argv_array_push(&pack_objects.args, "--include-tag");
+ if (have_partial_by_size)
+ argv_array_push(&pack_objects.args, partial_by_size.buf);
+ if (have_partial_special)
+ argv_array_push(&pack_objects.args, "--partial-special");
pack_objects.in = -1;
pack_objects.out = -1;
@@ -793,6 +802,23 @@ static void receive_needs(void)
deepen_rev_list = 1;
continue;
}
+ if (skip_prefix(line, "partial-by-size ", &arg)) {
+ unsigned long s;
+ if (!client_requested_partial_capability)
+ die("git upload-pack: 'partial-by-size' option requires 'partial' capability");
+ if (!git_parse_ulong(arg, &s))
+ die("git upload-pack: invalid partial-by-size value: %s", line);
+ strbuf_addstr(&partial_by_size, "--partial-by-size=");
+ strbuf_addstr(&partial_by_size, arg);
+ have_partial_by_size = 1;
+ continue;
+ }
+ if (skip_prefix(line, "partial-special", &arg)) {
+ if (!client_requested_partial_capability)
+ die("git upload-pack: 'partial-special' option requires 'partial' capability");
+ have_partial_special = 1;
+ continue;
+ }
if (!skip_prefix(line, "want ", &arg) ||
get_sha1_hex(arg, sha1_buf))
die("git upload-pack: protocol error, "
@@ -820,6 +846,8 @@ static void receive_needs(void)
no_progress = 1;
if (parse_feature_request(features, "include-tag"))
use_include_tag = 1;
+ if (parse_feature_request(features, "partial"))
+ client_requested_partial_capability = 1;
o = parse_object(sha1_buf);
if (!o)
@@ -924,7 +952,9 @@ static int send_ref(const char *refname, const struct object_id *oid,
{
static const char *capabilities = "multi_ack thin-pack side-band"
" side-band-64k ofs-delta shallow deepen-since deepen-not"
- " deepen-relative no-progress include-tag multi_ack_detailed";
+ " deepen-relative no-progress include-tag multi_ack_detailed"
+ " partial"
+ ;
const char *refname_nons = strip_namespace(refname);
struct object_id peeled;
--
2.7.4
^ permalink raw reply related
* [PATCH 05/10] fetch-pack: add partial-by-size and partial-special
From: Jeff Hostetler @ 2017-03-08 17:38 UTC (permalink / raw)
To: git
Cc: jeffhost, peff, gitster, markbt, benpeart, jonathantanmy,
Jeff Hostetler
In-Reply-To: <1488994685-37403-1-git-send-email-jeffhost@microsoft.com>
From: Jeff Hostetler <git@jeffhostetler.com>
Teach fetch-pack to take --partial-by-size and --partial-special
arguments and pass them via the transport to upload-pack to
request that certain blobs be omitted from the resulting packfile.
Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
---
builtin/fetch-pack.c | 9 +++++++++
fetch-pack.c | 17 +++++++++++++++++
fetch-pack.h | 2 ++
transport.c | 8 ++++++++
transport.h | 8 ++++++++
5 files changed, 44 insertions(+)
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index cfe9e44..324d7b2 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -8,6 +8,7 @@
static const char fetch_pack_usage[] =
"git fetch-pack [--all] [--stdin] [--quiet | -q] [--keep | -k] [--thin] "
"[--include-tag] [--upload-pack=<git-upload-pack>] [--depth=<n>] "
+"[--partial-by-size=<n>] [--partial-special] "
"[--no-progress] [--diag-url] [-v] [<host>:]<directory> [<refs>...]";
static void add_sought_entry(struct ref ***sought, int *nr, int *alloc,
@@ -143,6 +144,14 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
args.update_shallow = 1;
continue;
}
+ if (skip_prefix(arg, "--partial-by-size=", &arg)) {
+ args.partial_by_size = xstrdup(arg);
+ continue;
+ }
+ if (!strcmp("--partial-special", arg)) {
+ args.partial_special = 1;
+ continue;
+ }
usage(fetch_pack_usage);
}
if (deepen_not.nr)
diff --git a/fetch-pack.c b/fetch-pack.c
index e0f5d5c..e355c38 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -372,6 +372,8 @@ static int find_common(struct fetch_pack_args *args,
if (prefer_ofs_delta) strbuf_addstr(&c, " ofs-delta");
if (deepen_since_ok) strbuf_addstr(&c, " deepen-since");
if (deepen_not_ok) strbuf_addstr(&c, " deepen-not");
+ if (args->partial_by_size || args->partial_special)
+ strbuf_addstr(&c, " partial");
if (agent_supported) strbuf_addf(&c, " agent=%s",
git_user_agent_sanitized());
packet_buf_write(&req_buf, "want %s%s\n", remote_hex, c.buf);
@@ -402,6 +404,12 @@ static int find_common(struct fetch_pack_args *args,
packet_buf_write(&req_buf, "deepen-not %s", s->string);
}
}
+
+ if (args->partial_by_size)
+ packet_buf_write(&req_buf, "partial-by-size %s", args->partial_by_size);
+ if (args->partial_special)
+ packet_buf_write(&req_buf, "partial-special");
+
packet_buf_flush(&req_buf);
state_len = req_buf.len;
@@ -807,6 +815,10 @@ static int get_pack(struct fetch_pack_args *args,
"--keep=fetch-pack %"PRIuMAX " on %s",
(uintmax_t)getpid(), hostname);
}
+
+ if (args->partial_by_size || args->partial_special)
+ argv_array_push(&cmd.args, "--allow-partial");
+
if (args->check_self_contained_and_connected)
argv_array_push(&cmd.args, "--check-self-contained-and-connected");
}
@@ -920,6 +932,11 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args,
else
prefer_ofs_delta = 0;
+ if (server_supports("partial"))
+ print_verbose(args, _("Server supports partial"));
+ else if (args->partial_by_size || args->partial_special)
+ die(_("Server does not support 'partial'"));
+
if ((agent_feature = server_feature_value("agent", &agent_len))) {
agent_supported = 1;
if (agent_len)
diff --git a/fetch-pack.h b/fetch-pack.h
index c912e3d..b8a26e0 100644
--- a/fetch-pack.h
+++ b/fetch-pack.h
@@ -12,6 +12,7 @@ struct fetch_pack_args {
int depth;
const char *deepen_since;
const struct string_list *deepen_not;
+ const char *partial_by_size;
unsigned deepen_relative:1;
unsigned quiet:1;
unsigned keep_pack:1;
@@ -29,6 +30,7 @@ struct fetch_pack_args {
unsigned cloning:1;
unsigned update_shallow:1;
unsigned deepen:1;
+ unsigned partial_special:1;
};
/*
diff --git a/transport.c b/transport.c
index 5828e06..45f35a4 100644
--- a/transport.c
+++ b/transport.c
@@ -160,6 +160,12 @@ static int set_git_option(struct git_transport_options *opts,
} else if (!strcmp(name, TRANS_OPT_DEEPEN_RELATIVE)) {
opts->deepen_relative = !!value;
return 0;
+ } else if (!strcmp(name, TRANS_OPT_PARTIAL_BY_SIZE)) {
+ opts->partial_by_size = xstrdup(value);
+ return 0;
+ } else if (!strcmp(name, TRANS_OPT_PARTIAL_SPECIAL)) {
+ opts->partial_special = !!value;
+ return 0;
}
return 1;
}
@@ -227,6 +233,8 @@ static int fetch_refs_via_pack(struct transport *transport,
data->options.check_self_contained_and_connected;
args.cloning = transport->cloning;
args.update_shallow = data->options.update_shallow;
+ args.partial_by_size = data->options.partial_by_size;
+ args.partial_special = data->options.partial_special;
if (!data->got_remote_heads) {
connect_setup(transport, 0);
diff --git a/transport.h b/transport.h
index bc55715..c3f2d52 100644
--- a/transport.h
+++ b/transport.h
@@ -15,12 +15,14 @@ struct git_transport_options {
unsigned self_contained_and_connected : 1;
unsigned update_shallow : 1;
unsigned deepen_relative : 1;
+ unsigned partial_special : 1;
int depth;
const char *deepen_since;
const struct string_list *deepen_not;
const char *uploadpack;
const char *receivepack;
struct push_cas_option *cas;
+ const char *partial_by_size;
};
enum transport_family {
@@ -210,6 +212,12 @@ void transport_check_allowed(const char *type);
/* Send push certificates */
#define TRANS_OPT_PUSH_CERT "pushcert"
+/* Partial fetch to only include small files */
+#define TRANS_OPT_PARTIAL_BY_SIZE "partial-by-size"
+
+/* Partial fetch to only include special files, like ".gitignore" */
+#define TRANS_OPT_PARTIAL_SPECIAL "partial-special"
+
/**
* Returns 0 if the option was used, non-zero otherwise. Prints a
* message to stderr if the option is not used.
--
2.7.4
^ permalink raw reply related
* [PATCH 07/10] index-pack: add --allow-partial option to relax blob existence checks
From: Jeff Hostetler @ 2017-03-08 17:38 UTC (permalink / raw)
To: git
Cc: jeffhost, peff, gitster, markbt, benpeart, jonathantanmy,
Jeff Hostetler
In-Reply-To: <1488994685-37403-1-git-send-email-jeffhost@microsoft.com>
From: Jeff Hostetler <git@jeffhostetler.com>
Teach index-pack to optionally not complain when there are missing
blobs. This is for use following a partial clone or fetch when
the server omitted certain blobs.
Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
---
builtin/index-pack.c | 20 ++++++++++++++++++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index f4b87c6..8f99408 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -13,7 +13,7 @@
#include "thread-utils.h"
static const char index_pack_usage[] =
-"git index-pack [-v] [-o <index-file>] [--keep | --keep=<msg>] [--verify] [--strict] (<pack-file> | --stdin [--fix-thin] [<pack-file>])";
+"git index-pack [-v] [-o <index-file>] [--keep | --keep=<msg>] [--verify] [--allow-partial] [--strict] (<pack-file> | --stdin [--fix-thin] [<pack-file>])";
struct object_entry {
struct pack_idx_entry idx;
@@ -81,6 +81,9 @@ static int show_resolving_progress;
static int show_stat;
static int check_self_contained_and_connected;
+static int allow_partial;
+static struct trace_key trace_partial = TRACE_KEY_INIT(PARTIAL);
+
static struct progress *progress;
/* We always read in 4kB chunks. */
@@ -220,9 +223,18 @@ static unsigned check_object(struct object *obj)
if (!(obj->flags & FLAG_CHECKED)) {
unsigned long size;
int type = sha1_object_info(obj->oid.hash, &size);
- if (type <= 0)
+ if (type <= 0) {
+ if (allow_partial > 0 && obj->type == OBJ_BLOB) {
+ /* Assume a previous partial clone/fetch omitted it. */
+ trace_printf_key(
+ &trace_partial, "omitted blob '%s'\n",
+ oid_to_hex(&obj->oid));
+ obj->flags |= FLAG_CHECKED;
+ return 0;
+ }
die(_("did not receive expected object %s"),
oid_to_hex(&obj->oid));
+ }
if (type != obj->type)
die(_("object %s: expected type %s, found %s"),
oid_to_hex(&obj->oid),
@@ -1718,6 +1730,10 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)
die(_("bad %s"), arg);
} else if (skip_prefix(arg, "--max-input-size=", &arg)) {
max_input_size = strtoumax(arg, NULL, 10);
+ } else if (!strcmp(arg, "--allow-partial")) {
+ allow_partial = 1;
+ } else if (!strcmp(arg, "--no-allow-partial")) {
+ allow_partial = 0;
} else
usage(index_pack_usage);
continue;
--
2.7.4
^ permalink raw reply related
* [PATCH 02/10] pack-objects: add --partial-by-size=n --partial-special
From: Jeff Hostetler @ 2017-03-08 17:37 UTC (permalink / raw)
To: git
Cc: jeffhost, peff, gitster, markbt, benpeart, jonathantanmy,
Jeff Hostetler
In-Reply-To: <1488994685-37403-1-git-send-email-jeffhost@microsoft.com>
From: Jeff Hostetler <git@jeffhostetler.com>
Teach pack-objects to omit blobs from the generated packfile.
When the --partial-by-size=n[kmg] argument is used, only blobs
smaller than the requested size are included. When n is zero,
no blobs are included.
When the --partial-special argument is used, git special files,
such as ".gitattributes" and ".gitignores" are included.
When both are given, the union of two are included.
This is intended to be used in a partial clone or fetch.
(This has also been called sparse- or lazy-clone.)
Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
---
builtin/pack-objects.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 61 insertions(+), 1 deletion(-)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 7e052bb..2df2f49 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -77,6 +77,10 @@ static unsigned long cache_max_small_delta_size = 1000;
static unsigned long window_memory_limit = 0;
+static signed long partial_by_size = -1;
+static int partial_special = 0;
+static struct trace_key trace_partial = TRACE_KEY_INIT(PARTIAL);
+
/*
* stats
*/
@@ -2532,6 +2536,54 @@ static void show_object(struct object *obj, const char *name, void *data)
obj->flags |= OBJECT_ADDED;
}
+/*
+ * If ANY --partial-* option was given, we want to OMIT all
+ * blobs UNLESS they match one of our patterns. We treat
+ * the options as OR's so that we get the resulting UNION.
+ */
+static void show_object_partial(struct object *obj, const char *name, void *data)
+{
+ unsigned long s = 0;
+
+ if (obj->type != OBJ_BLOB)
+ goto include_it;
+
+ /*
+ * When (partial_by_size == 0), we want to OMIT all blobs.
+ * When (partial_by_size > 0), we want blobs smaller than that.
+ */
+ if (partial_by_size > 0) {
+ enum object_type t = sha1_object_info(obj->oid.hash, &s);
+ assert(t == OBJ_BLOB);
+ if (s < partial_by_size)
+ goto include_it;
+ }
+
+ /*
+ * When (partial_special), we want the .git* special files.
+ */
+ if (partial_special) {
+ if (strcmp(name, GITATTRIBUTES_FILE) == 0 ||
+ strcmp(name, ".gitignore") == 0)
+ goto include_it;
+ else {
+ const char *last_slash = strrchr(name, '/');
+ if (last_slash)
+ if (strcmp(last_slash+1, GITATTRIBUTES_FILE) == 0 ||
+ strcmp(last_slash+1, ".gitignore") == 0)
+ goto include_it;
+ }
+ }
+
+ trace_printf_key(
+ &trace_partial, "omitting blob '%s' %"PRIuMAX" '%s'\n",
+ oid_to_hex(&obj->oid), (uintmax_t)s, name);
+ return;
+
+include_it:
+ show_object(obj, name, data);
+}
+
static void show_edge(struct commit *commit)
{
add_preferred_base(commit->object.oid.hash);
@@ -2794,7 +2846,11 @@ static void get_object_list(int ac, const char **av)
if (prepare_revision_walk(&revs))
die("revision walk setup failed");
mark_edges_uninteresting(&revs, show_edge);
- traverse_commit_list(&revs, show_commit, show_object, NULL);
+
+ if (partial_by_size >= 0 || partial_special)
+ traverse_commit_list(&revs, show_commit, show_object_partial, NULL);
+ else
+ traverse_commit_list(&revs, show_commit, show_object, NULL);
if (unpack_unreachable_expiration) {
revs.ignore_missing_links = 1;
@@ -2930,6 +2986,10 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
N_("use a bitmap index if available to speed up counting objects")),
OPT_BOOL(0, "write-bitmap-index", &write_bitmap_index,
N_("write a bitmap index together with the pack index")),
+ OPT_MAGNITUDE(0, "partial-by-size", (unsigned long *)&partial_by_size,
+ N_("only include blobs smaller than size in result")),
+ OPT_BOOL(0, "partial-special", &partial_special,
+ N_("only include blobs for git special files")),
OPT_END(),
};
--
2.7.4
^ permalink raw reply related
* Re: Crash on MSYS2 with GIT_WORK_TREE
From: Junio C Hamano @ 2017-03-08 17:24 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Brandon Williams, Stefan Beller, git
In-Reply-To: <alpine.DEB.2.20.1703081304130.3767@virtualbox>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> Rather than a heavy-handed reversal, I would really prefer to perform a
> diligent audit of all real_pathdup() callers and adjust them
> appropriately.
>
> Turns out that the canonicalize_ceiling_entry() caller is *the only one*
> handling NULL correctly. All other callers need to be changed.
The observation matches what I saw.
I would have added a patch that introduces real_pathdup_gently()
that returns NULL and made real_pathdup() die as before immediately
on top of the Brandon's series (which would ensure that any branch
that would want to use real_pathdup() would get the dying version by
default) and flipped selected callers to call the gently() version
in that patch, but what you posted is a lot more apprporiate for a
regression fix. By changing the function signature, the patch
ensures that it covers all the callsites.
Among the two-and-half regressions known to us post 2.12 so far,
this one probably needs to go into a maintenance release without
waiting for the other one-and-half, I think.
Thanks.
^ permalink raw reply
* Re: [PATCH v3 0/9] Fix the early config
From: Junio C Hamano @ 2017-03-08 17:09 UTC (permalink / raw)
To: Jeff King; +Cc: Johannes Schindelin, git, Duy Nguyen
In-Reply-To: <20170308073035.pt5ysp6bzpozoqps@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> Good catch. Another "non-gentle" thing I noticed here while looking at
> another thread: the repository-format version check uses the config
> parser, which will die() in certain circumstances. So for instance:
>
> $ git init
> $ git rev-parse && echo ok
> ok
>
> $ echo '[core]repositoryformatversion = 10' >.git/config
> $ git rev-parse && echo ok
> fatal: Expected git repo version <= 1, found 10
Just to set my expectation straight. Do you expect/wish this not to
fail because of this in cmd_rev_parse()?
/* No options; just report on whether we're in a git repo or not. */
if (argc == 1) {
setup_git_directory();
git_config(git_default_config, NULL);
return 0;
}
Because we do not have anything other than yes/no to the question
"Are we in Git repository?", I'd expect that the expected answer to
the question would be "no" (if we could say "Yes, we are in a Git
repository but its version and layout is unknown to us so we are not
supposed to look at or touch it", that is a different matter).
So "fatal:" may be bad, but I think not seeing "ok" is what we
want to happen in this case.
Having said that, I am not sure asking for default-config is what we
wanted to do in the above code. Perhaps a more modern way to write
the above code would be to do the "gently" version of setup, without
calling git_config() ourselves, and return the resulting value
returned in *nongit_ok? If we can do so without triggering "fatal:"
and still return "no, we are not in a Git repository we are supposed
to touch", that would be good.
Or are you discussing a more general issue, iow, anything that can
work without repository (i.e. those who do _gently version of the
setup and act on *nongit_ok) should pretend as if there were no
(broken) repository and take the "no we are not in a repository"
codepath?
^ permalink raw reply
* Re: [PATCH 4/6] send-pack: improve unpack-status error messages
From: Junio C Hamano @ 2017-03-08 17:19 UTC (permalink / raw)
To: Jeff King; +Cc: Horst Schirmeier, git
In-Reply-To: <20170308054533.7wy52myrbpovyoet@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
>> > And finally, neither message was marked for translation. The
>> > message from the remote probably won't be translated, but
>> > there's no reason we can't do better for the local half.
>>
>> Hmm, OK.
>
> I'll admit that I don't actually use the translations myself, being a
> native English speaker. So I am just guessing that somebody for whom
> English is a second language would rather see the first half in a more
> intelligible format. That at least tells them what the second half _is_,
> so they might be able to search for the error with more context.
>
> If my guess is wrong, though, I'm happy to retract that part or bump it
> out to a separate patch.
I was merely undecided between "at least half is in my language" and
"both are consistently untranslated" which one is easier to use by
my highschool friends who do not grok English. And I still cannot
decide.
When responding to a request-for-help that quotes messages that was
translated, I would imagine we would need one extra "git grep" to
find the message (without understanding that ourselves) from po/ for
the original before running "git grep" to find the code that
produced the message [*1*].
On the other hand, we get request-for-help from those having issues
in a setup where the software running on the other side is not even
ours, so I am (slightly) more inclined to agree that "half is in my
language" is better than nothing.
Thanks.
[Footnote]
*1* If the translation came from us, that is. If I recall
correctly, some distros do their own po/ and core Git developers are
not likely to have them around, so "git grep" in our po/ may not see
any hit.
^ permalink raw reply
* Re: [PATCH v3 0/9] Fix the early config
From: Johannes Schindelin @ 2017-03-08 16:18 UTC (permalink / raw)
To: Jeff King; +Cc: git, Junio C Hamano, Duy Nguyen
In-Reply-To: <20170308073035.pt5ysp6bzpozoqps@sigill.intra.peff.net>
Hi Peff,
On Wed, 8 Mar 2017, Jeff King wrote:
> Another "non-gentle" thing I noticed here while looking at
> another thread: the repository-format version check uses the config
> parser, which will die() in certain circumstances. [...]
Yes, that is part of the reason why I was not eager to add that check to
discover_git_directory(). The config code is die()-happy.
This is a much bigger problem, of course, and related to a constant gripe
of mine: I *always* get the quoting wrong in my aliases. Always. And when
I want to fix it, `git config -e` simply errors out because of an invalid
config. Yes, Git, I know, that is the exact reason why I want to edit the
config in the first place.
I am certain you will agree that this is a different topic, therefore
subject to a separate patch series.
In any case, I am fairly certain that the examples you showed demonstrate
that the config has to be rather broken for this patch series to have a
negative impact. And it still would report the broken config so that the
user is not blocked (she can fix the config and call the paginating
command again).
> On Tue, Mar 07, 2017 at 03:31:43PM +0100, Johannes Schindelin wrote:
>
> > And another change: the GIT_DIR_NONE value was handled incorrectly in
> > discover_git_directory().
>
> This is the "if (setup_git_directory_1() <= 0)" change from the
> interdiff? That's subtle.
Yes, it is subtle.
> The compiler could have noticed if we used a switch statement here. But
> then any new error conditions would have to be added to that switch
> statement.
We could still do that.
> > I am slightly disappointed that the these additional problems were not
> > spotted in any review but my own. And I had not even included a Duck.
>
> Get used to being disappointed, I guess. A non-zero number of bugs will
> slip through when writing code _and_ when reviewing it.
I know that. I know that bugs are prone to come in through code
contributions. I don't go for 100%. But I would hope for a better rate
than we have right now: we pride ourselves in the OSS community to make bugs
shallow. I really would like to believe that we catch bugs rather than
discuss formatting (which should be automated) or white-space mangling
(which should not even be a problem if we were truly open for
contributions).
Maybe I am just a grumpy old guy. But I *hate* seeing how much buggy code
we get into Git, despite having a review process that does a very good job
of deterring seasoned developers from contributing.
I really wish it were different. I really wish that we did a better job at
catching bugs before they enter `master`. I really wish that I could be
proud of our code review process.
> > [ceil_offset]
> > Hopefully that clears up the picture?
>
> Yes, it does. Thanks.
Perfect. Then the time I spent trying to figure all of this out was not
spent in vain.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 0/2] Fix crashes due to real_pathdup() potentially returning NULL
From: Jeff King @ 2017-03-08 16:17 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Junio C Hamano, Brandon Williams, Stefan Beller
In-Reply-To: <cover.1488987786.git.johannes.schindelin@gmx.de>
On Wed, Mar 08, 2017 at 04:43:27PM +0100, Johannes Schindelin wrote:
> We may want to consider fast-tracking this into v2.12.1, and to that
> end, I would appreciate code reviews that focus on the correctness of
> this patch and that try to consider undesired side effects.
I don't see how it could be not-correct, in the sense that every caller
now passes the die_on_error flag (restoring the original behavior)
except for the one which clearly checks for a NULL return immediately
afterward.
The only exception would be if there were new calls to real_pathdup()
that did not originally use real_path(). But:
# 7241764076 introduced real_pathdup
git log -Sreal_pathdup 7241764076..
shows only one other introduction, and it's just duplicating an existing
call.
It's possible that some of these _could_ handle the error case more
gracefully (I already fixed one such case in 3a1345af2). But even if
we wanted to do so, that should come separately on top of this patch.
This can go to 'maint' as a regression fix, and then that gives a stable
base for making potential improvements.
-Peff
^ permalink raw reply
* Re: diff.ignoreSubmoudles config setting broken?
From: Sebastian Schuberth @ 2017-03-08 13:43 UTC (permalink / raw)
To: Jeff King; +Cc: Git Mailing List, Stefan Beller
In-Reply-To: <20170308133348.2ovfsi44vq2rpgom@sigill.intra.peff.net>
On Wed, Mar 8, 2017 at 2:33 PM, Jeff King <peff@peff.net> wrote:
>> I'm getting
>>
>> $ git config --global diff.ignoreSubmodules all
>> $ git diff
>> diff --git a/scanners/scancode-toolkit b/scanners/scancode-toolkit
>> index 65e5c9c..6b021a8 160000
>> --- a/scanners/scancode-toolkit
>> +++ b/scanners/scancode-toolkit
>> @@ -1 +1 @@
>> -Subproject commit 65e5c9c9508441c5f62beff4749cf455c6eadc30
>> +Subproject commit 6b021a8addf6d3c5f2a6ef1af6245e095c21d8ec
>>
>> but with
>>
>> $ git diff --ignore-submodules=all
>
> Hrm. Isn't "all" the default? That's what git-diff(1) says (but I've
> never used the feature myself).
>
> That would imply to me that there's another config option set somewhere
> (perhaps in the repo-level config). What does:
>
> git config --show-origin --get-all diff.ignoresubmodules
>
> say?
It says:
file:/home/seschube/.gitconfig all
--
Sebastian Schuberth
^ permalink raw reply
* [PATCH 1/2] Demonstrate NULL pointer access with invalid GIT_WORK_TREE
From: Johannes Schindelin @ 2017-03-08 15:43 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Brandon Williams, Stefan Beller
In-Reply-To: <cover.1488987786.git.johannes.schindelin@gmx.de>
When GIT_WORK_TREE does not specify a valid path, we should error out.
Not crash.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
t/t1501-work-tree.sh | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/t/t1501-work-tree.sh b/t/t1501-work-tree.sh
index cc5b870e587..046d9b7909f 100755
--- a/t/t1501-work-tree.sh
+++ b/t/t1501-work-tree.sh
@@ -423,4 +423,12 @@ test_expect_success '$GIT_WORK_TREE overrides $GIT_DIR/common' '
)
'
+test_expect_failure 'error out gracefully on invalid $GIT_WORK_TREE' '
+ (
+ GIT_WORK_TREE=/.invalid/work/tree &&
+ export GIT_WORK_TREE &&
+ test_expect_code 128 git rev-parse
+ )
+'
+
test_done
--
2.12.0.windows.1.7.g94dafc3b124
^ permalink raw reply related
* Re: [PATCH] t2027: avoid using pipes
From: Jon Loeliger @ 2017-03-08 15:44 UTC (permalink / raw)
To: Prathamesh Chavan; +Cc: git
In-Reply-To: <0102015aae7b8536-00c57d0a-1d48-4153-a202-87c4ea9e0e19-000000@eu-west-1.amazonses.com>
So, like, Prathamesh Chavan said:
> The exit code of the upstream of a pipe is ignored thus we should avoid
> using it. By writing out the output of the git command to a file, we
> can test the exit codes of both the commands.
>
> Signed-off-by: Prathamesh <pc44800@gmail.com>
> ---
> t/t2027-worktree-list.sh | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/t/t2027-worktree-list.sh b/t/t2027-worktree-list.sh
> index 848da5f..daa7a04 100755
> --- a/t/t2027-worktree-list.sh
> +++ b/t/t2027-worktree-list.sh
> @@ -31,7 +31,7 @@ test_expect_success '"list" all worktrees from main' '
> test_when_finished "rm -rf here && git worktree prune" &&
> git worktree add --detach here master &&
> echo "$(git -C here rev-parse --show-toplevel) $(git rev-parse --short
> HEAD) (detached HEAD)" >>expect &&
> - git worktree list | sed "s/ */ /g" >actual &&
> + git worktree list >out && sed "s/ */ /g" <out >actual &&
> test_cmp expect actual
> '
I confess I am not familiar with the test set up.
However, I'd ask the question do we care about the
lingering "out" and "actual" files here? Or will
they silently be cleaned up along the way later?
Thanks,
jdl
^ permalink raw reply
* [PATCH 2/2] Fix callsites of real_pathdup() that wanted it to die on error
From: Johannes Schindelin @ 2017-03-08 15:43 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Brandon Williams, Stefan Beller
In-Reply-To: <cover.1488987786.git.johannes.schindelin@gmx.de>
In 4ac9006f832 (real_path: have callers use real_pathdup and
strbuf_realpath, 2016-12-12), we changed the xstrdup(real_path())
pattern to use real_pathdup() directly.
The only problem with this change is that real_path() calls
strbuf_realpath() with die_on_error = 1 while real_pathdup() calls it
with die_on_error = 0. Meaning that in cases where real_path() causes
Git to die() with an error message, real_pathdup() is silent and returns
NULL instead.
The callers, however, are ill-prepared for that change, as they expect
the return value to be non-NULL.
This patch fixes that by extending real_pathdup()'s signature to accept
the die_on_error flag and simply pass it through to strbuf_realpath(),
and then adjust all callers after a careful audit whether they would
handle NULLs well.
Note: this fix not only prevents NULL pointer accesses, but it also
reintroduces the error messages that were lost with the change to
real_pathdup().
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
abspath.c | 4 ++--
builtin/init-db.c | 6 +++---
cache.h | 2 +-
dir.c | 4 ++--
environment.c | 2 +-
setup.c | 4 ++--
submodule.c | 10 +++++-----
t/t1501-work-tree.sh | 2 +-
worktree.c | 2 +-
9 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/abspath.c b/abspath.c
index 2f0c26e0e2c..b02e068aa34 100644
--- a/abspath.c
+++ b/abspath.c
@@ -214,12 +214,12 @@ const char *real_path_if_valid(const char *path)
return strbuf_realpath(&realpath, path, 0);
}
-char *real_pathdup(const char *path)
+char *real_pathdup(const char *path, int die_on_error)
{
struct strbuf realpath = STRBUF_INIT;
char *retval = NULL;
- if (strbuf_realpath(&realpath, path, 0))
+ if (strbuf_realpath(&realpath, path, die_on_error))
retval = strbuf_detach(&realpath, NULL);
strbuf_release(&realpath);
diff --git a/builtin/init-db.c b/builtin/init-db.c
index 1d4d6a00789..8a6acb0ec69 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -338,7 +338,7 @@ int init_db(const char *git_dir, const char *real_git_dir,
{
int reinit;
int exist_ok = flags & INIT_DB_EXIST_OK;
- char *original_git_dir = real_pathdup(git_dir);
+ char *original_git_dir = real_pathdup(git_dir, 1);
if (real_git_dir) {
struct stat st;
@@ -489,7 +489,7 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
argc = parse_options(argc, argv, prefix, init_db_options, init_db_usage, 0);
if (real_git_dir && !is_absolute_path(real_git_dir))
- real_git_dir = real_pathdup(real_git_dir);
+ real_git_dir = real_pathdup(real_git_dir, 1);
if (argc == 1) {
int mkdir_tried = 0;
@@ -560,7 +560,7 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
const char *git_dir_parent = strrchr(git_dir, '/');
if (git_dir_parent) {
char *rel = xstrndup(git_dir, git_dir_parent - git_dir);
- git_work_tree_cfg = real_pathdup(rel);
+ git_work_tree_cfg = real_pathdup(rel, 1);
free(rel);
}
if (!git_work_tree_cfg)
diff --git a/cache.h b/cache.h
index 80b6372cf76..ae0b4fc70b5 100644
--- a/cache.h
+++ b/cache.h
@@ -1153,7 +1153,7 @@ char *strbuf_realpath(struct strbuf *resolved, const char *path,
int die_on_error);
const char *real_path(const char *path);
const char *real_path_if_valid(const char *path);
-char *real_pathdup(const char *path);
+char *real_pathdup(const char *path, int die_on_error);
const char *absolute_path(const char *path);
char *absolute_pathdup(const char *path);
const char *remove_leading_path(const char *in, const char *prefix);
diff --git a/dir.c b/dir.c
index 4541f9e1460..aeeb5ce1049 100644
--- a/dir.c
+++ b/dir.c
@@ -2730,8 +2730,8 @@ void connect_work_tree_and_git_dir(const char *work_tree_, const char *git_dir_)
{
struct strbuf file_name = 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 = real_pathdup(git_dir_, 1);
+ char *work_tree = real_pathdup(work_tree_, 1);
/* Update gitfile */
strbuf_addf(&file_name, "%s/.git", work_tree);
diff --git a/environment.c b/environment.c
index c07fb17fb70..42dc3106d2f 100644
--- a/environment.c
+++ b/environment.c
@@ -259,7 +259,7 @@ void set_git_work_tree(const char *new_work_tree)
return;
}
git_work_tree_initialized = 1;
- work_tree = real_pathdup(new_work_tree);
+ work_tree = real_pathdup(new_work_tree, 1);
}
const char *get_git_work_tree(void)
diff --git a/setup.c b/setup.c
index 967f289f1ef..6b48cb91ff2 100644
--- a/setup.c
+++ b/setup.c
@@ -698,7 +698,7 @@ static const char *setup_discovered_git_dir(const char *gitdir,
/* --work-tree is set without --git-dir; use discovered one */
if (getenv(GIT_WORK_TREE_ENVIRONMENT) || git_work_tree_cfg) {
if (offset != cwd->len && !is_absolute_path(gitdir))
- gitdir = real_pathdup(gitdir);
+ gitdir = real_pathdup(gitdir, 1);
if (chdir(cwd->buf))
die_errno("Could not come back to cwd");
return setup_explicit_git_dir(gitdir, cwd, nongit_ok);
@@ -806,7 +806,7 @@ static int canonicalize_ceiling_entry(struct string_list_item *item,
/* Keep entry but do not canonicalize it */
return 1;
} else {
- char *real_path = real_pathdup(ceil);
+ char *real_path = real_pathdup(ceil, 0);
if (!real_path) {
return 0;
}
diff --git a/submodule.c b/submodule.c
index 3b98766a6bc..0a2831d846d 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1403,7 +1403,7 @@ static void relocate_single_git_dir_into_superproject(const char *prefix,
/* If it is an actual gitfile, it doesn't need migration. */
return;
- real_old_git_dir = real_pathdup(old_git_dir);
+ real_old_git_dir = real_pathdup(old_git_dir, 1);
sub = submodule_from_path(null_sha1, path);
if (!sub)
@@ -1412,7 +1412,7 @@ static void relocate_single_git_dir_into_superproject(const char *prefix,
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);
+ real_new_git_dir = real_pathdup(new_git_dir, 1);
if (!prefix)
prefix = get_super_prefix();
@@ -1472,14 +1472,14 @@ void absorb_git_dir_into_superproject(const char *prefix,
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);
+ real_new_git_dir = real_pathdup(new_git_dir, 1);
connect_work_tree_and_git_dir(path, real_new_git_dir);
free(real_new_git_dir);
} else {
/* Is it already absorbed into the superprojects git dir? */
- char *real_sub_git_dir = real_pathdup(sub_git_dir);
- char *real_common_git_dir = real_pathdup(get_git_common_dir());
+ char *real_sub_git_dir = real_pathdup(sub_git_dir, 1);
+ char *real_common_git_dir = real_pathdup(get_git_common_dir(), 1);
if (!starts_with(real_sub_git_dir, real_common_git_dir))
relocate_single_git_dir_into_superproject(prefix, path);
diff --git a/t/t1501-work-tree.sh b/t/t1501-work-tree.sh
index 046d9b7909f..b06210ec5e8 100755
--- a/t/t1501-work-tree.sh
+++ b/t/t1501-work-tree.sh
@@ -423,7 +423,7 @@ test_expect_success '$GIT_WORK_TREE overrides $GIT_DIR/common' '
)
'
-test_expect_failure 'error out gracefully on invalid $GIT_WORK_TREE' '
+test_expect_success 'error out gracefully on invalid $GIT_WORK_TREE' '
(
GIT_WORK_TREE=/.invalid/work/tree &&
export GIT_WORK_TREE &&
diff --git a/worktree.c b/worktree.c
index d633761575b..0486e31ad4a 100644
--- a/worktree.c
+++ b/worktree.c
@@ -255,7 +255,7 @@ struct worktree *find_worktree(struct worktree **list,
return wt;
arg = prefix_filename(prefix, strlen(prefix), arg);
- path = real_pathdup(arg);
+ path = real_pathdup(arg, 1);
for (; *list; list++)
if (!fspathcmp(path, real_path((*list)->path)))
break;
--
2.12.0.windows.1.7.g94dafc3b124
^ permalink raw reply related
* [PATCH 0/2] Fix crashes due to real_pathdup() potentially returning NULL
From: Johannes Schindelin @ 2017-03-08 15:43 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Brandon Williams, Stefan Beller
This problem was identified as the root cause for what seemed to be a
path conversion problem in the MSYS2 runtime at first glance.
Original bug report:
http://public-inbox.org/git/CAFKRc7y_kpCGNORENUZ2qw_4qBwjjyaaDFxAEQa52fTryj+w7A@mail.gmail.com/
We may want to consider fast-tracking this into v2.12.1, and to that
end, I would appreciate code reviews that focus on the correctness of
this patch and that try to consider undesired side effects.
Johannes Schindelin (2):
Demonstrate NULL pointer access with invalid GIT_WORK_TREE
Fix callsites of real_pathdup() that wanted it to die on error
abspath.c | 4 ++--
builtin/init-db.c | 6 +++---
cache.h | 2 +-
dir.c | 4 ++--
environment.c | 2 +-
setup.c | 4 ++--
submodule.c | 10 +++++-----
t/t1501-work-tree.sh | 8 ++++++++
worktree.c | 2 +-
9 files changed, 25 insertions(+), 17 deletions(-)
base-commit: e0688e9b28f2c5ff711460ee8b62077be5df2360
Published-As: https://github.com/dscho/git/releases/tag/real_pathdup-callers-v1
Fetch-It-Via: git fetch https://github.com/dscho/git real_pathdup-callers-v1
--
2.12.0.windows.1.7.g94dafc3b124
^ permalink raw reply
* Re: RFC: Another proposed hash function transition plan
From: Johannes Schindelin @ 2017-03-08 15:40 UTC (permalink / raw)
To: Ian Jackson
Cc: Linus Torvalds, Jonathan Nieder, Git Mailing List, Stefan Beller,
bmwill, Jonathan Tan, Jeff King
In-Reply-To: <22719.59633.269164.986923@chiark.greenend.org.uk>
Hi Ian,
On Wed, 8 Mar 2017, Ian Jackson wrote:
> Linus Torvalds writes ("Re: RFC: Another proposed hash function transition plan"):
> > 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.
>
> AFAIAA object names occur in publicly-visible filenames only in notes
> tree objects, which are manipulated by git internally and do not
> necessarily need to appear in the filesystem.
>
> The filenames in .git/objects/ can be in whatever encoding we like, so
> are not an obstacle.
Given that the idea was to encode the new hash in base64 or base85, we
*are* talking about an encoding. In that respect, yes, it can be whatever
encoding we like, and Linus just made a good point (with unnecessary foul
language) of explaining why base64/base85 is not that encoding.
Ciao,
Johannes
^ 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