{"thread":{"id":"58521","subject":"What's cooking in git.git (Sep 2022, #08; Tue, 27)","startedAt":"2022-09-27T21:11:15Z","lastAt":"2022-09-29T15:48:17Z","messageCount":11,"participants":["Junio C Hamano","Victoria Dye","Jeff King","Ævar Arnfjörð Bjarmason"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"463769","messageId":"xmqqtu4s1q1m.fsf@gitster.g","threadId":"58521","inReplyTo":null,"subject":"What's cooking in git.git (Sep 2022, #08; Tue, 27)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-09-27T21:11:01Z","receivedAt":"2022-09-27T21:11:15Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Here are the topics that have been cooking in my tree.  Commits\nprefixed with '+' are in 'next' (being in 'next' is a sign that a\ntopic is stable enough to be used and are candidate to be in a future\nrelease).  Commits prefixed with '-' are only in 'seen', and aren't\nconsidered \"accepted\" at all.  A topic without enough support may be\ndiscarded after a long period of no activity.\n\nThe -rc2 has been tagged for this cycle.  People are free to discuss\ntopics that has no relevance to the upcoming release, but we would\nappreciate if they instead concentrated on finding and fixing recent\nregressions in the upcoming release.  Even though I may be replacing\ntopics in 'seen' with their new iterations, I may not be picking up\npatches on new topics to 'seen', until the final around the\nbeginning of the next month (cf. https://tinyurl.com/gitCal).  These\npatches are welcome to come back in a more polished form after that\nhappens (read: discussions on them are not forbidden. just allow me\nto leave my tree less distracted by new topics).\n\nCopies of the source code to Git live in many repositories, and the\nfollowing is a list of the ones I push into or their mirrors.  Some\nrepositories have only a subset of branches.\n\nWith maint, master, next, seen, todo:\n\n\tgit://git.kernel.org/pub/scm/git/git.git/\n\tgit://repo.or.cz/alt-git.git/\n\thttps://kernel.googlesource.com/pub/scm/git/git/\n\thttps://github.com/git/git/\n\thttps://gitlab.com/git-vcs/git/\n\nWith all the integration branches and topics broken out:\n\n\thttps://github.com/gitster/git/\n\nEven though the preformatted documentation in HTML and man format\nare not sources, they are published in these repositories for\nconvenience (replace \"htmldocs\" with \"manpages\" for the manual\npages):\n\n\tgit://git.kernel.org/pub/scm/git/git-htmldocs.git/\n\thttps://github.com/gitster/git-htmldocs.git/\n\nRelease tarballs are available at:\n\n\thttps://www.kernel.org/pub/software/scm/git/\n\n--------------------------------------------------\n[Graduated to 'master']\n\n* ds/bitmap-lookup-remove-tracing (2022-09-26) 1 commit\n  (merged to 'next' on 2022-09-26 at a0d94b95e0)\n + pack-bitmap: remove trace2 region from hot path\n\n Perf-fix.\n source: <pull.1365.v2.git.1664198277250.gitgitgadget@gmail.com>\n\n--------------------------------------------------\n[New Topics]\n\n* rj/ref-filter-get-head-description-leakfix (2022-09-26) 1 commit\n - ref-filter.c: fix a leak in get_head_description\n\n Leakfix.\n\n Will merge to 'next'?\n source: <6ff29e96-7f8d-c354-dced-b1b363e54467@gmail.com>\n\n--------------------------------------------------\n[Stalled]\n\n* es/mark-gc-cruft-as-experimental (2022-08-03) 2 commits\n - config: let feature.experimental imply gc.cruftPacks=true\n - gc: add tests for --cruft and friends\n\n Enable gc.cruftpacks by default for those who opt into\n feature.experimental setting.\n\n Expecting a reroll.\n cf. <220804.86a68ke9d5.gmgdl@evledraar.gmail.com>\n cf. <6803b725-526e-a1c8-f15c-a9ed4a144d4c@github.com>\n source: <20220803205721.3686361-1-emilyshaffer@google.com>\n\n\n* es/doc-creation-factor-fix (2022-07-28) 2 commits\n - range-diff: clarify --creation-factor=<factor>\n - format-patch: clarify --creation-factor=<factor>\n\n Expecting a reroll by somebody more familiar with the logic\n cf. <xmqqo7wfix7p.fsf@gitster.g>\n source: <7229p500-p2r4-on87-6802-8o90s36rr3s4@tzk.qr>\n\n\n* cw/remote-object-info (2022-08-13) 7 commits\n - SQUASH???\n - cat-file: add remote-object-info to batch-command\n - transport: add client support for object-info\n - serve: advertise object-info feature\n - protocol-caps: initialization bug fix\n - fetch-pack: move fetch initialization\n - fetch-pack: refactor packet writing\n\n A client component to talk with the object-info endpoint.\n\n Expecting a reroll.\n cf. <20220728230210.2952731-1-calvinwan@google.com>\n cf. <CAFySSZDvgwbbHCHfyuaqX3tKsr-GjJ9iihygg6rNNe46Ys7_EA@mail.gmail.com>\n source: <20220728230210.2952731-1-calvinwan@google.com>\n\n--------------------------------------------------\n[Cooking]\n\n* ds/scalar-unregister-idempotent (2022-09-27) 4 commits\n - string-list: document iterator behavior on NULL input\n - gc: replace config subprocesses with API calls\n - scalar: make 'unregister' idempotent\n - maintenance: add 'unregister --force'\n\n \"git maintenance unregister\" in a repository that is already been\n unregistered reported an error.\n\n Will merge to 'next'.\n source: <pull.1358.v4.git.1664287021.gitgitgadget@gmail.com>\n\n\n* jk/clone-allow-bare-and-o-together (2022-09-22) 1 commit\n  (merged to 'next' on 2022-09-27 at 1feca721ac)\n + clone: allow \"--bare\" with \"-o\"\n\n \"git clone\" did not like to see the \"--bare\" and the \"--origin\"\n options used together without a good reason.\n\n Will cook in 'next'.\n source: <YyvzVdfQVdysvMp2@coredump.intra.peff.net>\n\n\n* jk/fsck-on-diet (2022-09-22) 3 commits\n  (merged to 'next' on 2022-09-27 at c2e93082a1)\n + parse_object_buffer(): respect save_commit_buffer\n + fsck: turn off save_commit_buffer\n + fsck: free tree buffers after walking unreachable objects\n\n \"git fsck\" failed to release contents of tree objects already used\n from the memory, which has been fixed.\n\n Will cook in 'next'.\n source: <Yyw0PSVe3YTQGgRS@coredump.intra.peff.net>\n\n\n* jk/remote-rename-without-fetch-refspec (2022-09-22) 1 commit\n  (merged to 'next' on 2022-09-27 at 165fe0a832)\n + remote: handle rename of remote without fetch refspec\n\n \"git remote rename\" failed to rename a remote without fetch\n refspec, which has been corrected.\n\n Will cook in 'next'.\n source: <YyvzqZ5tmI0UdRAW@coredump.intra.peff.net>\n\n\n* js/merge-ort-in-read-only-repo (2022-09-26) 2 commits\n - merge-ort: return early when failing to write a blob\n - merge-ort: fix segmentation fault in read-only repositories\n\n In read-only repositories, \"git merge-tree\" tried to come up with a\n merge result tree object, which it failed (which is not wrong) and\n led to a segfault (which is bad), which has been corrected.\n\n Expecting a reroll.\n cf. <CABPp-BGJueKtcdvzGWH_ZK6yeA6r3457ue5Uub9_gdU5M-WmvQ@mail.gmail.com>\n The proposed log message for the second step may need updating.\n source: <pull.1362.v4.git.1664229348.gitgitgadget@gmail.com>\n\n\n* mc/cred-helper-ignore-unknown (2022-09-22) 3 commits\n  (merged to 'next' on 2022-09-27 at ce6e3616aa)\n + osxkeychain: clarify that we ignore unknown lines\n + netrc: ignore unknown lines (do not die)\n + wincred: ignore unknown lines (do not die)\n\n Most credential helpers ignored unknown entries in a credential\n description, but a few died upon seeing them.  The latter were\n taught to ignore them, too\n\n Will cook in 'next'.\n source: <pull.1363.git.1663865974.gitgitgadget@gmail.com>\n\n\n* es/retire-efgrep (2022-09-23) 2 commits\n  (merged to 'next' on 2022-09-27 at 344fdd138e)\n + check-non-portable-shell: detect obsolescent egrep/fgrep\n + Merge branch 'dd/retire-efgrep' into es/retire-efgrep\n (this branch uses dd/retire-efgrep.)\n\n Prepare for GNU [ef]grep that throw warning of their uses.\n\n Will cook in 'next'.\n source: <pull.1338.git.git.1663805905554.gitgitgadget@gmail.com>\n\n\n* cw/submodule-status-in-parallel (2022-09-23) 4 commits\n - diff-lib: parallelize run_diff_files for submodules\n - diff-lib: refactor functions\n - submodule: move status parsing into function\n - run-command: add pipe_output to run_processes_parallel\n\n Allow the internal \"diff-files\" engine to run \"how has this\n submodule changed?\" in parallel to speed up \"git status\".\n\n Breaks its self check.\n cf. https://github.com/git/git/actions/runs/3115673002/jobs/5052804463\n source: <20220922232947.631309-1-calvinwan@google.com>\n\n\n* vd/fix-unaligned-read-index-v4 (2022-09-23) 1 commit\n - read-cache: avoid misaligned reads in index v4\n\n The codepath that reads from the index v4 had unaligned memory\n accesses, which has been corrected.\n\n Expecting a reroll.\n cf. <Yy4nkEnhuzt2iH+R@coredump.intra.peff.net>\n cf. <bb3a2470-7ff5-e4a6-040a-96e0e3833978@gmail.com>\n source: <pull.1366.git.1663962236069.gitgitgadget@gmail.com>\n\n\n* dd/retire-efgrep (2022-09-21) 4 commits\n  (merged to 'next' on 2022-09-21 at 22bc339be1)\n + t: convert fgrep usage to \"grep -F\"\n + t: convert egrep usage to \"grep -E\"\n + t: remove \\{m,n\\} from BRE grep usage\n + CodingGuidelines: allow grep -E\n (this branch is used by es/retire-efgrep.)\n\n Prepare for GNU [ef]grep that throw warning of their uses.\n\n Will cook in 'next'.\n source: <cover.1663765176.git.congdanhqx@gmail.com>\n\n\n* tb/midx-repack-ignore-cruft-packs (2022-09-21) 7 commits\n - midx.c: avoid cruft packs with non-zero `repack --batch-size`\n - midx.c: remove unnecessary loop condition\n - midx.c: replace `xcalloc()` with `CALLOC_ARRAY()`\n - midx.c: avoid cruft packs with `repack --batch-size=0`\n - midx.c: prevent `expire` from removing the cruft pack\n - Documentation/git-multi-pack-index.txt: clarify expire behavior\n - Documentation/git-multi-pack-index.txt: fix typo\n\n \"git multi-pack-index repack/expire\" used to repack unreachable\n cruft into a new pack, which have been corrected.\n\n Will merge to 'next'?\n cf. <63a1c3d4-eff3-af10-4263-058c88e74594@github.com>\n source: <cover.1663638929.git.me@ttaylorr.com>\n\n\n* ah/fsmonitor-daemon-usage-non-l10n (2022-09-21) 1 commit\n  (merged to 'next' on 2022-09-21 at bc69a73c6c)\n + fsmonitor--daemon: don't translate literal commands\n\n Fix messages incorrectly marked for translation.\n\n Will cook in 'next'.\n source: <20220920050709.326359-1-alexhenrie24@gmail.com>\n\n\n* so/diff-merges-cleanup (2022-09-16) 3 commits\n  (merged to 'next' on 2022-09-22 at 57694bbed7)\n + diff-merges: clarify log.diffMerges documentation\n + diff-merges: cleanup set_diff_merges()\n + diff-merges: cleanup func_by_opt()\n\n Code clean-up.\n\n Will cook in 'next'.\n source: <20220914193102.5275-1-sorganov@gmail.com>\n\n\n* ja/rebase-i-avoid-amending-self (2022-09-26) 1 commit\n - sequencer: avoid dropping fixup commit that targets self via commit-ish\n\n \"git rebase -i\" can mistakenly attempt to apply a fixup to a commit\n itself, which has been corrected.\n\n Will merge to 'next'?\n source: <20220924222904.1784975-1-aclopte@gmail.com>\n\n\n* ac/fuzzers (2022-09-19) 1 commit\n - fuzz: reorganise the path for existing oss-fuzz fuzzers\n\n Source file shuffling.\n\n Will merge to 'next'?\n source: <pull.1353.v4.git.1663598215154.gitgitgadget@gmail.com>\n\n\n* hn/parse-worktree-ref (2022-09-19) 1 commit\n - refs: unify parse_worktree_ref() and ref_type()\n\n Code and semantics cleaning.\n\n Will merge to 'next'?\n source: <pull.1325.v2.git.git.1663605291172.gitgitgadget@gmail.com>\n\n\n* ed/fsmonitor-on-networked-macos (2022-09-25) 6 commits\n - fsmonitor: add documentation for allowRemote and socketDir options\n - fsmonitor: check for compatability before communicating with fsmonitor\n - fsmonitor: deal with synthetic firmlinks on macOS\n - fsmonitor: avoid socket location check if using hook\n - fsmonitor: relocate socket file if .git directory is remote\n - fsmonitor: refactor filesystem checks to common interface\n\n By default, use of fsmonitor on a repository on networked\n filesystem is disabled. Add knobs to make it workable on macOS.\n\n Will merge to 'next'?\n source: <pull.1326.v12.git.1664048782.gitgitgadget@gmail.com>\n\n\n* rj/branch-edit-description-with-nth-checkout (2022-09-12) 2 commits\n - branch: support for shortcuts like @{-1} completed\n - branch: refactor \"edit_description\" code path\n\n \"git branch --edit-description @{-1}\" is now a way to edit branch\n description of the branch you were on before switching to the\n current branch.\n\n Needs review.\n source: <7abdb5a9-5707-7897-4196-8d2892beeb81@gmail.com>\n\n\n* mj/credential-helper-auth-headers (2022-09-13) 8 commits\n - http: set specific auth scheme depending on credential\n - http: move proactive auth to first slot creation\n - http: store all request headers on active_request_slot\n - credential: add WWW-Authenticate header to cred requests\n - http: read HTTP WWW-Authenticate response headers\n - osxkeychain: clarify that we ignore unknown lines\n - netrc: ignore unknown lines (do not die)\n - wincred: ignore unknown lines (do not die)\n\n RFC\n source: <pull.1352.git.1663097156.gitgitgadget@gmail.com>\n\n\n* jc/environ-docs (2022-09-16) 5 commits\n - environ: GIT_INDEX_VERSION affects not just a new repository\n - environ: simplify description of GIT_INDEX_FILE\n - environ: GIT_FLUSH should be made a usual Boolean\n - environ: explain Boolean environment variables\n - environ: document GIT_SSL_NO_VERIFY\n\n Documentation on various Boolean GIT_* environment variables have\n been clarified.\n\n Will merge to 'next'?\n source: <20220915160659.126441-1-gitster@pobox.com>\n\n\n* rs/diff-caret-bang-with-parents (2022-09-15) 6 commits\n - revision: add parents after child for ^!\n - revision: rename add_parents_only() to add_nth_parent()\n - revision: factor out add_parents()\n - revision: factor out add_parent()\n - revision: factor out get_commit()\n - revision: use strtol_i() for exclude_parent\n\n \"git diff rev^!\" did not show combined diff to go to the rev from\n its parents.\n\n Needs review.\n source: <ba6eea28-fb3a-b376-2529-351727c02f1a@web.de>\n\n\n* ab/doc-synopsis-and-cmd-usage (2022-09-07) 34 commits\n - tests: start asserting that *.txt SYNOPSIS matches -h output\n - doc txt & -h consistency: make \"worktree\" consistent\n - worktree: define subcommand -h in terms of command -h\n - reflog doc: list real subcommands up-front\n - doc txt & -h consistency: make \"commit\" consistent\n - doc txt & -h consistency: make \"diff-tree\" consistent\n - doc txt & -h consistency: use \"[<label>...]\" for \"zero or more\"\n - doc txt & -h consistency: make \"annotate\" consistent\n - doc txt & -h consistency: make \"stash\" consistent\n - doc txt & -h consistency: add missing options\n - doc txt & -h consistency: use \"git foo\" form, not \"git-foo\"\n - doc txt & -h consistency: make \"bundle\" consistent\n - doc txt & -h consistency: make \"read-tree\" consistent\n - doc txt & -h consistency: make \"rerere\" consistent\n - doc txt & -h consistency: add missing options and labels\n - doc txt & -h consistency: make output order consistent\n - doc txt & -h consistency: add or fix optional \"--\" syntax\n - doc txt & -h consistency: fix mismatching labels\n - t/helper/test-proc-receive.c: use \"<options>\", not \"<options>...\"\n - doc txt & -h consistency: use \"<options>\", not \"<options>...\"\n - stash doc SYNOPSIS & -h: correct padding around \"[]()\"\n - doc txt & -h consistency: correct padding around \"[]()\"\n - doc txt & -h consistency: add missing \"]\" to bugreport \"-h\"\n - doc txt & -h consistency: add \"-z\" to cat-file \"-h\"\n - doc txt & -h consistency: fix incorrect alternates syntax\n - doc txt & -h consistency: word-wrap\n - built-ins: consistently add \"\\n\" between \"usage\" and options\n - doc SYNOPSIS & -h: fix incorrect alternates syntax\n - doc SYNOPSIS: consistently use ' for commands\n - doc SYNOPSIS: don't use ' for subcommands\n - blame: use a more detailed usage_msg_optf() error on bad -L\n - bundle: define subcommand -h in terms of command -h\n - builtin/bundle.c: use \\t, not fix indentation 2-SP indentation\n - CodingGuidelines: update and clarify command-line conventions\n\n The short-help text shown by \"git cmd -h\" and the synopsis text\n shown at the beginning of \"git help cmd\" have been made more\n consistent.\n\n Needs review.\n source: <cover-00.34-00000000000-20220902T092734Z-avarab@gmail.com>\n\n\n* ab/coccicheck-incremental (2022-08-31) 9 commits\n - spatchcache: add a ccache-alike for \"spatch\"\n - cocci: optimistically use COMPUTE_HEADER_DEPENDENCIES\n - cocci: make \"coccicheck\" rule incremental\n - cocci: split off \"--all-includes\" from SPATCH_FLAGS\n - cocci: split off include-less \"tests\" from SPATCH_FLAGS\n - Makefile: split off SPATCH_BATCH_SIZE comment from \"cocci\" heading\n - Makefile: have \"coccicheck\" re-run if flags change\n - Makefile: add ability to TAB-complete cocci *.patch rules\n - cocci rules: remove unused \"F\" metavariable from pending rule\n\n \"make coccicheck\" is time consuming. It has been made to run more\n incrementally.\n\n Needs review.\n source: <cover-v2-0.9-00000000000-20220831T205130Z-avarab@gmail.com>\n\n\n* tb/diffstat-with-utf8-strwidth (2022-09-14) 1 commit\n - diff.c: use utf8_strwidth() to count display width\n\n \"git diff --stat\" etc. were invented back when everything was ASCII\n and strlen() was a way to measure the display width of a string;\n adjust them to compute the display width assuming UTF-8 pathnames.\n\n Expecting a reroll.\n source: <20220914151333.3309-1-tboegi@web.de>\n\n\n* gc/submodule-clone-update-with-branches (2022-08-29) 6 commits\n - clone, submodule update: check out branches\n - submodule--helper: refactor up-to-date criterion\n - submodule: return target of submodule symref\n - t5617: drop references to remote-tracking branches\n - repo-settings: add submodule_propagate_branches\n - clone: teach --detach option\n\n \"git clone --recurse-submodules\" and \"git submodule update\" learns\n to honor the \"propagete branches\" option.\n\n Expecting a reroll.\n cf. <20220901200047.515294-1-jonathantanmy@google.com> and others\n source: <pull.1321.git.git.1661806456.gitgitgadget@gmail.com>\n\n\n* sy/sparse-grep (2022-09-23) 1 commit\n - builtin/grep.c: integrate with sparse index\n\n \"git grep\" learned to expand the sparse-index more lazily and on\n demand in a sparse checkout.\n source: <20220923041842.27817-2-shaoxuan.yuan02@gmail.com>\n\n\n* ds/use-platform-regex-on-macos (2022-08-26) 1 commit\n  (merged to 'next' on 2022-09-14 at 80905596d8)\n + grep: fix multibyte regex handling under macOS\n\n With a bit of header twiddling, use the native regexp library on\n macOS instead of the compat/ one.\n\n Will cook in 'next'.\n cf. <xmqqzgf389k9.fsf@gitster.g>\n source: <20220826085815.2771102-1-dds@aueb.gr>\n\n\n* ds/bundle-uri-3 (2022-09-09) 10 commits\n - bundle-uri: fetch a list of bundles\n - bundle-uri: limit recursion depth for bundle lists\n - bundle-uri: parse bundle list in config format\n - bundle-uri: unit test \"key=value\" parsing\n - bundle-uri: create \"key=value\" line parsing\n - bundle-uri: create base key-value pair parsing\n - bundle-uri: create bundle_list struct and helpers\n - bundle-uri: use plain string in find_temp_filename()\n - bundle-uri: short-circuit capability parsing\n - Merge branch 'ds/bundle-uri-clone' into ds/bundle-uri-3\n\n Define the logical elements of a \"bundle list\", data structure to\n store them in-core, format to transfer them, and code to parse\n them.\n\n Needs review.\n source: <pull.1333.v2.git.1662734015.gitgitgadget@gmail.com>\n\n\n* js/cmake-updates (2022-08-24) 5 commits\n - cmake: increase time-out for a long-running test\n - cmake: avoid editing t/test-lib.sh\n - add -p: avoid ambiguous signed/unsigned comparison\n - cmake: copy the merge tools for testing\n - cmake: make it easier to diagnose regressions in CTest runs\n\n Update to build procedure with VS using CMake/CTest.\n\n Expecting a reroll.\n cf. <3df77ffd-85a2-3a54-9005-34a24ec6e82d@github.com>\n cf. <531620e1-de4c-74aa-c840-c12ce81f8740@github.com> and others\n source: <pull.1320.v2.git.1661243463.gitgitgadget@gmail.com>\n\n\n* pw/rebase-keep-base-fixes (2022-09-07) 7 commits\n - rebase --keep-base: imply --no-fork-point\n - rebase --keep-base: imply --reapply-cherry-picks\n - rebase: factor out branch_base calculation\n - rebase: rename merge_base to branch_base\n - rebase: store orig_head as a commit\n - t3416: set $EDITOR in subshell\n - t3416: tighten two tests\n\n \"git rebase --keep-base\" used to discard the commits that are\n already cherry-picked to the upstream, even when \"keep-base\" meant\n that the base, on top of which the history is being rebuilt, does\n not yet include these cherry-picked commits.  The --keep-base\n option now implies --reapply-cherry-picks and --no-fork-point\n options.\n\n Expecting a reroll.\n cf. <e25127f3-6135-b716-a12f-5dbe4f40dc42@gmail.com>\n source: <pull.1323.v2.git.1662561470.gitgitgadget@gmail.com>\n\n\n* ag/merge-strategies-in-c (2022-08-10) 14 commits\n - sequencer: use the \"octopus\" strategy without forking\n - sequencer: use the \"resolve\" strategy without forking\n - merge: use the \"octopus\" strategy without forking\n - merge: use the \"resolve\" strategy without forking\n - merge-octopus: rewrite in C\n - merge-recursive: move better_branch_name() to merge.c\n - merge-resolve: rewrite in C\n - merge-one-file: rewrite in C\n - update-index: move add_cacheinfo() to read-cache.c\n - merge-index: add a new way to invoke `git-merge-one-file'\n - merge-index: drop the index\n - merge-index: libify merge_one_path() and merge_all()\n - t6060: add tests for removed files\n - t6060: modify multiple files to expose a possible issue with merge-index\n\n An attempt to rewrite remaining merge strategies from shell to C.\n\n Needs more work.\n At the minimum, we should lose 11/14 and possibly 08/14.\n cf. <xmqq7d36vfur.fsf@gitster.g>\n source: <20220809185429.20098-1-alban.gruin@gmail.com>\n\n\n* po/glossary-around-traversal (2022-07-09) 3 commits\n - glossary: add reachability bitmap description\n - glossary: add commit graph description\n - glossary: add Object DataBase (ODB) abbreviation\n\n The glossary entries for \"commit-graph file\" and \"reachability\n bitmap\" have been added.\n\n Expecting a reroll.\n cf. <dfe0c1ab-33f8-f13e-71ce-1829bb0d2d7f@iee.email>\n source: <pull.1282.git.1657385781.gitgitgadget@gmail.com>\n\n\n* js/bisect-in-c (2022-08-30) 17 commits\n - bisect: no longer try to clean up left-over `.git/head-name` files\n - bisect: remove Cogito-related code\n - Turn `git bisect` into a full built-in\n - bisect: move even the command-line parsing to `bisect--helper`\n - bisect--helper: make `state` optional\n - bisect--helper: calling `bisect_state()` without an argument is a bug\n - bisect: avoid double-quoting when printing the failed command\n - bisect run: fix the error message\n - bisect: verify that a bogus option won't try to start a bisection\n - bisect--helper: migrate to OPT_SUBCOMMAND()\n - bisect--helper: make the order consistently `argc, argv`\n - bisect--helper: make `terms` an explicit singleton\n - bisect--helper: simplify exit code computation\n - bisect--helper: really retire `--bisect-autostart`\n - bisect--helper: really retire --bisect-next-check\n - bisect--helper: retire the --no-log option\n - Merge branch 'sg/parse-options-subcommand' into js/bisect-in-c\n\n Final bits of \"git bisect.sh\" have been rewritten in C.\n\n Needs review.\n cf. <xmqqv8pr8903.fsf@gitster.g>\n source: <pull.1132.v6.git.1661885419.gitgitgadget@gmail.com>\n"},{"id":"463781","messageId":"be8f11f2-c4ad-0542-066b-3bbc99a16dae@github.com","threadId":"58521","inReplyTo":"xmqqtu4s1q1m.fsf@gitster.g","subject":"Re: What's cooking in git.git (Sep 2022, #08; Tue, 27)","fromName":"Victoria Dye","fromEmail":"vdye@github.com","sentAt":"2022-09-28T01:52:02Z","receivedAt":"2022-09-28T01:52:09Z","isPatch":false,"sender":{"key":"vdye@github.com","avatar":"https://avatars.githubusercontent.com/u/3619353?v=4"},"body":"Junio C Hamano wrote:\n> * vd/fix-unaligned-read-index-v4 (2022-09-23) 1 commit\n>  - read-cache: avoid misaligned reads in index v4\n> \n>  The codepath that reads from the index v4 had unaligned memory\n>  accesses, which has been corrected.\n> \n>  Expecting a reroll.\n>  cf. <Yy4nkEnhuzt2iH+R@coredump.intra.peff.net>\n>  cf. <bb3a2470-7ff5-e4a6-040a-96e0e3833978@gmail.com>\n>  source: <pull.1366.git.1663962236069.gitgitgadget@gmail.com>\n> \n\nHow drastic an update were you expecting for this re-roll? To keep the fix\nminimal (that is, focused on 'create_from_disk()'), I was planning to just\nadd some comments explaining the implementation (in response to [1]). If the\ngoal is to get this merged quickly, I'd want to avoid a larger refactor\n(suggested in [2] & [3]), since doing so would either make the\nimplementations of \"read from disk\" ('create_from_disk()') and \"write to\ndisk\" ('copy_cache_entry_to_ondisk()') different/difficult to compare, or\nwould involve a more invasive refactor to update both functions [4].\n\nHowever, if there's no time pressure (after all, this bug has existed since\nthe introduction of index v4!), I'm happy to do that refactor. It would\nexpand the series to 2 or 3 patches, but should address all of the\nsuggestions made so far and make the code overall a bit clearer.\n\nLet me know what you think!\n\nThanks,\n- Victoria\n\n[1] https://lore.kernel.org/git/bb3a2470-7ff5-e4a6-040a-96e0e3833978@gmail.com/\n[2] https://lore.kernel.org/git/Yy4nkEnhuzt2iH+R@coredump.intra.peff.net/\n[3] https://lore.kernel.org/git/YzH+IPFBGleIsAUe@coredump.intra.peff.net/\n[4] https://lore.kernel.org/git/e5954e90-6b5c-46a6-0842-b3d7d1e06b33@github.com/\n"},{"id":"463783","messageId":"YzPLBN09zzlTdNgc@coredump.intra.peff.net","threadId":"58521","inReplyTo":"be8f11f2-c4ad-0542-066b-3bbc99a16dae@github.com","subject":"vd/fix-unaligned-read-index-v4, was Re: What's cooking in git.git (Sep 2022, #08; Tue, 27)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2022-09-28T04:18:12Z","receivedAt":"2022-09-28T04:18:18Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Sep 27, 2022 at 06:52:02PM -0700, Victoria Dye wrote:\n\n> Junio C Hamano wrote:\n> > * vd/fix-unaligned-read-index-v4 (2022-09-23) 1 commit\n> >  - read-cache: avoid misaligned reads in index v4\n> > \n> >  The codepath that reads from the index v4 had unaligned memory\n> >  accesses, which has been corrected.\n> > \n> >  Expecting a reroll.\n> >  cf. <Yy4nkEnhuzt2iH+R@coredump.intra.peff.net>\n> >  cf. <bb3a2470-7ff5-e4a6-040a-96e0e3833978@gmail.com>\n> >  source: <pull.1366.git.1663962236069.gitgitgadget@gmail.com>\n> > \n> \n> How drastic an update were you expecting for this re-roll? To keep the fix\n> minimal (that is, focused on 'create_from_disk()'), I was planning to just\n> add some comments explaining the implementation (in response to [1]). If the\n> goal is to get this merged quickly, I'd want to avoid a larger refactor\n> (suggested in [2] & [3]), since doing so would either make the\n> implementations of \"read from disk\" ('create_from_disk()') and \"write to\n> disk\" ('copy_cache_entry_to_ondisk()') different/difficult to compare, or\n> would involve a more invasive refactor to update both functions [4].\n\nThe first \"cf\" there is my initial request for a v2, but I since\nretracted that. I have no objection to adding more comments, but I am\nhappy enough without them (like Junio, it may be that I'm overly\nfamiliar with how I expect our get_be() functions to handle alignment).\n\nI think even if we want to go further in the near term, it is still best\nto build it on top of your basic fix. And we can take that fix and make\nfurther refactoring (if any) its own topic.\n\nI don't think even that basic fix needs to happen before the release,\nthough.  While it is a slight regression that SANITIZE=undefined does\nnot pass in the 2.38 release candidates:\n\n  - the bug really was there all along. It's just that a new test\n    triggers it.\n\n  - I don't think it's a sign of actual problems; we are forming an\n    unaligned pointer via a funny cast, which is technically UB, but I\n    don't think any real-world platforms actually care, since we\n    dereference it only via get_be16().\n\nSo it's mostly just a minor annoyance for running the tests; we're\nprobably better not to change any code, even trivially, this late in the\nrelease cycle.\n\nI was going to point to my branch with commits that are slightly less\nrough than what I posted, for somebody to work on it later if\ninterested. But after applying a minimal amount of polish, I think they\nare pretty reasonable, so I'll actually share them here.\n\nIMHO it would be OK to proceed with them even if we don't redo the\nwriting side. But I can see the argument that they should come together.\n\nI'm also OK if we just drop it. It was a fun puzzle to make it all work,\nand I do like the result. But arguably it's just churn, and in\nparticular I haven't measured the re-ordering patch to see if it\nproduces a performance difference. I doubt it does, but it's important\nenough that we should get a clear answer.\n\nSo here are my refactoring patches for the read side, built on top of\nyour earlier patch. They resolve the \"yuck\" comment by storing and\ncopying individual fields (which is much less verbose than you'd think\nbecause of struct assignment). I left the pointer type as signed; there\nare still come casts, but the same ones that were necessary before my\nseries.\n\n  [1/3]: pack-bitmap: make read_be32() public\n  [2/3]: read-cache: read on-disk entries in byte order\n  [3/3]: read-cache: use read_be32() in create_from_disk()\n\n compat/bswap.h | 22 ++++++++++++++++++\n pack-bitmap.c  | 12 ----------\n read-cache.c   | 60 +++++++++++++++++++++++++-------------------------\n 3 files changed, 52 insertions(+), 42 deletions(-)\n\n-Peff\n"},{"id":"463784","messageId":"YzPLZ3J+FDibgF7o@coredump.intra.peff.net","threadId":"58521","inReplyTo":"YzPLBN09zzlTdNgc@coredump.intra.peff.net","subject":"[PATCH 1/3] pack-bitmap: make read_be32() public","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2022-09-28T04:19:51Z","receivedAt":"2022-09-28T04:20:02Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"It's a handy shortcut for other in-order parsers of binary formats.\nWe'll drag along read_be8(), as well, for consistency, and add\nread_be16() to round out the set.\n\nWe'll also switch the signature to take a void pointer. This lets it be\nused equally well with either signed or unsigned byte strings (the\nget_be functions all cast to unsigned under the hood, so we don't care\neither way at this level).\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n compat/bswap.h | 22 ++++++++++++++++++++++\n pack-bitmap.c  | 12 ------------\n 2 files changed, 22 insertions(+), 12 deletions(-)\n\ndiff --git a/compat/bswap.h b/compat/bswap.h\nindex 512f6f4b99..6cda2cc50d 100644\n--- a/compat/bswap.h\n+++ b/compat/bswap.h\n@@ -190,4 +190,26 @@ static inline void put_be64(void *ptr, uint64_t value)\n \tp[7] = value >>  0;\n }\n \n+static inline uint32_t read_be32(const void *vbuffer, size_t *pos)\n+{\n+\tconst unsigned char *buffer = vbuffer;\n+\tuint32_t result = get_be32(buffer + *pos);\n+\t(*pos) += sizeof(result);\n+\treturn result;\n+}\n+\n+static inline uint16_t read_be16(const void *vbuffer, size_t *pos)\n+{\n+\tconst unsigned char *buffer = vbuffer;\n+\tuint16_t result = get_be16(buffer + *pos);\n+\t(*pos) += sizeof(result);\n+\treturn result;\n+}\n+\n+static inline uint8_t read_u8(const void *vbuffer, size_t *pos)\n+{\n+\tconst unsigned char *buffer = vbuffer;\n+\treturn buffer[(*pos)++];\n+}\n+\n #endif /* COMPAT_BSWAP_H */\ndiff --git a/pack-bitmap.c b/pack-bitmap.c\nindex 440407f1be..51d1e79b70 100644\n--- a/pack-bitmap.c\n+++ b/pack-bitmap.c\n@@ -242,18 +242,6 @@ static struct stored_bitmap *store_bitmap(struct bitmap_index *index,\n \treturn stored;\n }\n \n-static inline uint32_t read_be32(const unsigned char *buffer, size_t *pos)\n-{\n-\tuint32_t result = get_be32(buffer + *pos);\n-\t(*pos) += sizeof(result);\n-\treturn result;\n-}\n-\n-static inline uint8_t read_u8(const unsigned char *buffer, size_t *pos)\n-{\n-\treturn buffer[(*pos)++];\n-}\n-\n #define MAX_XOR_OFFSET 160\n \n static int nth_bitmap_object_oid(struct bitmap_index *index,\n-- \n2.38.0.rc2.615.g4fac75f9e3\n\n"},{"id":"463785","messageId":"YzPLuizoOlDuPu4G@coredump.intra.peff.net","threadId":"58521","inReplyTo":"YzPLBN09zzlTdNgc@coredump.intra.peff.net","subject":"[PATCH 2/3] read-cache: read on-disk entries in byte order","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2022-09-28T04:21:14Z","receivedAt":"2022-09-28T04:21:19Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"An index entry starts with stat data and mode, followed by an oid,\nfollowed by flags, followed by the entry name. We parse this out of\norder:\n\n  1. we must read the flags to know how long the name is\n\n  2. we must know how long the name is in order to allocate the\n     struct cache_entry, since it has a FLEX_ARRAY\n\n  3. we must allocate the cache_entry in order to parse the stat_data\n     and oid into it\n\nThis makes the parser a little hard to follow, because we have to access\nthe flags using an offset, rather than walking through it in byte order.\n\nWe can break the cyclic dependency by parsing the stat_data, etc, into\ntemporary variables, then allocating the cache_entry and copying the\nparsed values in. This sets us up for simplifying the parsing in the\nnext commit.\n\nThe downside is that we're copying the data an extra time. It's not very\nmuch data, and it's all fixed size, so the compiler should be able to do\na reasonable job of optimizing here. But I didn't time the potential\nimpact.\n\nNote one subtlety in the patch: besides reordering the flags/name versus\nstat data, we adjust the order within the stat data to match the on-disk\norder (notably both fields of each cache_time struct are adjacent).\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n read-cache.c | 42 +++++++++++++++++++++++++-----------------\n 1 file changed, 25 insertions(+), 17 deletions(-)\n\ndiff --git a/read-cache.c b/read-cache.c\nindex d16eb97906..efb9efa5ee 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -1879,11 +1879,14 @@ static struct cache_entry *create_from_disk(struct mem_pool *ce_mem_pool,\n \t\t\t\t\t    unsigned long *ent_size,\n \t\t\t\t\t    const struct cache_entry *previous_ce)\n {\n+\tstruct stat_data sd;\n+\tunsigned int mode;\n+\tstruct object_id oid;\n \tstruct cache_entry *ce;\n \tsize_t len;\n \tconst char *name;\n \tconst unsigned hashsz = the_hash_algo->rawsz;\n-\tconst char *flagsp = ondisk + offsetof(struct ondisk_cache_entry, data) + hashsz;\n+\tconst char *flagsp;\n \tunsigned int flags;\n \tsize_t copy_len = 0;\n \t/*\n@@ -1895,6 +1898,24 @@ static struct cache_entry *create_from_disk(struct mem_pool *ce_mem_pool,\n \t */\n \tint expand_name_field = version == 4;\n \n+\tsd.sd_ctime.sec = get_be32(ondisk + offsetof(struct ondisk_cache_entry, ctime)\n+\t\t\t\t\t\t\t+ offsetof(struct cache_time, sec));\n+\tsd.sd_ctime.nsec = get_be32(ondisk + offsetof(struct ondisk_cache_entry, ctime)\n+\t\t\t\t\t\t\t + offsetof(struct cache_time, nsec));\n+\tsd.sd_mtime.sec = get_be32(ondisk + offsetof(struct ondisk_cache_entry, mtime)\n+\t\t\t\t\t\t\t+ offsetof(struct cache_time, sec));\n+\tsd.sd_mtime.nsec = get_be32(ondisk + offsetof(struct ondisk_cache_entry, mtime)\n+\t\t\t\t\t\t\t + offsetof(struct cache_time, nsec));\n+\tsd.sd_dev   = get_be32(ondisk + offsetof(struct ondisk_cache_entry, dev));\n+\tsd.sd_ino   = get_be32(ondisk + offsetof(struct ondisk_cache_entry, ino));\n+\tmode        = get_be32(ondisk + offsetof(struct ondisk_cache_entry, mode));\n+\tsd.sd_uid   = get_be32(ondisk + offsetof(struct ondisk_cache_entry, uid));\n+\tsd.sd_gid   = get_be32(ondisk + offsetof(struct ondisk_cache_entry, gid));\n+\tsd.sd_size  = get_be32(ondisk + offsetof(struct ondisk_cache_entry, size));\n+\n+\toidread(&oid, (const unsigned char *)ondisk + offsetof(struct ondisk_cache_entry, data));\n+\tflagsp = ondisk + offsetof(struct ondisk_cache_entry, data) + hashsz;\n+\n \t/* On-disk flags are just 16 bits */\n \tflags = get_be16(flagsp);\n \tlen = flags & CE_NAMEMASK;\n@@ -1934,25 +1955,12 @@ static struct cache_entry *create_from_disk(struct mem_pool *ce_mem_pool,\n \t}\n \n \tce = mem_pool__ce_alloc(ce_mem_pool, len);\n-\n-\tce->ce_stat_data.sd_ctime.sec = get_be32(ondisk + offsetof(struct ondisk_cache_entry, ctime)\n-\t\t\t\t\t\t\t+ offsetof(struct cache_time, sec));\n-\tce->ce_stat_data.sd_mtime.sec = get_be32(ondisk + offsetof(struct ondisk_cache_entry, mtime)\n-\t\t\t\t\t\t\t+ offsetof(struct cache_time, sec));\n-\tce->ce_stat_data.sd_ctime.nsec = get_be32(ondisk + offsetof(struct ondisk_cache_entry, ctime)\n-\t\t\t\t\t\t\t + offsetof(struct cache_time, nsec));\n-\tce->ce_stat_data.sd_mtime.nsec = get_be32(ondisk + offsetof(struct ondisk_cache_entry, mtime)\n-\t\t\t\t\t\t\t + offsetof(struct cache_time, nsec));\n-\tce->ce_stat_data.sd_dev   = get_be32(ondisk + offsetof(struct ondisk_cache_entry, dev));\n-\tce->ce_stat_data.sd_ino   = get_be32(ondisk + offsetof(struct ondisk_cache_entry, ino));\n-\tce->ce_mode  = get_be32(ondisk + offsetof(struct ondisk_cache_entry, mode));\n-\tce->ce_stat_data.sd_uid   = get_be32(ondisk + offsetof(struct ondisk_cache_entry, uid));\n-\tce->ce_stat_data.sd_gid   = get_be32(ondisk + offsetof(struct ondisk_cache_entry, gid));\n-\tce->ce_stat_data.sd_size  = get_be32(ondisk + offsetof(struct ondisk_cache_entry, size));\n+\tce->ce_stat_data = sd;\n+\tce->ce_mode = mode;\n \tce->ce_flags = flags & ~CE_NAMEMASK;\n \tce->ce_namelen = len;\n \tce->index = 0;\n-\toidread(&ce->oid, (const unsigned char *)ondisk + offsetof(struct ondisk_cache_entry, data));\n+\toidcpy(&ce->oid, &oid);\n \n \tif (expand_name_field) {\n \t\tif (copy_len)\n-- \n2.38.0.rc2.615.g4fac75f9e3\n\n"},{"id":"463786","messageId":"YzPMWnx1wJlpICHr@coredump.intra.peff.net","threadId":"58521","inReplyTo":"YzPLBN09zzlTdNgc@coredump.intra.peff.net","subject":"[PATCH 3/3] read-cache: use read_be32() in create_from_disk()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2022-09-28T04:23:54Z","receivedAt":"2022-09-28T04:23:59Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Since the previous patch put our parsing in byte order, we can simplify\nthe function by keeping a \"pos\" variable with our current offset into\nthe on-disk data. That lets us use read_be32() to read and advance the\npos, rather than calculating an offset for each field. This is easier to\nread and harder to get wrong.\n\nLikewise, we no longer need the \"flagsp\" pointer; by the time we read\nthe flags, our \"pos\" index has advanced there. And we no longer need a\nseparate \"name\" pointer. After parsing the flags, \"pos\" points there\n(and we increment as expected if CE_EXTENDED asks us to read extra\nflags).\n\nWe can also drop the local \"hashsz\", as its value is used in only one\nplace now.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nI guess you could argue that dropping \"name\" reduces readability, as now\nwe just say \"ondisk + pos\". It is nice having only one position pointer,\nbut we lose the human-readable word \"name\" which tells what we expect\nthere. I dunno.\n\n read-cache.c | 53 ++++++++++++++++++++++------------------------------\n 1 file changed, 22 insertions(+), 31 deletions(-)\n\ndiff --git a/read-cache.c b/read-cache.c\nindex efb9efa5ee..a277836ee2 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -1884,11 +1884,9 @@ static struct cache_entry *create_from_disk(struct mem_pool *ce_mem_pool,\n \tstruct object_id oid;\n \tstruct cache_entry *ce;\n \tsize_t len;\n-\tconst char *name;\n-\tconst unsigned hashsz = the_hash_algo->rawsz;\n-\tconst char *flagsp;\n \tunsigned int flags;\n \tsize_t copy_len = 0;\n+\tsize_t pos = 0;\n \t/*\n \t * Adjacent cache entries tend to share the leading paths, so it makes\n \t * sense to only store the differences in later entries.  In the v4\n@@ -1898,42 +1896,35 @@ static struct cache_entry *create_from_disk(struct mem_pool *ce_mem_pool,\n \t */\n \tint expand_name_field = version == 4;\n \n-\tsd.sd_ctime.sec = get_be32(ondisk + offsetof(struct ondisk_cache_entry, ctime)\n-\t\t\t\t\t\t\t+ offsetof(struct cache_time, sec));\n-\tsd.sd_ctime.nsec = get_be32(ondisk + offsetof(struct ondisk_cache_entry, ctime)\n-\t\t\t\t\t\t\t + offsetof(struct cache_time, nsec));\n-\tsd.sd_mtime.sec = get_be32(ondisk + offsetof(struct ondisk_cache_entry, mtime)\n-\t\t\t\t\t\t\t+ offsetof(struct cache_time, sec));\n-\tsd.sd_mtime.nsec = get_be32(ondisk + offsetof(struct ondisk_cache_entry, mtime)\n-\t\t\t\t\t\t\t + offsetof(struct cache_time, nsec));\n-\tsd.sd_dev   = get_be32(ondisk + offsetof(struct ondisk_cache_entry, dev));\n-\tsd.sd_ino   = get_be32(ondisk + offsetof(struct ondisk_cache_entry, ino));\n-\tmode        = get_be32(ondisk + offsetof(struct ondisk_cache_entry, mode));\n-\tsd.sd_uid   = get_be32(ondisk + offsetof(struct ondisk_cache_entry, uid));\n-\tsd.sd_gid   = get_be32(ondisk + offsetof(struct ondisk_cache_entry, gid));\n-\tsd.sd_size  = get_be32(ondisk + offsetof(struct ondisk_cache_entry, size));\n-\n-\toidread(&oid, (const unsigned char *)ondisk + offsetof(struct ondisk_cache_entry, data));\n-\tflagsp = ondisk + offsetof(struct ondisk_cache_entry, data) + hashsz;\n+\tsd.sd_ctime.sec = read_be32(ondisk, &pos);\n+\tsd.sd_ctime.nsec = read_be32(ondisk, &pos);\n+\tsd.sd_mtime.sec = read_be32(ondisk, &pos);\n+\tsd.sd_mtime.nsec = read_be32(ondisk, &pos);\n+\tsd.sd_dev   = read_be32(ondisk, &pos);\n+\tsd.sd_ino   = read_be32(ondisk, &pos);\n+\tmode        = read_be32(ondisk, &pos);\n+\tsd.sd_uid   = read_be32(ondisk, &pos);\n+\tsd.sd_gid   = read_be32(ondisk, &pos);\n+\tsd.sd_size  = read_be32(ondisk, &pos);\n+\n+\toidread(&oid, (const unsigned char *)ondisk + pos);\n+\tpos += the_hash_algo->rawsz;\n \n \t/* On-disk flags are just 16 bits */\n-\tflags = get_be16(flagsp);\n+\tflags = read_be16(ondisk, &pos);\n \tlen = flags & CE_NAMEMASK;\n \n \tif (flags & CE_EXTENDED) {\n \t\tint extended_flags;\n-\t\textended_flags = get_be16(flagsp + sizeof(uint16_t)) << 16;\n+\t\textended_flags = read_be16(ondisk, &pos) << 16;\n \t\t/* We do not yet understand any bit out of CE_EXTENDED_FLAGS */\n \t\tif (extended_flags & ~CE_EXTENDED_FLAGS)\n \t\t\tdie(_(\"unknown index entry format 0x%08x\"), extended_flags);\n \t\tflags |= extended_flags;\n-\t\tname = (const char *)(flagsp + 2 * sizeof(uint16_t));\n \t}\n-\telse\n-\t\tname = (const char *)(flagsp + sizeof(uint16_t));\n \n \tif (expand_name_field) {\n-\t\tconst unsigned char *cp = (const unsigned char *)name;\n+\t\tconst unsigned char *cp = (const unsigned char *)ondisk + pos;\n \t\tsize_t strip_len, previous_len;\n \n \t\t/* If we're at the beginning of a block, ignore the previous name */\n@@ -1945,11 +1936,11 @@ static struct cache_entry *create_from_disk(struct mem_pool *ce_mem_pool,\n \t\t\t\t\tprevious_ce->name);\n \t\t\tcopy_len = previous_len - strip_len;\n \t\t}\n-\t\tname = (const char *)cp;\n+\t\tpos = (const char *)cp - ondisk;\n \t}\n \n \tif (len == CE_NAMEMASK) {\n-\t\tlen = strlen(name);\n+\t\tlen = strlen(ondisk + pos);\n \t\tif (expand_name_field)\n \t\t\tlen += copy_len;\n \t}\n@@ -1965,10 +1956,10 @@ static struct cache_entry *create_from_disk(struct mem_pool *ce_mem_pool,\n \tif (expand_name_field) {\n \t\tif (copy_len)\n \t\t\tmemcpy(ce->name, previous_ce->name, copy_len);\n-\t\tmemcpy(ce->name + copy_len, name, len + 1 - copy_len);\n-\t\t*ent_size = (name - ((char *)ondisk)) + len + 1 - copy_len;\n+\t\tmemcpy(ce->name + copy_len, ondisk + pos, len + 1 - copy_len);\n+\t\t*ent_size = pos + len + 1 - copy_len;\n \t} else {\n-\t\tmemcpy(ce->name, name, len + 1);\n+\t\tmemcpy(ce->name, ondisk + pos, len + 1);\n \t\t*ent_size = ondisk_ce_size(ce);\n \t}\n \treturn ce;\n-- \n2.38.0.rc2.615.g4fac75f9e3\n"},{"id":"463855","messageId":"xmqqv8p7xxi3.fsf@gitster.g","threadId":"58521","inReplyTo":"YzPLBN09zzlTdNgc@coredump.intra.peff.net","subject":"Re: vd/fix-unaligned-read-index-v4, was Re: What's cooking in git.git (Sep 2022, #08; Tue, 27)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-09-28T16:41:08Z","receivedAt":"2022-09-28T16:41:22Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> ... I have no objection to adding more comments, but I am\n> happy enough without them (like Junio, it may be that I'm overly\n> familiar with how I expect our get_be() functions to handle alignment).\n> ...\n> So it's mostly just a minor annoyance for running the tests; we're\n> probably better not to change any code, even trivially, this late in the\n> release cycle.\n\nYup.  I never planned to merge the topic to 'master'.  The finishing\ntouch I expected was to help Phillip and friends with a bit of\nexplanation in the log message, and then it would be ready to wait\nin 'next' for the next cycle.\n"},{"id":"463856","messageId":"220928.86h70rwhxh.gmgdl@evledraar.gmail.com","threadId":"58521","inReplyTo":"xmqqv8p7xxi3.fsf@gitster.g","subject":"Re: vd/fix-unaligned-read-index-v4, was Re: What's cooking in git.git (Sep 2022, #08; Tue, 27)","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-09-28T17:01:59Z","receivedAt":"2022-09-28T17:03:00Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Sep 28 2022, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n>\n>> ... I have no objection to adding more comments, but I am\n>> happy enough without them (like Junio, it may be that I'm overly\n>> familiar with how I expect our get_be() functions to handle alignment).\n>> ...\n>> So it's mostly just a minor annoyance for running the tests; we're\n>> probably better not to change any code, even trivially, this late in the\n>> release cycle.\n>\n> Yup.  I never planned to merge the topic to 'master'.  The finishing\n> touch I expected was to help Phillip and friends with a bit of\n> explanation in the log message, and then it would be ready to wait\n> in 'next' for the next cycle.\n\nIn the interim are we interested in a minimal patch to the specific\nscalar test that's finding this under SANITIZE=undefined, as running\nun-cleanly will be new in this release?\n"},{"id":"463863","messageId":"xmqqa66jv1kb.fsf@gitster.g","threadId":"58521","inReplyTo":"220928.86h70rwhxh.gmgdl@evledraar.gmail.com","subject":"Re: vd/fix-unaligned-read-index-v4, was Re: What's cooking in git.git (Sep 2022, #08; Tue, 27)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-09-28T17:41:40Z","receivedAt":"2022-09-28T17:41:50Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n\n> On Wed, Sep 28 2022, Junio C Hamano wrote:\n>\n>> Jeff King <peff@peff.net> writes:\n>>\n>>> ... I have no objection to adding more comments, but I am\n>>> happy enough without them (like Junio, it may be that I'm overly\n>>> familiar with how I expect our get_be() functions to handle alignment).\n>>> ...\n>>> So it's mostly just a minor annoyance for running the tests; we're\n>>> probably better not to change any code, even trivially, this late in the\n>>> release cycle.\n>>\n>> Yup.  I never planned to merge the topic to 'master'.  The finishing\n>> touch I expected was to help Phillip and friends with a bit of\n>> explanation in the log message, and then it would be ready to wait\n>> in 'next' for the next cycle.\n>\n> In the interim are we interested in a minimal patch to the specific\n> scalar test that's finding this under SANITIZE=undefined, as running\n> un-cleanly will be new in this release?\n\nYes, it is a new minor annoyance that is better left this late in\nthe cycle.\n"},{"id":"463900","messageId":"YzWBEoRkCmbUkIs8@coredump.intra.peff.net","threadId":"58521","inReplyTo":"YzPLuizoOlDuPu4G@coredump.intra.peff.net","subject":"Re: [PATCH 2/3] read-cache: read on-disk entries in byte order","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2022-09-29T11:27:14Z","receivedAt":"2022-09-29T11:27:21Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Sep 28, 2022 at 12:21:15AM -0400, Jeff King wrote:\n\n> The downside is that we're copying the data an extra time. It's not very\n> much data, and it's all fixed size, so the compiler should be able to do\n> a reasonable job of optimizing here. But I didn't time the potential\n> impact.\n\nI timed this using \"test-tool read-cache\". It's kind of an artificial\nbenchmark, but it does isolate the code we're touching here. The results\nare...not good. Here's the time to read the index of linux.git 1000\ntimes, before and after this reordering patch:\n\n  Benchmark 1: ./test-tool.old read-cache 1000\n    Time (mean ± σ):      2.870 s ±  0.073 s    [User: 2.555 s, System: 0.315 s]\n    Range (min … max):    2.789 s …  3.001 s    10 runs\n  \n  Benchmark 2: ./test-tool.new read-cache 1000\n    Time (mean ± σ):      3.180 s ±  0.080 s    [User: 2.849 s, System: 0.331 s]\n    Range (min … max):    3.092 s …  3.297 s    10 runs\n  \n  Summary\n    './test-tool.old read-cache 1000' ran\n      1.11 ± 0.04 times faster than './test-tool.new read-cache 1000'\n\nI think that's probably the nail in the coffin for my proposed approach.\nTo be fair, it's only .3ms extra for a normal program which reads the\nindex once. That's not that big in absolute numbers. But there are\nlarger index files in the wild. And the improvement in simplicity and\nreadability is simply not that great.\n\n-Peff\n"},{"id":"463902","messageId":"xmqqczbep4hf.fsf@gitster.g","threadId":"58521","inReplyTo":"YzWBEoRkCmbUkIs8@coredump.intra.peff.net","subject":"Re: [PATCH 2/3] read-cache: read on-disk entries in byte order","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-09-29T15:47:24Z","receivedAt":"2022-09-29T15:48:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Wed, Sep 28, 2022 at 12:21:15AM -0400, Jeff King wrote:\n>\n>> The downside is that we're copying the data an extra time. It's not very\n>> much data, and it's all fixed size, so the compiler should be able to do\n>> a reasonable job of optimizing here. But I didn't time the potential\n>> impact.\n> ...\n>   Summary\n>     './test-tool.old read-cache 1000' ran\n>       1.11 ± 0.04 times faster than './test-tool.new read-cache 1000'\n>\n> I think that's probably the nail in the coffin for my proposed approach.\n> To be fair, it's only .3ms extra for a normal program which reads the\n> index once. That's not that big in absolute numbers. But there are\n> larger index files in the wild. And the improvement in simplicity and\n> readability is simply not that great.\n\nThanks for the due diligence.  This result may be an indication of\nhow efficient the existing code is, but it is a bit surprising that\none more copy of the stat_data struct makes that much difference.\n\n"}]}