Re: [PATCH v2 2/3] for-each-ref: let upstream/push optionally remote ref name

[Johannes Schindelin]

Hi Junio,

On Mon, 16 Oct 2017, Junio C Hamano wrote:

> Johannes Schindelin  writes:
> 
> >> >> -Also respects `:remotename` to state the name of the *remote* instead 
> >> >> of
> >> >> -the ref.
> >> >> +Also respects `:remotename` to state the name of the *remote* instead
> >> >> +of the ref, and `:remoteref` to state the name of the *reference* as
> >> >> +locally known by the remote.
> >> >
> >> > What does "locally known by the remote" mean in this sentence?
> >> 
> >> Good question.  I cannot offhand offer a better and concise
> >> phrasing, but I think can explain what it wants to describe ;-).
> >
> > Yep, described it well.
> >
> > Maybe "and `:remoteref` to state the name by which the remote knows the
> > *reference*"?
> 
> I dunno.  
> 
> The original and your update both seem to come from a worldview
> where there is a single conceptual thing called "reference" that is
> shared between our repository and the remote repository we pull from
> (or push to), and the name we call it is "refs/remotes/origin/devel"
> while the name they use to call it is "refs/heads/devel".  If you
> subscribe to that worldview, then the updated sentence might make it
> clearer than the original.
> 
> But I do not share that worldview, and I am not sure (note: I am
> truly unsure---it's not like I am convinced it is a good idea but
> sugarcoating my expression, at least in this case) if subscribing to
> the worldview helps users' understanding.
> 
> In my view "refs/remotes/origin/devel" is a reference we use to keep
> track of (or "a reference that corresponds to") the reference they
> have called "refs/heads/devel" at the remote, and these are two
> separate entities, so it's not like "this single thing is called
> differently by us and them".
> 
> Stepping back a bit; here is how the description begins.
> 
> upstream::
> The name of a local ref which can be considered ``upstream''
> from the displayed ref.
> 
> So 'upstream' is like 'refs/remotes/origin/devel' in the example in
> the message you are responding to.  Perhaps we can make it clear in
> the description, and then add :remote* modifiers are about asking
> where that remote-tracking branch comes from.  For example, instead
> of these "Also respects...", something like:
> 
> For a %(upstream) that is a remote-tracking branch, the name of
> the remote repository it is copied from can be obtained with
> %(upstream:remotename).  Simiarly, the branch at the remote
> repository whose tip is copioed to this remote-tracking branch
> can be obtined with %(upstream:remoteref) as a full refname.
> 
> may reduce the chance of confusion?

Let's take two more steps back.

First, for-each-ref is a low-level command (I do not remember whether our
nickname for "low-level" is "plumbing" or "porcelain" or anything, so I
stick with "low-level" which I deem descriptive enough). That is, users of
this command (and therefore, readers of this man page) need to be quite
familiar with Git's worldview.

