1. 12 Apr, 2018 4 commits
  2. 14 Mar, 2018 1 commit
    • brian m. carlson's avatar
      sha1_file: convert sha1_object_info* to object_id · abef9020
      brian m. carlson authored
      Convert sha1_object_info and sha1_object_info_extended to take pointers
      to struct object_id and rename them to use "oid" instead of "sha1" in
      their names.  Update the declaration and definition and apply the
      following semantic patch, plus the standard object_id transforms:
      
      @@
      expression E1, E2;
      @@
      - sha1_object_info(E1.hash, E2)
      + oid_object_info(&E1, E2)
      
      @@
      expression E1, E2;
      @@
      - sha1_object_info(E1->hash, E2)
      + oid_object_info(E1, E2)
      
      @@
      expression E1, E2, E3;
      @@
      - sha1_object_info_extended(E1.hash, E2, E3)
      + oid_object_info_extended(&E1, E2, E3)
      
      @@
      expression E1, E2, E3;
      @@
      - sha1_object_info_extended(E1->hash, E2, E3)
      + oid_object_info_extended(E1, E2, E3)
      Signed-off-by: default avatarJunio C Hamano <gitster@pobox.com>
      abef9020
  3. 22 Nov, 2017 1 commit
    • Rafael Ascensão's avatar
      log: add option to choose which refs to decorate · 65516f58
      Rafael Ascensão authored
      When `log --decorate` is used, git will decorate commits with all
      available refs. While in most cases this may give the desired effect,
      under some conditions it can lead to excessively verbose output.
      
      Introduce two command line options, `--decorate-refs=<pattern>` and
      `--decorate-refs-exclude=<pattern>` to allow the user to select which
      refs are used in decoration.
      
      When "--decorate-refs=<pattern>" is given, only the refs that match the
      pattern are used in decoration. The refs that match the pattern when
      "--decorate-refs-exclude=<pattern>" is given, are never used in
      decoration.
      
      These options follow the same convention for mixing negative and
      positive patterns across the system, assuming that the inclusive default
      is to match all refs available.
      
       (1) if there is no positive pattern given, pretend as if an
           inclusive default positive pattern was given;
      
       (2) for each candidate, reject it if it matches no positive
           pattern, or if it matches any one of the negative patterns.
      
      The rules for what is considered a match are slightly different from the
      rules used elsewhere.
      
      Commands like `log --glob` assume a trailing '/*' when glob chars are
      not present in the pattern. This makes it difficult to specify a single
      ref.  On the other hand, commands like `describe --match --all` allow
      specifying exact refs, but do not have the convenience of allowing
      "shorthand refs" like 'refs/heads' or 'heads' to refer to
      'refs/heads/*'.
      
      The commands introduced in this patch consider a match if:
      
        (a) the pattern contains globs chars,
      	and regular pattern matching returns a match.
      
        (b) the pattern does not contain glob chars,
               and ref '<pattern>' exists, or if ref exists under '<pattern>/'
      
      This allows both behaviours (allowing single refs and shorthand refs)
      yet remaining compatible with existent commands.
      Helped-by: default avatarKevin Daudt <me@ikke.info>
      Helped-by: default avatarJunio C Hamano <gitster@pobox.com>
      Signed-off-by: default avatarRafael Ascensão <rafa.almas@gmail.com>
      Signed-off-by: default avatarJunio C Hamano <gitster@pobox.com>
      65516f58
  4. 06 Nov, 2017 3 commits
  5. 16 Oct, 2017 15 commits
  6. 07 Oct, 2017 1 commit
    • Jeff King's avatar
      refs_resolve_ref_unsafe: handle d/f conflicts for writes · a1c1d817
      Jeff King authored
      If our call to refs_read_raw_ref() fails, we check errno to
      see if the ref is simply missing, or if we encountered a
      more serious error. If it's just missing, then in "write"
      mode (i.e., when RESOLVE_REFS_READING is not set), this is
      perfectly fine.
      
      However, checking for ENOENT isn't sufficient to catch all
      missing-ref cases. In the filesystem backend, we may also
      see EISDIR when we try to resolve "a" and "a/b" exists.
      Likewise, we may see ENOTDIR if we try to resolve "a/b" and
      "a" exists. In both of those cases, we know that our
      resolved ref doesn't exist, but we return an error (rather
      than reporting the refname and returning a null sha1).
      
      This has been broken for a long time, but nobody really
      noticed because the next step after resolving without the
      READING flag is usually to lock the ref and write it. But in
      both of those cases, the write will fail with the same
      errno due to the directory/file conflict.
      
      There are two cases where we can notice this, though:
      
        1. If we try to write "a" and there's a leftover directory
           already at "a", even though there is no ref "a/b". The
           actual write is smart enough to move the empty "a" out
           of the way.
      
           This is reasonably rare, if only because the writing
           code has to do an independent resolution before trying
           its write (because the actual update_ref() code handles
           this case fine). The notes-merge code does this, and
           before the fix in the prior commit t3308 erroneously
           expected this case to fail.
      
        2. When resolving symbolic refs, we typically do not use
           the READING flag because we want to resolve even
           symrefs that point to unborn refs. Even if those unborn
           refs could not actually be written because of d/f
           conflicts with existing refs.
      
           You can see this by asking "git symbolic-ref" to report
           the target of a symref pointing past a d/f conflict.
      
      We can fix the problem by recognizing the other "missing"
      errnos and treating them like ENOENT. This should be safe to
      do even for callers who are then going to actually write the
      ref, because the actual writing process will fail if the d/f
      conflict is a real one (and t1404 checks these cases).
      
      Arguably this should be the responsibility of the
      files-backend to normalize all "missing ref" errors into
      ENOENT (since something like EISDIR may not be meaningful at
      all to a database backend). However other callers of
      refs_read_raw_ref() may actually care about the distinction;
      putting this into resolve_ref() is the minimal fix for now.
      
      The new tests in t1401 use git-symbolic-ref, which is the
      most direct way to check the resolution by itself.
      Interestingly we actually had a test that setup this case
      already, but we only used it to verify that the funny state
      could be overwritten, not that it could be resolved.
      
      We also add a new test in t3200, as "branch -m" was the
      original motivation for looking into this. What happens is
      this:
      
        0. HEAD is pointing to branch "a"
      
        1. The user asks to rename "a" to "a/b".
      
        2. We create "a/b" and delete "a".
      
        3. We then try to update any worktree HEADs that point to
           the renamed ref (including the main repo HEAD). To do
           that, we have to resolve each HEAD. But now our HEAD is
           pointing at "a", and we get EISDIR due to the loose
           "a/b". As a result, we think there is no HEAD, and we
           do not update it. It now points to the bogus "a".
      
      Interestingly this case used to work, but only accidentally.
      Before 31824d18 (branch: fix branch renaming not updating
      HEADs correctly, 2017-08-24), we'd update any HEAD which we
      couldn't resolve. That was wrong, but it papered over the
      fact that we were incorrectly failing to resolve HEAD.
      
      So while the bug demonstrated by the git-symbolic-ref is
      quite old, the regression to "branch -m" is recent.
      Signed-off-by: default avatarJeff King <peff@peff.net>
      Signed-off-by: default avatarJunio C Hamano <gitster@pobox.com>
      a1c1d817
  7. 25 Sep, 2017 1 commit
    • Michael Haggerty's avatar
      ref_store: implement `refs_peel_ref()` generically · ba1c052f
      Michael Haggerty authored
      We're about to stop storing packed refs in a `ref_cache`. That means
      that the only way we have left to optimize `peel_ref()` is by checking
      whether the reference being peeled is the one currently being iterated
      over (in `current_ref_iter`), and if so, using `ref_iterator_peel()`.
      But this can be done generically; it doesn't have to be implemented
      per-backend.
      
      So implement `refs_peel_ref()` in `refs.c` and remove the `peel_ref()`
      method from the refs API.
      
      This removes the last callers of a couple of functions, so delete
      them. More cleanup to come...
      Signed-off-by: default avatarMichael Haggerty <mhagger@alum.mit.edu>
      Signed-off-by: default avatarJunio C Hamano <gitster@pobox.com>
      ba1c052f
  8. 24 Sep, 2017 2 commits
  9. 14 Sep, 2017 4 commits
    • Michael Haggerty's avatar
      ref_iterator: keep track of whether the iterator output is ordered · 8738a8a4
      Michael Haggerty authored
      References are iterated over in order by refname, but reflogs are not.
      Some consumers of reference iteration care about the difference. Teach
      each `ref_iterator` to keep track of whether its output is ordered.
      
      `overlay_ref_iterator` is one of the picky consumers. Add a sanity
      check in `overlay_ref_iterator_begin()` to verify that its inputs are
      ordered.
      Signed-off-by: default avatarMichael Haggerty <mhagger@alum.mit.edu>
      Signed-off-by: default avatarJunio C Hamano <gitster@pobox.com>
      8738a8a4
    • Jeff King's avatar
      avoid "write_in_full(fd, buf, len) != len" pattern · 06f46f23
      Jeff King authored
      The return value of write_in_full() is either "-1", or the
      requested number of bytes[1]. If we make a partial write
      before seeing an error, we still return -1, not a partial
      value. This goes back to f6aa66cb (write_in_full: really
      write in full or return error on disk full., 2007-01-11).
      
      So checking anything except "was the return value negative"
      is pointless. And there are a couple of reasons not to do
      so:
      
        1. It can do a funny signed/unsigned comparison. If your
           "len" is signed (e.g., a size_t) then the compiler will
           promote the "-1" to its unsigned variant.
      
           This works out for "!= len" (unless you really were
           trying to write the maximum size_t bytes), but is a
           bug if you check "< len" (an example of which was fixed
           recently in config.c).
      
           We should avoid promoting the mental model that you
           need to check the length at all, so that new sites are
           not tempted to copy us.
      
        2. Checking for a negative value is shorter to type,
           especially when the length is an expression.
      
        3. Linus says so. In d34cf19b (Clean up write_in_full()
           users, 2007-01-11), right after the write_in_full()
           semantics were changed, he wrote:
      
             I really wish every "write_in_full()" user would just
             check against "<0" now, but this fixes the nasty and
             stupid ones.
      
           Appeals to authority aside, this makes it clear that
           writing it this way does not have an intentional
           benefit. It's a historical curiosity that we never
           bothered to clean up (and which was undoubtedly
           cargo-culted into new sites).
      
      So let's convert these obviously-correct cases (this
      includes write_str_in_full(), which is just a wrapper for
      write_in_full()).
      
      [1] A careful reader may notice there is one way that
          write_in_full() can return a different value. If we ask
          write() to write N bytes and get a return value that is
          _larger_ than N, we could return a larger total. But
          besides the fact that this would imply a totally broken
          version of write(), it would already invoke undefined
          behavior. Our internal remaining counter is an unsigned
          size_t, which means that subtracting too many byte will
          wrap it around to a very large number. So we'll instantly
          begin reading off the end of the buffer, trying to write
          gigabytes (or petabytes) of data.
      Signed-off-by: default avatarJeff King <peff@peff.net>
      Reviewed-by: default avatarJonathan Nieder <jrnieder@gmail.com>
      Signed-off-by: default avatarJunio C Hamano <gitster@pobox.com>
      06f46f23
    • Stefan Beller's avatar
      replace-objects: evaluate replacement refs without using the object store · 006f3f28
      Stefan Beller authored
      Pass DO_FOR_EACH_INCLUDE_BROKEN when iterating over replacement refs
      so that the iteration does not require opening the named objects from
      the object store. This avoids a dependency cycle between object access
      and replace ref iteration.
      
      Moreover the ref subsystem has not been migrated yet to access the
      object store via passed in repository objects.  As a result, without
      this patch, iterating over replace refs in a repository other than
      the_repository it produces errors:
      
         error: refs/replace/3afabef75c627b894cccc3bcae86837abc7c32fe does not point to a valid object!
      
      Noticed while adapting the object store (and in particular its
      evaluation of replace refs) to handle arbitrary repositories.
      Signed-off-by: Stefan Beller's avatarStefan Beller <sbeller@google.com>
      Signed-off-by: default avatarJonathan Nieder <jrnieder@gmail.com>
      Signed-off-by: default avatarJunio C Hamano <gitster@pobox.com>
      006f3f28
    • Thomas Gummerer's avatar
      refs: strip out not allowed flags from ref_transaction_update · c788c54c
      Thomas Gummerer authored
      Callers are only allowed to pass certain flags into
      ref_transaction_update, other flags are internal to it.  To prevent
      mistakes from the callers, strip the internal only flags out before
      continuing.
      
      This was noticed because of a compiler warning gcc 7.1.1 issued about
      passing a NULL parameter as second parameter to memcpy (through
      hashcpy):
      
      In file included from refs.c:5:0:
      refs.c: In function ‘ref_transaction_verify’:
      cache.h:948:2: error: argument 2 null where non-null expected [-Werror=nonnull]
        memcpy(sha_dst, sha_src, GIT_SHA1_RAWSZ);
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      In file included from git-compat-util.h:165:0,
                       from cache.h:4,
                       from refs.c:5:
      /usr/include/string.h:43:14: note: in a call to function ‘memcpy’ declared here
       extern void *memcpy (void *__restrict __dest, const void *__restrict __src,
                    ^~~~~~
      
      The call to hascpy in ref_transaction_add_update is protected by the
      passed in flags, but as we only add flags there, gcc notices
      REF_HAVE_NEW or REF_HAVE_OLD flags could be passed in from the outside,
      which would potentially result in passing in NULL as second parameter to
      memcpy.
      
      Fix both the compiler warning, and make the interface safer for its
      users by stripping the internal flags out.
      Suggested-by: default avatarMichael Haggerty <mhagger@alum.mit.edu>
      Signed-off-by: default avatarThomas Gummerer <t.gummerer@gmail.com>
      Signed-off-by: default avatarJunio C Hamano <gitster@pobox.com>
      c788c54c
  10. 06 Sep, 2017 1 commit
  11. 24 Aug, 2017 7 commits