git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Josh Steadmon <steadmon@google.com>
To: Stefan Beller <sbeller@google.com>
Cc: git <git@vger.kernel.org>
Subject: Re: [PATCH v2 0/4] Add proto v2 archive command with HTTP support
Date: Thu, 27 Sep 2018 11:30:36 -0700	[thread overview]
Message-ID: <20180927183036.GA55647@google.com> (raw)
In-Reply-To: <CAGZ79kaBvHwUUf0rXeAwBY-M+Oi9JjsQnLs4v3FAvx4a9ZRbSA@mail.gmail.com>

On 2018.09.27 11:20, Stefan Beller wrote:
> On Wed, Sep 26, 2018 at 6:25 PM Josh Steadmon <steadmon@google.com> wrote:
> >
> > This is the second version of my series to add a new protocol v2 command
> > for archiving, with support for HTTP(S).
> >
> > NEEDSWORK: a server built with this series is not backwards-compatible
> > with clients that set GIT_PROTOCOL=version=2 or configure
> > protocol.version=2. The old client will unconditionally send "argument
> > ..." packet lines, which breaks the server's expectations of a
> > "command=archive" request,
> 
> So if an old client sets protocol to v2, it would only apply that
> protocol version
> to fetch, not archive, so it would start following a v0 conversation, but
> as the protocol version is set, it would be transmitted to the server.
> This sounds like a bug in the client?

Yeah, basically. We're telling the server we support v2, even if the
specific operation we're trying to do doesn't have a v2 implementation
on the client. So this is going to make it ugly to replace existing
commands.

> >  while the server's capability advertisement
> > in turn breaks the clients expectation of either an ACK or NACK.
> 
> Could a modern client send either another protocol version (3?)
> or a special capability along the protocol version ("fixed_archive")
> 
> > I've been discussing workarounds for this with Jonathan Nieder, but
> > please let me know if you have any suggestions for v3 of this series.
> 
> Care to open the discussion to the list? What are the different
> approaches, what are the pros/cons?

Jonathan suggested something along the lines of what you said above,
adding a new field in GIT_PROTOCOL. So we'd send something like
"version=2:archive_version=2" and have the server detect the latter.

I'm not sure if that's the best way to go about this since I'm not
familiar with the version detection code for other parts of the system.
I worry that it will lead us down the path of having to specify a
version for every command that we eventually convert to protocol v2. On
the other hand, I don't see any other way to work around this, at least
in the archive case. We can't peek at the client's transmissions on the
server, because v2 requires that the server speaks first...

      parent reply	other threads:[~2018-09-27 18:30 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-12  5:35 Add proto v2 archive command with HTTP support Josh Steadmon
2018-09-12  5:35 ` [PATCH 1/3] archive: use packet_reader for communications Josh Steadmon
2018-09-12 22:01   ` Stefan Beller
2018-09-13 14:58   ` Junio C Hamano
2018-09-13 15:34     ` Junio C Hamano
2018-09-12  5:35 ` [PATCH 2/3] archive: implement protocol v2 archive command Josh Steadmon
2018-09-12 22:28   ` Stefan Beller
2018-09-13 18:45     ` Ævar Arnfjörð Bjarmason
2018-09-14  6:05       ` Jonathan Nieder
2018-09-14 14:31         ` Ævar Arnfjörð Bjarmason
2018-09-14 16:14         ` Junio C Hamano
2018-09-14 16:19           ` Jonathan Nieder
2018-09-13 16:31   ` Junio C Hamano
2018-09-14  5:39   ` Jonathan Nieder
2018-09-12  5:35 ` [PATCH 3/3] archive: allow archive over HTTP(S) with proto v2 Josh Steadmon
2018-09-12 22:38   ` Stefan Beller
2018-09-13 16:47   ` Junio C Hamano
2018-09-27 20:28     ` Josh Steadmon
2018-09-14  5:57   ` Jonathan Nieder
2018-09-14  5:36 ` Add proto v2 archive command with HTTP support Jonathan Nieder
2018-09-27  1:24 ` [PATCH v2 0/4] " Josh Steadmon
2018-09-27  1:24   ` [PATCH v2 1/4] archive: follow test standards around assertions Josh Steadmon
2018-09-27 18:38     ` Stefan Beller
2018-09-27  1:24   ` [PATCH v2 2/4] archive: use packet_reader for communications Josh Steadmon
2018-09-27 18:42     ` Stefan Beller
2018-09-27  1:24   ` [PATCH v2 3/4] archive: implement protocol v2 archive command Josh Steadmon
2018-09-27  1:24   ` [PATCH v2 4/4] archive: allow archive over HTTP(S) with proto v2 Josh Steadmon
2018-09-27 18:20   ` [PATCH v2 0/4] Add proto v2 archive command with HTTP support Stefan Beller
2018-09-27 18:30     ` Jonathan Nieder
2018-09-27 22:20       ` Junio C Hamano
2018-09-27 22:33         ` Josh Steadmon
2018-09-28  1:25           ` Junio C Hamano
2018-09-27 18:30     ` Josh Steadmon [this message]

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=20180927183036.GA55647@google.com \
    --to=steadmon@google.com \
    --cc=git@vger.kernel.org \
    --cc=sbeller@google.com \
    /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).