1. 07 Oct, 2018 1 commit
    • Jonathan Tan's avatar
      transport: list refs before fetch if necessary · 6ab40557
      Jonathan Tan authored
      The built-in bundle transport and the transport helper interface do not
      work when transport_fetch_refs() is called immediately after transport
      creation. This will be needed in a subsequent patch, so fix this.
      
      Evidence: fetch_refs_from_bundle() relies on data->header being
      initialized in get_refs_from_bundle(), and fetch() in transport-helper.c
      relies on either data->fetch or data->import being set by get_helper(),
      but neither transport_helper_init() nor fetch() calls get_helper().
      
      Up until the introduction of the partial clone feature, this has not
      been a problem, because transport_fetch_refs() is always called after
      transport_get_remote_refs(). With the introduction of the partial clone
      feature, which involves calling transport_fetch_refs() (to fetch objects
      by their OIDs) without transport_get_remote_refs(), this is still not a
      problem, but only coincidentally - we do not support partially cloning a
      bundle, and as for cloning using a transport-helper-using protocol, it
      so happens that before transport_fetch_refs() is called, fetch_refs() in
      fetch-object.c calls transport_set_option(), which means that the
      aforementioned get_helper() is invoked through set_helper_option() in
      transport-helper.c.
      
      This could be fixed by fixing the transports themselves, but it doesn't
      seem like a good idea to me to open up previously untested code paths;
      also, there may be transport helpers in the wild that assume that "list"
      is always called before "fetch". Instead, fix this by having
      transport_fetch_refs() call transport_get_remote_refs() to ensure that
      the latter is always called at least once, unless the transport
      explicitly states that it supports fetching without listing refs.
      Signed-off-by: default avatarJonathan Tan <jonathantanmy@google.com>
      Signed-off-by: default avatarJunio C Hamano <gitster@pobox.com>
      6ab40557
  2. 01 Aug, 2018 1 commit
    • Jonathan Tan's avatar
      fetch-pack: unify ref in and out param · e2842b39
      Jonathan Tan authored
      When a user fetches:
       - at least one up-to-date ref and at least one non-up-to-date ref,
       - using HTTP with protocol v0 (or something else that uses the fetch
         command of a remote helper)
      some refs might not be updated after the fetch.
      
      This bug was introduced in commit 989b8c44 ("fetch-pack: put shallow
      info in output parameter", 2018-06-28) which allowed transports to
      report the refs that they have fetched in a new out-parameter
      "fetched_refs". If they do so, transport_fetch_refs() makes this
      information available to its caller.
      
      Users of "fetched_refs" rely on the following 3 properties:
       (1) it is the complete list of refs that was passed to
           transport_fetch_refs(),
       (2) it has shallow information (REF_STATUS_REJECT_SHALLOW set if
           relevant), and
       (3) it has updated OIDs if ref-in-want was used (introduced after
           989b8c44).
      
      In an effort to satisfy (1), whenever transport_fetch_refs()
      filters the refs sent to the transport, it re-adds the filtered refs to
      whatever the transport supplies before returning it to the user.
      However, the implementation in 989b8c44 unconditionally re-adds the
      filtered refs without checking if the transport refrained from reporting
      anything in "fetched_refs" (which it is allowed to do), resulting in an
      incomplete list, no longer satisfying (1).
      
      An earlier effort to resolve this [1] solved the issue by readding the
      filtered refs only if the transport did not refrain from reporting in
      "fetched_refs", but after further discussion, it seems that the better
      solution is to revert the API change that introduced "fetched_refs".
      This API change was first suggested as part of a ref-in-want
      implementation that allowed for ref patterns and, thus, there could be
      drastic differences between the input refs and the refs actually fetched
      [2]; we eventually decided to only allow exact ref names, but this API
      change remained even though its necessity was decreased.
      
      Therefore, revert this API change by reverting commit 989b8c44, and
      make receive_wanted_refs() update the OIDs in the sought array (like how
      update_shallow() updates shallow information in the sought array)
      instead. A test is also included to show that the user-visible bug
      discussed at the beginning of this commit message no longer exists.
      
      [1] https://public-inbox.org/git/20180801171806.GA122458@google.com/
      [2] https://public-inbox.org/git/86a128c5fb710a41791e7183207c4d64889f9307.1485381677.git.jonathantanmy@google.com/Signed-off-by: default avatarJonathan Tan <jonathantanmy@google.com>
      Signed-off-by: default avatarJunio C Hamano <gitster@pobox.com>
      e2842b39
  3. 28 Jun, 2018 1 commit
    • Brandon Williams's avatar
      fetch-pack: put shallow info in output parameter · 989b8c44
      Brandon Williams authored
      Expand the transport fetch method signature, by adding an output
      parameter, to allow transports to return information about the refs they
      have fetched.  Then communicate shallow status information through this
      mechanism instead of by modifying the input list of refs.
      
      This does require clients to sometimes generate the ref map twice: once
      from the list of refs provided by the remote (as is currently done) and
      potentially once from the new list of refs that the fetch mechanism
      provides.
      Signed-off-by: default avatarBrandon Williams <bmwill@google.com>
      Signed-off-by: default avatarJunio C Hamano <gitster@pobox.com>
      989b8c44
  4. 15 Mar, 2018 1 commit
  5. 14 Dec, 2017 1 commit
    • Jonathan Tan's avatar
      transport: make transport vtable more private · e967ca38
      Jonathan Tan authored
      Move the definition of the transport-specific functions provided by
      transports, whether declared in transport.c or transport-helper.c, into
      an internal header. This means that transport-using code (as opposed to
      transport-declaring code) can no longer access these functions (without
      importing the internal header themselves), making it clear that they
      should use the transport_*() functions instead, and also allowing the
      interface between the transport mechanism and an individual transport to
      independently evolve.
      
      This is superficially a reversal of commit 824d5776 ("Refactor
      struct transport_ops inlined into struct transport", 2007-09-19).
      However, the scope of the involved variables was neither affected nor
      discussed in that commit, and I think that the advantages in making
      those functions more private outweigh the advantages described in that
      commit's commit message. A minor additional point is that the code has
      gotten more complicated since then, in that the function-pointer
      variables are potentially mutated twice (once initially and once if
      transport_take_over() is invoked), increasing the value of corralling
      them into their own struct.
      Signed-off-by: default avatarJonathan Tan <jonathantanmy@google.com>
      Signed-off-by: default avatarJunio C Hamano <gitster@pobox.com>
      e967ca38