{"thread":{"id":"66741","subject":"[Outreachy] Proposal draft v1: Improve how command arguments and options are scanned and parsed","startedAt":"2026-11-01T21:06:23Z","lastAt":"2026-11-05T00:55:44Z","messageCount":4,"participants":["Cynthia Uche","Usman Akinyemi","Priestess"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"557092","messageId":"20261101210623.622-1-herroyalpriestess@gmail.com","threadId":"66741","inReplyTo":null,"subject":"[Outreachy] Proposal draft v1: Improve how command arguments and options are scanned and parsed","fromName":"Cynthia Uche","fromEmail":"herroyalpriestess@gmail.com","sentAt":"2026-11-01T21:06:23Z","receivedAt":"2026-11-01T21:06:23Z","isPatch":false,"sender":{"key":"herroyalpriestess@gmail.com","avatar":null},"body":"Hi team,\n\nThis is the first draft of my proposal for \"Improve how command\narguments and options are scanned and parsed\" for the December 2026\nOutreachy round. I would appreciate feedback, especially on the\ncandidate inventory (section 4) and the timeline (section 5).\n\n1. About me and logistics\n-------------------------\nName:     Cynthia Uche. I'm also called Priestess.\nPronouns: she/her\nEmail:    herroyalpriestess@gmail.com\nGitHub:   https://github.com/herrpriestess\nBlog:     https://medium.com/@thegirlSynth\nLocation: Abia, Nigeria (WAT, UTC+1)\n\nI have read the Outreachy eligibility rules and I meet all the\nrequirements.\n\nI am a software engineer, mostly working on Python and backend\nprojects. C was the first language I learned, so I am comfortable\nreading and writing it, although I have not used it much day to day\nuntil now.\n\nI have a merged patch in OpenStack Shared File Systems Service (Manila)\n(https://review.opendev.org/c/openstack/manila/+/982431), which went\nthrough Gerrit review, so reworking a change after review comments\nis not new to me.\n\nAvailability: 30 hours per week\n\n2. My Git work\n--------------\nMicroproject: t2104: avoid suppressing git's exit code\n  Thread: https://lore.kernel.org/git/20261021033723.5265-1-herroyalpriestess@gmail.com/\n  Topic:  cu/t2104-avoid-pipes\n  Status: merged to master (merge commit 84bea4324a)\n  Patrick reviewed it and found it good, so it needed no second\n  version.\n\nReview of the early-scan series:\n  [PATCH v3 4/4] fast-import: use early_scan_options() ...\n  https://lore.kernel.org/git/20261031142255.4941-1-herroyalpriestess@gmail.com/\n  I ran t9300 (261 tests) and t0040 (118 tests) on the topic as in\n  origin/seen (tip dabb68a20d); all pass. I noticed that\n  EARLY_SCAN_STOP_AT_NON_OPTION is passed although the real parse\n  does not use PARSE_OPT_STOP_AT_NON_OPTION. With that argument set\n  to 0 locally, \"--depth 5 extra --allow-unsafe-features\" reached\n  the usage error (exit 129) instead of the \"forbidden\" error\n  (exit 128), so the unsafe feature line had been accepted first.\n  I suggested documenting this in the comment at the call site.\n\nSteps I have taken: read the microproject and application pages,\nSubmittingPatches and MyFirstContribution; built Git and ran the\nrelevant tests; sent my microproject by git send-email; read the\nearly-scan series v1 to v3 with its reviews; reproduced the\ngit column behaviour (section 4); ran the new fast-import\ntests in t9300 and tried two command lines by hand (section 2).\n\n3. Project understanding\n------------------------\n3.1 The problem: two parses that can disagree\n\nSome commands must look at a few options before they call\nparse_options(), because the result changes how they read their input.\nfast-import is the example: the \"feature\" lines at the start of the\nstream are handled before the command line is parsed, so\ncmd_fast_import() scans argv early for --allow-unsafe-features.\n\nThe old scan was a hand-rolled loop. It compared each argument with\nthe exact option name and stopped at the first argument that did not\nstart with a dash. parse_options() is more permissive: it accepts\nunambiguous abbreviations and values separated from their option by a\nspace. So the two disagree:\n\n - \"--depth 5 --allow-unsafe-features\": the scan stops at \"5\" and\n   never sees the flag, although parse_options() accepts it. Unsafe\n   features are refused. That is the safe direction, but still wrong.\n - \"--quiet --export-pack --allow-unsafe-features\": every argument\n   starts with a dash, so the old scan saw the flag and allowed\n   unsafe features. But parse_options() takes it as the value of\n   --export-pack-edges, which --export-pack abbreviates. That is the\n   dangerous direction.\n\n3.2 The API in v3\n\nearly_scan_options(argc, argv, option, flags, fn, data) walks argv\nusing the same struct option array that parse_options() uses. Options\nthe caller cares about are marked PARSE_OPT_EARLY, and the callback\nreceives (option, value, unset, pos, data); a non-zero return stops\nthe scan. It returns the index of the first argument not consumed.\nTwo helpers make it agree with parse_options():\nparse_options_takes_argument() (1/4, which also counts\nPARSE_OPT_LASTARG_DEFAULT options as taking a value) and\nlookup_long_opt(), extracted from parse_long_opt() (2/4), so negated\nforms, abbreviations and aliases are looked up the same way without\nside effects. The scan always stops at \"--\" and \"--end-of-options\";\nEARLY_SCAN_STOP_AT_NON_OPTION is for callers whose parse_options()\ncall uses PARSE_OPT_STOP_AT_NON_OPTION or subcommands. It should not\nbe used with PARSE_OPT_KEEP_UNKNOWN_OPT.\n\n3.3 Why \"stop when unsure\" is safe\n\nThe scan can miss an option that parse_options() would accept, but it\nnever reports one that parse_options() would not. It stops at anything\nit cannot reliably skip: short options, unknown options, ambiguous or\ndisallowed abbreviations, low-level callbacks (which may consume any\nnumber of arguments), a value stuck to a negated or no-argument\noption, and a value-taking option given as the last argument. Missing\nan option only makes the caller behave as if it was not given (for\nfast-import, unsafe features are refused). Mistaking an option's value\nfor an option is the dangerous failure.\n\nWhy not make parse_options() do a dry run? Because it would be a much\nlarger refactoring. According to the 3/4 commit message, a dry run\nwould have to avoid writing through opt->value, calling callbacks,\ndying on invalid values, handling --help and tracking command mode\nconflicts. Junio has said he would prefer this in the long term [2].\nThe scan is simpler, and it keeps the option array and the\nPARSE_OPT_EARLY marks, so it does not block a dry run later.\n\n3.4 History of the series\n\n - v1 (2 Sep, 6 patches): early_scan_options() with its own option\n   table, a bisect fix for \"--\" detection when a term is \"--\", a\n   rev-parse fix for \"--\" when it is an option value,\n   parse_options_takes_argument(), building the scan from a struct\n   option array, and the fast-import conversion.\n - v2 (23 Sep, 3 patches): the scan reads the real struct option\n   array and PARSE_OPT_EARLY marks the wanted options. The bisect and\n   rev-parse patches are gone.\n - v3 (12 Oct, 4 patches, rebased on Git 2.56, a018953688): adds the\n   lookup_long_opt() extraction, handles LASTARG_DEFAULT, stops at\n   more cases, adds the \"unset\" callback argument, and removes the\n   NEEDSWORK comment in fast-import.\n\nWhy bisect and rev-parse were dropped: Junio said he saw little\npractical value in them, since he doubted users would name a bisect\nterm or a --default value \"--\", unlike fast-import, where the bug\naffects a safety check. Christian removed both patches in v2 and said\nthat bisect_start() could use the early scan once it has been converted\nto parse-options [2], [3].\n\n3.5 Current state and open follow-ups\n\nAs of Nov 01, cc/early-scan-options is in seen and not in next\n(early_scan_options() is not in origin/next). Junio's Oct 28 \"What's\ncooking\" listed it as \"Needs review\". Kaartic reviewed v3 on\n30-31 Oct and found it in much better shape. His points:\n - consume the lookup_long_opt() result in parse_long_opt() with a\n   switch instead of an if chain (2/4);\n - whether the PARSE_OPT_EARLY checks in parse_options_check() should\n   also be in early_scan_options(), in case it is called before\n   parse_options_start() (3/4);\n - document that the callback must not keep the \"option\" pointer,\n   because it is freed after the scan (3/4).\n\nNot covered by the series: commands that do not use parse-options\nat all, short options (the scan stops at them) and low-level\ncallbacks. Those are what section 4 is about.\n\n4. Candidate inventory and risks\n--------------------------------\nMethod: I counted hand-written option checks per builtin with\n  git grep -c 'strcmp(arg' -- builtin/ | sort -t: -k2 -n\n  git grep -n 'skip_prefix(arg, \"--' -- builtin/\nand listed builtins that never call parse_options with\n  git grep -L parse_options -- 'builtin/*.c'\nThe counts include small value converters (am, config, log, revert),\nwhich I treat as out of scope. The densest real parsing is in\nrev-parse (43), rev-list (18), index-pack and diff (13 each) and\nbisect (12).\n\n - git column (builtin/column.c, cmd_column())\n   It already uses parse_options(). Only the early step is by hand:\n   if argv[1] starts with \"--command=\", config is read with that\n   name before parse_options() runs, and a die() follows if the\n   parsed and scanned values differ. Observed:\n     git column --command=foo               works (exit 0)\n     git column --command foo               dies (exit 128)\n     git column --width=40 --command=foo    dies (exit 128)\n     git column --com=foo                   dies (exit 128)\n   The three failures all print \"fatal: --command must be the\n   first argument\", even when \"--command foo\" really is first. I found\n   this rule only in the die() message: searching Documentation/ and\n   t/ for \"must be the first\" finds nothing related, and t9002 does not\n   test --command.\n   Fits: replacing the scan with early_scan_options() and\n   PARSE_OPT_EARLY on --command, once the series is in next. I have not\n   yet checked whether any option in column's table would\n   make the scan stop.\n   Risk: user-visible change (errors become success). Question for\n   the list: is the \"must be first\" rule intentional? The callback\n   receives pos, so it could be kept (pos == 0) or dropped.\n\n - bisect (builtin/bisect.c)\n   bisect_start() has two hand-written loops: one looks for \"--\"\n   (the bug of the dropped v1 patch) and one parses --no-checkout,\n   --first-parent, --reset-when-found[=...] and the four --term-*\n   options, each in a \"--opt value\" and a \"--opt=value\" branch.\n   bisect_run() checks only argv[0] for --reset-when-found, and\n   bisect_terms() compares one string with --term-good/old/bad/new.\n   Fits: convert to parse-options first, order: bisect_terms(),\n   bisect_run(), then bisect_start() as a stretch.\n   Risk: bisect_start() is called from \"bisect start\", from\n   bisect_autostart() and from \"bisect replay\"; the original\n   arguments are written to BISECT_LOG and replayed, so they must be\n   kept. Abbreviations and \"--no-\" forms would start to be accepted.\n\n - check-ref-format (builtin/check-ref-format.c)\n   One loop over leading dash arguments handles --normalize/--print,\n   --allow-onelevel, --no-allow-onelevel and --refspec-pattern;\n   --branch is special-cased as exactly argc == 3; anything else\n   calls usage(). git.c lists it as NO_PARSEOPT.\n   Tests: t1402 covers --normalize, --allow-onelevel and\n   --refspec-pattern, alone and combined, and --branch (also from\n   outside a repository, and with the name -nain). It does not test\n   --print, abbreviations or \"--\". Fien Dora has a patch adding tests\n   for the precedence of --allow-onelevel and --no-allow-onelevel [10];\n   Junio said he would queue it, and it would guard that part of a\n   conversion.\n   Observed on origin/master (v2.56.0-307-g50fe5cc63f):\n     --norm refs/heads/x           usage text, exit=129\n     -- refs/heads/x               usage text, exit=129\n     --normalize -- refs/heads/x   usage text, exit=129\n     refs/heads/x --normalize      usage text, exit=129\n     --branch -nain                \"fatal: '-nain' is not a valid\n                                   branch name\", exit=128\n   Fits: parse-options: OPT_BIT for the flags (--no-allow-onelevel\n   then comes for free), OPT_ALIAS for --print, and dropping\n   NO_PARSEOPT in git.c.\n   Risks: commands that fail today would succeed: abbreviations, \"--\"\n   and negated forms such as --no-normalize would be accepted, and\n   so would options after <refname> unless I pass\n   PARSE_OPT_STOP_AT_NON_OPTION. --branch needs an explicit check\n   that it is not combined with other options, and \"--branch -nain\"\n   must keep failing with the invalid branch name error (exit 128),\n   not become a usage error from short option parsing.\n   Question for the list: is accepting \"--\" and abbreviations\n   welcome, should options after <refname> keep being rejected?\n\n - unpack-objects (builtin/unpack-objects.c) [if time allows]\n   -n, -q, -r, --strict[=<msg-types>], --pack_header=,\n   --max-input-size=. Risk: other Git programs run it with fixed\n   arguments, so those must keep working; I will list the callers\n   before choosing it. index-pack and fetch-pack are larger,\n   same issue.\n\n - Not proposed: rerere (Patrick's ps/rerere-subcommands (v2, 29 Oct),\n   would convert it to OPT_SUBCOMMAND [9]), rev-parse (options are\n   actions run in order and mixed with revisions, and its main option\n   handling is not table-driven), rev-list, diff and setup_revisions()\n   in revision.c (Christian thinks it should be converted to struct\n   option first).\n\nCommon risks: behaviour change for scripts; abbreviations\n(GIT_TEST_DISALLOW_ABBREVIATED_OPTIONS); short options and low-level\ncallbacks that stop the scan; PARSE_OPT_KEEP_UNKNOWN_OPT; test\ncoverage before the change. parse-options.c is changed by several\nexperienced contributors, so I will coordinate on the list before\nany API change.\n\n5. Plan and timeline\n--------------------\nInternship dates: 7 Dec 2026 to 8 Mar 2027.\n\nNow to Dec 6:  Follow cc/early-scan-options, help it get to next\n               with reviews, and discuss the targets on the list,\n               including check-ref-format.\nWeeks 1-2:     Agree on the inventory and a first target with the\n               mentors. Write tests that show the current\n               behaviour. Start the weekly blog.\nWeeks 2-5:     First series: bisect_terms() and bisect_run(), tests\n               first, then the conversion.\nWeeks 4-8:     Second series: check-ref-format if it is still free,\n               otherwise bisect_start(), which is then a main item and\n               not a stretch.\nWeeks 6-10:    unpack-objects, if time allows; index-pack and\n               fetch-pack only if time remains.\nWeeks 8-11:    column on early_scan_options(), once the series is in\n               next and the \"must be first\" question is settled.\n               Tests first, then the change.\nWeeks 11-13:   Rerolls, documentation, final report, and a list of\n               remaining candidates for the next intern.\nEvery week:    Weekly blog post.\n\nThe ranges overlap on purpose: each series usually waits days for\nreview, so I can work on the next one meanwhile. If I fall behind\nI drop unpack-objects first, then the bisect_start() when it is only a\nstretch. If the early-scan series changes shape or stalls, the column\nstep becomes another conversion from section 4 that does not depend on\nit. I have not planned any time off, except possibly 1-3 January, which\nI will arrange with the mentors beforehand.\n\n6. Communication\n----------------\nA weekly blog post (Christian asks for one), linked on the list;\nweekly written updates to Siddharth; calls with Christian planned in\nadvance; questions asked on the list first; replies to review within\n1-2 days.\n\nI will publish the blog on Medium and link each post on the list.\nI love sharing about what I'm learning, and I trust that I'll be\nlearning a lot in the next few weeks.\n\n7. After the internship\n-----------------------\nAfter my internship I intend to keep reviewing patches, help\nnewcomers learn the ropes, just like I did, and keep working on the\nfollow-ups that did not fit the internship. I believe in paying it\nforward, so if I ever get the chance to mentor someone else, I will\nconsider it a privilege.\n\n8. References\n-------------\n[1] v1 cover:\n    https://lore.kernel.org/git/20260902161047.476753-1-christian.couder@gmail.com/\n[2] Junio on the v1 cover and Christian's answer:\n    https://lore.kernel.org/git/xmqqpkyviizc.fsf@gitster.g/\n    https://lore.kernel.org/git/CAP8UFD3qUpjUayhkMumZ41iMut=1=Pcmzx1YYcEV9NMG18OPsw@mail.gmail.com/\n[3] Junio on v1 2/6 (bisect) and Christian's answer:\n    https://lore.kernel.org/git/xmqqse3rffr4.fsf@gitster.g/\n    https://lore.kernel.org/git/CAP8UFD3sh9Ejfgv7CB33LRU_z3i662_+tjdW7JqwRrzexWX_Ow@mail.gmail.com/\n[4] v3 cover and 4/4:\n    https://lore.kernel.org/git/20261012074413.337183-1-christian.couder@gmail.com/\n    https://lore.kernel.org/git/20261012074413.337183-5-christian.couder@gmail.com/\n[5] Kaartic on 2/4, 3/4 and the v3 cover:\n    https://lore.kernel.org/git/02ba1b9d-b598-454b-897f-cb33ba190b13@gmail.com\n    https://lore.kernel.org/git/6782fa63-1e46-4ae9-b129-a157986c2556@gmail.com\n    https://lore.kernel.org/git/1e194ea7-2e28-4142-b678-d8cb8bca8e35@gmail.com\n[6] \"What's cooking\" Oct 2026 #10:\n    https://lore.kernel.org/git/xmqqo6cd5hnt.fsf@gitster.g/\n[7] My microproject thread and merge commit:\n    https://lore.kernel.org/git/20261021033723.5265-1-herroyalpriestess@gmail.com/\n    84bea4324a\n[8] My review of 4/4:\n    https://lore.kernel.org/git/20261031142255.4941-1-herroyalpriestess@gmail.com/\n[9] Patrick's ps/rerere-subcommands:\n    https://lore.kernel.org/git/20261029-pks-rerere-subcommands-v2-0-07d5ba18e882@pks.im/\n[10] Fien Dora's t1402 patch (v3) and Junio's reply:\n     https://lore.kernel.org/git/20261029121715.3845-1-fiendora5@gmail.com/\n     https://lore.kernel.org/git/xmqqqzh81goi.fsf@gitster.g/\n[11] Earlier drafts: none, this is v1.\n\n9. Closing remarks\n------------------\nGit has a VAST codebase, and it can seem quite intimidating at first\nto a newcomer, but the support in this community has made it much\neasier to start. Resilience is one of my core attributes, and I also\nknow to ask for help when I need it, so I won't be the first to give\nup should things get more demanding than I expect. I would be glad to\nbe mentored by experienced contributors and to keep contributing after\nthe internship.\n\nThanks,\nCynthia Uche\n\n"},{"id":"557396","messageId":"20261104025340.7799-1-herroyalpriestess@gmail.com","threadId":"66741","inReplyTo":"20261101210623.622-1-herroyalpriestess@gmail.com","subject":"[Outreachy] Proposal draft v2: Improve how command arguments and options are scanned and parsed","fromName":"Cynthia Uche","fromEmail":"herroyalpriestess@gmail.com","sentAt":"2026-11-04T02:53:40Z","receivedAt":"2026-11-04T02:53:40Z","isPatch":false,"sender":{"key":"herroyalpriestess@gmail.com","avatar":null},"body":"Hi team,\n\nThis is the second draft of my proposal for \"Improve how command\narguments and options are scanned and parsed\" for the December 2026\nOutreachy round. v1 is here: [11]. I would appreciate\nfeedback, especially on the candidate inventory (section 4) and the\ntimeline (section 5).\n\nChanges since v1\n================\n - Section 2: Christian replied to my review of 4/4 and v4 of the\n   early-scan series includes my suggestion; I also added my other\n   two reviews.\n - Sections 3.2 to 3.5: v4 of the early-scan series and where it\n   stands now.\n - Section 4: Fien Dora's t1402 tests are in 'next'; the status of\n   rerere; the build the observations were made on.\n - Steps I have taken: updated.\n - About me: updated.\n - Communication: updated.\n\n1. About me and logistics\n-------------------------\nName:     Cynthia Uche. I'm also called Priestess.\nPronouns: she/her\nEmail:    herroyalpriestess@gmail.com\nIRC:      Priestess\nGitHub:   https://github.com/herr-priestess\nBlog:     https://herr-priestess.github.io/\nLinkedIn: https://linkedin.com/in/thecynthiauche\nLocation: Nigeria (WAT, UTC+1)\n\nI have read the Outreachy eligibility rules and I meet all the\nrequirements.\n\nI am a software engineer, mostly working on Python and backend\nprojects. C was the first language I learned, so I am comfortable\nreading and writing it, although I have not used it much day to day\nuntil now.\n\nI have a merged patch in OpenStack Shared File Systems Service (Manila)\n(https://review.opendev.org/c/openstack/manila/+/982431), which went\nthrough Gerrit review, so reworking a change after review comments\nis not new to me.\n\nAvailability: 30 hours per week\n\n2. My Git work\n--------------\nMicroproject: t2104: avoid suppressing git's exit code\n  Thread: https://lore.kernel.org/git/20261021033723.5265-1-herroyalpriestess@gmail.com/\n  Topic:  cu/t2104-avoid-pipes\n  Status: merged to 'next' on 2026-10-24 and graduated to master\n          (merge commit 84bea4324a); What's cooking Nov 2026 #01\n          [6b] lists it under \"Graduated to 'master'\".\n  Patrick reviewed it and found it good, so it needed no second\n  version.\n\nReview of the early-scan series:\n  (a) [PATCH v3 4/4] fast-import: use early_scan_options() ... [8]\n  I ran t9300 (261 tests) and t0040 (118 tests) on the topic as in\n  origin/seen (tip dabb68a20d); all pass. I noticed that\n  EARLY_SCAN_STOP_AT_NON_OPTION is passed although the real parse\n  does not use PARSE_OPT_STOP_AT_NON_OPTION. With that argument set\n  to 0 locally, \"--depth 5 extra --allow-unsafe-features\" reached\n  the usage error (exit 129) instead of the \"forbidden\" error\n  (exit 128), so the unsafe feature line had been accepted first.\n  I suggested documenting this in the comment at the call site.\n  Outcome: Christian replied on 2 Nov and sent v4 [12]. In v4 the\n  call site comment explains why the flag is passed, the header\n  comment of early_scan_options() says when callers should pass it\n  and when they should not use the scan, and a new t9300 test, \"R:\n  --allow-unsafe-features after a non-option\", covers that command\n  line. Kaartic then wrote that v4 looks good to him, and Junio that\n  he would mark the topic for 'next' [13].\n\n  I tested v4 (tip c2b4099f84 in origin/seen): t0040 (118 tests) and\n  t9300 (262 tests) pass, and the three command lines from my\n  review and from the series behave as described.\n\nOther reviews:\n  (b) [PATCH v2] test-chmtime: use parse_options() ... [14]\n  With the PARSE_OPT_KEEP_UNKNOWN_OPT argument set to 0, the new\n  t0082 still passed 3 of 3, while \"test-tool chmtime --get -86400\"\n  failed with \"unknown switch `8'\". I suggested a test for negative\n  offsets and listed three behaviour changes that the commit message\n  did not mention: flags after the timespec are now accepted,\n  \"-g -v\" now dies, and usage errors exit with 129 instead of 1.\n  Outcome: Shashank answered that all three are intended and that v3\n  will add the test and document them. v3 was sent on 2 Nov [14].\n\n  (c) [Outreachy PATCH v3 1/2] parse-options: answer\n  --git-completion-helper before command runs [15]\n  On master (ebdc087dc9) with the series applied, t9300, t0012 and\n  t9902 pass, and \"git --list-cmds=parseopt\" lists the four builtins\n  that the series unmarks. I compared \"git <cmd> --git-completion-\n  helper\" and the \"-all\" form for every builtin, with builds of\n  master and of the series, and the output was identical. The new\n  test only covers the plain form: with the \"-all\" branch changed to\n  show_gitcomp(opts, 0) it still passed. I suggested a test for that\n  branch.\n\nHow these relate to the project: its tasks are to find hand-rolled\nargument parsing, move it to parse-options or the early scan, and\ndiscuss the patches with reviewers. The three reviews are in that\ncode: the early-scan series itself, a test helper moved to\nparse_options(), and the parse-options completion code. Each time I\nstarted from running something (a mutation of one argument, a\ncomparison with a base build, a loop over all builtins) and not from\nreading alone. In (a) that led to changes in v4. Junio told another\napplicant that topics move forward through positive reviews [16], and\nthat is how I want to work during the internship: start from the\nfailing command line, make it a test, then change the code.\n(b) is also a real case of the flag that the v4 header tells callers\nof the early scan to avoid: test-chmtime keeps PARSE_OPT_KEEP_UNKNOWN_OPT\nso that a negative offset like \"-86400\" is not rejected as an unknown\nswitch, so a command like that could not use early_scan_options() as\nit is today. So before choosing a candidate in section 4, I would\ncheck whether its parse_options() call uses that flag.\n\nSteps I have taken: read the microproject and application pages,\nSubmittingPatches and MyFirstContribution; built Git and ran the\nrelevant tests; sent my microproject by git send-email; read the\nearly-scan series v1 to v4 with its reviews; reproduced the\ngit column behaviour (section 4); reviewed 4/4 of v3 and tested v4,\nand reviewed two other patches (above).\n\n3. Project understanding\n------------------------\n3.1 The problem: two parses that can disagree\n\nSome commands must look at a few options before they call\nparse_options(), because the result changes how they read their input.\nfast-import is the example: the \"feature\" lines at the start of the\nstream are handled before the command line is parsed, so\ncmd_fast_import() scans argv early for --allow-unsafe-features.\n\nThe old scan was a hand-rolled loop. It compared each argument with\nthe exact option name and stopped at the first argument that did not\nstart with a dash. parse_options() is more permissive: it accepts\nunambiguous abbreviations and values separated from their option by a\nspace. So the two disagree:\n\n - \"--depth 5 --allow-unsafe-features\": the scan stops at \"5\" and\n   never sees the flag, although parse_options() accepts it. Unsafe\n   features are refused. That is the safe direction, but still wrong.\n - \"--quiet --export-pack --allow-unsafe-features\": every argument\n   starts with a dash, so the old scan saw the flag and allowed\n   unsafe features. But parse_options() takes it as the value of\n   --export-pack-edges, which --export-pack abbreviates. That is the\n   dangerous direction. (This example is from the 4/4 commit\n   message [4]; I did not run it on an old build.)\n\n3.2 The API in v3 and v4\n\nearly_scan_options(argc, argv, option, flags, fn, data) walks argv\nusing the same struct option array that parse_options() uses. Options\nthe caller cares about are marked PARSE_OPT_EARLY, and the callback\nreceives (option, value, unset, pos, data); a non-zero return stops\nthe scan. It returns the index of the first argument not consumed.\nTwo helpers make it agree with parse_options():\nparse_options_takes_argument() (1/4, which also counts\nPARSE_OPT_LASTARG_DEFAULT options as taking a value) and\nlookup_long_opt(), extracted from parse_long_opt() (2/4), so negated\nforms, abbreviations and aliases are looked up the same way without\nside effects. The scan always stops at \"--\" and \"--end-of-options\";\nEARLY_SCAN_STOP_AT_NON_OPTION is for callers whose parse_options()\ncall uses PARSE_OPT_STOP_AT_NON_OPTION or subcommands. As of v4 the\nheader also says that passing it in other cases is safe, and useful\nwhen the caller rejects non-option arguments itself, and that the\nscan should not be used when parse_options() is called with\nPARSE_OPT_KEEP_UNKNOWN_OPT [12]. Since v4, early_scan_options() also\ncalls parse_options_check() on the options, so a bad PARSE_OPT_EARLY\nmark is caught before the scan relies on it.\n\n3.3 Why \"stop when unsure\" is safe\n\nThe scan can miss an option that parse_options() would accept, but it\nnever reports one that parse_options() would not. It stops at anything\nit cannot reliably skip: short options, unknown options, ambiguous or\ndisallowed abbreviations, low-level callbacks (which may consume any\nnumber of arguments), a value stuck to a negated or no-argument\noption, and a value-taking option given as the last argument. Missing\nan option only makes the caller behave as if it was not given (for\nfast-import, unsafe features are refused). Mistaking an option's value\nfor an option is the dangerous failure.\n\nWhy not make parse_options() do a dry run? Because it would be a much\nlarger refactoring. According to the 3/4 commit message, a dry run\nwould have to avoid writing through opt->value, calling callbacks,\ndying on invalid values, handling --help and tracking command mode\nconflicts. Junio has said he would prefer this in the long term [2].\nThe scan is simpler, and it keeps the option array and the\nPARSE_OPT_EARLY marks, so it does not block a dry run later.\n\n3.4 History of the series\n\n - v1 (2 Sep, 6 patches): early_scan_options() with its own option\n   table, a bisect fix for \"--\" detection when a term is \"--\", a\n   rev-parse fix for \"--\" when it is an option value,\n   parse_options_takes_argument(), building the scan from a struct\n   option array, and the fast-import conversion.\n - v2 (23 Sep, 3 patches): the scan reads the real struct option\n   array and PARSE_OPT_EARLY marks the wanted options. The bisect and\n   rev-parse patches are gone.\n - v3 (12 Oct, 4 patches, rebased on Git 2.56, a018953688): adds the\n   lookup_long_opt() extraction, handles LASTARG_DEFAULT, stops at\n   more cases, adds the \"unset\" callback argument, and removes the\n   NEEDSWORK comment in fast-import.\n - v4 (2 Nov, 4 patches, same base): a switch in parse_long_opt()\n   (2/4); parse_options_check() called from early_scan_options() and\n   clearer header comments (3/4); a clearer comment at the call site\n   and a new t9300 test (4/4).\n\nWhy bisect and rev-parse were dropped: Junio said he saw little\npractical value in them, since he doubted users would name a bisect\nterm or a --default value \"--\", unlike fast-import, where the bug\naffects a safety check. Christian removed both patches in v2 and said\nthat bisect_start() could use the early scan once it has been converted\nto parse-options [2], [3].\n\n3.5 Current state and open follow-ups\n\nv4 was sent on 2 Nov and addresses the three points Kaartic made on\nv3 (the switch in 2/4, the parse_options_check() call and the comment\nabout the option pointer in 3/4). Kaartic said v4 looks good [13].\nJunio replied that it looks good and that he would mark the topic for\n'next' [13]. As of 3 Nov, cc/early-scan-options is still only in seen\n(git grep -n early_scan_options origin/next).\n\nNot covered by the series: commands that do not use parse-options\nat all, short options (the scan stops at them) and low-level\ncallbacks. Those are what section 4 is about.\n\n4. Candidate inventory and risks\n--------------------------------\nMethod: I counted hand-written option checks per builtin with\n  git grep -c 'strcmp(arg' -- builtin/ | sort -t: -k2 -n\n  git grep -n 'skip_prefix(arg, \"--' -- builtin/\nand listed builtins that never call parse_options with\n  git grep -L parse_options -- 'builtin/*.c'\nThe counts include small value converters (am, config, log, revert),\nwhich I treat as out of scope. The densest real parsing is in\nrev-parse (43), rev-list (18), index-pack and diff (13 each) and\nbisect (12). Observations in this section were made on origin/master\nat v2.56.0-307-g50fe5cc63f, unless stated otherwise. I found no topic\nin What's cooking #01 [6b] that touches column, bisect or\nunpack-objects.\n\n - git column (builtin/column.c, cmd_column())\n   It already uses parse_options(). Only the early step is by hand:\n   if argv[1] starts with \"--command=\", config is read with that\n   name before parse_options() runs, and a die() follows if the\n   parsed and scanned values differ. Observed:\n     git column --command=foo               works (exit 0)\n     git column --command foo               dies (exit 128)\n     git column --width=40 --command=foo    dies (exit 128)\n     git column --com=foo                   dies (exit 128)\n   The three failures all print \"fatal: --command must be the\n   first argument\", even when \"--command foo\" really is first. I found\n   this rule only in the die() message: searching Documentation/ and\n   t/ for \"must be the first\" finds nothing related, and t9002 does not\n   test --command.\n   Fits: replacing the scan with early_scan_options() and\n   PARSE_OPT_EARLY on --command, once the series is in next. I have not\n   yet checked whether any option in column's table would\n   make the scan stop.\n   Risk: user-visible change (errors become success). Question for\n   the list: is the \"must be first\" rule intentional? The callback\n   receives pos, so it could be kept (pos == 0) or dropped.\n\n - bisect (builtin/bisect.c)\n   bisect_start() has two hand-written loops: one looks for \"--\"\n   (the bug of the dropped v1 patch) and one parses --no-checkout,\n   --first-parent, --reset-when-found[=...] and the four --term-*\n   options, each in a \"--opt value\" and a \"--opt=value\" branch.\n   bisect_run() checks only argv[0] for --reset-when-found, and\n   bisect_terms() compares one string with --term-good/old/bad/new.\n   Fits: convert to parse-options first, order: bisect_terms(),\n   bisect_run(), then bisect_start() as a stretch.\n   Risk: bisect_start() is called from \"bisect start\", from\n   bisect_autostart() and from \"bisect replay\"; the original\n   arguments are written to BISECT_LOG and replayed, so they must be\n   kept. Abbreviations and \"--no-\" forms would start to be accepted.\n\n - check-ref-format (builtin/check-ref-format.c)\n   One loop over leading dash arguments handles --normalize/--print,\n   --allow-onelevel, --no-allow-onelevel and --refspec-pattern;\n   --branch is special-cased as exactly argc == 3; anything else\n   calls usage(). git.c lists it as NO_PARSEOPT.\n   Tests: t1402 covers --normalize, --allow-onelevel and\n   --refspec-pattern, alone and combined, and --branch (also from\n   outside a repository, and with the name -nain). It does not test\n   --print, abbreviations or \"--\". Fien Dora's patch adding tests for\n   the precedence of --allow-onelevel and --no-allow-onelevel [10]\n   was merged to 'next' on 1 Nov and is marked \"Will merge to\n   'master'\" in What's cooking #01 [6b]. Her commit message says she\n   plans to explore converting this parser in a separate series [10].\n   Observed on origin/master (v2.56.0-307-g50fe5cc63f):\n     --norm refs/heads/x           usage text, exit=129\n     -- refs/heads/x               usage text, exit=129\n     --normalize -- refs/heads/x   usage text, exit=129\n     refs/heads/x --normalize      usage text, exit=129\n     --branch -nain                \"fatal: '-nain' is not a valid\n                                   branch name\", exit=128\n   Fits: parse-options: OPT_BIT for the flags (--no-allow-onelevel\n   then comes for free), OPT_ALIAS for --print, and dropping\n   NO_PARSEOPT in git.c.\n   Risks: commands that fail today would succeed: abbreviations, \"--\"\n   and negated forms such as --no-normalize would be accepted, and\n   so would options after <refname> unless I pass\n   PARSE_OPT_STOP_AT_NON_OPTION. --branch needs an explicit check\n   that it is not combined with other options, and \"--branch -nain\"\n   must keep failing with the invalid branch name error (exit 128),\n   not become a usage error from short option parsing.\n   Question for the list: is accepting \"--\" and abbreviations\n   welcome, should options after <refname> keep being rejected?\n\n - unpack-objects (builtin/unpack-objects.c) [if time allows]\n   -n, -q, -r, --strict[=<msg-types>], --pack_header=,\n   --max-input-size=. Risk: other Git programs run it with fixed\n   arguments, so those must keep working; I will list the callers\n   before choosing it. index-pack and fetch-pack are larger,\n   same issue.\n\n - Not proposed: rerere (Patrick's ps/rerere-subcommands, v2 of\n   29 Oct, would convert it to OPT_SUBCOMMAND [9]; \"Needs review\" in\n   What's cooking #01 [6b]), rev-parse (options are actions run in\n   order and mixed with revisions, and its main option handling is\n   not table-driven), rev-list, diff and setup_revisions() in\n   revision.c (Christian has said it should be converted to struct\n   option first).\n\nCommon risks: behaviour change for scripts; abbreviations\n(GIT_TEST_DISALLOW_ABBREVIATED_OPTIONS); short options and low-level\ncallbacks that stop the scan; PARSE_OPT_KEEP_UNKNOWN_OPT; test\ncoverage before the change. parse-options.c is changed by several\nexperienced contributors, so I will coordinate on the list before\nany API change.\n\n5. Plan and timeline\n--------------------\nInternship dates: 7 Dec 2026 to 8 Mar 2027.\n\nNow to Dec 6:  Follow cc/early-scan-options until it reaches\n               master, review related patches, and discuss the\n               targets on the list, including check-ref-format.\nWeeks 1-2:     Agree on the inventory and a first target with the\n               mentors. Write tests that show the current\n               behaviour. Start the weekly blog.\nWeeks 2-5:     First series: bisect_terms() and bisect_run(), tests\n               first, then the conversion.\nWeeks 4-8:     Second series: check-ref-format if it is still free,\n               otherwise bisect_start(), which is then a main item and\n               not a stretch.\nWeeks 6-10:    unpack-objects, if time allows; index-pack and\n               fetch-pack only if time remains.\nWeeks 8-11:    column on early_scan_options(), once the series is in\n               next and the \"must be first\" question is settled.\n               Tests first, then the change.\nWeeks 11-13:   Rerolls, documentation, final report, and a list of\n               remaining candidates for the next intern.\nEvery week:    Weekly blog post.\n\nThe ranges overlap on purpose: each series usually waits days for\nreview, so I can work on the next one meanwhile. If I fall behind\nI drop unpack-objects first, then the bisect_start() when it is only a\nstretch. If the early-scan series changes shape or stalls, the column\nstep becomes another conversion from section 4 that does not depend on\nit. I have not planned any time off, except possibly 1-3 January, which\nI will arrange with the mentors beforehand.\n\n6. Communication\n----------------\nA weekly blog post (Christian asks for one), linked on the list;\nweekly written updates to Siddharth; calls with Christian planned in\nadvance; questions asked on the list first; replies to review within\n1-2 days.\n\nI will publish the blog on GitHub pages and link each post on the list.\nI love sharing about what I'm learning, and I trust that I'll be\nlearning a lot in the next few weeks.\n\n7. After the internship\n-----------------------\nAfter my internship I intend to keep reviewing patches, help\nnewcomers learn the ropes, just like I did, and keep working on the\nfollow-ups that did not fit the internship. I believe in paying it\nforward, so if I ever get the chance to mentor someone else, I will\nconsider it a privilege.\n\n8. References\n-------------\n[1] v1 cover:\n    https://lore.kernel.org/git/20260902161047.476753-1-christian.couder@gmail.com/\n[2] Junio on the v1 cover and Christian's answer:\n    https://lore.kernel.org/git/xmqqpkyviizc.fsf@gitster.g/\n    https://lore.kernel.org/git/CAP8UFD3qUpjUayhkMumZ41iMut=1=Pcmzx1YYcEV9NMG18OPsw@mail.gmail.com/\n[3] Junio on v1 2/6 (bisect) and Christian's answer:\n    https://lore.kernel.org/git/xmqqse3rffr4.fsf@gitster.g/\n    https://lore.kernel.org/git/CAP8UFD3sh9Ejfgv7CB33LRU_z3i662_+tjdW7JqwRrzexWX_Ow@mail.gmail.com/\n[4] v3 cover and 4/4:\n    https://lore.kernel.org/git/20261012074413.337183-1-christian.couder@gmail.com/\n    https://lore.kernel.org/git/20261012074413.337183-5-christian.couder@gmail.com/\n[5] Kaartic on 2/4, 3/4 and the v3 cover:\n    https://lore.kernel.org/git/02ba1b9d-b598-454b-897f-cb33ba190b13@gmail.com\n    https://lore.kernel.org/git/6782fa63-1e46-4ae9-b129-a157986c2556@gmail.com\n    https://lore.kernel.org/git/1e194ea7-2e28-4142-b678-d8cb8bca8e35@gmail.com\n[6] \"What's cooking\" Oct 2026 #10:\n    https://lore.kernel.org/git/xmqqo6cd5hnt.fsf@gitster.g/\n[6b] \"What's cooking\" Nov 2026 #01:\n    https://lore.kernel.org/git/xmqqcxsmejd9.fsf@gitster.g/\n[7] My microproject thread and merge commit:\n    https://lore.kernel.org/git/20261021033723.5265-1-herroyalpriestess@gmail.com/\n    84bea4324a\n[8] My review of 4/4:\n    https://lore.kernel.org/git/20261031142255.4941-1-herroyalpriestess@gmail.com/\n[9] Patrick's ps/rerere-subcommands:\n    https://lore.kernel.org/git/20261029-pks-rerere-subcommands-v2-0-07d5ba18e882@pks.im/\n[10] Fien Dora's t1402 patch (v3) and Junio's reply:\n     https://lore.kernel.org/git/20261029121715.3845-1-fiendora5@gmail.com/\n     https://lore.kernel.org/git/xmqqqzh81goi.fsf@gitster.g/\n[11] Earlier drafts: v1:\n     https://lore.kernel.org/git/20261101210623.622-1-herroyalpriestess@gmail.com/\n[12] v4 cover and Christian's reply to my review:\n     https://lore.kernel.org/git/20261102151447.734680-6-christian.couder@gmail.com/\n     https://lore.kernel.org/git/CAP8UFD2m+wpcgqB2SasWLxyekY+5khrYyPkOuLoYwANqZws8fw@mail.gmail.com/\n[13] Kaartic on v4 and Junio's reply:\n     https://lore.kernel.org/git/faf0b5e6-1c63-4c68-9f23-42a452879854@gmail.com/\n     https://lore.kernel.org/git/xmqqv76dal2l.fsf@gitster.g/\n[14] My review of test-chmtime v2, Shashank's answer and his v3:\n     https://lore.kernel.org/git/20261102024623.24897-1-herroyalpriestess@gmail.com/\n     https://lore.kernel.org/git/CANHmUQS15GGDOXm-WZ9M-UyKf2N0K+VixowL_heNKW0vjE0vJw@mail.gmail.com/\n     https://lore.kernel.org/git/pull.2280.v3.git.1793639275978.gitgitgadget@gmail.com/\n[15] My review of Tanga Joris's v3 1/2:\n     https://lore.kernel.org/git/20261103092503.14872-1-herroyalpriestess@gmail.com/\n[16] Junio to Tanga Joris, 26 Oct:\n     https://lore.kernel.org/git/xmqq33ttvz0q.fsf@gitster.g/\n\n9. Closing remarks\n------------------\nGit has a VAST codebase, and it can seem quite intimidating at first\nto a newcomer, but the support in this community has made it much\neasier to start. Resilience is one of my core attributes, and I also\nknow to ask for help when I need it, so I won't be the first to give\nup should things get more demanding than I expect. I would be glad to\nbe mentored by experienced contributors and to keep contributing after\nthe internship.\n\nThanks,\nCynthia Uche\n\n"},{"id":"557518","messageId":"CAPSxiM8E3LrJeba_f1gj3mxRVUYapp-nSgj+++pcgJEUJpxeLQ@mail.gmail.com","threadId":"66741","inReplyTo":"20261104025340.7799-1-herroyalpriestess@gmail.com","subject":"Re: [Outreachy] Proposal draft v2: Improve how command arguments and options are scanned and parsed","fromName":"Usman Akinyemi","fromEmail":"usmanakinyemi202@gmail.com","sentAt":"2026-11-05T00:08:47Z","receivedAt":"2026-11-05T00:08:47Z","isPatch":false,"sender":{"key":"usmanakinyemi202@gmail.com","avatar":"https://avatars.githubusercontent.com/u/86585626?v=4"},"body":"On Wed, Nov 4, 2026 at 4:03 AM Cynthia Uche <herroyalpriestess@gmail.com> wrote:\n>\n> Hi team,\n>\nHello,\n\nThanks for your proposal.\n\nDo not forget to submit the application on the Outreachy page.\n\nThanks.\n\n>\n\n"},{"id":"557525","messageId":"CAOcE0w+5jVvq9ZJW+3ZjJ3WGAnSHTj5wpNvbx7ZC2a0_aEtyxA@mail.gmail.com","threadId":"66741","inReplyTo":"CAPSxiM8E3LrJeba_f1gj3mxRVUYapp-nSgj+++pcgJEUJpxeLQ@mail.gmail.com","subject":"Re: [Outreachy] Proposal draft v2: Improve how command arguments and options are scanned and parsed","fromName":"Priestess","fromEmail":"herroyalpriestess@gmail.com","sentAt":"2026-11-05T00:55:44Z","receivedAt":"2026-11-05T00:55:44Z","isPatch":false,"sender":{"key":"herroyalpriestess@gmail.com","avatar":null},"body":"On Thu, Nov 5, 2026 at 1:08 AM Usman Akinyemi\n<usmanakinyemi202@gmail.com> wrote:\n>\n> Hello,\nHi, Usman\n\n> Thanks for your proposal.\n>\n> Do not forget to submit the application on the Outreachy page.\n\nWill do. Thank you for reading and reviewing!\n\n"}]}