KEMBAR78
Comparing v2.50.1.vfs.0.1...v2.50.1.vfs.0.2 · microsoft/git · GitHub
Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: microsoft/git
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: v2.50.1.vfs.0.1
Choose a base ref
...
head repository: microsoft/git
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: v2.50.1.vfs.0.2
Choose a head ref
  • 8 commits
  • 3 files changed
  • 4 contributors

Commits on Aug 22, 2025

  1. fixup! codeql: run static analysis as part of CI builds

    codeql: run the CodeQL workflow every week
    
    Run the CodeQL GitHub workflow every week, on Mondays at 03:00 UTC.
    It's good practice to run this regularly as queries are updated.
    
    [This was accidentally dropped from vfs-2.50.1]
    
    Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
    Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
    mjcheetham authored and dscho committed Aug 22, 2025
    Configuration menu
    Copy the full SHA
    f08bde2 View commit details
    Browse the repository at this point in the history
  2. vfs-2.50.1: codeql: run static analysis as part of CI builds (#786)

    I had forgotten to integrate #772
    into the `vfs-2.50.1` branch... sorry!
    dscho authored Aug 22, 2025
    Configuration menu
    Copy the full SHA
    336bf91 View commit details
    Browse the repository at this point in the history

Commits on Sep 2, 2025

  1. midx-write: only load initialized packs

    The fill_packs_from_midx() method was refactored in fcb2205 (midx:
    implement support for writing incremental MIDX chains, 2024-08-06) to
    allow for preferred packfiles and incremental multi-pack-indexes.
    However, this led to some conditions that can cause improperly
    initialized memory in the context's list of packfiles.
    
    The conditions caring about the preferred pack name or the incremental
    flag are currently necessary to load a packfile. But the context is
    still being populated with pack_info structs based on the packfile array
    for the existing multi-pack-index even if prepare_midx_pack() isn't
    called.
    
    Add a new test that breaks under --stress when compiled with
    SANITIZE=address. The chosen number of 100 packfiles was selected to get
    the --stress output to fail about 50% of the time, while 50 packfiles
    could not get a failure in most --stress runs. This test has a very
    minor check at the end confirming only one packfile remaining. The
    failing nature of this test actually relies on auto-GC cleaning up some
    packfiles during the creation of the commits, as tests setting gc.auto
    to zero make the packfile count match the number of added commits but
    also avoids hitting the memory issue.
    
    The test case is marked as EXPENSIVE not only because of the number of
    packfiles it creates, but because some CI environments were reporting
    errors during the test that I could not reproduce, specifically around
    being unable to open the packfiles or their pack-indexes.
    
    When it fails under SANITIZE=address, it provides the following error:
    
    AddressSanitizer:DEADLYSIGNAL
    =================================================================
    ==3263517==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000027
    ==3263517==The signal is caused by a READ memory access.
    ==3263517==Hint: address points to the zero page.
        #0 0x562d5d82d1fb in close_pack_windows packfile.c:299
        #1 0x562d5d82d3ab in close_pack packfile.c:354
        #2 0x562d5d7bfdb4 in write_midx_internal midx-write.c:1490
        #3 0x562d5d7c7aec in midx_repack midx-write.c:1795
        #4 0x562d5d46fff6 in cmd_multi_pack_index builtin/multi-pack-index.c:305
        ...
    
    This failure stack trace is disconnected from the real fix because it
    the bad pointers are accessed later when closing the packfiles from the
    context.
    
    There are a few different aspects to this fix that are worth noting:
    
     1. We return to the previous behavior of fill_packs_from_midx to not
        rely on the incremental flag or existence of a preferred pack.
    
     2. The behavior to scan all layers of an incremental midx is kept, so
        this is not a full revert of the change.
    
     3. We skip allocating more room in the pack_info array if the pack
        fails prepare_midx_pack().
    
     4. The method has always returned 0 for success and 1 for failure, but
        the condition checking for error added a check for a negative result
        for failure, so that is now updated.
    
     5. The call to open_pack_index() is removed, but this is needed later
        in the case of a preferred pack. That call is moved to immediately
        before its result is needed (checking for the object count).
    
    Signed-off-by: Derrick Stolee <stolee@gmail.com>
    derrickstolee committed Sep 2, 2025
    Configuration menu
    Copy the full SHA
    5151bb5 View commit details
    Browse the repository at this point in the history
  2. midx-write: put failing response value back

    This instance of setting the result to 1 before going to cleanup was
    accidentally removed in fcb2205 (midx: implement support for writing
    incremental MIDX chains, 2024-08-06).
    
    Signed-off-by: Derrick Stolee <stolee@gmail.com>
    derrickstolee committed Sep 2, 2025
    Configuration menu
    Copy the full SHA
    721dab7 View commit details
    Browse the repository at this point in the history
  3. midx-write: use cleanup when incremental midx fails

    The incremental mode of writing a multi-pack-index has a few extra
    conditions that could lead to failure, but these are currently
    short-ciruiting with 'return -1' instead of setting the method's
    'result' variable and going to the cleanup tag.
    
    Replace these returns with gotos to avoid memory issues when exiting
    early due to error conditions.
    
    Unfortunately, these error conditions are difficult to reproduce with
    test cases, which is perhaps one reason why the memory loss was not
    caught by existing test cases in memory tracking modes.
    
    Signed-off-by: Derrick Stolee <stolee@gmail.com>
    derrickstolee committed Sep 2, 2025
    Configuration menu
    Copy the full SHA
    49b6afa View commit details
    Browse the repository at this point in the history
  4. midx-write: use uint32_t for preferred_pack_idx

    midx-write.c has the DISABLE_SIGN_COMPARE_WARNINGS macro defined for a
    few reasons, but the biggest one is the use of a signed
    preferred_pack_idx member inside the write_midx_context struct. The code
    currently uses -1 to indicate an unset preferred pack but pack int ids
    are normally handled as uint32_t. There are also a few loops that search
    for the preferred pack by name and those iterators will need updates to
    uint32_t in the next change.
    
    For now, replace the use of -1 with a 'NO_PREFERRED_PACK' macro and an
    equality check. The macro stores the max value of a uint32_t, so we
    cannot store a preferred pack that appears last in a list of 2^32 total
    packs, but that's expected to be unreasonable already. This improves the
    range from 2^31 already.
    
    There are some careful things to worry about with initializing the
    preferred pack in the struct and using that value when searching for a
    preferred pack that was already incorrect but accidentally working when
    the index was initialized to zero.
    
    Signed-off-by: Derrick Stolee <stolee@gmail.com>
    derrickstolee committed Sep 2, 2025
    Configuration menu
    Copy the full SHA
    194db7c View commit details
    Browse the repository at this point in the history
  5. midx-write: reenable signed comparison errors

    Remove the remaining signed comparison warnings in midx-write.c so that
    they can be enforced as errors in the future. After the previous change,
    the remaining errors are due to iterator variables named 'i'.
    
    The strategy here involves defining the variable within the for loop
    syntax to make sure we use the appropriate bitness for the loop
    sentinel. This matters in at least one method where the variable was
    compared to uint32_t in some loops and size_t in others.
    
    While adjusting these loops, there were some where the loop boundary was
    checking against a uint32_t value _plus one_. These were replaced with
    non-strict comparisons, but also the value is checked to not be
    UINT32_MAX. Since the value is the number of incremental multi-pack-
    indexes, this is not a meaningful restriction. The new die() is about
    defensive programming more than it being realistically possible.
    
    Signed-off-by: Derrick Stolee <stolee@gmail.com>
    derrickstolee committed Sep 2, 2025
    Configuration menu
    Copy the full SHA
    1b1942d View commit details
    Browse the repository at this point in the history
  6. [2.50.1] midx-write: fix segfault and other cleanups (#790)

    This is a copy of #787 but for the 2.50.1 branch.
    derrickstolee authored Sep 2, 2025
    Configuration menu
    Copy the full SHA
    c0d7923 View commit details
    Browse the repository at this point in the history
Loading