* Re: [PATCHv3] rev-parse: add --show-superproject-working-tree
From: Junio C Hamano @ 2017-03-08 22:28 UTC (permalink / raw)
To: Stefan Beller; +Cc: szeder.dev, email, git, sandals, ville.skytta
In-Reply-To: <20170308192037.21847-1-sbeller@google.com>
Stefan Beller <sbeller@google.com> writes:
> +--show-superproject-working-tree
> + Show the absolute path of the top-level directory of
> + the superproject. A superproject is a repository that records
> + this repository as a submodule.
The top-level directory of the superproject's working tree?
Describe error conditions, like "you get an error if there is no
such project"?
> diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
> index e08677e559..2549643267 100644
> --- a/builtin/rev-parse.c
> +++ b/builtin/rev-parse.c
> @@ -12,6 +12,7 @@
> #include "diff.h"
> #include "revision.h"
> #include "split-index.h"
> +#include "submodule.h"
>
> #define DO_REVS 1
> #define DO_NOREV 2
> @@ -779,6 +780,12 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
> puts(work_tree);
> continue;
> }
> + if (!strcmp(arg, "--show-superproject-working-tree")) {
> + const char *superproject = get_superproject_working_tree();
> + if (superproject)
> + puts(superproject);
> + continue;
When is superproject NULL? Shouldn't we die with "there is no
superproject that uses us as a submodule"?
I can accept "it is not an error to ask for the root of the
superproject's working tree when we do not have any---that is one
way to ask if we have a superproject or not". But if that is the
case, the code can stay the same, but the documentation should say
so, something like...
Show the absolute path of the root of the superproject's
working tree (if exists) that uses the current repository as
its submodule. Outputs nothing if the current repository is
not used as a submodule by any project.
^ permalink raw reply
* [PATCH] branch: honor --abbrev/--no-abbrev in --list mode
From: Junio C Hamano @ 2017-03-08 22:16 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak, Guillaume Wenzek
In-Reply-To: <xmqqzigv1lc3.fsf@gitster.mtv.corp.google.com>
When the "branch --list" command was converted to use the --format
facility from the ref-filter API, we forgot to honor the --abbrev
setting in the default output format and instead used a hardcoded
"7".
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* This time with test. I am building directly this fix on top of
the kn/ref-filter-branch-test topic that was merged to 'master'
after 2.12, and haven't checked if there will be conflicts with
other topics in-flight (I am expecting none, but I do not know
until I start today's integration cycle).
builtin/branch.c | 19 +++++++++++++++----
t/t3200-branch.sh | 25 +++++++++++++++++++++++++
2 files changed, 40 insertions(+), 4 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index cbaa6d03c0..537c47811a 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -335,9 +335,18 @@ static char *build_format(struct ref_filter *filter, int maxwidth, const char *r
branch_get_color(BRANCH_COLOR_CURRENT));
if (filter->verbose) {
+ struct strbuf obname = STRBUF_INIT;
+
+ if (filter->abbrev < 0)
+ strbuf_addf(&obname, "%%(objectname:short)");
+ else if (!filter->abbrev)
+ strbuf_addf(&obname, "%%(objectname)");
+ else
+ strbuf_addf(&obname, " %%(objectname:short=%d) ", filter->abbrev);
+
strbuf_addf(&local, "%%(align:%d,left)%%(refname:lstrip=2)%%(end)", maxwidth);
strbuf_addf(&local, "%s", branch_get_color(BRANCH_COLOR_RESET));
- strbuf_addf(&local, " %%(objectname:short=7) ");
+ strbuf_addf(&local, " %s ", obname.buf);
if (filter->verbose > 1)
strbuf_addf(&local, "%%(if)%%(upstream)%%(then)[%s%%(upstream:short)%s%%(if)%%(upstream:track)"
@@ -346,10 +355,12 @@ static char *build_format(struct ref_filter *filter, int maxwidth, const char *r
else
strbuf_addf(&local, "%%(if)%%(upstream:track)%%(then)%%(upstream:track) %%(end)%%(contents:subject)");
- strbuf_addf(&remote, "%s%%(align:%d,left)%s%%(refname:lstrip=2)%%(end)%s%%(if)%%(symref)%%(then) -> %%(symref:short)"
- "%%(else) %%(objectname:short=7) %%(contents:subject)%%(end)",
+ strbuf_addf(&remote, "%s%%(align:%d,left)%s%%(refname:lstrip=2)%%(end)%s"
+ "%%(if)%%(symref)%%(then) -> %%(symref:short)"
+ "%%(else) %s %%(contents:subject)%%(end)",
branch_get_color(BRANCH_COLOR_REMOTE), maxwidth, quote_literal_for_format(remote_prefix),
- branch_get_color(BRANCH_COLOR_RESET));
+ branch_get_color(BRANCH_COLOR_RESET), obname.buf);
+ strbuf_release(&obname);
} else {
strbuf_addf(&local, "%%(refname:lstrip=2)%s%%(if)%%(symref)%%(then) -> %%(symref:short)%%(end)",
branch_get_color(BRANCH_COLOR_RESET));
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 8a833f354e..39bd5ac8fa 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -207,6 +207,31 @@ test_expect_success 'git branch --list -d t should fail' '
test_path_is_missing .git/refs/heads/t
'
+test_expect_success 'git branch --list -v with --abbrev' '
+ test_when_finished "git branch -D t" &&
+ git branch t &&
+ git branch -v --list t >actual.default &&
+ git branch -v --list --abbrev t >actual.abbrev &&
+ test_cmp actual.default actual.abbrev &&
+
+ git branch -v --list --no-abbrev t >actual.noabbrev &&
+ git branch -v --list --abbrev=0 t >actual.0abbrev &&
+ test_cmp actual.noabbrev actual.0abbrev &&
+
+ git branch -v --list --abbrev=36 t >actual.36abbrev &&
+ # how many hexdigits are used?
+ read name objdefault rest <actual.abbrev &&
+ read name obj36 rest <actual.36abbrev &&
+ objfull=$(git rev-parse --verify t) &&
+
+ # are we really getting abbreviations?
+ test "$obj36" != "$objdefault" &&
+ expr "$obj36" : "$objdefault" >/dev/null &&
+ test "$objfull" != "$obj36" &&
+ expr "$objfull" : "$obj36" >/dev/null
+
+'
+
test_expect_success 'git branch --column' '
COLUMNS=81 git branch --column=column >actual &&
cat >expected <<\EOF &&
--
2.12.0.246.ga2ecc84866-goog
^ permalink raw reply related
* Re: [PATCH v3 0/9] Fix the early config
From: Junio C Hamano @ 2017-03-08 22:43 UTC (permalink / raw)
To: Jeff King; +Cc: Johannes Schindelin, git, Duy Nguyen
In-Reply-To: <20170308174237.cm6e5uvve6hu7lpf@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
>> Or are you discussing a more general issue, iow, anything that can
>> work without repository (i.e. those who do _gently version of the
>> setup and act on *nongit_ok) should pretend as if there were no
>> (broken) repository and take the "no we are not in a repository"
>> codepath?
>
> Yes, exactly. It would have been less confusing if I picked something
> that passed nongit_ok. Like hash-object:
>
> $ git init
> $ echo content >file
> $ git hash-object file
> d95f3ad14dee633a758d2e331151e950dd13e4ed
>
> $ echo '[core]repositoryformatversion = 10' >.git/config
> $ git hash-object file
> warning: Expected git repo version <= 1, found 10
> d95f3ad14dee633a758d2e331151e950dd13e4ed
>
> The warning is fine and reasonable here. But then:
>
> $ echo '[core]repositoryformatversion = foobar' >.git/config
> $ git hash-object file
> fatal: bad numeric config value 'foobar' for 'core.repositoryformatversion' in file .git/config: invalid unit
>
> That's wrong. We're supposed to be gentle. And ditto:
>
> $ echo '[co' >.git/config
> $ git hash-object file
> fatal: bad config line 1 in file .git/config
>
> Those last two should issue a warning at most, and then let the command
> continue.
Yeah, I agree with that as one of the worthy goals. IIUC, we
decided to leave that outside of this series and later fix on top,
which is fine by me, too.
^ permalink raw reply
* Re: diff.ignoreSubmoudles config setting broken?
From: Stefan Beller @ 2017-03-08 22:11 UTC (permalink / raw)
To: Junio C Hamano
Cc: Sebastian Schuberth, Jacob Keller, Jeff King, Git Mailing List,
Jens Lehmann
In-Reply-To: <xmqq8tof32x9.fsf@gitster.mtv.corp.google.com>
On Wed, Mar 8, 2017 at 12:54 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>> On Wed, Mar 8, 2017 at 7:07 AM, Sebastian Schuberth
>> <sschuberth@gmail.com> wrote:
>>>
>>> + Jens
>>>
>>
>> + Jacob Keller, who touched submodule diff display code last.
>> (I am thinking of fd47ae6a, diff: teach diff to display submodule
>> difference with an inline diff, 2016-08-31), which is first release as
>> part of v2.11.0 (that would fit your observance)
>
> Between these two:
>
> git -c diff.ignoresubmodules=all diff
> git diff --ignore-submodules=all
>
> one difference is that the latter disables reading extra config from
> .gitmodules (!) while the former makes the command honor it.
>
Yeah the .gitmodules file is a good hint.
Here is my understanding of the precedence:
command line options > .git/config (in various forms) > .gitmodules
where in the .git config we have precedence levels for different files
.git/config > ~/.gitconfig
as well as different settings:
submodule.<name>.ignore > diff.ignoreSubmodules
It is not clear to me how a specific setting in .gitmodules
would interact with a submodule-global setting in the config,
e.g.
git config -f .gitmodules submodule. \
"$(git submodule--helper name scanners/scancode-toolkit)". \
ignore none
git config diff.ignoreSubmodules all
git diff scanners/scancode-toolkit
From reading the code, I assume "diff.ignoreSubmodules all"
takes precedence here nevertheless because the diff.ignoreSubmodules
setting is treated on a higher level than the submodule specific setting,
despite the submodule specific setting being more specific.
This is a bad example, because it may be intuitive that the
value in the .git/config file takes precedence over .gitmodules,
but we cannot set diff.ignoreSubmodules in .gitmodules.
Stefan
^ permalink raw reply
* Re: [PATCH] branch & tag: Add a --no-contains option
From: Junio C Hamano @ 2017-03-08 23:02 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: git, Lars Hjemli, Jeff King, Christian Couder
In-Reply-To: <20170308202025.17900-1-avarab@gmail.com>
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> More notes about this patch:
>
> * I'm not really happy with the "special attention" documentation
> example in git-branch.txt, but it follows logically from the
> description for --contains just above it which I think is overly
> specific as well. IMO that entire NOTES section in git-branch.txt
> could just be removed.
The first paragraph of the section is unrelated to the topic and I
do not think anybody would miss it if it goes, but I always feel
uncomfortable between --contains and --merged. I do not expect
anybody needs lengthy explanation to tell --merged and --no-merged
(similarly --contains and --no-contains) apart, but perhaps because
I often use --with (which is a hidden synonym to --contains) and
almost never --merged, and as we are adding the fourth, I find it a
very good idea to extend the description to tell users what they
want to use "contains" for (i.e. find the set of containers given a
commit) and what they want to use "merged" for (i.e. find the set of
containees given a commit).
> * I'm adding a --without option as an alias for --no-contains for
> consistency with --with and --contains. Since we don't even
> document --with anymore (or test it) perhaps we shouldn't be adding
> --without.
I do not think anybody other than me uses "--with" to begin with, so
I do not care too deeply about it. If it makes the patch simpler
not to support "--without", I'd be supportive if you want to drop it.
I'll review the body of the patch later.
Thanks.
^ permalink raw reply
* Re: [PATCH] submodule--helper.c: remove duplicate code
From: Stefan Beller @ 2017-03-08 23:15 UTC (permalink / raw)
To: Valery Tolstov; +Cc: git@vger.kernel.org
In-Reply-To: <20170308230512.30572-1-me@vtolstov.org>
On Wed, Mar 8, 2017 at 3:05 PM, Valery Tolstov <me@vtolstov.org> wrote:
>> Then the next step (as outlined by Documentation/SubmittingPatches)
>> is to figure out how to best present this to the mailing list; I think the best
>> way is to send out a patch series consisting of both of these 2 patches,
>> the "connect_work_tree_and_git_dir: safely create leading directories,"
>> first and then your deduplication patch.
>
> Is there a handy way to forward your patch in new patch series?
No there is not. :(
Here is what I would do:
* Take the patch and "git am" it (you probably did that for testing already.)
* use "git rebase --interactive" to get the order right,
* use "git format-patch HEAD^^ --cover-letter" to create the series.
(This will take care of e.g. rewriting the patch to be numbered correctly)
* use "git send-email 00* --to=list --cc=Junio ..." etc to send it out
as a new series.
Alternatively you can take the patch file you sent and that patch and
manually edit their numbering and send them out.
(PATCH 1/2 and PATCH 2/2)
> Also,
> should I start new thread for new patch series?
As you like.
As far as I understand, it is very easy for Junio to take a whole
(sub-)thread of patches and apply that and make a branch with
multiple commits out of it as he has tooling for that.
>
> Regards,
> Valery Tolstov
Thanks,
Stefan
^ permalink raw reply
* Re: diff.ignoreSubmoudles config setting broken?
From: Junio C Hamano @ 2017-03-08 23:08 UTC (permalink / raw)
To: Stefan Beller
Cc: Sebastian Schuberth, Jacob Keller, Jeff King, Git Mailing List,
Jens Lehmann
In-Reply-To: <CAGZ79kZFGP0zMP5CtOH3poU9vx8FoT25UVr8ridQo0_VdH2cmA@mail.gmail.com>
Stefan Beller <sbeller@google.com> writes:
> Yeah the .gitmodules file is a good hint.
>
> Here is my understanding of the precedence:
>
> command line options > .git/config (in various forms) > .gitmodules
>
> where in the .git config we have precedence levels for different files
>
> .git/config > ~/.gitconfig
>
> as well as different settings:
>
> submodule.<name>.ignore > diff.ignoreSubmodules
I've never understood why people thought it a good idea to let
.gitmodules supplied by the upstream override the configuration
setting the end user has like this. This is quite bad.
Perhaps this is a good starting point?
diff.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/diff.c b/diff.c
index a628ac3a95..75b7140c63 100644
--- a/diff.c
+++ b/diff.c
@@ -273,8 +273,11 @@ int git_diff_ui_config(const char *var, const char *value, void *cb)
if (!strcmp(var, "diff.orderfile"))
return git_config_pathname(&diff_order_file_cfg, var, value);
- if (!strcmp(var, "diff.ignoresubmodules"))
+ if (!strcmp(var, "diff.ignoresubmodules")) {
handle_ignore_submodules_arg(&default_diff_options, value);
+ DIFF_OPT_SET(options, OVERRIDE_SUBMODULE_CONFIG);
+ return 0;
+ }
if (!strcmp(var, "diff.submodule")) {
if (parse_submodule_params(&default_diff_options, value))
^ permalink raw reply related
* [PATCHv4] rev-parse: add --show-superproject-working-tree
From: Stefan Beller @ 2017-03-08 23:07 UTC (permalink / raw)
To: szeder.dev, email, git, sandals, ville.skytta; +Cc: Stefan Beller
In-Reply-To: <xmqqmvcv1jz9.fsf@gitster.mtv.corp.google.com>
In some situations it is useful to know if the given repository
is a submodule of another repository.
Add the flag --show-superproject-working-tree to git-rev-parse
to make it easy to find out if there is a superproject. When no
superproject exists, the output will be empty.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
> I can accept "it is not an error to ask for the root of the
> superproject's working tree when we do not have any---that is one
> way to ask if we have a superproject or not". But if that is the
> case, the code can stay the same, but the documentation should say
> so, something like...
*this patch*
* Documentation shamelessly stolen from Junio.
Thanks,
Stefan
Documentation/git-rev-parse.txt | 6 +++
builtin/rev-parse.c | 7 ++++
submodule.c | 82 +++++++++++++++++++++++++++++++++++++++++
submodule.h | 8 ++++
t/t1500-rev-parse.sh | 14 +++++++
5 files changed, 117 insertions(+)
diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
index 91c02b8c85..c40c470448 100644
--- a/Documentation/git-rev-parse.txt
+++ b/Documentation/git-rev-parse.txt
@@ -261,6 +261,12 @@ print a message to stderr and exit with nonzero status.
--show-toplevel::
Show the absolute path of the top-level directory.
+--show-superproject-working-tree
+ Show the absolute path of the root of the superproject's
+ working tree (if exists) that uses the current repository as
+ its submodule. Outputs nothing if the current repository is
+ not used as a submodule by any project.
+
--shared-index-path::
Show the path to the shared index file in split index mode, or
empty if not in split-index mode.
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index e08677e559..2549643267 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -12,6 +12,7 @@
#include "diff.h"
#include "revision.h"
#include "split-index.h"
+#include "submodule.h"
#define DO_REVS 1
#define DO_NOREV 2
@@ -779,6 +780,12 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
puts(work_tree);
continue;
}
+ if (!strcmp(arg, "--show-superproject-working-tree")) {
+ const char *superproject = get_superproject_working_tree();
+ if (superproject)
+ puts(superproject);
+ continue;
+ }
if (!strcmp(arg, "--show-prefix")) {
if (prefix)
puts(prefix);
diff --git a/submodule.c b/submodule.c
index 3b98766a6b..bb405653fd 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1514,3 +1514,85 @@ void absorb_git_dir_into_superproject(const char *prefix,
strbuf_release(&sb);
}
}
+
+const char *get_superproject_working_tree(void)
+{
+ struct child_process cp = CHILD_PROCESS_INIT;
+ struct strbuf sb = STRBUF_INIT;
+ const char *one_up = real_path_if_valid("../");
+ const char *cwd = xgetcwd();
+ const char *ret = NULL;
+ const char *subpath;
+ int code;
+ ssize_t len;
+
+ if (!is_inside_work_tree())
+ /*
+ * FIXME:
+ * We might have a superproject, but it is harder
+ * to determine.
+ */
+ return NULL;
+
+ if (!one_up)
+ return NULL;
+
+ subpath = relative_path(cwd, one_up, &sb);
+
+ prepare_submodule_repo_env(&cp.env_array);
+ argv_array_pop(&cp.env_array);
+
+ argv_array_pushl(&cp.args, "--literal-pathspecs", "-C", "..",
+ "ls-files", "-z", "--stage", "--full-name", "--",
+ subpath, NULL);
+ strbuf_reset(&sb);
+
+ cp.no_stdin = 1;
+ cp.no_stderr = 1;
+ cp.out = -1;
+ cp.git_cmd = 1;
+
+ if (start_command(&cp))
+ die(_("could not start ls-files in .."));
+
+ len = strbuf_read(&sb, cp.out, PATH_MAX);
+ close(cp.out);
+
+ if (starts_with(sb.buf, "160000")) {
+ int super_sub_len;
+ int cwd_len = strlen(cwd);
+ char *super_sub, *super_wt;
+
+ /*
+ * There is a superproject having this repo as a submodule.
+ * The format is <mode> SP <hash> SP <stage> TAB <full name> \0,
+ * We're only interested in the name after the tab.
+ */
+ super_sub = strchr(sb.buf, '\t') + 1;
+ super_sub_len = sb.buf + sb.len - super_sub - 1;
+
+ if (super_sub_len > cwd_len ||
+ strcmp(&cwd[cwd_len - super_sub_len], super_sub))
+ die (_("BUG: returned path string doesn't match cwd?"));
+
+ super_wt = xstrdup(cwd);
+ super_wt[cwd_len - super_sub_len] = '\0';
+
+ ret = real_path(super_wt);
+ free(super_wt);
+ }
+ strbuf_release(&sb);
+
+ code = finish_command(&cp);
+
+ if (code == 128)
+ /* '../' is not a git repository */
+ return NULL;
+ if (code == 0 && len == 0)
+ /* There is an unrelated git repository at '../' */
+ return NULL;
+ if (code)
+ die(_("ls-tree returned unexpected return code %d"), code);
+
+ return ret;
+}
diff --git a/submodule.h b/submodule.h
index 05ab674f06..c8a0c9cb29 100644
--- a/submodule.h
+++ b/submodule.h
@@ -93,4 +93,12 @@ extern void prepare_submodule_repo_env(struct argv_array *out);
extern void absorb_git_dir_into_superproject(const char *prefix,
const char *path,
unsigned flags);
+
+/*
+ * Return the absolute path of the working tree of the superproject, which this
+ * project is a submodule of. If this repository is not a submodule of
+ * another repository, return NULL.
+ */
+extern const char *get_superproject_working_tree(void);
+
#endif
diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh
index 9ed8b8ccba..03d3c7f6d6 100755
--- a/t/t1500-rev-parse.sh
+++ b/t/t1500-rev-parse.sh
@@ -116,4 +116,18 @@ test_expect_success 'git-path inside sub-dir' '
test_cmp expect actual
'
+test_expect_success 'showing the superproject correctly' '
+ git rev-parse --show-superproject-working-tree >out &&
+ test_must_be_empty out &&
+
+ test_create_repo super &&
+ test_commit -C super test_commit &&
+ test_create_repo sub &&
+ test_commit -C sub test_commit &&
+ git -C super submodule add ../sub dir/sub &&
+ echo $(pwd)/super >expect &&
+ git -C super/dir/sub rev-parse --show-superproject-working-tree >out &&
+ test_cmp expect out
+'
+
test_done
--
2.12.0.190.g6e60aba09d.dirty
^ permalink raw reply related
* Re: [PATCH 15/18] read-cache, remove_marked_cache_entries: wipe selected submodules.
From: Stefan Beller @ 2017-03-08 22:39 UTC (permalink / raw)
To: Junio C Hamano
Cc: git@vger.kernel.org, Brandon Williams, David Turner,
brian m. carlson, Heiko Voigt, Jonathan Nieder, Ramsay Jones
In-Reply-To: <xmqqlgsg4lj4.fsf@gitster.mtv.corp.google.com>
On Tue, Mar 7, 2017 at 5:14 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>> submodule_from_ce returns always NULL, when such flag is not given.
>> From 10/18:
>>
>> +const struct submodule *submodule_from_ce(const struct cache_entry *ce)
>> +{
>> + if (!S_ISGITLINK(ce->ce_mode))
>> + return NULL;
>> +
>> + if (!should_update_submodules())
>> + return NULL;
>> +
>> + return submodule_from_path(null_sha1, ce->name);
>> +}
>>
>> should_update_submodules is always false if such a flag is not set,
>> such that we end up in the else case which is literally the same as
>> the removed lines (they are just indented).
>
> I see.
>
> I didn't think a function this deep in the callchain that does not
> take any parameter could possibly change the behaviour based on the
> end-user input. I was expecting that such a state (i.e. are we
> recursive? are we allowed to forcibly update the working tree
> files?) would be kept part of something like "struct checkout" and
> passed around the callchain.
>
> That was why I didn't look at how that function answers "should
> update?" question, and got puzzled. Because it would imply there is
> some hidden state that is accessible by everybody--a global variable
> or something--which would point at a deeper design issue.
Well it is just as deep as e.g. reacting on some bits of
struct unpack_trees_options in unpack-trees.c ?
But I can see how this is an issue with design.
I also think my previous answer was a straw man.
We only need to differentiate between 'ce' being a submodule
or not, because it should not be marked CE_REMOVE in
the non-recursive state.
Side-question:
Is there some doc (commit message), that explains the difference
between CE_REMOVE and CE_WT_REMOVE ?
Thanks,
Stefan
^ permalink raw reply
* Re: [PATCH 15/18] read-cache, remove_marked_cache_entries: wipe selected submodules.
From: Junio C Hamano @ 2017-03-08 23:37 UTC (permalink / raw)
To: Stefan Beller
Cc: git@vger.kernel.org, Brandon Williams, David Turner,
brian m. carlson, Heiko Voigt, Jonathan Nieder, Ramsay Jones
In-Reply-To: <CAGZ79kYF63E17SNa9wt3X_28kbvjNujUBPoMct=RvDcyOeJm-w@mail.gmail.com>
Stefan Beller <sbeller@google.com> writes:
> Side-question:
> Is there some doc (commit message), that explains the difference
> between CE_REMOVE and CE_WT_REMOVE ?
That's something you need to ask Duy, I think, as it was introduced
at e663db2f ("unpack-trees(): add CE_WT_REMOVE to remove on worktree
alone", 2009-08-20) and was added for the sparse checkout stuff.
During my review of the series that added the feature, I only had
time to make sure that the patches do not change the behaviour when
it is not in use, ignoring the other side of "if" statement that
checked ce_skip_worktree(ce) ;-)
^ permalink raw reply
* Re: [PATCH] submodule--helper.c: remove duplicate code
From: Junio C Hamano @ 2017-03-08 23:24 UTC (permalink / raw)
To: Stefan Beller; +Cc: Valery Tolstov, git@vger.kernel.org
In-Reply-To: <CAGZ79ka7PNKq5JWLPujvVHJWf6eEUadaJXd5AmKEvKT_y1ghOA@mail.gmail.com>
Stefan Beller <sbeller@google.com> writes:
>> Also,
>> should I start new thread for new patch series?
>
> As you like.
> As far as I understand, it is very easy for Junio to take a whole
> (sub-)thread of patches and apply that and make a branch with
> multiple commits out of it as he has tooling for that.
Note that the world does not revolve around _me_. I was once asked
for my preference and I responded and that is what you are recalling
here.
Others on the list do review and keeping it easy for them to is also
important. What's _your_ preference?
^ permalink raw reply
* Re: [PATCHv4] rev-parse: add --show-superproject-working-tree
From: Junio C Hamano @ 2017-03-08 23:51 UTC (permalink / raw)
To: Stefan Beller; +Cc: szeder.dev, email, git, sandals, ville.skytta
In-Reply-To: <20170308230742.5185-1-sbeller@google.com>
Stefan Beller <sbeller@google.com> writes:
> *this patch*
>
> * Documentation shamelessly stolen from Junio.
You stole it, and then ...
> +test_expect_success 'showing the superproject correctly' '
> + git rev-parse --show-superproject-working-tree >out &&
> + test_must_be_empty out &&
... made sure that the newly documented behaviour is tested as a
feature. Good.
> + test_create_repo super &&
> + test_commit -C super test_commit &&
> + test_create_repo sub &&
> + test_commit -C sub test_commit &&
> + git -C super submodule add ../sub dir/sub &&
> + echo $(pwd)/super >expect &&
> + git -C super/dir/sub rev-parse --show-superproject-working-tree >out &&
> + test_cmp expect out
> +'
> +
^ permalink raw reply
* What's cooking in git.git (Mar 2017, #03; Wed, 8)
From: Junio C Hamano @ 2017-03-08 23:47 UTC (permalink / raw)
To: git
Here are the topics that have been cooking. Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'. The ones marked with '.' do not appear in any of
the integration branches, but I am still holding onto them.
You can find the changes described here in the integration branches
of the repositories listed at
http://git-blame.blogspot.com/p/git-public-repositories.html
--------------------------------------------------
[New Topics]
* ew/http-alternates-as-redirects-warning (2017-03-06) 2 commits
- http: release strbuf on disabled alternates
- http: inform about alternates-as-redirects behavior
Recent versions of Git treats http alternates (used in dumb http
transport) just like HTTP redirects and requires the client to
enable following it, due to security concerns. But we forgot to
give a warning when we decide not to honor the alternates.
Will merge to 'next'.
* jk/ewah-use-right-type-in-sizeof (2017-03-06) 1 commit
- ewah: fix eword_t/uint64_t confusion
Code clean-up.
Will merge to 'next'.
* jk/push-deadlock-regression-fix (2017-03-07) 6 commits
- send-pack: report signal death of pack-objects
- send-pack: read "unpack" status even on pack-objects failure
- send-pack: improve unpack-status error messages
- send-pack: use skip_prefix for parsing unpack status
- send-pack: extract parsing of "unpack" response
- receive-pack: fix deadlock when we cannot create tmpdir
"git push" had a handful of codepaths that could lead to a deadlock
when unexpected error happened, which has been fixed.
Will merge to 'next'.
* vn/line-log-memcpy-size-fix (2017-03-06) 1 commit
- line-log: use COPY_ARRAY to fix mis-sized memcpy
The command-line parsing of "git log -L" copied internal data
structures using incorrect size on ILP32 systems.
Will merge to 'next'.
* js/realpath-pathdup-fix (2017-03-08) 2 commits
- real_pathdup(): fix callsites that wanted it to die on error
- t1501: demonstrate NULL pointer access with invalid GIT_WORK_TREE
Git v2.12 was shipped with an embarrassing breakage where various
operations that verify paths given from the user stopped dying when
seeing an issue, and instead later triggering segfault.
Will merge to 'next' and then to 'master', eventually down to 'maint'.
* kn/ref-filter-branch-list (2017-03-08) 1 commit
- branch: honor --abbrev/--no-abbrev in --list mode
"git branch --list" takes the "--abbrev" and "--no-abbrev" options
to control the output of the object name in its "-v"(erbose)
output, but a recent update started ignoring them; this fixes it
before the breakage reaches to any released version.
Will merge to 'next'.
* sb/rev-parse-show-superproject-root (2017-03-08) 1 commit
- rev-parse: add --show-superproject-working-tree
From a working tree of a repository, a new option of "rev-parse"
lets you ask if the repository is used as a submodule of another
project, and where the root level of the working tree of that
project (i.e. your superproject) is.
Almost there, but documentation needs a bit more work.
--------------------------------------------------
[Stalled]
* nd/worktree-move (2017-01-27) 7 commits
. fixup! worktree move: new command
. worktree remove: new command
. worktree move: refuse to move worktrees with submodules
. worktree move: accept destination as directory
. worktree move: new command
. worktree.c: add update_worktree_location()
. worktree.c: add validate_worktree()
"git worktree" learned move and remove subcommands.
Tentatively ejected as it seems to break 'pu' when merged.
* pb/bisect (2017-02-18) 28 commits
- fixup! bisect--helper: `bisect_next_check` & bisect_voc shell function in C
- bisect--helper: remove the dequote in bisect_start()
- bisect--helper: retire `--bisect-auto-next` subcommand
- bisect--helper: retire `--bisect-autostart` subcommand
- bisect--helper: retire `--bisect-write` subcommand
- bisect--helper: `bisect_replay` shell function in C
- bisect--helper: `bisect_log` shell function in C
- bisect--helper: retire `--write-terms` subcommand
- bisect--helper: retire `--check-expected-revs` subcommand
- bisect--helper: `bisect_state` & `bisect_head` shell function in C
- bisect--helper: `bisect_autostart` shell function in C
- bisect--helper: retire `--next-all` subcommand
- bisect--helper: retire `--bisect-clean-state` subcommand
- bisect--helper: `bisect_next` and `bisect_auto_next` shell function in C
- t6030: no cleanup with bad merge base
- bisect--helper: `bisect_start` shell function partially in C
- bisect--helper: `get_terms` & `bisect_terms` shell function in C
- bisect--helper: `bisect_next_check` & bisect_voc shell function in C
- bisect--helper: `check_and_set_terms` shell function in C
- bisect--helper: `bisect_write` shell function in C
- bisect--helper: `is_expected_rev` & `check_expected_revs` shell function in C
- bisect--helper: `bisect_reset` shell function in C
- wrapper: move is_empty_file() and rename it as is_empty_or_missing_file()
- t6030: explicitly test for bisection cleanup
- bisect--helper: `bisect_clean_state` shell function in C
- bisect--helper: `write_terms` shell function in C
- bisect: rewrite `check_term_format` shell function in C
- bisect--helper: use OPT_CMDMODE instead of OPT_BOOL
Move more parts of "git bisect" to C.
Expecting a reroll.
cf. <CAFZEwPPXPPHi8KiEGS9ggzNHDCGhuqMgH9Z8-Pf9GLshg8+LPA@mail.gmail.com>
cf. <CAFZEwPM9RSTGN54dzaw9gO9iZmsYjJ_d1SjUD4EzSDDbmh-XuA@mail.gmail.com>
cf. <CAFZEwPNUXcNY9Qdz=_B7q2kQuaecPzJtTMGdv8YMUPEz2vnp8A@mail.gmail.com>
* sh/grep-tree-obj-tweak-output (2017-01-20) 2 commits
- grep: use '/' delimiter for paths
- grep: only add delimiter if there isn't one already
"git grep", when fed a tree-ish as an input, shows each hit
prefixed with "<tree-ish>:<path>:<lineno>:". As <tree-ish> is
almost always either a commit or a tag that points at a commit, the
early part of the output "<tree-ish>:<path>" can be used as the
name of the blob and given to "git show". When <tree-ish> is a
tree given in the extended SHA-1 syntax (e.g. "<commit>:", or
"<commit>:<dir>"), however, this results in a string that does not
name a blob (e.g. "<commit>::<path>" or "<commit>:<dir>:<path>").
"git grep" has been taught to be a bit more intelligent about these
cases and omit a colon (in the former case) or use slash (in the
latter case) to produce "<commit>:<path>" and
"<commit>:<dir>/<path>" that can be used as the name of a blob.
Expecting a reroll? Is this good enough with known limitations?
* jc/diff-b-m (2015-02-23) 5 commits
. WIPWIP
. WIP: diff-b-m
- diffcore-rename: allow easier debugging
- diffcore-rename.c: add locate_rename_src()
- diffcore-break: allow debugging
"git diff -B -M" produced incorrect patch when the postimage of a
completely rewritten file is similar to the preimage of a removed
file; such a resulting file must not be expressed as a rename from
other place.
The fix in this patch is broken, unfortunately.
Will discard.
--------------------------------------------------
[Cooking]
* ls/filter-process-delayed (2017-03-06) 1 commit
- convert: add "status=delayed" to filter process protocol
cf. <xmqq60jmnmef.fsf@junio-linux.mtv.corp.google.com>
* ew/markdown-url-in-readme (2017-03-01) 1 commit
(merged to 'next' on 2017-03-03 at 3d35e3a991)
+ README: create HTTP/HTTPS links from URLs in Markdown
Doc update.
Will merge to 'master'.
* jk/add-i-patch-do-prompt (2017-03-02) 1 commit
- add--interactive: fix missing file prompt for patch mode with "-i"
The patch subcommand of "git add -i" was meant to have paths
selection prompt just like other subcommand, unlike "git add -p"
directly jumps to hunk selection. Recently, this was broken and
"add -i" lost the paths selection dialog, but it now has been
fixed.
Will merge to 'next'.
* ax/line-log-range-merge-fix (2017-03-03) 1 commit
- line-log.c: prevent crash during union of too many ranges
The code to parse "git log -L..." command line was buggy when there
are many ranges specified with -L; overrun of the allocated buffer
has been fixed.
Will merge to 'next'.
* js/early-config (2017-03-07) 10 commits
- setup_git_directory_gently_1(): avoid die()ing
- t1309: test read_early_config()
- read_early_config(): really discover .git/
- read_early_config(): avoid .git/config hack when unneeded
- setup: make read_early_config() reusable
- setup: introduce the discover_git_directory() function
- setup_git_directory_1(): avoid changing global state
- setup: prepare setup_discovered_git_directory() the root directory
- setup_git_directory(): use is_dir_sep() helper
- t7006: replace dubious test
The start-up sequence of "git" needs to figure out some configured
settings before it finds and set itself up in the location of the
repository and was quite messy due to its "chicken-and-egg" nature.
The code has been restructured.
Will merge to 'next' after waiting for a few days.
* jt/perf-updates (2017-03-03) 3 commits
- t/perf: add fallback for pre-bin-wrappers versions of git
- t/perf: use $MODERN_GIT for all repo-copying steps
- t/perf: export variable used in other blocks
The t/perf performance test suite was not prepared to test not so
old versions of Git, but now it covers versions of Git that are not
so ancient.
Will merge to 'next'.
* ss/remote-bzr-hg-placeholder-wo-python (2017-03-03) 1 commit
- contrib: git-remote-{bzr,hg} placeholders don't need Python
There is no need for Python only to give a few messages to the
standard error stream, but we somehow did.
Will merge to 'next'.
* jk/interpret-branch-name (2017-03-02) 9 commits
- checkout: restrict @-expansions when finding branch
- strbuf_check_ref_format(): expand only local branches
- branch: restrict @-expansions when deleting
- t3204: test git-branch @-expansion corner cases
- interpret_branch_name: allow callers to restrict expansions
- strbuf_branchname: add docstring
- strbuf_branchname: drop return value
- interpret_branch_name: move docstring to header file
- interpret_branch_name(): handle auto-namelen for @{-1}
"git branch @" created refs/heads/@ as a branch, and in general the
code that handled @{-1} and @{upstream} was a bit too loose in
disambiguating.
Will merge to 'next'.
* jk/sha1dc (2017-03-01) 7 commits
- Put sha1dc on a diet
- sha1dc: avoid 'for' loop initial decl
- sha1dc: resurrect LICENSE file
- sha1dc: avoid c99 declaration-after-statement
- Makefile: add USE_SHA1DC knob
- sha1dc: adjust header includes for git
- add collision-detecting sha1 implementation
Borrow "detect attempt to create collisions" variant of SHA-1
implementation by Marc Stevens (CWI) and Dan Shumow (Microsoft).
Expecting a cleaned-up reroll after discussion settles.
cf. <CA+55aFxTWqsTTiDKo4DBZT-8Z9t80bGMD3uijzKONa_bYEZABQ@mail.gmail.com>
* js/travis-32bit-linux (2017-03-06) 1 commit
- Travis: also test on 32-bit Linux
Add 32-bit Linux variant to the set of platforms to be tested with
Travis CI.
Will merge to 'next'.
* jt/http-base-url-update-upon-redirect (2017-02-28) 1 commit
(merged to 'next' on 2017-03-03 at 5225bd3ef8)
+ http: attempt updating base URL only if no error
When a redirected http transport gets an error during the
redirected request, we ignored the error we got from the server,
and ended up giving a not-so-useful error message.
Will merge to 'master'.
* jt/mark-tree-uninteresting-for-uninteresting-commit (2017-02-28) 3 commits
- upload-pack: compute blob reachability correctly
- revision: exclude trees/blobs given commit
- revision: unify {tree,blob}_objects in rev_info
The revision/object traversal machinery did not mark all tree and
blob objects that are contained in an uninteresting commit as
uninteresting, because that is quite costly. Instead, it only
marked those that are contained in an uninteresting boundary commit
as uninteresting.
cf. <20170228215937.yd4juycjf7y3vish@sigill.intra.peff.net>
* ps/docs-diffcore (2017-02-28) 2 commits
(merged to 'next' on 2017-03-03 at 9ca5691de2)
+ docs/diffcore: unquote "Complete Rewrites" in headers
+ docs/diffcore: fix grammar in diffcore-rename header
Doc update.
Will merge to 'master'.
* rj/remove-unused-mktemp (2017-02-28) 2 commits
(merged to 'next' on 2017-03-03 at 4512f0c5ab)
+ wrapper.c: remove unused gitmkstemps() function
+ wrapper.c: remove unused git_mkstemp() function
Code cleanup.
Will merge to 'master'.
* sb/submodule-init-url-selection (2017-02-28) 1 commit
(merged to 'next' on 2017-03-03 at 847d1f9a91)
+ submodule init: warn about falling back to a local path
Give a warning when "git submodule init" decides that the submodule
in the working tree is its upstream, as it is not a very common
setup.
Will merge to 'master'.
* jc/diff-populate-filespec-size-only-fix (2017-03-02) 1 commit
- diff: do not short-cut CHECK_SIZE_ONLY check in diff_populate_filespec()
"git diff --quiet" relies on the size field in diff_filespec to be
correctly populated, but diff_populate_filespec() helper function
made an incorrect short-cut when asked only to populate the size
field for paths that need to go through convert_to_git() (e.g. CRLF
conversion).
Will merge to 'next'.
* nd/conditional-config-include (2017-03-03) 5 commits
- SQUASH??? cond config include test
- SQUASH???
- config: add conditional include
- config.txt: reflow the second include.path paragraph
- config.txt: clarify multiple key values in include.path
The configuration file learned a new "includeIf.<condition>.path"
that includes the contents of the given path only when the
condition holds. This allows you to say "include this work-related
bit only in the repositories under my ~/work/ directory".
Will merge to 'next' after squashing niggle-fixes in.
* rs/log-email-subject (2017-03-01) 2 commits
(merged to 'next' on 2017-03-03 at a2ecc84866)
+ pretty: use fmt_output_email_subject()
+ log-tree: factor out fmt_output_email_subject()
Code clean-up.
Will merge to 'master'.
* cc/split-index-config (2017-03-06) 22 commits
- Documentation/git-update-index: explain splitIndex.*
- Documentation/config: add splitIndex.sharedIndexExpire
- read-cache: use freshen_shared_index() in read_index_from()
- read-cache: refactor read_index_from()
- t1700: test shared index file expiration
- read-cache: unlink old sharedindex files
- config: add git_config_get_expiry() from gc.c
- read-cache: touch shared index files when used
- sha1_file: make check_and_freshen_file() non static
- Documentation/config: add splitIndex.maxPercentChange
- t1700: add tests for splitIndex.maxPercentChange
- read-cache: regenerate shared index if necessary
- config: add git_config_get_max_percent_split_change()
- Documentation/git-update-index: talk about core.splitIndex config var
- Documentation/config: add information for core.splitIndex
- t1700: add tests for core.splitIndex
- update-index: warn in case of split-index incoherency
- read-cache: add and then use tweak_split_index()
- split-index: add {add,remove}_split_index() functions
- config: add git_config_get_split_index()
- t1700: change here document style
- config: mark an error message up for translation
The experimental "split index" feature has gained a few
configuration variables to make it easier to use.
I think this is almost ready for 'next'.
* dp/filter-branch-prune-empty (2017-03-03) 4 commits
- p7000: add test for filter-branch with --prune-empty
- filter-branch: fix --prune-empty on parentless commits
- t7003: ensure --prune-empty removes entire branch when applicable
- t7003: ensure --prune-empty can prune root commit
"git filter-branch --prune-empty" drops a single-parent commit that
becomes a no-op, but did not drop a root commit whose tree is empty.
Will merge to 'next'.
* jc/config-case-cmdline-take-2 (2017-02-23) 2 commits
(merged to 'next' on 2017-03-01 at 2e9920eeeb)
+ config: use git_config_parse_key() in git_config_parse_parameter()
+ config: move a few helper functions up
The code to parse "git -c VAR=VAL cmd" and set configuration
variable for the duration of cmd had two small bugs, which have
been fixed.
Will merge to 'master'.
This supersedes jc/config-case-cmdline topic that has been discarded.
* ab/cond-skip-tests (2017-03-01) 3 commits
- gitweb tests: skip tests when we don't have Time::HiRes
- gitweb tests: change confusing "skip_all" phrasing
- cvs tests: skip tests that call "cvs commit" when running as root
A few tests were run conditionally under (rare) conditions where
they cannot be run (like running cvs tests under 'root' account).
Will merge to 'next'.
* jk/interop-test (2017-02-27) 2 commits
- t/interop: add test of old clients against modern git-daemon
- t: add an interoperability test harness
Picking two versions of Git and running tests to make sure the
older one and the newer one interoperate happily has now become
possible.
Needs review.
* jk/parse-config-key-cleanup (2017-02-24) 3 commits
(merged to 'next' on 2017-03-01 at e531d8d3a9)
+ parse_hide_refs_config: tell parse_config_key we don't want a subsection
+ parse_config_key: allow matching single-level config
+ parse_config_key: use skip_prefix instead of starts_with
(this branch uses sb/parse-hide-refs-config-cleanup.)
The "parse_config_key()" API function has been cleaned up.
Will merge to 'master'.
* jk/t6300-cleanup (2017-02-27) 1 commit
(merged to 'next' on 2017-03-02 at 3087521bea)
+ t6300: avoid creating refs/heads/HEAD
A test that creates a confusing branch whose name is HEAD has been
corrected not to do so.
Will merge to 'master'.
* rs/commit-parsing-optim (2017-02-27) 2 commits
(merged to 'next' on 2017-03-02 at 22239f35df)
+ commit: don't check for space twice when looking for header
+ commit: be more precise when searching for headers
The code that parses header fields in the commit object has been
updated for (micro)performance and code hygiene.
Will merge to 'master'.
* rs/sha1-file-plug-fallback-base-leak (2017-02-27) 1 commit
(merged to 'next' on 2017-03-02 at 03344b1119)
+ sha1_file: release fallback base's memory in unpack_entry()
A leak in a codepath to read from a packed object in (rare) cases
has been plugged.
Will merge to 'master'.
* rs/strbuf-add-real-path (2017-02-27) 2 commits
(merged to 'next' on 2017-03-02 at 69191becd6)
+ strbuf: add strbuf_add_real_path()
+ cocci: use ALLOC_ARRAY
An helper function to make it easier to append the result from
real_path() to a strbuf has been added.
Will merge to 'master'.
* sb/parse-hide-refs-config-cleanup (2017-02-24) 1 commit
(merged to 'next' on 2017-03-01 at fd722ba039)
+ refs: parse_hide_refs_config to use parse_config_key
(this branch is used by jk/parse-config-key-cleanup.)
Code clean-up.
Will merge to 'master'.
* sg/clone-refspec-from-command-line-config (2017-02-27) 1 commit
- clone: respect configured fetch respecs during initial fetch
Needs review.
cf. <20170227211217.73gydlxb2qu2sp3m@sigill.intra.peff.net>
* sk/dash-is-previous (2017-03-01) 5 commits
- revert.c: delegate handling of "-" shorthand to setup_revisions
- sha1_name.c: teach get_sha1_1 "-" shorthand for "@{-1}"
- revision.c: args starting with "-" might be a revision
- revision.c: swap if/else blocks
- revision.c: do not update argv with unknown option
A dash "-" can be written to mean "the branch that was previously
checked out" in more places.
Needs review.
cf. <1488007487-12965-1-git-send-email-kannan.siddharth12@gmail.com>
* jh/send-email-one-cc (2017-02-27) 1 commit
(merged to 'next' on 2017-03-02 at 32c0e6ad88)
+ send-email: only allow one address per body tag
"Cc:" on the trailer part does not have to conform to RFC strictly,
unlike in the e-mail header. "git send-email" has been updated to
ignore anything after '>' when picking addresses, to allow non-address
cruft like " # stable 4.4" after the address.
Will merge to 'master'.
* jk/http-auth (2017-02-27) 2 commits
(merged to 'next' on 2017-03-02 at 87f81b4395)
+ http: add an "auto" mode for http.emptyauth
+ http: restrict auth methods to what the server advertises
Reduce authentication round-trip over HTTP when the server supports
just a single authentication method.
Will merge to 'master'.
* jk/ident-empty (2017-02-23) 4 commits
(merged to 'next' on 2017-03-01 at ff80031ce6)
+ ident: do not ignore empty config name/email
+ ident: reject all-crud ident name
+ ident: handle NULL email when complaining of empty name
+ ident: mark error messages for translation
user.email that consists of only cruft chars should consistently
error out, but didn't.
Will merge to 'master'.
* jt/upload-pack-error-report (2017-02-23) 1 commit
(merged to 'next' on 2017-03-01 at aea583dbe5)
+ upload-pack: report "not our ref" to client
"git upload-pack", which is a counter-part of "git fetch", did not
report a request for a ref that was not advertised as invalid.
This is generally not a problem (because "git fetch" will stop
before making such a request), but is the right thing to do.
Will merge to 'master'.
* ah/doc-ls-files-quotepath (2017-03-02) 1 commit
- Documentation: improve description for core.quotePath
Documentation for "git ls-files" did not refer to core.quotePath
Will merge to 'next'.
* jh/memihash-opt (2017-02-17) 5 commits
- name-hash: remember previous dir_entry during lazy_init_name_hash
- name-hash: specify initial size for istate.dir_hash table
- name-hash: precompute hash values during preload-index
- hashmap: allow memihash computation to be continued
- name-hash: eliminate duplicate memihash call
Expecting an update for perf.
cf. <MWHPR03MB29581B0EDDEDCA7D51EC396F8A280@MWHPR03MB2958.namprd03.prod.outlook.com>
* nd/prune-in-worktree (2017-02-19) 15 commits
. rev-list: expose and document --single-worktree
. revision.c: --reflog add HEAD reflog from all worktrees
. files-backend: make reflog iterator go through per-worktree reflog
. refs: add refs_for_each_reflog[_ent]()
. revision.c: --all adds HEAD from all worktrees
. refs: remove dead for_each_*_submodule()
. revision.c: use refs_for_each*() instead of for_each_*_submodule()
. refs: add a refs_for_each_in() and friends
. refs: add refs_for_each_ref()
. refs: add refs_head_ref()
. refs: add refs_read_ref[_full]()
. refs: move submodule slash stripping code to get_submodule_ref_store
. revision.c: --indexed-objects add objects from all worktrees
. revision.c: refactor add_index_objects_to_pending()
. revision.h: new flag in struct rev_info wrt. worktree-related refs
(this branch uses nd/worktree-kill-parse-ref; is tangled with nd/files-backend-git-dir.)
"git gc" and friends when multiple worktrees are used off of a
single repository did not consider the index and per-worktree refs
of other worktrees as the root for reachability traversal, making
objects that are in use only in other worktrees to be subject to
garbage collection.
* mm/fetch-show-error-message-on-unadvertised-object (2017-03-02) 3 commits
- fetch-pack: add specific error for fetching an unadvertised object
- fetch_refs_via_pack: call report_unmatched_refs
- fetch-pack: move code to report unmatched refs to a function
"git fetch" that requests a commit by object name, when the other
side does not allow such an request, failed without much
explanation.
Will merge to 'next'.
* nd/worktree-kill-parse-ref (2017-02-19) 22 commits
. refs: kill set_worktree_head_symref()
. refs: add refs_create_symref()
. worktree.c: kill parse_ref() in favor of refs_resolve_ref_unsafe()
. refs.c: add refs_resolve_ref_unsafe()
. refs: introduce get_worktree_ref_store()
. refs: rename get_ref_store() to get_submodule_ref_store() and make it public
. files-backend: remove submodule_allowed from files_downcast()
. refs: move submodule code out of files-backend.c
. path.c: move some code out of strbuf_git_path_submodule()
. refs.c: make get_main_ref_store() public and use it
. refs.c: kill register_ref_store(), add register_submodule_ref_store()
. refs.c: flatten get_ref_store() a bit
. refs: rename lookup_ref_store() to lookup_submodule_ref_store()
. refs.c: introduce get_main_ref_store()
. files-backend: remove the use of git_path()
. refs.c: share is_per_worktree_ref() to files-backend.c
. files-backend: replace *git_path*() with files_path()
. files-backend: add files_path()
. files-backend: convert git_path() to strbuf_git_path()
. refs-internal.c: make files_log_ref_write() static
. Merge branch 'mh/ref-remove-empty-directory' into nd/files-backend-git-dir
. Merge branch 'mh/submodule-hash' into nd/files-backend-git-dir
(this branch is used by nd/prune-in-worktree; is tangled with nd/files-backend-git-dir.)
(hopefully) a beginning of safer "git worktree" that is resistant
to "gc".
Waiting for nd/files-backend-git-dir to settle.
* nd/files-backend-git-dir (2017-02-22) 26 commits
. t1406: new tests for submodule ref store
. t1405: some basic tests on main ref store
. t/helper: add test-ref-store to test ref-store functions
. refs: delete pack_refs() in favor of refs_pack_refs()
. files-backend: avoid ref api targetting main ref store
. refs: new transaction related ref-store api
. refs: add new ref-store api
. refs: rename get_ref_store() to get_submodule_ref_store() and make it public
. files-backend: replace submodule_allowed check in files_downcast()
. refs: move submodule code out of files-backend.c
. path.c: move some code out of strbuf_git_path_submodule()
. refs.c: make get_main_ref_store() public and use it
. refs.c: kill register_ref_store(), add register_submodule_ref_store()
. refs.c: flatten get_ref_store() a bit
. refs: rename lookup_ref_store() to lookup_submodule_ref_store()
. refs.c: introduce get_main_ref_store()
. files-backend: remove the use of git_path()
. files-backend: add and use files_refname_path()
. files-backend: add and use files_reflog_path()
. files-backend: move "logs/" out of TMP_RENAMED_LOG
. files-backend: convert git_path() to strbuf_git_path()
. files-backend: add and use files_packed_refs_path()
. files-backend: make files_log_ref_write() static
. refs.h: add forward declaration for structs used in this file
. Merge branch 'mh/ref-remove-empty-directory' into nd/files-backend-git-dir
. Merge branch 'mh/submodule-hash' into nd/files-backend-git-dir
(this branch is tangled with nd/prune-in-worktree and nd/worktree-kill-parse-ref.)
The "submodule" specific field in the ref_store structure is
replaced with a more generic "gitdir" that can later be used also
when dealing with ref_store that represents the set of refs visible
from the other worktrees.
* sb/checkout-recurse-submodules (2017-03-07) 18 commits
- builtin/read-tree: add --recurse-submodules switch
- builtin/checkout: add --recurse-submodules switch
- entry.c: update submodules when interesting
- read-cache, remove_marked_cache_entries: wipe selected submodules.
- unpack-trees: check if we can perform the operation for submodules
- unpack-trees: pass old oid to verify_clean_submodule
- update submodules: add submodule_move_head
- update submodules: move up prepare_submodule_repo_env
- submodules: introduce check to see whether to touch a submodule
- update submodules: add a config option to determine if submodules are updated
- update submodules: add submodule config parsing
- connect_work_tree_and_git_dir: safely create leading directories
- make is_submodule_populated gently
- lib-submodule-update.sh: define tests for recursing into submodules
- lib-submodule-update.sh: replace sha1 by hash
- lib-submodule-update: teach test_submodule_content the -C <dir> flag
- lib-submodule-update.sh: do not use ./. as submodule remote
- lib-submodule-update.sh: reorder create_lib_submodule_repo
"git checkout" is taught --recurse-submodules option.
* tg/stash-push (2017-02-28) 6 commits
(merged to 'next' on 2017-03-03 at b50fda0389)
+ stash: allow pathspecs in the no verb form
+ stash: use stash_push for no verb form
+ stash: teach 'push' (and 'create_stash') to honor pathspec
+ stash: refactor stash_create
+ stash: add test for the create command line arguments
+ stash: introduce push verb
Allow "git stash" to take pathspec so that the local changes can be
stashed away only partially.
Will merge to 'master'.
* bc/object-id (2017-02-22) 19 commits
- wt-status: convert to struct object_id
- builtin/merge-base: convert to struct object_id
- Convert object iteration callbacks to struct object_id
- sha1_file: introduce an nth_packed_object_oid function
- refs: simplify parsing of reflog entries
- refs: convert each_reflog_ent_fn to struct object_id
- reflog-walk: convert struct reflog_info to struct object_id
- builtin/replace: convert to struct object_id
- Convert remaining callers of resolve_refdup to object_id
- builtin/merge: convert to struct object_id
- builtin/clone: convert to struct object_id
- builtin/branch: convert to struct object_id
- builtin/grep: convert to struct object_id
- builtin/fmt-merge-message: convert to struct object_id
- builtin/fast-export: convert to struct object_id
- builtin/describe: convert to struct object_id
- builtin/diff-tree: convert to struct object_id
- builtin/commit: convert to struct object_id
- hex: introduce parse_oid_hex
"uchar [40]" to "struct object_id" conversion continues.
Now at v5.
cf. <20170221234737.894681-1-sandals@crustytoothpaste.net>
* jh/mingw-openssl-sha1 (2017-02-09) 1 commit
- mingw: use OpenSSL's SHA-1 routines
Windows port wants to use OpenSSL's implementation of SHA-1
routines, so let them.
Will merge to 'next'.
* sg/completion-refs-speedup (2017-02-13) 13 commits
- squash! completion: fill COMPREPLY directly when completing refs
- completion: fill COMPREPLY directly when completing refs
- completion: list only matching symbolic and pseudorefs when completing refs
- completion: let 'for-each-ref' sort remote branches for 'checkout' DWIMery
- completion: let 'for-each-ref' filter remote branches for 'checkout' DWIMery
- completion: let 'for-each-ref' strip the remote name from remote branches
- completion: let 'for-each-ref' and 'ls-remote' filter matching refs
- completion: don't disambiguate short refs
- completion: don't disambiguate tags and branches
- completion: support excluding full refs
- completion: support completing full refs after '--option=refs/<TAB>'
- completion: wrap __git_refs() for better option parsing
- completion: remove redundant __gitcomp_nl() options from _git_commit()
The refs completion for large number of refs has been sped up,
partly by giving up disambiguating ambiguous refs and partly by
eliminating most of the shell processing between 'git for-each-ref'
and 'ls-remote' and Bash's completion facility.
What's the donness of this topic?
* jk/no-looking-at-dotgit-outside-repo-final (2016-10-26) 1 commit
(merged to 'next' on 2017-02-27 at 7373a1b73d)
+ setup_git_env: avoid blind fall-back to ".git"
This is the endgame of the topic to avoid blindly falling back to
".git" when the setup sequence said we are _not_ in Git repository.
A corner case that happens to work right now may be broken by a
call to die("BUG").
Will cook in 'next'.
* jc/merge-drop-old-syntax (2015-04-29) 1 commit
(merged to 'next' on 2017-02-27 at 2c0f5f73d8)
+ merge: drop 'git merge <message> HEAD <commit>' syntax
Stop supporting "git merge <message> HEAD <commit>" syntax that has
been deprecated since October 2007, and issues a deprecation
warning message since v2.5.0.
Will cook in 'next'.
* jc/bundle (2016-03-03) 6 commits
- index-pack: --clone-bundle option
- Merge branch 'jc/index-pack' into jc/bundle
- bundle v3: the beginning
- bundle: keep a copy of bundle file name in the in-core bundle header
- bundle: plug resource leak
- bundle doc: 'verify' is not about verifying the bundle
The beginning of "split bundle", which could be one of the
ingredients to allow "git clone" traffic off of the core server
network to CDN.
--------------------------------------------------
[Discarded]
* jk/auto-namelen-in-interpret-branch-name (2017-02-27) 1 commit
. interpret_branch_name(): handle auto-namelen for @{-1}
A small bug in the code that parses @{...} has been fixed.
This is now a part of jk/interpret-branch-name topic.
^ permalink raw reply
* [PATCH v2 1/2] connect_work_tree_and_git_dir: safely create leading directories
From: Valery Tolstov @ 2017-03-09 0:03 UTC (permalink / raw)
To: git; +Cc: sbeller, me, gitster
In-Reply-To: <20170309000352.18330-1-me@vtolstov.org>
From: Stefan Beller <sbeller@google.com>
In a later patch we'll use connect_work_tree_and_git_dir when the
directory for the gitlink file doesn't exist yet. This patch makes
connect_work_tree_and_git_dir safe to use for both cases of
either the git dir or the working dir missing.
To do so, we need to call safe_create_leading_directories[_const]
on both directories. However this has to happen before we construct
the absolute paths as real_pathdup assumes the directories to
be there already.
So for both the config file in the git dir as well as the .git link
file we need to
a) construct the name
b) call SCLD
c) get the absolute path
d) once a-c is done for both we can consume the absolute path
to compute the relative path to each other and store those
relative paths.
The implementation provided here puts a) and b) for both cases first,
and then performs c and d after.
One of the two users of 'connect_work_tree_and_git_dir' already checked
for the directory being there, so we can loose that check as
connect_work_tree_and_git_dir handles this functionality now.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
dir.c | 32 +++++++++++++++++++++-----------
submodule.c | 11 ++---------
2 files changed, 23 insertions(+), 20 deletions(-)
diff --git a/dir.c b/dir.c
index 4541f9e14..6f52af7ab 100644
--- a/dir.c
+++ b/dir.c
@@ -2728,23 +2728,33 @@ void untracked_cache_add_to_index(struct index_state *istate,
/* Update gitfile and core.worktree setting to connect work tree and git dir */
void connect_work_tree_and_git_dir(const char *work_tree_, const char *git_dir_)
{
- struct strbuf file_name = STRBUF_INIT;
+ struct strbuf gitfile_sb = STRBUF_INIT;
+ struct strbuf cfg_sb = STRBUF_INIT;
struct strbuf rel_path = STRBUF_INIT;
- char *git_dir = real_pathdup(git_dir_);
- char *work_tree = real_pathdup(work_tree_);
+ char *git_dir, *work_tree;
- /* Update gitfile */
- strbuf_addf(&file_name, "%s/.git", work_tree);
- write_file(file_name.buf, "gitdir: %s",
- relative_path(git_dir, work_tree, &rel_path));
+ /* Prepare .git file */
+ strbuf_addf(&gitfile_sb, "%s/.git", work_tree_);
+ if (safe_create_leading_directories_const(gitfile_sb.buf))
+ die(_("could not create directories for %s"), gitfile_sb.buf);
+
+ /* Prepare config file */
+ strbuf_addf(&cfg_sb, "%s/config", git_dir_);
+ if (safe_create_leading_directories_const(cfg_sb.buf))
+ die(_("could not create directories for %s"), cfg_sb.buf);
+ git_dir = real_pathdup(git_dir_);
+ work_tree = real_pathdup(work_tree_);
+
+ /* Write .git file */
+ write_file(gitfile_sb.buf, "gitdir: %s",
+ relative_path(git_dir, work_tree, &rel_path));
/* Update core.worktree setting */
- strbuf_reset(&file_name);
- strbuf_addf(&file_name, "%s/config", git_dir);
- git_config_set_in_file(file_name.buf, "core.worktree",
+ git_config_set_in_file(cfg_sb.buf, "core.worktree",
relative_path(work_tree, git_dir, &rel_path));
- strbuf_release(&file_name);
+ strbuf_release(&gitfile_sb);
+ strbuf_release(&cfg_sb);
strbuf_release(&rel_path);
free(work_tree);
free(git_dir);
diff --git a/submodule.c b/submodule.c
index 3b98766a6..45e93a1d5 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1445,8 +1445,6 @@ void absorb_git_dir_into_superproject(const char *prefix,
/* Not populated? */
if (!sub_git_dir) {
- char *real_new_git_dir;
- const char *new_git_dir;
const struct submodule *sub;
if (err_code == READ_GITFILE_ERR_STAT_FAILED) {
@@ -1469,13 +1467,8 @@ void absorb_git_dir_into_superproject(const char *prefix,
sub = submodule_from_path(null_sha1, path);
if (!sub)
die(_("could not lookup name for submodule '%s'"), path);
- new_git_dir = git_path("modules/%s", sub->name);
- if (safe_create_leading_directories_const(new_git_dir) < 0)
- die(_("could not create directory '%s'"), new_git_dir);
- real_new_git_dir = real_pathdup(new_git_dir);
- connect_work_tree_and_git_dir(path, real_new_git_dir);
-
- free(real_new_git_dir);
+ connect_work_tree_and_git_dir(path,
+ git_path("modules/%s", sub->name));
} else {
/* Is it already absorbed into the superprojects git dir? */
char *real_sub_git_dir = real_pathdup(sub_git_dir);
--
2.12.0.192.gbdb9d28a5
^ permalink raw reply related
* [PATCH v2 2/2] submodule--helper.c: remove duplicate code
From: Valery Tolstov @ 2017-03-09 0:03 UTC (permalink / raw)
To: git; +Cc: sbeller, me, gitster
In-Reply-To: <20170309000352.18330-1-me@vtolstov.org>
Remove code fragment from module_clone that duplicates functionality
of connect_work_tree_and_git_dir in dir.c
Signed-off-by: Valery Tolstov <me@vtolstov.org>
---
builtin/submodule--helper.c | 20 ++------------------
1 file changed, 2 insertions(+), 18 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 899dc334e..405cbea07 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -579,7 +579,6 @@ static int module_clone(int argc, const char **argv, const char *prefix)
const char *name = NULL, *url = NULL, *depth = NULL;
int quiet = 0;
int progress = 0;
- FILE *submodule_dot_git;
char *p, *path = NULL, *sm_gitdir;
struct strbuf rel_path = STRBUF_INIT;
struct strbuf sb = STRBUF_INIT;
@@ -653,27 +652,12 @@ static int module_clone(int argc, const char **argv, const char *prefix)
strbuf_reset(&sb);
}
- /* Write a .git file in the submodule to redirect to the superproject. */
- strbuf_addf(&sb, "%s/.git", path);
- if (safe_create_leading_directories_const(sb.buf) < 0)
- die(_("could not create leading directories of '%s'"), sb.buf);
- submodule_dot_git = fopen(sb.buf, "w");
- if (!submodule_dot_git)
- die_errno(_("cannot open file '%s'"), sb.buf);
-
- fprintf_or_die(submodule_dot_git, "gitdir: %s\n",
- relative_path(sm_gitdir, path, &rel_path));
- if (fclose(submodule_dot_git))
- die(_("could not close file %s"), sb.buf);
- strbuf_reset(&sb);
- strbuf_reset(&rel_path);
+ /* Connect module worktree and git dir */
+ connect_work_tree_and_git_dir(path, sm_gitdir);
- /* Redirect the worktree of the submodule in the superproject's config */
p = git_pathdup_submodule(path, "config");
if (!p)
die(_("could not get submodule directory for '%s'"), path);
- git_config_set_in_file(p, "core.worktree",
- relative_path(path, sm_gitdir, &rel_path));
/* setup alternateLocation and alternateErrorStrategy in the cloned submodule if needed */
git_config_get_string("submodule.alternateLocation", &sm_alternate);
--
2.12.0.192.gbdb9d28a5
^ permalink raw reply related
* Re: [PATCH] submodule--helper.c: remove duplicate code
From: Stefan Beller @ 2017-03-08 23:34 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Valery Tolstov, git@vger.kernel.org
In-Reply-To: <xmqq60jj1heh.fsf@gitster.mtv.corp.google.com>
On Wed, Mar 8, 2017 at 3:24 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>>> Also,
>>> should I start new thread for new patch series?
>>
>> As you like.
>> As far as I understand, it is very easy for Junio to take a whole
>> (sub-)thread of patches and apply that and make a branch with
>> multiple commits out of it as he has tooling for that.
>
> Note that the world does not revolve around _me_. I was once asked
> for my preference and I responded and that is what you are recalling
> here.
>
> Others on the list do review and keeping it easy for them to is also
> important. What's _your_ preference?
>
I use gmail, that has a broken threading model
(it groups emails by subject lines; apparently not using any "in-reply-to"
relationship to build up a thread), so I do not care. At all.
I do not know about the preference of the next most likely
people to review submodule code, so I refrained from giving advice
depending on reviewer preference, but instead gave advice that would
ease your work as I recall.
After thinking about it further, you may want to make sure that the
topic is coherent and it is easy to discover all related emails in an
archive of the mailing list. https://public-inbox.org/git/
So in the cover letter you could link to the previous thread,
https://public-inbox.org/git/20170308174449.24266-1-me@vtolstov.org/
or just continue that thread by replying to an email that is appropriate for
the series.
Thanks,
Stefan
^ permalink raw reply
* [PATCH v2 0/2] Remove duplicate code from module_clone()
From: Valery Tolstov @ 2017-03-09 0:03 UTC (permalink / raw)
To: git; +Cc: sbeller, me, gitster
In-Reply-To: <CAGZ79kbnpUtrKdjQdQ-r6rRuVvnawooLFk1bO8jOSgxNkx2Dbg@mail.gmail.com>
> Then the next step (as outlined by Documentation/SubmittingPatches)
> is to figure out how to best present this to the mailing list; I think the best
> way is to send out a patch series consisting of both of these 2 patches,
> the "connect_work_tree_and_git_dir: safely create leading directories,"
> first and then your deduplication patch.
Combined two patches
Stefan Beller (1):
connect_work_tree_and_git_dir: safely create leading directories
Valery Tolstov (1):
submodule--helper.c: remove duplicate code
builtin/submodule--helper.c | 20 ++------------------
dir.c | 32 +++++++++++++++++++++-----------
submodule.c | 11 ++---------
3 files changed, 25 insertions(+), 38 deletions(-)
--
2.12.0.192.gbdb9d28a5
^ permalink raw reply
* Re: [PATCH v2 0/2] Remove duplicate code from module_clone()
From: Brandon Williams @ 2017-03-09 0:28 UTC (permalink / raw)
To: Valery Tolstov; +Cc: git, sbeller, gitster
In-Reply-To: <20170309000352.18330-1-me@vtolstov.org>
On 03/09, Valery Tolstov wrote:
> > Then the next step (as outlined by Documentation/SubmittingPatches)
> > is to figure out how to best present this to the mailing list; I think the best
> > way is to send out a patch series consisting of both of these 2 patches,
> > the "connect_work_tree_and_git_dir: safely create leading directories,"
> > first and then your deduplication patch.
>
> Combined two patches
>
> Stefan Beller (1):
> connect_work_tree_and_git_dir: safely create leading directories
It looks like this patch is apart of Stefan's checkout series. So I was
slightly confused when first looking at this patch since I'd seen it before.
The usual protocol would be to rebase off of Stefan's series and build
on that (assuming you have a dependency against his series), indicating
that you are doing as such in your cover letter. Though what you have
here should hopefully give the maintainer enough context to know where to
put it, so you should be good :)
>
> Valery Tolstov (1):
> submodule--helper.c: remove duplicate code
>
> builtin/submodule--helper.c | 20 ++------------------
> dir.c | 32 +++++++++++++++++++++-----------
> submodule.c | 11 ++---------
> 3 files changed, 25 insertions(+), 38 deletions(-)
>
> --
> 2.12.0.192.gbdb9d28a5
>
--
Brandon Williams
^ permalink raw reply
* More about blob reachability (while fetching arbitrary blobs)
From: Jonathan Tan @ 2017-03-09 0:35 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan, git, markbt
There has been some talk about fetching blobs from repos with missing
objects [1] [2]. I took a further look at the issue of blob
reachability.
I have written earlier that there is a bug in rev-list when used to
compute reachability [3], but someone pointed me to bitmaps, and I found
out that "--use-bitmap-index" makes rev-list use an alternative code
path in the presence of a bitmap (whether fully covering or partially
covering the relevant objects), and that code doesn't contain that bug.
If reachability checks are desired, bitmaps are probably required for
performance anyway [4].
We should implement things so that users will be strongly recommended to
do one of the following:
1) configure explicitly (instead of it being the default) the
equivalent of allowanysha1inwant to disable reachability checks, or
2) use a bitmap (and it is recommended to periodically repack).
(This strong recommendation is currently a requirement due to the fact
that the 3rd option, "do nothing", is not possible due to the bug in
rev-list described above.)
I have included a small patch (below) to do that for upload-pack. (For
the case of upload-pack, which until now does not really need to process
blob wants, a possible alternative is to forbid trees and blobs from
being processed by upload-pack at all.)
[1] <20170304191901.9622-1-markbt@efaref.net>
[2] <1488999039-37631-1-git-send-email-git@jeffhostetler.com>
[3] <cover.1487984670.git.jonathantanmy@google.com>
[4] I checked, and got 0m0.281s for "time git rev-list --objects
--use-bitmap-index HEAD^{tree} --not --all" on a fully repacked
git.git. (Not sure whether that's OK performance.)
-- 8< --
Subject: [PATCH] upload-pack: forbid fetching unreachable blobs
If allowreachablesha1inwant is set, upload-pack will provide a blob to a
user, provided its hash, regardless of whether the blob is reachable or
not.
Partially fix this by (i) including the "--objects" argument, if
necessary, when invoking "rev-list", and (ii) including the
"--use-bitmap-index" argument.
(i) is so that rev-list operates on the correct granularity, depending
on the type of objects checked.
(ii) needs more explanation:
Our invocation of "rev-list" is supposed to return `A - B`, where `A`
is the set of all objects reachable from the objects that we want to
check the reachability of, and `B` is the set of all objects reachable
from any of our refs. If all our objects are reachable, then the
resulting set is empty. However, the non-bitmap-using part of the
code wrongly excludes some trees and blobs from `B`, making the
resulting set sometimes non-empty and thus wrongly concluding that an
object is unreachable when it is, in fact, reachable. (However, the
error does not run the other way - an object will not be indicated as
reachable when it is unreachable.)
The "--use-bitmap-index" argument partially solves this problem by
activating another code path when the serving repository has a bitmap.
This other code path does not contain the bug described above.
("--use-bitmap-index" has other benefits besides correctness, but has
not yet been included likely because bitmaps were introduced in commit
fff4275 ("pack-bitmap: add support for bitmap indexes", 2013-12-21),
later than the last time this part of the code was touched in commit
051e400 ("helping smart-http/stateless-rpc fetch race", 2011-08-05)).
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
t/t5500-fetch-pack.sh | 31 +++++++++++++++++++++++++++++++
upload-pack.c | 17 ++++++++++++++++-
2 files changed, 47 insertions(+), 1 deletion(-)
diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
index 505e1b4a7..1568aed82 100755
--- a/t/t5500-fetch-pack.sh
+++ b/t/t5500-fetch-pack.sh
@@ -547,6 +547,37 @@ test_expect_success 'fetch-pack can fetch a raw sha1' '
git fetch-pack hidden $(git -C hidden rev-parse refs/hidden/one)
'
+test_expect_success 'setup for tests that fetch blobs by hash' '
+ git init blobserver &&
+ test_commit -C blobserver 1 &&
+ git -C blobserver repack -a -d --write-bitmap-index &&
+ test_commit -C blobserver 2 &&
+ test_commit -C blobserver 3 &&
+ blob1=$(echo 1 | git hash-object --stdin) &&
+ blob2=$(echo 2 | git hash-object --stdin) &&
+ blob3=$(echo 3 | git hash-object --stdin) &&
+
+ unreachable=$(echo 4 | git -C blobserver hash-object -w --stdin) &&
+ git -C blobserver cat-file -e "$unreachable"
+'
+
+test_expect_success 'fetch-pack can fetch reachable blobs by hash' '
+ test_config -C blobserver uploadpack.allowreachablesha1inwant 1 &&
+
+ git init reachabletest &&
+ git -C reachabletest fetch-pack ../blobserver "$blob1" "$blob2" &&
+ git -C reachabletest cat-file -e "$blob1" &&
+ git -C reachabletest cat-file -e "$blob2" &&
+ test_must_fail git -C reachabletest cat-file -e "$blob3"
+'
+
+test_expect_success 'fetch-pack cannot fetch unreachable blobs' '
+ test_config -C blobserver uploadpack.allowreachablesha1inwant 1 &&
+
+ git init unreachabletest &&
+ test_must_fail git -C unreachabletest fetch-pack ../blobserver "$blob1" "$unreachable"
+'
+
check_prot_path () {
cat >expected <<-EOF &&
Diag: url=$1
diff --git a/upload-pack.c b/upload-pack.c
index 7597ba340..a00f0034c 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -469,7 +469,10 @@ static int do_reachable_revlist(struct child_process *cmd,
struct object_array *reachable)
{
static const char *argv[] = {
- "rev-list", "--stdin", NULL,
+ "rev-list", "--use-bitmap-index", "--stdin", NULL,
+ };
+ static const char *argv_with_objects[] = {
+ "rev-list", "--use-bitmap-index", "--objects", "--stdin", NULL,
};
struct object *o;
char namebuf[42]; /* ^ + SHA-1 + LF */
@@ -488,6 +491,18 @@ static int do_reachable_revlist(struct child_process *cmd,
*/
sigchain_push(SIGPIPE, SIG_IGN);
+ /*
+ * If we are testing reachability of a tree or blob, rev-list needs to
+ * operate more granularly.
+ */
+ for (i = 0; i < src->nr; i++) {
+ o = src->objects[i].item;
+ if (o->type == OBJ_TREE || o->type == OBJ_BLOB) {
+ cmd->argv = argv_with_objects;
+ break;
+ }
+ }
+
if (start_command(cmd))
goto error;
--
2.12.0.246.ga2ecc84866-goog
^ permalink raw reply related
* Re: Re: [PATCH v2 0/2] Remove duplicate code from module_clone()
From: Valery Tolstov @ 2017-03-09 0:56 UTC (permalink / raw)
To: bmwill; +Cc: git, sbeller, me, gitster
In-Reply-To: <20170309002818.GA153031@google.com>
> The usual protocol would be to rebase off of Stefan's series and build
> on that (assuming you have a dependency against his series), indicating
> that you are doing as such in your cover letter.
So, should I send only my patch, or current format (patch and dependency)
is acceptalbe?
Regards,
Valery Tolstov
^ permalink raw reply
* Re: What's cooking in git.git (Mar 2017, #03; Wed, 8)
From: brian m. carlson @ 2017-03-09 1:01 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqvarjz5yv.fsf@gitster.mtv.corp.google.com>
[-- Attachment #1: Type: text/plain, Size: 1611 bytes --]
On Wed, Mar 08, 2017 at 03:47:20PM -0800, Junio C Hamano wrote:
> * bc/object-id (2017-02-22) 19 commits
> - wt-status: convert to struct object_id
> - builtin/merge-base: convert to struct object_id
> - Convert object iteration callbacks to struct object_id
> - sha1_file: introduce an nth_packed_object_oid function
> - refs: simplify parsing of reflog entries
> - refs: convert each_reflog_ent_fn to struct object_id
> - reflog-walk: convert struct reflog_info to struct object_id
> - builtin/replace: convert to struct object_id
> - Convert remaining callers of resolve_refdup to object_id
> - builtin/merge: convert to struct object_id
> - builtin/clone: convert to struct object_id
> - builtin/branch: convert to struct object_id
> - builtin/grep: convert to struct object_id
> - builtin/fmt-merge-message: convert to struct object_id
> - builtin/fast-export: convert to struct object_id
> - builtin/describe: convert to struct object_id
> - builtin/diff-tree: convert to struct object_id
> - builtin/commit: convert to struct object_id
> - hex: introduce parse_oid_hex
>
> "uchar [40]" to "struct object_id" conversion continues.
>
> Now at v5.
> cf. <20170221234737.894681-1-sandals@crustytoothpaste.net>
Were you expecting more work on this series? I believe I've addressed
all the review comments that were outstanding, but if I've missed
something, please let me know.
--
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]
^ permalink raw reply
* Re: [PATCH v2 2/2] submodule--helper.c: remove duplicate code
From: Brandon Williams @ 2017-03-09 0:38 UTC (permalink / raw)
To: Valery Tolstov; +Cc: git, sbeller, gitster
In-Reply-To: <20170309000352.18330-3-me@vtolstov.org>
On 03/09, Valery Tolstov wrote:
> Remove code fragment from module_clone that duplicates functionality
> of connect_work_tree_and_git_dir in dir.c
>
> Signed-off-by: Valery Tolstov <me@vtolstov.org>
Patch looks good all the tests pass when running this on top of
Stefan's checkout series 'origin/sb/checkout-recurse-submodules'.
There is still one more bit of cleanup that you can do.
> ---
> builtin/submodule--helper.c | 20 ++------------------
> 1 file changed, 2 insertions(+), 18 deletions(-)
>
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index 899dc334e..405cbea07 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -579,7 +579,6 @@ static int module_clone(int argc, const char **argv, const char *prefix)
> const char *name = NULL, *url = NULL, *depth = NULL;
> int quiet = 0;
> int progress = 0;
> - FILE *submodule_dot_git;
> char *p, *path = NULL, *sm_gitdir;
> struct strbuf rel_path = STRBUF_INIT;
rel_path is no longer used so it and the call to strbuf_release() to
free its memory can be removed.
--
Brandon Williams
^ permalink raw reply
* Re: What's cooking in git.git (Mar 2017, #03; Wed, 8)
From: Jonathan Tan @ 2017-03-09 0:46 UTC (permalink / raw)
To: Junio C Hamano, git
In-Reply-To: <xmqqvarjz5yv.fsf@gitster.mtv.corp.google.com>
On 03/08/2017 03:47 PM, Junio C Hamano wrote:
> * jt/mark-tree-uninteresting-for-uninteresting-commit (2017-02-28) 3 commits
> - upload-pack: compute blob reachability correctly
> - revision: exclude trees/blobs given commit
> - revision: unify {tree,blob}_objects in rev_info
>
> The revision/object traversal machinery did not mark all tree and
> blob objects that are contained in an uninteresting commit as
> uninteresting, because that is quite costly. Instead, it only
> marked those that are contained in an uninteresting boundary commit
> as uninteresting.
>
> cf. <20170228215937.yd4juycjf7y3vish@sigill.intra.peff.net>
This is superseded by [1] which I just sent out. I currently have no
idea how to fix the "revision" commits to be correct and still
performant (and this might not be possible), so I wrote [1] which just
contains a partial "upload-pack" fix.
[1] <20170309003547.6930-1-jonathantanmy@google.com>
^ permalink raw reply
* Re: diff.ignoreSubmoudles config setting broken?
From: Stefan Beller @ 2017-03-09 1:30 UTC (permalink / raw)
To: Junio C Hamano
Cc: Sebastian Schuberth, Jacob Keller, Jeff King, Git Mailing List,
Jens Lehmann
In-Reply-To: <xmqqa88v1i5f.fsf@gitster.mtv.corp.google.com>
On Wed, Mar 8, 2017 at 3:08 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>> Yeah the .gitmodules file is a good hint.
And by that I meant that I am not sure if we're going
down the right rabbit hole here. So before we take action
maybe Sebastian can tell us more about his project (and all
configurations and settings involved)
>>
>> Here is my understanding of the precedence:
>>
>> command line options > .git/config (in various forms) > .gitmodules
>>
>> where in the .git config we have precedence levels for different files
>>
>> .git/config > ~/.gitconfig
>>
>> as well as different settings:
>>
>> submodule.<name>.ignore > diff.ignoreSubmodules
>
> I've never understood why people thought it a good idea to let
> .gitmodules supplied by the upstream override the configuration
> setting the end user has like this. This is quite bad.
Apart from from the name <-> path mapping, the .gitmodules
file is a collection of suggestions, some more severe than
others.
I think the issue here is to define the correct
and clear order of precedence, specifically between along these
2 different dimensions (different configuration settings vs different
files with configuration), such that the .gitmodules file is only ever
consulted when the user has obviously nothing configured that
would contradict the .gitmodules file.
>
> Perhaps this is a good starting point?
>
> diff.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/diff.c b/diff.c
> index a628ac3a95..75b7140c63 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -273,8 +273,11 @@ int git_diff_ui_config(const char *var, const char *value, void *cb)
> if (!strcmp(var, "diff.orderfile"))
> return git_config_pathname(&diff_order_file_cfg, var, value);
>
> - if (!strcmp(var, "diff.ignoresubmodules"))
> + if (!strcmp(var, "diff.ignoresubmodules")) {
> handle_ignore_submodules_arg(&default_diff_options, value);
> + DIFF_OPT_SET(options, OVERRIDE_SUBMODULE_CONFIG);
s/options/&default_diff_options/ makes it compile. (I did not think
whether that is
correct though.)
In other occurrences of handle_ignore_submodules_arg, the DIFF_OPT_SET
is set before the handle_ignore_submodules_arg, though.
When trying these suggestions, ./t4027-diff-submodule.sh breaks.
log on that file yields e.g. 302ad7a9930 (Submodules: Use "ignore" settings
from .gitmodules too for diff and status), which tells us that in 2010 people
were not as concerned by this, but the user had to use the exact option to
override the upstream default-suggestion.
Thanks,
Stefan
^ permalink raw reply
* [PATCH v3 2/2] submodule--helper.c: remove duplicate code
From: Valery Tolstov @ 2017-03-09 1:27 UTC (permalink / raw)
To: git; +Cc: bmwill, sbeller, me, gitster
In-Reply-To: <20170309012734.21541-1-me@vtolstov.org>
Remove code fragment from module_clone that duplicates functionality
of connect_work_tree_and_git_dir in dir.c
Signed-off-by: Valery Tolstov <me@vtolstov.org>
---
builtin/submodule--helper.c | 22 ++--------------------
1 file changed, 2 insertions(+), 20 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 899dc334e..86bafe166 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -579,9 +579,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)
const char *name = NULL, *url = NULL, *depth = NULL;
int quiet = 0;
int progress = 0;
- FILE *submodule_dot_git;
char *p, *path = NULL, *sm_gitdir;
- struct strbuf rel_path = STRBUF_INIT;
struct strbuf sb = STRBUF_INIT;
struct string_list reference = STRING_LIST_INIT_NODUP;
char *sm_alternate = NULL, *error_strategy = NULL;
@@ -653,27 +651,12 @@ static int module_clone(int argc, const char **argv, const char *prefix)
strbuf_reset(&sb);
}
- /* Write a .git file in the submodule to redirect to the superproject. */
- strbuf_addf(&sb, "%s/.git", path);
- if (safe_create_leading_directories_const(sb.buf) < 0)
- die(_("could not create leading directories of '%s'"), sb.buf);
- submodule_dot_git = fopen(sb.buf, "w");
- if (!submodule_dot_git)
- die_errno(_("cannot open file '%s'"), sb.buf);
-
- fprintf_or_die(submodule_dot_git, "gitdir: %s\n",
- relative_path(sm_gitdir, path, &rel_path));
- if (fclose(submodule_dot_git))
- die(_("could not close file %s"), sb.buf);
- strbuf_reset(&sb);
- strbuf_reset(&rel_path);
+ /* Connect module worktree and git dir */
+ connect_work_tree_and_git_dir(path, sm_gitdir);
- /* Redirect the worktree of the submodule in the superproject's config */
p = git_pathdup_submodule(path, "config");
if (!p)
die(_("could not get submodule directory for '%s'"), path);
- git_config_set_in_file(p, "core.worktree",
- relative_path(path, sm_gitdir, &rel_path));
/* setup alternateLocation and alternateErrorStrategy in the cloned submodule if needed */
git_config_get_string("submodule.alternateLocation", &sm_alternate);
@@ -689,7 +672,6 @@ static int module_clone(int argc, const char **argv, const char *prefix)
free(error_strategy);
strbuf_release(&sb);
- strbuf_release(&rel_path);
free(sm_gitdir);
free(path);
free(p);
--
2.12.0.192.gbdb9d28a5
^ 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