Second, the main purpose of this patch series is to answer the question
"what is the default  in `git push 
:`?" for *many* refs at once.

Maybe it would be better to describe the functionality by describing the
question it tries to answer.

Ciao,
Dscho

Re: [PATCH v2 2/3] for-each-ref: let upstream/push optionally remote ref name

[Junio C Hamano]

Johannes Schindelin  writes:

>> >> -Also respects `:remotename` to state the name of the *remote* instead of
>> >> -the ref.
>> >> +Also respects `:remotename` to state the name of the *remote* instead
>> >> +of the ref, and `:remoteref` to state the name of the *reference* as
>> >> +locally known by the remote.
>> >
>> > What does "locally known by the remote" mean in this sentence?
>> 
>> Good question.  I cannot offhand offer a better and concise
>> phrasing, but I think can explain what it wants to describe ;-).
>
> Yep, described it well.
>
> Maybe "and `:remoteref` to state the name by which the remote knows the
> *reference*"?

I dunno.  

The original and your update both seem to come from a worldview
where there is a single conceptual thing called "reference" that is
shared between our repository and the remote repository we pull from
(or push to), and the name we call it is "refs/remotes/origin/devel"
while the name they use to call it is "refs/heads/devel".  If you
subscribe to that worldview, then the updated sentence might make it
clearer than the original.

But I do not share that worldview, and I am not sure (note: I am
truly unsure---it's not like I am convinced it is a good idea but
sugarcoating my expression, at least in this case) if subscribing to
the worldview helps users' understanding.

In my view "refs/remotes/origin/devel" is a reference we use to keep
track of (or "a reference that corresponds to") the reference they
have called "refs/heads/devel" at the remote, and these are two
separate entities, so it's not like "this single thing is called
differently by us and them".

Stepping back a bit; here is how the description begins.

upstream::
The name of a local ref which can be considered ``upstream''
from the displayed ref.

So 'upstream' is like 'refs/remotes/origin/devel' in the example in
the message you are responding to.  Perhaps we can make it clear in
the description, and then add :remote* modifiers are about asking
where that remote-tracking branch comes from.  For example, instead
of these "Also respects...", something like:

For a %(upstream) that is a remote-tracking branch, the name of
the remote repository it is copied from can be obtained with
%(upstream:remotename).  Simiarly, the branch at the remote
repository whose tip is copioed to this remote-tracking branch
can be obtined with %(upstream:remoteref) as a full refname.

may reduce the chance of confusion?

Re: [PATCH v2 2/3] for-each-ref: let upstream/push optionally remote ref name

[Johannes Schindelin]

Hi,

On Sat, 14 Oct 2017, Junio C Hamano wrote:

> Kevin Daudt  writes:
> 
> > On Thu, Oct 05, 2017 at 02:19:15PM +0200, Johannes Schindelin wrote:
> >> From: J Wyman 
> >> [..] 
> >> 
> >> diff --git a/Documentation/git-for-each-ref.txt 
> >> b/Documentation/git-for-each-ref.txt
> >> index 39f50bd53eb..22767025850 100644
> >> --- a/Documentation/git-for-each-ref.txt
> >> +++ b/Documentation/git-for-each-ref.txt
> >> @@ -142,8 +142,9 @@ upstream::
> >>encountered. Append `:track,nobracket` to show tracking
> >>information without brackets (i.e "ahead N, behind M").
> >>  +
> >> -Also respects `:remotename` to state the name of the *remote* instead of
> >> -the ref.
> >> +Also respects `:remotename` to state the name of the *remote* instead
> >> +of the ref, and `:remoteref` to state the name of the *reference* as
> >> +locally known by the remote.
> >
> > What does "locally known by the remote" mean in this sentence?
> 
> Good question.  I cannot offhand offer a better and concise
> phrasing, but I think can explain what it wants to describe ;-).

Yep, described it well.

Maybe "and `:remoteref` to state the name by which the remote knows the
*reference*"?

Ciao,
Dscho

Re: [PATCH v2 2/3] for-each-ref: let upstream/push optionally remote ref name

[Johannes Schindelin]

Hi Junio,

On Fri, 13 Oct 2017, Junio C Hamano wrote:

> Johannes Schindelin  writes:
> 
> >> (The funny "squash!" followed by a complete version of the
> >> rewritten commit message is what I do until I -- or anybody else
> >> -- comes up with a patch to implement support for "reword!".)
> 
> I have wished for something like that for a long time; it has been
> (and it still is) a bit unclear what the semantics and UI should
> look like, but still, such a feature would be quite useful.

Oh, the reword! semantics are relatively easy in my mind: it would be
called as

git commit --reword 

would take staged changes (if any), otherwise implicitly --allow-empty,
then create a commit message to be edited in this style:

reword! 





This would be presented to the user in an editor (similar to `git commit
--amend`).

Upon rebase --autosquash, it would work like a squash! but comment out all
previous commit messages, and also comment out the reword! line and the
empty line after that.

In case of multiple reword!, the last one would win, and it would
naturally integrate in any fixup!/squash! workflow.

What is *more* difficult is something else I frequently need: drop!
. That is, I want to explicitly mark a commit to be excluded in
subsequent rebase --autosquash. I *guess* the best way would be to
implement a

git revert --drop 

that would work as if you called `git revert -n  && git commit -m
'drop! '"$(git show -s --oneline )", and upon rebase --autosquash,
it would reorder like fixup!/squash!/reword!, replace the `pick` of the
previous line (if it was a `pick`) by `drop` and comment out the current
`pick drop! ` line.

