Git development
 help / color / mirror / Atom feed
* [Bug?] git notes are not copied during rebase
From: Norbert Kiesel @ 2016-11-16 23:38 UTC (permalink / raw)
  To: git

Hi,

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).

Below is a use case that shows that a commit on a branch got rebased,
but the note was not copied to the new commit.

% mkdir notes
% cd notes
% git init
Initialized empty Git repository in /tmp/notes/.git/
% date
% git add a
% git commit -m c1
[master (root-commit) 2e24a91] c1
 1 file changed, 1 insertion(+)
 create mode 100644 a
% git checkout -b mybranch
Switched to a new branch 'mybranch'
% date
% git add b
% git commit -m c2
[mybranch 5ef9954] c2
 1 file changed, 1 insertion(+)
 create mode 100644 b
% git notes add -m note1
% git log
commit 5ef9954 (HEAD -> mybranch)
Author: Norbert Kiesel <nkiesel@metricstream.com>
Date:   Mon Nov 14 15:48:00 2016 -0800

    c2

Notes:
    note1

commit 2e24a91 (master)
Author: Norbert Kiesel <nkiesel@metricstream.com>
Date:   Mon Nov 14 15:48:00 2016 -0800

    c1
% git notes
c39895a0948c17df2028f07c3ec0993a532edabf
5ef9954dbadddfccefe95277be5e7a995335124b
% git checkout master
Switched to branch 'master'
% date
% git commit -a -m c3
[master 1368832] c3
 1 file changed, 1 insertion(+)
% git rebase master mybranch
First, rewinding head to replay your work on top of it...
Applying: c2
% git log
commit 8921cb7 (HEAD -> mybranch)
Author: Norbert Kiesel <nkiesel@metricstream.com>
Date:   Mon Nov 14 15:48:00 2016 -0800

    c2

commit 1368832 (master)
Author: Norbert Kiesel <nkiesel@metricstream.com>
Date:   Mon Nov 14 15:48:00 2016 -0800

    c3

commit 2e24a91
Author: Norbert Kiesel <nkiesel@metricstream.com>
Date:   Mon Nov 14 15:48:00 2016 -0800

    c1
% git notes
c39895a0948c17df2028f07c3ec0993a532edabf
5ef9954dbadddfccefe95277be5e7a995335124b

^ permalink raw reply

* Rebasing cascading topic trees
From: Norbert Kiesel @ 2016-11-16 23:39 UTC (permalink / raw)
  To: git

I currently have a situation with cascading topic branches that I need to rebase
regularly.  In the picture below, I want to rebase the tree starting with `E` to
be rebased onto master (my actually cascade is 4 branches deep).

A--B--C--D (master)
   \
    E--F (topic1)
       \
        G--H (topic2)

After running `git rebase --onto master master topic1`, I end up with

A--B--C--D (master)
   |     \
   \      E'--F' (topic1)
    E--F
       \
        G--H (topic2)

I then need to also run `git rebase --onto topic1 F topic2` to arrive at the
desired

A--B--C--D (master)
   |     \
   \      E'--F' (topic1)
    E--F      \
       |       G'--H' (topic2)
       \
        G--H

Problem here is that I don't have a nice symbolic name for `F` anymore after the
first rebase. Rebasing `topic2` first is not really possible, because I do not
have a new graft-point yet.  I currently write down `F` ahead of time (or use
`reflog` if I forgot) `F`, but I wonder if there is a better solution.

^ permalink raw reply

* Re: Rebasing cascading topic trees
From: Junio C Hamano @ 2016-11-16 23:44 UTC (permalink / raw)
  To: Norbert Kiesel; +Cc: git
In-Reply-To: <CAM+g_Nsiu_qqapB+FvwJCBfwEYLTPdHg4DueQWHq4XDNXMCgpQ@mail.gmail.com>

Norbert Kiesel <nkiesel@gmail.com> writes:

> I currently have a situation with cascading topic branches that I need to rebase
> regularly.  In the picture below, I want to rebase the tree starting with `E` to
> be rebased onto master (my actually cascade is 4 branches deep).
>
> A--B--C--D (master)
>    \
>     E--F (topic1)
>        \
>         G--H (topic2)
>
> After running `git rebase --onto master master topic1`, I end up with
>
> A--B--C--D (master)
>    |     \
>    \      E'--F' (topic1)
>     E--F
>        \
>         G--H (topic2)
>
> I then need to also run `git rebase --onto topic1 F topic2` to arrive at the
> desired
>
> A--B--C--D (master)
>    |     \
>    \      E'--F' (topic1)
>     E--F      \
>        |       G'--H' (topic2)
>        \
>         G--H
>
> Problem here is that I don't have a nice symbolic name for `F` anymore after the
> first rebase. Rebasing `topic2` first is not really possible, because I do not
> have a new graft-point yet.  I currently write down `F` ahead of time (or use
> `reflog` if I forgot) `F`, but I wonder if there is a better solution.

Doesn't topic1@{1} point at "F" after the rebase of the topic1
finishes?


^ permalink raw reply

* Re: RFC: Enable delayed responses to Git clean/smudge filter requests
From: Junio C Hamano @ 2016-11-16 23:46 UTC (permalink / raw)
  To: Jakub Narębski; +Cc: Lars Schneider, Eric Wong, git
In-Reply-To: <5eb682e8-13cb-67f2-a8a9-ec1fa1d139c6@gmail.com>

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".


^ permalink raw reply

