From: Tanga Joris <tangajoris@gmail.com>
To: git@vger.kernel.org
Cc: christian.couder@gmail.com, siddharthasthana31@gmail.com,
kaartic.sivaraam@gmail.com, usmanakinyemi202@gmail.com,
Tanga Joris <tangajoris@gmail.com>
Subject: [Outreachy] Proposal draft v3: Improve how command arguments and options are scanned and parsed
Date: Mon, 9 Nov 2026 18:09:10 +0300 [thread overview]
Message-ID: <20261109150910.169348-1-tangajoris@gmail.com> (raw)
In-Reply-To: <20261108175038.946313-1-tangajoris@gmail.com>
Hi everyone,
This is the third draft of my proposal for "Improve how command
arguments and options are scanned and parsed" [1].
Changes since v2:
- Moved the git daemon conversion into the plan, so the plan does
not depend on the early-scan work.
- Tried early_scan_options() in "git diff", and added what happened
to 3B.
- Removed the fallback of fixing the loop alone from the minimum
outcome.
- Updated the timeline.
I would be grateful for feedback on section 3, and on the questions
in section 7. I plan to start with stash show. If someone is already
working on one of these commands, please tell me and I will pick
another one.
1. About me
-----------
Name: Tanga Joris
Pronouns: he/him
Email: tangajoris@gmail.com
GitHub: https://github.com/tmjoris
Blog: https://tmjoris.github.io
Location: Nairobi, Kenya (EAT, UTC+3)
I meet all the Outreachy eligibility rules.
I am about to graduate from the United States International
University - Africa, where I study Applied Computer Technology. I
learned C at ALX, where it was my introduction to programming.
I had done some small parsing work before, in my own project, kiln
(https://github.com/tmjoris/kiln), a Go CLI that runs code in
sandboxes. "kiln -addr host:7070 run" works, but "kiln run -addr
host:7070" fails. This is because main() and each subcommand parse
their flags separately. Git has the same question of who looks at an
argument first, and what they do with it.
That work made me curious about how much goes on behind something as
seemingly simple as parsing command-line arguments. I enjoy that kind
of work: starting with something that looks straightforward,
following the implementation, and gradually understanding the
decisions and assumptions behind it.
Availability: 40 hours a week from 7 December 2026 to 8 March 2027.
I have no classes, exams or other work during the internship.
Nairobi is UTC+3, so my working day overlaps with European hours.
2. My Git work so far
---------------------
- Microproject: checkout: use Q_() for messages with counts [2].
Merged to master in a04194e134.
- tj/drop-no-parseopt, v5 [3]. Four builtins used parse-options but
were still marked NO_PARSEOPT, so completion never asked them for
their options. The topic removes the flag from them, and adds a
t0012 test that catches such builtins. Junio: "OK. Let's mark the
topic as ready for 'next'."
- tj/annotate-blame-args [4]. I found it while trying to make
--git-completion-helper work outside a repository. cmd_annotate()
put "-c" in front of the user's arguments, so the helper option was
no longer the only argument. The diff option parser answered it
instead, and "git annotate --<TAB>" offered the options of
"git diff". The "-c" was not needed, so the fix deletes code.
Junio: "Nice simplification."
Both topics are marked "Will merge to 'next'?" in What's cooking
[5].
Both topics merge cleanly on top of 'next', which has Christian's
early-scan series, and t9300, t0040, t0012 and t9902 pass.
- apply: a fix for the "git apply --check" crash reported on the
list [6]. While looking into it, I found two more ways to trigger
the same assertion. Phillip asked for an error instead of accepting
these broken patches, and I agreed. v2 will follow once we settle
the details.
- A fix for the splitIndex bug that Andy Koppe reported on the list
[7]. With core.splitIndex and index.skipHash both set, index
entries are lost when a new shared index is written.
- Reviews:
- Edson's credential conversion [8]. I suggested a helper for the
code that was repeated in four functions, and a test for the
check on extra arguments, which no test covered.
- Azuama's rev-list early scan fix [9]. The tests failed without
the fix, but --author, --committer and --grep-reflog had the
same problem. Azuama's v4 skips their values too, and its
NEEDSWORK comment now uses my "--author --missing-only" example
[10]. Junio then asked for a different approach than a list of
options in rev-list.c.
- Joy's pack size limit patch [11]. 2/2 added a member to struct
repo_config_values, which Junio had asked people not to do for
now. So I asked whether 1/2 could be sent on its own, and Joy
sent it as a separate patch [12].
- Nelly's promisor.fetchOrder patch [13]. Setting the new variable
made "git show" in a partial clone die with a BUG() in
string-list.c, because an in-place split was used on a list that
duplicates its strings.
- shrimech's Rust patch for formatting object IDs [14]. "cargo
test" passed and the new test failed without the fix. I asked
whether algo 0 should be printed the way the C code does it.
brian explained that algo 0 coming from C is meant to panic.
3. Plan
-------
Before the internship, I plan to land my two topics. If there is
time, I would also send one small git.c patch so that
--git-completion-helper works outside a repository, as was done for
--help-all last year. That patch needs both of my topics first.
To pick the commands, I started from the 29 builtins marked
NO_PARSEOPT in git.c. I also read builtins that loop over argv by
hand before or after their real parse. That is how I found stash
show and the --no-index loop in "git diff". I ran each
candidate on master (fb255012ac), and I was able to reproduce
everything below. Other places I found are in section 4.
A. Convert hand-written parsing to parse-options
1. stash show. git stash show has a problem when somebody uses
-G <word>, which keeps only the changes that mention <word>.
Running "git stash show -p -Gbar" shows the diff of the patch,
but running "git stash show -p -G bar" fails with "error: bar is
not a valid reference". -S has the same problem.
It fails because, after parse_options(), stash show runs a loop
over each argument that is left over. If that argument starts
with "-", it goes to the diff options. If it does not, it is
treated as a stash name. That is also what happens to "bar".
Since it is treated as a stash name, and it is not a valid one,
the command fails.
blame and shortlog do not have this problem. They parse their
own options and the diff options together with
parse_revision_opt(), so -G gets its value. I would do the same
in stash show. What is left at the end would still be the stash
name, so "git stash show 1" keeps working. Some options, like
--all, are ignored today but would start to fail, so the commit
message has to say so.
2. Numbers in index-pack and fetch-pack. index-pack reads
--max-input-size with strtoumax(). The issue is that it only
reads the digits at the start, and silently ignores the rest of
the characters:
git index-pack --stdin --max-input-size=1k limit of 1 byte
git index-pack --stdin --max-input-size=abc no limit
So "1k" means 1 byte, and "abc" means 0, which means no limit.
fetch-pack reads --depth with strtol() in the same way, so a bad
depth fetches the whole history:
git fetch-pack --depth=abc ../src HEAD no depth limit
The receive.maxInputSize config rejects "abc" and reads "1k" as
1024, so --max-input-size and the config do not agree.
These are plumbing, and Junio asked on the diff-files
conversion whether converting is worth it for plumbing "meant
for use by scripts" [15]. So I would send small fixes for the
numbers, and convert the rest only if the list wants it.
unpack-objects has the same --max-input-size code, so I would
ask on the list whether to fix it in the same series.
3. git daemon. It reads all its options by hand, in one loop of
about 150 lines. --port is read with strtoul(), and nothing
checks that the number is a valid port:
git daemon --port=70000 ... listens on port 4464, no error
I would convert the loop to parse-options, with
PARSE_OPT_STOP_AT_NON_OPTION for the list of directories. Other
programs outside builtin/, like imap-send and scalar, already
use parse_options(). Existing inetd and start-up scripts must
keep working, so every argument form stays, and --no- forms that
do not exist today stay rejected. The daemon re-runs itself with
its original arguments and --serve, and parse_options() changes
argv, so I have to keep a copy. Tests in t5570.
Junio's question about plumbing [15] may apply here too. No
other Git program runs git daemon, which is why I picked it over
index-pack and fetch-pack. I would ask on the list first. If the
list does not want it, I would send a range check for --port and
pick another command with the mentors.
For each command I will first add tests for what works today, like
"git stash show 1". The test for the bug must fail before the fix.
I will also break each new branch on purpose, to check that a test
catches it.
B. Improve the early scan where "git diff" needs it
Before parsing anything, "git diff" runs a small loop whose job is
to look for --no-index. The loop's answer determines which parser
reads the rest of the command line: either the no-index one or the
normal one. An issue arises because the loop stops at the first
argument that doesn't start with "-". So in "git diff -G x
--no-index a b", it stops at "x", the value of -G, and never sees
--no-index. Since it never saw --no-index, the loop picks the
normal parser, which then gets --no-index as an option and fails
with "invalid option".
The opposite also happens. In "git diff -S --no-index HEAD~1 HEAD",
"--no-index" is the value of -S, a search for that text. But the
loop takes it for the option, so Git switches to no-index mode and
fails with "Could not access 'HEAD~1'". Inside a repository, with
a and b in the work tree:
git diff -G x --no-index a b error: invalid option:
--no-index
git diff -S --no-index HEAD~1 HEAD error: Could not access
'HEAD~1'
With the value stuck (-Gx, -S--no-index), both work.
Christian's v1 cover letter lists "git diff" among the commands
that could use the early scan, but were left out because it "has a
number of short options like `-S`, `-G`, `-O` taking separate
values" [16]. Few people type --no-index after -G. But "git diff -G
x ../a ../b" does not need --no-index at all, and it fails too.
Junio asked in Azuama's rev-list thread whether a separate early
scan is a good direction at all [17]. For "git diff" I think one
is still needed, because --no-index decides which parser reads the
rest of the command line. So it has to be found before either
parser runs.
The early scan has one rule that I find useful. It may miss an
option that parse_options() would accept, but it must never report
one that parse_options() would not. So when it is unsure, it stops.
Missing an option is the safe failure.
To see how far the scan gets today, I replaced the loop with
early_scan_options() on top of 'next', with add_diff_options() and
--no-index as its table. "git diff -S --no-index HEAD~1 HEAD" then
works. But "git diff -G x --no-index a b" still fails, and these
stop working:
git diff -p --no-index a b
git diff -U1 --no-index a b
git diff -Gx --no-index a b
git diff -w ../a ../b
The scan stops at every short option, so it never reaches
--no-index. t4053 still passes with this change, because it has no
case with a short option before --no-index. So the scan has to
step over short options first. For long options, patch 2/4 of
Christian's series split lookup_long_opt() out of parse_long_opt(),
so the scan can find them. Short options have nothing like that
yet.
Where the loop stops also decides when two paths outside the
repository are treated as --no-index. So "git diff -G x ../a ../b"
fails too, while "git diff ../a ../b" works. I will add tests for
these cases first.
Question: should I teach the scan to step over short options? Or
would a smaller fix be better, where the loop looks options up in
the add_diff_options() table? Junio called a hand-kept list of
options "inherently fragile" [18], so I don't plan on adding one.
4. Other places I found
-----------------------
This is not in the timeline, but I would start it with the mentors
if there is time.
Revision options. handle_revision_opt() in revision.c parses the
options of log, rev-list, show and others by hand. Christian's v1
cover letter suggests converting them to struct option [16]. The
diff options were moved to parse-options in 2019, but the revision
options were not. This is core code, so I would only start it if the
mentors want it.
5. Timeline
-----------
Now to 6 Dec Land tj/drop-no-parseopt and tj/annotate-blame-args.
Weeks 1-2 stash show: tests first, then the fix.
Weeks 3-4 The number fixes in index-pack and fetch-pack, and the
"git diff" tests from 3B. This is the holidays, so
small patches.
Weeks 5-7 Convert git daemon to parse-options.
Weeks 8-12 "git diff" --no-index, in the way the list prefers.
RFC first if it changes the scan.
Week 13 Spare week for late reviews, and the final blog post.
Review on the list takes days, so the weeks overlap. While one series
waits, I work on the next one.
At minimum: stash show, the number fixes and the git daemon
conversion sent and reviewed, and the "git diff" tests and an RFC
sent. If I fall behind, the "git diff" work stops at the tests and
the RFC.
6. Communication
----------------
A blog post every week for Christian, and a short written update
every week for Siddharth, by email. The update says what I sent,
what is in review, what is blocked, and what I will do next week.
Design questions go to the list before code. I answer every review
comment on the list before sending another version, as Kaartic
reminded me to.
7. Questions for mentors
------------------------
1. Should I start with stash show, or would you pick another command?
2. Would you like to see the revision options work from section 4?
The question about "git diff" is at the end of 3B.
8. After the internship
-----------------------
I want to keep working on the early scan and the revision options,
and keep reviewing parse-options patches. I would also like to help
the next group of applicants the way reviewers on the list have
helped me.
9. References
-------------
[1] Proposal v2:
https://lore.kernel.org/git/20261108175038.946313-1-tangajoris@gmail.com/
[2] Microproject, checkout: use Q_():
https://lore.kernel.org/git/20261022004834.246892-1-tangajoris@gmail.com/
[3] tj/drop-no-parseopt v5:
https://lore.kernel.org/git/20261105223458.1534785-1-tangajoris@gmail.com/
[4] annotate: pass the arguments to blame unchanged:
https://lore.kernel.org/git/20261105073101.1449292-1-tangajoris@gmail.com/
[5] What's cooking in git.git (Nov 2026, #03):
https://lore.kernel.org/git/xmqqmrrlfzcd.fsf@gitster.g/
[6] apply fix for "git apply --check":
https://lore.kernel.org/git/20261107195429.96842-1-tangajoris@gmail.com/
[7] splitIndex fix:
https://lore.kernel.org/git/20261108234235.1594316-1-tangajoris@gmail.com/
[8] My review of Edson's credential patch:
https://lore.kernel.org/git/CAKrRMNqsh=d+_C8u+q1EBeQVwDZaywGQGZ89oYLyyvmB=8u1jw@mail.gmail.com/
[9] My review of Azuama's rev-list v3:
https://lore.kernel.org/git/CAKrRMNrnd2+HxTxadKaha+K9ASWi_oN4h7gdjVYAGmyODwmi=w@mail.gmail.com/
[10] Azuama's rev-list v4:
https://lore.kernel.org/git/20261105120937.10269-1-azuama69@gmail.com/
[11] My review of Joy's pack size limit v3:
https://lore.kernel.org/git/CAKrRMNqzOX=vUUMrMf5P_Atar8_qCSbGdtBktfT+iL4hJfFX-Q@mail.gmail.com/
[12] Joy's separate patch for 1/2:
https://lore.kernel.org/git/pull.2298.git.1793966368860.gitgitgadget@gmail.com/
[13] My review of Nelly's promisor.fetchOrder patch:
https://lore.kernel.org/git/CAKrRMNpAW+f2XXMFqpB8ffZsTdyPsaFtHe8dku=JVXACp1M3Hw@mail.gmail.com/
[14] My review of shrimech's Rust patch:
https://lore.kernel.org/git/CAKrRMNrLvPck5psPpKX1eWe26To5srfFSLFObZ4zTX09RE551w@mail.gmail.com/
[15] Junio on the diff-files conversion:
https://lore.kernel.org/git/xmqq8q3blfh1.fsf@gitster.g/
[16] Christian's early-scan v1 cover letter:
https://lore.kernel.org/git/20260902161047.476753-1-christian.couder@gmail.com/
[17] Junio on whether rev-list needs an early scan:
https://lore.kernel.org/git/xmqqa4nlkelo.fsf@gitster.g/
[18] Junio on Azuama's rev-list v4:
https://lore.kernel.org/git/xmqqwlqqokxr.fsf@gitster.g/
Thanks,
Tanga
next prev parent reply other threads:[~2026-11-09 15:10 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-11-05 19:57 [Outreachy] Proposal draft v1: Improve how command arguments and options are scanned and parsed Tanga Joris
2026-11-08 17:50 ` [Outreachy] Proposal draft v2: " Tanga Joris
2026-11-09 15:09 ` Tanga Joris [this message]
-- strict thread matches above, loose matches on Subject: below --
2026-11-04 2:53 Cynthia Uche
2026-11-08 22:06 ` [Outreachy] Proposal draft v3: " Cynthia Uche
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20261109150910.169348-1-tangajoris@gmail.com \
--to=tangajoris@gmail.com \
--cc=christian.couder@gmail.com \
--cc=git@vger.kernel.org \
--cc=kaartic.sivaraam@gmail.com \
--cc=siddharthasthana31@gmail.com \
--cc=usmanakinyemi202@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox