1. 26 Nov, 2018 1 commit
  2. 22 Oct, 2018 1 commit
    • Duy Nguyen's avatar
      refs: new ref types to make per-worktree refs visible to all worktrees · 3a3b9d8c
      Duy Nguyen authored
      One of the problems with multiple worktree is accessing per-worktree
      refs of one worktree from another worktree. This was sort of solved by
      multiple ref store, where the code can open the ref store of another
      worktree and has access to the ref space of that worktree.
      
      The problem with this is reporting. "HEAD" in another ref space is
      also called "HEAD" like in the current ref space. In order to
      differentiate them, all the code must somehow carry the ref store
      around and print something like "HEAD from this ref store".
      
      But that is not feasible (or possible with a _lot_ of work). With the
      current design, we pass a reference around as a string (so called
      "refname"). Extending this design to pass a string _and_ a ref store
      is a nightmare, especially when handling extended SHA-1 syntax.
      
      So we do it another way. Instead of entering a separate ref space, we
      make refs from other worktrees available in the current ref space. So
      "HEAD" is always HEAD of the current worktree, but then we can have
      "worktrees/blah/HEAD" to denote HEAD from a worktree named
      "blah". This syntax coincidentally matches the underlying directory
      structure which makes implementation a bit easier.
      
      The main worktree has to be treated specially because well... it's
      special from the beginning. So HEAD from the main worktree is
      acccessible via the name "main-worktree/HEAD" instead of
      "worktrees/main/HEAD" because "main" could be just another secondary
      worktree.
      
      This patch also makes it possible to specify refs from one worktree in
      another one, e.g.
      
          git log worktrees/foo/HEAD
      Signed-off-by: Duy Nguyen's avatarNguyễn Thái Ngọc Duy <pclouds@gmail.com>
      Signed-off-by: default avatarJunio C Hamano <gitster@pobox.com>
      3a3b9d8c
  3. 15 Oct, 2018 1 commit
  4. 06 Oct, 2018 1 commit
    • Duy Nguyen's avatar
      Add a place for (not) sharing stuff between worktrees · 8aff1a9c
      Duy Nguyen authored
      When multiple worktrees are used, we need rules to determine if
      something belongs to one worktree or all of them. Instead of keeping
      adding rules when new stuff comes (*), have a generic rule:
      
      - Inside $GIT_DIR, which is per-worktree by default, add
        $GIT_DIR/common which is always shared. New features that want to
        share stuff should put stuff under this directory.
      
      - Inside refs/, which is shared by default except refs/bisect, add
        refs/worktree/ which is per-worktree. We may eventually move
        refs/bisect to this new location and remove the exception in refs
        code.
      
      (*) And it may also include stuff from external commands which will
          have no way to modify common/per-worktree rules.
      Signed-off-by: Duy Nguyen's avatarNguyễn Thái Ngọc Duy <pclouds@gmail.com>
      Signed-off-by: default avatarJunio C Hamano <gitster@pobox.com>
      8aff1a9c
  5. 20 Sep, 2018 3 commits
  6. 17 Sep, 2018 1 commit
  7. 29 Aug, 2018 2 commits
    • Jeff King's avatar
      convert "oidcmp() != 0" to "!oideq()" · 9001dc2a
      Jeff King authored
      This is the flip side of the previous two patches: checking
      for a non-zero oidcmp() can be more strictly expressed as
      inequality. Like those patches, we write "!= 0" in the
      coccinelle transformation, which covers by isomorphism the
      more common:
      
        if (oidcmp(E1, E2))
      
      As with the previous two patches, this patch can be achieved
      almost entirely by running "make coccicheck"; the only
      differences are manual line-wrap fixes to match the original
      code.
      
      There is one thing to note for anybody replicating this,
      though: coccinelle 1.0.4 seems to miss the case in
      builtin/tag.c, even though it's basically the same as all
      the others. Running with 1.0.7 does catch this, so
      presumably it's just a coccinelle bug that was fixed in the
      interim.
      Signed-off-by: default avatarJeff King <peff@peff.net>
      Signed-off-by: default avatarJunio C Hamano <gitster@pobox.com>
      9001dc2a
    • Jeff King's avatar
      convert "oidcmp() == 0" to oideq() · 4a7e27e9
      Jeff King authored
      Using the more restrictive oideq() should, in the long run,
      give the compiler more opportunities to optimize these
      callsites. For now, this conversion should be a complete
      noop with respect to the generated code.
      
      The result is also perhaps a little more readable, as it
      avoids the "zero is equal" idiom. Since it's so prevalent in
      C, I think seasoned programmers tend not to even notice it
      anymore, but it can sometimes make for awkward double
      negations (e.g., we can drop a few !!oidcmp() instances
      here).
      
      This patch was generated almost entirely by the included
      coccinelle patch. This mechanical conversion should be
      completely safe, because we check explicitly for cases where
      oidcmp() is compared to 0, which is what oideq() is doing
      under the hood. Note that we don't have to catch "!oidcmp()"
      separately; coccinelle's standard isomorphisms make sure the
      two are treated equivalently.
      
      I say "almost" because I did hand-edit the coccinelle output
      to fix up a few style violations (it mostly keeps the
      original formatting, but sometimes unwraps long lines).
      Signed-off-by: default avatarJeff King <peff@peff.net>
      Signed-off-by: default avatarJunio C Hamano <gitster@pobox.com>
      4a7e27e9
  8. 21 Aug, 2018 1 commit
    • Stefan Beller's avatar
      refs.c: migrate internal ref iteration to pass thru repository argument · 4a6067cd
      Stefan Beller authored
      In 60ce76d3 (refs: add repository argument to for_each_replace_ref,
      2018-04-11) and 0d296c57 (refs: allow for_each_replace_ref to handle
      arbitrary repositories, 2018-04-11), for_each_replace_ref learned how
      to iterate over refs by a given arbitrary repository.
      New attempts in the object store conversion have shown that it is useful
      to have the repository handle available that the refs iteration is
      currently iterating over.
      
      To achieve this goal we will need to add a repository argument to
      each_ref_fn in refs.h. However as many callers rely on the signature
      such a patch would be too large.
      
      So convert the internals of the ref subsystem first to pass through a
      repository argument without exposing the change to the user. Assume
      the_repository for the passed through repository, although it is not
      used anywhere yet.
      Signed-off-by: Stefan Beller's avatarStefan Beller <sbeller@google.com>
      Signed-off-by: Derrick Stolee's avatarDerrick Stolee <dstolee@microsoft.com>
      Signed-off-by: default avatarJunio C Hamano <gitster@pobox.com>
      4a6067cd
  9. 24 Jul, 2018 1 commit
    • Jeff King's avatar
      pass st.st_size as hint for strbuf_readlink() · 765b496d
      Jeff King authored
      When we initially added the strbuf_readlink() function in
      b11b7e13 (Add generic 'strbuf_readlink()' helper function,
      2008-12-17), the point was that we generally have a _guess_
      as to the correct size based on the stat information, but we
      can't necessarily trust it.
      
      Over the years, a few callers have grown up that simply pass
      in 0, even though they have the stat information. Let's have
      them pass in their hint for consistency (and in theory
      efficiency, since it may avoid an extra resize/syscall loop,
      but neither location is probably performance critical).
      
      Note that st.st_size is actually an off_t, so in theory we
      need xsize_t() here. But none of the other callsites use it,
      and since this is just a hint, it doesn't matter either way
      (if we wrap we'll simply start with a too-small hint and
      then eventually complain when we cannot allocate the
      memory).
      Signed-off-by: default avatarJeff King <peff@peff.net>
      Signed-off-by: default avatarJunio C Hamano <gitster@pobox.com>
      765b496d
  10. 16 Jul, 2018 1 commit
  11. 10 Jul, 2018 1 commit
  12. 09 Jul, 2018 2 commits
  13. 29 Jun, 2018 1 commit
  14. 01 Jun, 2018 1 commit
  15. 10 May, 2018 1 commit
    • Martin Ågren's avatar
      lock_file: make function-local locks non-static · b2275868
      Martin Ågren authored
      Placing `struct lock_file`s on the stack used to be a bad idea, because
      the temp- and lockfile-machinery would keep a pointer into the struct.
      But after 076aa2cb (tempfile: auto-allocate tempfiles on heap,
      2017-09-05), we can safely have lockfiles on the stack. (This applies
      even if a user returns early, leaving a locked lock behind.)
      
      These `struct lock_file`s are local to their respective functions and we
      can drop their staticness.
      
      For good measure, I have inspected these sites and come to believe that
      they always release the lock, with the possible exception of bailing out
      using `die()` or `exit()` or by returning from a `cmd_foo()`.
      
      As pointed out by Jeff King, it would be bad if someone held on to a
      `struct lock_file *` for some reason. After some grepping, I agree with
      his findings: no-one appears to be doing that.
      Signed-off-by: default avatarMartin Ågren <martin.agren@gmail.com>
      Signed-off-by: default avatarJunio C Hamano <gitster@pobox.com>
      b2275868
  16. 06 May, 2018 1 commit
    • Johannes Schindelin's avatar
      Replace all die("BUG: ...") calls by BUG() ones · 033abf97
      Johannes Schindelin authored
      In d8193743 (usage.c: add BUG() function, 2017-05-12), a new macro
      was introduced to use for reporting bugs instead of die(). It was then
      subsequently used to convert one single caller in 588a538a
      (setup_git_env: convert die("BUG") to BUG(), 2017-05-12).
      
      The cover letter of the patch series containing this patch
      (cf 20170513032414.mfrwabt4hovujde2@sigill.intra.peff.net) is not
      terribly clear why only one call site was converted, or what the plan
      is for other, similar calls to die() to report bugs.
      
      Let's just convert all remaining ones in one fell swoop.
      
      This trick was performed by this invocation:
      
      	sed -i 's/die("BUG: /BUG("/g' $(git grep -l 'die("BUG' \*.c)
      Signed-off-by: Johannes Schindelin's avatarJohannes Schindelin <johannes.schindelin@gmx.de>
      Signed-off-by: default avatarJunio C Hamano <gitster@pobox.com>
      033abf97
  17. 12 Apr, 2018 1 commit
  18. 30 Mar, 2018 1 commit
  19. 24 Jan, 2018 6 commits
  20. 22 Jan, 2018 1 commit
  21. 19 Jan, 2018 1 commit
    • Mathias Rav's avatar
      files_initial_transaction_commit(): only unlock if locked · 81fcb698
      Mathias Rav authored
      Running git clone --single-branch --mirror -b TAGNAME previously
      triggered the following error message:
      
      	fatal: multiple updates for ref 'refs/tags/TAGNAME' not allowed.
      
      This error condition is handled in files_initial_transaction_commit().
      
      42c7f7ff ("commit_packed_refs(): remove call to `packed_refs_unlock()`", 2017-06-23)
      introduced incorrect unlocking in the error path of this function,
      which changes the error message to
      
      	fatal: BUG: packed_refs_unlock() called when not locked
      
      Move the call to packed_refs_unlock() above the "cleanup:" label
      since the unlocking should only be done in the last error path.
      Signed-off-by: default avatarMathias Rav <m@git.strova.dk>
      Signed-off-by: default avatarJunio C Hamano <gitster@pobox.com>
      81fcb698
  22. 06 Nov, 2017 8 commits
  23. 30 Oct, 2017 1 commit
    • Michael Haggerty's avatar
      files-backend: don't rewrite the `packed-refs` file unnecessarily · 7c6bd25c
      Michael Haggerty authored
      Even when we are deleting references, we needn't overwrite the
      `packed-refs` file if the references that we are deleting only exist
      as loose references. Implement this optimization as follows:
      
      * Add a function `is_packed_transaction_needed()`, which checks
        whether a given packed-refs transaction actually needs to be carried
        out (i.e., it returns false if the transaction obviously wouldn't
        have any effect). This function must be called while holding the
        `packed-refs` lock to avoid races.
      
      * Change `files_transaction_prepare()` to check whether the
        packed-refs transaction is actually needed. If not, squelch it, but
        continue holding the `packed-refs` lock until the end of the
        transaction to avoid races.
      
      This fixes a mild regression caused by dc39e099 (files_ref_store:
      use a transaction to update packed refs, 2017-09-08). Before that
      commit, unnecessary rewrites of `packed-refs` were suppressed by
      `repack_without_refs()`. But the transaction-based writing introduced
      by that commit didn't perform that optimization.
      
      Note that the pre-dc39e099 code still had to *read* the whole
      `packed-refs` file to determine that the rewrite could be skipped, so
      the performance for the cases that the write could be elided was
      `O(N)` in the number of packed references both before and after
      dc39e099. But after that commit the constant factor increased.
      
      This commit reimplements the optimization of eliding unnecessary
      `packed-refs` rewrites. That, plus the fact that since
      cfa2e29c34 (packed_ref_store: get rid of the `ref_cache` entirely,
      2017-03-17) we don't necessarily have to read the whole `packed-refs`
      file at all, means that deletes of one or a few loose references can
      now be done with `O(n lg N)` effort, where `n` is the number of loose
      references being deleted and `N` is the total number of packed
      references.
      
      This commit fixes two tests in t1409.
      Signed-off-by: default avatarMichael Haggerty <mhagger@alum.mit.edu>
      Signed-off-by: default avatarJunio C Hamano <gitster@pobox.com>
      7c6bd25c
  24. 25 Oct, 2017 1 commit
    • Michael Haggerty's avatar
      files_transaction_prepare(): fix handling of ref lock failure · da5267f1
      Michael Haggerty authored
      Since dc39e099 (files_ref_store: use a transaction to update packed
      refs, 2017-09-08), failure to lock a reference has been handled
      incorrectly by `files_transaction_prepare()`. If
      `lock_ref_for_update()` fails in the lock-acquisition loop of that
      function, it sets `ret` then breaks out of that loop. Prior to
      dc39e099, that was OK, because the only thing following the loop was
      the cleanup code. But dc39e099 added another blurb of code between
      the loop and the cleanup. That blurb sometimes resets `ret` to zero,
      making the cleanup code think that the locking was successful.
      
      Specifically, whenever
      
      * One or more reference deletions have been processed successfully in
        the lock-acquisition loop. (Processing the first such reference
        causes a packed-ref transaction to be initialized.)
      
      * Then `lock_ref_for_update()` fails for a subsequent reference. Such
        a failure can happen for a number of reasons, such as the old SHA-1
        not being correct, lock contention, etc. This causes a `break` out
        of the lock-acquisition loop.
      
      * The `packed-refs` lock is acquired successfully and
        `ref_transaction_prepare()` succeeds for the packed-ref transaction.
        This has the effect of resetting `ret` back to 0, and making the
        cleanup code think that lock acquisition was successful.
      
      In that case, any reference updates that were processed prior to
      breaking out of the loop would be carried out (loose and packed), but
      the reference that couldn't be locked and any subsequent references
      would silently be ignored.
      
      This can easily cause data loss if, for example, the user was trying
      to push a new name for an existing branch while deleting the old name.
      After the push, the branch could be left unreachable, and could even
      subsequently be garbage-collected.
      
      This problem was noticed in the context of deleting one reference and
      creating another in a single transaction, when the two references D/F
      conflict with each other, like
      
          git update-ref --stdin <<EOF
          delete refs/foo
          create refs/foo/bar HEAD
          EOF
      
      This triggers the above bug because the deletion is processed
      successfully for `refs/foo`, then the D/F conflict causes
      `lock_ref_for_update()` to fail when `refs/foo/bar` is processed. In
      this case the transaction *should* fail, but instead it causes
      `refs/foo` to be deleted without creating `refs/foo`. This could
      easily result in data loss.
      
      The fix is simple: instead of just breaking out of the loop, jump
      directly to the cleanup code. This fixes some tests in t1404 that were
      added in the previous commit.
      Signed-off-by: default avatarMichael Haggerty <mhagger@alum.mit.edu>
      Signed-off-by: default avatarJunio C Hamano <gitster@pobox.com>
      da5267f1