* Re: [PATCH v15 08/27] bisect--helper: `is_expected_rev` & `check_expected_revs` shell function in C
From: Stephan Beyer @ 2016-11-16 23:47 UTC (permalink / raw)
  To: Pranit Bauva, git
In-Reply-To: <01020157c38b1ab6-bda8420e-9a63-47d7-9b99-47465b6333d9-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 d84ba86..c542e8b 100644
> --- a/builtin/bisect--helper.c
> +++ b/builtin/bisect--helper.c
> @@ -123,13 +123,40 @@ static int bisect_reset(const char *commit)
>  	return bisect_clean_state();
>  }
>  
> +static int is_expected_rev(const char *expected_hex)
> +{
> +	struct strbuf actual_hex = STRBUF_INIT;
> +	int res = 0;
> +	if (strbuf_read_file(&actual_hex, git_path_bisect_expected_rev(), 0) >= 40) {
> +		strbuf_trim(&actual_hex);
> +		res = !strcmp(actual_hex.buf, expected_hex);
> +	}
> +	strbuf_release(&actual_hex);
> +	return res;
> +}

I am not sure it does what it should.

I would expect the following behavior from this function:
 - file does not exist (or is "broken") => return 0
 - actual_hex != expected_hex => return 0
 - otherwise return 1

If I am not wrong, the code does the following instead:
 - file does not exist (or is "broken") => return 0
 - actual_hex != expected_hex => return 1
 - otherwise => return 0

> +static int check_expected_revs(const char **revs, int rev_nr)
> +{
> +	int i;
> +
> +	for (i = 0; i < rev_nr; i++) {
> +		if (!is_expected_rev(revs[i])) {
> +			unlink_or_warn(git_path_bisect_ancestors_ok());
> +			unlink_or_warn(git_path_bisect_expected_rev());
> +			return 0;
> +		}
> +	}
> +	return 0;
> +}

Here I am not sure what the function *should* do. However, I see that it
basically mimics the behavior of the shell function (assuming
is_expected_rev() is implemented correctly).

I don't understand why the return value is int and not void. To avoid a
"return 0;" line when calling this function?

> @@ -167,6 +196,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
>  		if (argc > 1)
>  			die(_("--bisect-reset requires either zero or one arguments"));
>  		return bisect_reset(argc ? argv[0] : NULL);
> +	case CHECK_EXPECTED_REVS:
> +		return check_expected_revs(argv, argc);

I note that you check the correct number of arguments for some
subcommands and you do not check it for some other subcommands like this
one. (I don't care, I just want to mention it.)

>  	default:
>  		die("BUG: unknown subcommand '%d'", cmdmode);
>  	}

~Stephan

^ permalink raw reply

* Re: Rebasing cascading topic trees
From: Norbert Kiesel @ 2016-11-17  0:12 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqq7f83ouqs.fsf@gitster.mtv.corp.google.com>

Yes, `git rebase --onto topic1 topic1@{1} topic2` is the answer!

Thanks so much, learned something new today.

On Wed, Nov 16, 2016 at 3:44 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Norbert Kiesel <nkiesel@gmail.com> writes:
>
>> I currently have a situation with cascading topic branches that I need to rebase
>> regularly.  In the picture below, I want to rebase the tree starting with `E` to
>> be rebased onto master (my actually cascade is 4 branches deep).
>>
>> A--B--C--D (master)
>>    \
>>     E--F (topic1)
>>        \
>>         G--H (topic2)
>>
>> After running `git rebase --onto master master topic1`, I end up with
>>
>> A--B--C--D (master)
>>    |     \
>>    \      E'--F' (topic1)
>>     E--F
>>        \
>>         G--H (topic2)
>>
>> I then need to also run `git rebase --onto topic1 F topic2` to arrive at the
>> desired
>>
>> A--B--C--D (master)
>>    |     \
>>    \      E'--F' (topic1)
>>     E--F      \
>>        |       G'--H' (topic2)
>>        \
>>         G--H
>>
>> Problem here is that I don't have a nice symbolic name for `F` anymore after the
>> first rebase. Rebasing `topic2` first is not really possible, because I do not
>> have a new graft-point yet.  I currently write down `F` ahead of time (or use
>> `reflog` if I forgot) `F`, but I wonder if there is a better solution.
>
> Doesn't topic1@{1} point at "F" after the rebase of the topic1
> finishes?
>

^ permalink raw reply

* Re: Protecting old temporary objects being reused from concurrent "git gc"?
From: Jeff King @ 2016-11-17  1:04 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Matt McCutchen, git
In-Reply-To: <xmqq1sybqmjt.fsf@gitster.mtv.corp.google.com>

On Wed, Nov 16, 2016 at 10:58:30AM -0800, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > I suspect the issue is that read-tree populates the cache-tree index
> > extension, and then write-tree omits the object write before it even
> > gets to write_sha1_file().
> 
> Wait a minute.  The entries in the index and trees in the cache-tree
> are root of "still in use" traversal for the purpose of pruning,
> which makes the "something like this" patch unnecessary for the real
> index file.
> 
> And for temporary index files that is kept for 6 months, touching
> tree objects that cache-tree references is irrelevant---the blobs
> recorded in the "list of objects" part of the index will go stale,
> which is a lot more problematic.

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").

I could actually see an argument that the read-tree operation should
freshen the blobs themselves (because we know those blobs are now in
active use, and probably shouldn't be pruned), but I am not sure I agree
there. If only because it is weird that an operation which is otherwise
read-only with respect to the repository would modify the object
database.

-Peff

^ permalink raw reply

* 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


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox