Re: linux-next: unnecessary merge in the v4l-dvb tree

[Junio C Hamano]

Linus Torvalds  writes:

> On Tue, Feb 13, 2018 at 9:18 AM, Junio C Hamano  wrote:
>>
>> That makes me wonder if another heuristic I floated earlier is more
>> appropriate.  When merging a tag object T, if refs/tags/T exists and
>> it is that tag object, then an updated "merge" would default to "--ff";
>> otherwise, it would keep the current default of creating a merge even
>> when we could fast-forward, in order to record that tag T in the
>> resulting history.
>
> Oooh. Yes, that sounds like the right thing to do.
>
> So the "no fast-forward" logic would trigger only if the name we used
> for merging is one of the temporary ones (ie .git/{FETCH,MERGE}_HEAD),
> not if the mentioned tag is already a normal tag reference.
>
> Then it's very explicitly about "don't lose the signing information".
>
> I'd still have to teach people to use "--only-ff" if they don't do the
> "fetch and merge" model but literally just do  "git pull upstream
> vX.Y", but at least the case Mauro describes would automatically just
> DTRT.
>
> Me likey.

The implementation cannot exactly be "did the user give FETCH_HEAD
or v4.16-rc1 from the command line?", because we'd want to catch it
when Mauro says "git fetch linus && git merge v4.16-rc1" and behave
identically as "git pull linus v4.16-rc1" (and the latter internally
gets turned into "git merge FETCH_HEAD").

So, instead, we read the "tag" line from the tag object to learn the
tagname T, see if refs/tags/T exists and points at that object, to
see if we are Mauro who follows your tags, or if we are you who
fetch and merge contributors' "for-linus" signed tag (which I am
assuming you won't contaminate your refs/tags/ hierarchy with).

There are a few fallouts in the testsuite if we go this route.  I am
not quite decided if I like the approach.

 builtin/merge.c  | 42 ++
 t/t6200-fmt-merge-msg.sh |  2 +-
 t/t7600-merge.sh |  2 +-
 3 files changed, 40 insertions(+), 6 deletions(-)

diff --git a/builtin/merge.c b/builtin/merge.c
index 30264cfd7c..45c7916505 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -33,6 +33,7 @@
 #include "sequencer.h"
 #include "string-list.h"
 #include "packfile.h"
+#include "tag.h"
 
 #define DEFAULT_TWOHEAD (1<<0)
 #define DEFAULT_OCTOPUS (1<<1)
@@ -1125,6 +1126,42 @@ static struct commit_list *collect_parents(struct commit 
*head_commit,
return remoteheads;
 }
 
+static int merging_a_throwaway_tag(struct commit *commit)
+{
+   const char *tag_ref;
+   struct object_id oid;
+
+   /* Are we merging a tag? */
+   if (!merge_remote_util(commit) ||
+   !merge_remote_util(commit)->obj ||
+   merge_remote_util(commit)->obj->type != OBJ_TAG)
+   return 0;
+
+   /*
+* Now we know we are merging a tag object.  Are we downstream
+* and following the tags from upstream?  If so, we must have
+* the tag object pointed at by "refs/tags/$T" where $T is the
+* tagname recorded in the tag object.  We want to allow such
+* a "just to catch up" merge to fast-forward.
+*/
+   tag_ref = xstrfmt("refs/tags/%s",
+ ((struct tag *)merge_remote_util(commit)->obj)->tag);
+
+   if (!read_ref(tag_ref, ) &&
+   !oidcmp(, _remote_util(commit)->obj->oid))
+   return 0;
+
+   /*
+* Otherwise, we are playing an integrator's role, making a
+* merge with a throw-away tag from a contributor with
+* something like "git pull $contributor $signed_tag".
+* We want to forbid such a merge from fast-forwarding
+* by default; otherwise we would not keep the signature
+* anywhere.
+*/
+   return 1;
+}
+
 int cmd_merge(int argc, const char **argv, const char *prefix)
 {
struct object_id result_tree, stash, head_oid;
@@ -1322,10 +1359,7 @@ int cmd_merge(int argc, const char **argv, const char 
*prefix)
oid_to_hex(>object.oid));
setenv(buf.buf, merge_remote_util(commit)->name, 1);
strbuf_reset();
-   if (fast_forward != FF_ONLY &&
-   merge_remote_util(commit) &&
-   merge_remote_util(commit)->obj &&
-   merge_remote_util(commit)->obj->type == OBJ_TAG)
+   if (fast_forward != FF_ONLY && merging_a_throwaway_tag(commit))
fast_forward = FF_NO;
}
 
diff --git a/t/t6200-fmt-merge-msg.sh b/t/t6200-fmt-merge-msg.sh
index 2e2fb0e957..a54a52aaa4 100755
--- a/t/t6200-fmt-merge-msg.sh
+++ b/t/t6200-fmt-merge-msg.sh
@@ -512,7 +512,7 @@ test_expect_success 'merge-msg with "merging" an annotated 
tag' '
 
