1. 06 Oct, 2017 1 commit
    • refs: do not use peeled OID if peeling to a tag · b112b1e9
      If a reference stored in a packed-refs file does not directly point to a
      commit, tree or blob, the packed-refs file will also will include a
      fully-peeled OID pointing to the first underlying object of that type.
      If we try to peel a reference to an object, we will use that peeled OID
      to speed up resolving the object.
      
      As a reference for an annotated tag does not directly point to a commit,
      tree or blob but instead to the tag object, the packed-refs file will
      have an accomodating fully-peeled OID pointing to the object referenced
      by that tag. When we use the fully-peeled OID pointing to the referenced
      object when peeling, we obviously cannot peel that to the tag anymore.
      
      Fix this issue by not using the fully-peeled OID whenever we want to
      peel to a tag. Note that this does not include the case where we want to
      resolve to _any_ object type. Existing code may make use from the fact
      that we resolve those to commit objects instead of tag objects, even
      though that behaviour is inconsistent between packed and loose
      references. Furthermore, some tests of ours make the assumption that we
      in fact resolve those references to a commit.
      Patrick Steinhardt committed
  2. 04 Oct, 2017 1 commit
  3. 20 Sep, 2017 3 commits
    • cmake: fix static linking for bundled deps · 8c19969a
      Our bundled deps are being built as simple static libraries which are
      then linked into the libgit2 library via `TARGET_LINK_LIBRARIES`. While
      this works for a dynamically built libgit2 library, using this function
      to link two static libraries does not have the expected outcome of
      merging those static libraries into one big library. This leads to
      symbols of our bundled deps being undefined in the resulting libgit2
      archive.
      
      As we have bumped our minimum CMake version to 2.8.11, we can now easily
      make use of object libraries for our bundled dependencies. So build
      instructions are still self-contained inside of the dependency
      directories and the resulting object libraries can just be added to the
      LIBGIT2_OBJECTS list, which will cause them to be linked into the final
      resulting static library. This fixes the issue of undefined symbols.
      Patrick Steinhardt committed
    • cmake: always use object library for git2internal · 172a585f
      As we have bumped our minimum CMake version to 2.8.11, we can now
      unconditionally make use of object libraries. So remove the version
      check for the git2internal object library and always use it.
      Patrick Steinhardt committed
    • cmake: distinguish libgit2 objects and sources · 1d9dd882
      Distinguish variables keeping track of our internal libgit2 sources and
      the final objects which shall be linked into the library. This will ease
      the transition to use object libraries for our bundled dependencies
      instead of linking them in.
      Patrick Steinhardt committed
  4. 15 Sep, 2017 1 commit
    • diff: cleanup hash ctx in `git_diff_patchid` · 046b081a
      After initializing the hash context in `git_diff_patchid`, we never
      proceed to call `git_hash_ctx_cleanup` on it. While this doesn't really
      matter on most hash implementations, this causes a memory leak on Win32
      due to CNG system requiring a `malloc` call.
      
      Fix the memory leak by always calling `git_hash_ctx_cleanup` before
      exiting.
      Patrick Steinhardt committed
  5. 12 Sep, 2017 1 commit
    • features.h: allow building without CMake-generated feature header · 26f531d3
      In commit a390a846 (cmake: move defines into "features.h" header,
      2017-07-01), we have introduced a new "features.h" header. This file is
      being generated by the CMake build system based on how the libgit2 build
      has been configured, replacing the preexisting method of simply setting
      the defines inside of the CMake build system. This was done to help
      splitting up the build instructions into multiple separate
      subdirectories.
      
      An overlooked shortcoming of this approach is that some projects making
      use of libgit2 build the library with custom build systems, without
      making use of CMake. For those users, the introduction of the
      "features.h" file makes their life harder as they would have to also
      generate this file.
      
      Fix this issue by guarding all inclusions of the generated header file
      by the `LIBGIT2_NO_FEATURES_H` define. Like this, other build systems
      can skip the feature header and simply define all used features by
      specifying `-D` flags for the compiler again.
      Patrick Steinhardt committed
  6. 11 Sep, 2017 1 commit
  7. 25 Aug, 2017 5 commits
    • submodule: refuse lookup in bare repositories · 477b3e04
      While it is technically possible to look up submodules inside of a
      bare repository by reading the submodule configuration of a specific
      commit, we do not offer this functionality right now. As such, calling
      both `git_submodule_lookup` and `git_submodule_foreach` should error out
      early when these functions encounter a bare repository. While
      `git_submodule_lookup` already does return an error due to not being
      able to parse the configuration, `git_submodule_foreach` simply returns
      success and never invokes the callback function.
      
      Fix the issue by having both functions check whether the repository is
      bare and returning an error in that case.
      Patrick Steinhardt committed
    • ignore: honor case insensitivity for negative ignores · 2d9ff8f5
      When computing negative ignores, we throw away any rule which does not
      undo a previous rule to optimize. But on case insensitive file systems,
      we need to keep in mind that a negative ignore can also undo a previous
      rule with different case, which we did not yet honor while determining
      whether a rule undoes a previous one. So in the following example, we
      fail to unignore the "/Case" directory:
      
          /case
          !/Case
      
      Make both paths checking whether a plain- or wildcard-based rule undo a
      previous rule aware of case-insensitivity. This fixes the described
      issue.
      Patrick Steinhardt committed
    • ignore: keep negative rules containing wildcards · b8922fc8
      Ignore rules allow for reverting a previously ignored rule by prefixing
      it with an exclamation mark. As such, a negative rule can only override
      previously ignored files. While computing all ignore patterns, we try to
      use this fact to optimize away some negative rules which do not override
      any previous patterns, as they won't change the outcome anyway.
      
      In some cases, though, this optimization causes us to get the actual
      ignores wrong for some files. This may happen whenever the pattern
      contains a wildcard, as we are unable to reason about whether a pattern
      overrides a previous pattern in a sane way. This happens for example in
      the case where a gitignore file contains "*.c" and "!src/*.c", where we
      wouldn't un-ignore files inside of the "src/" subdirectory.
      
      In this case, the first solution coming to mind may be to just strip the
      "src/" prefix and simply compare the basenames. While that would work
      here, it would stop working as soon as the basename pattern itself is
      different, like for example with "*x.c" and "!src/*.c. As such, we
      settle for the easier fix of just not optimizing away rules that contain
      a wildcard.
      Patrick Steinhardt committed
  8. 17 Aug, 2017 1 commit
    • cmake: fix output location of import libraries and DLLs · a3a35473
      As observed by Edward Thomson, the libgit2 DLL built by Windows will not
      end up in the top-level build directory but instead inside of the 'src/'
      subdirectory. While confusing at first because we are actually setting
      the LIBRARY_OUTPUT_DIRECTORY to the project's binary directory, the
      manual page of LIBRARY_OUTPUT_DIRECTORY clears this up:
      
          There are three kinds of target files that may be built: archive,
          library, and runtime. Executables are always treated as runtime
          targets. Static libraries are always treated as archive targets.
          Module libraries are always treated as library targets. For non-DLL
          platforms shared libraries are treated as library targets. For DLL
          platforms the DLL part of a shared library is treated as a runtime
          target and the corresponding import library is treated as an archive
          target. All Windows-based systems including Cygwin are DLL
          platforms.
      
      So in fact, DLLs and import libraries are not treated as libraries at
      all by CMake but instead as runtime and archive targets. To fix the
      issue, we can thus simply set the variables RUNTIME_OUTPUT_DIRECTORY and
      ARCHIVE_OUTPUT_DIRECTORY to the project's root binary directory.
      Patrick Steinhardt committed
  9. 16 Aug, 2017 4 commits
    • cmake: always include our own headers first · 8a43161b
      With c26ce784 (Merge branch 'AndreyG/cmake/modernization', 2017-06-28),
      we have recently introduced a regression in the way we are searching for
      headers. We have made sure to always include our own headers first, but
      due to the changes in c26ce784  this is no longer guaranteed. In fact,
      this already leads the compiler into picking "config.h" from the
      "deps/regex" dependency, if it is used.
      
      Fix the issue by declaring our internal include directories up front,
      before any of the other search directories is added.
      Patrick Steinhardt committed
    • cmake: move library build instructions into subdirectory · e5c9723d
      To fix leaking build instructions into different targets and to make
      the build instructions easier to handle, create a new CMakeLists.txt
      file containing build instructions for the libgit2 target.
      
      By now, the split is rather easy to achieve. Due to the preparatory
      steps, we can now simply move over all related build instructions, only
      needing to remove the "src/" prefix from some files.
      Patrick Steinhardt committed
    • cmake: move regcomp and futimens checks to "features.h" · 8341d6cf
      In our CMakeLists.txt, we have to check multiple functions in order to
      determine if we have to use our own or whether we can use the
      platform-provided one. For two of these functions, namely `regcomp_l()`
      and `futimens`, the defined macro is actually used inside of the header
      file "src/unix/posix.h". As such, these macros are not only required by
      the library, but also by our test suite, which is makes use of internal
      headers.
      
      To prepare for the CMakeLists.txt split, move these two defines inside
      of the "features.h" header.
      Patrick Steinhardt committed
    • cmake: move defines into "features.h" header · a390a846
      In a future commit, we will split out the build instructions for our
      library directory and move them into a subdirectory. One of the benefits
      is fixing scoping issues, where e.g. defines do not leak to build
      targets where they do not belong to. But unfortunately, this does also
      pose the problem of how to propagate some defines which are required by
      both the library and the test suite.
      
      One way would be to create another variable keeping track of all added
      defines and declare it inside of the parent scope. While this is the
      most obvious and simplest way of going ahead, it is kind of unfortunate.
      The main reason to not use this is that these defines become implicit
      dependencies between the build targets. By simply observing a define
      inside of the CMakeLists.txt file, one cannot reason whether this define
      is only required by the current target or whether it is required by
      different targets, as well.
      
      Another approach would be to use an internal header file keeping track
      of all defines shared between targets. While configuring the library, we
      will set various variables and let CMake configure the file, adding or
      removing defines based on what has been configured. Like this, one can
      easily keep track of the current environment by simply inspecting the
      header file. Furthermore, these dependencies are becoming clear inside
      the CMakeLists.txt, as instead of simply adding a define, we now call
      e.g. `SET(GIT_THREADSAFE 1)`.
      
      Having this header file though requires us to make sure it is always
      included before any "#ifdef"-preprocessor checks are executed. As we
      have already refactored code to always include the "common.h" header
      file before any statement inside of a file, this becomes easy: just make
      sure "common.h" includes the new "features.h" header file first.
      Patrick Steinhardt committed
  10. 09 Aug, 2017 3 commits
    • oid: use memcmp in git_oid__hashcmp · c9b1e646
      The open-coded version was inherited from git.git. But it
      turns out it was based on an older version of glibc, whose
      memcmp was not very optimized.
      
      Modern glibc does much better, and some compilers (like gcc
      7) can even inline the memcmp into a series of multi-byte
      xors.
      
      Upstream is switching to using memcmp in
      git/git@0b006014c87f400bd9a86267ed30fd3e7b383884.
      Jeff King committed
    • sha1_lookup: drop sha1_entry_pos function · 9842b327
      This was pulled over from git.git, and is an experiment in
      making binary-searching lists of sha1s faster. It was never
      compiled by default (nor was it used upstream by default
      without a special environment variable).
      
      Unfortunately, it is actually slower in practice, and
      upstream is planning to drop it in
      git/git@f1068efefe6dd3beaa89484db5e2db730b094e0b (which has
      some timing results). It's worth doing the same here for
      simplicity.
      Jeff King committed
    • sha1_position: convert do-while to while · 09930192
      If we enter the sha1_position() function with "lo == hi",
      we have no elements. But the do-while loop means that we'll
      enter the loop body once anyway, picking "mi" at that same
      value and comparing nonsense to our desired key. This is
      unlikely to match in practice, but we still shouldn't be
      looking at the memory in the first place.
      
      This bug is inherited from git.git; it was fixed there in
      e01580cfe01526ec2c4eb4899f776a82ade7e0e1.
      Jeff King committed
  11. 26 Jul, 2017 2 commits
  12. 21 Jul, 2017 1 commit
  13. 15 Jul, 2017 6 commits
    • config_file: refuse modifying included variables · 1b329089
      Modifying variables pulled in by an included file currently succeeds,
      but it doesn't actually do what one would expect, as refreshing the
      configuration will cause the values to reappear. As we are currently not
      really able to support this use case, we will instead just return an
      error for deleting and setting variables which were included via an
      include.
      Patrick Steinhardt committed
    • config_file: move reader into `config_read` only · 28c2cc3d
      Right now, we have multiple call sites which initialize a `reader`
      structure. As the structure is only actually used inside of
      `config_read`, we can instead just move the reader inside of the
      `config_read` function. Instead, we can just pass in the configuration
      file into `config_read`, which eases code readability.
      Patrick Steinhardt committed
    • config_file: refresh all files if includes were modified · 83bcd3a1
      Currently, we only re-parse the top-level configuration file when it has
      changed itself. This can cause problems when an include is changed, as
      we were not updating all values correctly.
      
      Instead of conditionally reparsing only refreshed files, the logic
      becomes much clearer and easier to follow if we always re-parse the
      top-level configuration file when either the file itself or one of its
      included configuration files has changed on disk. This commit implements
      this logic.
      
      Note that this might impact performance in some cases, as we need to
      re-read all configuration files whenever any of the included files
      changed. It could increase performance to just re-parse include files
      which have actually changed, but this would compromise maintainability
      of the code without much gain. The only case where we will gain anything
      is when we actually use includes and when only these includes are
      updated, which will probably be quite an unusual scenario to actually be
      worthwhile to optimize.
      Patrick Steinhardt committed
    • config_file: remove unused backend field from parse data · 56a7a264
      The backend passed to `config_read` is never actually used anymore, so
      we can remove it from the function and the `parse_data` structure.
      Patrick Steinhardt committed
    • config_file: pass reader directly to callbacks · 3a7f7a6e
      Previously, the callbacks passed to `config_parse` got the reader via a
      pointer to a pointer. This allowed the callbacks to update the callers
      `reader` variable when the array holding it has been reallocated. As the
      array is no longer present, we can simply the code by making the reader
      a simple pointer.
      Patrick Steinhardt committed
    • config_file: refactor include handling · 73df75d8
      Current code for configuration files uses the `reader` structure to
      parse configuration files and store additional metadata like the file's
      path and checksum. These structures are stored within an array in the
      backend itself, which causes multiple problems.
      
      First, it does not make sense to keep around the file's contents with
      the backend itself. While this data is usually free'd before being added
      to the backend, this brings along somewhat intricate lifecycle problems.
      A better solution would be to store only the file paths as well as the
      checksum of the currently parsed content only.
      
      The second problem is that the `reader` structures are stored inside an
      array. When re-parsing configuration files due to changed contents, we
      may cause this array to be reallocated, requiring us to update pointers
      hold by callers. Furthermore, we do not keep track of includes which
      are already associated to a reader inside of this array. This causes us
      to add readers multiple times to the backend, e.g. in the scenario of
      refreshing configurations.
      
      This commit fixes these shortcomings. We introduce a split between the
      parsing data and the configuration file's metadata. The `reader` will
      now only hold the file's contents and the parser state and the new
      `config_file` structure holds the file's path and checksum. Furthermore,
      the new structure is a recursive structure in that it will also hold
      references to the files it directly includes. The diskfile is changed to
      only store the top-level configuration file.
      
      These changes allow us further refactorings and greatly simplify
      understanding the code.
      Patrick Steinhardt committed
  14. 12 Jul, 2017 1 commit
  15. 10 Jul, 2017 1 commit
    • patch_generate: represent buffers as void pointers · 9093ced6
      Pointers to general data should usually be used as a void pointer such
      that it is possible to hand in variables of a different pointer type
      without the need to cast. This is the same when creating patches from
      buffers, where the buffers may contain arbitrary data. Instead of
      requiring the caller to care whether his buffer is e.g. `char *` or
      `unsigned char *`, we should instead just accept a `void *`. This is
      also consistent in how we tread other types like for example `git_blob`,
      which also just has a void pointer as its raw contents.
      Patrick Steinhardt committed
  16. 03 Jul, 2017 3 commits
    • Make sure to always include "common.h" first · 0c7f49dd
      Next to including several files, our "common.h" header also declares
      various macros which are then used throughout the project. As such, we
      have to make sure to always include this file first in all
      implementation files. Otherwise, we might encounter problems or even
      silent behavioural differences due to macros or defines not being
      defined as they should be. So in fact, our header and implementation
      files should make sure to always include "common.h" first.
      
      This commit does so by establishing a common include pattern. Header
      files inside of "src" will now always include "common.h" as its first
      other file, separated by a newline from all the other includes to make
      it stand out as special. There are two cases for the implementation
      files. If they do have a matching header file, they will always include
      this one first, leading to "common.h" being transitively included as
      first file. If they do not have a matching header file, they instead
      include "common.h" as first file themselves.
      
      This fixes the outlined problems and will become our standard practice
      for header and source files inside of the "src/" from now on.
      Patrick Steinhardt committed
    • Add missing license headers · 2480d0eb
      Some implementation files were missing the license headers. This commit
      adds them.
      Patrick Steinhardt committed
    • Fix missing include for header files · 0fb4b351
      Some of our header files are not included at all by any of their
      implementing counter-parts. Including them inside of these files leads
      to some compile errors mostly due to unknown types because of missing
      includes. But there's also one case where a declared function does not
      match the implementation's prototype.
      
      Fix all these errors by fixing up the prototype and adding missing
      includes. This is preparatory work for fixing up missing includes in the
      implementation files.
      Patrick Steinhardt committed
  17. 30 Jun, 2017 2 commits
    • win32: fix circular include deps with w32_crtdbg · 459fb8fe
      The current order of declarations and includes between "common.h" and
      "w32_crtdbg_stacktrace.h" is rather complicated. Both header files make
      use of things defined in the other one and are thus circularly dependent
      on each other. This makes it currently impossible to compile the
      "w32_crtdbg_stacktrace.c" file when including "common.h" inside of
      "w32_crtdbg_stacktrace.h".
      
      We can disentangle the mess by moving declaration of the inline crtdbg
      functions into the "w32_crtdbg_stacktrace.h" file and adding additional
      includes inside of it, such that all required functions are available to
      it. This allows us to break the dependency cycle.
      Patrick Steinhardt committed
  18. 26 Jun, 2017 1 commit
    • diff: implement function to calculate patch ID · 89a34828
      The upstream git project provides the ability to calculate a so-called
      patch ID. Quoting from git-patch-id(1):
      
          A "patch ID" is nothing but a sum of SHA-1 of the file diffs
          associated with a patch, with whitespace and line numbers ignored."
      
      Patch IDs can be used to identify two patches which are probably the
      same thing, e.g. when a patch has been cherry-picked to another branch.
      
      This commit implements a new function `git_diff_patchid`, which gets a
      patch and derives an OID from the diff. Note the different terminology
      here: a patch in libgit2 are the differences in a single file and a diff
      can contain multiple patches for different files. The implementation
      matches the upstream implementation and should derive the same OID for
      the same diff. In fact, some code has been directly derived from the
      upstream implementation.
      
      The upstream implementation has two different modes to calculate patch
      IDs, which is the stable and unstable mode. The old way of calculating
      the patch IDs was unstable in a sense that a different ordering the
      diffs was leading to different results. This oversight was fixed in git
      1.9, but as git tries hard to never break existing workflows, the old
      and unstable way is still default. The newer and stable way does not
      care for ordering of the diff hunks, and in fact it is the mode that
      should probably be used today. So right now, we only implement the
      stable way of generating the patch ID.
      Patrick Steinhardt committed
  19. 23 Jun, 2017 1 commit
  20. 21 Jun, 2017 1 commit
    • merge: fix potential free of uninitialized memory · 4dc87e72
      The function `merge_diff_mark_similarity_exact` may error our early and,
      when it does so, free the `ours_deletes_by_oid` and
      `theirs_deletes_by_oid` variables. While the first one can never be
      uninitialized due to the first call actually assigning to it, the second
      variable can be freed without being initialized.
      
      Fix the issue by initializing both variables to `NULL`.
      Patrick Steinhardt committed