[PATCH 00/23] builtin-remote improvments

[PATCH 00/23] builtin-remote improvments

[Jay Soffian]

Junio, this is a big series, so let me explain.

Patches 1-5 are js/remote-set-head, but rebased on master (2d602e9). I did
this because patch 18 in this series depends on commit cfa1ee6 which is now in
master, but was not when you originally spawned js/remote-set-head. I also
corrected two failing tests in #3.

Patches 6-18 are the 13 patch series I sent out earlier, directly on-top of
1-5, instead of on pu. I incorporated the feedback I received, and re-ordered
the patches to put all the refactoring, as well as non-builtin-remote changes,
first among the 13.

Patches 19-23 are jk/head-lookup, now based at the end of my changes.
Previously they were effectively between 1-5 and 6-18.

You might consider applying #6 (make match_refs() copy src...) independently
to master if it is obvious enough, but in any case it has to come before #18
in this series.

I wasn't sure what to do about the SOB on the rebased patches, so I kept
Jeff's, removed yours, and added my own. Please correct me if this was the
wrong thing to do.

Finally, I've and compiled and run t5505-remote.sh after each individual
commit, as well as the entire default test suite (but
w/NO_SVN_TESTS=YesPlease) after #10 and #23.

Thank you for your patience with this topic.

j.

Jay Soffian (18):
  builtin-clone: move locate_head() to remote.c so it can be re-used
  builtin-remote: move duplicated cleanup code its own function
  builtin-remote: teach show to display remote HEAD
  builtin-remote: add set-head subcommand
  builtin-remote: better handling of multiple remote HEADs

  remote.c: make match_refs() copy src ref before assigning to peer_ref
  remote.c: don't short-circuit match_refs() when error in match_explicit_refs()
  refactor duplicated get_local_heads() to remote.c
  refactor duplicated ref_newer() to remote.c
  string-list: new for_each_string_list() function
  builtin-remote: rename variable and eliminate redundant function call
  builtin-remote: name remote_refs consistently
  builtin-remote: remove unused code in get_ref_states
  builtin-remote: fix two inconsistencies in the output of "show <remote>"
  builtin-remote: make get_remote_ref_states() always populate states.tracked
  builtin-remote: refactor get_remote_ref_states()
  builtin-remote: new show output style
  builtin-remote: new show output style for push refspecs

Jeff King (5):
  test scripts: refactor start_httpd helper
  add basic http clone/fetch tests
  refactor find_refs_by_name to accept const list
  remote: refactor guess_remote_head
  remote: use exact HEAD lookup if it is available

 Documentation/git-remote.txt           |   28 ++-
 Makefile                               |    1 +
 builtin-clone.c                        |   42 +---
 builtin-remote.c                       |  562 ++++++++++++++++++++++++++------
 builtin-send-pack.c                    |   79 +-----
 cache.h                                |    2 +-
 contrib/completion/git-completion.bash |    2 +-
 http-push.c                            |   72 +----
 refs.c                                 |    4 +-
 remote.c                               |  137 ++++++++-
 remote.h                               |   12 +
 string-list.c                          |   10 +
 string-list.h                          |    5 +
 t/lib-httpd.sh                         |    9 +-
 t/t5505-remote.sh                      |  122 +++++--
 t/t5540-http-push.sh                   |    9 +-
 t/t5550-http-fetch.sh                  |   57 ++++
 17 files changed, 817 insertions(+), 336 deletions(-)
 create mode 100755 t/t5550-http-fetch.sh

Re: [PATCH 00/23] builtin-remote improvments

[Jay Soffian]

On Tue, Feb 24, 2009 at 4:50 AM, Jay Soffian <jaysoffian@gmail.com> wrote:
> Jay Soffian (18):
>  builtin-clone: move locate_head() to remote.c so it can be re-used
>  builtin-remote: move duplicated cleanup code its own function
>  builtin-remote: teach show to display remote HEAD
>  builtin-remote: add set-head subcommand
>  builtin-remote: better handling of multiple remote HEADs
>
>  remote.c: make match_refs() copy src ref before assigning to peer_ref
>  remote.c: don't short-circuit match_refs() when error in match_explicit_refs()
>  refactor duplicated get_local_heads() to remote.c
>  refactor duplicated ref_newer() to remote.c
>  string-list: new for_each_string_list() function
>  builtin-remote: rename variable and eliminate redundant function call
>  builtin-remote: name remote_refs consistently
>  builtin-remote: remove unused code in get_ref_states
>  builtin-remote: fix two inconsistencies in the output of "show <remote>"
>  builtin-remote: make get_remote_ref_states() always populate states.tracked
>  builtin-remote: refactor get_remote_ref_states()
>  builtin-remote: new show output style
>  builtin-remote: new show output style for push refspecs
>
> Jeff King (5):
>  test scripts: refactor start_httpd helper
>  add basic http clone/fetch tests
>  refactor find_refs_by_name to accept const list
>  remote: refactor guess_remote_head
>  remote: use exact HEAD lookup if it is available