test_when_finished "git reset --hard" &&
annote=$(git rev-parse annote) &&
-   git merge --no-commit $annote &&
+   git merge --no-commit --no-ff $annote &&
{
cat <<-EOF
 

Re: linux-next: unnecessary merge in the v4l-dvb tree

[Junio C Hamano]

Linus Torvalds  writes:

> On Tue, Feb 13, 2018 at 9:18 AM, Junio C Hamano  wrote:
>>
>> That makes me wonder if another heuristic I floated earlier is more
>> appropriate.  When merging a tag object T, if refs/tags/T exists and
>> it is that tag object, then an updated "merge" would default to "--ff";
>> otherwise, it would keep the current default of creating a merge even
>> when we could fast-forward, in order to record that tag T in the
>> resulting history.
>
> Oooh. Yes, that sounds like the right thing to do.
>
> So the "no fast-forward" logic would trigger only if the name we used
> for merging is one of the temporary ones (ie .git/{FETCH,MERGE}_HEAD),
> not if the mentioned tag is already a normal tag reference.
>
> Then it's very explicitly about "don't lose the signing information".
>
> I'd still have to teach people to use "--only-ff" if they don't do the
> "fetch and merge" model but literally just do  "git pull upstream
> vX.Y", but at least the case Mauro describes would automatically just
> DTRT.
>
> Me likey.

The implementation cannot exactly be "did the user give FETCH_HEAD
or v4.16-rc1 from the command line?", because we'd want to catch it
when Mauro says "git fetch linus && git merge v4.16-rc1" and behave
identically as "git pull linus v4.16-rc1" (and the latter internally
gets turned into "git merge FETCH_HEAD").

So, instead, we read the "tag" line from the tag object to learn the
tagname T, see if refs/tags/T exists and points at that object, to
see if we are Mauro who follows your tags, or if we are you who
fetch and merge contributors' "for-linus" signed tag (which I am
assuming you won't contaminate your refs/tags/ hierarchy with).

There are a few fallouts in the testsuite if we go this route.  I am
not quite decided if I like the approach.

 builtin/merge.c  | 42 ++
 t/t6200-fmt-merge-msg.sh |  2 +-
 t/t7600-merge.sh |  2 +-
 3 files changed, 40 insertions(+), 6 deletions(-)

diff --git a/builtin/merge.c b/builtin/merge.c
index 30264cfd7c..45c7916505 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -33,6 +33,7 @@
 #include "sequencer.h"
 #include "string-list.h"
 #include "packfile.h"
+#include "tag.h"
 
 #define DEFAULT_TWOHEAD (1<<0)
 #define DEFAULT_OCTOPUS (1<<1)
@@ -1125,6 +1126,42 @@ static struct commit_list *collect_parents(struct commit 
*head_commit,
return remoteheads;
 }
 
+static int merging_a_throwaway_tag(struct commit *commit)
+{
+   const char *tag_ref;
+   struct object_id oid;
+
+   /* Are we merging a tag? */
+   if (!merge_remote_util(commit) ||
+   !merge_remote_util(commit)->obj ||
+   merge_remote_util(commit)->obj->type != OBJ_TAG)
+   return 0;
+
+   /*
+* Now we know we are merging a tag object.  Are we downstream
+* and following the tags from upstream?  If so, we must have
+* the tag object pointed at by "refs/tags/$T" where $T is the
+* tagname recorded in the tag object.  We want to allow such
+* a "just to catch up" merge to fast-forward.
+*/
+   tag_ref = xstrfmt("refs/tags/%s",
+ ((struct tag *)merge_remote_util(commit)->obj)->tag);
+
+   if (!read_ref(tag_ref, ) &&
+   !oidcmp(, _remote_util(commit)->obj->oid))
+   return 0;
+
+   /*
+* Otherwise, we are playing an integrator's role, making a
+* merge with a throw-away tag from a contributor with
+* something like "git pull $contributor $signed_tag".
+* We want to forbid such a merge from fast-forwarding
+* by default; otherwise we would not keep the signature
+* anywhere.
+*/
+   return 1;
+}
+
 int cmd_merge(int argc, const char **argv, const char *prefix)
 {
struct object_id result_tree, stash, head_oid;
@@ -1322,10 +1359,7 @@ int cmd_merge(int argc, const char **argv, const char 
*prefix)
oid_to_hex(>object.oid));
setenv(buf.buf, merge_remote_util(commit)->name, 1);
strbuf_reset();
-   if (fast_forward != FF_ONLY &&
-   merge_remote_util(commit) &&
-   merge_remote_util(commit)->obj &&
-   merge_remote_util(commit)->obj->type == OBJ_TAG)
+   if (fast_forward != FF_ONLY && merging_a_throwaway_tag(commit))
fast_forward = FF_NO;
}
 
diff --git a/t/t6200-fmt-merge-msg.sh b/t/t6200-fmt-merge-msg.sh
index 2e2fb0e957..a54a52aaa4 100755
--- a/t/t6200-fmt-merge-msg.sh
+++ b/t/t6200-fmt-merge-msg.sh
@@ -512,7 +512,7 @@ test_expect_success 'merge-msg with "merging" an annotated 
tag' '
 
