git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Glen Choo <chooglen@google.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH v3 2/4] remote: use remote_state parameter internally
Date: Mon, 25 Oct 2021 13:33:50 -0700	[thread overview]
Message-ID: <xmqqzgqwzvwx.fsf@gitster.g> (raw)
In-Reply-To: <kl6lzgqwaocl.fsf@chooglen-macbookpro.roam.corp.google.com> (Glen Choo's message of "Mon, 25 Oct 2021 12:36:26 -0700")

Glen Choo <chooglen@google.com> writes:

> Glen Choo <chooglen@google.com> writes:
>
>>> The caller is remote_get_1(); this funciton was called with
>>> "current_branch", which can be NULL until you have a repository and
>>> you've called read_config(), but otherwise shouldn't be.
>
> One possible culprit is working with detached HEAD, are you pushing with
> detached HEAD?
>
> "current_branch" is allowed to be NULL when HEAD does not point to a
> branch.

Good point.  It is a good justification to make the remote_state
available to the function, as branch==NULL that signals "there is no
current branch in the repository" cannot be dereferenced to get to
either the repository or the remote_state, yet the function needs
access to remote_state even when branch==NULL.

What "branch" is pushremote_for_branch() meant to take?  Is there a
caller that asks a hypothetical "I know this is not a branch that is
the current branch in the repository, but to which remote would we
push if this branch _were_ the current one?" (and passes NULL to
mean "there is a checked out branch, but what happens if our HEAD
were detached?") question?  Even if there isn't currently, do we
want to add such callers in the future?

If the answer is yes, then the function need to take both branch and
remote_state as two separate parameters.  If the answer is no (i.e.
we never ask hypothetical questions, we just ask what we should do
in the current, real, state), then the function can just take the
remote_state and remote_state->branch being NULL would be used as a
signal that the HEAD is detached.  I suspect it is the former, as
this information is used in string-name-to-object translation for
"topic@{push}" in object-name.c::interpret_branch_mark() function.

> However, if you are not using detached HEAD, the problem might be
> elsewhere..

I just checked.  The repository the push is run in is bare and its
HEAD is detached, pointing at a commit directly.

  reply	other threads:[~2021-10-25 20:33 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-07 19:07 [PATCH 0/2] remote: replace static variables with struct remote_state Glen Choo via GitGitGadget
2021-10-07 19:07 ` [PATCH 1/2] remote: move static variables into struct Glen Choo via GitGitGadget
2021-10-07 23:36   ` Junio C Hamano
2021-10-07 19:07 ` [PATCH 2/2] remote: add remote_state to struct repository Glen Choo via GitGitGadget
2021-10-07 23:39   ` Junio C Hamano
2021-10-08 17:30     ` Glen Choo
2021-10-13 19:31 ` [PATCH v2 0/3] remote: replace static variables with struct remote_state Glen Choo
2021-10-13 19:31   ` [PATCH v2 1/3] remote: move static variables into per-repository struct Glen Choo
2021-10-13 20:21     ` Junio C Hamano
2021-10-14 17:25       ` Glen Choo
2021-10-14 18:33         ` Junio C Hamano
2021-10-13 19:31   ` [PATCH v2 2/3] remote: use remote_state parameter internally Glen Choo
2021-10-13 20:23     ` Junio C Hamano
2021-10-13 19:31   ` [PATCH v2 3/3] remote: add struct repository parameter to external functions Glen Choo
2021-10-13 20:24     ` Junio C Hamano
2021-10-13 20:11   ` [PATCH v2 0/3] remote: replace static variables with struct remote_state Junio C Hamano
2021-10-13 20:27     ` Junio C Hamano
2021-10-13 22:00       ` Glen Choo
2021-10-13 21:56     ` Glen Choo
2021-10-13 23:37       ` Junio C Hamano
2021-10-14  1:25         ` Glen Choo
2021-10-19 22:43   ` [PATCH v3 0/4] " Glen Choo
2021-10-19 22:43     ` [PATCH v3 1/4] remote: move static variables into per-repository struct Glen Choo
2021-10-19 22:43     ` [PATCH v3 2/4] remote: use remote_state parameter internally Glen Choo
2021-10-20 19:45       ` Junio C Hamano
2021-10-20 20:31         ` Junio C Hamano
2021-10-20 22:08           ` Junio C Hamano
2021-10-25 18:09           ` Glen Choo
2021-10-25 19:36             ` Glen Choo
2021-10-25 20:33               ` Junio C Hamano [this message]
2021-10-25 23:00                 ` Glen Choo
2021-10-26  0:45                   ` Junio C Hamano
2021-10-26  1:22                     ` Junio C Hamano
2021-10-26 17:04                       ` Glen Choo
2021-10-27  2:28                         ` Junio C Hamano
2021-10-27 17:59                           ` Glen Choo
2021-10-27 20:03                             ` Junio C Hamano
2021-10-19 22:43     ` [PATCH v3 3/4] remote: remove the_repository->remote_state from static methods Glen Choo
2021-10-19 22:43     ` [PATCH v3 4/4] remote: add struct repository parameter to external functions Glen Choo
2021-10-28 18:30     ` [PATCH v4 0/6] remote: replace static variables with struct remote_state Glen Choo
2021-10-28 18:30       ` [PATCH v4 1/6] t5516: add test case for pushing remote refspecs Glen Choo
2021-10-28 20:17         ` Junio C Hamano
2021-11-15 18:42         ` Jonathan Tan
2021-11-15 20:09           ` Glen Choo
2021-10-28 18:30       ` [PATCH v4 2/6] remote: move static variables into per-repository struct Glen Choo
2021-10-28 18:30       ` [PATCH v4 3/6] remote: use remote_state parameter internally Glen Choo
2021-10-28 18:30       ` [PATCH v4 4/6] remote: remove the_repository->remote_state from static methods Glen Choo
2021-11-15 18:48         ` Jonathan Tan
2021-10-28 18:31       ` [PATCH v4 5/6] remote: die if branch is not found in repository Glen Choo
2021-11-15 18:50         ` Jonathan Tan
2021-11-15 20:06           ` Glen Choo
2021-11-16 17:45             ` Jonathan Tan
2021-10-28 18:31       ` [PATCH v4 6/6] remote: add struct repository parameter to external functions Glen Choo
2021-11-15 18:55         ` Jonathan Tan
2021-11-15 21:44           ` Glen Choo
2021-11-12  0:01       ` [PATCH v4 0/6] remote: replace static variables with struct remote_state Glen Choo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqzgqwzvwx.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=chooglen@google.com \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).