The reason why the semantics are more difficult in that case is that drop!
does not mix well with fixup!/squash!/reword!.

Ciao,
Dscho

Re: [PATCH v2 2/3] for-each-ref: let upstream/push optionally remote ref name

[Junio C Hamano]

Kevin Daudt  writes:

> On Thu, Oct 05, 2017 at 02:19:15PM +0200, Johannes Schindelin wrote:
>> From: J Wyman 
>> [..] 
>> 
>> diff --git a/Documentation/git-for-each-ref.txt 
>> b/Documentation/git-for-each-ref.txt
>> index 39f50bd53eb..22767025850 100644
>> --- a/Documentation/git-for-each-ref.txt
>> +++ b/Documentation/git-for-each-ref.txt
>> @@ -142,8 +142,9 @@ upstream::
>>  encountered. Append `:track,nobracket` to show tracking
>>  information without brackets (i.e "ahead N, behind M").
>>  +
>> -Also respects `:remotename` to state the name of the *remote* instead of
>> -the ref.
>> +Also respects `:remotename` to state the name of the *remote* instead
>> +of the ref, and `:remoteref` to state the name of the *reference* as
>> +locally known by the remote.
>
> What does "locally known by the remote" mean in this sentence?

Good question.  I cannot offhand offer a better and concise
phrasing, but I think can explain what it wants to describe ;-).

For a local branch we have (say, 'topic'), there may be different
things outside this repository that regularly interact with it.

We may update 'topic' with work others did, by

git checkout topic && git pull

without any extra command line argument to "git pull".  We are
pulling from the 'upstream' of the 'topic', which is a branch in a
remote repository, and the (nick)name of the remote is :remotename.
When we do this pull, we would grab one of the branches the remote
has and merge it into our 'topic'.  IOW, when the above command line
is written in longhand, the "git pull" step may look like this [*1*]:

git pull origin devel

if we were building on top of the 'devel' branch at the 'origin'
remote.  The full refname of that branch, 'refs/heads/devel', is
:remoteref, and that is the reference locally known to the 'origin'
remote.

The "locally known by the remote" is an attempt by the patch authors
to stress the fact that the result is not refs/remotes/origin/devel
(which is where _you_ would keep a local copy of that reference in
this repository).

Two other things outside this repository that interact with 'topic'
are where the result of our work is pushed out to, which is a branch
at the 'push' remote.


[Footnotes]

*1* Modulo the details of other refs fetched that are unrelated to
the resulting merge and local copies stored as remote-tracking
branches in this repository.

Re: [PATCH v2 2/3] for-each-ref: let upstream/push optionally remote ref name

[Kevin Daudt]

On Thu, Oct 05, 2017 at 02:19:15PM +0200, Johannes Schindelin wrote:
> From: J Wyman 
> [..] 
> 
> diff --git a/Documentation/git-for-each-ref.txt 
> b/Documentation/git-for-each-ref.txt
> index 39f50bd53eb..22767025850 100644
> --- a/Documentation/git-for-each-ref.txt
> +++ b/Documentation/git-for-each-ref.txt
> @@ -142,8 +142,9 @@ upstream::
>   encountered. Append `:track,nobracket` to show tracking
>   information without brackets (i.e "ahead N, behind M").
>  +
> -Also respects `:remotename` to state the name of the *remote* instead of
> -the ref.
> +Also respects `:remotename` to state the name of the *remote* instead
> +of the ref, and `:remoteref` to state the name of the *reference* as
> +locally known by the remote.

What does "locally known by the remote" mean in this sentence?