Argh. Looking at this again, it seems clear to me now that I should've put Jeff's series at the beginning, followed by my refactoring (6-10), followed by the rest of my patches, so that all the refactoring is early.

And I forgot that I'd done patch #5 (better handling of multiple remote HEADs) a few days after 1-4. As part of this re-roll I should've redone 1-4 to include #5's functionality.

I also noticed I'm passing an argument to get_head_names() that it never uses. But fixing that touches at least 3 of the patches.

All of this is my way of saying ignore what I've emailed tonight till I can clean it up a bit more.

I apologize to the list for the spam.

When I have the series really ready, would you prefer I setup a repo for you to pull from? I'm loath to spam the list with another 24 messages.

j.

Re: [PATCH 00/23] builtin-remote improvments

[Marius Storm-Olsen]

[-- Attachment #1: Type: text/plain, Size: 558 bytes --]

> All of this is my way of saying ignore what I've emailed tonight
> till I can clean it up a bit more.
> 
> I apologize to the list for the spam.
> 
> When I have the series really ready, would you prefer I setup a
> repo for you to pull from? I'm loath to spam the list with another
> 24 messages.

It would at least be great if you could include --thread in your 
options to git format-patch, so your patches are threaded underneath 
your cover letter ;-)

-- 
.marius [@trolltech.com]
'if you know what you're doing, it's not research'


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 187 bytes --]

Re: [PATCH 00/23] builtin-remote improvments

[Jay Soffian]

On Tue, Feb 24, 2009 at 6:01 AM, Marius Storm-Olsen
<marius@trolltech.com> wrote:
>> All of this is my way of saying ignore what I've emailed tonight
>> till I can clean it up a bit more.
>>
>> I apologize to the list for the spam.
>>
>> When I have the series really ready, would you prefer I setup a
>> repo for you to pull from? I'm loath to spam the list with another
>> 24 messages.
>
> It would at least be great if you could include --thread in your options to
> git format-patch, so your patches are threaded underneath your cover letter
> ;-)

I did, but I had run format-patch twice because I changed a patch.
Unfortunately I kept the cover-letter from the first run but forgot to
update its message-id. So all the patches reply to a non-existent
message.

:-(

j.

Re: [PATCH 00/23] builtin-remote improvments

[Junio C Hamano]

Jay Soffian <jaysoffian@gmail.com> writes:

> When I have the series really ready, would you prefer I setup a repo for
> you to pull from? I'm loath to spam the list with another 24 messages.

I don't mind pulling, as it is less work for me, but it would make
harder for others to see the changes in flight and discuss, so...

Even though I'd need to re-read them before merging them to 'next', I also
think the earlier remote-set-head series and Jeff's changes on top of it
looked more or less reasonable.  Perhaps I should rebase them on top of
'master' before moving them to 'next' and then you can build on top, which
would reduce your new series by about 10 commits.

Re: [PATCH 00/23] builtin-remote improvments

[Jay Soffian]

On Tue, Feb 24, 2009 at 12:39 PM, Junio C Hamano <gitster@pobox.com> wrote:
> I don't mind pulling, as it is less work for me, but it would make
> harder for others to see the changes in flight and discuss, so...
>
> Even though I'd need to re-read them before merging them to 'next', I also
> think the earlier remote-set-head series and Jeff's changes on top of it
> looked more or less reasonable.  Perhaps I should rebase them on top of
> 'master' before moving them to 'next' and then you can build on top, which
> would reduce your new series by about 10 commits.

Putting Jeff's changes first reduces churn a small amount, as we don't
introduce new code just to change it a few commits later in the same
topic. So it keeps the history marginally cleaner. That was my
thinking anyway. Otherwise you may as well just use what I sent last
night and let me add one more patch on-top to clean-up that unused
variable that gets passed to get_head_names().

Your call. :-)

j.