* Re: [BUG] git help -a should no use page when in pipe
From: Junio C Hamano @ 2018-12-11 8:52 UTC (permalink / raw)
To: 许铖; +Cc: git
In-Reply-To: <CALmyJ=OaZAgr1i1DXMRQqCVWnKvO=QZH6AeGj8YfKMxAnRz6GA@mail.gmail.com>
许铖 <thexuc@gmail.com> writes:
> Since git 2.20.0, invoking `git help -a` will cause git using page (e.g. less)
> to display help message.
> ...
> I would suggest `git help -a` only invokes page when stdout is a tty.
It already does so for me. "GIT_PAGER=uc git help -a | cat" where
"uc" is a custom "pager" that is "tr '[a-z]' '[A-Z]'" does not show
things in ucase, but without piping to cat, the output is in ucase
as expected. Perhaps there is something different between what we
are doing.
FWIW, I am not on Windows or on a Mac.
^ permalink raw reply
* Re: [PATCH v2 0/2] support for filtering trees and blobs based on depth
From: Junio C Hamano @ 2018-12-11 8:45 UTC (permalink / raw)
To: Matthew DeVore
Cc: git, sbeller, git, jeffhost, peff, stefanbeller, jonathantanmy,
pclouds
In-Reply-To: <20181210234030.176178-1-matvore@google.com>
Matthew DeVore <matvore@google.com> writes:
> This is a follow-up to the original patchset at
> https://public-inbox.org/git/cover.1539298957.git.matvore@google.com/ -
> the purpose of that patchset and this one is to support positive integers for
> tree:<depth>. Some of the prior patchset's patches were either already queued
> (tree traversal optimization) or abandoned (documentation edit). This rendition
> of the patchset adds a commit which optimizes away tree traversal when
> collecting omits when iterating over a tree a second time.
>
> Thanks,
>
> Matthew DeVore (2):
> list-objects-filter: teach tree:# how to handle >0
> tree:<depth>: skip some trees even when collecting omits
This seems to require at least two topics still not in 'master';
I've bookmarked the topic by merging sb/more-repo-in-api and
nd/the-index into 'master' and then queueing these two patches on
top, to be able to merge it into 'pu' to see if there are bad
interactions with other topics and also give others easier access to
the topic in the integrated form.
Thanks.
^ permalink raw reply
* (no subject)
From: Łukasz Opasiak @ 2018-12-11 6:49 UTC (permalink / raw)
To: git
^ permalink raw reply
* Re: [PATCH] http: add http.version option to select http protocol version
From: S. Fricke @ 2018-12-11 6:56 UTC (permalink / raw)
To: Eric Sunshine; +Cc: Git List
In-Reply-To: <CAPig+cQW5_9fH-P8X50Mx5kGJRwEOskw2L49Ajk+3D4xWpcOHg@mail.gmail.com>
Hi Eric,
> On Mon, Dec 10, 2018 at 5:50 PM Silvio Fricke <silvio.fricke@gmail.com> wrote:
> > HTTP has several protocol versions. By default, libcurl is using HTTP/2
[...]
> This looks very similar to [1] which is already in Junio's "next"
> branch (although not yet in a released version of Git).
>
> [1]: https://public-inbox.org/git/71f8b71b346f132d0dc9a23c9a7f2ca2cb91966f.1541735051.git.gitgitgadget@gmail.com/
Thanks for the pointer. Looks like I need todo more search than to develop
solutions. ^^
The 8th patch version from charlieio@outlook.com looks better than my
implementation.
Thanks and bye,
Silvio
^ permalink raw reply
* [BUG] git help -a should no use page when in pipe
From: 许铖 @ 2018-12-11 6:26 UTC (permalink / raw)
To: git
To whom may concern,
Since git 2.20.0, invoking `git help -a` will cause git using page (e.g. less)
to display help message. However, such behaviour is not desirable when
`git help -a` is run inside a pipe or invoked by another program.
Step to reproduce the bug:
git help -a | cat # this freezes the command line prompt.
This bug also causes issues in `hub` when invoking `git <alias>`.
More detail can be found at https://github.com/github/hub/issues/1963
I would suggest `git help -a` only invokes page when stdout is a tty.
Thanks for your attention.
Best Regards,
Cheng
^ permalink raw reply
* Re: What's cooking in git.git (Dec 2018, #01; Sun, 9)
From: Junio C Hamano @ 2018-12-11 6:17 UTC (permalink / raw)
To: Stefan Beller; +Cc: git
In-Reply-To: <CAGZ79kZrYP05=eSx4=09Y9Nx9pNMyyKz=tGXjueuhVgJo=Z5bQ@mail.gmail.com>
Stefan Beller <sbeller@google.com> writes:
> I saw you picked up the latest iteration of the last patch at
> https://public-inbox.org/git/20181206212655.145586-1-sbeller@google.com/
> which has received no review comments, yet, and you seem to have
> just taken it for replacement without looking closely.
What's there in 'pu' does not even count as getting "picked up".
Near the tip of 'pu' outside 'next' is merely serving as a bookmark
so that I do not have to hunt around in the list archive after
people comment on the patches.
> I think it is ready, but I seem to be an optimist at times
> when it comes to my own code. :-)
Oh, who isn't?
^ permalink raw reply
* Re: [PATCH 6/8] checkout: add --cached option
From: Elijah Newren @ 2018-12-11 6:12 UTC (permalink / raw)
To: Junio C Hamano
Cc: Nguyễn Thái Ngọc, Thomas Gummerer,
Git Mailing List
In-Reply-To: <xmqqva40lps2.fsf@gitster-ct.c.googlers.com>
On Mon, Dec 10, 2018 at 7:13 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Duy Nguyen <pclouds@gmail.com> writes:
>
> > Elijah wanted another mode (and I agree) that modifies worktree but
> > leaves the index alone. This is most useful (or least confusing) when
> > used with <tree-ish> and would be default in restore-files. I'm not
> > saying you have to implement it, but how do the new command line
> > options are designed to make sense?
>
> I'd model it after "git apply", i.e.
>
> git restore-files [--tree=<treeish>] <pathspec>
>
> would work only on the working tree files,
>
> git restore-files --tree=<treeish> --cached <pathspec>
>
> would match the entries in the index that match pathspec to the
> given treeish without touching the working tree, and
>
> git restore-files --tree=<treeish> --index <pathspec>
>
> would be both.
>
> I have never been happy with the phraso, the (arbitrary) distinction
> between --cached/--index we use, so in the very longer term (like,
> introducing synonym at 3.0 boundary and deprecating older ones at
> 4.0 boundary), it may not be a bad idea to rename "--index" to
> "--index-and-working-tree" and "--cached" to "--index-only".
>
> But until that happens, it would be better to use these two
> consistently.
I agree that consistency is important, but trying to distinguish
between "--cached" and "--index" is _extremely_ painful. I still
can't keep the distinction straight and have to look it up every time
I want to use either. I don't know how to possibly teach users the
meaning. Could we introduce synonyms earlier at least, and make the
synonyms more prominent than the "--cached" and "--index" terms in the
documentation?
^ permalink raw reply
* Re: [PATCH v3 1/1] git clone <url> C:\cygwin\home\USER\repo' is working (again)
From: Torsten Bögershausen @ 2018-12-11 6:12 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, svnpenn
In-Reply-To: <nycvar.QRO.7.76.6.1812100931390.43@tvgsbejvaqbjf.bet>
On Mon, Dec 10, 2018 at 09:32:03AM +0100, Johannes Schindelin wrote:
> Hi Torsten,
>
> On Sat, 8 Dec 2018, tboegi@web.de wrote:
>
> > And, before any cleanup is done, I sould like to ask if anybody
> > can build the code with VS and confirm that it works, please ?
>
> Can you give me an easy-to-fetch branch?
>
> Thanks,
> Dscho
@Dscho: The branch should be here:
https://github.com/tboegi/git/tree/tb.181208_0844_cygwin-dos-drive
(or fetch it from Junio, please see below:)
@Junio:
Please keep tb/use-common-win32-pathfuncs-on-cygwin
in pu for a while. I need to send a V4 to fix t5601 for cygwin.
^ permalink raw reply
* Re: [PATCH 5/8] checkout: introduce --{,no-}overlay option
From: Elijah Newren @ 2018-12-11 6:04 UTC (permalink / raw)
To: Junio C Hamano
Cc: Thomas Gummerer, Git Mailing List,
Nguyễn Thái Ngọc
In-Reply-To: <xmqqzhtclq22.fsf@gitster-ct.c.googlers.com>
On Mon, Dec 10, 2018 at 7:07 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Elijah Newren <newren@gmail.com> writes:
>
> >> Note that 'git checkout -p <tree-ish> -- [<pathspec>]' already works
> >> this way, so no changes are needed for the patch mode. We disallow
> >> 'git checkout --overlay -p' to avoid confusing users who would expect
> >> to be able to force overlay mode in 'git checkout -p' this way.
> >
> > Whoa...that's interesting. To me, that argues even further that the
> > traditional checkout behavior was wrong all along and the choice of
> > --overlay vs. --no-overlay in the original implementation was a total
> > oversight. I'm really tempted to say that --no-overlay should just be
> > the default in checkout too...but maybe that's too high a hill to
> > climb, at least for now.
>
> You are exhibiting a typical reaction to a shiny new toy.
>
> The original "checkout paths out of the trees and/or the index to
> recover what was lost" is modeled after "cp -R .../else/where .",
> which is an overlay operation, and you wouldn't imagine complaining
> that "cp -R" is wrong not to remove things in the target, to be
> equivalent to "rm -fr where && cp -R .../else/where .".
>
> Each has its own uses. We just didn't have the other half of the
> pair.
Ah, modelled on cp -R. I think that rather than reacting "to a shiny
new toy", I just had always had a different mental model AND failed to
figure out what the original model was, leading me to always view it
as buggy. Thanks for giving me the model I was missing.
> If anything, I would consider "checkout -p" that does not do overlay
> is a bug that needs to be fixed.
Yeah, --no-overlay being the default for -p, and --overlay being the
default otherwise is rather inconsistent. (Though I'm also fine with
that being fixed by some future patch series.)
^ permalink raw reply
* [PATCH] run-command: report exec failure
From: Junio C Hamano @ 2018-12-11 5:46 UTC (permalink / raw)
To: git; +Cc: Jeff King, John Passaro
In 321fd823 ("run-command: mark path lookup errors with ENOENT",
2018-10-24), we rewrote the logic to execute a command by looking
in the directories on $PATH; as a side effect, a request to run a
command that is not found on $PATH is noticed even before a child
process is forked to execute it.
We however stopped to report an exec failure in such a case by
mistake. Add a logic to report the error unless silent-exec-failure
is requested, to match the original code.
Reported-by: John Passaro <john.a.passaro@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* Strictly speaking, the failure that is diagnosed by the spawned
child is reported with die() and prefixed with "failure:"; I am
adding error_errno(), so this will be reported with "error:"
prefix, which is a slight change in behaviour, but I am guessing
that this should be OK.
run-command.c | 2 ++
t/t0061-run-command.sh | 9 ++++++---
2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/run-command.c b/run-command.c
index d679cc267c..e2bc18a083 100644
--- a/run-command.c
+++ b/run-command.c
@@ -728,6 +728,8 @@ int start_command(struct child_process *cmd)
if (prepare_cmd(&argv, cmd) < 0) {
failed_errno = errno;
cmd->pid = -1;
+ if (!cmd->silent_exec_failure)
+ error_errno("cannot run %s", cmd->argv[0]);
goto end_of_spawn;
}
diff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh
index cf932c8514..9c83d44d9c 100755
--- a/t/t0061-run-command.sh
+++ b/t/t0061-run-command.sh
@@ -13,11 +13,13 @@ cat >hello-script <<-EOF
EOF
test_expect_success 'start_command reports ENOENT (slash)' '
- test-tool run-command start-command-ENOENT ./does-not-exist
+ test-tool run-command start-command-ENOENT ./does-not-exist 2>err &&
+ test_i18ngrep "cannot run" err
'
test_expect_success 'start_command reports ENOENT (no slash)' '
- test-tool run-command start-command-ENOENT does-not-exist
+ test-tool run-command start-command-ENOENT does-not-exist 2>err &&
+ test_i18ngrep "cannot run" err
'
test_expect_success 'run_command can run a command' '
@@ -33,7 +35,8 @@ test_expect_success 'run_command is restricted to PATH' '
write_script should-not-run <<-\EOF &&
echo yikes
EOF
- test_must_fail test-tool run-command run-command should-not-run
+ test_must_fail test-tool run-command run-command should-not-run 2>err &&
+ test_i18ngrep "cannot run" err
'
test_expect_success !MINGW 'run_command can run a script without a #! line' '
--
2.20.0
^ permalink raw reply related
* [PATCH v1 6/8] Use promisors_get_direct() and has_promisor_remote()
From: Christian Couder @ 2018-12-11 5:27 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Ben Peart, Jonathan Tan,
Jonathan Nieder, Stefan Beller, Nguyen Thai Ngoc Duy, Mike Hommey,
Lars Schneider, Eric Wong, Christian Couder, Jeff Hostetler,
Eric Sunshine, Beat Bolli
In-Reply-To: <20181211052746.16218-1-chriscool@tuxfamily.org>
Instead of using the repository_format_partial_clone global
and fetch_objects() directly, let's use has_promisor_remote()
and promisors_get_direct().
This way all the configured promisor remotes will be taken
into account, not only the one specified by
extensions.partialClone.
Also when cloning or fetching using a partial clone filter,
remote.origin.promisor will be set to "true" instead of
setting extensions.partialClone to "origin". This makes it
possible to use many promisor remote just by fetching from
them.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin/cat-file.c | 5 +++--
builtin/fetch.c | 11 ++++++-----
builtin/gc.c | 3 ++-
builtin/repack.c | 3 ++-
cache-tree.c | 3 ++-
connected.c | 3 ++-
list-objects-filter-options.c | 28 +++++++++++++++-------------
packfile.c | 3 ++-
sha1-file.c | 14 ++++++++------
t/t5601-clone.sh | 2 +-
t/t5616-partial-clone.sh | 2 +-
unpack-trees.c | 6 +++---
12 files changed, 47 insertions(+), 36 deletions(-)
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 2ca56fd086..bdb0331ba4 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -14,6 +14,7 @@
#include "sha1-array.h"
#include "packfile.h"
#include "object-store.h"
+#include "promisor-remote.h"
struct batch_options {
int enabled;
@@ -517,8 +518,8 @@ static int batch_objects(struct batch_options *opt)
if (opt->all_objects) {
struct object_cb_data cb;
- if (repository_format_partial_clone)
- warning("This repository has extensions.partialClone set. Some objects may not be loaded.");
+ if (has_promisor_remote())
+ warning("This repository uses promisor remotes. Some objects may not be loaded.");
cb.opt = opt;
cb.expand = &data;
diff --git a/builtin/fetch.c b/builtin/fetch.c
index e0140327aa..647a60b26c 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -23,6 +23,7 @@
#include "packfile.h"
#include "list-objects-filter-options.h"
#include "commit-reach.h"
+#include "promisor-remote.h"
static const char * const builtin_fetch_usage[] = {
N_("git fetch [<options>] [<repository> [<refspec>...]]"),
@@ -1459,7 +1460,7 @@ static inline void fetch_one_setup_partial(struct remote *remote)
* If no prior partial clone/fetch and the current fetch DID NOT
* request a partial-fetch, do a normal fetch.
*/
- if (!repository_format_partial_clone && !filter_options.choice)
+ if (!has_promisor_remote() && !filter_options.choice)
return;
/*
@@ -1467,7 +1468,7 @@ static inline void fetch_one_setup_partial(struct remote *remote)
* on this repo and remember the given filter-spec as the default
* for subsequent fetches to this remote.
*/
- if (!repository_format_partial_clone && filter_options.choice) {
+ if (!has_promisor_remote() && filter_options.choice) {
partial_clone_register(remote->name, &filter_options);
return;
}
@@ -1476,7 +1477,7 @@ static inline void fetch_one_setup_partial(struct remote *remote)
* We are currently limited to only ONE promisor remote and only
* allow partial-fetches from the promisor remote.
*/
- if (strcmp(remote->name, repository_format_partial_clone)) {
+ if (!find_promisor_remote(remote->name)) {
if (filter_options.choice)
die(_("--filter can only be used with the remote configured in core.partialClone"));
return;
@@ -1607,7 +1608,7 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
if (depth || deepen_since || deepen_not.nr)
deepen = 1;
- if (filter_options.choice && !repository_format_partial_clone)
+ if (filter_options.choice && !has_promisor_remote())
die("--filter can only be used when extensions.partialClone is set");
if (all) {
@@ -1641,7 +1642,7 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
}
if (remote) {
- if (filter_options.choice || repository_format_partial_clone)
+ if (filter_options.choice || has_promisor_remote())
fetch_one_setup_partial(remote);
result = fetch_one(remote, argc, argv, prune_tags_ok);
} else {
diff --git a/builtin/gc.c b/builtin/gc.c
index 871a56f1c5..42dfa3a23c 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -27,6 +27,7 @@
#include "pack-objects.h"
#include "blob.h"
#include "tree.h"
+#include "promisor-remote.h"
#define FAILED_RUN "failed to run %s"
@@ -640,7 +641,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
argv_array_push(&prune, prune_expire);
if (quiet)
argv_array_push(&prune, "--no-progress");
- if (repository_format_partial_clone)
+ if (has_promisor_remote())
argv_array_push(&prune,
"--exclude-promisor-objects");
if (run_command_v_opt(prune.argv, RUN_GIT_CMD))
diff --git a/builtin/repack.c b/builtin/repack.c
index 45583683ee..26e02e3135 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -11,6 +11,7 @@
#include "midx.h"
#include "packfile.h"
#include "object-store.h"
+#include "promisor-remote.h"
static int delta_base_offset = 1;
static int pack_kept_objects = -1;
@@ -366,7 +367,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
argv_array_push(&cmd.args, "--all");
argv_array_push(&cmd.args, "--reflog");
argv_array_push(&cmd.args, "--indexed-objects");
- if (repository_format_partial_clone)
+ if (has_promisor_remote())
argv_array_push(&cmd.args, "--exclude-promisor-objects");
if (write_bitmaps)
argv_array_push(&cmd.args, "--write-bitmap-index");
diff --git a/cache-tree.c b/cache-tree.c
index 9d454d24bc..a1ecb2a1fa 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -5,6 +5,7 @@
#include "cache-tree.h"
#include "object-store.h"
#include "replace-object.h"
+#include "promisor-remote.h"
#ifndef DEBUG
#define DEBUG 0
@@ -357,7 +358,7 @@ static int update_one(struct cache_tree *it,
}
ce_missing_ok = mode == S_IFGITLINK || missing_ok ||
- (repository_format_partial_clone &&
+ (has_promisor_remote() &&
ce_skip_worktree(ce));
if (is_null_oid(oid) ||
(!ce_missing_ok && !has_object_file(oid))) {
diff --git a/connected.c b/connected.c
index 1bba888eff..0eaaedee6a 100644
--- a/connected.c
+++ b/connected.c
@@ -4,6 +4,7 @@
#include "connected.h"
#include "transport.h"
#include "packfile.h"
+#include "promisor-remote.h"
/*
* If we feed all the commits we want to verify to this command
@@ -56,7 +57,7 @@ int check_connected(oid_iterate_fn fn, void *cb_data,
argv_array_push(&rev_list.args,"rev-list");
argv_array_push(&rev_list.args, "--objects");
argv_array_push(&rev_list.args, "--stdin");
- if (repository_format_partial_clone)
+ if (has_promisor_remote())
argv_array_push(&rev_list.args, "--exclude-promisor-objects");
if (!opt->is_deepening_fetch) {
argv_array_push(&rev_list.args, "--not");
diff --git a/list-objects-filter-options.c b/list-objects-filter-options.c
index e8da2e8581..3b5ff55480 100644
--- a/list-objects-filter-options.c
+++ b/list-objects-filter-options.c
@@ -6,6 +6,7 @@
#include "list-objects.h"
#include "list-objects-filter.h"
#include "list-objects-filter-options.h"
+#include "promisor-remote.h"
/*
* Parse value of the argument to the "filter" keyword.
@@ -125,30 +126,31 @@ void partial_clone_register(
const char *remote,
const struct list_objects_filter_options *filter_options)
{
- /*
- * Record the name of the partial clone remote in the
- * config and in the global variable -- the latter is
- * used throughout to indicate that partial clone is
- * enabled and to expect missing objects.
- */
- if (repository_format_partial_clone &&
- *repository_format_partial_clone &&
- strcmp(remote, repository_format_partial_clone))
- die(_("cannot change partial clone promisor remote"));
+ char *cfg_name;
- git_config_set("core.repositoryformatversion", "1");
- git_config_set("extensions.partialclone", remote);
+ /* Check if it is already registered */
+ if (!find_promisor_remote(remote)) {
+ git_config_set("core.repositoryformatversion", "1");
- repository_format_partial_clone = xstrdup(remote);
+ /* Add promisor config for the remote */
+ cfg_name = xstrfmt("remote.%s.promisor", remote);
+ git_config_set(cfg_name, "true");
+ free(cfg_name);
+ }
/*
* Record the initial filter-spec in the config as
* the default for subsequent fetches from this remote.
+ *
+ * TODO: record it into remote.<name>.partialclonefilter
*/
core_partial_clone_filter_default =
xstrdup(filter_options->filter_spec);
git_config_set("core.partialclonefilter",
core_partial_clone_filter_default);
+
+ /* Make sure the config info are reset */
+ promisor_remote_reinit();
}
void partial_clone_get_default_filter_spec(
diff --git a/packfile.c b/packfile.c
index d1e6683ffe..7aaeea7693 100644
--- a/packfile.c
+++ b/packfile.c
@@ -16,6 +16,7 @@
#include "tree.h"
#include "object-store.h"
#include "midx.h"
+#include "promisor-remote.h"
char *odb_pack_name(struct strbuf *buf,
const unsigned char *sha1,
@@ -2113,7 +2114,7 @@ int is_promisor_object(const struct object_id *oid)
static int promisor_objects_prepared;
if (!promisor_objects_prepared) {
- if (repository_format_partial_clone) {
+ if (has_promisor_remote()) {
for_each_packed_object(add_promisor_object,
&promisor_objects,
FOR_EACH_OBJECT_PROMISOR_ONLY);
diff --git a/sha1-file.c b/sha1-file.c
index cf88eeb147..f1acaa3254 100644
--- a/sha1-file.c
+++ b/sha1-file.c
@@ -32,6 +32,7 @@
#include "packfile.h"
#include "fetch-object.h"
#include "object-store.h"
+#include "promisor-remote.h"
/* The maximum size for an object header. */
#define MAX_HEADER_LEN 32
@@ -1309,15 +1310,16 @@ int oid_object_info_extended(struct repository *r, const struct object_id *oid,
}
/* Check if it is a missing object */
- if (fetch_if_missing && repository_format_partial_clone &&
+ if (fetch_if_missing && has_promisor_remote() &&
!already_retried && r == the_repository) {
/*
- * TODO Investigate checking fetch_object() return
- * TODO value and stopping on error here.
- * TODO Pass a repository struct through fetch_object,
- * such that arbitrary repositories work.
+ * TODO Investigate checking promisors_get_direct()
+ * TODO return value and stopping on error here.
+ * TODO Pass a repository struct through
+ * promisors_get_direct(), such that arbitrary
+ * repositories work.
*/
- fetch_objects(repository_format_partial_clone, real, 1);
+ promisors_get_direct(real, 1);
already_retried = 1;
continue;
}
diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index 8bbc7068ac..aafe55b8b7 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -653,7 +653,7 @@ partial_clone () {
git -C client fsck &&
# Ensure that unneeded blobs are not inadvertently fetched.
- test_config -C client extensions.partialclone "not a remote" &&
+ test_config -C client remote.origin.promisor "false" &&
test_must_fail git -C client cat-file -e "$HASH1" &&
# But this blob was fetched, because clone performs an initial checkout
diff --git a/t/t5616-partial-clone.sh b/t/t5616-partial-clone.sh
index 336f02a41a..d09dee1f14 100755
--- a/t/t5616-partial-clone.sh
+++ b/t/t5616-partial-clone.sh
@@ -42,7 +42,7 @@ test_expect_success 'do partial clone 1' '
test_cmp expect_1.oids observed.oids &&
test "$(git -C pc1 config --local core.repositoryformatversion)" = "1" &&
- test "$(git -C pc1 config --local extensions.partialclone)" = "origin" &&
+ test "$(git -C pc1 config --local remote.origin.promisor)" = "true" &&
test "$(git -C pc1 config --local core.partialclonefilter)" = "blob:none"
'
diff --git a/unpack-trees.c b/unpack-trees.c
index 7570df481b..9ad157eeab 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -18,6 +18,7 @@
#include "fsmonitor.h"
#include "object-store.h"
#include "fetch-object.h"
+#include "promisor-remote.h"
/*
* Error messages expected by scripts out of plumbing commands such as
@@ -418,7 +419,7 @@ static int check_updates(struct unpack_trees_options *o)
load_gitmodules_file(index, &state);
enable_delayed_checkout(&state);
- if (repository_format_partial_clone && o->update && !o->dry_run) {
+ if (has_promisor_remote() && o->update && !o->dry_run) {
/*
* Prefetch the objects that are to be checked out in the loop
* below.
@@ -435,8 +436,7 @@ static int check_updates(struct unpack_trees_options *o)
}
}
if (to_fetch.nr)
- fetch_objects(repository_format_partial_clone,
- to_fetch.oid, to_fetch.nr);
+ promisors_get_direct(to_fetch.oid, to_fetch.nr);
fetch_if_missing = fetch_if_missing_store;
oid_array_clear(&to_fetch);
}
--
2.20.0.rc2.14.g1379de12fa.dirty
^ permalink raw reply related
* [PATCH v1 8/8] t0410: test fetching from many promisor remotes
From: Christian Couder @ 2018-12-11 5:27 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Ben Peart, Jonathan Tan,
Jonathan Nieder, Stefan Beller, Nguyen Thai Ngoc Duy, Mike Hommey,
Lars Schneider, Eric Wong, Christian Couder, Jeff Hostetler,
Eric Sunshine, Beat Bolli, Christian Couder
In-Reply-To: <20181211052746.16218-1-chriscool@tuxfamily.org>
From: Christian Couder <christian.couder@gmail.com>
This shows that it is now possible to fetch objects from more
than one promisor remote.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
t/t0410-partial-clone.sh | 24 +++++++++++++++++++++++-
1 file changed, 23 insertions(+), 1 deletion(-)
diff --git a/t/t0410-partial-clone.sh b/t/t0410-partial-clone.sh
index 462229e445..a09a53ab0d 100755
--- a/t/t0410-partial-clone.sh
+++ b/t/t0410-partial-clone.sh
@@ -182,8 +182,30 @@ test_expect_success 'fetching of missing objects works with ref-in-want enabled'
grep "git< fetch=.*ref-in-want" trace
'
+test_expect_success 'fetching of missing objects from another promisor remote' '
+ git clone "file://$(pwd)/server" server2 &&
+ test_commit -C server2 bar &&
+ git -C server2 repack -a -d --write-bitmap-index &&
+ HASH2=$(git -C server2 rev-parse bar) &&
+
+ git -C repo remote add server2 "file://$(pwd)/server2" &&
+ git -C repo config remote.server2.promisor true &&
+ git -C repo cat-file -p "$HASH2" &&
+
+ git -C repo fetch server2 &&
+ rm -rf repo/.git/objects/* &&
+ git -C repo cat-file -p "$HASH2" &&
+
+ # Ensure that the .promisor file is written, and check that its
+ # associated packfile contains the object
+ ls repo/.git/objects/pack/pack-*.promisor >promisorlist &&
+ test_line_count = 1 promisorlist &&
+ IDX=$(cat promisorlist | sed "s/promisor$/idx/") &&
+ git verify-pack --verbose "$IDX" | grep "$HASH2"
+'
+
test_expect_success 'fetching of missing blobs works' '
- rm -rf server repo &&
+ rm -rf server server2 repo &&
test_create_repo server &&
test_commit -C server foo &&
git -C server repack -a -d --write-bitmap-index &&
--
2.20.0.rc2.14.g1379de12fa.dirty
^ permalink raw reply related
* [PATCH v1 7/8] promisor-remote: parse remote.*.partialclonefilter
From: Christian Couder @ 2018-12-11 5:27 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Ben Peart, Jonathan Tan,
Jonathan Nieder, Stefan Beller, Nguyen Thai Ngoc Duy, Mike Hommey,
Lars Schneider, Eric Wong, Christian Couder, Jeff Hostetler,
Eric Sunshine, Beat Bolli
In-Reply-To: <20181211052746.16218-1-chriscool@tuxfamily.org>
This makes it possible to specify a different partial clone
filter for each promisor remote.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin/fetch.c | 2 +-
list-objects-filter-options.c | 27 +++++++++++++++------------
list-objects-filter-options.h | 3 ++-
promisor-remote.c | 13 +++++++++++--
promisor-remote.h | 5 ++++-
t/t0410-partial-clone.sh | 2 +-
t/t5601-clone.sh | 1 +
t/t5616-partial-clone.sh | 2 +-
8 files changed, 36 insertions(+), 19 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 647a60b26c..53494c5864 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1489,7 +1489,7 @@ static inline void fetch_one_setup_partial(struct remote *remote)
* the config.
*/
if (!filter_options.choice)
- partial_clone_get_default_filter_spec(&filter_options);
+ partial_clone_get_default_filter_spec(&filter_options, remote->name);
return;
}
diff --git a/list-objects-filter-options.c b/list-objects-filter-options.c
index 3b5ff55480..ea0148f709 100644
--- a/list-objects-filter-options.c
+++ b/list-objects-filter-options.c
@@ -29,6 +29,9 @@ static int gently_parse_list_objects_filter(
{
const char *v0;
+ if (!arg)
+ return 0;
+
if (filter_options->choice) {
if (errbuf) {
strbuf_addstr(
@@ -127,6 +130,7 @@ void partial_clone_register(
const struct list_objects_filter_options *filter_options)
{
char *cfg_name;
+ char *filter_name;
/* Check if it is already registered */
if (!find_promisor_remote(remote)) {
@@ -141,27 +145,26 @@ void partial_clone_register(
/*
* Record the initial filter-spec in the config as
* the default for subsequent fetches from this remote.
- *
- * TODO: record it into remote.<name>.partialclonefilter
*/
- core_partial_clone_filter_default =
- xstrdup(filter_options->filter_spec);
- git_config_set("core.partialclonefilter",
- core_partial_clone_filter_default);
+ filter_name = xstrfmt("remote.%s.partialclonefilter", remote);
+ git_config_set(filter_name, filter_options->filter_spec);
+ free(filter_name);
/* Make sure the config info are reset */
promisor_remote_reinit();
}
void partial_clone_get_default_filter_spec(
- struct list_objects_filter_options *filter_options)
+ struct list_objects_filter_options *filter_options,
+ const char *remote)
{
+ struct promisor_remote *promisor = find_promisor_remote(remote);
+
/*
* Parse default value, but silently ignore it if it is invalid.
*/
- if (!core_partial_clone_filter_default)
- return;
- gently_parse_list_objects_filter(filter_options,
- core_partial_clone_filter_default,
- NULL);
+ if (promisor)
+ gently_parse_list_objects_filter(filter_options,
+ promisor->partial_clone_filter,
+ NULL);
}
diff --git a/list-objects-filter-options.h b/list-objects-filter-options.h
index af64e5c66f..8c10ec2d3a 100644
--- a/list-objects-filter-options.h
+++ b/list-objects-filter-options.h
@@ -75,6 +75,7 @@ void partial_clone_register(
const char *remote,
const struct list_objects_filter_options *filter_options);
void partial_clone_get_default_filter_spec(
- struct list_objects_filter_options *filter_options);
+ struct list_objects_filter_options *filter_options,
+ const char *remote);
#endif /* LIST_OBJECTS_FILTER_OPTIONS_H */
diff --git a/promisor-remote.c b/promisor-remote.c
index e4a0625426..d1bd9fbf49 100644
--- a/promisor-remote.c
+++ b/promisor-remote.c
@@ -60,6 +60,16 @@ static int promisor_remote_config(const char *var, const char *value, void *data
free(remote_name);
return 0;
}
+ if (!strcmp(subkey, "partialclonefilter")) {
+ char *remote_name = xmemdupz(name, namelen);
+
+ o = do_find_promisor_remote(remote_name);
+ if (!o)
+ o = promisor_remote_new(remote_name);
+
+ free(remote_name);
+ return git_config_string(&o->partial_clone_filter, var, value);
+ }
return 0;
}
@@ -76,8 +86,7 @@ static void promisor_remote_do_init(int force)
if (repository_format_partial_clone &&
!do_find_promisor_remote(repository_format_partial_clone))
- promisor_remote_new(repository_format_partial_clone,
- strlen(repository_format_partial_clone));
+ promisor_remote_new(repository_format_partial_clone);
}
static inline void promisor_remote_init(void)
diff --git a/promisor-remote.h b/promisor-remote.h
index 873ddfb5ed..431edafcdd 100644
--- a/promisor-remote.h
+++ b/promisor-remote.h
@@ -3,10 +3,13 @@
/*
* Promisor remote linked list
- * Its information come from remote.XXX config entries.
+ *
+ * Information in its fields come from remote.XXX config entries or
+ * from extensions.partialclone or core.partialclonefilter.
*/
struct promisor_remote {
const char *remote_name;
+ const char *partial_clone_filter;
struct promisor_remote *next;
};
diff --git a/t/t0410-partial-clone.sh b/t/t0410-partial-clone.sh
index ba3887f178..462229e445 100755
--- a/t/t0410-partial-clone.sh
+++ b/t/t0410-partial-clone.sh
@@ -26,7 +26,7 @@ promise_and_delete () {
test_expect_success 'extensions.partialclone without filter' '
test_create_repo server &&
git clone --filter="blob:none" "file://$(pwd)/server" client &&
- git -C client config --unset core.partialclonefilter &&
+ git -C client config --unset remote.origin.partialclonefilter &&
git -C client fetch origin
'
diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index aafe55b8b7..72a04669e4 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -654,6 +654,7 @@ partial_clone () {
# Ensure that unneeded blobs are not inadvertently fetched.
test_config -C client remote.origin.promisor "false" &&
+ git -C client config --unset remote.origin.partialclonefilter &&
test_must_fail git -C client cat-file -e "$HASH1" &&
# But this blob was fetched, because clone performs an initial checkout
diff --git a/t/t5616-partial-clone.sh b/t/t5616-partial-clone.sh
index d09dee1f14..9057ad25d3 100755
--- a/t/t5616-partial-clone.sh
+++ b/t/t5616-partial-clone.sh
@@ -43,7 +43,7 @@ test_expect_success 'do partial clone 1' '
test_cmp expect_1.oids observed.oids &&
test "$(git -C pc1 config --local core.repositoryformatversion)" = "1" &&
test "$(git -C pc1 config --local remote.origin.promisor)" = "true" &&
- test "$(git -C pc1 config --local core.partialclonefilter)" = "blob:none"
+ test "$(git -C pc1 config --local remote.origin.partialclonefilter)" = "blob:none"
'
# checkout master to force dynamic object fetch of blobs at HEAD.
--
2.20.0.rc2.14.g1379de12fa.dirty
^ permalink raw reply related
* [PATCH v1 2/8] Add initial support for many promisor remotes
From: Christian Couder @ 2018-12-11 5:27 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Ben Peart, Jonathan Tan,
Jonathan Nieder, Stefan Beller, Nguyen Thai Ngoc Duy, Mike Hommey,
Lars Schneider, Eric Wong, Christian Couder, Jeff Hostetler,
Eric Sunshine, Beat Bolli, Christian Couder
In-Reply-To: <20181211052746.16218-1-chriscool@tuxfamily.org>
From: Christian Couder <christian.couder@gmail.com>
The promisor-remote.{c,h} files will contain functions to
manage many promisor remotes.
We expect that there will not be a lot of promisor remotes,
so it is ok to use a simple linked list to manage them.
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
Makefile | 1 +
promisor-remote.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++
promisor-remote.h | 17 +++++++++
3 files changed, 108 insertions(+)
create mode 100644 promisor-remote.c
create mode 100644 promisor-remote.h
diff --git a/Makefile b/Makefile
index 1a44c811aa..565a5fbb4e 100644
--- a/Makefile
+++ b/Makefile
@@ -949,6 +949,7 @@ LIB_OBJS += preload-index.o
LIB_OBJS += pretty.o
LIB_OBJS += prio-queue.o
LIB_OBJS += progress.o
+LIB_OBJS += promisor-remote.o
LIB_OBJS += prompt.o
LIB_OBJS += protocol.o
LIB_OBJS += quote.o
diff --git a/promisor-remote.c b/promisor-remote.c
new file mode 100644
index 0000000000..701f5a351b
--- /dev/null
+++ b/promisor-remote.c
@@ -0,0 +1,90 @@
+#include "cache.h"
+#include "promisor-remote.h"
+#include "config.h"
+
+static struct promisor_remote *promisors;
+static struct promisor_remote **promisors_tail = &promisors;
+
+struct promisor_remote *promisor_remote_new(const char *remote_name)
+{
+ struct promisor_remote *o;
+
+ o = xcalloc(1, sizeof(*o));
+ o->remote_name = xstrdup(remote_name);
+
+ *promisors_tail = o;
+ promisors_tail = &o->next;
+
+ return o;
+}
+
+static struct promisor_remote *do_find_promisor_remote(const char *remote_name)
+{
+ struct promisor_remote *o;
+
+ for (o = promisors; o; o = o->next)
+ if (o->remote_name && !strcmp(o->remote_name, remote_name))
+ return o;
+
+ return NULL;
+}
+
+static int promisor_remote_config(const char *var, const char *value, void *data)
+{
+ struct promisor_remote *o;
+ const char *name;
+ int namelen;
+ const char *subkey;
+
+ if (parse_config_key(var, "remote", &name, &namelen, &subkey) < 0)
+ return 0;
+
+ if (!strcmp(subkey, "promisor")) {
+ char *remote_name;
+
+ if (!git_config_bool(var, value))
+ return 0;
+
+ remote_name = xmemdupz(name, namelen);
+
+ if (do_find_promisor_remote(remote_name)) {
+ free(remote_name);
+ return error(_("when parsing config key '%s' "
+ "promisor remote '%s' already exists"),
+ var, remote_name);
+ }
+
+ promisor_remote_new(remote_name);
+
+ free(remote_name);
+ return 0;
+ }
+
+ return 0;
+}
+
+static void promisor_remote_init(void)
+{
+ static int initialized;
+
+ if (initialized)
+ return;
+ initialized = 1;
+
+ git_config(promisor_remote_config, NULL);
+}
+
+struct promisor_remote *find_promisor_remote(const char *remote_name)
+{
+ promisor_remote_init();
+
+ if (!remote_name)
+ return promisors;
+
+ return do_find_promisor_remote(remote_name);
+}
+
+int has_promisor_remote(void)
+{
+ return !!find_promisor_remote(NULL);
+}
diff --git a/promisor-remote.h b/promisor-remote.h
new file mode 100644
index 0000000000..d07ac07a43
--- /dev/null
+++ b/promisor-remote.h
@@ -0,0 +1,17 @@
+#ifndef PROMISOR_REMOTE_H
+#define PROMISOR_REMOTE_H
+
+/*
+ * Promisor remote linked list
+ * Its information come from remote.XXX config entries.
+ */
+struct promisor_remote {
+ const char *remote_name;
+ struct promisor_remote *next;
+};
+
+extern struct promisor_remote *promisor_remote_new(const char *remote_name);
+extern struct promisor_remote *find_promisor_remote(const char *remote_name);
+extern int has_promisor_remote(void);
+
+#endif /* PROMISOR_REMOTE_H */
--
2.20.0.rc2.14.g1379de12fa.dirty
^ permalink raw reply related
* [PATCH v1 5/8] promisor-remote: use repository_format_partial_clone
From: Christian Couder @ 2018-12-11 5:27 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Ben Peart, Jonathan Tan,
Jonathan Nieder, Stefan Beller, Nguyen Thai Ngoc Duy, Mike Hommey,
Lars Schneider, Eric Wong, Christian Couder, Jeff Hostetler,
Eric Sunshine, Beat Bolli
In-Reply-To: <20181211052746.16218-1-chriscool@tuxfamily.org>
A remote specified using the extensions.partialClone config
option should be considered a promisor remote too.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
promisor-remote.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/promisor-remote.c b/promisor-remote.c
index ca2b8bf6bb..e4a0625426 100644
--- a/promisor-remote.c
+++ b/promisor-remote.c
@@ -73,6 +73,11 @@ static void promisor_remote_do_init(int force)
initialized = 1;
git_config(promisor_remote_config, NULL);
+
+ if (repository_format_partial_clone &&
+ !do_find_promisor_remote(repository_format_partial_clone))
+ promisor_remote_new(repository_format_partial_clone,
+ strlen(repository_format_partial_clone));
}
static inline void promisor_remote_init(void)
--
2.20.0.rc2.14.g1379de12fa.dirty
^ permalink raw reply related
* [PATCH v1 4/8] promisor-remote: add promisor_remote_reinit()
From: Christian Couder @ 2018-12-11 5:27 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Ben Peart, Jonathan Tan,
Jonathan Nieder, Stefan Beller, Nguyen Thai Ngoc Duy, Mike Hommey,
Lars Schneider, Eric Wong, Christian Couder, Jeff Hostetler,
Eric Sunshine, Beat Bolli, Christian Couder
In-Reply-To: <20181211052746.16218-1-chriscool@tuxfamily.org>
From: Christian Couder <christian.couder@gmail.com>
We will need to reinitialize the promisor remote configuration
as we will make some changes to it in a later commit.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
promisor-remote.c | 14 ++++++++++++--
promisor-remote.h | 1 +
2 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/promisor-remote.c b/promisor-remote.c
index e0724bdc20..ca2b8bf6bb 100644
--- a/promisor-remote.c
+++ b/promisor-remote.c
@@ -64,17 +64,27 @@ static int promisor_remote_config(const char *var, const char *value, void *data
return 0;
}
-static void promisor_remote_init(void)
+static void promisor_remote_do_init(int force)
{
static int initialized;
- if (initialized)
+ if (!force && initialized)
return;
initialized = 1;
git_config(promisor_remote_config, NULL);
}
+static inline void promisor_remote_init(void)
+{
+ promisor_remote_do_init(0);
+}
+
+void promisor_remote_reinit(void)
+{
+ promisor_remote_do_init(1);
+}
+
struct promisor_remote *find_promisor_remote(const char *remote_name)
{
promisor_remote_init();
diff --git a/promisor-remote.h b/promisor-remote.h
index 8b89221b33..873ddfb5ed 100644
--- a/promisor-remote.h
+++ b/promisor-remote.h
@@ -10,6 +10,7 @@ struct promisor_remote {
struct promisor_remote *next;
};
+extern void promisor_remote_reinit(void);
extern struct promisor_remote *promisor_remote_new(const char *remote_name);
extern struct promisor_remote *find_promisor_remote(const char *remote_name);
extern int has_promisor_remote(void);
--
2.20.0.rc2.14.g1379de12fa.dirty
^ permalink raw reply related
* [PATCH v1 3/8] promisor-remote: implement promisors_get_direct()
From: Christian Couder @ 2018-12-11 5:27 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Ben Peart, Jonathan Tan,
Jonathan Nieder, Stefan Beller, Nguyen Thai Ngoc Duy, Mike Hommey,
Lars Schneider, Eric Wong, Christian Couder, Jeff Hostetler,
Eric Sunshine, Beat Bolli, Christian Couder
In-Reply-To: <20181211052746.16218-1-chriscool@tuxfamily.org>
From: Christian Couder <christian.couder@gmail.com>
This is implemented for now by calling fetch_objects(). It fetches
from all the promisor remotes.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
promisor-remote.c | 33 +++++++++++++++++++++++++++++++++
promisor-remote.h | 1 +
2 files changed, 34 insertions(+)
diff --git a/promisor-remote.c b/promisor-remote.c
index 701f5a351b..e0724bdc20 100644
--- a/promisor-remote.c
+++ b/promisor-remote.c
@@ -1,6 +1,7 @@
#include "cache.h"
#include "promisor-remote.h"
#include "config.h"
+#include "fetch-object.h"
static struct promisor_remote *promisors;
static struct promisor_remote **promisors_tail = &promisors;
@@ -88,3 +89,35 @@ int has_promisor_remote(void)
{
return !!find_promisor_remote(NULL);
}
+
+static int promisor_remote_get_direct(struct promisor_remote *o,
+ const struct object_id *oids,
+ int oid_nr)
+{
+ int res;
+ uint64_t start = getnanotime();
+
+ res = fetch_objects(o->remote_name, oids, oid_nr);
+
+ trace_performance_since(start, "promisor_remote_get_direct");
+
+ return res;
+}
+
+int promisors_get_direct(const struct object_id *oids, int oid_nr)
+{
+ struct promisor_remote *o;
+
+ trace_printf("trace: promisor_remote_get_direct: nr: %d", oid_nr);
+
+ promisor_remote_init();
+
+ for (o = promisors; o; o = o->next) {
+ if (promisor_remote_get_direct(o, oids, oid_nr) < 0)
+ continue;
+ return 0;
+ }
+
+ return -1;
+}
+
diff --git a/promisor-remote.h b/promisor-remote.h
index d07ac07a43..8b89221b33 100644
--- a/promisor-remote.h
+++ b/promisor-remote.h
@@ -13,5 +13,6 @@ struct promisor_remote {
extern struct promisor_remote *promisor_remote_new(const char *remote_name);
extern struct promisor_remote *find_promisor_remote(const char *remote_name);
extern int has_promisor_remote(void);
+extern int promisors_get_direct(const struct object_id *oids, int oid_nr);
#endif /* PROMISOR_REMOTE_H */
--
2.20.0.rc2.14.g1379de12fa.dirty
^ permalink raw reply related
* [PATCH v1 1/8] fetch-object: make functions return an error code
From: Christian Couder @ 2018-12-11 5:27 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Ben Peart, Jonathan Tan,
Jonathan Nieder, Stefan Beller, Nguyen Thai Ngoc Duy, Mike Hommey,
Lars Schneider, Eric Wong, Christian Couder, Jeff Hostetler,
Eric Sunshine, Beat Bolli, Christian Couder
In-Reply-To: <20181211052746.16218-1-chriscool@tuxfamily.org>
From: Christian Couder <christian.couder@gmail.com>
The callers of the fetch_object() and fetch_objects() might
be interested in knowing if these functions succeeded or not.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
fetch-object.c | 13 ++++++++-----
fetch-object.h | 4 ++--
sha1-file.c | 4 ++--
3 files changed, 12 insertions(+), 9 deletions(-)
diff --git a/fetch-object.c b/fetch-object.c
index 4266548800..eac4d448ef 100644
--- a/fetch-object.c
+++ b/fetch-object.c
@@ -5,11 +5,12 @@
#include "transport.h"
#include "fetch-object.h"
-static void fetch_refs(const char *remote_name, struct ref *ref)
+static int fetch_refs(const char *remote_name, struct ref *ref)
{
struct remote *remote;
struct transport *transport;
int original_fetch_if_missing = fetch_if_missing;
+ int res;
fetch_if_missing = 0;
remote = remote_get(remote_name);
@@ -19,12 +20,14 @@ static void fetch_refs(const char *remote_name, struct ref *ref)
transport_set_option(transport, TRANS_OPT_FROM_PROMISOR, "1");
transport_set_option(transport, TRANS_OPT_NO_DEPENDENTS, "1");
- transport_fetch_refs(transport, ref);
+ res = transport_fetch_refs(transport, ref);
fetch_if_missing = original_fetch_if_missing;
+
+ return res;
}
-void fetch_objects(const char *remote_name, const struct object_id *oids,
- int oid_nr)
+int fetch_objects(const char *remote_name, const struct object_id *oids,
+ int oid_nr)
{
struct ref *ref = NULL;
int i;
@@ -36,5 +39,5 @@ void fetch_objects(const char *remote_name, const struct object_id *oids,
new_ref->next = ref;
ref = new_ref;
}
- fetch_refs(remote_name, ref);
+ return fetch_refs(remote_name, ref);
}
diff --git a/fetch-object.h b/fetch-object.h
index d6444caa5a..7bcc7cadb0 100644
--- a/fetch-object.h
+++ b/fetch-object.h
@@ -3,7 +3,7 @@
struct object_id;
-void fetch_objects(const char *remote_name, const struct object_id *oids,
- int oid_nr);
+int fetch_objects(const char *remote_name, const struct object_id *oids,
+ int oid_nr);
#endif
diff --git a/sha1-file.c b/sha1-file.c
index 5bd11c85bc..cf88eeb147 100644
--- a/sha1-file.c
+++ b/sha1-file.c
@@ -1312,8 +1312,8 @@ int oid_object_info_extended(struct repository *r, const struct object_id *oid,
if (fetch_if_missing && repository_format_partial_clone &&
!already_retried && r == the_repository) {
/*
- * TODO Investigate having fetch_object() return
- * TODO error/success and stopping the music here.
+ * TODO Investigate checking fetch_object() return
+ * TODO value and stopping on error here.
* TODO Pass a repository struct through fetch_object,
* such that arbitrary repositories work.
*/
--
2.20.0.rc2.14.g1379de12fa.dirty
^ permalink raw reply related
* [PATCH v1 0/8] Many promisor remotes
From: Christian Couder @ 2018-12-11 5:27 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Ben Peart, Jonathan Tan,
Jonathan Nieder, Stefan Beller, Nguyen Thai Ngoc Duy, Mike Hommey,
Lars Schneider, Eric Wong, Christian Couder, Jeff Hostetler,
Eric Sunshine, Beat Bolli
This path series is a follow up from the "remote odb" patch series
that I sent earlier this year, which were a follow up from previous
series. See the links section for more information.
The goal of this patch series is to make it possible to have and to
fetch missing objects from multiple remotes instead of only one.
For now the fetch order is the order of the remotes in the config.
I selected the name "Promisor remote" over "Partial clone remote"
because it is shorter and because it is not only about cloning.
The existing extensions.partialclone is respected, but it is not
written in the config when a partial clone or fetch is made. Instead
remote.<name>.promisor is set to "true". This may create a
compatibility issue, but it makes it possible to start using many
promisor remotes by just cloning and fetching from different remotes
with partial clone filters. The compatibility issue could be resolved
in a future iteration by just setting extensions.partialclone instead
of remote.<name>.promisor the first time a promisor remote is used.
In general I have tried to change as few things as possible.
Yeah, this is missing documentation for now.
High level overview of this patch series
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
- Patch 1/8:
This makes functions in fetch-object.c return an error code, which is
necessary to later tell that they failed and try another promisor
remote when there is more than one. This could also just be seen as a
fix to these functions.
- Patch 2/8:
This introduces the minimum infrastructure for promisor remotes.
- Patch 3/8, 4/8 and 5/8:
This add a few missing bits in the promisor remote infrastructure that
will be needed in the following patches.
- Patch 6/8:
This replaces the previous interface to use only one promisor remote
defined in extensions.partialclone by the new interface created by the
previous patches.
- Patch 7/8:
This replaces the way a partial clone filter was handled by a new way
based on the previous patches that support more than one partial clone
filter.
- Patch 8/8:
This adds a test case that shows that now more than one promisor
remote can be used.
Links
~~~~~
This new patch series is a follow up from the discussions related to
the remote odb V4 patch series:
https://public-inbox.org/git/20180802061505.2983-1-chriscool@tuxfamily.org/
Especially in:
https://public-inbox.org/git/CAP8UFD3nrhjANwNDqTwx5ZtnZNcnbAFqUN=u=LrvzuH4+3wQQA@mail.gmail.com/
I said that I would like to work on things in the following order:
1) Teaching partial clone to attempt to fetch missing objects from
multiple remotes instead of only one using the order in the config.
2) Simplifying the protocol for fetching missing objects so that it
can be satisfied by a lighter weight object storage system than a full
Git server.
3) Making it possible to explicitly define an order in which the
remotes are accessed.
4) Making the criteria for what objects can be missing more
aggressive, so that I can "git add" a large file and work with it
using Git without even having a second copy of that object in my local
object store.
And this patch series is about the 1).
This patch series on GitHub:
https://github.com/chriscool/git/commits/many-promisor-remotes
The previous remote odb patch series on GitHub:
V5: https://github.com/chriscool/git/commits/remote-odb
V4: https://github.com/chriscool/git/commits/remote-odb5
V3: https://github.com/chriscool/git/commits/remote-odb3
V2: https://github.com/chriscool/git/commits/remote-odb2
V1: https://github.com/chriscool/git/commits/remote-odb1
Discussions related to previous versions of the odb patch series:
V4: https://public-inbox.org/git/20180802061505.2983-1-chriscool@tuxfamily.org/
V3: https://public-inbox.org/git/20180713174959.16748-1-chriscool@tuxfamily.org/
V2: https://public-inbox.org/git/20180630083542.20347-1-chriscool@tuxfamily.org/
V1: https://public-inbox.org/git/20180623121846.19750-1-chriscool@tuxfamily.org/
Christian Couder (8):
fetch-object: make functions return an error code
Add initial support for many promisor remotes
promisor-remote: implement promisors_get_direct()
promisor-remote: add promisor_remote_reinit()
promisor-remote: use repository_format_partial_clone
Use promisors_get_direct() and has_promisor_remote()
promisor-remote: parse remote.*.partialclonefilter
t0410: test fetching from many promisor remotes
Makefile | 1 +
builtin/cat-file.c | 5 +-
builtin/fetch.c | 13 +--
builtin/gc.c | 3 +-
builtin/repack.c | 3 +-
cache-tree.c | 3 +-
connected.c | 3 +-
fetch-object.c | 13 +--
fetch-object.h | 4 +-
list-objects-filter-options.c | 51 ++++++------
list-objects-filter-options.h | 3 +-
packfile.c | 3 +-
promisor-remote.c | 147 ++++++++++++++++++++++++++++++++++
promisor-remote.h | 22 +++++
sha1-file.c | 14 ++--
t/t0410-partial-clone.sh | 26 +++++-
t/t5601-clone.sh | 3 +-
t/t5616-partial-clone.sh | 4 +-
unpack-trees.c | 6 +-
19 files changed, 269 insertions(+), 58 deletions(-)
create mode 100644 promisor-remote.c
create mode 100644 promisor-remote.h
--
2.20.0.rc2.14.g1379de12fa.dirty
^ permalink raw reply
* Re: [PATCH] terminology tweak: prune -> path limiting
From: MATTHEW DEVORE @ 2018-12-11 5:16 UTC (permalink / raw)
To: Junio C Hamano, Matthew DeVore
Cc: git, Jonathan Nieder, dstolee, pclouds, Jeff King
In-Reply-To: <xmqq1s6oon7a.fsf@gitster-ct.c.googlers.com>
> On December 10, 2018 at 5:40 PM Junio C Hamano <gitster@pobox.com> wrote:
> So I think it is a good idea to get rid of prune_data and make it
> clear it is no longer a generic thing but cannot be anything but a
> pathspec. I am not sure what the bit should be called, though. It
> is a bit to enable any history simplification and not limited to
> pathspec limiting (e.g. simplify-by-decoration enables it, too).
>
How about the below patch? I used "can_ignore_commits" as the bit name, which - as the commit msg states - reflects the terminology used in get_commit_action and related functions.
Subject: [PATCH] terminology tweak: prune -> path limiting
In the codebase, "prune" is a highly overloaded term, and it caused me a
lot of trouble to figure out what it meant when it was used in the
context of path limiting and stripping commits from history.
Rename two identifiers: prune_data (which used to be a void* argument to
a function called prune_fn) to path_limits, which correctly describes
its current use; and prune to can_ignore_commits, which describes well
what it means and parallels the terminology used in the
get_commit_action function.
Signed-off-by: Matthew DeVore <matvore@google.com>
---
Documentation/technical/api-history-graph.txt | 5 +-
builtin/add.c | 4 +-
builtin/diff.c | 8 +-
builtin/fast-export.c | 2 +-
builtin/log.c | 2 +-
builtin/rev-list.c | 2 +-
diff-lib.c | 6 +-
revision.c | 92 ++++++++++---------
revision.h | 4 +-
t/t7811-grep-open.sh | 2 +-
tree-walk.c | 10 +-
wt-status.c | 4 +-
12 files changed, 73 insertions(+), 68 deletions(-)
diff --git a/Documentation/technical/api-history-graph.txt b/Documentation/technical/api-history-graph.txt
index d0d1707c8c..f9a100f88c 100644
--- a/Documentation/technical/api-history-graph.txt
+++ b/Documentation/technical/api-history-graph.txt
@@ -100,8 +100,9 @@ Limitations
on all parents of that commit. Parents must not be skipped, or the graph
output will appear incorrect.
+
-`graph_update()` may be used on a pruned set of commits only if the parent list
-has been rewritten so as to include only ancestors from the pruned set.
+`graph_update()` may be used on a pruned (e.g. path-limited) set of commits only
+if the parent list has been rewritten so as to include only ancestors from the
+pruned set.
* The graph API does not currently support reverse commit ordering. In
order to implement reverse ordering, the graphing API needs an
diff --git a/builtin/add.c b/builtin/add.c
index f65c172299..4abd8ebba8 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -113,14 +113,14 @@ int add_files_to_cache(const char *prefix,
repo_init_revisions(the_repository, &rev, prefix);
setup_revisions(0, NULL, &rev, NULL);
if (pathspec)
- copy_pathspec(&rev.prune_data, pathspec);
+ copy_pathspec(&rev.path_limits, pathspec);
rev.diffopt.output_format = DIFF_FORMAT_CALLBACK;
rev.diffopt.format_callback = update_callback;
rev.diffopt.format_callback_data = &data;
rev.diffopt.flags.override_submodule_config = 1;
rev.max_count = 0; /* do not compare unmerged paths with stage #2 */
run_diff_files(&rev, DIFF_RACY_IS_MODIFIED);
- clear_pathspec(&rev.prune_data);
+ clear_pathspec(&rev.path_limits);
return !!data.add_errors;
}
diff --git a/builtin/diff.c b/builtin/diff.c
index f0393bba23..9010b3228a 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -74,8 +74,8 @@ static int builtin_diff_b_f(struct rev_info *revs,
if (argc > 1)
usage(builtin_diff_usage);
- GUARD_PATHSPEC(&revs->prune_data, PATHSPEC_FROMTOP | PATHSPEC_LITERAL);
- path = revs->prune_data.items[0].match;
+ GUARD_PATHSPEC(&revs->path_limits, PATHSPEC_FROMTOP | PATHSPEC_LITERAL);
+ path = revs->path_limits.items[0].match;
if (lstat(path, &st))
die_errno(_("failed to stat '%s'"), path);
@@ -421,8 +421,8 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
die(_("unhandled object '%s' given."), name);
}
}
- if (rev.prune_data.nr)
- paths += rev.prune_data.nr;
+ if (rev.path_limits.nr)
+ paths += rev.path_limits.nr;
/*
* Now, do the arguments look reasonable?
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 9e283482ef..6a675b5737 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -1143,7 +1143,7 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
import_marks(import_filename);
lastimportid = last_idnum;
- if (import_filename && revs.prune_data.nr)
+ if (import_filename && revs.path_limits.nr)
full_tree = 1;
get_tags_and_duplicates(&revs.cmdline);
diff --git a/builtin/log.c b/builtin/log.c
index 45aa376a59..f8554d7fa1 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -690,7 +690,7 @@ static void log_setup_revisions_tweak(struct rev_info *rev,
struct setup_revision_opt *opt)
{
if (rev->diffopt.flags.default_follow_renames &&
- rev->prune_data.nr == 1)
+ rev->path_limits.nr == 1)
rev->diffopt.flags.follow_renames = 1;
/* Turn --cc/-c into -p --cc/-c when -p was not given */
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 3a2c0c23b6..f6ce622dd1 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -517,7 +517,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
if (show_progress)
progress = start_delayed_progress(show_progress, 0);
- if (use_bitmap_index && !revs.prune) {
+ if (use_bitmap_index && !revs.can_ignore_commits) {
if (revs.count && !revs.left_right && !revs.cherry_mark) {
uint32_t commit_count;
int max_count = revs.max_count;
diff --git a/diff-lib.c b/diff-lib.c
index 23c8d351b3..431bed0e5a 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -110,7 +110,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
if (diff_can_quit_early(&revs->diffopt))
break;
- if (!ce_path_match(istate, ce, &revs->prune_data, NULL))
+ if (!ce_path_match(istate, ce, &revs->path_limits, NULL))
continue;
if (ce_stage(ce)) {
@@ -477,7 +477,7 @@ static int oneway_diff(const struct cache_entry * const *src,
if (ce_path_match(revs->diffopt.repo->index,
idx ? idx : tree,
- &revs->prune_data, NULL)) {
+ &revs->path_limits, NULL)) {
do_oneway_diff(o, idx, tree);
if (diff_can_quit_early(&revs->diffopt)) {
o->exiting_early = 1;
@@ -543,7 +543,7 @@ int do_diff_cache(const struct object_id *tree_oid, struct diff_options *opt)
struct rev_info revs;
repo_init_revisions(opt->repo, &revs, NULL);
- copy_pathspec(&revs.prune_data, &opt->pathspec);
+ copy_pathspec(&revs.path_limits, &opt->pathspec);
revs.diffopt = *opt;
if (diff_cache(&revs, tree_oid, NULL, 1))
diff --git a/revision.c b/revision.c
index 13e0519c02..9525c9f161 100644
--- a/revision.c
+++ b/revision.c
@@ -488,7 +488,7 @@ static int rev_compare_tree(struct rev_info *revs,
* tagged commit by specifying both --simplify-by-decoration
* and pathspec.
*/
- if (!revs->prune_data.nr)
+ if (!revs->path_limits.nr)
return REV_TREE_SAME;
}
@@ -616,14 +616,14 @@ static unsigned update_treesame(struct rev_info *revs, struct commit *commit)
static inline int limiting_can_increase_treesame(const struct rev_info *revs)
{
/*
- * TREESAME is irrelevant unless prune && dense;
+ * TREESAME is irrelevant unless can_ignore_commits && dense;
* if simplify_history is set, we can't have a mixture of TREESAME and
* !TREESAME INTERESTING parents (and we don't have treesame[]
* decoration anyway);
* if first_parent_only is set, then the TREESAME flag is locked
* against the first parent (and again we lack treesame[] decoration).
*/
- return revs->prune && revs->dense &&
+ return revs->can_ignore_commits && revs->dense &&
!revs->simplify_history &&
!revs->first_parent_only;
}
@@ -636,9 +636,9 @@ static void try_to_simplify_commit(struct rev_info *revs, struct commit *commit)
int relevant_parents, nth_parent;
/*
- * If we don't do pruning, everything is interesting
+ * If we don't do commit pruning, everything is interesting
*/
- if (!revs->prune)
+ if (!revs->can_ignore_commits)
return;
if (!get_commit_tree(commit))
@@ -786,7 +786,7 @@ static int process_parents(struct rev_info *revs, struct commit *commit,
/*
* If the commit is uninteresting, don't try to
- * prune parents - we want the maximal uninteresting
+ * path limit parents - we want the maximal uninteresting
* set.
*
* Normally we haven't parsed the parent
@@ -1511,8 +1511,8 @@ static void prepare_show_merge(struct rev_info *revs)
struct commit_list *bases;
struct commit *head, *other;
struct object_id oid;
- const char **prune = NULL;
- int i, prune_num = 1; /* counting terminating NULL */
+ const char **limiting_paths = NULL;
+ int i, limiting_paths_num = 1; /* counting terminating NULL */
struct index_state *istate = revs->repo->index;
if (get_oid("HEAD", &oid))
@@ -1535,19 +1535,20 @@ static void prepare_show_merge(struct rev_info *revs)
const struct cache_entry *ce = istate->cache[i];
if (!ce_stage(ce))
continue;
- if (ce_path_match(istate, ce, &revs->prune_data, NULL)) {
- prune_num++;
- REALLOC_ARRAY(prune, prune_num);
- prune[prune_num-2] = ce->name;
- prune[prune_num-1] = NULL;
+ if (ce_path_match(istate, ce, &revs->path_limits, NULL)) {
+ limiting_paths_num++;
+ REALLOC_ARRAY(limiting_paths, limiting_paths_num);
+ limiting_paths[limiting_paths_num-2] = ce->name;
+ limiting_paths[limiting_paths_num-1] = NULL;
}
while ((i+1 < istate->cache_nr) &&
ce_same_name(ce, istate->cache[i+1]))
i++;
}
- clear_pathspec(&revs->prune_data);
- parse_pathspec(&revs->prune_data, PATHSPEC_ALL_MAGIC & ~PATHSPEC_LITERAL,
- PATHSPEC_PREFER_FULL | PATHSPEC_LITERAL_PATH, "", prune);
+ clear_pathspec(&revs->path_limits);
+ parse_pathspec(&revs->path_limits, PATHSPEC_ALL_MAGIC & ~PATHSPEC_LITERAL,
+ PATHSPEC_PREFER_FULL | PATHSPEC_LITERAL_PATH, "",
+ limiting_paths);
revs->limited = 1;
}
@@ -1736,14 +1737,14 @@ int handle_revision_arg(const char *arg_, struct rev_info *revs, int flags, unsi
}
static void read_pathspec_from_stdin(struct rev_info *revs, struct strbuf *sb,
- struct argv_array *prune)
+ struct argv_array *limiting_paths)
{
while (strbuf_getline(sb, stdin) != EOF)
- argv_array_push(prune, sb->buf);
+ argv_array_push(limiting_paths, sb->buf);
}
static void read_revisions_from_stdin(struct rev_info *revs,
- struct argv_array *prune)
+ struct argv_array *limiting_paths)
{
struct strbuf sb;
int seen_dashdash = 0;
@@ -1769,7 +1770,7 @@ static void read_revisions_from_stdin(struct rev_info *revs,
die("bad revision '%s'", sb.buf);
}
if (seen_dashdash)
- read_pathspec_from_stdin(revs, &sb, prune);
+ read_pathspec_from_stdin(revs, &sb, limiting_paths);
strbuf_release(&sb);
warn_on_object_refname_ambiguity = save_warning;
@@ -1884,7 +1885,7 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
revs->simplify_history = 0;
revs->simplify_by_decoration = 1;
revs->limited = 1;
- revs->prune = 1;
+ revs->can_ignore_commits = 1;
load_ref_decorations(NULL, DECORATE_SHORT_REFS);
} else if (!strcmp(arg, "--date-order")) {
revs->sort_order = REV_SORT_BY_COMMIT_DATE;
@@ -2338,7 +2339,7 @@ static void NORETURN diagnose_missing_default(const char *def)
int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct setup_revision_opt *opt)
{
int i, flags, left, seen_dashdash, got_rev_arg = 0, revarg_opt;
- struct argv_array prune_data = ARGV_ARRAY_INIT;
+ struct argv_array path_limits = ARGV_ARRAY_INIT;
const char *submodule = NULL;
if (opt)
@@ -2356,7 +2357,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
argv[i] = NULL;
argc = i;
if (argv[i + 1])
- argv_array_pushv(&prune_data, argv + i + 1);
+ argv_array_pushv(&path_limits, argv + i + 1);
seen_dashdash = 1;
break;
}
@@ -2387,7 +2388,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
}
if (revs->read_from_stdin++)
die("--stdin given twice?");
- read_revisions_from_stdin(revs, &prune_data);
+ read_revisions_from_stdin(revs, &path_limits);
continue;
}
@@ -2416,32 +2417,32 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
for (j = i; j < argc; j++)
verify_filename(revs->prefix, argv[j], j == i);
- argv_array_pushv(&prune_data, argv + i);
+ argv_array_pushv(&path_limits, argv + i);
break;
}
else
got_rev_arg = 1;
}
- if (prune_data.argc) {
+ if (path_limits.argc) {
/*
* If we need to introduce the magic "a lone ':' means no
* pathspec whatsoever", here is the place to do so.
*
- * if (prune_data.nr == 1 && !strcmp(prune_data[0], ":")) {
- * prune_data.nr = 0;
- * prune_data.alloc = 0;
- * free(prune_data.path);
- * prune_data.path = NULL;
+ * if (path_limits.nr == 1 && !strcmp(path_limits[0], ":")) {
+ * path_limits.nr = 0;
+ * path_limits.alloc = 0;
+ * free(path_limits.path);
+ * path_limits.path = NULL;
* } else {
- * terminate prune_data.alloc with NULL and
- * call init_pathspec() to set revs->prune_data here.
+ * terminate path_limits.alloc with NULL and
+ * call init_pathspec() to set revs->path_limits here.
* }
*/
- parse_pathspec(&revs->prune_data, 0, 0,
- revs->prefix, prune_data.argv);
+ parse_pathspec(&revs->path_limits, 0, 0,
+ revs->prefix, path_limits.argv);
}
- argv_array_clear(&prune_data);
+ argv_array_clear(&path_limits);
if (revs->def == NULL)
revs->def = opt ? opt->def : NULL;
@@ -2475,14 +2476,17 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
if (revs->topo_order && !generation_numbers_enabled(the_repository))
revs->limited = 1;
- if (revs->prune_data.nr) {
- copy_pathspec(&revs->pruning.pathspec, &revs->prune_data);
- /* Can't prune commits with rename following: the paths change.. */
+ if (revs->path_limits.nr) {
+ copy_pathspec(&revs->pruning.pathspec, &revs->path_limits);
+ /*
+ * Can't path-limit commits with rename following; the paths
+ * change.
+ */
if (!revs->diffopt.flags.follow_renames)
- revs->prune = 1;
+ revs->can_ignore_commits = 1;
if (!revs->full_diff)
copy_pathspec(&revs->diffopt.pathspec,
- &revs->prune_data);
+ &revs->path_limits);
}
if (revs->combine_merges)
revs->ignore_merges = 0;
@@ -2845,7 +2849,7 @@ static void simplify_merges(struct rev_info *revs)
struct commit_list *yet_to_do, **tail;
struct commit *commit;
- if (!revs->prune)
+ if (!revs->can_ignore_commits)
return;
/* feed the list reversed */
@@ -3361,7 +3365,7 @@ enum commit_action get_commit_action(struct rev_info *revs, struct commit *commi
}
if (!commit_match(commit, revs))
return commit_ignore;
- if (revs->prune && revs->dense) {
+ if (revs->can_ignore_commits && revs->dense) {
/* Commit without changes? */
if (commit->object.flags & TREESAME) {
int n;
@@ -3446,7 +3450,7 @@ enum commit_action simplify_commit(struct rev_info *revs, struct commit *commit)
enum commit_action action = get_commit_action(revs, commit);
if (action == commit_show &&
- revs->prune && revs->dense && want_ancestry(revs)) {
+ revs->can_ignore_commits && revs->dense && want_ancestry(revs)) {
/*
* --full-diff on simplified parents is no good: it
* will show spurious changes from the commits that
diff --git a/revision.h b/revision.h
index 7987bfcd2e..b19d5536f1 100644
--- a/revision.h
+++ b/revision.h
@@ -87,7 +87,7 @@ struct rev_info {
/* Basic information */
const char *prefix;
const char *def;
- struct pathspec prune_data;
+ struct pathspec path_limits;
/*
* Whether the arguments parsed by setup_revisions() included any
@@ -111,7 +111,7 @@ struct rev_info {
/* Traversal flags */
unsigned int dense:1,
- prune:1,
+ can_ignore_commits:1,
no_walk:2,
remove_empty_trees:1,
simplify_history:1,
diff --git a/t/t7811-grep-open.sh b/t/t7811-grep-open.sh
index d1ebfd88c7..79af1b7187 100755
--- a/t/t7811-grep-open.sh
+++ b/t/t7811-grep-open.sh
@@ -23,7 +23,7 @@ enum grep_pat_token {
test_commit add-user revision.c "
}
if (seen_dashdash)
- read_pathspec_from_stdin(revs, &sb, prune);
+ read_pathspec_from_stdin(revs, &sb, limiting_paths);
strbuf_release(&sb);
}
diff --git a/tree-walk.c b/tree-walk.c
index 79bafbd1a2..b60170b6b4 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -365,10 +365,10 @@ static void free_extended_entry(struct tree_desc_x *t)
}
}
-static inline int prune_traversal(struct name_entry *e,
- struct traverse_info *info,
- struct strbuf *base,
- int still_interesting)
+static inline int path_limit_traversal(struct name_entry *e,
+ struct traverse_info *info,
+ struct strbuf *base,
+ int still_interesting)
{
if (!info->pathspec || still_interesting == 2)
return 2;
@@ -461,7 +461,7 @@ int traverse_trees(int n, struct tree_desc *t, struct traverse_info *info)
}
if (!mask)
break;
- interesting = prune_traversal(e, info, &base, interesting);
+ interesting = path_limit_traversal(e, info, &base, interesting);
if (interesting < 0)
break;
if (interesting) {
diff --git a/wt-status.c b/wt-status.c
index 0fe3bcd4cd..b0a3efea4b 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -603,7 +603,7 @@ static void wt_status_collect_changes_worktree(struct wt_status *s)
rev.diffopt.detect_rename = s->detect_rename >= 0 ? s->detect_rename : rev.diffopt.detect_rename;
rev.diffopt.rename_limit = s->rename_limit >= 0 ? s->rename_limit : rev.diffopt.rename_limit;
rev.diffopt.rename_score = s->rename_score >= 0 ? s->rename_score : rev.diffopt.rename_score;
- copy_pathspec(&rev.prune_data, &s->pathspec);
+ copy_pathspec(&rev.path_limits, &s->pathspec);
run_diff_files(&rev, 0);
}
@@ -639,7 +639,7 @@ static void wt_status_collect_changes_index(struct wt_status *s)
rev.diffopt.detect_rename = s->detect_rename >= 0 ? s->detect_rename : rev.diffopt.detect_rename;
rev.diffopt.rename_limit = s->rename_limit >= 0 ? s->rename_limit : rev.diffopt.rename_limit;
rev.diffopt.rename_score = s->rename_score >= 0 ? s->rename_score : rev.diffopt.rename_score;
- copy_pathspec(&rev.prune_data, &s->pathspec);
+ copy_pathspec(&rev.path_limits, &s->pathspec);
run_diff_index(&rev, 1);
}
--
2.17.1
^ permalink raw reply related
* Re: [PATCH] Re: [wishlist] submodule.update config
From: Yaroslav O Halchenko @ 2018-12-11 5:10 UTC (permalink / raw)
To: git
In-Reply-To: <20181211000859.130266-1-sbeller@google.com>
On Mon, 10 Dec 2018, Stefan Beller wrote:
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
> > > So you are proposing a variable like submodule.update
> > [...]
> > Glad to hear that. Not sure though I would know where to stick my
> > nose to figure out what to change. ;-)
> The update_module is computed via the submodule--helpers
> update-module-mode command, which is a small wrapper
> around determine_submodule_update_strategy()
> which you already touched in the other patch that makes
> --reset-hard another mode.
> This contains code and tests, but we'd need some docs as well.
> I am not sure about this patch as it allows for easier experimentation
> with submodules (e.g. "git config submodule.update '!git reset --hard'"
> sounds like what you're trying to get)
;-) it was indeed one of the original approaches I considered instead of
having "update --reset-hard"...
> and using them, but as discussed
> below this might be too much convenience already and we'd rather want to
> have it properly integrated into the real commands.
indeed, having "update --reset-hard" provides necessary to me
convenience for my use cases. Motivation behind submodule.update was
primarily to allow for heterogeneous (but still simple to define)
strategies, where for some subproject I could just define
submodule.update to be "reset-hard" (I do not expect my local commits
matter) and in the others -- "merge" (I carry my changes on top).
But again, I must confess, that either I forgot or just do not see a
clear use-case/demand for submodule.update config myself any longer,
besides providing a potentially useful default over
submodule.MODULE.update config.
> > Well, not sure... In the long run, if UX is to be tuned up, I wonder if
> > it would be more worthwhile to look toward making all those git commands
> > (git merge, git checkout, git rebase, ..., git revert, git cherry-pick)
> > support --recurse-submodules with a consistent with the non-recursive
> > operation by default behavior
> That is the end goal, very much.
> > (e.g. not introducing detached HEADs or
> > controlling that via a set of additional options where needed).
> As with the discussion of the submodule.repoLike option (the patch I
> referenced in the other discussion), this is tricky to get the right
> behavior, so it takes some more time to do.
> Also what is right for a "git merge --recursive" might be totally different
> from a "git submodule update --merge" as the former is not centered around
> submodules but merging, such that a sensible default would be expected,
> whereas the "submodule update" is allowed to have a rough edge.
Probably I need to try "submodules update --merge" to see what is that
rough edge which makes it different from the potential "merge
--recurse-submodules", or is it easy to describe? ;-)
I wonder if may be instead of pestering you about this config one, I
should ask about pointers on how to accomplish "revert
--recurse-submodules" or where to poke to make it possible to clone
recursively from http://datasets.datalad.org/ where we do not place
submodules all under the very top /.git/modules ;-)
--
Yaroslav O. Halchenko
Center for Open Neuroscience http://centerforopenneuroscience.org
Dartmouth College, 419 Moore Hall, Hinman Box 6207, Hanover, NH 03755
Phone: +1 (603) 646-9834 Fax: +1 (603) 646-1419
WWW: http://www.linkedin.com/in/yarik
^ permalink raw reply
* Re: silent_exec_failure when calling gpg
From: Junio C Hamano @ 2018-12-11 4:09 UTC (permalink / raw)
To: John Passaro; +Cc: git, Jeff King
In-Reply-To: <xmqqlg4wlocc.fsf@gitster-ct.c.googlers.com>
Junio C Hamano <gitster@pobox.com> writes:
> John Passaro <john.a.passaro@gmail.com> writes:
>
>> I've noticed that in v2.19.1, when using git to pretty print
>> information about the signature, if git cannot find gpg (e.g. "git
>> config gpg.program nogpg"), it prints an error to stderr:
>>
>> $ git show -s --pretty=%G?
>> fatal: cannot run nogpg: No such file or directory
>> N
>
> I think the uninteded behaviour change was in 17809a98 ("Merge
> branch 'jk/run-command-notdot'", 2018-10-30).
Perhaps something like this. There needs an additional test added
for this codepath, which I haven't done yet, though.
run-command.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/run-command.c b/run-command.c
index d679cc267c..e2bc18a083 100644
--- a/run-command.c
+++ b/run-command.c
@@ -728,6 +728,8 @@ int start_command(struct child_process *cmd)
if (prepare_cmd(&argv, cmd) < 0) {
failed_errno = errno;
cmd->pid = -1;
+ if (!cmd->silent_exec_failure)
+ error_errno("cannot run %s", cmd->argv[0]);
goto end_of_spawn;
}
^ permalink raw reply related
* [PATCH 2/2] RF+ENH(TST): compare the entire list of submodule status --recursive to stay intact
From: Yaroslav Halchenko @ 2018-12-11 4:08 UTC (permalink / raw)
To: git; +Cc: Yaroslav Halchenko
In-Reply-To: <20181211040839.17472-1-debian@onerussian.com>
For submodule update --reset-hard the best test is comparison of the
entire status as shown by submodule status --recursive. Upon update
--reset-hard we should get back to the original state, with all the
branches being the same (no detached HEAD) and commits identical to
original (so no merges, new commits, etc).
For that, I have introduced two helpers: {record,compare}_submodules_status and
an additional test for --reset-hard in nested submodule.
I have kept this as a separate PATCH to demonstrate the diff from the original
test composition as introduced in the prior patch, and this one where
all tests could be of the same type:
record_submodule_status &&
perform evil actions &&
! compare_submodule_status && # to double check that evil was done
git submodule --reset-hard . &&
compare_submodule_status # assure that we are all good
Signed-off-by: Yaroslav Halchenko <debian@onerussian.com>
---
t/t7406-submodule-update.sh | 37 ++++++++++++++++++++++++++++++-------
1 file changed, 30 insertions(+), 7 deletions(-)
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index 2e08e0047c..1927424f47 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -21,6 +21,17 @@ compare_head()
test "$sha_master" = "$sha_head"
}
+record_submodules_status()
+{
+ git submodule status --recursive >expect
+}
+
+compare_submodules_status()
+{
+ git submodule status --recursive >actual &&
+ test_i18ncmp expect actual
+}
+
test_expect_success 'setup a submodule tree' '
echo file > file &&
@@ -294,7 +305,7 @@ test_expect_success 'submodule update --rebase staying on master' '
test_expect_success 'submodule update --merge staying on master' '
(cd super/submodule &&
- git reset --hard HEAD~1
+ git reset --hard HEAD~1
) &&
(cd super &&
(cd submodule &&
@@ -307,16 +318,28 @@ test_expect_success 'submodule update --merge staying on master' '
'
test_expect_success 'submodule update --reset-hard staying on master' '
- (cd super/submodule &&
- git reset --hard HEAD~1
- ) &&
(cd super &&
+ record_submodules_status &&
(cd submodule &&
- compare_head
+ git reset --hard HEAD~1
) &&
+ ! compare_submodules_status &&
git submodule update --reset-hard submodule &&
- cd submodule &&
- compare_head
+ compare_submodules_status
+ )
+'
+
+test_expect_success 'submodule update --reset-hard in nested submodule' '
+ (cd recursivesuper &&
+ git submodule update --init --recursive &&
+ record_submodules_status &&
+ (cd super/submodule &&
+ echo 123 >> file &&
+ git commit -m "new commit" file
+ ) &&
+ ! compare_submodules_status &&
+ git submodule update --reset-hard --recursive &&
+ compare_submodules_status
)
'
--
2.20.0.rc2.8.g0a3bec4a1c.dirty
^ permalink raw reply related
* [PATCH 1/2] submodule: Add --reset-hard option for git submodule update
From: Yaroslav Halchenko @ 2018-12-11 4:08 UTC (permalink / raw)
To: git; +Cc: Yaroslav Halchenko
In-Reply-To: <CAGZ79kYDa27EFk4A9uEzCnoW7scjb1U8fKwCo0P7rUZESto+Qg@mail.gmail.com>
This patch adds a --reset-hard option for the update command to hard
reset submodule(s) to the gitlink for the corresponding submodule in
the superproject. This feature is desired e.g. to be able to discard
recent changes in the entire hierarchy of the submodules after running
git reset --hard PREVIOUS_STATE
in the superproject which leaves submodules in their original state,
and
git reset --hard --recurse-submodules PREVIOUS_STATE
would result in the submodules being checked into detached HEADs.
As in the original git reset --hard no checks or any kind of
safe-guards against jumping into some state which was never on the
current branch is done.
must_die_on_failure is not set to yes to mimic behavior of a update
--checkout strategy, which would leave user with a non-clean state
immediately apparent via git status so an informed decision/actions
could be done manually.
Signed-off-by: Yaroslav Halchenko <debian@onerussian.com>
---
Documentation/git-submodule.txt | 12 +++++++++++-
Documentation/gitmodules.txt | 4 ++--
builtin/submodule--helper.c | 3 ++-
git-submodule.sh | 10 +++++++++-
submodule.c | 4 ++++
submodule.h | 1 +
t/t7406-submodule-update.sh | 17 ++++++++++++++++-
7 files changed, 45 insertions(+), 6 deletions(-)
diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index ba3c4df550..f4e0483997 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -124,7 +124,7 @@ If you really want to remove a submodule from the repository and commit
that use linkgit:git-rm[1] instead. See linkgit:gitsubmodules[7] for removal
options.
-update [--init] [--remote] [-N|--no-fetch] [--[no-]recommend-shallow] [-f|--force] [--checkout|--rebase|--merge] [--reference <repository>] [--depth <depth>] [--recursive] [--jobs <n>] [--] [<path>...]::
+update [--init] [--remote] [-N|--no-fetch] [--[no-]recommend-shallow] [-f|--force] [--checkout|--rebase|--merge|--reset-hard] [--reference <repository>] [--depth <depth>] [--recursive] [--jobs <n>] [--] [<path>...]::
+
--
Update the registered submodules to match what the superproject
@@ -358,6 +358,16 @@ the submodule itself.
If the key `submodule.$name.update` is set to `rebase`, this option is
implicit.
+--reset-hard::
+ This option is only valid for the update command.
+ Hard reset current state to the commit recorded in the superproject.
+ If this option is given, the submodule's HEAD will not get detached
+ if it was not detached before. Note that, like with a regular
+ `git reset --hard` no safe-guards are in place to prevent jumping
+ to a commit which was never part of the current branch.
+ If the key `submodule.$name.update` is set to `reset-hard`, this
+ option is implicit.
+
--init::
This option is only valid for the update command.
Initialize all submodules for which "git submodule init" has not been
diff --git a/Documentation/gitmodules.txt b/Documentation/gitmodules.txt
index 312b6f9259..e085dbc01f 100644
--- a/Documentation/gitmodules.txt
+++ b/Documentation/gitmodules.txt
@@ -43,8 +43,8 @@ submodule.<name>.update::
command in the superproject. This is only used by `git
submodule init` to initialize the configuration variable of
the same name. Allowed values here are 'checkout', 'rebase',
- 'merge' or 'none'. See description of 'update' command in
- linkgit:git-submodule[1] for their meaning. Note that the
+ 'merge', 'reset-hard' or 'none'. See description of 'update' command
+ in linkgit:git-submodule[1] for their meaning. Note that the
'!command' form is intentionally ignored here for security
reasons.
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index d38113a31a..31d95c3cd6 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1481,6 +1481,7 @@ static void determine_submodule_update_strategy(struct repository *r,
if (just_cloned &&
(out->type == SM_UPDATE_MERGE ||
out->type == SM_UPDATE_REBASE ||
+ out->type == SM_UPDATE_RESET_HARD ||
out->type == SM_UPDATE_NONE))
out->type = SM_UPDATE_CHECKOUT;
@@ -1851,7 +1852,7 @@ static int update_clone(int argc, const char **argv, const char *prefix)
"submodule boundaries")),
OPT_STRING(0, "update", &update,
N_("string"),
- N_("rebase, merge, checkout or none")),
+ N_("rebase, merge, checkout, reset-hard or none")),
OPT_STRING_LIST(0, "reference", &suc.references, N_("repo"),
N_("reference repository")),
OPT_BOOL(0, "dissociate", &suc.dissociate,
diff --git a/git-submodule.sh b/git-submodule.sh
index 5e608f8bad..b5d6fad983 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -9,7 +9,7 @@ USAGE="[--quiet] add [-b <branch>] [-f|--force] [--name <name>] [--reference <re
or: $dashless [--quiet] status [--cached] [--recursive] [--] [<path>...]
or: $dashless [--quiet] init [--] [<path>...]
or: $dashless [--quiet] deinit [-f|--force] (--all| [--] <path>...)
- or: $dashless [--quiet] update [--init] [--remote] [-N|--no-fetch] [-f|--force] [--checkout|--merge|--rebase] [--[no-]recommend-shallow] [--reference <repository>] [--recursive] [--] [<path>...]
+ or: $dashless [--quiet] update [--init] [--remote] [-N|--no-fetch] [-f|--force] [--checkout|--merge|--rebase|--reset-hard] [--[no-]recommend-shallow] [--reference <repository>] [--recursive] [--] [<path>...]
or: $dashless [--quiet] summary [--cached|--files] [--summary-limit <n>] [commit] [--] [<path>...]
or: $dashless [--quiet] foreach [--recursive] <command>
or: $dashless [--quiet] sync [--recursive] [--] [<path>...]
@@ -483,6 +483,9 @@ cmd_update()
-m|--merge)
update="merge"
;;
+ --reset-hard)
+ update="reset-hard"
+ ;;
--recursive)
recursive=1
;;
@@ -621,6 +624,11 @@ cmd_update()
say_msg="$(eval_gettext "Submodule path '\$displaypath': merged in '\$sha1'")"
must_die_on_failure=yes
;;
+ reset-hard)
+ command="git reset --hard"
+ die_msg="$(eval_gettext "Unable to reset --hard to '\$sha1' in submodule path '\$displaypath'")"
+ say_msg="$(eval_gettext "Submodule path '\$displaypath': was reset --hard to '\$sha1'")"
+ ;;
!*)
command="${update_module#!}"
die_msg="$(eval_gettext "Execution of '\$command \$sha1' failed in submodule path '\$displaypath'")"
diff --git a/submodule.c b/submodule.c
index 6415cc5580..4580cf0944 100644
--- a/submodule.c
+++ b/submodule.c
@@ -373,6 +373,8 @@ enum submodule_update_type parse_submodule_update_type(const char *value)
return SM_UPDATE_REBASE;
else if (!strcmp(value, "merge"))
return SM_UPDATE_MERGE;
+ else if (!strcmp(value, "reset-hard"))
+ return SM_UPDATE_RESET_HARD;
else if (*value == '!')
return SM_UPDATE_COMMAND;
else
@@ -406,6 +408,8 @@ const char *submodule_strategy_to_string(const struct submodule_update_strategy
return "checkout";
case SM_UPDATE_MERGE:
return "merge";
+ case SM_UPDATE_RESET_HARD:
+ return "reset-hard";
case SM_UPDATE_REBASE:
return "rebase";
case SM_UPDATE_NONE:
diff --git a/submodule.h b/submodule.h
index a680214c01..f23ac4630e 100644
--- a/submodule.h
+++ b/submodule.h
@@ -29,6 +29,7 @@ enum submodule_update_type {
SM_UPDATE_CHECKOUT,
SM_UPDATE_REBASE,
SM_UPDATE_MERGE,
+ SM_UPDATE_RESET_HARD,
SM_UPDATE_NONE,
SM_UPDATE_COMMAND
};
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index e87164aa8f..2e08e0047c 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -6,7 +6,8 @@
test_description='Test updating submodules
This test verifies that "git submodule update" detaches the HEAD of the
-submodule and "git submodule update --rebase/--merge" does not detach the HEAD.
+submodule and "git submodule update --rebase/--merge/--reset-hard" does
+not detach the HEAD.
'
. ./test-lib.sh
@@ -305,6 +306,20 @@ test_expect_success 'submodule update --merge staying on master' '
)
'
+test_expect_success 'submodule update --reset-hard staying on master' '
+ (cd super/submodule &&
+ git reset --hard HEAD~1
+ ) &&
+ (cd super &&
+ (cd submodule &&
+ compare_head
+ ) &&
+ git submodule update --reset-hard submodule &&
+ cd submodule &&
+ compare_head
+ )
+'
+
test_expect_success 'submodule update - rebase in .git/config' '
(cd super &&
git config submodule.submodule.update rebase
--
2.20.0.rc2.8.g0a3bec4a1c.dirty
^ permalink raw reply related
* Re: silent_exec_failure when calling gpg
From: Junio C Hamano @ 2018-12-11 3:44 UTC (permalink / raw)
To: John Passaro; +Cc: git, Jeff King
In-Reply-To: <CAJdN7Kj5RaAsTstx_G14a_bR5Y92M3rtWAiMNPnQWgmz4JgEOg@mail.gmail.com>
John Passaro <john.a.passaro@gmail.com> writes:
> I've noticed that in v2.19.1, when using git to pretty print
> information about the signature, if git cannot find gpg (e.g. "git
> config gpg.program nogpg"), it prints an error to stderr:
>
> $ git show -s --pretty=%G?
> fatal: cannot run nogpg: No such file or directory
> N
>
> When I build from master, that no longer happens:
>
> $ ../git/git show -s --pretty=%G?
> N
>
> Is this intentional behavior, i.e. something I can count on being the
> case in future releases? Or should I treat this as a bug report?
In general, behaviour of Porcelain commands like "git show" can and
will be changed to match interactive use by humans, and should never
be relied on in your scripts.
Having said that, I think we did not mean to kill the diagnositic
message.
> This behavior makes sense in a lot of ways. If you're interested in
> verifying commit signatures, it's hard to imagine needing a reminder
> to install the program it depends on (though the error might help you
> identify bad configuration for "gpg.program").
Quite the contrary, from a single "N", you cannot immediately tell
if the commit is not signed, if your GPG is misconfigured, or if you
have a stray gpg.program that points at nowhere.
I think the uninteded behaviour change was in 17809a98 ("Merge
branch 'jk/run-command-notdot'", 2018-10-30).
Thanks for a report.
^ 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