test_when_finished "git reset --hard" &&
annote=$(git rev-parse annote) &&
-   git merge --no-commit $annote &&
+   git merge --no-commit --no-ff $annote &&
{
cat <<-EOF
Merge tag '\''$annote'\''
diff --git 

Re: linux-next: unnecessary merge in the v4l-dvb tree

[Linus Torvalds]

On Tue, Feb 13, 2018 at 9:18 AM, Junio C Hamano  wrote:
>
> That makes me wonder if another heuristic I floated earlier is more
> appropriate.  When merging a tag object T, if refs/tags/T exists and
> it is that tag object, then an updated "merge" would default to "--ff";
> otherwise, it would keep the current default of creating a merge even
> when we could fast-forward, in order to record that tag T in the
> resulting history.

Oooh. Yes, that sounds like the right thing to do.

So the "no fast-forward" logic would trigger only if the name we used
for merging is one of the temporary ones (ie .git/{FETCH,MERGE}_HEAD),
not if the mentioned tag is already a normal tag reference.

Then it's very explicitly about "don't lose the signing information".

I'd still have to teach people to use "--only-ff" if they don't do the
"fetch and merge" model but literally just do  "git pull upstream
vX.Y", but at least the case Mauro describes would automatically just
DTRT.

Me likey.

   Linus

Re: linux-next: unnecessary merge in the v4l-dvb tree

[Linus Torvalds]

On Tue, Feb 13, 2018 at 9:18 AM, Junio C Hamano  wrote:
>
> That makes me wonder if another heuristic I floated earlier is more
> appropriate.  When merging a tag object T, if refs/tags/T exists and
> it is that tag object, then an updated "merge" would default to "--ff";
> otherwise, it would keep the current default of creating a merge even
> when we could fast-forward, in order to record that tag T in the
> resulting history.

Oooh. Yes, that sounds like the right thing to do.

So the "no fast-forward" logic would trigger only if the name we used
for merging is one of the temporary ones (ie .git/{FETCH,MERGE}_HEAD),
not if the mentioned tag is already a normal tag reference.

Then it's very explicitly about "don't lose the signing information".

I'd still have to teach people to use "--only-ff" if they don't do the
"fetch and merge" model but literally just do  "git pull upstream
vX.Y", but at least the case Mauro describes would automatically just
DTRT.

Me likey.

   Linus

Re: linux-next: unnecessary merge in the v4l-dvb tree

[Junio C Hamano]

Mauro Carvalho Chehab  writes:

> Yes, that's my pain. I don't want ff only when pulling from others,
> only when pulling from upstream tree.
>
>> 
>> We may want per-remote equivalent for it, i.e. e.g.
>> 
>>  [pull]
>>  ff=false ;# good default for collecting contributions
>> 
>>  [remote "torvalds"] 
>>  pullFF = only ;# good default for catching up
>> 
>> or something like that, perhaps?
>
>
> Yeah, something like that works. Please notice, however, that what I
> usually do is:
>
>   $ git remote update torvalds
>   $ git merge 
> (or git pull . )
>
> So, for the above to work, it should store somehow the remote from
> where a tag came from.

That makes me wonder if another heuristic I floated earlier is more
appropriate.  When merging a tag object T, if refs/tags/T exists and
it is that tag object, then an updated "merge" would default to "--ff";
otherwise, it would keep the current default of creating a merge even
when we could fast-forward, in order to record that tag T in the
resulting history.

Of course, end users can use command line options to override such
heuristics anyway, but if the behaviour based on the new heuristic
is easy to explain and understand, and covers majority of the use
cases without command line override, then we might not even need a
new configuration mechanism like remove.torvalds.pullFF mentioned
above.

Re: linux-next: unnecessary merge in the v4l-dvb tree

[Junio C Hamano]

Mauro Carvalho Chehab  writes:

> Yes, that's my pain. I don't want ff only when pulling from others,
> only when pulling from upstream tree.
>
>> 
>> We may want per-remote equivalent for it, i.e. e.g.
>> 
>>  [pull]
>>  ff=false ;# good default for collecting contributions
>> 
>>  [remote "torvalds"] 
>>  pullFF = only ;# good default for catching up
>> 
>> or something like that, perhaps?
>
>
> Yeah, something like that works. Please notice, however, that what I
> usually do is:
>
>   $ git remote update torvalds
>   $ git merge 
> (or git pull . )
>
> So, for the above to work, it should store somehow the remote from
> where a tag came from.

That makes me wonder if another heuristic I floated earlier is more
appropriate.  When merging a tag object T, if refs/tags/T exists and
it is that tag object, then an updated "merge" would default to "--ff";
otherwise, it would keep the current default of creating a merge even
when we could fast-forward, in order to record that tag T in the
resulting history.

Of course, end users can use command line options to override such
heuristics anyway, but if the behaviour based on the new heuristic
is easy to explain and understand, and covers majority of the use
cases without command line override, then we might not even need a
new configuration mechanism like remove.torvalds.pullFF mentioned
above.

Re: linux-next: unnecessary merge in the v4l-dvb tree

[Mauro Carvalho Chehab]

Em Mon, 12 Feb 2018 15:42:44 -0800
Junio C Hamano  escreveu:

> Linus Torvalds  writes:
> 
> > And some maintainers end up using multiple repositories as branches
> > (the old _original_ git model). Again, you can just use "git fetch +
> > git reset", of course, but that's a bit unsafe. In contrast, doing
> > "git pull --ff-only" is a safe convenient operation that does both the
> > fetch and the update to whatever state.
> >
> > But you do need that "--ff-only" to avoid the merge.  
> 
> OK.  I guess it is legit (and semi-sensible) for downstream
> contributors to "git pull --ff-only $upstream $release_tag_X" to
> bring their long-running topic currently based on release X-1 up to
> date with respect to release X.  It probably makes more sense than
> rebasing on top of release X, even though it makes a lot less sense
> than merging their topics into release X.
> 
> As you said, pull of a tag that forbids fast-forward by default is
> rather old development (I am kind of surprised that it was so old,
> in v1.7.9), so it may be a bit difficult to transition.
> 
> There is 
> 
>   [pull]
> ff = only
> 
> but pull.ff is quite global, and not good for intermediate level
> maintainers who pull to integrate work of their downstream (for
> which they do want the current "do not ff, record the tag in a merge
> commit" behaviour) and also pull to catch up from their upstream
> (which they want "ff-when-able").  They need to control between
> ff=only and ff=when-able, depending on whom they are pulling from.

Yes, that's my pain. I don't want ff only when pulling from others,
only when pulling from upstream tree.

> 
> We may want per-remote equivalent for it, i.e. e.g.
> 
>   [pull]
>   ff=false ;# good default for collecting contributions
> 
>   [remote "torvalds"] 
>   pullFF = only ;# good default for catching up
> 
> or something like that, perhaps?


Yeah, something like that works. Please notice, however, that what I
usually do is:

$ git remote update torvalds
$ git merge 
  (or git pull . )

So, for the above to work, it should store somehow the remote from
where a tag came from.




The reason is that I keep locally a cache with several tree clones
(in bare mode) s that I bother enough to cache (linus, -stable, -next),
as pulling from BR is time consuming, and I want to do it only once
and use the same "cache" for all my git clones.

I have a few git workdirs for my upstream work, but, as a patch
developer, I also have "independent"[1] git repositories.

[1] Due to disk constraints, the clones actually use --shared. So,
the common objects are actually stored inside a single tree.

Thanks,
Mauro

Re: linux-next: unnecessary merge in the v4l-dvb tree

[Mauro Carvalho Chehab]

Em Mon, 12 Feb 2018 15:42:44 -0800
Junio C Hamano  escreveu:

> Linus Torvalds  writes:
> 
> > And some maintainers end up using multiple repositories as branches
> > (the old _original_ git model). Again, you can just use "git fetch +
> > git reset", of course, but that's a bit unsafe. In contrast, doing
> > "git pull --ff-only" is a safe convenient operation that does both the
> > fetch and the update to whatever state.
> >
> > But you do need that "--ff-only" to avoid the merge.  
> 
> OK.  I guess it is legit (and semi-sensible) for downstream
> contributors to "git pull --ff-only $upstream $release_tag_X" to
> bring their long-running topic currently based on release X-1 up to
> date with respect to release X.  It probably makes more sense than
> rebasing on top of release X, even though it makes a lot less sense
> than merging their topics into release X.
> 
> As you said, pull of a tag that forbids fast-forward by default is
> rather old development (I am kind of surprised that it was so old,
> in v1.7.9), so it may be a bit difficult to transition.
> 
> There is 
> 
>   [pull]
> ff = only
> 
> but pull.ff is quite global, and not good for intermediate level
> maintainers who pull to integrate work of their downstream (for
> which they do want the current "do not ff, record the tag in a merge
> commit" behaviour) and also pull to catch up from their upstream
> (which they want "ff-when-able").  They need to control between
> ff=only and ff=when-able, depending on whom they are pulling from.

Yes, that's my pain. I don't want ff only when pulling from others,
only when pulling from upstream tree.

> 
> We may want per-remote equivalent for it, i.e. e.g.
> 
>   [pull]
>   ff=false ;# good default for collecting contributions
> 
>   [remote "torvalds"] 
>   pullFF = only ;# good default for catching up
> 
> or something like that, perhaps?


Yeah, something like that works. Please notice, however, that what I
usually do is:

$ git remote update torvalds
$ git merge 
  (or git pull . )

So, for the above to work, it should store somehow the remote from
where a tag came from.




The reason is that I keep locally a cache with several tree clones
(in bare mode) s that I bother enough to cache (linus, -stable, -next),
as pulling from BR is time consuming, and I want to do it only once
and use the same "cache" for all my git clones.

I have a few git workdirs for my upstream work, but, as a patch
developer, I also have "independent"[1] git repositories.

[1] Due to disk constraints, the clones actually use --shared. So,
the common objects are actually stored inside a single tree.

Thanks,
Mauro

Re: linux-next: unnecessary merge in the v4l-dvb tree

[Junio C Hamano]

Linus Torvalds  writes:

> And some maintainers end up using multiple repositories as branches
> (the old _original_ git model). Again, you can just use "git fetch +
> git reset", of course, but that's a bit unsafe. In contrast, doing
> "git pull --ff-only" is a safe convenient operation that does both the
> fetch and the update to whatever state.
>
> But you do need that "--ff-only" to avoid the merge.

OK.  I guess it is legit (and semi-sensible) for downstream
contributors to "git pull --ff-only $upstream $release_tag_X" to
bring their long-running topic currently based on release X-1 up to
date with respect to release X.  It probably makes more sense than
rebasing on top of release X, even though it makes a lot less sense
than merging their topics into release X.

As you said, pull of a tag that forbids fast-forward by default is
rather old development (I am kind of surprised that it was so old,
in v1.7.9), so it may be a bit difficult to transition.

There is 

[pull]
ff = only

but pull.ff is quite global, and not good for intermediate level
maintainers who pull to integrate work of their downstream (for
which they do want the current "do not ff, record the tag in a merge
commit" behaviour) and also pull to catch up from their upstream
(which they want "ff-when-able").  They need to control between
ff=only and ff=when-able, depending on whom they are pulling from.

We may want per-remote equivalent for it, i.e. e.g.

[pull]
ff=false ;# good default for collecting contributions

[remote "torvalds"] 
pullFF = only ;# good default for catching up

or something like that, perhaps?

Re: linux-next: unnecessary merge in the v4l-dvb tree

[Junio C Hamano]

Linus Torvalds  writes:

> And some maintainers end up using multiple repositories as branches
> (the old _original_ git model). Again, you can just use "git fetch +
> git reset", of course, but that's a bit unsafe. In contrast, doing
> "git pull --ff-only" is a safe convenient operation that does both the
> fetch and the update to whatever state.
>
> But you do need that "--ff-only" to avoid the merge.

OK.  I guess it is legit (and semi-sensible) for downstream
contributors to "git pull --ff-only $upstream $release_tag_X" to
bring their long-running topic currently based on release X-1 up to
date with respect to release X.  It probably makes more sense than
rebasing on top of release X, even though it makes a lot less sense
than merging their topics into release X.

As you said, pull of a tag that forbids fast-forward by default is
rather old development (I am kind of surprised that it was so old,
in v1.7.9), so it may be a bit difficult to transition.

There is 

[pull]
ff = only

but pull.ff is quite global, and not good for intermediate level
maintainers who pull to integrate work of their downstream (for
which they do want the current "do not ff, record the tag in a merge
commit" behaviour) and also pull to catch up from their upstream
(which they want "ff-when-able").  They need to control between
ff=only and ff=when-able, depending on whom they are pulling from.

We may want per-remote equivalent for it, i.e. e.g.

[pull]
ff=false ;# good default for collecting contributions

[remote "torvalds"] 
pullFF = only ;# good default for catching up

or something like that, perhaps?

Re: linux-next: unnecessary merge in the v4l-dvb tree

[Linus Torvalds]

On Mon, Feb 12, 2018 at 1:44 PM, Junio C Hamano  wrote:
>
> But I wonder why "update to upstream" is merging a signed tag in the
> first place.  Wouldn't downstream's "try to keep up with" pull be
> grabbing from branch tips, not tags?

I'm actually encouraging maintainers to *not* start their work on some
random "kernel of the day".

Particularly during the kernel merge window, the upstream master
branch can be pretty flaky. It's *not* a good point to start new
development on, so if you're a maintainer, you really don't want to
use that as the basis for your work for the next merge window.

So I encourage people to use a stable point for new development, and
particularly actual release kernels. The natural way to do that is
obviously just to create a new branch:

   git checkout -b topicbranch v4.15

and now you have a good new clean branch for your development.

But clearly we've got a few people who have gotten used to the whole
"git pull" convenience of both fetching and updating.

Some maintainers don't even use topic branches, because their main
work is just merging work by subdevelepoers (that goes for my own
tree: I use topic branches for merging patch queues and to
occasionally track my own experimental patches, but 99% of the time
I'm just on "master" and obviously pull other peoples branches).

And some maintainers end up using multiple repositories as branches
(the old _original_ git model). Again, you can just use "git fetch +
git reset", of course, but that's a bit unsafe. In contrast, doing
"git pull --ff-only" is a safe convenient operation that does both the
fetch and the update to whatever state.

But you do need that "--ff-only" to avoid the merge.

  Linus

Re: linux-next: unnecessary merge in the v4l-dvb tree

[Linus Torvalds]

On Mon, Feb 12, 2018 at 1:44 PM, Junio C Hamano  wrote:
>
> But I wonder why "update to upstream" is merging a signed tag in the
> first place.  Wouldn't downstream's "try to keep up with" pull be
> grabbing from branch tips, not tags?

I'm actually encouraging maintainers to *not* start their work on some
random "kernel of the day".

Particularly during the kernel merge window, the upstream master
branch can be pretty flaky. It's *not* a good point to start new
development on, so if you're a maintainer, you really don't want to
use that as the basis for your work for the next merge window.

So I encourage people to use a stable point for new development, and
particularly actual release kernels. The natural way to do that is
obviously just to create a new branch:

   git checkout -b topicbranch v4.15

and now you have a good new clean branch for your development.

But clearly we've got a few people who have gotten used to the whole
"git pull" convenience of both fetching and updating.

Some maintainers don't even use topic branches, because their main
work is just merging work by subdevelepoers (that goes for my own
tree: I use topic branches for merging patch queues and to
occasionally track my own experimental patches, but 99% of the time
I'm just on "master" and obviously pull other peoples branches).

And some maintainers end up using multiple repositories as branches
(the old _original_ git model). Again, you can just use "git fetch +
git reset", of course, but that's a bit unsafe. In contrast, doing
"git pull --ff-only" is a safe convenient operation that does both the
fetch and the update to whatever state.

But you do need that "--ff-only" to avoid the merge.

  Linus

Re: linux-next: unnecessary merge in the v4l-dvb tree

[Junio C Hamano]

Linus Torvalds  writes:

> Maybe we could just tell people to have something like
>
>git config --global alias.update pull --ff-only
>
> and use that for "try to update to upstream".

I guess our mails crossed.  I admit that I indeed wondered why you
were not giving your usual "downstream shouldn't do pointless pull
from upstream" briefly but focused too much on how to tweak the
default without thinking through.

But I wonder why "update to upstream" is merging a signed tag in the
first place.  Wouldn't downstream's "try to keep up with" pull be
grabbing from branch tips, not tags?

Re: linux-next: unnecessary merge in the v4l-dvb tree

[Junio C Hamano]

Linus Torvalds  writes:

> Maybe we could just tell people to have something like
>
>git config --global alias.update pull --ff-only
>
> and use that for "try to update to upstream".

I guess our mails crossed.  I admit that I indeed wondered why you
were not giving your usual "downstream shouldn't do pointless pull
from upstream" briefly but focused too much on how to tweak the
default without thinking through.

But I wonder why "update to upstream" is merging a signed tag in the
first place.  Wouldn't downstream's "try to keep up with" pull be
grabbing from branch tips, not tags?

Re: linux-next: unnecessary merge in the v4l-dvb tree

[Linus Torvalds]

On Mon, Feb 12, 2018 at 1:15 PM, Linus Torvalds
 wrote:
>
> The reasoning is to avoid losing the signature from the tag (when
> merging a signed tag, the signature gets inserted into the merge
> commit itself - use "git log --show-signature" to see them).

I think the commit that actually introduced the behavior was

fab47d057: merge: force edit and no-ff mode when merging a tag object

back in 2011, so we've had this behavior for a long time. So it's
probably not be worth tweaking the behavior any more, and maybe we
need to educate people to not update to other peoples state with "git
pull".

Maybe we could just tell people to have something like

   git config --global alias.update pull --ff-only

and use that for "try to update to upstream".

   Linus

Re: linux-next: unnecessary merge in the v4l-dvb tree

[Junio C Hamano]

Linus Torvalds  writes:

> On Mon, Feb 12, 2018 at 1:00 PM, Stephen Rothwell  
> wrote:
>
> The problem, of course, is that since git is distributed, git doesn't
> know who is "upstream" and who is "downstream", so there's no
> _technical_ difference between merging a development tree, and a
> development tree doing a back-merge of the upstream tree.
>
> Maybe it was a mistake to make signed tag merges non-fast-forward,
> since they cause these kinds of issues with people who use "pull" to
> update their otherwise unmodified trees.
>
> I can always teach myself to just use --no-ff, since I end up doing
> things like verifying at the signatures anyway.
>
> Junio, comments?

I have a slight suspicion that allowing 'pull' to fast-forward even
when merging a signed tag when it is pulling from a configured
default remote for the branch the user is on, and otherwise keeping
the current behaviour, would make majority of people from both camps
happier, but I also have a strong conviction that it is being too
clever and making it hard to explain to people to do such a dwim
that tries to guess which way is 'upstream'.

Another clue we _might_ be able to take advantage of is that when
upstream maintainers merge a signed tag, we do *not* fetch and store
the tag from downstream contributers in our local repository (it is
likely that we have --no-tags in remote..tagopt), but when
downstream contributers sync from us with "git pull", they do fetch
and store our tags in their local repository.

So "git pull $somewhere $tag" that defaults to "--ff" when the tag
gets stored somewhere in refs/ (or more explicitly, in refs/tags/)
and defaults to "--no-ff" otherwise (i.e. the tag is fetched only to
be recorded in the resulting merge, without ever stored in any of
our refs), might be a good balance.  

And it is easy to explain: "We realize that it was a mistake to
unconditionally default to --no-ff and we are reverting the default
to --ff, but with a twist.  When we tell 'pull' to grab a tag, if
we do not store it anywhere in our local ref space, that would mean
the tag is totally lost if the pull fast-forwards.  That is why we
still use --no-ff in such a case."

Re: linux-next: unnecessary merge in the v4l-dvb tree

[Linus Torvalds]

On Mon, Feb 12, 2018 at 1:15 PM, Linus Torvalds
 wrote:
>
> The reasoning is to avoid losing the signature from the tag (when
> merging a signed tag, the signature gets inserted into the merge
> commit itself - use "git log --show-signature" to see them).

I think the commit that actually introduced the behavior was

fab47d057: merge: force edit and no-ff mode when merging a tag object

back in 2011, so we've had this behavior for a long time. So it's
probably not be worth tweaking the behavior any more, and maybe we
need to educate people to not update to other peoples state with "git
pull".

Maybe we could just tell people to have something like

   git config --global alias.update pull --ff-only

and use that for "try to update to upstream".

   Linus

Re: linux-next: unnecessary merge in the v4l-dvb tree

[Junio C Hamano]

Linus Torvalds  writes:

> On Mon, Feb 12, 2018 at 1:00 PM, Stephen Rothwell  
> wrote:
>
> The problem, of course, is that since git is distributed, git doesn't
> know who is "upstream" and who is "downstream", so there's no
> _technical_ difference between merging a development tree, and a
> development tree doing a back-merge of the upstream tree.
>
> Maybe it was a mistake to make signed tag merges non-fast-forward,
> since they cause these kinds of issues with people who use "pull" to
> update their otherwise unmodified trees.
>
> I can always teach myself to just use --no-ff, since I end up doing
> things like verifying at the signatures anyway.
>
> Junio, comments?

I have a slight suspicion that allowing 'pull' to fast-forward even
when merging a signed tag when it is pulling from a configured
default remote for the branch the user is on, and otherwise keeping
the current behaviour, would make majority of people from both camps
happier, but I also have a strong conviction that it is being too
clever and making it hard to explain to people to do such a dwim
that tries to guess which way is 'upstream'.

Another clue we _might_ be able to take advantage of is that when
upstream maintainers merge a signed tag, we do *not* fetch and store
the tag from downstream contributers in our local repository (it is
likely that we have --no-tags in remote..tagopt), but when
downstream contributers sync from us with "git pull", they do fetch
and store our tags in their local repository.

So "git pull $somewhere $tag" that defaults to "--ff" when the tag
gets stored somewhere in refs/ (or more explicitly, in refs/tags/)
and defaults to "--no-ff" otherwise (i.e. the tag is fetched only to
be recorded in the resulting merge, without ever stored in any of
our refs), might be a good balance.  

And it is easy to explain: "We realize that it was a mistake to
unconditionally default to --no-ff and we are reverting the default
to --ff, but with a twist.  When we tell 'pull' to grab a tag, if
we do not store it anywhere in our local ref space, that would mean
the tag is totally lost if the pull fast-forwards.  That is why we
still use --no-ff in such a case."

Re: linux-next: unnecessary merge in the v4l-dvb tree

[Mauro Carvalho Chehab]

Em Mon, 12 Feb 2018 13:15:04 -0800
Linus Torvalds  escreveu:

> On Mon, Feb 12, 2018 at 1:00 PM, Stephen Rothwell  
> wrote:
> >
> > Linus, this happens a bit after the merge window, so I am wondering
> > about the rational of not doing a fast forward merge when merging a
> > signed tag (I forget the reasoning).  
> 
> The reasoning is to avoid losing the signature from the tag (when
> merging a signed tag, the signature gets inserted into the merge
> commit itself - use "git log --show-signature" to see them).
> 
> So when I merge a signed tag, I do *not* want to fast-forward to the
> top commit, because then I'd lose the signature from the tag. Thus the
> "merging signed tags are non-fast-forward by default" reasoning.
> 
> But, yes, that reasoning is really only valid for proper merges of new
> features, not for back-merges.
> 
> The problem, of course, is that since git is distributed, git doesn't
> know who is "upstream" and who is "downstream", so there's no
> _technical_ difference between merging a development tree, and a
> development tree doing a back-merge of the upstream tree.
> 
> Maybe it was a mistake to make signed tag merges non-fast-forward,
> since they cause these kinds of issues with people who use "pull" to
> update their otherwise unmodified trees.
> 
> I can always teach myself to just use --no-ff, since I end up doing
> things like verifying at the signatures anyway.

Hmm... at least at git version 2.14.3, git documentation doesn't
mention that signed pull requests won't do fast forward. Instead,
it says that --ff is the default behavior:


   --ff
   When the merge resolves as a fast-forward, only update the branch 
pointer, without creating a merge commit. This is the
   default behavior.

Btw, even doing:

$ git merge -ff v4.16-rc1

it will still produce a git commit for the merge.


-- 
Thanks,
Mauro

Re: linux-next: unnecessary merge in the v4l-dvb tree

[Mauro Carvalho Chehab]

Em Mon, 12 Feb 2018 13:15:04 -0800
Linus Torvalds  escreveu:

> On Mon, Feb 12, 2018 at 1:00 PM, Stephen Rothwell  
> wrote:
> >
> > Linus, this happens a bit after the merge window, so I am wondering
> > about the rational of not doing a fast forward merge when merging a
> > signed tag (I forget the reasoning).  
> 
> The reasoning is to avoid losing the signature from the tag (when
> merging a signed tag, the signature gets inserted into the merge
> commit itself - use "git log --show-signature" to see them).
> 
> So when I merge a signed tag, I do *not* want to fast-forward to the
> top commit, because then I'd lose the signature from the tag. Thus the
> "merging signed tags are non-fast-forward by default" reasoning.
> 
> But, yes, that reasoning is really only valid for proper merges of new
> features, not for back-merges.
> 
> The problem, of course, is that since git is distributed, git doesn't
> know who is "upstream" and who is "downstream", so there's no
> _technical_ difference between merging a development tree, and a
> development tree doing a back-merge of the upstream tree.
> 
> Maybe it was a mistake to make signed tag merges non-fast-forward,
> since they cause these kinds of issues with people who use "pull" to
> update their otherwise unmodified trees.
> 
> I can always teach myself to just use --no-ff, since I end up doing
> things like verifying at the signatures anyway.

Hmm... at least at git version 2.14.3, git documentation doesn't
mention that signed pull requests won't do fast forward. Instead,
it says that --ff is the default behavior:


   --ff
   When the merge resolves as a fast-forward, only update the branch 
pointer, without creating a merge commit. This is the
   default behavior.

Btw, even doing:

$ git merge -ff v4.16-rc1

it will still produce a git commit for the merge.


-- 
Thanks,
Mauro

Re: linux-next: unnecessary merge in the v4l-dvb tree

[Mauro Carvalho Chehab]

Hi Stephen,

Em Tue, 13 Feb 2018 08:00:36 +1100
Stephen Rothwell  escreveu:

> Hi Mauro,
> 
> By merging the v4.16-rc1 tag into the v4l-dvb tree, you have created an
> unnecessary merge commit.  The v4l-dvb tree was already contained in
> v4.16-rc1, so a fast forward merge was possible, but explicitly merging
> a signed tag will give you a commit instead.  In this case, you could
> have just reset your branch to v4.16-rc1 or merged v4.16-rc1^0.

I did the usual way I used to do in the past. Not sure why it ended
by being an explicit merge instead of a fast forward.

On a quick test here, doing:

$ git checkout -b test media/v4.16-2
$ git merge v4.16-rc1

indeed makes produce a merging commit.

I never realized that I could force a fast forward using the weird

$ git merge v4.16-rc1^0

If this is the way we should do git merges, are there a way to make
it default, suppressing the need of a "^0" (either via some .git/config
parameter or changing at git code)? Doing just one such merge once
or twice on every Kernel cycle, I'm pretty sure I'll forget to do 
it next time :-)

> Linus, this happens a bit after the merge window, so I am wondering
> about the rational of not doing a fast forward merge when merging a
> signed tag (I forget the reasoning).

Reset my tree on the top of v4.16-rc1 doesn't seem a good option,
as it causes merge issues from media sub-maintainers and from other
developer's trees.


-- 
Thanks,
Mauro

Re: linux-next: unnecessary merge in the v4l-dvb tree

[Mauro Carvalho Chehab]

Hi Stephen,

Em Tue, 13 Feb 2018 08:00:36 +1100
Stephen Rothwell  escreveu:

> Hi Mauro,
> 
> By merging the v4.16-rc1 tag into the v4l-dvb tree, you have created an
> unnecessary merge commit.  The v4l-dvb tree was already contained in
> v4.16-rc1, so a fast forward merge was possible, but explicitly merging
> a signed tag will give you a commit instead.  In this case, you could
> have just reset your branch to v4.16-rc1 or merged v4.16-rc1^0.

I did the usual way I used to do in the past. Not sure why it ended
by being an explicit merge instead of a fast forward.

On a quick test here, doing:

$ git checkout -b test media/v4.16-2
$ git merge v4.16-rc1

indeed makes produce a merging commit.

I never realized that I could force a fast forward using the weird

$ git merge v4.16-rc1^0

If this is the way we should do git merges, are there a way to make
it default, suppressing the need of a "^0" (either via some .git/config
parameter or changing at git code)? Doing just one such merge once
or twice on every Kernel cycle, I'm pretty sure I'll forget to do 
it next time :-)

> Linus, this happens a bit after the merge window, so I am wondering
> about the rational of not doing a fast forward merge when merging a
> signed tag (I forget the reasoning).

Reset my tree on the top of v4.16-rc1 doesn't seem a good option,
as it causes merge issues from media sub-maintainers and from other
developer's trees.


-- 
Thanks,
Mauro

Re: linux-next: unnecessary merge in the v4l-dvb tree

[Linus Torvalds]

On Mon, Feb 12, 2018 at 1:00 PM, Stephen Rothwell  wrote:
>
> Linus, this happens a bit after the merge window, so I am wondering
> about the rational of not doing a fast forward merge when merging a
> signed tag (I forget the reasoning).

The reasoning is to avoid losing the signature from the tag (when
merging a signed tag, the signature gets inserted into the merge
commit itself - use "git log --show-signature" to see them).

So when I merge a signed tag, I do *not* want to fast-forward to the
top commit, because then I'd lose the signature from the tag. Thus the
"merging signed tags are non-fast-forward by default" reasoning.

But, yes, that reasoning is really only valid for proper merges of new
features, not for back-merges.

The problem, of course, is that since git is distributed, git doesn't
know who is "upstream" and who is "downstream", so there's no
_technical_ difference between merging a development tree, and a
development tree doing a back-merge of the upstream tree.

Maybe it was a mistake to make signed tag merges non-fast-forward,
since they cause these kinds of issues with people who use "pull" to
update their otherwise unmodified trees.

I can always teach myself to just use --no-ff, since I end up doing
things like verifying at the signatures anyway.

Junio, comments?

   Linus

Re: linux-next: unnecessary merge in the v4l-dvb tree

[Linus Torvalds]

On Mon, Feb 12, 2018 at 1:00 PM, Stephen Rothwell  wrote:
>
> Linus, this happens a bit after the merge window, so I am wondering
> about the rational of not doing a fast forward merge when merging a
> signed tag (I forget the reasoning).

The reasoning is to avoid losing the signature from the tag (when
merging a signed tag, the signature gets inserted into the merge
commit itself - use "git log --show-signature" to see them).

So when I merge a signed tag, I do *not* want to fast-forward to the
top commit, because then I'd lose the signature from the tag. Thus the
"merging signed tags are non-fast-forward by default" reasoning.

But, yes, that reasoning is really only valid for proper merges of new
features, not for back-merges.

The problem, of course, is that since git is distributed, git doesn't
know who is "upstream" and who is "downstream", so there's no
_technical_ difference between merging a development tree, and a
development tree doing a back-merge of the upstream tree.

Maybe it was a mistake to make signed tag merges non-fast-forward,
since they cause these kinds of issues with people who use "pull" to
update their otherwise unmodified trees.

I can always teach myself to just use --no-ff, since I end up doing
things like verifying at the signatures anyway.

Junio, comments?

   Linus

linux-next: unnecessary merge in the v4l-dvb tree

[Stephen Rothwell]

Hi Mauro,

By merging the v4.16-rc1 tag into the v4l-dvb tree, you have created an
unnecessary merge commit.  The v4l-dvb tree was already contained in
v4.16-rc1, so a fast forward merge was possible, but explicitly merging
a signed tag will give you a commit instead.  In this case, you could
have just reset your branch to v4.16-rc1 or merged v4.16-rc1^0.

Linus, this happens a bit after the merge window, so I am wondering
about the rational of not doing a fast forward merge when merging a
signed tag (I forget the reasoning).
-- 
Cheers,
Stephen Rothwell

linux-next: unnecessary merge in the v4l-dvb tree

[Stephen Rothwell]

Hi Mauro,

By merging the v4.16-rc1 tag into the v4l-dvb tree, you have created an
unnecessary merge commit.  The v4l-dvb tree was already contained in
v4.16-rc1, so a fast forward merge was possible, but explicitly merging
a signed tag will give you a commit instead.  In this case, you could
have just reset your branch to v4.16-rc1 or merged v4.16-rc1^0.

Linus, this happens a bit after the merge window, so I am wondering
about the rational of not doing a fast forward merge when merging a
signed tag (I forget the reasoning).
-- 
Cheers,
Stephen Rothwell