* Re* Protecting old temporary objects being reused from concurrent "git gc"?
From: Junio C Hamano @ 2016-11-17 1:35 UTC (permalink / raw)
To: Jeff King; +Cc: Matt McCutchen, git
In-Reply-To: <20161117010449.6k3cwo3njvrid4jy@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> I think the case that is helped here is somebody who runs "git
> write-tree" and expects that the timestamp on those trees is fresh. So
> even more a briefly used index, like:
>
> export GIT_INDEX_FILE=/tmp/foo
> git read-tree ...
> git write-tree
> rm -f $GIT_INDEX_FILE
>
> we'd expect that a "git gc" which runs immediately after would see those
> trees as recent and avoid pruning them (and transitively, any blobs that
> are reachable from the trees). But I don't think that write-tree
> actually freshens them (it sees "oh, we already have these; there is
> nothing to write").
OK, here is what I have queued.
-- >8 --
Subject: cache-tree: make sure to "touch" tree objects the cache-tree records
The cache_tree_fully_valid() function is called by callers that want
to know if they need to call cache_tree_update(), i.e. as an attempt
to optimize. They all want to have a fully valid cache-tree in the
end so that they can write a tree object out.
We used to check if the cached tree object is up-to-date (i.e. the
index entires covered by the cache-tree entry hasn't been changed
since the roll-up hash was computed for the cache-tree entry) and
made sure the tree object is already in the object store. Since the
top-level tree we would soon be asked to write out (and would find
in the object store) may not be anchored to any refs or commits
immediately, freshen the tree object when it happens.
Similarly, when we actually compute the cache-tree entries in
cache_tree_update(), we refrained from writing a tree object out
when it already exists in the object store. We would want to
freshen the tree object in that case to protect it from premature
pruning.
Strictly speaking, freshing these tree objects at each and every
level is probably unnecessary, given that anything reachable from a
young object inherits the youth from the referring object to be
protected from pruning. It should be sufficient to freshen only the
very top-level tree instead. Benchmarking and optimization is left
as an exercise for later days.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
cache-tree.c | 6 +++---
cache.h | 1 +
sha1_file.c | 9 +++++++--
3 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/cache-tree.c b/cache-tree.c
index 3ebf9c3aa4..c8c74a1e07 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -225,7 +225,7 @@ int cache_tree_fully_valid(struct cache_tree *it)
int i;
if (!it)
return 0;
- if (it->entry_count < 0 || !has_sha1_file(it->sha1))
+ if (it->entry_count < 0 || !freshen_object(it->sha1))
return 0;
for (i = 0; i < it->subtree_nr; i++) {
if (!cache_tree_fully_valid(it->down[i]->cache_tree))
@@ -253,7 +253,7 @@ static int update_one(struct cache_tree *it,
*skip_count = 0;
- if (0 <= it->entry_count && has_sha1_file(it->sha1))
+ if (0 <= it->entry_count && freshen_object(it->sha1))
return it->entry_count;
/*
@@ -393,7 +393,7 @@ static int update_one(struct cache_tree *it,
if (repair) {
unsigned char sha1[20];
hash_sha1_file(buffer.buf, buffer.len, tree_type, sha1);
- if (has_sha1_file(sha1))
+ if (freshen_object(sha1))
hashcpy(it->sha1, sha1);
else
to_invalidate = 1;
diff --git a/cache.h b/cache.h
index 4ff196c259..72c0b321ac 100644
--- a/cache.h
+++ b/cache.h
@@ -1077,6 +1077,7 @@ extern int sha1_object_info(const unsigned char *, unsigned long *);
extern int hash_sha1_file(const void *buf, unsigned long len, const char *type, unsigned char *sha1);
extern int write_sha1_file(const void *buf, unsigned long len, const char *type, unsigned char *return_sha1);
extern int hash_sha1_file_literally(const void *buf, unsigned long len, const char *type, unsigned char *sha1, unsigned flags);
+extern int freshen_object(const unsigned char *);
extern int pretend_sha1_file(void *, unsigned long, enum object_type, unsigned char *);
extern int force_object_loose(const unsigned char *sha1, time_t mtime);
extern int git_open_noatime(const char *name);
diff --git a/sha1_file.c b/sha1_file.c
index d0f2aa029b..9acce3d3b8 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -3151,6 +3151,11 @@ static int freshen_packed_object(const unsigned char *sha1)
return 1;
}
+int freshen_object(const unsigned char *sha1)
+{
+ return freshen_packed_object(sha1) || freshen_loose_object(sha1);
+}
+
int write_sha1_file(const void *buf, unsigned long len, const char *type, unsigned char *sha1)
{
char hdr[32];
@@ -3160,7 +3165,7 @@ int write_sha1_file(const void *buf, unsigned long len, const char *type, unsign
* it out into .git/objects/??/?{38} file.
*/
write_sha1_file_prepare(buf, len, type, sha1, hdr, &hdrlen);
- if (freshen_packed_object(sha1) || freshen_loose_object(sha1))
+ if (freshen_object(sha1))
return 0;
return write_loose_object(sha1, hdr, hdrlen, buf, len, 0);
}
@@ -3178,7 +3183,7 @@ int hash_sha1_file_literally(const void *buf, unsigned long len, const char *typ
if (!(flags & HASH_WRITE_OBJECT))
goto cleanup;
- if (freshen_packed_object(sha1) || freshen_loose_object(sha1))
+ if (freshen_object(sha1))
goto cleanup;
status = write_loose_object(sha1, header, hdrlen, buf, len, 0);
--
2.11.0-rc1-156-g07127df5c1
^ permalink raw reply related
* Re: Re* Protecting old temporary objects being reused from concurrent "git gc"?
From: Jeff King @ 2016-11-17 1:43 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Matt McCutchen, git
In-Reply-To: <xmqqvavmopl8.fsf_-_@gitster.mtv.corp.google.com>
On Wed, Nov 16, 2016 at 05:35:47PM -0800, Junio C Hamano wrote:
> OK, here is what I have queued.
>
> -- >8 --
> Subject: cache-tree: make sure to "touch" tree objects the cache-tree records
>
> The cache_tree_fully_valid() function is called by callers that want
> to know if they need to call cache_tree_update(), i.e. as an attempt
> to optimize. They all want to have a fully valid cache-tree in the
> end so that they can write a tree object out.
That makes sense. I was focusing on cache_tree_update() call, but we do
not even get there in the fully-valid case.
So I think this approach is nice as long as there is not a caller who
asks "are we fully valid? I do not need to write, but was just
wondering". That should be a read-only operation, but the freshen calls
may fail with EPERM, for example.
I do not see any such callers, nor do I really expect any. Just trying
to think through the possible consequences.
> Strictly speaking, freshing these tree objects at each and every
> level is probably unnecessary, given that anything reachable from a
> young object inherits the youth from the referring object to be
> protected from pruning. It should be sufficient to freshen only the
> very top-level tree instead. Benchmarking and optimization is left
> as an exercise for later days.
Good observation, and nicely explained all around.
-Peff
^ permalink raw reply
* Re: Rebasing cascading topic trees
From: Jeff King @ 2016-11-17 1:45 UTC (permalink / raw)
To: Norbert Kiesel; +Cc: Junio C Hamano, git
In-Reply-To: <CAM+g_Ns-9Sj5r0V2XXZfGQz+0XiO1O-hT03japEGibkNgh8a4A@mail.gmail.com>
On Wed, Nov 16, 2016 at 04:12:20PM -0800, Norbert Kiesel wrote:
> Yes, `git rebase --onto topic1 topic1@{1} topic2` is the answer!
See also the `--fork-point` option, which (I think) should do this for
you (and is the default if "topic1" is the configured upstream for
topic2 and you just run "git rebase").
-Peff
^ permalink raw reply
* Re: Rebasing cascading topic trees
From: Norbert Kiesel @ 2016-11-17 3:21 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20161117014522.lzk43sfmpl4ted3d@sigill.intra.peff.net>
More things I learned!
So there are (at least) 2 possible approaches: using history, or using
local tracking branches. The latter looks actually nicer to me, with
the exception that if asks for a `git pull`. Using `git pull
--rebase` actually also ends up with the same tree, but I like the
rebase better.
The following 2 scripts show the 2 approaches. Only difference is in
the creation of topic/b and the last rebase command.
# History
mkdir topic; cd topic
git init
date > a; git add a; git commit -m a
date > b; git add b; git commit -m b
git branch -b topic/a
git checkout -b topic1
date > c; git add c; git commit -m c
date > d; git add d; git commit -m d
git checkout -b topic2
date > e; git add e; git commit -m e
date > f; git add f; git commit -m f
git checkout master
date > g; git add g; git commit -m g
echo "before rebase"; git log --oneline --graph --all
git rebase master topic1
echo "after rebase of topic1"; git log --oneline --graph --all
git rebase --onto=topic1 topic1@{1} topic2
echo "after rebase of topic2"; git log --oneline --graph --all
# Tracking
mkdir topic; cd topic
git init
date > a; git add a; git commit -m a
date > b; git add b; git commit -m b
git branch -b topic/a
git checkout -b topic1
date > c; git add c; git commit -m c
date > d; git add d; git commit -m d
git checkout --track topic1 -b topic2
date > e; git add e; git commit -m e
date > f; git add f; git commit -m f
git checkout master
date > g; git add g; git commit -m g
echo "before rebase"; git log --oneline --graph --all
git rebase master topic1
echo "after rebase of topic1"; git log --oneline --graph --all
git rebase --onto=topic1 master topic2
echo "after rebase of topic2"; git log --oneline --graph --all
On Wed, Nov 16, 2016 at 5:45 PM, Jeff King <peff@peff.net> wrote:
> On Wed, Nov 16, 2016 at 04:12:20PM -0800, Norbert Kiesel wrote:
>
>> Yes, `git rebase --onto topic1 topic1@{1} topic2` is the answer!
>
> See also the `--fork-point` option, which (I think) should do this for
> you (and is the default if "topic1" is the configured upstream for
> topic2 and you just run "git rebase").
>
> -Peff
^ permalink raw reply
* Re: [PATCH v15 07/27] bisect--helper: `bisect_reset` shell function in C
From: Pranit Bauva @ 2016-11-17 3:56 UTC (permalink / raw)
To: Stephan Beyer; +Cc: Git List
In-Reply-To: <1190e37e-d54a-7dbf-412d-8dff90ca677a@gmx.net>
Hey Stephan,
On Thu, Nov 17, 2016 at 4:53 AM, Stephan Beyer <s-beyer@gmx.net> wrote:
> Hi,
>
> On 10/14/2016 04:14 PM, Pranit Bauva wrote:
>> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
>> index 4254d61..d84ba86 100644
>> --- a/builtin/bisect--helper.c
>> +++ b/builtin/bisect--helper.c
>> @@ -84,12 +89,47 @@ static int write_terms(const char *bad, const char *good)
>> return (res < 0) ? -1 : 0;
>> }
>>
>> +static int bisect_reset(const char *commit)
>> +{
>> + struct strbuf branch = STRBUF_INIT;
>> +
>> + if (!commit) {
>> + if (strbuf_read_file(&branch, git_path_bisect_start(), 0) < 1) {
>> + printf("We are not bisecting.\n");
>
> I think this string should be marked for translation.
True. Thanks!
>> + return 0;
>> + }
>> + strbuf_rtrim(&branch);
> [...]
>> @@ -121,6 +163,10 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
>> if (argc != 0)
>> die(_("--bisect-clean-state requires no arguments"));
>> return bisect_clean_state();
>> + case BISECT_RESET:
>> + if (argc > 1)
>> + die(_("--bisect-reset requires either zero or one arguments"));
>
> This error message is imho a little unspecific (but this might not be an
> issue because bisect--helper commands are not really exposed to the
> user). Maybe "--bisect-reset requires either no argument or a commit."?
That sounds good.
>> + return bisect_reset(argc ? argv[0] : NULL);
>> default:
>> die("BUG: unknown subcommand '%d'", cmdmode);
>> }
Regards,
Pranit Bauva
^ permalink raw reply
* [ANNOUNCE] tig-2.2.1
From: Jonas Fonseca @ 2016-11-17 4:04 UTC (permalink / raw)
To: git
Hello,
A new minor version of tig is available which adds support for
diff-highlight (see instructions below) and navigation between merge
commits as well as several keybinding tweaks.
Tarballs should now be downloaded from GitHub. Either go to
https://github.com/jonas/tig/releases or use the following pattern:
https://github.com/jonas/tig/releases/download/tig-VERSION/tig-VERSION.tar.gz
MD5 checksums can be found at:
https://github.com/jonas/tig/releases/download/tig-VERSION/tig-VERSION.tar.gz.md5
Similarly, the home page is now also on GitHub at
https://jonas.github.io/tig/. A big thanks to Simon L. B. Nielsen for
generously hosting Tig on nitro.dk!
Release notes
-------------
Improvements:
- Support Git's 'diff-highlight' program when `diff-highlight` is set
to either true or the path of the script to use for post-processing.
- Add navigation between merge commits. (GH #525)
- Add 'A' as a binding to apply a stash without dropping it.
- Bind 'Ctrl-D' and 'Ctrl-U' to half-page movements by default.
- manual: Mention how to change default Up/Down behavior in diff view.
Bug fixes
- Reorganize checking of libraries for termcap functions.
- Fix `:goto <id>` error message.
Change summary
--------------
The short diffstat and log summary for changes made in this release.
118 files changed, 3765 insertions(+), 3284 deletions(-)
22 Jonas Fonseca
1 Frank Fesevur
1 Jelte Fennema
1 Jeremy Lin
1 Parker Coates
1 Philipp Gesang
1 Ramsay Jones
1 David Lin
1 lightside
--
Jonas Fonseca
^ permalink raw reply
* Re: RFC: Enable delayed responses to Git clean/smudge filter requests
From: Lars Schneider @ 2016-11-17 9:19 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jakub Narębski, Eric Wong, git
In-Reply-To: <xmqq37irouni.fsf@gitster.mtv.corp.google.com>
> On 17 Nov 2016, at 00:46, Junio C Hamano <gitster@pobox.com> wrote:
>
> Jakub Narębski <jnareb@gmail.com> writes:
>
>>> I intend to implement this feature only for the new long running filter
>>> process protocol. OK with you?
>>
>> If I remember and understand it correctly, current version of long
>> running process protocol processes files sequentially, one by one:
>> git sends file to filter wholly, and receives response wholly.
>>
>> In the single-file filter case, git calls filter process as async
>> task, in a separate thread, so that one thread feeds the filter,
>> and main thread (I think?) reads from it, to avoid deadlocks.
>>
>> Couldn't something like this be done for long running filter process,
>> via protocol extension?
>
> My reading of the message you are responding to is that Lars means
> doing so by "implement this feature". Instead of returning the
> filtered bytes, a new protocol lets his filter to say "No result yet
> for you to process, ask me later".
Correct!
^ permalink raw reply
* Re: [PATCH v15 09/27] bisect--helper: `bisect_write` shell function in C
From: Stephan Beyer @ 2016-11-17 9:40 UTC (permalink / raw)
To: Pranit Bauva, git
In-Reply-To: <01020157c38b1ad1-f3a59b1f-9cdb-4d91-b28e-2501facdcb45-000000@eu-west-1.amazonses.com>
Hi,
I've only got some minors to mention here ;)
On 10/14/2016 04:14 PM, Pranit Bauva wrote:
> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
> index c542e8b..3f19b68 100644
> --- a/builtin/bisect--helper.c
> +++ b/builtin/bisect--helper.c
> @@ -19,9 +19,15 @@ static const char * const git_bisect_helper_usage[] = {
> N_("git bisect--helper --write-terms <bad_term> <good_term>"),
> 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>]"),
> NULL
> };
I wouldn't write "<TERM_GOOD <TERM_BAD>" in capital letters. I'd use
something like "<good_term> <bad_term>" as you have used for
--write-terms. Note that "git bisect --help" uses "<term-old>
<term-new>" in that context.
> @@ -149,6 +155,63 @@ static int check_expected_revs(const char **revs, int rev_nr)
> return 0;
> }
>
> +static int bisect_write(const char *state, const char *rev,
> + const struct bisect_terms *terms, int nolog)
> +{
> + struct strbuf tag = STRBUF_INIT;
> + struct strbuf commit_name = STRBUF_INIT;
> + struct object_id oid;
> + struct commit *commit;
> + struct pretty_print_context pp = {0};
> + FILE *fp = NULL;
> + int retval = 0;
> +
> + if (!strcmp(state, terms->term_bad))
> + strbuf_addf(&tag, "refs/bisect/%s", state);
> + else if (one_of(state, terms->term_good, "skip", NULL))
> + strbuf_addf(&tag, "refs/bisect/%s-%s", state, rev);
> + else {
> + error(_("Bad bisect_write argument: %s"), state);
> + retval = -1;
> + goto finish;
> + }
> +
> + if (get_oid(rev, &oid)) {
> + error(_("couldn't get the oid of the rev '%s'"), rev);
> + retval = -1;
> + goto finish;
> + }
> +
> + if (update_ref(NULL, tag.buf, oid.hash, NULL, 0,
> + UPDATE_REFS_MSG_ON_ERR)) {
> + retval = -1;
> + goto finish;
> + }
I'd like to mention that the "goto fail;" trick could apply in this
function, too.
> @@ -156,9 +219,10 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
> WRITE_TERMS,
> BISECT_CLEAN_STATE,
> BISECT_RESET,
> - CHECK_EXPECTED_REVS
> + CHECK_EXPECTED_REVS,
> + BISECT_WRITE
> } cmdmode = 0;
> - int no_checkout = 0;
> + int no_checkout = 0, res = 0;
Why do you do this "direct return" -> "set res and return res" transition?
You don't need it in this patch, you do not need it in the subsequent
patches (you always set "res" exactly once after the initialization),
and you don't need cleanup code in this function.
> struct option options[] = {
> OPT_CMDMODE(0, "next-all", &cmdmode,
> N_("perform 'git bisect next'"), NEXT_ALL),
> @@ -170,10 +234,13 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
> N_("reset the bisection state"), BISECT_RESET),
> OPT_CMDMODE(0, "check-expected-revs", &cmdmode,
> N_("check for expected revs"), CHECK_EXPECTED_REVS),
> + OPT_CMDMODE(0, "bisect-write", &cmdmode,
> + N_("write out the bisection state in BISECT_LOG"), BISECT_WRITE),
That info text is confusing, especially considering that there is a
"nolog" option. I think the action of bisect-write is two-fold: (1)
update the refs, (2) log.
> @@ -182,24 +249,37 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
> usage_with_options(git_bisect_helper_usage, options);
>
> switch (cmdmode) {
> + int nolog;
> case NEXT_ALL:
> return bisect_next_all(prefix, no_checkout);
> case WRITE_TERMS:
> if (argc != 2)
> die(_("--write-terms requires two arguments"));
> - return write_terms(argv[0], argv[1]);
> + res = write_terms(argv[0], argv[1]);
> + break;
As indicated above, I think the direct "return ...;" is cleaner.
~Stephan
^ permalink raw reply
* [PATCH/RFC] ref-filter: support sorting case-insensitively
From: Nguyễn Thái Ngọc Duy @ 2016-11-17 10:21 UTC (permalink / raw)
To: git; +Cc: karthik.188, Nguyễn Thái Ngọc Duy
Similar to version:refname sorting refs by versions, icase:refname
will sort by refnames are usually, but strcasecmp will be used instead
of strcmp. This may be helpful sometimes when people name their
branches <group>-<details> but somebody names it <group>-, some goes
with <Group>-, or even <GROUP>-
Syntax is a big problem. This patch does not support
icase:version:refname or version:icase:refname, for example. If
version sorting learns about this thing, I think I prefer
iversion:refname...
Or perhaps we can. I'm losing touch with for-each-ref "pretty"
formats, I'm not quite sure what's the guideline here.
Another option is just use a symbol, like '-' or '*' to mark
case-insensitivity. But that does not look very descriptive. I don't
see any symbol suggesting this case stuff.
What do you think?
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
Documentation/git-for-each-ref.txt | 3 ++-
ref-filter.c | 8 ++++++--
ref-filter.h | 1 +
3 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index f57e69bc83..e41005cf0e 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -171,7 +171,8 @@ For sorting purposes, fields with numeric values sort in numeric order
All other fields are used to sort in their byte-value order.
There is also an option to sort by versions, this can be done by using
-the fieldname `version:refname` or its alias `v:refname`.
+the fieldname `version:refname` or its alias `v:refname`. Prefixing
+"icase:" (e.g. `icase:refname`) makes sorting case-insensitive.
In any case, a field name that refers to a field inapplicable to
the object referred by the ref does not cause an error. It
diff --git a/ref-filter.c b/ref-filter.c
index d4c2931f3a..fd63b9c710 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1542,12 +1542,12 @@ static int cmp_ref_sorting(struct ref_sorting *s, struct ref_array_item *a, stru
if (s->version)
cmp = versioncmp(va->s, vb->s);
else if (cmp_type == FIELD_STR)
- cmp = strcmp(va->s, vb->s);
+ cmp = s->strcmp(va->s, vb->s);
else {
if (va->ul < vb->ul)
cmp = -1;
else if (va->ul == vb->ul)
- cmp = strcmp(a->refname, b->refname);
+ cmp = s->strcmp(a->refname, b->refname);
else
cmp = 1;
}
@@ -1646,6 +1646,7 @@ struct ref_sorting *ref_default_sorting(void)
sorting->next = NULL;
sorting->atom = parse_ref_filter_atom(cstr_name, cstr_name + strlen(cstr_name));
+ sorting->strcmp = strcmp;
return sorting;
}
@@ -1660,6 +1661,7 @@ int parse_opt_ref_sorting(const struct option *opt, const char *arg, int unset)
s = xcalloc(1, sizeof(*s));
s->next = *sorting_tail;
+ s->strcmp = strcmp;
*sorting_tail = s;
if (*arg == '-') {
@@ -1669,6 +1671,8 @@ int parse_opt_ref_sorting(const struct option *opt, const char *arg, int unset)
if (skip_prefix(arg, "version:", &arg) ||
skip_prefix(arg, "v:", &arg))
s->version = 1;
+ else if (skip_prefix(arg, "icase:", &arg))
+ s->strcmp = strcasecmp;
len = strlen(arg);
s->atom = parse_ref_filter_atom(arg, arg+len);
return 0;
diff --git a/ref-filter.h b/ref-filter.h
index 14d435e2cc..ea2db565f1 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -30,6 +30,7 @@ struct ref_sorting {
int atom; /* index into used_atom array (internal) */
unsigned reverse : 1,
version : 1;
+ int (*strcmp)(const char *, const char *);
};
struct ref_array_item {
--
2.11.0.rc0.161.g80d5b92
^ permalink raw reply related
* Re: [Bug?] git notes are not copied during rebase
From: SZEDER Gábor @ 2016-11-17 10:30 UTC (permalink / raw)
To: Norbert Kiesel; +Cc: SZEDER Gábor, git
In-Reply-To: <CAM+g_NvFhyReNREpTYKbXKm=8QmSH1tnrTCyFm9HusOnfAbCCA@mail.gmail.com>
> I am currently a heavy user of rebasing and noticed that my notes
> don't get correctly applied, even if notes.rewrite.rebase is set
> explicitly to true (though manual says that is the default).
Setting 'notes.rewrite.rebase' is, as you mentioned, not necessary,
but not sufficient either. See here, especially the second paragraph:
notes.rewriteRef
When copying notes during a rewrite, specifies the (fully
qualified) ref whose notes should be copied. May be a glob, in
which case notes in all matching refs will be copied. You may also
specify this configuration several times.
Does not have a default value; you must configure this variable to
enable note rewriting.
Can be overridden with the GIT_NOTES_REWRITE_REF environment
variable.
Gábor
^ permalink raw reply
* RE: merge --no-ff is NOT mentioned in help
From: Vanderhoof, Tzadik @ 2016-11-17 14:03 UTC (permalink / raw)
To: Mike Rappazzo; +Cc: git@vger.kernel.org
In-Reply-To: <CANoM8SVXeeZsc40xgVqZep_9oT=J2h4mOO0Ksn+kb0g8Ct=KrQ@mail.gmail.com>
-----Original Message-----
From: Mike Rappazzo [mailto:rappazzo@gmail.com]
Sent: Wednesday, November 16, 2016 7:58 AM
To: Vanderhoof, Tzadik
Cc: git@vger.kernel.org
Subject: Re: merge --no-ff is NOT mentioned in help
>(Please reply inline)
>
>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
>> --stat show a diffstat at the end of the merge
>> --summary (synonym to --stat)
>> --log[=<n>] add (at most <n>) entries from shortlog to merge commit message
>> --squash create a single commit instead of doing a merge
>> --commit perform a commit if the merge succeeds (default)
>> -e, --edit edit message before committing
>> --ff allow fast-forward (default)
>> --ff-only abort if fast-forward is not possible
>> --rerere-autoupdate update the index with reused conflict resolution if possible
>> --verify-signatures verify that the named commit has a valid GPG signature
>> -s, --strategy <strategy>
>> merge strategy to use
>> -X, --strategy-option <option=value>
>> option for selected merge strategy
>> -m, --message <message>
>> merge commit message (for a non-fast-forward merge)
>> -v, --verbose be more verbose
>> -q, --quiet be more quiet
>> --abort abort the current in-progress merge
>> --allow-unrelated-histories
>> allow merging unrelated histories
>> --progress force progress reporting
>> -S, --gpg-sign[=<key-id>]
>> GPG sign commit
>> --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.
>
Would you accept a pull request from me for this change?
>
>>
>> -----Original Message-----
>> From: Mike Rappazzo [mailto:rappazzo@gmail.com]
>> Sent: Wednesday, November 16, 2016 7:37 AM
>> To: Vanderhoof, Tzadik
>> Cc: git@vger.kernel.org
>> Subject: Re: merge --no-ff is NOT mentioned in help
>>
>> On Wed, Nov 16, 2016 at 10:16 AM, Vanderhoof, Tzadik <tzadik.vanderhoof@optum360.com> wrote:
>>> When I do: "git merge -h" to get help, the option "--no-ff" is left out of the list of options.
>>
>> I am running git version 2.10.0, and running git merge --help contains these lines:
>>
>> --ff
>> When the merge resolves as a fast-forward, only update the branch pointer, without creating a merge commit. This is the default behavior.
>>
>> --no-ff
>> Create a merge commit even when the merge resolves as a fast-forward. This is the default behaviour when merging an annotated (and possibly signed) tag.
>>
>> --ff-only
>> Refuse to merge and exit with a non-zero status unless the current HEAD is already up-to-date or the merge can be resolved as a fast-forward.
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: merge --no-ff is NOT mentioned in help
From: Junio C Hamano @ 2016-11-17 17:10 UTC (permalink / raw)
To: Mike Rappazzo; +Cc: Vanderhoof, Tzadik, git@vger.kernel.org
In-Reply-To: <CANoM8SVXeeZsc40xgVqZep_9oT=J2h4mOO0Ksn+kb0g8Ct=KrQ@mail.gmail.com>
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"));
^ permalink raw reply related
* Re: [PATCH v4 0/4] Speedup finding of unpushed submodules
From: Stefan Beller @ 2016-11-17 17:41 UTC (permalink / raw)
To: Heiko Voigt
Cc: Junio C Hamano, Brandon Williams, git@vger.kernel.org, Jeff King,
Jens Lehmann, Fredrik Gustafsson, Leandro Lucarella
In-Reply-To: <cover.1479308877.git.hvoigt@hvoigt.net>
On Wed, Nov 16, 2016 at 7:11 AM, Heiko Voigt <hvoigt@hvoigt.net> wrote:
> You can find the third iteration of this series here:
>
> http://public-inbox.org/git/cover.1479221071.git.hvoigt@hvoigt.net/
>
> All comments from the last iteration should be addressed.
>
> Cheers Heiko
Thanks for this series!
I looked at the updated diff of hv/submodule-not-yet-pushed-fix
(git diff a1a385d..250ab24) and the series looks good to me.
Thanks,
Stefan
^ permalink raw reply
* Re: [PATCH 01/16] submodule.h: add extern keyword to functions, break line before 80
From: Stefan Beller @ 2016-11-17 18:29 UTC (permalink / raw)
To: Junio C Hamano
Cc: git@vger.kernel.org, Brandon Williams, Jonathan Nieder,
Martin Fick, David Turner
In-Reply-To: <xmqqshqrp7i8.fsf@gitster.mtv.corp.google.com>
On Wed, Nov 16, 2016 at 11:08 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>> submodule.h: add extern keyword to functions, break line before 80
>
> The former is probably a good change for consistency. As the latter
> change breaks a workflow around quickly checking the output from
> "git grep funcname \*.h", I am not sure if it is a good idea.
>
> Especially things like this look like a usability regression:
>
> -void handle_ignore_submodules_arg(struct diff_options *diffopt, const char *);
> +extern void handle_ignore_submodules_arg(struct diff_options *diffopt,
> + const char *);
>
> Perhaps the name "diffopt" can be dropped from there if we want it
> to be shorter.
Does the Git community have an opinion on dropping the name?
Usually the meaning can be inferred from the type such as in this example.
"diffopt" doesn't add information to my understanding when looking at
the header file.
One (cherry-picked) counter example is:
/**
* Like `strbuf_getwholeline`, but operates on a file descriptor.
* It reads one character at a time, so it is very slow. Do not
* use it unless you need the correct position in the file
* descriptor.
*/
extern int strbuf_getwholeline_fd(struct strbuf *, int, int);
where I'd need a bit of time to figure out which of the 2 ints is the
fd and which is the line termination character.
So I'd rather have a broken line than dropping names,
as when grepping for names I'd look them up most of the time
anyway (to e.g. read additional documentation or just reading the
source)
> Names in prototypes are valuable for parameters of
> more generic types like "char *", especially when there are more
> than one parameters of the same type, but in this case the typename
> is specific enough for readers to tell what it is.
Ok, that seems a reasonable to me.
I'll squash the changes below, with an updated commit message:
submodule.h: add extern keyword to functions
As the upcoming series will add a lot of functions to the submodule
header, let's first make the header consistent to the rest of the project
by adding the extern keyword to functions.
As per the CodingGuidelines we try to stay below 80 characters per line,
so adapt all those functions to stay below 80 characters that are already
using more than one line. Those function using just one line are better
kept in one line than breaking them up into multiple lines just for the
goal of staying below the character limit as it makes grepping
for functions easier if they are one liners.
(diff on top of the patch under discussion)
diff --git a/submodule.h b/submodule.h
index afc58d0..2082847 100644
--- a/submodule.h
+++ b/submodule.h
@@ -33,17 +33,14 @@ extern int is_staging_gitmodules_ok(void);
extern int update_path_in_gitmodules(const char *oldpath, const char *newpath);
extern int remove_path_from_gitmodules(const char *path);
extern void stage_updated_gitmodules(void);
-extern void set_diffopt_flags_from_submodule_config(
- struct diff_options *diffopt,
+extern void set_diffopt_flags_from_submodule_config(struct diff_options *,
const char *path);
extern int submodule_config(const char *var, const char *value, void *cb);
extern void gitmodules_config(void);
extern int parse_submodule_update_strategy(const char *value,
struct submodule_update_strategy *dst);
-extern const char *submodule_strategy_to_string(
- const struct submodule_update_strategy *s);
-extern void handle_ignore_submodules_arg(struct diff_options *diffopt,
- const char *);
+extern const char *submodule_strategy_to_string(const struct
submodule_update_strategy *s);
+extern void handle_ignore_submodules_arg(struct diff_options *, const char *);
extern void show_submodule_summary(FILE *f, const char *path,
const char *line_prefix,
struct object_id *one, struct object_id *two,
@@ -72,8 +69,7 @@ extern int find_unpushed_submodules(unsigned char
new_sha1[20],
struct string_list *needs_pushing);
extern int push_unpushed_submodules(unsigned char new_sha1[20],
const char *remotes_name);
-extern void connect_work_tree_and_git_dir(const char *work_tree,
- const char *git_dir);
+extern void connect_work_tree_and_git_dir(const char *work_tree,
const char *git_dir);
extern int parallel_submodules(void);
/*
^ permalink raw reply related
* Re: [PATCH v7 13/17] ref-filter: add `:dir` and `:base` options for ref printing atoms
From: Junio C Hamano @ 2016-11-17 18:35 UTC (permalink / raw)
To: Karthik Nayak; +Cc: Jacob Keller, Git mailing list
In-Reply-To: <CAOLa=ZSaTdACC60g6D6k5frjKkChbkBL8+kLJjNgoutLSe8mOQ@mail.gmail.com>
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?
^ permalink raw reply
* Re: [PATCH 02/16] submodule: modernize ok_to_remove_submodule to use argv_array
From: Stefan Beller @ 2016-11-17 18:36 UTC (permalink / raw)
To: Junio C Hamano
Cc: David Turner, git@vger.kernel.org, bmwill@google.com,
jrnieder@gmail.com, mogulguy10@gmail.com
In-Reply-To: <xmqqwpg3p7rm.fsf@gitster.mtv.corp.google.com>
On Wed, Nov 16, 2016 at 11:03 AM, Junio C Hamano <gitster@pobox.com> wrote:
> David Turner <David.Turner@twosigma.com> writes:
>
>>> - "-u",
>> ...
>>> + argv_array_pushl(&cp.args, "status", "--porcelain", "-uall",
>>
>> This also changes -u to -uall, which is not mentioned in the
>> commit message. That should probably be called out.
>
> Or not making that change at all. Isn't "-u" the same as "-uall"?
Yes it is. My original line of thinking was to have it spelled out
clearly in case
it changes in the future, but then we could argue that the --porcelain parameter
ought to keep the default of -u to "all".
I'll undo that change then.
Thanks,
Stefan
^ permalink raw reply
* [PATCH v2 0/2] bug fix with push --dry-run and submodules
From: Brandon Williams @ 2016-11-17 18:46 UTC (permalink / raw)
To: git; +Cc: Brandon Williams, hvoigt, sbeller, j6t
In-Reply-To: <1479172735-698-1-git-send-email-bmwill@google.com>
v2 of this series is just a small cleanup of removing a nested sub-shell from a
test and rebasing on the latest version of
'origin/hv/submodule-not-yet-pushed-fix'
As stated above this series is based on 'origin/hv/submodule-not-yet-pushed-fix'
Brandon Williams (2):
push: --dry-run updates submodules when --recurse-submodules=on-demand
push: fix --dry-run to not push submodules
submodule.c | 13 ++++++++-----
submodule.h | 4 +++-
t/t5531-deep-submodule-push.sh | 24 ++++++++++++++++++++++++
transport.c | 11 +++++++----
4 files changed, 42 insertions(+), 10 deletions(-)
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply
* [PATCH v2 1/2] push: --dry-run updates submodules when --recurse-submodules=on-demand
From: Brandon Williams @ 2016-11-17 18:46 UTC (permalink / raw)
To: git; +Cc: Brandon Williams, hvoigt, sbeller, j6t
In-Reply-To: <1479408364-150268-1-git-send-email-bmwill@google.com>
This patch adds a test to illustrate how push run with --dry-run doesn't
actually perform a dry-run when push is configured to push submodules
on-demand. Instead all submodules which need to be pushed are actually
pushed to their remotes while any updates for the superproject are
performed as a dry-run. This is a bug and not the intended behaviour of
a dry-run.
Signed-off-by: Brandon Williams <bmwill@google.com>
---
t/t5531-deep-submodule-push.sh | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/t/t5531-deep-submodule-push.sh b/t/t5531-deep-submodule-push.sh
index 198ce84..7840032 100755
--- a/t/t5531-deep-submodule-push.sh
+++ b/t/t5531-deep-submodule-push.sh
@@ -427,7 +427,31 @@ test_expect_success 'push unpushable submodule recursively fails' '
cd submodule.git &&
git rev-parse master >../actual
) &&
+ test_when_finished git -C work reset --hard master^ &&
test_cmp expected actual
'
+test_expect_failure 'push --dry-run does not recursively update submodules' '
+ (
+ cd work/gar/bage &&
+ git checkout master &&
+ git rev-parse master >../../../expected_submodule &&
+ > junk9 &&
+ git add junk9 &&
+ git commit -m "Ninth junk" &&
+
+ # Go up to 'work' directory
+ cd ../.. &&
+ git checkout master &&
+ git rev-parse master >../expected_pub &&
+ git add gar/bage &&
+ git commit -m "Ninth commit for gar/bage" &&
+ git push --dry-run --recurse-submodules=on-demand ../pub.git master
+ ) &&
+ git -C submodule.git rev-parse master >actual_submodule &&
+ git -C pub.git rev-parse master >actual_pub &&
+ test_cmp expected_pub actual_pub &&
+ test_cmp expected_submodule actual_submodule
+'
+
test_done
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* [PATCH v2 2/2] push: fix --dry-run to not push submodules
From: Brandon Williams @ 2016-11-17 18:46 UTC (permalink / raw)
To: git; +Cc: Brandon Williams, hvoigt, sbeller, j6t
In-Reply-To: <1479408364-150268-1-git-send-email-bmwill@google.com>
Teach push to respect the --dry-run option when configured to
recursively push submodules 'on-demand'. This is done by passing the
--dry-run flag to the child process which performs a push for a
submodules when performing a dry-run.
In order to preserve good user experience, the additional check for
unpushed submodules is skipped during a dry-run when
--recurse-submodules=on-demand. The check is skipped because the submodule
pushes were performed as dry-runs and this check would always fail as the
submodules would still need to be pushed.
Signed-off-by: Brandon Williams <bmwill@google.com>
---
submodule.c | 13 ++++++++-----
submodule.h | 4 +++-
t/t5531-deep-submodule-push.sh | 2 +-
transport.c | 11 +++++++----
4 files changed, 19 insertions(+), 11 deletions(-)
diff --git a/submodule.c b/submodule.c
index b509488..de20588 100644
--- a/submodule.c
+++ b/submodule.c
@@ -683,16 +683,17 @@ int find_unpushed_submodules(struct sha1_array *commits,
return needs_pushing->nr;
}
-static int push_submodule(const char *path)
+static int push_submodule(const char *path, int dry_run)
{
if (add_submodule_odb(path))
return 1;
if (for_each_remote_ref_submodule(path, has_remote, NULL) > 0) {
struct child_process cp = CHILD_PROCESS_INIT;
- const char *argv[] = {"push", NULL};
+ argv_array_push(&cp.args, "push");
+ if (dry_run)
+ argv_array_push(&cp.args, "--dry-run");
- cp.argv = argv;
prepare_submodule_repo_env(&cp.env_array);
cp.git_cmd = 1;
cp.no_stdin = 1;
@@ -705,7 +706,9 @@ static int push_submodule(const char *path)
return 1;
}
-int push_unpushed_submodules(struct sha1_array *commits, const char *remotes_name)
+int push_unpushed_submodules(struct sha1_array *commits,
+ const char *remotes_name,
+ int dry_run)
{
int i, ret = 1;
struct string_list needs_pushing = STRING_LIST_INIT_DUP;
@@ -716,7 +719,7 @@ int push_unpushed_submodules(struct sha1_array *commits, const char *remotes_nam
for (i = 0; i < needs_pushing.nr; i++) {
const char *path = needs_pushing.items[i].string;
fprintf(stderr, "Pushing submodule '%s'\n", path);
- if (!push_submodule(path)) {
+ if (!push_submodule(path, dry_run)) {
fprintf(stderr, "Unable to push submodule '%s'\n", path);
ret = 0;
}
diff --git a/submodule.h b/submodule.h
index 9454806..23d7668 100644
--- a/submodule.h
+++ b/submodule.h
@@ -65,7 +65,9 @@ int merge_submodule(unsigned char result[20], const char *path, const unsigned c
const unsigned char a[20], const unsigned char b[20], int search);
int find_unpushed_submodules(struct sha1_array *commits, const char *remotes_name,
struct string_list *needs_pushing);
-int push_unpushed_submodules(struct sha1_array *commits, const char *remotes_name);
+extern int push_unpushed_submodules(struct sha1_array *commits,
+ const char *remotes_name,
+ int dry_run);
void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir);
int parallel_submodules(void);
diff --git a/t/t5531-deep-submodule-push.sh b/t/t5531-deep-submodule-push.sh
index 7840032..1524ff5 100755
--- a/t/t5531-deep-submodule-push.sh
+++ b/t/t5531-deep-submodule-push.sh
@@ -431,7 +431,7 @@ test_expect_success 'push unpushable submodule recursively fails' '
test_cmp expected actual
'
-test_expect_failure 'push --dry-run does not recursively update submodules' '
+test_expect_success 'push --dry-run does not recursively update submodules' '
(
cd work/gar/bage &&
git checkout master &&
diff --git a/transport.c b/transport.c
index c3fdd5d..940b75d 100644
--- a/transport.c
+++ b/transport.c
@@ -921,15 +921,18 @@ int transport_push(struct transport *transport,
if (!is_null_oid(&ref->new_oid))
sha1_array_append(&commits, ref->new_oid.hash);
- if (!push_unpushed_submodules(&commits, transport->remote->name)) {
+ if (!push_unpushed_submodules(&commits,
+ transport->remote->name,
+ pretend)) {
sha1_array_clear(&commits);
- die("Failed to push all needed submodules!");
+ die ("Failed to push all needed submodules!");
}
sha1_array_clear(&commits);
}
- if ((flags & (TRANSPORT_RECURSE_SUBMODULES_ON_DEMAND |
- TRANSPORT_RECURSE_SUBMODULES_CHECK)) && !is_bare_repository()) {
+ if (((flags & TRANSPORT_RECURSE_SUBMODULES_CHECK) ||
+ ((flags & TRANSPORT_RECURSE_SUBMODULES_ON_DEMAND) &&
+ !pretend)) && !is_bare_repository()) {
struct ref *ref = remote_refs;
struct string_list needs_pushing = STRING_LIST_INIT_DUP;
struct sha1_array commits = SHA1_ARRAY_INIT;
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* Re: [PATCH v2 2/2] push: fix --dry-run to not push submodules
From: Stefan Beller @ 2016-11-17 18:59 UTC (permalink / raw)
To: Brandon Williams; +Cc: git@vger.kernel.org, Heiko Voigt, Johannes Sixt
In-Reply-To: <1479408364-150268-3-git-send-email-bmwill@google.com>
On Thu, Nov 17, 2016 at 10:46 AM, Brandon Williams <bmwill@google.com> wrote:
> sha1_array_clear(&commits);
> - die("Failed to push all needed submodules!");
> + die ("Failed to push all needed submodules!");
huh? Is this a whitespace change?
^ permalink raw reply
* Re: [PATCH v2 0/2] bug fix with push --dry-run and submodules
From: Stefan Beller @ 2016-11-17 19:01 UTC (permalink / raw)
To: Brandon Williams; +Cc: git@vger.kernel.org, Heiko Voigt, Johannes Sixt
In-Reply-To: <1479408364-150268-1-git-send-email-bmwill@google.com>
On Thu, Nov 17, 2016 at 10:46 AM, Brandon Williams <bmwill@google.com> wrote:
> v2 of this series is just a small cleanup of removing a nested sub-shell from a
> test and rebasing on the latest version of
> 'origin/hv/submodule-not-yet-pushed-fix'
>
> As stated above this series is based on 'origin/hv/submodule-not-yet-pushed-fix'
an interdiff to v1 would be nice :)
Now t5531 is inconsistent in style,
how much time would you estimate to add a commit to refactor
that test to follow the style with excessive use of -C for
all the other tests and avoiding subshells there, too?
(It's mainly Windows that benefits from such a refactoring immediately,
but consistency is a really nice feat for the long term code health
of the Git project)
Apart from one nit (and this refactoring decision/consistency discussion),
the series looks good!
Thanks,
Stefan
^ permalink raw reply
* [Bug] gpgsign bashcompletion not available
From: NicoHood @ 2016-11-17 18:52 UTC (permalink / raw)
To: git
[-- Attachment #1.1: Type: text/plain, Size: 484 bytes --]
The gpgsign feature is not available via bashcompletion on git.
$ git config --global commit.
commit.status commit.template
$ git config --global commit.gpgsign true
$ git --version
git version 2.10.2
Due to this I could also find no option to automatically sign every tag
that I do. If this option does not exist yet, please add it to the todo
list. If those reports are tracked anywhere else, please also let me
know, so I can test that out.
Cheers,
Nico
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Re: [PATCH v2 2/2] push: fix --dry-run to not push submodules
From: Brandon Williams @ 2016-11-17 19:02 UTC (permalink / raw)
To: Stefan Beller; +Cc: git@vger.kernel.org, Heiko Voigt, Johannes Sixt
In-Reply-To: <CAGZ79kY1x1HWJFjiyFdMFh8S_Y1F0ecLB5-JPb+nPE0gujfF-A@mail.gmail.com>
On 11/17, Stefan Beller wrote:
> On Thu, Nov 17, 2016 at 10:46 AM, Brandon Williams <bmwill@google.com> wrote:
>
> > sha1_array_clear(&commits);
> > - die("Failed to push all needed submodules!");
> > + die ("Failed to push all needed submodules!");
>
> huh? Is this a whitespace change?
That's odd...I didn't mean to add that lone space.
--
Brandon Williams
^ permalink raw reply
* Re: [PATCH v2 0/2] bug fix with push --dry-run and submodules
From: Brandon Williams @ 2016-11-17 19:06 UTC (permalink / raw)
To: Stefan Beller; +Cc: git@vger.kernel.org, Heiko Voigt, Johannes Sixt
In-Reply-To: <CAGZ79kYC51zC4nF3crtuJXK7uwK=Lh9X8LnRke5C87Vo46Gb+A@mail.gmail.com>
On 11/17, Stefan Beller wrote:
> On Thu, Nov 17, 2016 at 10:46 AM, Brandon Williams <bmwill@google.com> wrote:
> > v2 of this series is just a small cleanup of removing a nested sub-shell from a
> > test and rebasing on the latest version of
> > 'origin/hv/submodule-not-yet-pushed-fix'
> >
> > As stated above this series is based on 'origin/hv/submodule-not-yet-pushed-fix'
>
> an interdiff to v1 would be nice :)
>
> Now t5531 is inconsistent in style,
> how much time would you estimate to add a commit to refactor
> that test to follow the style with excessive use of -C for
> all the other tests and avoiding subshells there, too?
I didn't change to an excessive use of the -C option, but rather
eliminated the nested-subshell and instead cd'ed to the required
directories in the subshell. Excessive use of -C seemed to greatly
reduce the readability of the test (at least it did to me).
--
Brandon Williams
^ 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