Skip to content

update rebase man page description #1

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

update rebase man page description #1

wants to merge 5 commits into from

Conversation

jvns
Copy link
Owner

@jvns jvns commented Aug 6, 2025 •
edited
Loading

This patch series reorganizes the DESCRIPTION section of the git rebase man page. My goal is to make the page more accessible to git newcomers (who from my experience have an extremely difficult time understanding git man pages) while keeping the exact same information available for more advanced users. This series:

  • puts more commonly used information first (a basic example of a rebase and "how to fix merge conflicts" are moved to before "how rebase modifies ORIG_HEAD" and "what git rebase <upstream> <branch> does")
  • removes duplication
  • more closely parallels the git merge man page's structure by starting it with the same example history
  • moves some parts to later in the man page.

I've been using Git, reading its documentation, and writing Git explanations for a long time but I've never contributed to Git before so I'd appreciate any pointers to past discussions of Git's documentation philosophy or who the target audience for Git documentation is.

Currently the organization of the rebase DESCRIPTION section is:

  1. Command line arguments (git rebase without an upstream, )
  2. Explanation of internals and ORIG_HEAD
  3. Basic rebase example
  4. How rebase skips commits already in the upstream
  5. How to use --onto
  6. How to handle merge conflicts

I'm proposing rearranging the information as follows, since from my experience users are more likely to need to know how to handle merge conflicts in a rebase than how ORIG_HEAD works or what git rebase <upstream> <branch> means:

  1. Basic rebase example
  2. How to handle merge conflicts
  3. Command line arguments
  4. Explanation of internals and ORIG_HEAD
  5. How rebase skips commits already in the upstream

and move "How to use --onto" into a separate section further down.

@jvns jvns force-pushed the rebase-stacked branch 7 times, most recently from f8827a2 to 170d85d Compare August 7, 2025 16:11
jvns added 4 commits August 7, 2025 13:06
@jvns
doc: git-rebase: start with an example
1992c76 
Start with an example that mirrors the example in the `git-merge` man
page, to make it easier for folks to understand the difference between a
rebase and a merge.
@jvns
doc: git rebase: dedup merge conflict discussion
02ae00f 
Previously there were two explanations, this combines them both into a
single explanation.
@jvns
doc: git rebase: clarify arguments syntax
136f0d2 
I found this extremely hard to understand as it was and this seemed like
a clearer wording to me.
@jvns
doc: git-rebase: move --onto explanation down
0b3f224 
The DESCRIPTION section in `man git-rebase` is extremely long, much
longer than in other git man pages. This moves the explanation of
how to use `--onto` further down in the man page, with other similar
explanations, like "RECOVERING FROM UPSTREAM REBASE"
@jvns jvns force-pushed the rebase-stacked branch 2 times, most recently from 34bf76e to debbd75 Compare August 7, 2025 18:00
@jvns
doc: git-rebase: update discussion of internals
82b3419 
- make it clearer that we're talking about three steps of a process
- delete a duplicate explanation of how git rebase skips commits with
  the same textual changes (it's explained in more detail a few lines
  further down)
- move the `ORIG_HEAD` note down so that it doesn't interrupt the
  discussion of the mechanics.
jvns pushed a commit that referenced this pull request Sep 19, 2025
@derrickstolee @gitster
midx-write: only load initialized packs
c9388d9 
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.

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 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 <[email protected]>
Signed-off-by: Junio C Hamano <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@jvns