1. 28 Feb, 2019 1 commit
    • Martin Ågren's avatar
      setup: fix memory leaks with `struct repository_format` · e8805af1
      Martin Ågren authored
      After we set up a `struct repository_format`, it owns various pieces of
      allocated memory. We then either use those members, because we decide we
      want to use the "candidate" repository format, or we discard the
      candidate / scratch space. In the first case, we transfer ownership of
      the memory to a few global variables. In the latter case, we just
      silently drop the struct and end up leaking memory.
      Introduce an initialization macro `REPOSITORY_FORMAT_INIT` and a
      function `clear_repository_format()`, to be used on each side of
      `read_repository_format()`. To have a clear and simple memory ownership,
      let all users of `struct repository_format` duplicate the strings that
      they take from it, rather than stealing the pointers.
      Call `clear_...()` at the start of `read_...()` instead of just zeroing
      the struct, since we sometimes enter the function multiple times. Thus,
      it is important to initialize the struct before calling `read_...()`, so
      document that. It's also important because we might not even call
      `read_...()` before we call `clear_...()`, see, e.g., builtin/init-db.c.
      Teach `read_...()` to clear the struct on error, so that it is reset to
      a safe state, and document this. (In `setup_git_directory_gently()`, we
      look at `repo_fmt.hash_algo` even if `repo_fmt.version` is -1, which we
      weren't actually supposed to do per the API. After this commit, that's
      We inherit the existing code's combining "error" and "no version found".
      Both are signalled through `version == -1` and now both cause us to
      clear any partial configuration we have picked up. For "extensions.*",
      that's fine, since they require a positive version number. For
      "core.bare" and "core.worktree", we're already verifying that we have a
      non-negative version number before using them.
      Signed-off-by: 's avatarMartin Ågren <martin.agren@gmail.com>
      Signed-off-by: 's avatarJunio C Hamano <gitster@pobox.com>
  2. 31 Oct, 2018 1 commit
  3. 22 Oct, 2018 2 commits
    • Duy Nguyen's avatar
      revision.c: better error reporting on ref from different worktrees · ab3e1f78
      Duy Nguyen authored
      Make use of the new ref aliases to pass refs from another worktree
      around and access them from the current ref store instead. This does
      not change any functionality, but when a problem arises, we would like
      the reported messages to mention full ref aliases, like this:
          fatal: bad object worktrees/ztemp/HEAD
          warning: reflog of 'main-worktree/HEAD' references pruned commits
      instead of
          fatal: bad object HEAD
          warning: reflog of 'HEAD' references pruned commits
      which does not really tell where the refs are from.
      Signed-off-by: Duy Nguyen's avatarNguyễn Thái Ngọc Duy <pclouds@gmail.com>
      Signed-off-by: 's avatarJunio C Hamano <gitster@pobox.com>
    • 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
      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: 's avatarJunio C Hamano <gitster@pobox.com>
  4. 30 Aug, 2018 1 commit
    • Eric Sunshine's avatar
      worktree: don't die() in library function find_worktree() · 4c5fa9e6
      Eric Sunshine authored
      Callers don't expect library function find_worktree() to die(); they
      expect it to return the named worktree if found, or NULL if not.
      Although find_worktree() itself never invokes die(), it calls
      real_pathdup() with 'die_on_error' incorrectly set to 'true', thus will
      die() indirectly if the user-provided path is not to real_pathdup()'s
      liking. This can be observed, for instance, with any git-worktree
      command which searches for an existing worktree:
          $ git worktree unlock foo
          fatal: 'foo' is not a working tree
          $ git worktree unlock foo/bar
          fatal: Invalid path '.../foo': No such file or directory
      The first error message is the expected one from "git worktree unlock"
      not finding the specified worktree; the second is from find_worktree()
      invoking real_pathdup() incorrectly and die()ing prematurely.
      Aside from the inconsistent error message between the two cases, this
      bug hasn't otherwise been a serious problem since existing callers all
      die() anyhow when the worktree can't be found. However, that may not be
      true of callers added in the future, so fix find_worktree() to avoid
      Signed-off-by: Eric Sunshine's avatarEric Sunshine <sunshine@sunshineco.com>
      Signed-off-by: 's avatarJunio C Hamano <gitster@pobox.com>
  5. 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: 's avatarJunio C Hamano <gitster@pobox.com>
  6. 12 Feb, 2018 2 commits
  7. 24 Jan, 2018 1 commit
  8. 21 Oct, 2017 1 commit
    • Jeff King's avatar
      worktree: handle broken symrefs in find_shared_symref() · dbd2b55c
      Jeff King authored
      The refs_resolve_ref_unsafe() function may return NULL even
      with a REF_ISSYMREF flag if a symref points to a broken ref.
      As a result, it's possible for find_shared_symref() to
      segfault when it passes NULL to strcmp().
      This is hard to trigger for most code paths. We typically
      pass HEAD to the function as the symref to resolve, and
      programs like "git branch" will bail much earlier if HEAD
      isn't valid.
      I did manage to trigger it through one very obscure
        # You have multiple notes refs which conflict.
        git notes add -m base
        git notes --ref refs/notes/foo add -m foo
        # There's left-over cruft in NOTES_MERGE_REF that
        # makes it a broken symref (in this case we point
        # to a syntactically invalid ref).
        echo "ref: refs/heads/master.lock" >.git/NOTES_MERGE_REF
        # You try to merge the notes. We read the broken value in
        # order to complain that another notes-merge is
        # in-progress, but we segfault in find_shared_symref().
        git notes merge refs/notes/foo
      This is obviously silly and almost certainly impossible to
      trigger accidentally, but it does show that the bug is
      triggerable from at least one code path. In addition, it
      would trigger if we saw a transient filesystem error when
      resolving the pointed-to ref.
      We can fix this by treating NULL the same as a non-matching
      symref. Arguably we'd prefer to know if a symref points to
      "refs/heads/foo", but "refs/heads/foo" is broken. But
      refs_resolve_ref_unsafe() isn't capable of giving us that
      information, so this is the best we can do.
      Signed-off-by: 's avatarJeff King <peff@peff.net>
      Signed-off-by: 's avatarJunio C Hamano <gitster@pobox.com>
  9. 16 Oct, 2017 2 commits
  10. 24 Sep, 2017 1 commit
  11. 24 Aug, 2017 2 commits
    • Duy Nguyen's avatar
      revision.c: --all adds HEAD from all worktrees · d0c39a49
      Duy Nguyen authored
      Unless single_worktree is set, --all now adds HEAD from all worktrees.
      Since reachable.c code does not use setup_revisions(), we need to call
      other_head_refs_submodule() explicitly there to have the same effect on
      "git prune", so that we won't accidentally delete objects needed by some
      other HEADs.
      A new FIXME is added because we would need something like
          int refs_other_head_refs(struct ref_store *, each_ref_fn, cb_data);
      in addition to other_head_refs() to handle it, which might require
          int get_submodule_worktrees(const char *submodule, int flags);
      It could be a separate topic to reduce the scope of this one.
      Signed-off-by: Duy Nguyen's avatarNguyễn Thái Ngọc Duy <pclouds@gmail.com>
      Signed-off-by: 's avatarJunio C Hamano <gitster@pobox.com>
    • Duy Nguyen's avatar
      branch: fix branch renaming not updating HEADs correctly · 31824d18
      Duy Nguyen authored
      There are two bugs that sort of work together and cause
      problems. Let's start with one in replace_each_worktree_head_symref.
      Before fa099d23 (worktree.c: kill parse_ref() in favor of
      refs_resolve_ref_unsafe() - 2017-04-24), this code looks like this:
          if (strcmp(oldref, worktrees[i]->head_ref))
      After fa099d23, it is possible that head_ref can be NULL. However,
      the updated code takes the wrong exit. In the error case (NULL
      head_ref), we should "continue;" to the next worktree. The updated
      code makes us _skip_ "continue;" and update HEAD anyway.
      The NULL head_ref is triggered by the second bug in add_head_info (in
      the same commit). With the flag RESOLVE_REF_READING, resolve_ref_unsafe()
      will abort if it cannot resolve the target ref. For orphan checkouts,
      HEAD always points to an unborned branch, resolving target ref will
      always fail. Now we have NULL head_ref. Now we always update HEAD.
      Correct the logic in replace_ function so that we don't accidentally
      update HEAD on error. As it turns out, correcting the logic bug above
      breaks branch renaming completely, thanks to the second bug.
      "git branch -[Mm]" does two steps (on a normal checkout, no orphan!):
       - rename the branch on disk (e.g. refs/heads/abc to refs/heads/def)
       - update HEAD if it points to the branch being renamed.
      At the second step, since the branch pointed to by HEAD (e.g. "abc") no
      longer exists on disk, we run into a temporary orphan checkout situation
      that has been just corrected to _not_ update HEAD. But we need to update
      HEAD since it's not actually an orphan checkout. We need to update HEAD
      to move out of that orphan state.
      Correct add_head_info(), remove RESOLVE_REF_READING flag. With the flag
      gone, we should always return good "head_ref" in orphan checkouts (either
      temporary or permanent). With good head_ref, things start to work again.
      Noticed-by: 's avatarNish Aravamudan <nish.aravamudan@canonical.com>
      Signed-off-by: Duy Nguyen's avatarNguyễn Thái Ngọc Duy <pclouds@gmail.com>
      Signed-off-by: 's avatarJunio C Hamano <gitster@pobox.com>
  12. 24 Jun, 2017 1 commit
  13. 08 May, 2017 1 commit
  14. 25 Apr, 2017 1 commit
  15. 21 Mar, 2017 2 commits
    • Jeff King's avatar
      prefix_filename: return newly allocated string · e4da43b1
      Jeff King authored
      The prefix_filename() function returns a pointer to static
      storage, which makes it easy to use dangerously. We already
      fixed one buggy caller in hash-object recently, and the
      calls in apply.c are suspicious (I didn't dig in enough to
      confirm that there is a bug, but we call the function once
      in apply_all_patches() and then again indirectly from
      Let's make it harder to get wrong by allocating the return
      value. For simplicity, we'll do this even when the prefix is
      empty (and we could just return the original file pointer).
      That will cause us to allocate sometimes when we wouldn't
      otherwise need to, but this function isn't called in
      performance critical code-paths (and it already _might_
      allocate on any given call, so a caller that cares about
      performance is questionable anyway).
      The downside is that the callers need to remember to free()
      the result to avoid leaking. Most of them already used
      xstrdup() on the result, so we know they are OK. The
      remainder have been converted to use free() as appropriate.
      I considered retaining a prefix_filename_unsafe() for cases
      where we know the static lifetime is OK (and handling the
      cleanup is awkward). This is only a handful of cases,
      though, and it's not worth the mental energy in worrying
      about whether the "unsafe" variant is OK to use in any
      Signed-off-by: 's avatarJeff King <peff@peff.net>
      Signed-off-by: 's avatarJunio C Hamano <gitster@pobox.com>
    • Jeff King's avatar
      prefix_filename: drop length parameter · 116fb64e
      Jeff King authored
      This function takes the prefix as a ptr/len pair, but in
      every caller the length is exactly strlen(ptr). Let's
      simplify the interface and just take the string. This saves
      callers specifying it (and in some cases handling a NULL
      In a handful of cases we had the length already without
      calling strlen, so this is technically slower. But it's not
      likely to matter (after all, if the prefix is non-empty
      we'll allocate and copy it into a buffer anyway).
      Signed-off-by: 's avatarJeff King <peff@peff.net>
      Signed-off-by: 's avatarJunio C Hamano <gitster@pobox.com>
  16. 08 Mar, 2017 1 commit
    • Johannes Schindelin's avatar
      real_pathdup(): fix callsites that wanted it to die on error · ce83eadd
      Johannes Schindelin authored
      In 4ac9006f (real_path: have callers use real_pathdup and
      strbuf_realpath, 2016-12-12), we changed the xstrdup(real_path())
      pattern to use real_pathdup() directly.
      The problem with this change is that real_path() calls
      strbuf_realpath() with die_on_error = 1 while real_pathdup() calls
      it with die_on_error = 0. Meaning that in cases where real_path()
      causes Git to die() with an error message, real_pathdup() is silent
      and returns NULL instead.
      The callers, however, are ill-prepared for that change, as they expect
      the return value to be non-NULL (and otherwise the function died
      with an appropriate error message).
      Fix this by extending real_pathdup()'s signature to accept the
      die_on_error flag and simply pass it through to strbuf_realpath(),
      and then adjust all callers after a careful audit whether they would
      handle NULLs well.
      Signed-off-by: Johannes Schindelin's avatarJohannes Schindelin <johannes.schindelin@gmx.de>
      Signed-off-by: 's avatarJunio C Hamano <gitster@pobox.com>
  17. 27 Feb, 2017 1 commit
  18. 27 Jan, 2017 1 commit
  19. 27 Dec, 2016 1 commit
  20. 12 Dec, 2016 2 commits
    • Brandon Williams's avatar
      real_path: have callers use real_pathdup and strbuf_realpath · 4ac9006f
      Brandon Williams authored
      Migrate callers of real_path() who duplicate the retern value to use
      real_pathdup or strbuf_realpath.
      Signed-off-by: 's avatarBrandon Williams <bmwill@google.com>
      Signed-off-by: 's avatarJunio C Hamano <gitster@pobox.com>
    • Stefan Beller's avatar
      worktree: check if a submodule uses worktrees · 1a248cf2
      Stefan Beller authored
      In a later patch we want to move around the the git directory of
      a submodule. Both submodules as well as worktrees are involved in
      placing git directories at unusual places, so their functionality
      may collide. To react appropriately to situations where worktrees
      in submodules are in use, offer a new function to query the
      a submodule if it uses the worktree feature.
      An earlier approach:
        "Implement submodule_get_worktrees and just count them", however:
        This can be done cheaply (both in new code to write as well as run time)
        by obtaining the list of worktrees based off that submodules git
        directory. However as we have loaded the variables for the current
        repository, the values in the submodule worktree
        can be wrong, e.g.
        * core.ignorecase may differ between these two repositories
        * the ref resolution is broken (refs/heads/branch in the submodule
          resolves to the sha1 value of the `branch` in the current repository
          that may not exist or have another sha1)
      The implementation here is just checking for any files in
      $GIT_COMMON_DIR/worktrees for the submodule, which ought to be sufficient
      if the submodule is using the current repository format, which we also
      Signed-off-by: Stefan Beller's avatarStefan Beller <sbeller@google.com>
      Signed-off-by: 's avatarJunio C Hamano <gitster@pobox.com>
  21. 28 Nov, 2016 3 commits
    • Duy Nguyen's avatar
      worktree list: keep the list sorted · 4df1d4d4
      Duy Nguyen authored
      It makes it easier to write tests for. But it should also be good for
      the user since locating a worktree by eye would be easier once they
      notice this.
      Signed-off-by: Duy Nguyen's avatarNguyễn Thái Ngọc Duy <pclouds@gmail.com>
      Signed-off-by: 's avatarJunio C Hamano <gitster@pobox.com>
    • Duy Nguyen's avatar
      worktree.c: get_worktrees() takes a new flag argument · 4fff1ef7
      Duy Nguyen authored
      This is another no-op patch, in preparation for get_worktrees() to do
      optional things, like sorting.
      Signed-off-by: Duy Nguyen's avatarNguyễn Thái Ngọc Duy <pclouds@gmail.com>
      Signed-off-by: 's avatarJunio C Hamano <gitster@pobox.com>
    • Duy Nguyen's avatar
      get_worktrees() must return main worktree as first item even on error · a234563a
      Duy Nguyen authored
      This is required by git-worktree.txt, stating that the main worktree is
      the first line (especially in --porcelain mode when we can't just change
      behavior at will).
      There's only one case when get_worktrees() may skip main worktree, when
      parse_ref() fails. Update the code so that we keep first item as main
      worktree and return something sensible in this case:
       - In user-friendly mode, since we're not constraint by anything,
         returning "(error)" should do the job (we already show "(detached
         HEAD)" which is not machine-friendly). Actually errors should be
         printed on stderr by parse_ref() (*)
       - In plumbing mode, we do not show neither 'bare', 'detached' or
         'branch ...', which is possible by the format description if I read
         it right.
      Careful readers may realize that when the local variable "head_ref" in
      get_main_worktree() is emptied, add_head_info() will do nothing to
      wt->head_sha1. But that's ok because head_sha1 is zero-ized in the
      previous patch.
      (*) Well, it does not. But it's supposed to be a stop gap implementation
          until we can reuse refs code to parse "ref: " stuff in HEAD, from
          resolve_refs_unsafe(). Now may be the time since refs refactoring is
          mostly done.
      Signed-off-by: Duy Nguyen's avatarNguyễn Thái Ngọc Duy <pclouds@gmail.com>
      Signed-off-by: 's avatarJunio C Hamano <gitster@pobox.com>
  22. 23 Nov, 2016 1 commit
  23. 14 Oct, 2016 1 commit
  24. 12 Jul, 2016 1 commit
  25. 08 Jul, 2016 1 commit
  26. 13 Jun, 2016 1 commit
  27. 04 Jun, 2016 2 commits
  28. 24 May, 2016 2 commits
  29. 06 May, 2016 1 commit
    • Li Peng's avatar
      typofix: assorted typofixes in comments, documentation and messages · 832c0e5e
      Li Peng authored
      Many instances of duplicate words (e.g. "the the path") and
      a few typoes are fixed, originally in multiple patches.
          wildmatch: fix duplicate words of "the"
          t: fix duplicate words of "output"
          transport-helper: fix duplicate words of "read"
          Git.pm: fix duplicate words of "return"
          path: fix duplicate words of "look"
          pack-protocol.txt: fix duplicate words of "the"
          precompose-utf8: fix typo of "sequences"
          split-index: fix typo
          worktree.c: fix typo
          remote-ext: fix typo
          utf8: fix duplicate words of "the"
          git-cvsserver: fix duplicate words
      Signed-off-by: 's avatarLi Peng <lip@dtdream.com>
      Signed-off-by: 's avatarJunio C Hamano <gitster@pobox.com>
  30. 22 Apr, 2016 1 commit
    • Duy Nguyen's avatar
      branch: do not rename a branch under bisect or rebase · 14ace5b7
      Duy Nguyen authored
      The branch name in that case could be saved in rebase's head_name or
      bisect's BISECT_START files. Ideally we should try to update them as
      well. But it's trickier (*). Let's play safe and see if the user
      complains about inconveniences before doing that.
      (*) If we do it, bisect and rebase need to provide an API to rename
      branches. We can't do it in worktree.c or builtin/branch.c because
      when other people change rebase/bisect code, they may not be aware of
      this code and accidentally break it (e.g. rename the branch file, or
      refer to the branch in new files). It's a lot more work.
      Signed-off-by: Duy Nguyen's avatarNguyễn Thái Ngọc Duy <pclouds@gmail.com>
      Signed-off-by: 's avatarJunio C Hamano <gitster@pobox.com>