threads / patch / 20838

patch, 8 partsVCS helpers

Subject: [PATCH 0/8] VCS helpers

## tl;dr

4 messages between Sep 4, 2009 and Sep 4, 2009. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

Daniel Barkalow· Sep 4, 2009, 02:13 UTC · lore
This is the next version of the db/vcs-helper series in pu.

The first patch is new, a rework of the remote-curl build to produce "remote-curl" and call it as a special case for the sorts of URLs that we accept as indicating something that it now handled by this helper.

The series is rebased onto current next, with some conflicts resolved.

Two patches have been dropped: a memory leak fix for code that was removed entirely by the first patch, and the "mark" helper capability, which is not needed (I believe) due to the "option" fast-import command.

Daniel Barkalow (8):
  Make the "traditionally-supported" URLs a special case
  Use a clearer style to issue commands to remote helpers
  Use a function to determine whether a remote is valid
  Allow fetch to modify refs
  Allow programs to not depend on remotes having urls
  Add a config option for remotes to specify a foreign vcs
  Add support for "import" helper command
  Allow helpers to report in "list" command that the ref is unchanged
 Documentation/config.txt             |    4 +
 Documentation/git-remote-helpers.txt |   14 ++++-
 Makefile                             |   17 +-----
 builtin-clone.c                      |    6 +-
 builtin-fetch.c                      |    7 ++-
 builtin-ls-remote.c                  |    4 +-
 builtin-push.c                       |   67 +++++++++++++--------
 remote.c                             |   15 ++++-
 remote.h                             |    2 +
 transport-helper.c                   |  107 +++++++++++++++++++++++++++++-----
 transport.c                          |   23 +++++--
 transport.h                          |   43 +++++++++++++-
 12 files changed, 234 insertions(+), 75 deletions(-)
Junio C Hamano· Sep 4, 2009, 07:04 UTC · re: Daniel Barkalow · lore

Re: [PATCH 0/8] VCS helpers

Daniel Barkalow <barkalow@iabervon.org> writes:
> This is the next version of the db/vcs-helper series in pu.
Thanks.
Show 5 quoted lines
> The first patch is new, a rework of the remote-curl build to produce
> "remote-curl" and call it as a special case for the sorts of URLs that
> we accept as indicating something that it now handled by this helper.
>
> The series is rebased onto current next, with some conflicts resolved.

Because the theme of the topic does not have anything to do with fixing the deepening of shallow clones nor giving an extended error message from non-fast-forward git-push, I queued the series after reverse-rebasing onto old db/vcs-helper~8, in order to keep the topic branch pure, instead of merging unrelated topics from maint, master or next into it. The result merged in 'pu' obviously has to match what you expected by applying the patches on top of 'next', and I am reasonably sure it does.

> Two patches have been dropped: a memory leak fix for code that was
> removed entirely by the first patch, and the "mark" helper capability,
> which is not needed (I believe) due to the "option" fast-import command.

Johan's cvs-helper series were depending on the previous iteration of this series, but I thought it is being rerolled, so I'd drop it from pu for now.

Daniel Barkalow· Sep 4, 2009, 16:22 UTC · re: Junio C Hamano · lore

Re: [PATCH 0/8] VCS helpers

On Fri, 4 Sep 2009, Junio C Hamano wrote:
Show 19 quoted lines
> Daniel Barkalow <barkalow@iabervon.org> writes:
> 
> > This is the next version of the db/vcs-helper series in pu.
> 
> Thanks.
> 
> > The first patch is new, a rework of the remote-curl build to produce
> > "remote-curl" and call it as a special case for the sorts of URLs that
> > we accept as indicating something that it now handled by this helper.
> >
> > The series is rebased onto current next, with some conflicts resolved.
> 
> Because the theme of the topic does not have anything to do with fixing
> the deepening of shallow clones nor giving an extended error message from
> non-fast-forward git-push, I queued the series after reverse-rebasing onto
> old db/vcs-helper~8, in order to keep the topic branch pure, instead of
> merging unrelated topics from maint, master or next into it.  The result
> merged in 'pu' obviously has to match what you expected by applying the
> patches on top of 'next', and I am reasonably sure it does.