>  +
>  Has no effect if the ref does not have tracking information associated
>  with it.  All the options apart from `nobracket` are mutually exclusive,
> @@ -152,9 +153,9 @@ but if used together the last option is selected.

Re: [PATCH v2 2/3] for-each-ref: let upstream/push optionally remote ref name

[Junio C Hamano]

Johannes Schindelin  writes:

>> (The funny "squash!" followed by a complete version of the
>> rewritten commit message is what I do until I -- or anybody else
>> -- comes up with a patch to implement support for "reword!".)

I have wished for something like that for a long time; it has been
(and it still is) a bit unclear what the semantics and UI should
look like, but still, such a feature would be quite useful.

Re: [PATCH v2 2/3] for-each-ref: let upstream/push optionally remote ref name

[Johannes Schindelin]

Hi Junio,

On Wed, 11 Oct 2017, Junio C Hamano wrote:

> Junio C Hamano <gits...@pobox.com> writes:
> 
> > Johannes Schindelin <johannes.schinde...@gmx.de> writes:
> >
> >> Subject: Re: [PATCH v2 2/3] for-each-ref: let upstream/push optionally 
> >> remote ref name
> >
> > No verb?  s/optionally/report/ perhaps?
> 
> I just noticed that I didn't tweak the copy I have in my tree after
> sending this message, but now I did s/optionally/& report the/;

Yes, sorry for not reading this earlier, this is what I meant the commit
subject to say.

