1. 20 Jul, 2019 14 commits
    • checkout: postpone creation of symlinks to the end · 6be5ac23
      On most platforms it's fine to create symlinks to nonexisting files. Not
      so on Windows, where the type of a symlink (file or directory) needs to
      be set at creation time. So depending on whether the target file exists
      or not, we may end up with different symlink types. This creates a
      problem when performing checkouts, where we simply iterate over all blobs
      that need to be updated without treating symlinks any special. If the
      target file of the symlink is going to be checked out after the symlink
      itself, then the symlink will be created as directory symlink and not as
      file symlink.
      
      Fix the issue by iterating over blobs twice: once to perform postponed
      deletions and updates to non-symlink blobs, and once to perform updates
      to symlink blobs.
      Patrick Steinhardt committed
    • win32: fix symlinks to relative file targets · 50194dcd
      When creating a symlink in Windows, one needs to tell Windows whether
      the symlink should be a file or directory symlink. To determine which
      flag to pass, we call `GetFileAttributesW` on the target file to see
      whether it is a directory and then pass the flag accordingly. The
      problem though is if create a symlink with a relative target path, then
      we will check that relative path while not necessarily being inside of
      the working directory where the symlink is to be created. Thus, getting
      its attributes will either fail or return attributes of the wrong
      target.
      
      Fix this by resolving the target path relative to the directory in which
      the symlink is to be created.
      Patrick Steinhardt committed
    • tests: core: improve symlink test coverage · 93d37a1d
      Add two more tests to verify that we're not deleting symlink targets,
      but the symlinks themselves. Furthermore, convert several `cl_skip`s on
      Win32 to conditional skips depending on whether the clar sandbox
      supports symlinks or not. Windows is grown up now and may allow
      unprivileged symlinks if the machine has been configured accordingly.
      Patrick Steinhardt committed
    • tests: core: add missing asserts for several function calls · 683ea2b0
      Several function calls to `p_stat` and `p_close` have no verification if
      they actually succeeded. As these functions _may_ fail and as we also
      want to make sure that we're not doing anything dumb, let's check them,
      too.
      Patrick Steinhardt committed
    • win32: correctly unlink symlinks to directories · a00842c4
      When deleting a symlink on Windows, then the way to delete it depends on
      whether it is a directory symlink or a file symlink. In the first case,
      we need to use `DeleteFile`, in the second `RemoveDirectory`. Right now,
      `p_unlink` will only ever try to use `DeleteFile`, though, and thus fail
      to remove directory symlinks. This mismatches how unlink(3P) is expected
      to behave, though, as it shall remove any symlink disregarding whether
      it is a file or directory symlink.
      
      In order to correctly unlink a symlink, we thus need to check what kind
      of file this is. If we were to first query file attributes of every file
      upon calling `p_unlink`, then this would penalize the common case
      though. Instead, we can try to first delete the file with `DeleteFile`
      and only if the error returned is `ERROR_ACCESS_DENIED` will we query
      file attributes and determine whether it is a directory symlink to use
      `RemoveDirectory` instead.
      Patrick Steinhardt committed
    • path: extract function to check whether a path supports symlinks · ded77bb1
      When initializing a repository, we need to check whether its working
      directory supports symlinks to correctly set the initial value of the
      "core.symlinks" config variable. The code to check the filesystem is
      reusable in other parts of our codebase, like for example in our tests
      to determine whether certain tests can be expected to succeed or not.
      
      Extract the code into a new function `git_path_supports_symlinks` to
      avoid duplicate implementations. Remove a duplicate implementation in
      the repo test helper code.
      Patrick Steinhardt committed
    • fileops: rename to "futils.h" to match function signatures · e54343a4
      Our file utils functions all have a "futils" prefix, e.g.
      `git_futils_touch`. One would thus naturally guess that their
      definitions and implementation would live in files "futils.h" and
      "futils.c", respectively, but in fact they live in "fileops.h".
      
      Rename the files to match expectations.
      Patrick Steinhardt committed
    • Merge pull request #5179 from pks-t/pks/patch-parse-free · 1f44079c
      patch_parse: fix segfault due to line containing static contents
      Edward Thomson committed
    • patch_parse: fix segfault due to line containing static contents · a613832e
      With commit dedf70ad (patch_parse: do not depend on parsed buffer's
      lifetime, 2019-07-05), all lines of the patch are allocated with
      `strdup` to make lifetime of the parsed patch independent of the buffer
      that is currently being parsed. In patch b0893282 (patch_parse: ensure
      valid patch output with EOFNL, 2019-07-11), we introduced another
      code location where we add lines to the parsed patch. But as that one
      was implemented via a separate pull request, it wasn't converted to use
      `strdup`, as well. As a consequence, we generate a segfault when trying
      to deallocate the potentially static buffer that's now in some of the
      lines.
      
      Use `git__strdup` to fix the issue.
      Patrick Steinhardt committed
    • Merge pull request #5173 from pks-t/pks/gitignore-wildmatch-error · e07dbc92
      ignore: fix determining whether a shorter pattern negates another
      Edward Thomson committed
    • Merge pull request #5159 from pks-t/pks/patch-parse-old-missing-nl · fd7a384b
      patch_parse: handle missing newline indicator in old file
      Edward Thomson committed
    • Merge pull request #5158 from pks-t/pks/patch-parsed-lifetime · f33ca472
      patch_parse: do not depend on parsed buffer's lifetime
      Edward Thomson committed
    • Merge pull request #5174 from pks-t/pks/winhttp-hash · d78a1b18
      sha1: fix compilation of WinHTTP backend
      Edward Thomson committed
    • Merge pull request #5176 from pks-t/pks/repo-template-head · 964c1c60
      repository: do not initialize HEAD if it's provided by templates
      Edward Thomson committed
  2. 19 Jul, 2019 7 commits
    • repository: do not initialize HEAD if it's provided by templates · 9d46f167
      When using templates to initialize a git repository, then git-init(1)
      will copy over all contents of the template directory. These will be
      preferred over the default ones created by git-init(1). While we mostly
      do the same, there is the exception of "HEAD". While we do copy over the
      template's HEAD file, afterwards we'll immediately re-initialize its
      contents with either the default "ref: refs/origin/master" or the init
      option's `initial_head` field.
      
      Let's fix the inconsistency with upstream git-init(1) by not overwriting
      the template HEAD, but only if the user hasn't set `opts.initial_head`.
      If the `initial_head` field has been supplied, we should use that
      indifferent from whether the template contained a HEAD file or not. Add
      tests to verify we correctly use the template directory's HEAD file and
      that `initial_head` overrides the template.
      Patrick Steinhardt committed
    • repository: update error handling in `init_ext` · f3134a84
      Update `git_repository_init_ext` to use our typical style of error
      handling. The function had multiple statements which didn't `goto out`
      immediately but instead deferred it to later calls combined with `if`
      statements.
      Patrick Steinhardt committed
    • repository: avoid swallowing error codes in `create_head` · 869ae5a3
      The error handling in `git_repository_create_head` completely swallows
      all error codes. While probably not too much of a problem, this also
      violates our usual coding style.
      
      Refactor the code to use a local `error` variable with the typical `goto
      out` statements.
      Patrick Steinhardt committed
    • tests: repo: refactor setup of templates and repos · 0d12b8dd
      All tests in repo::template have a common pattern of first setting up
      templates, then settung up the repository that makes use of those
      templates via several init options. Refactor this pattern into two
      functions `setup_templates` and `setup_repo` that handle most of that
      logic to make it easier to spot what a test actually wants to check.
      
      Furthermore, this also refactors how we clean up after the tests.
      Previously, it was a combination of manually calling
      `cl_fixture_cleanup` and `cl_set_cleanup`, which really is kind of hard
      to read. This commit refactors this to instead provide the cleaning
      parameters in the setup functions. All cleanups are then performed in
      the suite's cleanup function.
      Patrick Steinhardt committed
    • tests: repo: refactor template path handling · 3b79ceaf
      The repo::template test suite makes use of quite a few local variables
      that could be consolidated. Do so to make the code easier to read.
      Patrick Steinhardt committed
    • tests: repo: move template tests into their own suite · ee193480
      There's quite a lot of supporting code for our templates and they are an
      obvious standalone feature. Thus, let's extract those tests into their
      own suite to also make refactoring of them easier.
      Patrick Steinhardt committed
    • Merge pull request #5138 from libgit2/ethomson/cvar · 3424c210
      configuration: cvar -> configmap
      Patrick Steinhardt committed
  3. 18 Jul, 2019 12 commits
  4. 17 Jul, 2019 3 commits
    • Merge pull request #5170 from bk2204/packbuilder-efficient-realloc · 51124a5b
      Allocate memory more efficiently when packing objects
      Edward Thomson committed
    • cache: evict items more efficiently · 770b91b1
      When our object cache is full, we pick eight items (or the whole cache,
      if there are fewer) and evict them. For small cache sizes, this is fine,
      but when we're dealing with a large number of objects, we can repeatedly
      exhaust the cache and spend a large amount of time in git_oidmap_iterate
      trying to find items to evict.
      
      Instead, let's assume that if the cache gets full, we have a large
      number of objects that we're handling, and be more aggressive about
      evicting items. Let's remove one item for every 2048 items, but not less
      than 8. This causes us to scale our evictions in proportion to the size
      of the cache and significantly reduces the time we spend in
      git_oidmap_iterate.
      
      Before this change, a full pack of all the non-blob objects in the Linux
      repository took in excess of 30 minutes and spent 62.3% of total runtime
      in odb_read_1 and its children, and 44.3% of the time in
      git_oidmap_iterate. With this change, the same operation now takes 14
      minutes and 44 seconds, and odb_read_1 accounts for only 35.9% of total
      time, whereas git_oidmap_iterate consists of 6.2%.
      
      Note that we do spend a little more time inflating objects and a decent
      amount more time in memcmp. However, overall, the time taken is
      significantly improved, and time in pack building is now dominated by
      git_delta_create_from_index (33.7%), which is what we would expect.
      brian m. carlson committed
    • pack-objects: allocate memory more efficiently · c4df926b
      The packbuilder code allocates memory in chunks. When it needs to
      allocate, it tries to add 1024 to the number of objects and multiply by
      3/2. However, it actually multiplies by 1 instead, since it performs an
      integral division in the expression "3 / 2" and only then multiplies by
      the increased number of objects.
      
      The current behavior causes the code to waste massive amounts of time
      copying memory when it reallocates, causing inserting all non-blob
      objects in the Linux repository into a new pack to take some
      indeterminate time greater than 5 minutes instead of 52 seconds.
      
      Correct this error by first dividing by two, and only then multiplying
      by 3. We still check for overflow for the multiplication, which is the
      only part that can overflow. This appears to be the only place in the
      code base which has this problem.
      brian m. carlson committed
  5. 16 Jul, 2019 1 commit
  6. 12 Jul, 2019 3 commits
    • Ignore VS2017 specific files and folders · 5f22f8d2
      Signed-off-by: Sven Strickroth <email@cs-ware.de>
      Sven Strickroth committed
    • Merge pull request #5131 from pks-t/pks/fileops-mkdir-in-root · f92d495d
      fileops: fix creation of directory in filesystem root
      Patrick Steinhardt committed
    • attr_file: ignore macros defined in subdirectories · f8346905
      Right now, we are unconditionally applying all macros found in a
      gitatttributes file. But quoting gitattributes(5):
      
          Custom macro attributes can be defined only in top-level
          gitattributes files ($GIT_DIR/info/attributes, the .gitattributes
          file at the top level of the working tree, or the global or
          system-wide gitattributes files), not in .gitattributes files in
          working tree subdirectories. The built-in macro attribute "binary"
          is equivalent to:
      
      So gitattribute files in subdirectories of the working tree may
      explicitly _not_ contain macro definitions, but we do not currently
      enforce this limitation.
      
      This patch introduces a new parameter to the gitattributes parser that
      tells whether macros are allowed in the current file or not. If set to
      `false`, we will still parse macros, but silently ignore them instead of
      adding them to the list of defined macros. Update all callers to
      correctly determine whether the to-be-parsed file may contain macros or
      not. Most importantly, when walking up the directory hierarchy, we will
      only set it to `true` once it reaches the root directory of the repo
      itself.
      
      Add a test that verifies that we are indeed not applying macros from
      subdirectories. Previous to these changes, the test would've failed.
      Patrick Steinhardt committed