* RE: merge --no-ff is NOT mentioned in help
From: Vanderhoof, Tzadik @ 2016-11-18 14:51 UTC (permalink / raw)
To: Jeff King, Junio C Hamano; +Cc: Mike Rappazzo, git@vger.kernel.org
In-Reply-To: <20161117222142.mca6lmhj5mvl4gbp@sigill.intra.peff.net>
> On Thu, Nov 17, 2016 at 09:10:22AM -0800, Junio C Hamano wrote:
>
> > People interested may want to try the attached single-liner patch to
> > see how the output from _ALL_ commands that use parse-options API
> > looks when given "-h". It could be that the result may not be too
> > bad.
>
> The output is less ugly than I expected, but still a bit cluttered IMHO.
> I was surprised that the column-adjustment did not need tweaked, but the code correctly increments "pos" from the return value of fprintf, which just works.
>
> Looking at the output for --ff, though:
>
> --[no-]ff allow fast-forward (default)
>
> I do not think it's improving the situation nearly as much as if we made the primary option "--no-ff" with a NONEG flga, and then added back in a HIDDEN "--ff". I thought we had done that in other cases, but I can't seem to find any. But it would make "--no-ff" the primary form, which makes sense, as "--ff" is already the default.
>
> Another option would be to teach parse-options to somehow treat the negated form as primary in the help text. That's a bit more code, but might be usable in other places.
>
> -Peff
>
What about leaving the help as is, but adding a sentence at the end (or beginning?) like: "The following options may be negated by adding 'no-' after the double dashes"?
This e-mail, including attachments, may include confidential and/or
proprietary information, and may be used only by the person or entity
to which it is addressed. If the reader of this e-mail is not the intended
recipient or his or her authorized agent, the reader is hereby notified
that any dissemination, distribution or copying of this e-mail is
prohibited. If you have received this e-mail in error, please notify the
sender by replying to this message and delete this e-mail immediately.
^ permalink raw reply
* Re: [PATCH v1 12/19] Documentation/config: add splitIndex.maxPercentChange
From: Christian Couder @ 2016-11-18 14:34 UTC (permalink / raw)
To: Duy Nguyen
Cc: Junio C Hamano, Ævar Arnfjörð Bjarmason, git,
Christian Couder
In-Reply-To: <CACsJy8BZNfESmFv=V89Cq-b+aMJWLH=qhXHNE8inZZRjvXB33Q@mail.gmail.com>
On Mon, Nov 7, 2016 at 10:38 AM, Duy Nguyen <pclouds@gmail.com> wrote:
> (sorry I got sick in the last few weeks and could not respond to this earlier)
(Yeah, I have also been sick during the last few weeks.)
> On Mon, Nov 7, 2016 at 4:44 AM, Christian Couder
> <christian.couder@gmail.com> wrote:
>> Le 6 nov. 2016 09:16, "Junio C Hamano" <gitster@pobox.com> a écrit :
>>>
>>> Christian Couder <christian.couder@gmail.com> writes:
>>>
>>> > I think it is easier for user to be able to just set core.splitIndex
>>> > to true to enable split-index.
>>>
>>> You can have that exact benefit by making core.splitIndex to
>>> bool-or-more. If your default is 20%, take 'true' as if the user
>>> specified 20% and take 'false' as if the user specified 100% (or is
>>> it 0%? I do not care about the details but you get the point).
>
>> Then if we ever add 'auto' and the user wants for example 10% instead of the
>> default 20%, we will have to make it accept things like "auto,10".
(Sorry for writing the above on my phone which added HTML, so that it
didn't reach the list.)
> In my opinion, "true" _is_ auto, which is a way to say "I trust you to
> do the right thing, just re-split the index when it makes sense", "no"
> is disabled of course. If the user wants to be specific, just write
> "10" or some other percentage.(and either 0 or 100 would mean enable
> split-index but do not re-split automatically, let _me_ do it when I
> want it)
The meaning of a future "auto" option for "core.splitIndex" could be
"use the split-index feature only if the number of entries in whole
index is greater than 10000 (by default)".
If there is no difference between "true" and "auto" then, when users
who have "core.splitIndex=true" will migrate to the git version that
adds the "auto" feature, their repos with under 10000 entires will not
use the split-index feature anymore. These users may then be annoyed
that the behavior has been switched under them, and that the
split-index feature is not always used despite having
"core.splitIndex=true" in their config.
^ permalink raw reply
* Staging chunks can get confused
From: Daurnimator @ 2016-11-18 11:29 UTC (permalink / raw)
To: Git Mailing List
Tonight I was staging changes with `git add -p` and noticed they got
applied at the *wrong* location.
My guess is that when you stage a hunk it doesn't do a line number
fix-up for earlier unstaged hunks in the file.
Screencast: https://asciinema.org/a/7y9qhr0837a7t96m8w14mupnk
Alternatively, an example follows:
$ git add -p
diff --git a/unistring/unistring.c b/unistring/unistring.c
index 62bbc8a..45ced5d 100644
--- a/unistring/unistring.c
+++ b/unistring/unistring.c
@@ -17,6 +17,27 @@ static const char *const uninormnames[] = {"NFD",
"NFC", "NFKD", "NFKC", NULL};
#define lunistring_optuninorm(L, arg)
(lua_isnoneornil(L,(arg))?NULL:lunistring_checkuninorm((L), (arg)))
+const uint8_t * u8_check (const uint8_t *s, size_t n);
+int u8_mblen (const uint8_t *s, size_t n);
+int u8_mbtouc_unsafe (ucs4_t *puc, const uint8_t *s, size_t n);
+int u8_mbtouc (ucs4_t *puc, const uint8_t *s, size_t n);
+int u8_mbtoucr (ucs4_t *puc, const uint8_t *s, size_t n);
+int u8_uctomb (uint8_t *s, ucs4_t uc, int n);
+size_t u8_mbsnlen (const uint8_t *s, size_t n);
+
+int u8_strmblen (const uint8_t *s);
+int u8_strmbtouc (ucs4_t *puc, const uint8_t *s);
+const uint8_t * u8_next (ucs4_t *puc, const uint8_t *s);
+const uint8_t * u8_prev (ucs4_t *puc, const uint8_t *s, const uint8_t *start);
+
+uint8_t * u8_conv_from_encoding (const char *fromcode, enum
iconv_ilseq_handler handler, const char *src, size_t srclen, size_t
*offsets, uint8_t *resultbuf, size_t *lengthp);
+char * u8_conv_to_encoding (const char *tocode, enum
iconv_ilseq_handler handler, const uint8_t *src, size_t srclen, size_t
*offsets, char *resultbuf, size_t *lengthp);
+uint8_t * u8_strconv_from_encoding (const char *string, const char
*fromcode, enum iconv_ilseq_handler handler);
+char * u8_strconv_to_encoding (const uint8_t *string, const char
*tocode, enum iconv_ilseq_handler handler);
+uint8_t * u8_strconv_from_locale (const char *string);
+char * u8_strconv_to_locale (const uint8_t *string);
+
+
static int lunistring_width(lua_State *L) {
size_t n;
const uint8_t *s = (const uint8_t*)luaL_checklstring(L, 1, &n);
Stage this hunk [y,n,q,a,d,/,j,J,g,e,?]? n
@@ -27,6 +48,17 @@ static int lunistring_width(lua_State *L) {
}
+int u8_strwidth (const uint8_t *s, const char *encoding);
+void u8_wordbreaks (const uint8_t *s, size_t n, char *p);
+void u8_possible_linebreaks (const uint8_t *s, size_t n, const char
*encoding, char *p);
+int u8_width_linebreaks (const uint8_t *s, size_t n, int width, int
start_column, int at_end_columns, const char *override, const char
*encoding, char *p);
+
+
+int uc_decomposition (ucs4_t uc, int *decomp_tag, ucs4_t *decomposition);
+int uc_canonical_decomposition (ucs4_t uc, ucs4_t *decomposition);
+ucs4_t uc_composition (ucs4_t uc1, ucs4_t uc2);
+
+
static int lunistring_normalize(lua_State *L) {
uninorm_t nf = lunistring_optuninorm(L, 1);
size_t n;
Stage this hunk [y,n,q,a,d,/,K,j,J,g,e,?]? n
@@ -54,6 +86,9 @@ static int lunistring_normalize(lua_State *L) {
}
+int u8_normcmp (const uint8_t *s1, size_t n1, const uint8_t *s2,
size_t n2, uninorm_t nf, int *resultp);
+
+
static int lunistring_normxfrm(lua_State *L) {
size_t n;
const uint8_t *s = (const uint8_t*)luaL_checklstring(L, 1, &n);
Stage this hunk [y,n,q,a,d,/,K,j,J,g,e,?]? n
@@ -83,6 +118,9 @@ static int lunistring_normxfrm(lua_State *L) {
}
+int u8_normcoll (const uint8_t *s1, size_t n1, const uint8_t *s2,
size_t n2, uninorm_t nf, int *resultp);
+
+
static int lunistring_uc_locale_language(lua_State *L) {
lua_pushstring(L, uc_locale_language());
return 1;
Stage this hunk [y,n,q,a,d,/,K,j,J,g,e,?]? n
@@ -173,6 +211,15 @@ static int lunistring_totitle(lua_State *L) {
}
+casing_prefix_context_t u8_casing_prefix_context (const uint8_t *s, size_t n);
+casing_prefix_context_t u8_casing_prefixes_context (const uint8_t *s,
size_t n, casing_prefix_context_t a_context);
+casing_suffix_context_t u8_casing_suffix_context (const uint8_t *s, size_t n);
+casing_suffix_context_t u8_casing_suffixes_context (const uint8_t *s,
size_t n, casing_suffix_context_t a_context);
+uint8_t * u8_ct_toupper (const uint8_t *s, size_t n,
casing_prefix_context_t prefix_context, casing_suffix_context_t
suffix_context, const char *iso639_language, uninorm_t nf, uint8_t
*resultbuf, size_t *lengthp);
+uint8_t * u8_ct_tolower (const uint8_t *s, size_t n,
casing_prefix_context_t prefix_context, casing_suffix_context_t
suffix_context, const char *iso639_language, uninorm_t nf, uint8_t
*resultbuf, size_t *lengthp);
+uint8_t * u8_ct_totitle (const uint8_t *s, size_t n,
casing_prefix_context_t prefix_context, casing_suffix_context_t
suffix_context, const char *iso639_language, uninorm_t nf, uint8_t
*resultbuf, size_t *lengthp);
+
+
static int lunistring_casefold(lua_State *L) {
size_t n;
const uint8_t *s = (const uint8_t*)luaL_checklstring(L, 1, &n);
Stage this hunk [y,n,q,a,d,/,K,j,J,g,e,?]? n
@@ -201,6 +248,10 @@ static int lunistring_casefold(lua_State *L) {
}
+uint8_t * u8_ct_casefold (const uint8_t *s, size_t n,
casing_prefix_context_t prefix_context, casing_suffix_context_t
suffix_context, const char *iso639_language, uninorm_t nf, uint8_t
*resultbuf, size_t *lengthp);
+int u8_casecmp (const uint8_t *s1, size_t n1, const uint8_t *s2,
size_t n2, const char *iso639_language, uninorm_t nf, int *resultp);
+
+
static int lunistring_casexfrm(lua_State *L) {
size_t n;
const uint8_t *s = (const uint8_t*)luaL_checklstring(L, 1, &n);
Stage this hunk [y,n,q,a,d,/,K,j,J,g,e,?]? n
@@ -230,6 +281,9 @@ static int lunistring_casexfrm(lua_State *L) {
}
+int u8_casecoll (const uint8_t *s1, size_t n1, const uint8_t *s2,
size_t n2, const char *iso639_language, uninorm_t nf, int *resultp);
+
+
static int lunistring_casecoll(lua_State *L) {
size_t n1, n2;
const uint8_t *s1 = (const uint8_t*)luaL_checklstring(L, 1, &n1);
Stage this hunk [y,n,q,a,d,/,K,j,J,g,e,?]? n
@@ -288,7 +342,7 @@ static int lunistring_is_titlecase(lua_State *L) {
return luaL_fileresult(L, 0, NULL);
}
- lua_pushboolean(L, resultp);
+ lua_pushinteger(L, resultp);
return 1;
}
Stage this hunk [y,n,q,a,d,/,K,g,e,?]? y
$ git diff --cached
diff --git a/unistring/unistring.c b/unistring/unistring.c
index 62bbc8a..ea85d65 100644
--- a/unistring/unistring.c
+++ b/unistring/unistring.c
@@ -318,7 +318,7 @@ static int lunistring_is_cased(lua_State *L) {
return luaL_fileresult(L, 0, NULL);
}
- lua_pushboolean(L, resultp);
+ lua_pushinteger(L, resultp);
return 1;
}
^ permalink raw reply related
* Re: [PATCH v7 13/17] ref-filter: add `:dir` and `:base` options for ref printing atoms
From: Jacob Keller @ 2016-11-18 8:19 UTC (permalink / raw)
To: Karthik Nayak; +Cc: Junio C Hamano, Git mailing list
In-Reply-To: <CAOLa=ZTVTZ+1dXpcp=kdoGbT1Feq=vOfFpNpBiZepajMucraPQ@mail.gmail.com>
On Thu, Nov 17, 2016 at 11:33 PM, Karthik Nayak <karthik.188@gmail.com> wrote:
> Hey,
>
> On Fri, Nov 18, 2016 at 12:05 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Karthik Nayak <karthik.188@gmail.com> writes:
>>
>>> On Tue, Nov 15, 2016 at 11:12 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>>> Jacob Keller <jacob.keller@gmail.com> writes:
>>>> ...
>>>> I think you are going in the right direction. I had a similar
>>>> thought but built around a different axis. I.e. if strip=1 strips
>>>> one from the left, perhaps we want to have rstrip=1 that strips one
>>>> from the right, and also strip=-1 to mean strip everything except
>>>> one from the left and so on?
>>> ...
>>
>>> If we do implement strip with negative numbers, it definitely
>>> would be neat, but to get the desired feature which I've mentioned
>>> below, we'd need to call strip twice, i.e
>>> to get remotes from /refs/foo/abc/xyz we'd need to do
>>> strip=1,strip=-1, which could be
>>> done but ...
>>
>> ... would be unnecessary if this is the only use case:
>>
>>> strbuf_addf(&fmt,
>>> "%%(if:notequals=remotes)%%(refname:base)%%(then)%s%%(else)%s%%(end)",
>>> local.buf, remote.buf);
>>
>> You can "strip to leave only 2 components" and compare the result
>> with refs/remotes instead, no?
>>
>
> Of course, my only objective was that someone would find it useful to
> have these two additional
> atoms. So if you think it's unnecessary we could drop it entirely :D
>
> --
> Regards,
> Karthik Nayak
I think having strip and rstrip make sense, (along with support for
negative numbers) I don't think we need to make them work together
unless someone is interested, since we can use strip=-2 to get the
behavior we need today.
Thanks,
Jake
^ permalink raw reply
* Re: [PATCH v7 13/17] ref-filter: add `:dir` and `:base` options for ref printing atoms
From: Karthik Nayak @ 2016-11-18 7:33 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jacob Keller, Git mailing list
In-Reply-To: <xmqqmvgynee4.fsf@gitster.mtv.corp.google.com>
Hey,
On Fri, Nov 18, 2016 at 12:05 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> On Tue, Nov 15, 2016 at 11:12 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>> Jacob Keller <jacob.keller@gmail.com> writes:
>>> ...
>>> I think you are going in the right direction. I had a similar
>>> thought but built around a different axis. I.e. if strip=1 strips
>>> one from the left, perhaps we want to have rstrip=1 that strips one
>>> from the right, and also strip=-1 to mean strip everything except
>>> one from the left and so on?
>> ...
>
>> If we do implement strip with negative numbers, it definitely
>> would be neat, but to get the desired feature which I've mentioned
>> below, we'd need to call strip twice, i.e
>> to get remotes from /refs/foo/abc/xyz we'd need to do
>> strip=1,strip=-1, which could be
>> done but ...
>
> ... would be unnecessary if this is the only use case:
>
>> strbuf_addf(&fmt,
>> "%%(if:notequals=remotes)%%(refname:base)%%(then)%s%%(else)%s%%(end)",
>> local.buf, remote.buf);
>
> You can "strip to leave only 2 components" and compare the result
> with refs/remotes instead, no?
>
Of course, my only objective was that someone would find it useful to
have these two additional
atoms. So if you think it's unnecessary we could drop it entirely :D
--
Regards,
Karthik Nayak
^ permalink raw reply
* Re: [PATCH] submodules: allow empty working-tree dirs in merge/cherry-pick
From: Junio C Hamano @ 2016-11-18 4:47 UTC (permalink / raw)
To: David Turner; +Cc: git, sbeller
In-Reply-To: <1478543491-6286-1-git-send-email-dturner@twosigma.com>
David Turner <dturner@twosigma.com> writes:
> diff --git a/t/t3030-merge-recursive.sh b/t/t3030-merge-recursive.sh
> index 470f334..be074a1 100755
> --- a/t/t3030-merge-recursive.sh
> +++ b/t/t3030-merge-recursive.sh
> @@ -575,13 +575,13 @@ test_expect_success 'merge removes empty directories' '
> test_must_fail test -d d
> '
>
> -test_expect_failure 'merge-recursive simple w/submodule' '
> +test_expect_success 'merge-recursive simple w/submodule' '
>
> git checkout submod &&
> git merge remove
> '
>
> -test_expect_failure 'merge-recursive simple w/submodule result' '
> +test_expect_sucess 'merge-recursive simple w/submodule result' '
Here is a typo. I wonder if we want to do "set -e" at the end of
test-lib.sh to catch a breakage like this. I only caught it by
being lucky (I was staring "make test" output as it flew by).
I've already amended the copy I have, but in case you are going to
reroll in the future, please do not forget to update your copy.
^ permalink raw reply
* Re: [PATCH 09/16] update submodules: add scheduling to update submodules
From: Stefan Beller @ 2016-11-18 0:28 UTC (permalink / raw)
To: Brandon Williams
Cc: git@vger.kernel.org, Junio C Hamano, Jonathan Nieder, Martin Fick,
David Turner
In-Reply-To: <20161116000236.GF66382@google.com>
On Tue, Nov 15, 2016 at 4:02 PM, Brandon Williams <bmwill@google.com> wrote:
> On 11/15, Stefan Beller wrote:
>> +
>> + child_process_clear(&cp);
>> + return 0;
>> +}
>
> If run command is successful then it handles the clearing of the child
> process struct, correct? Is there a negative to having all the explicit
> clears when the child was successful?
void child_process_clear(struct child_process *child)
{
argv_array_clear(&child->args);
argv_array_clear(&child->env_array);
}
I don't think so, as clearing empty arg arrays is a no op.
>> +#define SCHEDULED_SUBMODULES_INIT {NULL, NULL}
>
> I may not know enough about these types of initializors but that Init
> macro only has 2 entries while there are three entries in the struct
> itself.
Filled up to 3 to be explicit.
>
>> +
>> +int scheduled_submodules_nr, scheduled_submodules_alloc;
>
> Should these globals be static since they should be scoped to only this
> file?
Of course, done.
>
> nit: organization wise it makes more sense to me to have the
> 'update_submodule' helper function be located more closely to the
> 'update_submodules' function.
>
done
David wrote:
> In fact, only the first NULL is necessary; unspecified initializer entries in C default to zero.
as said above, I explicitly init all of them now.
^ permalink raw reply
* Re: [PATCH 08/16] update submodules: add depopulate_submodule
From: Stefan Beller @ 2016-11-18 0:16 UTC (permalink / raw)
To: Brandon Williams
Cc: git@vger.kernel.org, Junio C Hamano, Jonathan Nieder, Martin Fick,
David Turner
In-Reply-To: <CAGZ79ka0-JFvogHRoTA4ioMK86zD=zkgEfBb-gpU8tbOjwEoFA@mail.gmail.com>
On Thu, Nov 17, 2016 at 2:42 PM, Stefan Beller <sbeller@google.com> wrote:
>
> I think I'll just write this functionality in C and optionally expose
> it via the submodule--helper,
> such that the user facing git-submodule.sh only has to call that helper.
I think it will roughly look like this:
(white space mangled)
commit e72ef244c667920c874247aa32aa55845500aac8
Author: Stefan Beller <sbeller@google.com>
Date: Thu Nov 17 16:14:46 2016 -0800
submodule--helper: add intern-git-dir function
When a submodule has its git dir inside the working dir, the submodule
support for checkout that we plan to add in a later patch will fail.
Add functionality to migrate the git directory to be embedded
into the superprojects git directory.
Signed-off-by: Stefan Beller <sbeller@google.com>
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 4beeda5..4f31100 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1076,6 +1076,21 @@ static int resolve_remote_submodule_branch(int
argc, const char **argv,
return 0;
}
+static int intern_git_dir(int argc, const char **argv, const char *prefix)
+{
+ int i;
+ struct pathspec pathspec;
+ struct module_list list = MODULE_LIST_INIT;
+
+ if (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0)
+ return 1;
+
+ for (i = 0; i < list.nr; i++)
+ migrate_submodule_gitdir(list.entries[i]->name);
+
+ return 0;
+}
+
struct cmd_struct {
const char *cmd;
int (*fn)(int, const char **, const char *);
@@ -1090,7 +1105,8 @@ static struct cmd_struct commands[] = {
{"resolve-relative-url", resolve_relative_url},
{"resolve-relative-url-test", resolve_relative_url_test},
{"init", module_init},
- {"remote-branch", resolve_remote_submodule_branch}
+ {"remote-branch", resolve_remote_submodule_branch},
+ {"intern-git-dir", intern_git_dir}
};
int cmd_submodule__helper(int argc, const char **argv, const char *prefix)
diff --git a/submodule.c b/submodule.c
index 45b9060..e513bba 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1335,3 +1335,42 @@ void prepare_submodule_repo_env(struct argv_array *out)
}
argv_array_push(out, "GIT_DIR=.git");
}
+
+/*
+ * Migrate the given submodule (and all its submodules recursively) from
+ * having its git directory within the working tree to the git dir nested
+ * in its superprojects git dir under modules/.
+ */
+void migrate_submodule_gitdir(const char *path)
+{
+ char *old_git_dir;
+ const char *new_git_dir;
+ const struct submodule *sub;
+
+ struct child_process cp = CHILD_PROCESS_INIT;
+ cp.git_cmd = 1;
+ cp.no_stdin = 1;
+ cp.dir = path;
+ argv_array_pushl(&cp.args, "submodule", "foreach", "--recursive",
+ "git", "submodule--helper" "intern-git-dir", NULL);
+
+ if (run_command(&cp))
+ die(_("Could not migrate git directory in submodule '%s'"),
+ path);
+
+ old_git_dir = xstrfmt("%s/.git", path);
+ if (read_gitfile(old_git_dir))
+ /* If it is an actual gitfile, it doesn't need migration. */
+ goto out;
+
+ sub = submodule_from_path(null_sha1, path);
+ new_git_dir = git_common_path("modules/%s", sub->name);
+
+ if (rename(old_git_dir, new_git_dir) < 0)
+ die_errno(_("Could not migrate git directory from %s to %s"),
+ old_git_dir, new_git_dir);
+
+ connect_work_tree_and_git_dir(path, new_git_dir);
+out:
+ free(old_git_dir);
+}
diff --git a/submodule.h b/submodule.h
index aac202c..143ec18 100644
--- a/submodule.h
+++ b/submodule.h
@@ -90,5 +90,6 @@ extern int parallel_submodules(void);
* retaining any config in the environment.
*/
extern void prepare_submodule_repo_env(struct argv_array *out);
+extern void migrate_submodule_gitdir(const char *path);
#endif
^ permalink raw reply related
* Re: [PATCH v3 5/6] grep: enable recurse-submodules to work on <tree> objects
From: Brandon Williams @ 2016-11-17 23:34 UTC (permalink / raw)
To: Stefan Beller; +Cc: git@vger.kernel.org, Jonathan Tan, Junio C Hamano
In-Reply-To: <CAGZ79kbqZs+wQYUBnvdw1YhC4dtHeodJWdved+it2Zg076vyOA@mail.gmail.com>
On 11/15, Stefan Beller wrote:
> On Fri, Nov 11, 2016 at 3:51 PM, Brandon Williams <bmwill@google.com> wrote:
> > + /*
> > + * Add basename of parent project
> > + * When performing grep on a <tree> object the filename is prefixed
> > + * with the object's name: '<tree-name>:filename'.
>
> This comment is hard to read as it's unclear what the <angle brackets> mean.
> (Are the supposed to indicate a variable? If so why is file name not marked up?)
Yeah you're right, the angle brackets don't really add anything to the
comment. I'll drop them.
> > In order to
> > + * provide uniformity of output we want to pass the name of the
> > + * parent project's object name to the submodule so the submodule can
> > + * prefix its output with the parent's name and not its own SHA1.
> > + */
> > + if (end_of_base)
> > + argv_array_pushf(&cp.args, "--parent-basename=%.*s",
> > + (int) (end_of_base - gs->name),
> > + gs->name);
>
> Do we pass this only with the tree-ish?
> What if we are grepping the working tree and the file name contains a colon?
Actually you're right, this would only happen if we are passing a
tree-ish, which has a tree-name prefixed to the filename. I'll add that
as an additional check to ensure that this handles file names with a
colon correctly....though why you have a colon in a filename is beyond
me :P
> > +test_expect_success 'grep tree HEAD^' '
> > + cat >expect <<-\EOF &&
> > + HEAD^:a:foobar
> > + HEAD^:b/b:bar
> > + HEAD^:submodule/a:foobar
> > + EOF
> > +
> > + git grep -e "bar" --recurse-submodules HEAD^ > actual &&
> > + test_cmp expect actual
> > +'
> > +
> > +test_expect_success 'grep tree HEAD^^' '
> > + cat >expect <<-\EOF &&
> > + HEAD^^:a:foobar
> > + HEAD^^:b/b:bar
> > + EOF
> > +
> > + git grep -e "bar" --recurse-submodules HEAD^^ > actual &&
> > + test_cmp expect actual
> > +'
> > +
> > +test_expect_success 'grep tree and pathspecs' '
> > + cat >expect <<-\EOF &&
> > + HEAD:submodule/a:foobar
> > + HEAD:submodule/sub/a:foobar
> > + EOF
> > +
> > + git grep -e "bar" --recurse-submodules HEAD -- submodule > actual &&
> > + test_cmp expect actual
> > +'
>
> Mind to add tests for
> * recursive submodules (say 2 levels), preferrably not having the
> gitlink at the root each, i.e. root has a sub1 at path subs/sub1 and
> sub1 has a sub2
> at path subs/sub2, such that recursing would produce a path like
> HEAD:subs/sub1/subs/sub2/dir/file ?
> * file names with a colon in it
> * instead of just HEAD referencing trees, maybe a sha1 referenced test as well
> (though it is not immediately clear what the benefit would be)
> * what if the submodule doesn't have the commit referenced in the given sha1
I'll add more tests too!
--
Brandon Williams
^ permalink raw reply
* Re: [PATCH 08/16] update submodules: add depopulate_submodule
From: Stefan Beller @ 2016-11-17 22:42 UTC (permalink / raw)
To: Brandon Williams
Cc: git@vger.kernel.org, Junio C Hamano, Jonathan Nieder, Martin Fick,
David Turner
In-Reply-To: <20161117222926.GN66382@google.com>
On Thu, Nov 17, 2016 at 2:29 PM, Brandon Williams <bmwill@google.com> wrote:
> On 11/17, Stefan Beller wrote:
>> Well first you get the warning:
>>
>> "cannot remove submodule '%s' because it (or one of "
>> "its nested submodules) uses a .git directory"),
>>
>> and in case a d/f/ conflict arises in a later stage (e.g. when the submodule
>> is replaced by a file or symlink), you get another related error with
>> less helpful description how to debug it.
>
> Maybe a warning isn't the right thing? Shouldn't the checkout fail if
> there are any issues? This would force the user to stash/commit their
> changes and then retry.
Well if the path is not reused, e.g. you just delete a submodule in a commit
without anything else, you could proceed and have the submodule laying
around dirty?
>
>> > All other submodules will actually be removed? Couldn't
>> > you end up in an undesirable state with a checkout effecting one
>> > submodule but not another?
>>
>> Yes you could. Maybe it's time to add
>> "git submodule intern-git-dir", which can be given as a helpful hint
>> or even run here first.
>
> That would be a good idea, does that functionality already exist in one
> form or another? I'm assuming it must since git update does just that
> when cloning a submodule.
No it doesn't (it is roughly these three steps):
mv ${SUBMODULE_PATH}/.git ${GIT_DIR}/modules/${SUBMODULE_NAME}
git config -f ${GIT_DIR}/modules/${SUBMODULE_NAME}/config
core.worktree ${SUBMODULE_PATH}
echo "gitdir: ${GIT_DIR}/modules/${SUBMODULE_NAME}" >
${SUBMODULE_PATH}/.git
The last 2 steps are done via
void connect_work_tree_and_git_dir(const char *work_tree, const
char *git_dir);
in submodule.{c,h}
However we'd need to make sure the first step is performed correctly. (and make
damn sure we don't loose that git dir), so I think rename(2) does the
correct thing
for directories, except when these two locations are on a different mount point.
I think I'll just write this functionality in C and optionally expose
it via the submodule--helper,
such that the user facing git-submodule.sh only has to call that helper.
^ permalink raw reply
* Re: [PATCH 08/16] update submodules: add depopulate_submodule
From: Brandon Williams @ 2016-11-17 22:29 UTC (permalink / raw)
To: Stefan Beller
Cc: git@vger.kernel.org, Junio C Hamano, Jonathan Nieder, Martin Fick,
David Turner
In-Reply-To: <CAGZ79kbCqLsRzrsX29uM7pobs_11UZtFOQWP9RO8ptS5PyDfmw@mail.gmail.com>
On 11/17, Stefan Beller wrote:
> Well first you get the warning:
>
> "cannot remove submodule '%s' because it (or one of "
> "its nested submodules) uses a .git directory"),
>
> and in case a d/f/ conflict arises in a later stage (e.g. when the submodule
> is replaced by a file or symlink), you get another related error with
> less helpful description how to debug it.
Maybe a warning isn't the right thing? Shouldn't the checkout fail if
there are any issues? This would force the user to stash/commit their
changes and then retry.
> > All other submodules will actually be removed? Couldn't
> > you end up in an undesirable state with a checkout effecting one
> > submodule but not another?
>
> Yes you could. Maybe it's time to add
> "git submodule intern-git-dir", which can be given as a helpful hint
> or even run here first.
That would be a good idea, does that functionality already exist in one
form or another? I'm assuming it must since git update does just that
when cloning a submodule.
--
Brandon Williams
^ permalink raw reply
* Re: [PATCH 08/16] update submodules: add depopulate_submodule
From: Stefan Beller @ 2016-11-17 22:28 UTC (permalink / raw)
To: Heiko Voigt
Cc: git@vger.kernel.org, Brandon Williams, Junio C Hamano,
Jonathan Nieder, Martin Fick, David Turner
In-Reply-To: <20161117111337.GD39230@book.hvoigt.net>
On Thu, Nov 17, 2016 at 3:13 AM, Heiko Voigt <hvoigt@hvoigt.net> wrote:
> On Tue, Nov 15, 2016 at 03:06:43PM -0800, Stefan Beller wrote:
>> diff --git a/cache.h b/cache.h
>> index a50a61a..65c47e4 100644
>> --- a/cache.h
>> +++ b/cache.h
>> @@ -2018,4 +2018,6 @@ void sleep_millisec(int millisec);
>> */
>> void safe_create_dir(const char *dir, int share);
>>
>> +void remove_subtree_or_die(const char *path);
>
> It seems that it is called remove_subtree already internally but can we
> maybe change that name? The term 'subtree' refers to something else[1] for
> me.
Doh, right.
> Maybe just: remove_directory() would make it also clear that there
> is no special internal git datatype meant by that but just a directory
> in the filesystem.
I'll go with `remove_directory_or_die` for now.
>
>> +
>> #endif /* CACHE_H */
>> diff --git a/entry.c b/entry.c
>> index c6eea24..019826b 100644
>> --- a/entry.c
>> +++ b/entry.c
>> @@ -73,6 +73,14 @@ static void remove_subtree(struct strbuf *path)
>> die_errno("cannot rmdir '%s'", path->buf);
>> }
>>
>> +void remove_subtree_or_die(const char *path)
>> +{
>> + struct strbuf sb = STRBUF_INIT;
>> + strbuf_addstr(&sb, path);
>> + remove_subtree(&sb);
>> + strbuf_release(&sb);
>> +}
>
> Why are you exposing it with const char * instead of strbuf? We get
> unnecessary conversions in case a caller already has a strbuf ready.
> Just in case later code also wants to use it.
For the cleanliness of API design. (I thought `void remove(char *dir)` is
the closest approximation of `rm -rf $1`)
I'll use a strbuf instead.
Thanks!
Stefan
^ permalink raw reply
* [ANNOUNCE] Git v2.11.0-rc2
From: Junio C Hamano @ 2016-11-17 22:24 UTC (permalink / raw)
To: git; +Cc: Linux Kernel
A release candidate Git v2.11.0-rc2 is now available for testing
at the usual places. It is comprised of 646 non-merge commits
since v2.10.0, contributed by 68 people, 14 of which are new faces.
The tarballs are found at:
https://www.kernel.org/pub/software/scm/git/testing/
The following public repositories all have a copy of the
'v2.11.0-rc2' tag and the 'master' branch that the tag points at:
url = https://kernel.googlesource.com/pub/scm/git/git
url = git://repo.or.cz/alt-git.git
url = git://git.sourceforge.jp/gitroot/git-core/git.git
url = git://git-core.git.sourceforge.net/gitroot/git-core/git-core
url = https://github.com/gitster/git
New contributors whose contributions weren't in v2.10.0 are as follows.
Welcome to the Git development community!
Aaron M Watson, Brandon Williams, Brian Henderson, Emily Xie,
Gavin Lambert, Ian Kelling, Jeff Hostetler, Mantas Mikulėnas,
Petr Stodulka, Satoshi Yasushima, Stefan Christ, Vegard Nossum,
yaras, and Younes Khoudli.
Returning contributors who helped this release are as follows.
Thanks for your continued support.
Ævar Arnfjörð Bjarmason, Alexander Shopov, Alex Henrie,
Alex Riesen, Anders Kaseorg, Andreas Schwab, Beat Bolli, Ben
North, brian m. carlson, Chris Packham, Christian Couder, David
Aguilar, David Turner, Dennis Kaarsemaker, Dimitriy Ryazantcev,
Elia Pinto, Eric Wong, Jacob Keller, Jakub Narębski, Jean-Noël
AVILA, Jeff King, Jiang Xin, Johannes Schindelin, Johannes Sixt,
Jonathan Nieder, Jonathan Tan, Josh Triplett, Junio C Hamano,
Karsten Blees, Kevin Daudt, Kirill Smelkov, Lars Schneider,
Linus Torvalds, Matthieu Moy, Michael Haggerty, Michael J Gruber,
Mike Ralphson, Nguyễn Thái Ngọc Duy, Olaf Hering, Orgad
Shaneh, Patrick Steinhardt, Pat Thoyts, Philip Oakley, Pranit
Bauva, Ralf Thielow, Ray Chen, René Scharfe, Ronnie Sahlberg,
Stefan Beller, SZEDER Gábor, Thomas Gummerer, Tobias Klauser,
Vasco Almeida, and Дилян Палаузов.
----------------------------------------------------------------
Git 2.11 Release Notes (draft)
==============================
Backward compatibility notes.
* An empty string used as a pathspec element has always meant
'everything matches', but it is too easy to write a script that
finds a path to remove in $path and run 'git rm "$paht"' by
mistake (when the user meant to give "$path"), which ends up
removing everything. This release starts warning about the
use of an empty string that is used for 'everything matches' and
asks users to use a more explicit '.' for that instead.
The hope is that existing users will not mind this change, and
eventually the warning can be turned into a hard error, upgrading
the deprecation into removal of this (mis)feature.
* The historical argument order "git merge <msg> HEAD <commit>..."
has been deprecated for quite some time, and will be removed in the
next release (not this one).
* The default abbreviation length, which has historically been 7, now
scales as the repository grows, using the approximate number of
objects in the repository and a bit of math around the birthday
paradox. The logic suggests to use 12 hexdigits for the Linux
kernel, and 9 to 10 for Git itself.
Updates since v2.10
-------------------
UI, Workflows & Features
* Comes with new version of git-gui, now at its 0.21.0 tag.
* "git format-patch --cover-letter HEAD^" to format a single patch
with a separate cover letter now numbers the output as [PATCH 0/1]
and [PATCH 1/1] by default.
* An incoming "git push" that attempts to push too many bytes can now
be rejected by setting a new configuration variable at the receiving
end.
* "git nosuchcommand --help" said "No manual entry for gitnosuchcommand",
which was not intuitive, given that "git nosuchcommand" said "git:
'nosuchcommand' is not a git command".
* "git clone --recurse-submodules --reference $path $URL" is a way to
reduce network transfer cost by borrowing objects in an existing
$path repository when cloning the superproject from $URL; it
learned to also peek into $path for presence of corresponding
repositories of submodules and borrow objects from there when able.
* The "git diff --submodule={short,log}" mechanism has been enhanced
to allow "--submodule=diff" to show the patch between the submodule
commits bound to the superproject.
* Even though "git hash-objects", which is a tool to take an
on-filesystem data stream and put it into the Git object store,
allowed to perform the "outside-world-to-Git" conversions (e.g.
end-of-line conversions and application of the clean-filter), and
it had the feature on by default from very early days, its reverse
operation "git cat-file", which takes an object from the Git object
store and externalize for the consumption by the outside world,
lacked an equivalent mechanism to run the "Git-to-outside-world"
conversion. The command learned the "--filters" option to do so.
* Output from "git diff" can be made easier to read by selecting
which lines are common and which lines are added/deleted
intelligently when the lines before and after the changed section
are the same. A command line option is added to help with the
experiment to find a good heuristics.
* In some projects, it is common to use "[RFC PATCH]" as the subject
prefix for a patch meant for discussion rather than application. A
new option "--rfc" is a short-hand for "--subject-prefix=RFC PATCH"
to help the participants of such projects.
* "git add --chmod=+x <pathspec>" added recently only toggled the
executable bit for paths that are either new or modified. This has
been corrected to flip the executable bit for all paths that match
the given pathspec.
* When "git format-patch --stdout" output is placed as an in-body
header and it uses the RFC2822 header folding, "git am" failed to
put the header line back into a single logical line. The
underlying "git mailinfo" was taught to handle this properly.
* "gitweb" can spawn "highlight" to show blob contents with
(programming) language-specific syntax highlighting, but only
when the language is known. "highlight" can however be told
to make the guess itself by giving it "--force" option, which
has been enabled.
* "git gui" l10n to Portuguese.
* When given an abbreviated object name that is not (or more
realistically, "no longer") unique, we gave a fatal error
"ambiguous argument". This error is now accompanied by a hint that
lists the objects beginning with the given prefix. During the
course of development of this new feature, numerous minor bugs were
uncovered and corrected, the most notable one of which is that we
gave "short SHA1 xxxx is ambiguous." twice without good reason.
* "git log rev^..rev" is an often-used revision range specification
to show what was done on a side branch merged at rev. This has
gained a short-hand "rev^-1". In general "rev^-$n" is the same as
"^rev^$n rev", i.e. what has happened on other branches while the
history leading to nth parent was looking the other way.
* In recent versions of cURL, GSSAPI credential delegation is
disabled by default due to CVE-2011-2192; introduce a configuration
to selectively allow enabling this.
(merge 26a7b23429 ps/http-gssapi-cred-delegation later to maint).
* "git mergetool" learned to honor "-O<orderfile>" to control the
order of paths to present to the end user.
* "git diff/log --ws-error-highlight=<kind>" lacked the corresponding
configuration variable to set it by default.
* "git ls-files" learned "--recurse-submodules" option that can be
used to get a listing of tracked files across submodules (i.e. this
only works with "--cached" option, not for listing untracked or
ignored files). This would be a useful tool to sit on the upstream
side of a pipe that is read with xargs to work on all working tree
files from the top-level superproject.
* A new credential helper that talks via "libsecret" with
implementations of XDG Secret Service API has been added to
contrib/credential/.
* The GPG verification status shown in "%G?" pretty format specifier
was not rich enough to differentiate a signature made by an expired
key, a signature made by a revoked key, etc. New output letters
have been assigned to express them.
* In addition to purely abbreviated commit object names, "gitweb"
learned to turn "git describe" output (e.g. v2.9.3-599-g2376d31787)
into clickable links in its output.
* When new paths were added by "git add -N" to the index, it was
enough to circumvent the check by "git commit" to refrain from
making an empty commit without "--allow-empty". The same logic
prevented "git status" to show such a path as "new file" in the
"Changes not staged for commit" section.
* The smudge/clean filter API expect an external process is spawned
to filter the contents for each path that has a filter defined. A
new type of "process" filter API has been added to allow the first
request to run the filter for a path to spawn a single process, and
all filtering need is served by this single process for multiple
paths, reducing the process creation overhead.
* The user always has to say "stash@{$N}" when naming a single
element in the default location of the stash, i.e. reflogs in
refs/stash. The "git stash" command learned to accept "git stash
apply 4" as a short-hand for "git stash apply stash@{4}".
Performance, Internal Implementation, Development Support etc.
* The delta-base-cache mechanism has been a key to the performance in
a repository with a tightly packed packfile, but it did not scale
well even with a larger value of core.deltaBaseCacheLimit.
* Enhance "git status --porcelain" output by collecting more data on
the state of the index and the working tree files, which may
further be used to teach git-prompt (in contrib/) to make fewer
calls to git.
* Extract a small helper out of the function that reads the authors
script file "git am" internally uses.
(merge a77598e jc/am-read-author-file later to maint).
* Lifts calls to exit(2) and die() higher in the callchain in
sequencer.c files so that more helper functions in it can be used
by callers that want to handle error conditions themselves.
* "git am" has been taught to make an internal call to "git apply"'s
innards without spawning the latter as a separate process.
* The ref-store abstraction was introduced to the refs API so that we
can plug in different backends to store references.
* The "unsigned char sha1[20]" to "struct object_id" conversion
continues. Notable changes in this round includes that ce->sha1,
i.e. the object name recorded in the cache_entry, turns into an
object_id.
* JGit can show a fake ref "capabilities^{}" to "git fetch" when it
does not advertise any refs, but "git fetch" was not prepared to
see such an advertisement. When the other side disconnects without
giving any ref advertisement, we used to say "there may not be a
repository at that URL", but we may have seen other advertisement
like "shallow" and ".have" in which case we definitely know that a
repository is there. The code to detect this case has also been
updated.
* Some codepaths in "git pack-objects" were not ready to use an
existing pack bitmap; now they are and as the result they have
become faster.
* The codepath in "git fsck" to detect malformed tree objects has
been updated not to die but keep going after detecting them.
* We call "qsort(array, nelem, sizeof(array[0]), fn)", and most of
the time third parameter is redundant. A new QSORT() macro lets us
omit it.
* "git pack-objects" in a repository with many packfiles used to
spend a lot of time looking for/at objects in them; the accesses to
the packfiles are now optimized by checking the most-recently-used
packfile first.
(merge c9af708b1a jk/pack-objects-optim-mru later to maint).
* Codepaths involved in interacting alternate object store have
been cleaned up.
* In order for the receiving end of "git push" to inspect the
received history and decide to reject the push, the objects sent
from the sending end need to be made available to the hook and
the mechanism for the connectivity check, and this was done
traditionally by storing the objects in the receiving repository
and letting "git gc" to expire it. Instead, store the newly
received objects in a temporary area, and make them available by
reusing the alternate object store mechanism to them only while we
decide if we accept the check, and once we decide, either migrate
them to the repository or purge them immediately.
* The require_clean_work_tree() helper was recreated in C when "git
pull" was rewritten from shell; the helper is now made available to
other callers in preparation for upcoming "rebase -i" work.
* "git upload-pack" had its code cleaned-up and performance improved
by reducing use of timestamp-ordered commit-list, which was
replaced with a priority queue.
* "git diff --no-index" codepath has been updated not to try to peek
into .git/ directory that happens to be under the current
directory, when we know we are operating outside any repository.
* Update of the sequencer codebase to make it reusable to reimplement
"rebase -i" continues.
* Git generally does not explicitly close file descriptors that were
open in the parent process when spawning a child process, but most
of the time the child does not want to access them. As Windows does
not allow removing or renaming a file that has a file descriptor
open, a slow-to-exit child can even break the parent process by
holding onto them. Use O_CLOEXEC flag to open files in various
codepaths.
* Update "interpret-trailers" machinery and teaches it that people in
real world write all sorts of crufts in the "trailer" that was
originally designed to have the neat-o "Mail-Header: like thing"
and nothing else.
Also contains various documentation updates and code clean-ups.
Fixes since v2.10
-----------------
Unless otherwise noted, all the fixes since v2.9 in the maintenance
track are contained in this release (see the maintenance releases'
notes for details).
* Clarify various ways to specify the "revision ranges" in the
documentation.
* "diff-highlight" script (in contrib/) learned to work better with
"git log -p --graph" output.
* The test framework left the number of tests and success/failure
count in the t/test-results directory, keyed by the name of the
test script plus the process ID. The latter however turned out not
to serve any useful purpose. The process ID part of the filename
has been removed.
* Having a submodule whose ".git" repository is somehow corrupt
caused a few commands that recurse into submodules loop forever.
* "git symbolic-ref -d HEAD" happily removes the symbolic ref, but
the resulting repository becomes an invalid one. Teach the command
to forbid removal of HEAD.
* A test spawned a short-lived background process, which sometimes
prevented the test directory from getting removed at the end of the
script on some platforms.
* Update a few tests that used to use GIT_CURL_VERBOSE to use the
newer GIT_TRACE_CURL.
* "git pack-objects --include-tag" was taught that when we know that
we are sending an object C, we want a tag B that directly points at
C but also a tag A that points at the tag B. We used to miss the
intermediate tag B in some cases.
* Update Japanese translation for "git-gui".
* "git fetch http::/site/path" did not die correctly and segfaulted
instead.
* "git commit-tree" stopped reading commit.gpgsign configuration
variable that was meant for Porcelain "git commit" in Git 2.9; we
forgot to update "git gui" to look at the configuration to match
this change.
* "git add --chmod=+x" added recently lacked documentation, which has
been corrected.
* "git log --cherry-pick" used to include merge commits as candidates
to be matched up with other commits, resulting a lot of wasted time.
The patch-id generation logic has been updated to ignore merges to
avoid the wastage.
* The http transport (with curl-multi option, which is the default
these days) failed to remove curl-easy handle from a curlm session,
which led to unnecessary API failures.
* There were numerous corner cases in which the configuration files
are read and used or not read at all depending on the directory a
Git command was run, leading to inconsistent behaviour. The code
to set-up repository access at the beginning of a Git process has
been updated to fix them.
(merge 4d0efa1 jk/setup-sequence-update later to maint).
* "git diff -W" output needs to extend the context backward to
include the header line of the current function and also forward to
include the body of the entire current function up to the header
line of the next one. This process may have to merge two adjacent
hunks, but the code forgot to do so in some cases.
* Performance tests done via "t/perf" did not use the same set of
build configuration if the user relied on autoconf generated
configuration.
* "git format-patch --base=..." feature that was recently added
showed the base commit information after "-- " e-mail signature
line, which turned out to be inconvenient. The base information
has been moved above the signature line.
* More i18n.
* Even when "git pull --rebase=preserve" (and the underlying "git
rebase --preserve") can complete without creating any new commit
(i.e. fast-forwards), it still insisted on having a usable ident
information (read: user.email is set correctly), which was less
than nice. As the underlying commands used inside "git rebase"
would fail with a more meaningful error message and advice text
when the bogus ident matters, this extra check was removed.
* "git gc --aggressive" used to limit the delta-chain length to 250,
which is way too deep for gaining additional space savings and is
detrimental for runtime performance. The limit has been reduced to
50.
* Documentation for individual configuration variables to control use
of color (like `color.grep`) said that their default value is
'false', instead of saying their default is taken from `color.ui`.
When we updated the default value for color.ui from 'false' to
'auto' quite a while ago, all of them broke. This has been
corrected.
* The pretty-format specifier "%C(auto)" used by the "log" family of
commands to enable coloring of the output is taught to also issue a
color-reset sequence to the output.
* A shell script example in check-ref-format documentation has been
fixed.
* "git checkout <word>" does not follow the usual disambiguation
rules when the <word> can be both a rev and a path, to allow
checking out a branch 'foo' in a project that happens to have a
file 'foo' in the working tree without having to disambiguate.
This was poorly documented and the check was incorrect when the
command was run from a subdirectory.
* Some codepaths in "git diff" used regexec(3) on a buffer that was
mmap(2)ed, which may not have a terminating NUL, leading to a read
beyond the end of the mapped region. This was fixed by introducing
a regexec_buf() helper that takes a <ptr,len> pair with REG_STARTEND
extension.
* The procedure to build Git on Mac OS X for Travis CI hardcoded the
internal directory structure we assumed HomeBrew uses, which was a
no-no. The procedure has been updated to ask HomeBrew things we
need to know to fix this.
* When "git rebase -i" is given a broken instruction, it told the
user to fix it with "--edit-todo", but didn't say what the step
after that was (i.e. "--continue").
* Documentation around tools to import from CVS was fairly outdated.
* "git clone --recurse-submodules" lost the progress eye-candy in
recent update, which has been corrected.
* A low-level function verify_packfile() was meant to show errors
that were detected without dying itself, but under some conditions
it didn't and died instead, which has been fixed.
* When "git fetch" tries to find where the history of the repository
it runs in has diverged from what the other side has, it has a
mechanism to avoid digging too deep into irrelevant side branches.
This however did not work well over the "smart-http" transport due
to a design bug, which has been fixed.
* In the codepath that comes up with the hostname to be used in an
e-mail when the user didn't tell us, we looked at ai_canonname
field in struct addrinfo without making sure it is not NULL first.
* "git worktree", even though it used the default_abbrev setting that
ought to be affected by core.abbrev configuration variable, ignored
the variable setting. The command has been taught to read the
default set of configuration variables to correct this.
* "git init" tried to record core.worktree in the repository's
'config' file when GIT_WORK_TREE environment variable was set and
it was different from where GIT_DIR appears as ".git" at its top,
but the logic was faulty when .git is a "gitdir:" file that points
at the real place, causing trouble in working trees that are
managed by "git worktree". This has been corrected.
* Codepaths that read from an on-disk loose object were too loose in
validating what they are reading is a proper object file and
sometimes read past the data they read from the disk, which has
been corrected. H/t to Gustavo Grieco for reporting.
* The original command line syntax for "git merge", which was "git
merge <msg> HEAD <parent>...", has been deprecated for quite some
time, and "git gui" was the last in-tree user of the syntax. This
is finally fixed, so that we can move forward with the deprecation.
* An author name, that spelled a backslash-quoted double quote in the
human readable part "My \"double quoted\" name", was not unquoted
correctly while applying a patch from a piece of e-mail.
* Doc update to clarify what "log -3 --reverse" does.
* Almost everybody uses DEFAULT_ABBREV to refer to the default
setting for the abbreviation, but "git blame" peeked into
underlying variable bypassing the macro for no good reason.
* The "graph" API used in "git log --graph" miscounted the number of
output columns consumed so far when drawing a padding line, which
has been fixed; this did not affect any existing code as nobody
tried to write anything after the padding on such a line, though.
* The code that parses the format parameter of for-each-ref command
has seen a micro-optimization.
* When we started cURL to talk to imap server when a new enough
version of cURL library is available, we forgot to explicitly add
imap(s):// before the destination. To some folks, that didn't work
and the library tried to make HTTP(s) requests instead.
* The ./configure script generated from configure.ac was taught how
to detect support of SSL by libcurl better.
* The command-line completion script (in contrib/) learned to
complete "git cmd ^mas<HT>" to complete the negative end of
reference to "git cmd ^master".
(merge 49416ad22a cp/completion-negative-refs later to maint).
* The existing "git fetch --depth=<n>" option was hard to use
correctly when making the history of an existing shallow clone
deeper. A new option, "--deepen=<n>", has been added to make this
easier to use. "git clone" also learned "--shallow-since=<date>"
and "--shallow-exclude=<tag>" options to make it easier to specify
"I am interested only in the recent N months worth of history" and
"Give me only the history since that version".
(merge cccf74e2da nd/shallow-deepen later to maint).
* It is a common mistake to say "git blame --reverse OLD path",
expecting that the command line is dwimmed as if asking how lines
in path in an old revision OLD have survived up to the current
commit.
(merge e1d09701a4 jc/blame-reverse later to maint).
* http.emptyauth configuration is a way to allow an empty username to
pass when attempting to authenticate using mechanisms like
Kerberos. We took an unspecified (NULL) username and sent ":"
(i.e. no username, no password) to CURLOPT_USERPWD, but did not do
the same when the username is explicitly set to an empty string.
* "git clone" of a local repository can be done at the filesystem
level, but the codepath did not check errors while copying and
adjusting the file that lists alternate object stores.
* Documentation for "git commit" was updated to clarify that "commit
-p <paths>" adds to the current contents of the index to come up
with what to commit.
* A stray symbolic link in $GIT_DIR/refs/ directory could make name
resolution loop forever, which has been corrected.
* The "submodule.<name>.path" stored in .gitmodules is never copied
to .git/config and such a key in .git/config has no meaning, but
the documentation described it and submodule.<name>.url next to
each other as if both belong to .git/config. This has been fixed.
* In a worktree connected to a repository elsewhere, created via "git
worktree", "git checkout" attempts to protect users from confusion
by refusing to check out a branch that is already checked out in
another worktree. However, this also prevented checking out a
branch, which is designated as the primary branch of a bare
reopsitory, in a worktree that is connected to the bare
repository. The check has been corrected to allow it.
* "git rebase" immediately after "git clone" failed to find the fork
point from the upstream.
* When fetching from a remote that has many tags that are irrelevant
to branches we are following, we used to waste way too many cycles
when checking if the object pointed at by a tag (that we are not
going to fetch!) exists in our repository too carefully.
* Protect our code from over-eager compilers.
* Recent git allows submodule.<name>.branch to use a special token
"." instead of the branch name; the documentation has been updated
to describe it.
* A hot-fix for a test added by a recent topic that went to both
'master' and 'maint' already.
* "git send-email" attempts to pick up valid e-mails from the
trailers, but people in real world write non-addresses there, like
"Cc: Stable <add@re.ss> # 4.8+", which broke the output depending
on the availability and vintage of Mail::Address perl module.
(merge dcfafc5214 mm/send-email-cc-cruft-after-address later to maint).
* The Travis CI configuration we ship ran the tests with --verbose
option but this risks non-TAP output that happens to be "ok" to be
misinterpreted as TAP signalling a test that passed. This resulted
in unnecessary failure. This has been corrected by introducing a
new mode to run our tests in the test harness to send the verbose
output separately to the log file.
* Some AsciiDoc formatter mishandles a displayed illustration with
tabs in it. Adjust a few of them in merge-base documentation to
work around them.
* A minor regression fix for "git submodule" that was introduced
when more helper functions were reimplemented in C.
(merge 77b63ac31e sb/submodule-ignore-trailing-slash later to maint).
* The code that we have used for the past 10+ years to cycle
4-element ring buffers turns out to be not quite portable in
theoretical world.
(merge bb84735c80 rs/ring-buffer-wraparound later to maint).
* "git daemon" used fixed-length buffers to turn URL to the
repository the client asked for into the server side directory
path, using snprintf() to avoid overflowing these buffers, but
allowed possibly truncated paths to the directory. This has been
tightened to reject such a request that causes overlong path to be
required to serve.
(merge 6bdb0083be jk/daemon-path-ok-check-truncation later to maint).
* Recent update to git-sh-setup (a library of shell functions that
are used by our in-tree scripted Porcelain commands) included
another shell library git-sh-i18n without specifying where it is,
relying on the $PATH. This has been fixed to be more explicit by
prefixing $(git --exec-path) output in front.
(merge 1073094f30 ak/sh-setup-dot-source-i18n-fix later to maint).
* Fix for a racy false-positive test failure.
(merge fdf4f6c79b as/merge-attr-sleep later to maint).
* Portability update and workaround for builds on recent Mac OS X.
(merge a296bc0132 ls/macos-update later to maint).
* Other minor doc, test and build updates and code cleanups.
(merge 5c238e29a8 jk/common-main later to maint).
(merge 5a5749e45b ak/pre-receive-hook-template-modefix later to maint).
(merge 6d834ac8f1 jk/rebase-config-insn-fmt-docfix later to maint).
(merge de9f7fa3b0 rs/commit-pptr-simplify later to maint).
(merge 4259d693fc sc/fmt-merge-msg-doc-markup-fix later to maint).
(merge 28fab7b23d nd/test-helpers later to maint).
(merge c2bb0c1d1e rs/cocci later to maint).
(merge 3285b7badb ps/common-info-doc later to maint).
(merge 2b090822e8 nd/worktree-lock later to maint).
(merge 4bd488ea7c jk/create-branch-remove-unused-param later to maint).
(merge 974e0044d6 tk/diffcore-delta-remove-unused later to maint).
----------------------------------------------------------------
Changes since v2.10.0 are as follows:
Aaron M Watson (1):
stash: allow stashes to be referenced by index only
Alex Henrie (5):
am: put spaces around pipe in usage string
cat-file: put spaces around pipes in usage string
git-rebase--interactive: fix English grammar
git-merge-octopus: do not capitalize "octopus"
unpack-trees: do not capitalize "working"
Alex Riesen (2):
git-gui: support for $FILENAMES in tool definitions
git-gui: ensure the file in the diff pane is in the list of selected files
Alexander Shopov (2):
git-gui i18n: Updated Bulgarian translation (565,0f,0u)
git-gui: Mark 'All' in remote.tcl for translation
Anders Kaseorg (3):
imap-send: Tell cURL to use imap:// or imaps://
pre-receive.sample: mark it executable
git-sh-setup: be explicit where to dot-source git-sh-i18n from.
Andreas Schwab (2):
t6026-merge-attr: don't fail if sleep exits early
t6026-merge-attr: ensure that the merge driver was called
Beat Bolli (1):
SubmittingPatches: use gitk's "Copy commit summary" format
Ben North (1):
git-worktree.txt: fix typo "to"/"two", and add comma
Brandon Williams (6):
pathspec: remove unnecessary function prototypes
git: make super-prefix option
ls-files: optionally recurse into submodules
ls-files: pass through safe options for --recurse-submodules
ls-files: add pathspec matching for submodules
submodules doc: update documentation for "." used for submodule branches
Brian Henderson (3):
diff-highlight: add some tests
diff-highlight: add failing test for handling --graph output
diff-highlight: add support for --graph output
Chris Packham (1):
completion: support excluding refs
Christian Couder (43):
apply: make some names more specific
apply: move 'struct apply_state' to apply.h
builtin/apply: make apply_patch() return -1 or -128 instead of die()ing
builtin/apply: read_patch_file() return -1 instead of die()ing
builtin/apply: make find_header() return -128 instead of die()ing
builtin/apply: make parse_chunk() return a negative integer on error
builtin/apply: make parse_single_patch() return -1 on error
builtin/apply: make parse_whitespace_option() return -1 instead of die()ing
builtin/apply: make parse_ignorewhitespace_option() return -1 instead of die()ing
builtin/apply: move init_apply_state() to apply.c
apply: make init_apply_state() return -1 instead of exit()ing
builtin/apply: make check_apply_state() return -1 instead of die()ing
builtin/apply: move check_apply_state() to apply.c
builtin/apply: make apply_all_patches() return 128 or 1 on error
builtin/apply: make parse_traditional_patch() return -1 on error
builtin/apply: make gitdiff_*() return 1 at end of header
builtin/apply: make gitdiff_*() return -1 on error
builtin/apply: change die_on_unsafe_path() to check_unsafe_path()
builtin/apply: make build_fake_ancestor() return -1 on error
builtin/apply: make remove_file() return -1 on error
builtin/apply: make add_conflicted_stages_file() return -1 on error
builtin/apply: make add_index_file() return -1 on error
builtin/apply: make create_file() return -1 on error
builtin/apply: make write_out_one_result() return -1 on error
builtin/apply: make write_out_results() return -1 on error
unpack-objects: add --max-input-size=<size> option
builtin/apply: make try_create_file() return -1 on error
builtin/apply: make create_one_file() return -1 on error
builtin/apply: rename option parsing functions
apply: rename and move opt constants to apply.h
apply: move libified code from builtin/apply.c to apply.{c,h}
apply: make some parsing functions static again
apply: use error_errno() where possible
apply: make it possible to silently apply
apply: don't print on stdout in verbosity_silent mode
usage: add set_warn_routine()
usage: add get_error_routine() and get_warn_routine()
apply: change error_routine when silent
apply: refactor `git apply` option parsing
apply: pass apply state to build_fake_ancestor()
apply: learn to use a different index file
builtin/am: use apply API in run_apply()
split-index: s/eith/with/ typo fix
David Aguilar (4):
mergetool: add copyright
mergetool: move main program flow into a main() function
mergetool: honor diff.orderFile
mergetool: honor -O<orderfile>
David Turner (11):
rename_ref_available(): add docstring
refs: add methods for reflog
refs: add method for initial ref transaction commit
refs: make delete_refs() virtual
refs: add methods to init refs db
refs: add method to rename refs
refs: make lock generic
refs: implement iteration over only per-worktree refs
add David Turner's Two Sigma address
fsck: handle bad trees like other errors
http: http.emptyauth should allow empty (not just NULL) usernames
Dennis Kaarsemaker (1):
worktree: allow the main brach of a bare repository to be checked out
Dimitriy Ryazantcev (2):
l10n: ru.po: update Russian translation
git-gui: Update Russian translation
Elia Pinto (6):
t5541-http-push-smart.sh: use the GIT_TRACE_CURL environment var
test-lib.sh: preserve GIT_TRACE_CURL from the environment
t5550-http-fetch-dumb.sh: use the GIT_TRACE_CURL environment var
t5551-http-fetch-smart.sh: use the GIT_TRACE_CURL environment var
git-check-ref-format.txt: fixup documentation
git-gui/po/glossary/txt-to-pot.sh: use the $( ... ) construct for command substitution
Emily Xie (1):
pathspec: warn on empty strings as pathspec
Eric Wong (5):
http: warn on curl_multi_add_handle failures
http: consolidate #ifdefs for curl_multi_remove_handle
http: always remove curl easy from curlm session on release
git-svn: reduce scope of input record separator change
git-svn: "git worktree" awareness
Gavin Lambert (1):
git-svn: do not reuse caches memoized for a different architecture
Ian Kelling (2):
gitweb: remove unused guess_file_syntax() parameter
gitweb: use highlight's shebang detection
Jacob Keller (9):
format-patch: show 0/1 and 1/1 for singleton patch with cover letter
cache: add empty_tree_oid object and helper function
graph: add support for --line-prefix on all graph-aware output
diff: prepare for additional submodule formats
allow do_submodule_path to work even if submodule isn't checked out
submodule: convert show_submodule_summary to use struct object_id *
submodule: refactor show_submodule_summary with helper function
diff: teach diff to display submodule difference with an inline diff
rev-list: use hdr_termination instead of a always using a newline
Jakub Narębski (1):
configure.ac: improve description of NO_REGEX test
Jean-Noël AVILA (1):
i18n: i18n: diff: mark die messages for translation
Jeff Hostetler (9):
status: rename long-format print routines
status: cleanup API to wt_status_print
status: support --porcelain[=<version>]
status: collect per-file data for --porcelain=v2
status: print per-file porcelain v2 status data
status: print branch info with --porcelain=v2 --branch
git-status.txt: describe --porcelain=v2 format
test-lib-functions.sh: add lf_to_nul helper
status: unit tests for --porcelain=v2
Jeff King (116):
rebase-interactive: drop early check for valid ident
provide an initializer for "struct object_info"
sha1_file: make packed_object_info public
pack-objects: break delta cycles before delta-search phase
pack-objects: use mru list when iterating over packs
gc: default aggressive depth to 50
cache_or_unpack_entry: drop keep_cache parameter
clear_delta_base_cache_entry: use a more descriptive name
release_delta_base_cache: reuse existing detach function
delta_base_cache: use list.h for LRU
delta_base_cache: drop special treatment of blobs
delta_base_cache: use hashmap.h
t/perf: add basic perf tests for delta base cache
index-pack: add --max-input-size=<size> option
receive-pack: allow a maximum input size to be specified
test-lib: drop PID from test-results/*.count
diff-highlight: ignore test cruft
diff-highlight: add multi-byte tests
diff-highlight: avoid highlighting combined diffs
error_errno: use constant return similar to error()
color_parse_mem: initialize "struct color" temporary
t5305: move cleanup into test block
t5305: drop "dry-run" of unpack-objects
t5305: use "git -C"
t5305: simplify packname handling
pack-objects: walk tag chains for --include-tag
remote-curl: handle URLs without protocol
patch-ids: turn off rename detection
add_delta_base_cache: use list_for_each_safe
patch-ids: refuse to compute patch-id for merge commit
hash-object: always try to set up the git repository
patch-id: use RUN_SETUP_GENTLY
diff: skip implicit no-index check when given --no-index
diff: handle --no-index prefixes consistently
diff: always try to set up the repository
pager: remove obsolete comment
pager: stop loading git_default_config()
pager: make pager_program a file-local static
pager: use callbacks instead of configset
pager: handle early config
t1302: use "git -C"
test-config: setup git directory
config: only read .git/config from configured repos
init: expand comments explaining config trickery
init: reset cached config when entering new repo
t1007: factor out repeated setup
verify_packfile: check pack validity before accessing data
clone: pass --progress decision to recursive submodules
docs/cvsimport: prefer cvs-fast-export to parsecvs
docs/cvs-migration: update link to cvsps homepage
docs/cvs-migration: mention cvsimport caveats
ident: handle NULL ai_canonname
get_sha1: detect buggy calls with multiple disambiguators
get_sha1: avoid repeating ourselves via ONLY_TO_DIE
get_sha1: propagate flags to child functions
get_short_sha1: parse tags when looking for treeish
get_short_sha1: refactor init of disambiguation code
get_short_sha1: NUL-terminate hex prefix
get_short_sha1: mark ambiguity error for translation
sha1_array: let callbacks interrupt iteration
for_each_abbrev: drop duplicate objects
get_short_sha1: list ambiguous objects on error
xdiff: rename "struct group" to "struct xdlgroup"
get_short_sha1: make default disambiguation configurable
tree-walk: be more specific about corrupt tree errors
graph: fix extra spaces in graph_padding_line
t5613: drop reachable_via function
t5613: drop test_valid_repo function
t5613: use test_must_fail
t5613: whitespace/style cleanups
t5613: do not chdir in main process
find_unique_abbrev: move logic out of get_short_sha1()
clone: detect errors in normalize_path_copy
files_read_raw_ref: avoid infinite loop on broken symlinks
files_read_raw_ref: prevent infinite retry loops in general
t5613: clarify "too deep" recursion tests
link_alt_odb_entry: handle normalize_path errors
link_alt_odb_entry: refactor string handling
alternates: provide helper for adding to alternates list
alternates: provide helper for allocating alternate
alternates: encapsulate alt->base munging
alternates: use a separate scratch space
fill_sha1_file: write "boring" characters
alternates: store scratch buffer as strbuf
fill_sha1_file: write into a strbuf
count-objects: report alternates via verbose mode
sha1_file: always allow relative paths to alternates
alternates: use fspathcmp to detect duplicates
check_connected: accept an env argument
tmp-objdir: introduce API for temporary object directories
receive-pack: quarantine objects until pre-receive accepts
tmp-objdir: put quarantine information in the environment
tmp-objdir: do not migrate files starting with '.'
upload-pack: use priority queue in reachable() check
merge-base: handle --fork-point without reflog
fetch: use "quick" has_sha1_file for tag following
test-lib: handle TEST_OUTPUT_DIRECTORY with spaces
test-lib: add --verbose-log option
travis: use --verbose-log test option
test-lib: bail out when "-v" used under "prove"
daemon: detect and reject too-long paths
read info/{attributes,exclude} only when in repository
test-*-cache-tree: setup git dir
find_unique_abbrev: use 4-buffer ring
diff_unique_abbrev: rename to diff_aligned_abbrev
diff_aligned_abbrev: use "struct oid"
diff: handle sha1 abbreviations outside of repository
git-compat-util: move content inside ifdef/endif guards
doc: fix missing "::" in config list
t0021: use write_script to create rot13 shell script
t0021: put $TEST_ROOT in $PATH
t0021: use $PERL_PATH for rot13-filter.pl
t0021: fix filehandle usage on older perl
alternates: re-allow relative paths from environment
sequencer: silence -Wtautological-constant-out-of-range-compare
create_branch: drop unused "head" parameter
Jiang Xin (1):
l10n: zh_CN: fixed some typos for git 2.10.0
Johannes Schindelin (60):
cat-file: fix a grammo in the man page
sequencer: lib'ify sequencer_pick_revisions()
sequencer: do not die() in do_pick_commit()
sequencer: lib'ify write_message()
sequencer: lib'ify do_recursive_merge()
sequencer: lib'ify do_pick_commit()
sequencer: lib'ify walk_revs_populate_todo()
sequencer: lib'ify prepare_revs()
sequencer: lib'ify read_and_refresh_cache()
sequencer: lib'ify read_populate_todo()
sequencer: lib'ify read_populate_opts()
sequencer: lib'ify create_seq_dir()
sequencer: lib'ify save_head()
sequencer: lib'ify save_todo()
sequencer: lib'ify save_opts()
sequencer: lib'ify fast_forward_to()
sequencer: lib'ify checkout_fast_forward()
sequencer: ensure to release the lock when we could not read the index
cat-file: introduce the --filters option
cat-file --textconv/--filters: allow specifying the path separately
cat-file: support --textconv/--filters in batch mode
git-gui: respect commit.gpgsign again
regex: -G<pattern> feeds a non NUL-terminated string to regexec() and fails
regex: add regexec_buf() that can work on a non NUL-terminated string
regex: use regexec_buf()
pull: drop confusing prefix parameter of die_on_unclean_work_tree()
pull: make code more similar to the shell script again
wt-status: make the require_clean_work_tree() function reusable
wt-status: export also the has_un{staged,committed}_changes() functions
wt-status: teach has_{unstaged,uncommitted}_changes() about submodules
wt-status: begin error messages with lower-case
reset: fix usage
sequencer: use static initializers for replay_opts
sequencer: use memoized sequencer directory path
sequencer: avoid unnecessary indirection
sequencer: future-proof remove_sequencer_state()
sequencer: plug memory leaks for the option values
sequencer: future-proof read_populate_todo()
sequencer: refactor the code to obtain a short commit name
sequencer: completely revamp the "todo" script parsing
sequencer: strip CR from the todo script
sequencer: avoid completely different messages for different actions
sequencer: get rid of the subcommand field
sequencer: remember the onelines when parsing the todo file
sequencer: prepare for rebase -i's commit functionality
sequencer: introduce a helper to read files written by scripts
sequencer: allow editing the commit message on a case-by-case basis
sequencer: support amending commits
sequencer: support cleaning up commit messages
sequencer: left-trim lines read from the script
sequencer: stop releasing the strbuf in write_message()
sequencer: roll back lock file if write_message() failed
sequencer: refactor write_message() to take a pointer/length
sequencer: teach write_message() to append an optional LF
sequencer: remove overzealous assumption in rebase -i mode
sequencer: mark action_name() for translation
sequencer: quote filenames in error messages
sequencer: start error messages consistently with lower case
sequencer: mark all error messages for translation
t6026: ensure that long-running script really is
Johannes Sixt (9):
t9903: fix broken && chain
t6026-merge-attr: clean up background process at end of test case
t3700-add: create subdirectory gently
t3700-add: do not check working tree file mode without POSIXPERM
t0060: sidestep surprising path mangling results on Windows
t0021: expect more variations in the output of uniq -c
t0021: compute file size with a single process instead of a pipeline
t0021, t5615: use $PWD instead of $(pwd) in PATH-like shell variables
t6026: clarify the point of "kill $(cat sleep.pid)"
Jonathan Nieder (1):
connect: tighten check for unexpected early hang up
Jonathan Tan (14):
tests: move test_lazy_prereq JGIT to test-lib.sh
connect: advertized capability is not a ref
mailinfo: separate in-body header processing
mailinfo: make is_scissors_line take plain char *
mailinfo: handle in-body header continuations
fetch-pack: do not reset in_vain on non-novel acks
trailer: improve const correctness
trailer: use list.h for doubly-linked list
trailer: streamline trailer item create and add
trailer: make args have their own struct
trailer: clarify failure modes in parse_trailer
trailer: allow non-trailers in trailer block
trailer: forbid leading whitespace in trailers
trailer: support values folded to multiple lines
Josh Triplett (2):
format-patch: show base info before email signature
format-patch: add "--rfc" for the common case of [RFC PATCH]
Junio C Hamano (49):
blame: improve diagnosis for "--reverse NEW"
blame: dwim "blame --reverse OLD" as "blame --reverse OLD.."
am: refactor read_author_script()
diff.c: remove output_prefix_length field
submodule: avoid auto-discovery in prepare_submodule_repo_env()
symbolic-ref -d: do not allow removal of HEAD
Prepare for 2.9.4
Start the 2.11 cycle
First batch for 2.11
Second batch for 2.11
Third batch for 2.11
Start preparing for 2.10.1
Fourth batch for 2.11
streaming: make sure to notice corrupt object
unpack_sha1_header(): detect malformed object header
Fifth batch for 2.11
worktree: honor configuration variables
blame: use DEFAULT_ABBREV macro
Prepare for 2.10.1
Sixth batch for 2.11
diff_unique_abbrev(): document its assumption and limitation
abbrev: add FALLBACK_DEFAULT_ABBREV to prepare for auto sizing
abbrev: prepare for new world order
Git 2.10.1
Seventh batch for 2.11
t4015: split out the "setup" part of ws-error-highlight test
diff.c: refactor parse_ws_error_highlight()
diff.c: move ws-error-highlight parsing helpers up
diff: introduce diff.wsErrorHighlight option
Eighth batch for 2.11
Ninth batch for 2.11
Start preparing for 2.10.2
cocci: refactor common patterns to use xstrdup_or_null()
Tenth batch for 2.11
t3700: fix broken test under !SANITY
transport: pass summary_width down the callchain
fetch: pass summary_width down the callchain
transport: allow summary-width to be computed dynamically
transport: compute summary-width dynamically
Eleventh batch for 2.11
Getting ready for 2.11-rc0
Git 2.10.2
Git 2.11-rc0
A bit of updates post -rc0
Revert "t6026-merge-attr: ensure that the merge driver was called"
Revert "t6026-merge-attr: don't fail if sleep exits early"
t0021: remove debugging cruft
Git 2.11.0-rc1
Git 2.11-rc2
Karsten Blees (2):
git-gui: unicode file name support on windows
git-gui: handle the encoding of Git's output correctly
Kevin Daudt (2):
t5100-mailinfo: replace common path prefix with variable
mailinfo: unescape quoted-pair in header fields
Kirill Smelkov (3):
pack-objects: respect --local/--honor-pack-keep/--incremental when bitmap is in use
pack-objects: use reachability bitmap index when generating non-stdout pack
t/perf/run: copy config.mak.autogen & friends to build area
Lars Schneider (20):
travis-ci: ask homebrew for its path instead of hardcoding it
convert: quote filter names in error messages
convert: modernize tests
run-command: move check_pipe() from write_or_die to run_command
run-command: add clean_on_exit_handler
pkt-line: rename packet_write() to packet_write_fmt()
pkt-line: extract set_packet_header()
pkt-line: add packet_write_fmt_gently()
pkt-line: add packet_flush_gently()
pkt-line: add packet_write_gently()
pkt-line: add functions to read/write flush terminated packet streams
convert: make apply_filter() adhere to standard Git error handling
convert: prepare filter.<driver>.process option
convert: add filter.<driver>.process option
contrib/long-running-filter: add long running filter example
sha1_file: rename git_open_noatime() to git_open()
sha1_file: open window into packfiles with O_CLOEXEC
read-cache: make sure file handles are not inherited by child processes
Makefile: set NO_OPENSSL on macOS by default
travis-ci: disable GIT_TEST_HTTPD for macOS
Linus Torvalds (1):
abbrev: auto size the default abbreviation
Mantas Mikulėnas (1):
contrib: add credential helper for libsecret
Matthieu Moy (4):
Documentation/config: default for color.* is color.ui
parse_mailboxes: accept extra text after <...> address
t9000-addresses: update expected results after fix
Git.pm: add comment pointing to t9000
Michael Haggerty (36):
xdl_change_compact(): fix compaction heuristic to adjust ixo
xdl_change_compact(): only use heuristic if group can't be matched
is_blank_line(): take a single xrecord_t as argument
recs_match(): take two xrecord_t pointers as arguments
xdl_change_compact(): introduce the concept of a change group
resolve_gitlink_ref(): eliminate temporary variable
refs: rename struct ref_cache to files_ref_store
refs: create a base class "ref_store" for files_ref_store
add_packed_ref(): add a files_ref_store argument
get_packed_ref(): add a files_ref_store argument
resolve_missing_loose_ref(): add a files_ref_store argument
{lock,commit,rollback}_packed_refs(): add files_ref_store arguments
refs: reorder definitions
resolve_packed_ref(): rename function from resolve_missing_loose_ref()
resolve_gitlink_packed_ref(): remove function
read_raw_ref(): take a (struct ref_store *) argument
resolve_ref_recursively(): new function
resolve_gitlink_ref(): implement using resolve_ref_recursively()
resolve_gitlink_ref(): avoid memory allocation in many cases
resolve_gitlink_ref(): rename path parameter to submodule
refs: make read_raw_ref() virtual
refs: make verify_refname_available() virtual
refs: make pack_refs() virtual
refs: make create_symref() virtual
refs: make peel_ref() virtual
repack_without_refs(): add a files_ref_store argument
lock_raw_ref(): add a files_ref_store argument
commit_ref_update(): add a files_ref_store argument
lock_ref_for_update(): add a files_ref_store argument
lock_ref_sha1_basic(): add a files_ref_store argument
split_symref_update(): add a files_ref_store argument
files_ref_iterator_begin(): take a ref_store argument
refs: add method iterator_begin
diff: improve positioning of add/delete blocks in diffs
parse-options: add parse_opt_unknown_cb()
blame: honor the diff heuristic options and config
Michael J Gruber (1):
gpg-interface: use more status letters
Mike Ralphson (1):
vcs-svn/fast_export: fix timestamp fmt specifiers
Nguyễn Thái Ngọc Duy (40):
remote-curl.c: convert fetch_git() to use argv_array
transport-helper.c: refactor set_helper_option()
upload-pack: move shallow deepen code out of receive_needs()
upload-pack: move "shallow" sending code out of deepen()
upload-pack: remove unused variable "backup"
upload-pack: move "unshallow" sending code out of deepen()
upload-pack: use skip_prefix() instead of starts_with()
upload-pack: tighten number parsing at "deepen" lines
upload-pack: make check_non_tip() clean things up on error
upload-pack: move rev-list code out of check_non_tip()
fetch-pack: use skip_prefix() instead of starts_with()
fetch-pack: use a common function for verbose printing
fetch-pack.c: mark strings for translating
fetch-pack: use a separate flag for fetch in deepening mode
shallow.c: implement a generic shallow boundary finder based on rev-list
upload-pack: add deepen-since to cut shallow repos based on time
fetch: define shallow boundary with --shallow-since
clone: define shallow clone boundary based on time with --shallow-since
t5500, t5539: tests for shallow depth since a specific date
refs: add expand_ref()
upload-pack: support define shallow boundary by excluding revisions
fetch: define shallow boundary with --shallow-exclude
clone: define shallow clone boundary with --shallow-exclude
t5500, t5539: tests for shallow depth excluding a ref
upload-pack: split check_unreachable() in two, prep for get_reachable_list()
upload-pack: add get_reachable_list()
fetch, upload-pack: --deepen=N extends shallow boundary by N commits
checkout: add some spaces between code and comment
checkout.txt: document a common case that ignores ambiguation rules
checkout: fix ambiguity check in subdir
init: correct re-initialization from a linked worktree
init: call set_git_dir_init() from within init_db()
init: kill set_git_dir_init()
init: do not set unnecessary core.worktree
init: kill git_link variable
git-commit.txt: clarify --patch mode with pathspec
diff-lib: allow ita entries treated as "not yet exist in index"
diff: add --ita-[in]visible-in-index
commit: fix empty commit creation when there's no changes but ita entries
commit: don't be fooled by ita entries when creating initial commit
Olaf Hering (1):
git-gui: sort entries in tclIndex
Orgad Shaneh (1):
git-gui: Do not reset author details on amend
Pat Thoyts (7):
Allow keyboard control to work in the staging widgets.
Amend tab ordering and text widget border and highlighting.
git-gui: fix detection of Cygwin
git-gui (Windows): use git-gui.exe in `Create Desktop Shortcut`
git-gui: maintain backwards compatibility for merge syntax
git-gui: avoid persisting modified author identity
git-gui: set version 0.21
Patrick Steinhardt (1):
doc: fix location of 'info/' with $GIT_COMMON_DIR
Petr Stodulka (1):
http: control GSSAPI credential delegation
Philip Oakley (14):
doc: use 'symmetric difference' consistently
doc: revisions - name the left and right sides
doc: show the actual left, right, and boundary marks
doc: revisions: give headings for the two and three dot notations
doc: revisions: extra clarification of <rev>^! notation effects
doc: revisions: single vs multi-parent notation comparison
doc: gitrevisions - use 'reachable' in page description
doc: gitrevisions - clarify 'latter case' is revision walk
doc: revisions - define `reachable`
doc: revisions - clarify reachability examples
doc: revisions: show revision expansion in examples
doc: revisions: sort examples and fix alignment of the unchanged
doc: fix merge-base ASCII art tab spacing
doc: fix the 'revert a faulty merge' ASCII art tab spacing
Pranit Bauva (2):
rev-list-options: clarify the usage of --reverse
t0040: convert all possible tests to use `test-parse-options --expect`
Ralf Thielow (6):
help: introduce option --exclude-guides
help: make option --help open man pages only for Git commands
rebase -i: improve advice on bad instruction lines
l10n: de.po: fix translation of autostash
l10n: de.po: translate 260 new messages
fetch-pack.c: correct command at the beginning of an error message
Ray Chen (1):
l10n: zh_CN: review for git v2.10.0 l10n
René Scharfe (36):
compat: move strdup(3) replacement to its own file
introduce hex2chr() for converting two hexadecimal digits to a character
strbuf: use valid pointer in strbuf_remove()
checkout: constify parameters of checkout_stage() and checkout_merged()
unpack-trees: pass checkout state explicitly to check_updates()
sha1_file: use llist_mergesort() for sorting packs
xdiff: fix merging of hunks with -W context and -u context
contrib/coccinelle: fix semantic patch for oid_to_hex_r()
add coccicheck make target
use strbuf_addstr() for adding constant strings to a strbuf, part 2
pretty: let %C(auto) reset all attributes
introduce CHECKOUT_INIT
add COPY_ARRAY
use COPY_ARRAY
git-gui: stop using deprecated merge syntax
gitignore: ignore output files of coccicheck make target
use strbuf_addstr() instead of strbuf_addf() with "%s", part 2
use strbuf_add_unique_abbrev() for adding short hashes, part 2
add QSORT
use QSORT
remove unnecessary check before QSORT
coccicheck: use --all-includes by default
use QSORT, part 2
pretty: avoid adding reset for %C(auto) if output is empty
coccicheck: make transformation for strbuf_addf(sb, "...") more precise
show-branch: use QSORT
remove unnecessary NULL check before free(3)
use strbuf_add_unique_abbrev() for adding short hashes, part 3
pretty: fix document link for color specification
avoid pointer arithmetic involving NULL in FLEX_ALLOC_MEM
inline xalloc_flex() into FLEXPTR_ALLOC_MEM
hex: make wraparound of the index into ring-buffer explicit
valgrind: support test helpers
commit: simplify building parents list
sha1_name: make wraparound of the index into ring-buffer explicit
cocci: avoid self-references in object_id transformations
Ronnie Sahlberg (2):
refs: add a backend method structure
refs: add a transaction_commit() method
SZEDER Gábor (1):
ref-filter: strip format option after a field name only once while parsing
Satoshi Yasushima (6):
git-gui: consistently use the same word for "remote" in Japanese
git-gui: consistently use the same word for "blame" in Japanese
git-gui: apply po template to Japanese translation
git-gui: add Japanese language code
git-gui: update Japanese translation
git-gui: update Japanese information
Stefan Beller (16):
t7408: modernize style
t7408: merge short tests, factor out testing method
submodule--helper module-clone: allow multiple references
submodule--helper update-clone: allow multiple references
clone: factor out checking for an alternate path
clone: clarify option_reference as required
clone: implement optional references
clone: recursive and reference option triggers submodule alternates
xdiff: remove unneeded declarations
transport: report missing submodule pushes consistently on stderr
diff.c: use diff_options directly
diff: omit found pointer from emit_callback
diff: remove dead code
submodule: ignore trailing slash on superproject URL
submodule: ignore trailing slash in relative url
documentation: improve submodule.<name>.{url, path} description
Stefan Christ (1):
Documentation/fmt-merge-msg: fix markup in example
Thomas Gummerer (4):
add: document the chmod option
update-index: add test for chmod flags
read-cache: introduce chmod_index_entry
add: modify already added files when --chmod is given
Tobias Klauser (1):
diffcore-delta: remove unused parameter to diffcore_count_changes()
Vasco Almeida (32):
l10n: pt_PT: update Portuguese translation
l10n: pt_PT: update Portuguese repository info
i18n: blame: mark error messages for translation
i18n: branch: mark option description for translation
i18n: config: mark error message for translation
i18n: merge-recursive: mark error messages for translation
i18n: merge-recursive: mark verbose message for translation
i18n: notes: mark error messages for translation
notes: spell first word of error messages in lowercase
i18n: receive-pack: mark messages for translation
i18n: show-branch: mark error messages for translation
i18n: show-branch: mark plural strings for translation
i18n: update-index: mark warnings for translation
i18n: commit: mark message for translation
i18n: connect: mark die messages for translation
i18n: ident: mark hint for translation
i18n: notes-merge: mark die messages for translation
i18n: stash: mark messages for translation
git-gui i18n: mark strings for translation
git-gui: l10n: add Portuguese translation
git-gui i18n: internationalize use of colon punctuation
git-gui i18n: mark "usage:" strings for translation
git-gui: fix incorrect use of Tcl append command
git-gui i18n: mark string in lib/error.tcl for translation
t1512: become resilient to GETTEXT_POISON build
i18n: apply: mark plural string for translation
i18n: apply: mark info messages for translation
i18n: apply: mark error messages for translation
i18n: apply: mark error message for translation
i18n: convert mark error messages for translation
i18n: credential-cache--daemon: mark advice for translation
i18n: diff: mark warnings for translation
Vegard Nossum (1):
revision: new rev^-n shorthand for rev^n..rev
Younes Khoudli (1):
doc: remove reference to the traditional layout in git-tag.txt
brian m. carlson (20):
cache: convert struct cache_entry to use struct object_id
builtin/apply: convert static functions to struct object_id
builtin/blame: convert struct origin to use struct object_id
builtin/log: convert some static functions to use struct object_id
builtin/cat-file: convert struct expand_data to use struct object_id
builtin/cat-file: convert some static functions to struct object_id
builtin: convert textconv_object to use struct object_id
streaming: make stream_blob_to_fd take struct object_id
builtin/checkout: convert some static functions to struct object_id
notes-merge: convert struct notes_merge_pair to struct object_id
Convert read_mmblob to take struct object_id.
builtin/blame: convert file to use struct object_id
builtin/rm: convert to use struct object_id
notes: convert init_notes to use struct object_id
builtin/update-index: convert file to struct object_id
sha1_name: convert get_sha1_mb to struct object_id
refs: add an update_ref_oid function.
builtin/am: convert to struct object_id
builtin/commit-tree: convert to struct object_id
builtin/reset: convert to use struct object_id
yaras (1):
git-gui: fix initial git gui message encoding
Ævar Arnfjörð Bjarmason (3):
gitweb: fix a typo in a comment
gitweb: link to 7-char+ SHA-1s, not only 8-char+
gitweb: link to "git describe"'d commits in log messages
Дилян Палаузов (1):
./configure.ac: detect SSL in libcurl using curl-config
^ permalink raw reply
* Re: [PATCH 08/16] update submodules: add depopulate_submodule
From: Stefan Beller @ 2016-11-17 22:23 UTC (permalink / raw)
To: Brandon Williams
Cc: git@vger.kernel.org, Junio C Hamano, Jonathan Nieder, Martin Fick,
David Turner
In-Reply-To: <20161115234403.GE66382@google.com>
On Tue, Nov 15, 2016 at 3:44 PM, Brandon Williams <bmwill@google.com> wrote:
> "to that a deleted" did you mean "so that a deleted"
done
>> That will only work properly when the submodule uses a gitfile instead of
>> a .git directory and no untracked files are present. Otherwise the removal
>> will fail with a warning (which is just what happened until now).
>
> So if a submodule uses a .git directory then it will be ignored during
> the checkout?
Well first you get the warning:
"cannot remove submodule '%s' because it (or one of "
"its nested submodules) uses a .git directory"),
and in case a d/f/ conflict arises in a later stage (e.g. when the submodule
is replaced by a file or symlink), you get another related error with
less helpful description how to debug it.
> All other submodules will actually be removed? Couldn't
> you end up in an undesirable state with a checkout effecting one
> submodule but not another?
Yes you could. Maybe it's time to add
"git submodule intern-git-dir", which can be given as a helpful hint
or even run here first.
>
> Should probably place an explicit 'extern' in the function prototype.
done
^ permalink raw reply
* Re: merge --no-ff is NOT mentioned in help
From: Jeff King @ 2016-11-17 22:21 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Mike Rappazzo, Vanderhoof, Tzadik, git@vger.kernel.org
In-Reply-To: <xmqqr36anibl.fsf@gitster.mtv.corp.google.com>
On Thu, Nov 17, 2016 at 09:10:22AM -0800, Junio C Hamano wrote:
> People interested may want to try the attached single-liner patch to
> see how the output from _ALL_ commands that use parse-options API
> looks when given "-h". It could be that the result may not be too
> bad.
The output is less ugly than I expected, but still a bit cluttered IMHO.
I was surprised that the column-adjustment did not need tweaked, but the
code correctly increments "pos" from the return value of fprintf, which
just works.
Looking at the output for --ff, though:
--[no-]ff allow fast-forward (default)
I do not think it's improving the situation nearly as much as if we made
the primary option "--no-ff" with a NONEG flga, and then added back in a
HIDDEN "--ff". I thought we had done that in other cases, but I can't
seem to find any. But it would make "--no-ff" the primary form, which
makes sense, as "--ff" is already the default.
Another option would be to teach parse-options to somehow treat the
negated form as primary in the help text. That's a bit more code, but
might be usable in other places.
-Peff
^ permalink raw reply
* Re: [PATCH v3 4/6] grep: optionally recurse into submodules
From: Brandon Williams @ 2016-11-17 22:13 UTC (permalink / raw)
To: Stefan Beller; +Cc: git@vger.kernel.org, Jonathan Tan, Junio C Hamano
In-Reply-To: <CAGZ79kZiAWTySJrSvav6Yuj8v9PF0JzaSJHFTOdUo6eYFTS1+A@mail.gmail.com>
On 11/15, Stefan Beller wrote:
> > + /*
> > + * Limit number of threads for child process to use.
> > + * This is to prevent potential fork-bomb behavior of git-grep as each
> > + * submodule process has its own thread pool.
> > + */
> > + if (num_threads)
> > + argv_array_pushf(&submodule_options, "--threads=%d",
> > + (num_threads + 1) / 2);
>
> I think you would want to pass --threads=%d unconditionally,
> as it also serves as a weak defusal for fork bombs. Is it possible to come here
> with num_threads=0? (i.e. what happens if the user doesn't specify the number
> of threads or such, do we fall back to some default or is it just 0?)
>
> I have seen some other places that check for num_threads unequal to 0,
> as e.g. no mutex needs to be locked then (assuming we don't have any
> thread but grep within the main process), but as you intend to use this also
> as a helper to not blow up the number of threads recursively, we'd need to
> pass at a number != 0 here?
The option parsing logic in cmd_grep handles the cases where num_threads
is some odd value (and fails if <0). In the case where it is 0, it will
default to 8 under certain circumstances. I figured I would just let
that logic handle the cases where num_theads ends up being 0 instead of
explicitly passing threads=1. You can't pass threads=0 in some cases
due to the default "oh look threads==0, looks like we should use 8!"
case.
>
> > +
> > + git grep -e "bar" --and -e "foo" --recurse-submodules > actual &&
>
> nit here and in the tests below:
> We prefer to have no white space between > and the file piped to.
I'll fix that up everywhere.
--
Brandon Williams
^ permalink raw reply
* Re: [PATCH v7 16/17] branch: use ref-filter printing APIs
From: Junio C Hamano @ 2016-11-17 22:05 UTC (permalink / raw)
To: Karthik Nayak; +Cc: git, jacob.keller
In-Reply-To: <xmqqinrlopge.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
> One worry that I have is if the strings embedded in this function to
> the final format are safe. As far as I can tell, the pieces of
> strings that are literally inserted into the resulting format string
> by this function are maxwidth, remote_prefix, and return values from
> branch_get_color() calls.
>
> The maxwidth is inserted via "%d" and made into decimal constant,
> and there is no risk for it being in the resulting format. Are
> the return values of branch_get_color() calls safe? I do not think
> they can have '%' in them, but if they do, they need to be quoted.
> The same worry exists for remote_prefix. Currently it can either be
> an empty string or "remotes/", and is safe to be embedded in a
> format string.
In case it was not clear, in short, I do not think there is anything
broken in the code, but it is a longer-term improvement to introduce
a helper that takes a string and returns a version of the string
that is safely quoted to be used in the for-each-ref format string
use it like so:
strbuf_addf(&remote,
"%s"
"%%(align:%d,left)%s%%(refname:strip=2)%%(end)"
...
"%%(else) %%(objectname:short=7) %%(contents:subject)%%(end)",
quote_literal_for_format(branch_get_color(BRANCH_COLOR_REMOTE)),
...);
and the implementation of the helper may look like:
const char *quote_literal_for_format(const char *s)
{
static strbuf buf = STRBUF_INIT;
strbuf_reset(&buf);
while (*s) {
const char *ep = strchrnul(s, '%');
if (s < ep)
strbuf_add(&buf, s, ep - s);
if (*ep == '%') {
strbuf_addstr(&buf, "%%");
s = ep + 1;
} else {
s = ep;
}
}
return buf.buf;
}
^ permalink raw reply
* Re: [PATCH v15 22/27] bisect--helper: `bisect_log` shell function in C
From: Stephan Beyer @ 2016-11-17 21:47 UTC (permalink / raw)
To: Pranit Bauva, git
In-Reply-To: <01020157c38b1b18-b81203b0-122f-4244-bfb2-9fac8ae71767-000000@eu-west-1.amazonses.com>
Hi,
On 10/14/2016 04:14 PM, Pranit Bauva wrote:
> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
> index 493034c..c18ca07 100644
> --- a/builtin/bisect--helper.c
> +++ b/builtin/bisect--helper.c
> @@ -858,6 +858,23 @@ static int bisect_state(struct bisect_terms *terms, const char **argv,
> return -1;
> }
>
> +static int bisect_log(void)
> +{
> + int fd, status;
> + fd = open(git_path_bisect_log(), O_RDONLY);
> + if (fd < 0)
> + return -1;
> +
> + status = copy_fd(fd, 1);
Perhaps
status = copy_fd(fd, STDOUT_FILENO);
> + if (status) {
> + close(fd);
> + return -1;
> + }
> +
> + close(fd);
> + return status;
> +}
That's weird.
Either get rid of the if() and actually use status:
status = copy_fd(fd, STDOUT_FILENO);
close(fd);
return status ? -1 : 0;
or get rid of status and use the if:
if (copy_fd(fd, STDOUT_FILENO)) {
close(fd);
return -1;
}
close(fd);
return 0;
I'd recommend the shorter variant ;)
~Stephan
^ permalink raw reply
* Re: [PATCH v15 12/27] bisect--helper: `get_terms` & `bisect_terms` shell function in C
From: Stephan Beyer @ 2016-11-17 21:32 UTC (permalink / raw)
To: Pranit Bauva, git
In-Reply-To: <01020157c38b1ad5-0f90c88e-2077-4155-94e9-7d71dbbac38f-000000@eu-west-1.amazonses.com>
Hi,
On 10/14/2016 04:14 PM, Pranit Bauva wrote:
> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
> index 317d671..6a5878c 100644
> --- a/builtin/bisect--helper.c
> +++ b/builtin/bisect--helper.c
[...]
> +static int bisect_terms(struct bisect_terms *terms, const char **argv, int argc)
> +{
> + int i;
> + const char bisect_term_usage[] =
> +"git bisect--helper --bisect-terms [--term-good | --term-bad | ]"
> +"--term-old | --term-new";
Three things:
(1) Is that indentation intentional?
(2) You have a "]" at the end of the first part of the string instead of
the end of the second part.
(3) After the correction, bisect_term_usage and
git_bisect_helper_usage[7] are the same strings. I don't recommend to
use git_bisect_helper_usage[7] instead because keeping the index
up-to-date is a maintenance hell. (At the end of your patch series it is
a 3 instead of a 7.) However, if - for whatever reason - the usage of
bisect--helper --bisect-terms changes, you always have to sync the two
strings which is also nasty....
> +
> + if (get_terms(terms))
> + return error(_("no terms defined"));
> +
> + if (argc > 1) {
> + usage(bisect_term_usage);
> + return -1;
> + }
...and since you only use it once, why not simply do something like
return error(_("--bisect-term requires exactly one argument"));
and drop the definition of bisect_term_usage.
> +
> + if (argc == 0) {
> + printf(_("Your current terms are %s for the old state\nand "
> + "%s for the new state.\n"), terms->term_good,
> + terms->term_bad);
Very minor: It improves the readability if you'd split the string after
the \n and put the "and "in the next line.
> + return 0;
> + }
> +
> + for (i = 0; i < argc; i++) {
> + if (!strcmp(argv[i], "--term-good"))
> + printf("%s\n", terms->term_good);
> + else if (!strcmp(argv[i], "--term-bad"))
> + printf("%s\n", terms->term_bad);
> + else
> + die(_("invalid argument %s for 'git bisect "
> + "terms'.\nSupported options are: "
> + "--term-good|--term-old and "
> + "--term-bad|--term-new."), argv[i]);
Hm, "return error(...)" and "die(...)" seems to be quasi-equivalent in
this case. Because I am always looking from a library perspective, I'd
prefer "return error(...)".
> @@ -429,6 +492,11 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
> terms.term_bad = xstrdup(argv[1]);
> res = bisect_next_check(&terms, argc == 3 ? argv[2] : NULL);
> break;
> + case BISECT_TERMS:
> + if (argc > 1)
> + die(_("--bisect-terms requires 0 or 1 argument"));
> + res = bisect_terms(&terms, argv, argc);
> + break;
Also here: "terms" is leaking...
~Stephan
^ permalink raw reply
* Re: [PATCH v15 11/27] bisect--helper: `bisect_next_check` & bisect_voc shell function in C
From: Stephan Beyer @ 2016-11-17 20:59 UTC (permalink / raw)
To: Pranit Bauva, git
In-Reply-To: <01020157c38b1adb-ab4c90ed-d084-40b5-a037-f62c76e52ec4-000000@eu-west-1.amazonses.com>
Hi Pranit,
On 10/14/2016 04:14 PM, Pranit Bauva wrote:
> Also reimplement `bisect_voc` shell function in C and call it from
> `bisect_next_check` implementation in C.
Please don't! ;D
> +static char *bisect_voc(char *revision_type)
> +{
> + if (!strcmp(revision_type, "bad"))
> + return "bad|new";
> + if (!strcmp(revision_type, "good"))
> + return "good|old";
> +
> + return NULL;
> +}
Why not simply use something like this:
static const char *voc[] = {
"bad|new",
"good|old",
};
Then...
> +static int bisect_next_check(const struct bisect_terms *terms,
> + const char *current_term)
> +{
> + int missing_good = 1, missing_bad = 1, retval = 0;
> + char *bad_ref = xstrfmt("refs/bisect/%s", terms->term_bad);
> + char *good_glob = xstrfmt("%s-*", terms->term_good);
> + char *bad_syn, *good_syn;
...you don't need bad_syn and good_syn...
> + bad_syn = xstrdup(bisect_voc("bad"));
> + good_syn = xstrdup(bisect_voc("good"));
...and hence not these two lines...
> + if (!is_empty_or_missing_file(git_path_bisect_start())) {
> + error(_("You need to give me at least one %s and "
> + "%s revision. You can use \"git bisect %s\" "
> + "and \"git bisect %s\" for that. \n"),
> + bad_syn, good_syn, bad_syn, good_syn);
...and write
voc[0], voc[1], voc[0], voc[1]);
instead...
> + retval = -1;
> + goto finish;
> + }
> + else {
> + error(_("You need to start by \"git bisect start\". You "
> + "then need to give me at least one %s and %s "
> + "revision. You can use \"git bisect %s\" and "
> + "\"git bisect %s\" for that.\n"),
> + good_syn, bad_syn, bad_syn, good_syn);
...and here
voc[1], voc[0], voc[0], voc[1]);
...
> + retval = -1;
> + goto finish;
> + }
> + goto finish;
> +finish:
> + if (!bad_ref)
> + free(bad_ref);
> + if (!good_glob)
> + free(good_glob);
> + if (!bad_syn)
> + free(bad_syn);
> + if (!good_syn)
> + free(good_syn);
...and you can remove the 4 lines above.
> + return retval;
> +}
Besides that, there are again some things that I've already mentioned
and that can be applied here, too, for example, not capitalizing
TERM_GOOD and TERM_BAD, the goto fail simplification, the terms memory leak.
Cheers
Stephan
^ permalink raw reply
* Re: [PATCH v15 10/27] bisect--helper: `check_and_set_terms` shell function in C
From: Stephan Beyer @ 2016-11-17 20:25 UTC (permalink / raw)
To: Pranit Bauva, git
In-Reply-To: <01020157c38b1aca-0c26fb8c-404f-4f57-afe7-7ebb552a1002-000000@eu-west-1.amazonses.com>
Hi Pranit,
On 10/14/2016 04:14 PM, Pranit Bauva wrote:
> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
> index 3f19b68..c6c11e3 100644
> --- a/builtin/bisect--helper.c
> +++ b/builtin/bisect--helper.c
> @@ -20,6 +20,7 @@ static const char * const git_bisect_helper_usage[] = {
> N_("git bisect--helper --bisect-clean-state"),
> N_("git bisect--helper --bisect-reset [<commit>]"),
> N_("git bisect--helper --bisect-write <state> <revision> <TERM_GOOD> <TERM_BAD> [<nolog>]"),
> + N_("git bisect--helper --bisect-check-and-set-terms <command> <TERM_GOOD> <TERM_BAD>"),
Here's the same as in the previous patch... I'd not use
TERM_GOOD/TERM_BAD in capitals.
> NULL
> };
>
> @@ -212,6 +213,38 @@ static int bisect_write(const char *state, const char *rev,
> return retval;
> }
>
> +static int set_terms(struct bisect_terms *terms, const char *bad,
> + const char *good)
> +{
> + terms->term_good = xstrdup(good);
> + terms->term_bad = xstrdup(bad);
> + return write_terms(terms->term_bad, terms->term_good);
At this stage of the patch series I am wondering why you are setting
"terms" here, but I guess you'll need it later.
However, you are leaking memory here. Something like
free(terms->term_good);
free(terms->term_bad);
terms->term_good = xstrdup(good);
terms->term_bad = xstrdup(bad);
should be safe (because you've always used xstrdup() for the terms
members before). Or am I overseeing something?
> @@ -278,6 +314,13 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
> terms.term_bad = xstrdup(argv[3]);
> res = bisect_write(argv[0], argv[1], &terms, nolog);
> break;
> + case CHECK_AND_SET_TERMS:
> + if (argc != 3)
> + die(_("--check-and-set-terms requires 3 arguments"));
> + terms.term_good = xstrdup(argv[1]);
> + terms.term_bad = xstrdup(argv[2]);
> + res = check_and_set_terms(&terms, argv[0]);
> + break;
Ha! When I reviewed the last patch, I asked you why you changed the code
from returning directly from each subcommand to setting res; break; and
then return res at the bottom of the function.
Now I see why this was useful. The two members of "terms" are again
leaking memory: you are allocating memory by using xstrdup() but you are
not freeing it.
(That also applies to the last patch.)
Cheers,
Stephan
^ permalink raw reply
* Re: [PATCH 07/16] update submodules: introduce submodule_is_interesting
From: Stefan Beller @ 2016-11-17 20:08 UTC (permalink / raw)
To: Heiko Voigt
Cc: git@vger.kernel.org, Brandon Williams, Junio C Hamano,
Jonathan Nieder, Martin Fick, David Turner
In-Reply-To: <20161117105715.GC39230@book.hvoigt.net>
On Thu, Nov 17, 2016 at 2:57 AM, Heiko Voigt <hvoigt@hvoigt.net> wrote:
> It seems that you are only looking at the submodule config from a
> commit. Should a user be able to override this with local configuration?
> Haven't looked further in the patchseries so maybe that is somewhere
> else?
It turns out that in later patches we pass in null_sha1 only, which is
looking at the config and possible overrides.
I'll refactor to take no sha1 argument and use null_sha1 here directly.
^ permalink raw reply
* Re: [PATCH 07/16] update submodules: introduce submodule_is_interesting
From: Stefan Beller @ 2016-11-17 20:03 UTC (permalink / raw)
To: David Turner
Cc: git@vger.kernel.org, bmwill@google.com, gitster@pobox.com,
jrnieder@gmail.com, mogulguy10@gmail.com
In-Reply-To: <e748abfad8b04a8eaaa10797d9324891@exmbdft7.ad.twosigma.com>
On Tue, Nov 15, 2016 at 4:14 PM, David Turner <David.Turner@twosigma.com> wrote:
>> +int submodule_is_interesting(const char *path, const unsigned char
>> +*sha1) {
>
> This is apparently only ever (in this series) called with null_sha1. So either this arg is unnecessary, or there are bugs elsewhere in the code.
I was torn when writing the series, as I initially had submodule_is_interesting
with no sha1 argument and it turned out to be buggy in my first
initial implementation,
which lead me to thinking the sha1 actually matters.
The line of thinking was similar to loading the submodules from the
submodule-config cache as that also has different values for different sha1s,
e.g. a submodule is only interesting if submodule.<name>.update != none,
which can have changed with different sha1s.
I refactored the series since then to call the _is_initeresting method
at different times
(before and after the actual checkout), such that we implicitly have
the correct sha1
while calling it.
So I would argue the sha1 argument is not needed. I'll remove it.
^ permalink raw reply
* Re: [PATCH v7 16/17] branch: use ref-filter printing APIs
From: Junio C Hamano @ 2016-11-17 19:50 UTC (permalink / raw)
To: Karthik Nayak; +Cc: git, jacob.keller
In-Reply-To: <20161108201211.25213-17-Karthik.188@gmail.com>
Karthik Nayak <karthik.188@gmail.com> writes:
> +static char *build_format(struct ref_filter *filter, int maxwidth, const char *remote_prefix)
> +{
I understand that the return value of this function is used as if
the value given via --format=... option to for-each-ref.
> + struct strbuf fmt = STRBUF_INIT;
> + struct strbuf local = STRBUF_INIT;
> + struct strbuf remote = STRBUF_INIT;
> +
> + strbuf_addf(&fmt, "%%(if)%%(HEAD)%%(then)* %s%%(else) %%(end)", branch_get_color(BRANCH_COLOR_CURRENT));
This switches between "* " and " " prefixed for each line of output
in "git branch --list" output, where an asterisk is used to mark the
branch that is currently checked out. OK.
> + if (filter->verbose) {
> + strbuf_addf(&local, "%%(align:%d,left)%%(refname:strip=2)%%(end)", maxwidth);
> + strbuf_addf(&local, "%s", branch_get_color(BRANCH_COLOR_RESET));
> + strbuf_addf(&local, " %%(objectname:short=7) ");
> +
> + if (filter->verbose > 1)
> + strbuf_addf(&local, "%%(if)%%(upstream)%%(then)[%s%%(upstream:short)%s%%(if)%%(upstream:track)"
> + "%%(then): %%(upstream:track,nobracket)%%(end)] %%(end)%%(contents:subject)",
> + branch_get_color(BRANCH_COLOR_UPSTREAM), branch_get_color(BRANCH_COLOR_RESET));
> + else
> + strbuf_addf(&local, "%%(if)%%(upstream:track)%%(then)%%(upstream:track) %%(end)%%(contents:subject)");
> +
> + strbuf_addf(&remote, "%s%%(align:%d,left)%s%%(refname:strip=2)%%(end)%s%%(if)%%(symref)%%(then) -> %%(symref:short)"
> + "%%(else) %%(objectname:short=7) %%(contents:subject)%%(end)",
> + branch_get_color(BRANCH_COLOR_REMOTE), maxwidth,
> + remote_prefix, branch_get_color(BRANCH_COLOR_RESET));
> + } else {
> + strbuf_addf(&local, "%%(refname:strip=2)%s%%(if)%%(symref)%%(then) -> %%(symref:short)%%(end)",
> + branch_get_color(BRANCH_COLOR_RESET));
> + strbuf_addf(&remote, "%s%s%%(refname:strip=2)%s%%(if)%%(symref)%%(then) -> %%(symref:short)%%(end)",
> + branch_get_color(BRANCH_COLOR_REMOTE), remote_prefix, branch_get_color(BRANCH_COLOR_RESET));
> + }
This block prepares "local" and "remote", two formats that are used
for local and remote branches.
> + strbuf_addf(&fmt, "%%(if:notequals=remotes)%%(refname:base)%%(then)%s%%(else)%s%%(end)", local.buf, remote.buf);
And this uses the %(if)...%(then)...%(else)...%(end) construct to
switch between these formats.
Sounds good.
One worry that I have is if the strings embedded in this function to
the final format are safe. As far as I can tell, the pieces of
strings that are literally inserted into the resulting format string
by this function are maxwidth, remote_prefix, and return values from
branch_get_color() calls.
The maxwidth is inserted via "%d" and made into decimal constant,
and there is no risk for it being in the resulting format. Are
the return values of branch_get_color() calls safe? I do not think
they can have '%' in them, but if they do, they need to be quoted.
The same worry exists for remote_prefix. Currently it can either be
an empty string or "remotes/", and is safe to be embedded in a
format string.
^ permalink raw reply
* Re: merge --no-ff is NOT mentioned in help
From: Torsten Bögershausen @ 2016-11-17 19:18 UTC (permalink / raw)
To: Junio C Hamano, Mike Rappazzo; +Cc: Vanderhoof, Tzadik, git@vger.kernel.org
In-Reply-To: <xmqqr36anibl.fsf@gitster.mtv.corp.google.com>
On 17/11/16 18:10, Junio C Hamano wrote:
> Mike Rappazzo <rappazzo@gmail.com> writes:
>
>> (Please reply inline)
> Indeed ;-)
>
>> On Wed, Nov 16, 2016 at 10:48 AM, Vanderhoof, Tzadik
>> <tzadik.vanderhoof@optum360.com> wrote:
>>> I am running: git version 2.10.1.windows.1
>>>
>>> I typed: git merge -h
>>>
>>> and got:
>>>
>>> usage: git merge [<options>] [<commit>...]
>>> or: git merge [<options>] <msg> HEAD <commit>
>>> or: git merge --abort
>>>
>>> -n do not show a diffstat at the end of the merge
>>> ...
>>> --overwrite-ignore update ignored files (default)
>>>
>>> Notice there is NO mention of the "--no-ff" option
>> I understand. On my system I can reproduce this by providing a bad
>> argument to `git merge`. This is the output from the arg setup. For
>> "boolean" arguments (like '--ff'), there is an automatic counter
>> argument with "no-" in there ('--no-ff') to disable the option. Maybe
>> it would make sense to word the output to include both.
> I think that was a deliberate design decision to avoid cluttering
> the short help text with mention of both --option and --no-option.
>
> People interested may want to try the attached single-liner patch to
> see how the output from _ALL_ commands that use parse-options API
> looks when given "-h". It could be that the result may not be too
> bad.
>
> I suspect that we may discover that some options that should be
> marked with NONEG are not marked along the way, which need to be
> fixed.
>
>
> parse-options.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/parse-options.c b/parse-options.c
> index 312a85dbde..348be6b240 100644
> --- a/parse-options.c
> +++ b/parse-options.c
> @@ -626,7 +626,9 @@ static int usage_with_options_internal(struct parse_opt_ctx_t *ctx,
> if (opts->long_name && opts->short_name)
> pos += fprintf(outfile, ", ");
> if (opts->long_name)
> - pos += fprintf(outfile, "--%s", opts->long_name);
> + pos += fprintf(outfile, "--%s%s",
> + (opts->flags & PARSE_OPT_NONEG) ? "" : "[no-]",
> + opts->long_name);
> if (opts->type == OPTION_NUMBER)
> pos += utf8_fprintf(outfile, _("-NUM"));
>
+1 from my side
(As I once spend some time to find out that the "no--" is automatically available)
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox