* [PATCH 11/11] worktree remove: new command
From: Nguyễn Thái Ngọc Duy @ 2017-02-02 8:50 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <20170202085007.21418-1-pclouds@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Documentation/git-worktree.txt | 21 +++++----
builtin/worktree.c | 79 ++++++++++++++++++++++++++++++++++
contrib/completion/git-completion.bash | 5 ++-
t/t2028-worktree-move.sh | 26 +++++++++++
4 files changed, 121 insertions(+), 10 deletions(-)
diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index 13105138a7..bbde6b8110 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -14,6 +14,7 @@ SYNOPSIS
'git worktree lock' [--reason <string>] <worktree>
'git worktree move' <worktree> <new-path>
'git worktree prune' [-n] [-v] [--expire <expire>]
+'git worktree remove' [--force] <worktree>
'git worktree unlock' <worktree>
DESCRIPTION
@@ -81,6 +82,13 @@ prune::
Prune working tree information in $GIT_DIR/worktrees.
+remove::
+
+Remove a working tree. Only clean working trees (no untracked files
+and no modification in tracked files) can be removed. Unclean working
+trees can be removed with `--force`. The main working tree cannot be
+removed.
+
unlock::
Unlock a working tree, allowing it to be pruned, moved or deleted.
@@ -90,9 +98,10 @@ OPTIONS
-f::
--force::
- By default, `add` refuses to create a new working tree when `<branch>`
- is already checked out by another working tree. This option overrides
- that safeguard.
+ By default, `add` refuses to create a new working tree when
+ `<branch>` is already checked out by another working tree and
+ `remove` refuses to remove an unclean working tree. This option
+ overrides that safeguard.
-b <new-branch>::
-B <new-branch>::
@@ -253,12 +262,6 @@ Multiple checkout in general is still experimental, and the support
for submodules is incomplete. It is NOT recommended to make multiple
checkouts of a superproject.
-git-worktree could provide more automation for tasks currently
-performed manually, such as:
-
-- `remove` to remove a linked working tree and its administrative files (and
- warn if the working tree is dirty)
-
GIT
---
Part of the linkgit:git[1] suite
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 6c58d620ce..a1c91f1542 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -17,6 +17,7 @@ static const char * const worktree_usage[] = {
N_("git worktree lock [<options>] <path>"),
N_("git worktree move <worktree> <new-path>"),
N_("git worktree prune [<options>]"),
+ N_("git worktree remove [<options>] <worktree>"),
N_("git worktree unlock <path>"),
NULL
};
@@ -605,6 +606,82 @@ static int move_worktree(int ac, const char **av, const char *prefix)
return update_worktree_location(wt, dst.buf);
}
+static int remove_worktree(int ac, const char **av, const char *prefix)
+{
+ int force = 0;
+ struct option options[] = {
+ OPT_BOOL(0, "force", &force,
+ N_("force removing even if the worktree is dirty")),
+ OPT_END()
+ };
+ struct worktree **worktrees, *wt;
+ struct strbuf sb = STRBUF_INIT;
+ const char *reason;
+ int ret = 0;
+
+ ac = parse_options(ac, av, prefix, options, worktree_usage, 0);
+ if (ac != 1)
+ usage_with_options(worktree_usage, options);
+
+ worktrees = get_worktrees(0);
+ wt = find_worktree(worktrees, prefix, av[0]);
+ if (!wt)
+ die(_("'%s' is not a working directory"), av[0]);
+ if (is_main_worktree(wt))
+ die(_("'%s' is a main working directory"), av[0]);
+ reason = is_worktree_locked(wt);
+ if (reason) {
+ if (*reason)
+ die(_("already locked, reason: %s"), reason);
+ die(_("already locked, no reason"));
+ }
+ if (validate_worktree(wt, 0))
+ return -1;
+
+ if (!force) {
+ struct argv_array child_env = ARGV_ARRAY_INIT;
+ struct child_process cp;
+ char buf[1];
+
+ argv_array_pushf(&child_env, "%s=%s/.git",
+ GIT_DIR_ENVIRONMENT, wt->path);
+ argv_array_pushf(&child_env, "%s=%s",
+ GIT_WORK_TREE_ENVIRONMENT, wt->path);
+ memset(&cp, 0, sizeof(cp));
+ argv_array_pushl(&cp.args, "status", "--porcelain", NULL);
+ cp.env = child_env.argv;
+ cp.git_cmd = 1;
+ cp.dir = wt->path;
+ cp.out = -1;
+ ret = start_command(&cp);
+ if (ret)
+ die_errno(_("failed to run git-status on '%s', code %d"),
+ av[0], ret);
+ ret = xread(cp.out, buf, sizeof(buf));
+ if (ret)
+ die(_("'%s' is dirty, use --force to delete it"), av[0]);
+ close(cp.out);
+ ret = finish_command(&cp);
+ if (ret)
+ die_errno(_("failed to run git-status on '%s', code %d"),
+ av[0], ret);
+ }
+ strbuf_addstr(&sb, wt->path);
+ if (remove_dir_recursively(&sb, 0)) {
+ error_errno(_("failed to delete '%s'"), sb.buf);
+ ret = -1;
+ }
+ strbuf_reset(&sb);
+ strbuf_addstr(&sb, git_common_path("worktrees/%s", wt->id));
+ if (remove_dir_recursively(&sb, 0)) {
+ error_errno(_("failed to delete '%s'"), sb.buf);
+ ret = -1;
+ }
+ strbuf_release(&sb);
+ free_worktrees(worktrees);
+ return ret;
+}
+
int cmd_worktree(int ac, const char **av, const char *prefix)
{
struct option options[] = {
@@ -629,5 +706,7 @@ int cmd_worktree(int ac, const char **av, const char *prefix)
return unlock_worktree(ac - 1, av + 1, prefix);
if (!strcmp(av[1], "move"))
return move_worktree(ac - 1, av + 1, prefix);
+ if (!strcmp(av[1], "remove"))
+ return remove_worktree(ac - 1, av + 1, prefix);
usage_with_options(worktree_usage, options);
}
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 613e03b879..f6855af1f6 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2715,7 +2715,7 @@ _git_whatchanged ()
_git_worktree ()
{
- local subcommands="add list lock move prune unlock"
+ local subcommands="add list lock move prune remove unlock"
local subcommand="$(__git_find_on_cmdline "$subcommands")"
if [ -z "$subcommand" ]; then
__gitcomp "$subcommands"
@@ -2733,6 +2733,9 @@ _git_worktree ()
prune,--*)
__gitcomp "--dry-run --expire --verbose"
;;
+ remove,--*)
+ __gitcomp "--force"
+ ;;
*)
;;
esac
diff --git a/t/t2028-worktree-move.sh b/t/t2028-worktree-move.sh
index bef420a086..b3105eaaed 100755
--- a/t/t2028-worktree-move.sh
+++ b/t/t2028-worktree-move.sh
@@ -90,4 +90,30 @@ test_expect_success 'move main worktree' '
test_must_fail git worktree move . def
'
+test_expect_success 'remove main worktree' '
+ test_must_fail git worktree remove .
+'
+
+test_expect_success 'remove locked worktree' '
+ git worktree lock destination &&
+ test_must_fail git worktree remove destination &&
+ git worktree unlock destination
+'
+
+test_expect_success 'remove worktree with dirty tracked file' '
+ echo dirty >>destination/init.t &&
+ test_must_fail git worktree remove destination
+'
+
+test_expect_success 'remove worktree with untracked file' '
+ git -C destination checkout init.t &&
+ : >destination/untracked &&
+ test_must_fail git worktree remove destination
+'
+
+test_expect_success 'force remove worktree with untracked file' '
+ git worktree remove --force destination &&
+ test_path_is_missing destination
+'
+
test_done
--
2.11.0.157.gd943d85
^ permalink raw reply related
* [PATCH 10/11] worktree move: refuse to move worktrees with submodules
From: Nguyễn Thái Ngọc Duy @ 2017-02-02 8:50 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <20170202085007.21418-1-pclouds@gmail.com>
Submodules contains .git files with relative paths. After a worktree
move, these files need to be updated or they may point to nowhere.
This is a bandage patch to make sure "worktree move" don't break
people's worktrees by accident. When .git file update code is in
place, this validate_no_submodules() could be removed.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
builtin/worktree.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 900b68bb5d..6c58d620ce 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -525,6 +525,27 @@ static int unlock_worktree(int ac, const char **av, const char *prefix)
return ret;
}
+static void validate_no_submodules(const struct worktree *wt)
+{
+ struct index_state istate = { NULL };
+ int i, found_submodules = 0;
+
+ if (read_index_from(&istate, worktree_git_path(wt, "index")) > 0) {
+ for (i = 0; i < istate.cache_nr; i++) {
+ struct cache_entry *ce = istate.cache[i];
+
+ if (S_ISGITLINK(ce->ce_mode)) {
+ found_submodules = 1;
+ break;
+ }
+ }
+ }
+ discard_index(&istate);
+
+ if (found_submodules)
+ die(_("This working tree contains submodules and cannot be moved yet"));
+}
+
static int move_worktree(int ac, const char **av, const char *prefix)
{
struct option options[] = {
@@ -565,6 +586,8 @@ static int move_worktree(int ac, const char **av, const char *prefix)
if (validate_worktree(wt, 0))
return -1;
+ validate_no_submodules(wt);
+
if (is_directory(dst.buf)) {
const char *sep = find_last_dir_sep(wt->path);
--
2.11.0.157.gd943d85
^ permalink raw reply related
* Google Doc about the Contributors' Summit
From: Johannes Schindelin @ 2017-02-02 9:08 UTC (permalink / raw)
To: git
Hi team,
I just started typing stuff up in a Google Doc, and made it editable to
everyone, feel free to help and add things:
https://docs.google.com/document/d/1KDoSn4btbK5VJCVld32he29U0pUeFGhpFxyx9ZJDDB0/edit?usp=sharing
Ciao,
Johannes
^ permalink raw reply
* Re: [PATCH] color_parse_mem: allow empty color spec
From: Duy Nguyen @ 2017-02-02 9:16 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20170201002129.xb62hmxwrzwgp6vg@sigill.intra.peff.net>
On Wed, Feb 01, 2017 at 01:21:29AM +0100, Jeff King wrote:
> On Tue, Jan 31, 2017 at 02:45:40PM -0800, Junio C Hamano wrote:
>
> > * nd/log-graph-configurable-colors (2017-01-23) 3 commits
> > (merged to 'next' on 2017-01-23 at c369982ad8)
> > + log --graph: customize the graph lines with config log.graphColors
> > + color.c: trim leading spaces in color_parse_mem()
> > + color.c: fix color_parse_mem() with value_len == 0
> >
> > Some people feel the default set of colors used by "git log --graph"
> > rather limiting. A mechanism to customize the set of colors has
> > been introduced.
> >
> > Reported to break "add -p".
> > cf. <20170128040709.tqx4u45ktgpkbfm4@sigill.intra.peff.net>
>
> I hadn't heard anything back,
Sorry I was accidentally busy during Luna new year holiday.
> so I wrapped up my fix with a commit
> message and tests (it needs to go on top anyway, since the breakage is
> in 'next').
>
> -- >8 --
> Subject: [PATCH] color_parse_mem: allow empty color spec
>
> Prior to c2f41bf52 (color.c: fix color_parse_mem() with
> value_len == 0, 2017-01-19), the empty string was
> interpreted as a color "reset". This was an accidental
> outcome, and that commit turned it into an error.
>
> However, scripts may pass the empty string as a default
> value to "git config --get-color" to disable color when the
> value is not defined. The git-add--interactive script does
> this. As a result, the script is unusable since c2f41bf52
> unless you have color.diff.plain defined (if it is defined,
> then we don't parse the empty default at all).
..
> --- a/color.c
> +++ b/color.c
> @@ -212,8 +212,10 @@ int color_parse_mem(const char *value, int value_len, char *dst)
> len--;
> }
>
> - if (!len)
> - return -1;
> + if (!len) {
> + dst[0] = '\0';
> + return 0;
> + }
>
> if (!strncasecmp(ptr, "reset", len)) {
> xsnprintf(dst, end - dst, GIT_COLOR_RESET);
I wonder if it makes more sense to do this in builtin/config.c. The
"default value" business is strictly git-config command's. The parsing
function does not need to know. Maybe something like this?
diff --git a/builtin/config.c b/builtin/config.c
index 05843a0f96..ec1f4f0cf4 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -322,8 +322,10 @@ static void get_color(const char *var, const char *def_color)
git_config_with_options(git_get_color_config, NULL,
&given_config_source, respect_includes);
- if (!get_color_found && def_color) {
- if (color_parse(def_color, parsed_color) < 0)
+ if (!get_color_found) {
+ if (!def_color)
+ strcpy(parsed_color, GIT_COLOR_RESET);
+ else if (color_parse(def_color, parsed_color) < 0)
die(_("unable to parse default color value"));
}
This is also a good opportunity to document this behavior in
git-config.txt, I think.
--
Duy
^ permalink raw reply related
* Re: [PATCH 2/7] completion: add subcommand completion for rerere
From: Cornelius Weig @ 2017-02-02 9:16 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: bitte.keine.werbung.einwerfen, thomas.braun, john, git
In-Reply-To: <20170202005724.24754-1-szeder.dev@gmail.com>
On 02/02/2017 01:57 AM, SZEDER Gábor wrote:
> You didn't add 'rerere' to the list of porcelain commands, i.e. it
> won't be listed after 'git <TAB><TAB>'. I'm not saying it should be
> listed there, because I can't decide whether 'rerere' is considered
> porcelain or plumbing... Just wanted to make sure that this omission
> was intentional.
Yes this is intentional. There is a number of plumbing commands that
have command completion, but are not listed in 'git <Tab><Tab>' (e.g.
ls-tree, ls-files, ls-remote, ...). Given that rerere will not be
frequently invoked, I would not add it to the porcelain commands.
^ permalink raw reply
* Re: [PATCH 00/11] nd/worktree-move update
From: Johannes Schindelin @ 2017-02-02 9:16 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git, Junio C Hamano
In-Reply-To: <20170202085007.21418-1-pclouds@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 283 bytes --]
Hi Duy,
On Thu, 2 Feb 2017, Nguyễn Thái Ngọc Duy wrote:
> This squashes two changes from Johannes and Ramsay: [...]
Sorry, I lost track of the worktree discussions... Could you remind me
which patch is supposed to fix my continuous reflog corruption?
Thanks,
Dscho
^ permalink raw reply
* Re: git-daemon shallow checkout fail
From: Duy Nguyen @ 2017-02-02 9:26 UTC (permalink / raw)
To: Jeff King; +Cc: Git Mailing List
In-Reply-To: <20170130172730.x5guphyqf5fsfi7m@sigill.intra.peff.net>
On Tue, Jan 31, 2017 at 12:27 AM, Jeff King <peff@peff.net> wrote:
> On Sat, Jan 28, 2017 at 05:29:32PM -0700, Bob Proulx wrote:
>
>> However the problem driving me crazy is that this only fails this way
>> on one machine. Unfortunately failing on the machine I need to use.
>> If I try this same setup on any other machine I try then there is no
>> failure and it works okay. Therefore I conclude that in the failing
>> case it is trying to write a shallow_XXXXXX file in the repository but
>> in all of the passing cases it does not. I browsed through the
>> git-daemon source but couldn't deduce the flow yet.
>>
>> Does anyone know why one system would try to create shallow_XXXXXX
>> files in the repository while another one would not?
>
> It depends on the git version on the server. The interesting code is in
> upload-pack.c, which is spawned by git-daemon to serve a fetch or clone
> request.
>
> See commit b790e0f67 (upload-pack: send shallow info over stdin to
> pack-objects, 2014-03-11), which lays out the history. Since that commit
> (in git v2.0.0), there should be no tmpfile needed.
It must be it. There's nowhere else that upload-pack can create
shallow_XXXXXX. (receive-pack and fetch-pack can).
>> Of course git-daemon running as nobody can't create a temporary file
>> shallow_XXXXXX in the /srv/git/test-project.git because it has no
>> permissions by design. But why does this work on other systems and
>> not work on my target system?
>>
>> git --version # from today's git clone build
>> git version 2.11.0.485.g4e59582
>
> This shouldn't be happening with git v2.11. Are you sure that the "git
> daemon" invocation is running that same version? I notice you set up a
> restricted PATH. Is it possible that /usr/local/bin or /usr/bin has an
> older version of git?
One easy way to verify is clone or fetch again with GIT_TRACE_PACKET=1
since we send the server's version as a capability since 1.8.0
--
Duy
^ permalink raw reply
* Re: [PATCH 00/11] nd/worktree-move update
From: Duy Nguyen @ 2017-02-02 9:27 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <alpine.DEB.2.20.1702021015160.3496@virtualbox>
On Thu, Feb 2, 2017 at 4:16 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi Duy,
>
> On Thu, 2 Feb 2017, Nguyễn Thái Ngọc Duy wrote:
>
>> This squashes two changes from Johannes and Ramsay: [...]
>
> Sorry, I lost track of the worktree discussions... Could you remind me
> which patch is supposed to fix my continuous reflog corruption?
The corruption caused by git-gc? It's not fixed. All the changes in
this series is shown here.
--
Duy
^ permalink raw reply
* Re: [PATCH 2/2] completion: add completion for --recurse-submodules=only
From: Stefan Beller @ 2017-02-02 9:30 UTC (permalink / raw)
To: Cornelius Weig; +Cc: git@vger.kernel.org, Brandon Williams
In-Reply-To: <20170201230753.19462-2-cornelius.weig@tngtech.com>
On Wed, Feb 1, 2017 at 3:07 PM, <cornelius.weig@tngtech.com> wrote:
> From: Cornelius Weig <cornelius.weig@tngtech.com>
>
> Command completion for 'git-push --recurse-submodules' already knows to
> complete some modes. However, the recently added mode 'only' is missing.
>
> Adding 'only' to the recognized modes completes the list of non-trivial
> modes.
>
> Signed-off-by: Cornelius Weig <cornelius.weig@tngtech.com>
> ---
Looks good,
Thanks,
Stefan
> contrib/completion/git-completion.bash | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index ff7072a..fe3b0f8 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -1675,7 +1675,7 @@ _git_pull ()
> __git_complete_remote_or_refspec
> }
>
> -__git_push_recurse_submodules="check on-demand"
> +__git_push_recurse_submodules="check on-demand only"
>
> __git_complete_force_with_lease ()
> {
> --
> 2.10.2
>
^ permalink raw reply
* Re: init --separate-git-dir does not set core.worktree
From: Duy Nguyen @ 2017-02-02 9:31 UTC (permalink / raw)
To: Kyle Meyer; +Cc: Git Mailing List
In-Reply-To: <87h94d8cwi.fsf@kyleam.com>
On Thu, Feb 2, 2017 at 10:55 AM, Kyle Meyer <kyle@kyleam.com> wrote:
> Hello,
>
> As of 6311cfaf9 (init: do not set unnecessary core.worktree,
> 2016-09-25), "git init --separate-git-dir" no longer sets core.worktree
> (test below). Based on the commit message and the corresponding thread
> [1], I don't think this change in behavior was intentional, but I wasn't
> able to understand things well enough to attempt a patch.
I'm missing some context. Why does --separate-git-dir have to set
core.worktree? What fails for you exactly?
>
> Thanks.
>
> [1] https://public-inbox.org/git/CALqjkKZO_y0DNcRJjooyZ7Eso7yBMGhvZ6fE92oO4Su7JeCeng@mail.gmail.com/T/#u
>
> diff --git a/t/t0001-init.sh b/t/t0001-init.sh
> index b8fc588b1..444e75865 100755
> --- a/t/t0001-init.sh
> +++ b/t/t0001-init.sh
> @@ -309,6 +309,7 @@ test_expect_success 'init with separate gitdir' '
> git init --separate-git-dir realgitdir newdir &&
> echo "gitdir: $(pwd)/realgitdir" >expected &&
> test_cmp expected newdir/.git &&
> + check_config realgitdir false "$(pwd)/newdir" &&
> test_path_is_dir realgitdir/refs
> '
>
--
Duy
^ permalink raw reply
* Fwd: [PATCH 1/2] doc: add doc for git-push --recurse-submodules=only
From: Brandon Williams @ 2017-02-02 9:32 UTC (permalink / raw)
To: Git Mailing List
In-Reply-To: <CAKoko1q=6agpGsABxy8rmm6sGFWx9gE_c1j44h4M=yJ3r4JJBQ@mail.gmail.com>
Looks good to me! Thanks for writing the documentation. I really
need to be better about getting documentation done at the same time
I'm adding features :)
-Brandon
On Wed, Feb 1, 2017 at 3:16 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
> cornelius.weig@tngtech.com writes:
>
> > From: Cornelius Weig <cornelius.weig@tngtech.com>
> >
> > Add documentation for the `--recurse-submodules=only` option of
> > git-push. The feature was added in commit 225e8bf (add option to
> > push only submodules).
> >
> > Signed-off-by: Cornelius Weig <cornelius.weig@tngtech.com>
> > ---
> >
> > Notes:
> > This feature is already in 'next' but was undocumented. Unless somebody reads
> > the release notes, there is no way of knowing about it.
>
> Good eyes; the topic bw/push-submodule-only is already in 'master'.
>
> Looks good to me; Brandon?
>
> >
> > Documentation/git-push.txt | 13 +++++++------
> > 1 file changed, 7 insertions(+), 6 deletions(-)
> >
> > diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
> > index 8eefabd..1624a35 100644
> > --- a/Documentation/git-push.txt
> > +++ b/Documentation/git-push.txt
> > @@ -272,7 +272,7 @@ origin +master` to force a push to the `master` branch). See the
> > standard error stream is not directed to a terminal.
> >
> > --no-recurse-submodules::
> > ---recurse-submodules=check|on-demand|no::
> > +--recurse-submodules=check|on-demand|only|no::
> > May be used to make sure all submodule commits used by the
> > revisions to be pushed are available on a remote-tracking branch.
> > If 'check' is used Git will verify that all submodule commits that
> > @@ -280,11 +280,12 @@ origin +master` to force a push to the `master` branch). See the
> > remote of the submodule. If any commits are missing the push will
> > be aborted and exit with non-zero status. If 'on-demand' is used
> > all submodules that changed in the revisions to be pushed will be
> > - pushed. If on-demand was not able to push all necessary revisions
> > - it will also be aborted and exit with non-zero status. A value of
> > - 'no' or using `--no-recurse-submodules` can be used to override the
> > - push.recurseSubmodules configuration variable when no submodule
> > - recursion is required.
> > + pushed. If on-demand was not able to push all necessary revisions it will
> > + also be aborted and exit with non-zero status. If 'only' is used all
> > + submodules will be recursively pushed while the superproject is left
> > + unpushed. A value of 'no' or using `--no-recurse-submodules` can be used
> > + to override the push.recurseSubmodules configuration variable when no
> > + submodule recursion is required.
> >
> > --[no-]verify::
> > Toggle the pre-push hook (see linkgit:githooks[5]). The
^ permalink raw reply
* Re: [PATCH 4/7] completion: teach ls-remote to complete options
From: Cornelius Weig @ 2017-02-02 9:40 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: bitte.keine.werbung.einwerfen, thomas.braun, john, git
In-Reply-To: <20170202014014.25878-1-szeder.dev@gmail.com>
On 02/02/2017 02:40 AM, SZEDER Gábor wrote:
>
>> ls-remote needs to complete remote names and its own options.
>
> And refnames, too.
Yes, right. However, do you think it is reasonable to complete remote
refnames? I don't think so, because it would mean we would have to run
ls-remote during completion -- and waiting for ls-remote could be quite
lengthy.
>> In
>> addition to the existing remote name completions, do also complete
>> the options --heads, --tags, --refs, and --get-url.
>
> Why only these four options and not the other four?
>
> There are eight options in total here, so there is really no chance
> for cluttered terminals, and all eight are mentioned in the synopsis.
My line of thought is the following:
--quiet: does not print anything and is therefore only useful for
scripting. Thus, there is no need to have it on the command line completion.
--exit-code: has no visible effect and is only useful for scripting.
--upload-pack: is really exotic. Nobody will ever use it without digging
deep in the manuals. Therefore, I think it's unnecessary to have the
option completable.
--symref: Should probably be added, thanks.
However, if you don't find my reasoning for omitting the three options
above conclusive, I have no problem including them.
^ permalink raw reply
* Re: [PATCH 00/11] nd/worktree-move update
From: Johannes Schindelin @ 2017-02-02 9:43 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <CACsJy8B3bdokeYVt6aEyZVSzO50PiQRn+0sid9mSDTZ9q-mnww@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 701 bytes --]
Hi Duy,
On Thu, 2 Feb 2017, Duy Nguyen wrote:
> On Thu, Feb 2, 2017 at 4:16 PM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> > Hi Duy,
> >
> > On Thu, 2 Feb 2017, Nguyễn Thái Ngọc Duy wrote:
> >
> >> This squashes two changes from Johannes and Ramsay: [...]
> >
> > Sorry, I lost track of the worktree discussions... Could you remind me
> > which patch is supposed to fix my continuous reflog corruption?
>
> The corruption caused by git-gc? It's not fixed. All the changes in
> this series is shown here.
Oh sorry, I meant to ask "and if it is not in this patch series, would you
mind pointing me at the patch series that has that fix?"
Thanks,
Johannes
^ permalink raw reply
* Re: [PATCH 00/11] nd/worktree-move update
From: Duy Nguyen @ 2017-02-02 9:50 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <alpine.DEB.2.20.1702021043110.3496@virtualbox>
On Thu, Feb 2, 2017 at 4:43 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi Duy,
>
> On Thu, 2 Feb 2017, Duy Nguyen wrote:
>
>> On Thu, Feb 2, 2017 at 4:16 PM, Johannes Schindelin
>> <Johannes.Schindelin@gmx.de> wrote:
>> > Hi Duy,
>> >
>> > On Thu, 2 Feb 2017, Nguyễn Thái Ngọc Duy wrote:
>> >
>> >> This squashes two changes from Johannes and Ramsay: [...]
>> >
>> > Sorry, I lost track of the worktree discussions... Could you remind me
>> > which patch is supposed to fix my continuous reflog corruption?
>>
>> The corruption caused by git-gc? It's not fixed. All the changes in
>> this series is shown here.
>
> Oh sorry, I meant to ask "and if it is not in this patch series, would you
> mind pointing me at the patch series that has that fix?"
You meant this one [1]? There is nothing substantial since then.
[1] https://public-inbox.org/git/%3C20160601104519.16563-1-pclouds@gmail.com%3E/
--
Duy
^ permalink raw reply
* Re: [PATCH 00/11] nd/worktree-move update
From: Johannes Schindelin @ 2017-02-02 10:37 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <CACsJy8A-tuea7W+tj6rNddtM0j_374FODjQqKsT8eHfeZ0qDZg@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1221 bytes --]
Hi Duy,
On Thu, 2 Feb 2017, Duy Nguyen wrote:
> On Thu, Feb 2, 2017 at 4:43 PM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> >
> > On Thu, 2 Feb 2017, Duy Nguyen wrote:
> >
> >> On Thu, Feb 2, 2017 at 4:16 PM, Johannes Schindelin
> >> <Johannes.Schindelin@gmx.de> wrote:
> >> >
> >> > On Thu, 2 Feb 2017, Nguyễn Thái Ngọc Duy wrote:
> >> >
> >> >> This squashes two changes from Johannes and Ramsay: [...]
> >> >
> >> > Sorry, I lost track of the worktree discussions... Could you remind
> >> > me which patch is supposed to fix my continuous reflog corruption?
> >>
> >> The corruption caused by git-gc? It's not fixed. All the changes in
> >> this series is shown here.
> >
> > Oh sorry, I meant to ask "and if it is not in this patch series, would you
> > mind pointing me at the patch series that has that fix?"
>
> You meant this one [1]? There is nothing substantial since then.
>
> [1] https://public-inbox.org/git/%3C20160601104519.16563-1-pclouds@gmail.com%3E/
I guess I mean that.
Given that this results in real data loss, it is surprising that this has
not made it even into `pu` yet!
Would you mind rebasing and re-submitting?
Thanks,
Johannes
^ permalink raw reply
* Re: [PATCH v2 7/7] completion: recognize more long-options
From: Cornelius Weig @ 2017-02-02 10:40 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: j6t, Shawn Pearce, git
In-Reply-To: <CAM0VKjkAnsT_LE4OZRkLPuiEZW88P7_OBbOw0XovHhLYfBhbwg@mail.gmail.com>
On 02/02/2017 03:00 AM, SZEDER Gábor wrote:
>> Personally, I agree with you that
>>> Adding more long options that git commands learn along the way is
>>> always an improvement.
>> However, people may start complaining that their terminal becomes too
>> cluttered when doing a double-Tab. In my cover letter, I go to length
>> about this. My assumption was that all options that are mentioned in the
>> introduction of the command man-page should be important enough to have
>> them in the completion list.
>
> But that doesn't mean that the ones not mentioned in the synopsis
> section are not worth completing.
Absolutely. What I meant is that at least the options from the synopsis
should be contained in the set of completable options.
>> Btw, I haven't found that non-destructive options should not be eligible
>> for completion. To avoid confusion about this in the future, I suggest
>> to also change the documentation:
>>
>> index 933bb6e..96f1c7f 100644
>> --- a/contrib/completion/git-completion.bash
>> +++ b/contrib/completion/git-completion.bash
>> @@ -13,7 +13,7 @@
>> # *) git email aliases for git-send-email
>> # *) tree paths within 'ref:path/to/file' expressions
>> # *) file paths within current working directory and index
>> -# *) common --long-options
>> +# *) common non-destructive --long-options
>
> I don't mind such a change, but I don't think that list was ever meant
> to be comprehensive or decisive. It is definitely not the former, as
> it's missing several things that the completion script does support.
> OTOH, it talks about .git/remotes, which has been considered legacy
> for quite some years (though it's right, because the completion script
> still supports it).
Then let's not do that change, because for some commands destructive
long-options have been in the list of completed options for quite a
while. Given that, the above change of the documentation, might stir up
more confusion than it settles.
^ permalink raw reply
* Re: [PATCH 00/11] nd/worktree-move update
From: Duy Nguyen @ 2017-02-02 11:22 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <alpine.DEB.2.20.1702021136210.3496@virtualbox>
On Thu, Feb 2, 2017 at 5:37 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi Duy,
>
> On Thu, 2 Feb 2017, Duy Nguyen wrote:
>
>> On Thu, Feb 2, 2017 at 4:43 PM, Johannes Schindelin
>> <Johannes.Schindelin@gmx.de> wrote:
>> >
>> > On Thu, 2 Feb 2017, Duy Nguyen wrote:
>> >
>> >> On Thu, Feb 2, 2017 at 4:16 PM, Johannes Schindelin
>> >> <Johannes.Schindelin@gmx.de> wrote:
>> >> >
>> >> > On Thu, 2 Feb 2017, Nguyễn Thái Ngọc Duy wrote:
>> >> >
>> >> >> This squashes two changes from Johannes and Ramsay: [...]
>> >> >
>> >> > Sorry, I lost track of the worktree discussions... Could you remind
>> >> > me which patch is supposed to fix my continuous reflog corruption?
>> >>
>> >> The corruption caused by git-gc? It's not fixed. All the changes in
>> >> this series is shown here.
>> >
>> > Oh sorry, I meant to ask "and if it is not in this patch series, would you
>> > mind pointing me at the patch series that has that fix?"
>>
>> You meant this one [1]? There is nothing substantial since then.
>>
>> [1] https://public-inbox.org/git/%3C20160601104519.16563-1-pclouds@gmail.com%3E/
>
> I guess I mean that.
>
> Given that this results in real data loss, it is surprising that this has
> not made it even into `pu` yet!
I could rebase and clean it up a bit if you need it, but I don't
think it'll end up in 'pu' or anywhere near since Junio wanted a
cleaner approach [1]. That means (as far as I can see) a lot more work
around refs store and backend area before it's ready to handle "get
refs from this worktree store" (or "get refs from every reachable
stores").
[1] https://public-inbox.org/git/xmqqshwwzyee.fsf@gitster.mtv.corp.google.com/
--
Duy
^ permalink raw reply
* Re: [PATCH 00/11] nd/worktree-move update
From: Johannes Schindelin @ 2017-02-02 11:31 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <CACsJy8CBG_a_nX_syXKrdG2-ren=NO9CNxe6tm94FGnEo1HZLQ@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2071 bytes --]
Hi Duy,
On Thu, 2 Feb 2017, Duy Nguyen wrote:
> On Thu, Feb 2, 2017 at 5:37 PM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> >
> > On Thu, 2 Feb 2017, Duy Nguyen wrote:
> >
> >> On Thu, Feb 2, 2017 at 4:43 PM, Johannes Schindelin
> >> <Johannes.Schindelin@gmx.de> wrote:
> >> >
> >> > On Thu, 2 Feb 2017, Duy Nguyen wrote:
> >> >
> >> >> On Thu, Feb 2, 2017 at 4:16 PM, Johannes Schindelin
> >> >> <Johannes.Schindelin@gmx.de> wrote:
> >> >> >
> >> >> > On Thu, 2 Feb 2017, Nguyễn Thái Ngọc Duy wrote:
> >> >> >
> >> >> >> This squashes two changes from Johannes and Ramsay: [...]
> >> >> >
> >> >> > Sorry, I lost track of the worktree discussions... Could you
> >> >> > remind me which patch is supposed to fix my continuous reflog
> >> >> > corruption?
> >> >>
> >> >> The corruption caused by git-gc? It's not fixed. All the changes
> >> >> in this series is shown here.
> >> >
> >> > Oh sorry, I meant to ask "and if it is not in this patch series,
> >> > would you mind pointing me at the patch series that has that fix?"
> >>
> >> You meant this one [1]? There is nothing substantial since then.
> >>
> >> [1] https://public-inbox.org/git/%3C20160601104519.16563-1-pclouds@gmail.com%3E/
> >
> > I guess I mean that.
> >
> > Given that this results in real data loss, it is surprising that this
> > has not made it even into `pu` yet!
>
> I could rebase and clean it up a bit if you need it, but I don't think
> it'll end up in 'pu' or anywhere near since Junio wanted a cleaner
> approach [1]. That means (as far as I can see) a lot more work around
> refs store and backend area before it's ready to handle "get refs from
> this worktree store" (or "get refs from every reachable stores").
>
> [1] https://public-inbox.org/git/xmqqshwwzyee.fsf@gitster.mtv.corp.google.com/
That is a big, big bummer.
We are talking about a data corrupting bug here, yes? It should be
possible to do that redesign work while having a small workaround in place
that unbreaks, say, me?
Ciao,
Johannes
^ permalink raw reply
* Re: [PATCH 00/11] nd/worktree-move update
From: Johannes Schindelin @ 2017-02-02 12:33 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <alpine.DEB.2.20.1702021223320.3496@virtualbox>
Hi Duy,
On Thu, 2 Feb 2017, Johannes Schindelin wrote:
> Hi Duy,
>
> On Thu, 2 Feb 2017, Duy Nguyen wrote:
>
> > On Thu, Feb 2, 2017 at 5:37 PM, Johannes Schindelin
> > <Johannes.Schindelin@gmx.de> wrote:
> > >
> > > On Thu, 2 Feb 2017, Duy Nguyen wrote:
> > >
> > >> You meant this one [1]? There is nothing substantial since then.
> > >>
> > >> [1] https://public-inbox.org/git/%3C20160601104519.16563-1-pclouds@gmail.com%3E/
> >
> > I could rebase and clean it up a bit if you need it, but I don't
> > think it'll end up in 'pu' or anywhere near since Junio wanted a
> > cleaner approach [1]. That means (as far as I can see) a lot more work
> > around refs store and backend area before it's ready to handle "get
> > refs from this worktree store" (or "get refs from every reachable
> > stores").
> >
> > [1] https://public-inbox.org/git/xmqqshwwzyee.fsf@gitster.mtv.corp.google.com/
Given that
https://public-inbox.org/git/xmqqy46ntrhk.fsf@gitster.mtv.corp.google.com/
seems to have expected something to happen within a reasonable time frame,
and that 8 months is substantially longer than a reasonable time frame, I
am not sure that that position can still be defended.
Also, the more important reply was Peff's reply that suggested that the
proposed fix was incomplete, as it misses --unpack-unreachable:
https://public-inbox.org/git/20160601160143.GA9219@sigill.intra.peff.net/
Ciao,
Johannes
^ permalink raw reply
* Re: init --separate-git-dir does not set core.worktree
From: Kyle Meyer @ 2017-02-02 12:37 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Git Mailing List
In-Reply-To: <CACsJy8AZUBt76ZocS2JEr9FP_8Obv8L911AvZxE_sww3qXB7qw@mail.gmail.com>
Duy Nguyen <pclouds@gmail.com> writes:
> On Thu, Feb 2, 2017 at 10:55 AM, Kyle Meyer <kyle@kyleam.com> wrote:
>>
>> As of 6311cfaf9 (init: do not set unnecessary core.worktree,
>> 2016-09-25), "git init --separate-git-dir" no longer sets core.worktree
>> (test below). Based on the commit message and the corresponding thread
>> [1], I don't think this change in behavior was intentional, but I wasn't
>> able to understand things well enough to attempt a patch.
>
> I'm missing some context. Why does --separate-git-dir have to set
> core.worktree? What fails for you exactly?
Sorry for not providing enough information. I haven't run into a
failure.
In Magit, we need to determine the top-level of the working tree from
COMMIT_EDITMSG. Right now that logic [*1*] looks something like this:
* COMMIT_EDITMSG in .git/modules/<module>/: set working tree to the
output of "git rev-parse --show-toplevel"
* COMMIT_EDITMSG in .git/worktrees/<wtree>/: set working tree to the
path in .git/worktrees/<wtree>/gitdir, minus the trailing "/.git"
* COMMIT_EDITMSG in .git: set working tree to the parent directory
This fails for a repo set up with --separate-git-dir [*2*], where the
last step will go out into an unrelated repo. If core.worktree was set
and "git rev-parse --show-toplevel" returned the working tree like it
did for submodules, things would work.
Of course, the issue above isn't a reason that --separate-git-dir should
set core.worktree, but the submodule behavior is why we were wondering
if it should. And so I was going to ask here whether core.worktree
should be set, but then I confused myself into thinking 6311cfaf9
unintentionally changed this behavior.
[*1*] This is done by magit-toplevel:
https://github.com/magit/magit/blob/e34f4e8eb00f292e8c475489fa7caa286857a421/lisp/magit-git.el#L400
[*2*] https://github.com/magit/magit/issues/2955
https://github.com/magit/magit/issues/2981
^ permalink raw reply
* [PATCH] document behavior of empty color name
From: Jeff King @ 2017-02-02 12:42 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Junio C Hamano, git
In-Reply-To: <20170202091615.GA22337@ash>
On Thu, Feb 02, 2017 at 04:16:15PM +0700, Duy Nguyen wrote:
> > I hadn't heard anything back,
>
> Sorry I was accidentally busy during Luna new year holiday.
No problem. That sounds much more fun than working on Git. :)
> > - if (!len)
> > - return -1;
> > + if (!len) {
> > + dst[0] = '\0';
> > + return 0;
> > + }
> >
> > if (!strncasecmp(ptr, "reset", len)) {
> > xsnprintf(dst, end - dst, GIT_COLOR_RESET);
>
> I wonder if it makes more sense to do this in builtin/config.c. The
> "default value" business is strictly git-config command's. The parsing
> function does not need to know. Maybe something like this?
I don't think so. The default value is a git-config thing, but you would
want to be able to do the same thing in a config file. For example, to
disable coloring entirely for part of the diff, you could do:
[color "diff"]
meta = ""
> This is also a good opportunity to document this behavior in
> git-config.txt, I think.
Yeah. Maybe:
-- >8 --
Subject: [PATCH] document behavior of empty color name
Commit 55cccf4bb (color_parse_mem: allow empty color spec,
2017-02-01) clearly defined the behavior of an empty color
config variable. Let's document that, and give a hint about
why it might be useful.
It's important not to say that it makes the item uncolored,
because it doesn't. It just sets no attributes, which means
that any previous attributes continue to take effect.
Signed-off-by: Jeff King <peff@peff.net>
---
Documentation/config.txt | 3 +++
1 file changed, 3 insertions(+)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 33a007b52..49b264566 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -170,6 +170,9 @@ The position of any attributes with respect to the colors
be turned off by prefixing them with `no` or `no-` (e.g., `noreverse`,
`no-ul`, etc).
+
+An empty color string produces no color effect at all. This can be used
+to avoid coloring specific elements without disabling color entirely.
++
For git's pre-defined color slots, the attributes are meant to be reset
at the beginning of each item in the colored output. So setting
`color.decorate.branch` to `black` will paint that branch name in a
--
2.11.0.948.gca97da533
^ permalink raw reply related
* Re: [PATCH 4/7] completion: teach ls-remote to complete options
From: Junio C Hamano @ 2017-02-02 16:57 UTC (permalink / raw)
To: Cornelius Weig
Cc: SZEDER Gábor, bitte.keine.werbung.einwerfen, thomas.braun,
john, git
In-Reply-To: <603697da-c8e9-5644-e0f0-7ee265c069d8@tngtech.com>
Cornelius Weig <cornelius.weig@tngtech.com> writes:
> On 02/02/2017 02:40 AM, SZEDER Gábor wrote:
>>
>>> ls-remote needs to complete remote names and its own options.
>>
>> And refnames, too.
>
> Yes, right. However, do you think it is reasonable to complete remote
> refnames? I don't think so, because it would mean we would have to run
> ls-remote during completion -- and waiting for ls-remote could be quite
> lengthy.
... and by the time the completion script knew what they are, we
have all information necessary without actually having the user run
the command ;-) That does sound backwards.
I am however not sure what Szeder really meant by "refnames". For
example, you may want to see that this
$ git ls-remote origin mast<TAG>
peek into refs/remotes/origin/* and find those matching, i.e. most
likely "master", and that can be done without talking to the remote
side. It won't catch the case where the remote end added a new
branch that also match, e.g. "mastiff", and it will actively harm
the users by giving the impression that Git (collectively, even
though from technical point of view, what the completion script does
is not part of Git) told them that there is no such new branch they
need to worry about.
So probably even with the "peek local" optimization, I have a feeling
that completion of refnames may not be such a good idea.
^ permalink raw reply
* How to use git show's "%<(<N>[,trunc|ltrunc|mtrunc])"?
From: Hilco Wijbenga @ 2017-02-02 17:51 UTC (permalink / raw)
To: Git Users
Hi all,
I'm trying to get the committer date printed in a custom fashion.
Using "%cI" gets me close:
$ git show --format="%cI | %an" master | head -n 1
2017-01-31T17:02:13-08:00 | Hilco Wijbenga
I would like to get rid of the "-08:00" bit at the end of the
timestamp. According to the "git show" manual I should be able to use
"%<(<N>[,trunc|ltrunc|mtrunc])" to drop that last part.
$ git show --format="%<(19,trunc)cI | %an" master | head -n 1
cI | Hilco Wijbenga
Mmm, it seems to be recognized as a valid "something" but it's not
working as I had expected. :-) I tried several other versions of this
but no luck. Clearly, I'm misunderstanding the format description. How
do I get "2017-01-31T17:02:13 | Hilco Wijbenga" to be output?
Cheers,
Hilco
^ permalink raw reply
* Re: [PATCH v3 00/27] Revamp the attribute system; another round
From: Junio C Hamano @ 2017-02-02 19:14 UTC (permalink / raw)
To: Brandon Williams; +Cc: git, sbeller, pclouds
In-Reply-To: <20170128020207.179015-1-bmwill@google.com>
Brandon Williams <bmwill@google.com> writes:
> Per some of the discussion online and off I locally broke up up the question
> and answer and I wasn't very thrilled with the outcome for a number of reasons.
>
> 1. The API is more complex....
> 2. Performance hit....
> ...
> Given the above, v3 is a reroll of the same design as in v2. This is a good
> milestone in improving the attribute system as it achieves the goal of making
> the attribute subsystem thread-safe (ie multiple callers can be executing
> inside the attribute system at the same time) and will enable a future series
> to allow pathspec code to call into the attribute system.
>
> Most of the changes in this revision are cosmetic (variable renames, code
> movement, etc) but there was a memory leak that was also fixed.
I am OK with the patches presented in this round, but let me explain
why I still expect that we would eventually end up spliting the
question & answer into separate data structure before we can truly
go multi-threaded.
A typical application would do
for path taken from some set:
do_something(path)
and "do something with path" would be another helper function, which
may do
do_something(path):
ask 'text' attribute for the path
switch on the attribute and do different things
With the original API, the latter would statically allocate an array
of <question, answer> pairs, with an optimization to populate
<question> which is immutable (because the codepath is always and
only interested in 'text' attribute, and you need a hash lookup to
intern the string "text" which costs cycles) only once, and make a
call to git_check_attr() function with the "path". This obviously
will not work when two threads are calling this helper function, as
the threads both want their git_check_attr() to return their answers
to the array, but the <answer> part are shared between the threads.
A naive and inefficient way to split questions and answers is to
have two arrays, allocating the former statically (protected under a
mutex, of course) to avoid repeated cost of interning, while
allocating the latter (and some working area per invocation, like
the check_all_attr[]) dynamically and place it on stack or heap.
Because do_something() will be called number of times, the cost for
allocation and initialization of the <answer> part that is paid per
invocation will of course become very high.
We could in theory keep the original arrangement of having an
array of <question, answer> pairs and restructure the code to do:
prepare the <question, answer> array
for path taken from some set:
do_something(the array, path)
That way, do_something() do not have to keep allocating,
initializing and destroying the array.
But after looking at the current set of codepaths, before coming to
the conclusion that we need to split the static part that is
specific to the callsite for git_check_attr() and the dynamic part
that is specific to the <callsite, thread> pair, I noticed that
typically the callers that can prepare the array before going into
the loop (which will eventually be spread across multiple threads)
are many levels away in the callchain, and they are not even aware
of what attributes are going to be requested in the leaf level
helper functions. In other words, the approach to hoist "the
<question, answer> array" up in the callchain would not scale. A
caller that loops over paths in the index and check them out does
not want to know (and we do not want to tell it) what exact
attributes are involved in the decision convert_to_working_tree()
makes for each path, for example.
So how would we split questions and answers in a way that is not
naive and inefficient?
I envision that we would allow the attribute subsystem to keep track
of the dynamic part, which will receive the answers, holds working
area like check_all_attr[], and has the equivalent to the "attr
stack", indexed by <thread-id, callsite> pair (and the
identification of "callsite" can be done by using the address of the
static part, i.e. the array of questions that we initialize just
once when interning the list of attribute names for the first time).
The API to prepare and ask for attributes may look like:
static struct attr_static_part Q;
struct attr_dynamic_part *D;
attr_check_init(&Q, "text", ...);
D = git_attr_check(&Q, path);
where Q contains an array of interned attributes (i.e. questions)
and other immutable things that is unique to this callsite, but can
be shared across multiple threads asking the same question from
here. As an internal implementation detail, it probably will have a
mutex to make sure that init will run only once.
Then the implementation of git_attr_check(&Q, path) would be:
- see if there is already the "dynaic part" allocated for the
current thread asking the question Q. If there is not,
allocate one and remember it, so that it can be reused in
later calls by the same thread; if there is, use that existing
one.
- reinitialize the "dynamic part" as needed, e.g. clear the
equivalent to check_all_attr[], adjust the equivalent to
attr_stack for the current path, etc. Just like the current
code optimizes for the case where the entire program (a single
thread) will ask the same question for paths in traversal
order (i.e. falling in the same directory), this will optimize
for the access pattern where each thread asks the same
question for paths in its traversal order.
- do what the current collect_some_attrs() thing does.
And this hopefully won't be as costly as the naive and inefficient
one.
The reason why I was pushing hard to split the static part and the
dynamic part in our redesign of the API is primarily because I
didn't want to update the API callers twice. But I'd imagine that
your v3 (and your earlier "do not discard attr stack, but keep them
around, holding their tips in a hashmap for quick reuse") would at
least lay the foundation for the eventual shape of the API, let's
bite the bullet and accept that we will need to update the callers
again anyway.
Thanks.
^ permalink raw reply
* [PATCH] ls-remote: add "--diff" option to show only refs that differ
From: Linus Torvalds @ 2017-02-02 19:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Thu, 2 Feb 2017 11:37:49 -0800
Subject: [PATCH] ls-remote: add "--diff" option to show only refs that differ
My main use of "git ls-remote" tends to be to check what the other end
has when some pull request goes wrong (they forgot to push, or they used
the wrong ref name or whatever), and it ends up being hard to see all
the relevant data from the noise of people just having the same basic
tags etc from upstream.
So this adds a "--diff" option that shows only the refs that are
different from the local repository. So when somebody asks me to pull,
I can now just trivially look at what they have that isn't already my
basic branches and tags.
Note that "--diff" implies "--refs" (ie it also disables showing peeled
tags).
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
---
This is not a big deal, but maybe others have the same issues I've had.
And maybe nobody else ever uses "git ls-remote". I dunno.
Also, I considered adding this feature as a more generic flag to
"check_ref_type()" (ie add a REF_NONLOCAL option to complete the existing
REF_NORMAL/REF_HEAD/etc flags), but that would have been a more involved
patch and I'm not convinced it makes much sense for any other use, so I
made it a specific local hack to ls-remote instead.
Comments?
builtin/ls-remote.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
index 66cdd45cc..23469c3a6 100644
--- a/builtin/ls-remote.c
+++ b/builtin/ls-remote.c
@@ -2,6 +2,7 @@
#include "cache.h"
#include "transport.h"
#include "remote.h"
+#include "refs.h"
static const char * const ls_remote_usage[] = {
N_("git ls-remote [--heads] [--tags] [--refs] [--upload-pack=<exec>]\n"
@@ -31,6 +32,16 @@ static int tail_match(const char **pattern, const char *path)
return 0;
}
+static int has_ref_locally(const struct ref *ref)
+{
+ unsigned char sha1[20];
+
+ if (!resolve_ref_unsafe(ref->name, RESOLVE_REF_READING, sha1, NULL))
+ return 0;
+
+ return !hashcmp(ref->old_oid.hash, sha1);
+}
+
int cmd_ls_remote(int argc, const char **argv, const char *prefix)
{
const char *dest = NULL;
@@ -39,6 +50,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
int quiet = 0;
int status = 0;
int show_symref_target = 0;
+ int diff = 0;
const char *uploadpack = NULL;
const char **pattern = NULL;
@@ -62,6 +74,8 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
N_("exit with exit code 2 if no matching refs are found"), 2),
OPT_BOOL(0, "symref", &show_symref_target,
N_("show underlying ref in addition to the object pointed by it")),
+ OPT_BOOL(0, "diff", &diff,
+ N_("show only refs that differ from local refs")),
OPT_END()
};
@@ -98,6 +112,8 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
if (transport_disconnect(transport))
return 1;
+ if (diff)
+ flags |= REF_NORMAL;
if (!dest && !quiet)
fprintf(stderr, "From %s\n", *remote->url);
for ( ; ref; ref = ref->next) {
@@ -105,6 +121,8 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
continue;
if (!tail_match(pattern, ref->name))
continue;
+ if (diff && has_ref_locally(ref))
+ continue;
if (show_symref_target && ref->symref)
printf("ref: %s\t%s\n", ref->symref, ref->name);
printf("%s\t%s\n", oid_to_hex(&ref->old_oid), ref->name);
^ permalink raw reply related
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