Re: [PATCH 4/6] remote.c: introduce a way to have different remotes for fetch/push

4 messages, 3 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH 4/6] remote.c: introduce a way to have different remotes for fetch/push

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:56:29

Jonathan Nieder [off-list ref] writes:
quoted
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -322,7 +322,7 @@ static int push_with_options(struct transport *transport, int flags)
 static int do_push(const char *repo, int flags)
 {
 	int i, errs;
-	struct remote *remote = remote_get(repo);
+	struct remote *remote = pushremote_get(repo);
"struct remote" has url and pushurl fields.  What do they mean in the
context of these two accessors?  /me is confused.

Is the idea that now I should not use pushurl any more, and that I
should use pushremote_get and use url instead?
I thought the basic idea from the user-level is:

 - If you have to use different URL to push to and fetch from the
   logically same location (e.g. git://k.org/pub/scm/git/git.git
   used for fetch, k.org:/pub/scm/git/git.git/ used for push), use
   url for fetch, pushurl for push and you don't have to bother with
   per-branch pushremote at all.  You are logically working with the
   same remote, perhaps called 'origin'.

 - If you push to and fetch from logically different repositories,
   (e.g. fetch from https://github.com/gitster/git, push to
   github.com:artagnon/git), you may want to call your upstream
   'origin' and your publishing repository 'mine'.  You set the
   remote.pushdefault to 'mine', perhaps like:

	[remote "mine"]
        	url = github.com:artagnon/git

   (this can also be written with remote.mine.pushurl).

By splitting remote_get() used for fetch and pushremote_get() used
for push, the latter function can return 'origin' and 'mine' for
these two cases, while remote_get() will return 'origin' for both of
these cases.  At the programming level, you would still ask what the
URL to be pushed to to the remote obtained here, and would use
pushurl if defined, or url otherwise.

Ram, am I following your thoughts correctly?

Re: [PATCH 4/6] remote.c: introduce a way to have different remotes for fetch/push

From: Jonathan Nieder <hidden>
Date: 2016-06-15 22:56:29

Junio C Hamano wrote:
Jonathan Nieder [off-list ref] writes:
quoted
quoted
-	struct remote *remote = remote_get(repo);
+	struct remote *remote = pushremote_get(repo);
"struct remote" has url and pushurl fields.  What do they mean in the
context of these two accessors?  /me is confused.

Is the idea that now I should not use pushurl any more, and that I
should use pushremote_get and use url instead?
[...]
              At the programming level, you would still ask what the
URL to be pushed to to the remote obtained here, and would use
pushurl if defined, or url otherwise.
Ah, I think I see.  It might be more convenient to the caller if
pushremote_get returned a remote with url set to the pushurl, but
that would prevent sharing the struct with other callers that want
that remote for fetching.

So instead, the idea is something like

	remote: support a different default remote for pushing

	Teach remote_get() to accept an argument FOR_FETCH or FOR_PUSH
	that determines, when no remote is passed to it, whether to use
	the default remote for fetching or the default for pushing.

	The default remote for fetching is stored in the static var
	"default_remote_name", while the default for pushing, if set,
	is in "default_push_remote_name".

	Currently there is never a different default for pushing set
	but later patches will change that.

If remote_get() gained a new required parameter, that would force all
call sites to be examined (even any potential call sites added by new
patches in flight) and there would no longer be need for the
remote_get_1() function.

What do you think?
Jonathan

Re: [PATCH 4/6] remote.c: introduce a way to have different remotes for fetch/push

From: Ramkumar Ramachandra <hidden>
Date: 2016-06-15 22:56:29

Junio C Hamano wrote:
Jonathan Nieder [off-list ref] writes:
quoted
quoted
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -322,7 +322,7 @@ static int push_with_options(struct transport *transport, int flags)
 static int do_push(const char *repo, int flags)
 {
     int i, errs;
-    struct remote *remote = remote_get(repo);
+    struct remote *remote = pushremote_get(repo);
"struct remote" has url and pushurl fields.  What do they mean in the
context of these two accessors?  /me is confused.

Is the idea that now I should not use pushurl any more, and that I
should use pushremote_get and use url instead?
I thought the basic idea from the user-level is:

 - If you have to use different URL to push to and fetch from the
   logically same location (e.g. git://k.org/pub/scm/git/git.git
   used for fetch, k.org:/pub/scm/git/git.git/ used for push), use
   url for fetch, pushurl for push and you don't have to bother with
   per-branch pushremote at all.  You are logically working with the
   same remote, perhaps called 'origin'.

 - If you push to and fetch from logically different repositories,
   (e.g. fetch from https://github.com/gitster/git, push to
   github.com:artagnon/git), you may want to call your upstream
   'origin' and your publishing repository 'mine'.  You set the
   remote.pushdefault to 'mine', perhaps like:

        [remote "mine"]
                url = github.com:artagnon/git

   (this can also be written with remote.mine.pushurl).

By splitting remote_get() used for fetch and pushremote_get() used
for push, the latter function can return 'origin' and 'mine' for
these two cases, while remote_get() will return 'origin' for both of
these cases.  At the programming level, you would still ask what the
URL to be pushed to to the remote obtained here, and would use
pushurl if defined, or url otherwise.

Ram, am I following your thoughts correctly?
Exactly.  I couldn't have said it better myself.

Re: [PATCH 4/6] remote.c: introduce a way to have different remotes for fetch/push

From: Ramkumar Ramachandra <hidden>
Date: 2016-06-15 22:56:29

Jonathan Nieder wrote:
Junio C Hamano wrote:
quoted
Jonathan Nieder [off-list ref] writes:
quoted
quoted
quoted
-   struct remote *remote = remote_get(repo);
+   struct remote *remote = pushremote_get(repo);
"struct remote" has url and pushurl fields.  What do they mean in the
context of these two accessors?  /me is confused.

Is the idea that now I should not use pushurl any more, and that I
should use pushremote_get and use url instead?
[...]
quoted
              At the programming level, you would still ask what the
URL to be pushed to to the remote obtained here, and would use
pushurl if defined, or url otherwise.
Ah, I think I see.  It might be more convenient to the caller if
pushremote_get returned a remote with url set to the pushurl, but
that would prevent sharing the struct with other callers that want
that remote for fetching.
We started off with a generic "remote" (for both fetching and
pushing), then added .pushurl on top of this remote.  Now we've
introduced something called a pushremote, a logically distinct remote
from "remote"; pushremote_get() is meant to return this logically
different remote, falling back to the remote_get() codepath if not
present.  This is a perfect migration that trivially preserves
backward compatibility.

So instead, the idea is something like

        remote: support a different default remote for pushing

        Teach remote_get() to accept an argument FOR_FETCH or FOR_PUSH
        that determines, when no remote is passed to it, whether to use
        the default remote for fetching or the default for pushing.

        The default remote for fetching is stored in the static var
        "default_remote_name", while the default for pushing, if set,
        is in "default_push_remote_name".

        Currently there is never a different default for pushing set
        but later patches will change that.

If remote_get() gained a new required parameter, that would force all
call sites to be examined (even any potential call sites added by new
patches in flight) and there would no longer be need for the
remote_get_1() function.
I like this, but let's save it for a future patch as it requires some
extensive refactoring.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help