> I am still puzzled by the hunk below, though.
> 
> >> @@ -1262,6 +1265,14 @@ static void fill_remote_ref_details(struct 
> >> used_atom *atom, const char *refname,
> >>*s = xstrdup(remote);
> >>else
> >>*s = "";
> >> +  } else if (atom->u.remote_ref.option == RR_REMOTE_REF) {
> >> +  int explicit, for_push = starts_with(atom->name, "push");
> >
> > Hmph, the previous step got rid of starts_with() rather nicely by
> > introducing atom->u.remote_ref.push bit; can't we do the same in
> > this step?

Right, I forgot to edit this. FWIW I have this in my branch now:

-- snip --
[PATCH] squash! for-each-ref: let upstream/push optionally remote ref
 name

for-each-ref: let upstream/push optionally report the remote ref name

There are times when scripts want to know not only the name of the
push branch on the remote, but also the name of the branch as known
by the remote repository.

An example of this is when a tool wants to push to the very same branch
from which it would pull automatically, i.e. the `` and the ``
in `git push  :` would be provided by
`%(upstream:remotename)` and `%(upstream:remoteref)`, respectively.

This patch offers the new suffix :remoteref for the `upstream` and `push`
atoms, allowing to show exactly that. Example:

$ cat .git/config
...
[remote "origin"]
url = https://where.do.we.come/from
fetch = refs/heads/*:refs/remote/origin/*
[branch "master"]
remote = origin
merge = refs/heads/master
[branch "develop/with/topics"]
remote = origin
merge = refs/heads/develop/with/topics
...

$ git for-each-ref \
--format='%(push) %(push:remoteref)' \
refs/heads
refs/remotes/origin/master refs/heads/master
refs/remotes/origin/develop/with/topics refs/heads/develop/with/topics

Signed-off-by: J Wyman <jwy...@microsoft.com>
Signed-off-by: Johannes Schindelin <johannes.schinde...@gmx.de>
---
 ref-filter.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index 2556c7885de..6ab12658cb3 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1268,9 +1268,11 @@ static void fill_remote_ref_details(struct used_atom 
*atom, const char *refname,
else
*s = "";
} else if (atom->u.remote_ref.option == RR_REMOTE_REF) {
-   int explicit, for_push = starts_with(atom->name, "push");
-   const char *merge = remote_ref_for_branch(branch, for_push,
- );
+   int explicit;
+   const char *merge;
+
+   merge = remote_ref_for_branch(branch, atom->u.remote_ref.push,
+ );
if (explicit)
*s = xstrdup(merge);
else
-- 
2.14.2.windows.2
-- snap --

(The funny "squash!" followed by a complete version of the rewritten
commit message is what I do until I -- or anybody else -- comes up with a
patch to implement support for "reword!".)

I'll let this simmer until next week and send out a new iteration of the
patch series then.

Thanks,
Dscho

Re: [PATCH v2 2/3] for-each-ref: let upstream/push optionally remote ref name

[Junio C Hamano]

Junio C Hamano <gits...@pobox.com> writes:

> Johannes Schindelin <johannes.schinde...@gmx.de> writes:
>
>> Subject: Re: [PATCH v2 2/3] for-each-ref: let upstream/push optionally 
>> remote ref name
>
> No verb?  s/optionally/report/ perhaps?

I just noticed that I didn't tweak the copy I have in my tree after
sending this message, but now I did s/optionally/& report the/;

I am still puzzled by the hunk below, though.

>> @@ -1262,6 +1265,14 @@ static void fill_remote_ref_details(struct used_atom 
>> *atom, const char *refname,
>>  *s = xstrdup(remote);
>>  else
>>  *s = "";
>> +} else if (atom->u.remote_ref.option == RR_REMOTE_REF) {
>> +int explicit, for_push = starts_with(atom->name, "push");
>
> Hmph, the previous step got rid of starts_with() rather nicely by
> introducing atom->u.remote_ref.push bit; can't we do the same in
> this step?
>
> Other than that, looks nicer.

Thanks.

Re: [PATCH v2 2/3] for-each-ref: let upstream/push optionally remote ref name

[Junio C Hamano]

Johannes Schindelin <johannes.schinde...@gmx.de> writes:

> Subject: Re: [PATCH v2 2/3] for-each-ref: let upstream/push optionally remote 
> ref name

No verb?  s/optionally/report/ perhaps?

> diff --git a/ref-filter.c b/ref-filter.c
> index 4819707d032..b4b2c9b2190 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -77,7 +77,7 @@ static struct used_atom {
>   struct align align;
>   struct {
>   enum {
> - RR_REF, RR_TRACK, RR_TRACKSHORT, RR_REMOTE_NAME
> + RR_REF, RR_TRACK, RR_TRACKSHORT, 
> RR_REMOTE_NAME, RR_REMOTE_REF
>   } option;
>   struct refname_atom refname;
>   unsigned int nobracket : 1, push : 1, push_remote : 1;
> @@ -164,6 +164,9 @@ static void remote_ref_atom_parser(const struct 
> ref_format *format, struct used_
>   else if (!strcmp(s, "remotename")) {
>   atom->u.remote_ref.option = RR_REMOTE_NAME;
>   atom->u.remote_ref.push_remote = 1;
> + } else if (!strcmp(s, "remoteref")) {
> + atom->u.remote_ref.option = RR_REMOTE_REF;
> + atom->u.remote_ref.push_remote = 1;
>   } else {
>   atom->u.remote_ref.option = RR_REF;
>   
> refname_atom_parser_internal(>u.remote_ref.refname,
> @@ -1262,6 +1265,14 @@ static void fill_remote_ref_details(struct used_atom 
> *atom, const char *refname,
>   *s = xstrdup(remote);
>   else
>   *s = "";
> + } else if (atom->u.remote_ref.option == RR_REMOTE_REF) {
> + int explicit, for_push = starts_with(atom->name, "push");

Hmph, the previous step got rid of starts_with() rather nicely by
introducing atom->u.remote_ref.push bit; can't we do the same in
this step?

Other than that, looks nicer.

Thanks.

[PATCH v2 2/3] for-each-ref: let upstream/push optionally remote ref name

[Johannes Schindelin]

From: J Wyman 

There are times when scripts want to know not only the name of the
push branch on the remote, but also the name of the branch as known
by the remote repository.

An example of this is when a tool wants to push to the very same branch
from which it would pull automatically, i.e. the `` and the ``
in `git push  :` would be provided by
`%(upstream:remotename)` and `%(upstream:remoteref)`, respectively.

This patch offers the new suffix :remoteref for the `upstream` and `push`
atoms, allowing to show exactly that. Example:

$ cat .git/config
...
[remote "origin"]
url = https://where.do.we.come/from
fetch = refs/heads/*:refs/remote/origin/*
[branch "master"]
remote = origin
merge = refs/heads/master
[branch "develop/with/topics"]
remote = origin
merge = refs/heads/develop/with/topics
...

$ git for-each-ref \
--format='%(push) %(push:remoteref)' \
refs/heads
refs/remotes/origin/master refs/heads/master
refs/remotes/origin/develop/with/topics refs/heads/develop/with/topics

Signed-off-by: J Wyman 
Signed-off-by: Johannes Schindelin 
---
 Documentation/git-for-each-ref.txt | 11 ++-
 ref-filter.c   | 13 -
 remote.c   | 30 ++
 remote.h   |  2 ++
 4 files changed, 50 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt 
b/Documentation/git-for-each-ref.txt
index 39f50bd53eb..22767025850 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -142,8 +142,9 @@ upstream::
encountered. Append `:track,nobracket` to show tracking
information without brackets (i.e "ahead N, behind M").
 +
-Also respects `:remotename` to state the name of the *remote* instead of
-the ref.
+Also respects `:remotename` to state the name of the *remote* instead
+of the ref, and `:remoteref` to state the name of the *reference* as
+locally known by the remote.
 +
 Has no effect if the ref does not have tracking information associated
 with it.  All the options apart from `nobracket` are mutually exclusive,
@@ -152,9 +153,9 @@ but if used together the last option is selected.
 push::
The name of a local ref which represents the `@{push}`
location for the displayed ref. Respects `:short`, `:lstrip`,
-   `:rstrip`, `:track`, `:trackshort` and `:remotename` options as
-   `upstream` does. Produces an empty string if no `@{push}` ref is
-   configured.
+   `:rstrip`, `:track`, `:trackshort`, `:remotename`, and `:remoteref`
+   options as `upstream` does. Produces an empty string if no `@{push}`
+   ref is configured.
 
 HEAD::
'*' if HEAD matches current ref (the checked out branch), ' '
diff --git a/ref-filter.c b/ref-filter.c
index 4819707d032..b4b2c9b2190 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -77,7 +77,7 @@ static struct used_atom {
struct align align;
struct {
enum {
-   RR_REF, RR_TRACK, RR_TRACKSHORT, RR_REMOTE_NAME
+   RR_REF, RR_TRACK, RR_TRACKSHORT, 
RR_REMOTE_NAME, RR_REMOTE_REF
} option;
struct refname_atom refname;
unsigned int nobracket : 1, push : 1, push_remote : 1;
@@ -164,6 +164,9 @@ static void remote_ref_atom_parser(const struct ref_format 
*format, struct used_
else if (!strcmp(s, "remotename")) {
atom->u.remote_ref.option = RR_REMOTE_NAME;
atom->u.remote_ref.push_remote = 1;
+   } else if (!strcmp(s, "remoteref")) {
+   atom->u.remote_ref.option = RR_REMOTE_REF;
+   atom->u.remote_ref.push_remote = 1;
} else {
atom->u.remote_ref.option = RR_REF;

refname_atom_parser_internal(>u.remote_ref.refname,
@@ -1262,6 +1265,14 @@ static void fill_remote_ref_details(struct used_atom 
*atom, const char *refname,
*s = xstrdup(remote);
else
*s = "";
+   } else if (atom->u.remote_ref.option == RR_REMOTE_REF) {
+   int explicit, for_push = starts_with(atom->name, "push");
+   const char *merge = remote_ref_for_branch(branch, for_push,
+ );
+   if (explicit)
+   *s = xstrdup(merge);
+   else
+   *s = "";
} else
die("BUG: unhandled RR_* enum");
 }
diff --git a/remote.c b/remote.c
index b220f0dfc61..2bdcfc280cd 100644
---