I'd thought that topics in pu were carried as based on next, particularly once they depend on something (e.g., the beginning of the series) in next. I suppose there's better options, but what do you do to find them? (Feel free to refer me to the "note from the maintainer" if it's there, but I don't remember that detail)

FWIW, there was a semantic mismerge in the original basing of this series on 07a4a3b496, which I finally fixed in this version; the code to handle NULL urls in builtin-fetch was after a new conversion of the url.

In any case, I think both the reverse-rebase and merge are correct.
Show 6 quoted lines
> > Two patches have been dropped: a memory leak fix for code that was
> > removed entirely by the first patch, and the "mark" helper capability,
> > which is not needed (I believe) due to the "option" fast-import command.
> 
> Johan's cvs-helper series were depending on the previous iteration of this
> series, but I thought it is being rerolled, so I'd drop it from pu for now.

You could probably stick the "mark" patch into the start of the cvs-helper series until it gets rerolled if you want to keep that series in pu meanwhile; it was at the end of the series, and doesn't have subtle interactions or interact with anything that's changed.

	-Daniel
*This .sig left intentionally blank*
Junio C Hamano· Sep 4, 2009, 17:03 UTC · re: Daniel Barkalow · lore

Re: [PATCH 0/8] VCS helpers

Daniel Barkalow <barkalow@iabervon.org> writes:
Show 15 quoted lines
> On Fri, 4 Sep 2009, Junio C Hamano wrote:
>
>> Because the theme of the topic does not have anything to do with fixing
>> the deepening of shallow clones nor giving an extended error message from
>> non-fast-forward git-push, I queued the series after reverse-rebasing onto
>> old db/vcs-helper~8, in order to keep the topic branch pure, instead of
>> merging unrelated topics from maint, master or next into it.  The result
>> merged in 'pu' obviously has to match what you expected by applying the
>> patches on top of 'next', and I am reasonably sure it does.
>
> I'd thought that topics in pu were carried as based on next, particularly 
> once they depend on something (e.g., the beginning of the series) in next. 
> I suppose there's better options, but what do you do to find them? (Feel 
> free to refer me to the "note from the maintainer" if it's there, but I 
> don't remember that detail)

Oh, I was *not* complaining that you gave a patch based on 'next'. I was merely explaining what I did to your patch, in case somebody wonders why the output of "log -p -8 db/vcs-helper" looks different from what you posted on the list. I also wanted you to verify the result.

Show 5 quoted lines
> FWIW, there was a semantic mismerge in the original basing of this series 
> on 07a4a3b496, which I finally fixed in this version; the code to handle 
> NULL urls in builtin-fetch was after a new conversion of the url.
>
> In any case, I think both the reverse-rebase and merge are correct.

Thanks. Actually, the fact your patch was based on 'next' did help me verify the result of the rebase and the merge.

It is a good discipline to spend some extra effort to keep a topic pure if and only if the other topics that the topic textually depends on were of more dubious quality than the topioc itself. In such a case, there is no guarantee that they graduate ever, and the topic will be blocked without major effort later.

It does not matter in practice when we are reasonably sure that other topics that the topic depends on are likely to be already in when the topic is completed. In this particular case, many of the changes the reverse rebase needed to remove are in fact already in master and some are even in maint, and in retrospect, there was not much point doing this only to risk mistakes.

In fact, I originally merged the commits whose changes overlapped and were already in master (Nico's "deepening shallow", Matthieu's "give better warning to non-fast-forward push", to name a few) on top of the part of db/vcs-helper that was already in 'next', and then applied your patches on top of the result, as a middle ground solution. The topic would not have been as pure as the result I pushed out, but it would still have been much better than merging the whole master before applying the series.

The principle of keeping the topic branch pure in this case may fall into my OCD ;-).

← back to recent threads