What's cooking in git.git (Dec 2018, #02; Fri, 28)

What's cooking in git.git (Dec 2018, #02; Fri, 28)

[Junio C Hamano]

Here are the topics that have been cooking.  Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'.  The ones marked with '.' do not appear in any of
the integration branches, but I am still holding onto them.

The current state is that 'next' has been rewound, a few topics have
been updated, and almost no new topics have been picked up (yet),
even though many of them seem to have been reviewed on-list and I've
also looked at some of them.

Sorry for being slow to recover from the year-end slowness; the 2.20
release being rather large-ish and needed brown-paper-bag fixes soon
after it, my temporary trans-pacific migration, or me catching cold
that I haven't shaken off, did not help X-<.

You can find the changes described here in the integration branches
of the repositories listed at

    http://git-blame.blogspot.com/p/git-public-repositories.html

--------------------------------------------------
[Graduated to "master"]

* ds/hash-independent-tests-fix (2018-12-12) 1 commit
 + .gitattributes: ensure t/oid-info/* has eol=lf

 Test portability fix.


* jc/run-command-report-exec-failure-fix (2018-12-12) 1 commit
 + run-command: report exec failure

 A recent update accidentally squelched an error message when the
 run_command API failed to run a missing command, which has been
 corrected.


* js/help-commands-verbose-by-default-fix (2018-12-12) 2 commits
 + help -a: handle aliases with long names gracefully
 + help.h: fix coding style

 "git help -a" did not work well when an overly long alias is
 defined, which has been corrected.


* js/mailinfo-format-flowed-fix (2018-12-13) 1 commit
 + t4256: mark support files as LF-only

 Test portability fix.


* nd/show-gitcomp-compilation-fix (2018-12-12) 1 commit
 + parse-options: fix SunCC compiler warning

 Portability fix for a recent update to parse-options API.

--------------------------------------------------
[New Topics]

* ds/midx-expire-repack (2018-12-11) 5 commits
 - multi-pack-index: implement midx_repack()
 - multi-pack-index: prepare 'repack' verb
 - multi-pack-index: implement 'expire' verb
 - multi-pack-index: refactor permutation logic
 - multi-pack-index: prepare for 'expire' verb


* ds/push-sparse-tree-walk (2018-12-11) 6 commits
 - pack-objects: create GIT_TEST_PACK_SPARSE
 - pack-objects: create pack.useSparse setting
 - revision: implement sparse algorithm
 - pack-objects: add --sparse option
 - list-objects: consume sparse tree walk
 - revision: add mark_tree_uninteresting_sparse


* js/rebase-i-redo-exec (2018-12-11) 3 commits
 - rebase: introduce a shortcut for --reschedule-failed-exec
 - rebase: add a config option to default to --reschedule-failed-exec
 - rebase: introduce --reschedule-failed-exec


* md/list-objects-filter-by-depth (2018-12-11) 4 commits
 - tree:<depth>: skip some trees even when collecting omits
 - list-objects-filter: teach tree:# how to handle >0
 - Merge branch 'nd/the-index' into md/list-objects-filter-by-depth
 - Merge branch 'sb/more-repo-in-api' into md/list-objects-filter-by-depth
 (this branch uses nd/the-index and sb/more-repo-in-api; is tangled with jt/get-reference-with-commit-graph.)

 Needs to be rebuilt when sb/more-repo-in-api is rewound.


* nd/backup-log (2018-12-10) 24 commits
 - FIXME
 - rebase: keep backup of overwritten files on --skip or --abort
 - am: keep backup of overwritten files on --skip or --abort
 - checkout -f: keep backup of overwritten files
 - reset --hard: keep backup of overwritten files
 - unpack-trees.c: keep backup of ignored files being overwritten
 - refs: keep backup of deleted reflog
 - config --edit: support backup log
 - sha1-file.c: let index_path() accept NULL istate
 - backup-log: keep all blob references around
 - gc: prune backup logs
 - backup-log: add prune command
 - backup-log: add log command
 - backup-log: add diff command
 - backup-log: add cat command
 - backup-log.c: add API for walking backup log
 - add--interactive: support backup log
 - apply: support backup log with --keep-backup
 - commit: support backup log
 - update-index: support backup log with --keep-backup
 - add: support backup log
 - read-cache.c: new flag for add_index_entry() to write to backup log
 - backup-log: add "update" subcommand
 - doc: introduce new "backup log" concept


* nd/style-opening-brace (2018-12-10) 1 commit
 - style: the opening '{' of a function is in a separate line

 Code clean-up.


* sg/stress-test (2018-12-10) 8 commits
 - fixup! test-lib: add the '--stress' option to run a test repeatedly under load
 - test-lib: add the '--stress' option to run a test repeatedly under load
 - test-lib-functions: introduce the 'test_set_port' helper function
 - test-lib: extract Bash version check for '-x' tracing
 - test-lib: set $TRASH_DIRECTORY earlier
 - test-lib: consolidate naming of test-results paths
 - test-lib: parse some --options earlier
 - test-lib: translate SIGTERM and SIGHUP to an exit


* tg/checkout-no-overlay (2018-12-10) 8 commits
 - stash: use git checkout --no-overlay
 - checkout: allow ignoring unmatched pathspec
 - checkout: add --cached option
 - checkout: introduce --{,no-}overlay option
 - read-cache: add invalidate parameter to remove_marked_cache_entries
 - entry: support CE_WT_REMOVE flag in checkout_entry
 - entry: factor out unlink_entry function
 - move worktree tests to t24*


* jk/proto-v2-hidden-refs-fix (2018-12-14) 3 commits
 - upload-pack: support hidden refs with protocol v2
 - parse_hide_refs_config: handle NULL section
 - serve: pass "config context" through to individual commands


* la/quiltimport-keep-non-patch (2018-12-14) 1 commit
 - git-quiltimport: Add --keep-non-patch option


* sb/submodule-fetchjobs-default-to-one (2018-12-14) 1 commit
 - submodule update: run at most one fetch job unless otherwise set

--------------------------------------------------
[Stalled]

* lt/date-human (2018-07-09) 1 commit
 - Add 'human' date format

 A new date format "--date=human" that morphs its output depending
 on how far the time is from the current time has been introduced.
 "--date=auto" can be used to use this new format when the output is
 goint to the pager or to the terminal and otherwise the default
 format.

--------------------------------------------------
[Cooking]

* cb/openbsd-allows-reading-directory (2018-12-03) 1 commit
 - config.mak.uname: OpenBSD uses BSD semantics with fread for directories

 BSD port update.

 Will merge to 'next'.


* cb/t5004-empty-tar-archive-fix (2018-12-03) 1 commit
 - t5004: avoid using tar for empty packages

 BSD port update.

 Will merge to 'next'.


* cb/test-lint-cp-a (2018-12-03) 1 commit
 - tests: add lint for non portable cp -a

 BSD port update.

 Will merge to 'next'.


* hb/t0061-dot-in-path-fix (2018-12-03) 1 commit
 - t0061: do not fail test if '.' is part of $PATH

 Test update.

 Will merge to 'next'.


* hn/highlight-sideband-keywords (2018-12-04) 1 commit
 - sideband: color lines with keyword only

 Lines that begin with a certain keyword that come over the wire, as
 well as lines that consist only of one of these keywords, ought to
 be painted in color for easier eyeballing, but the latter was
 broken ever since the feature was introduced in 2.19, which has
 been corrected.

 Will merge to 'next'.


* js/commit-graph-chunk-table-fix (2018-12-14) 3 commits
 - Makefile: correct example fuzz build
 - commit-graph: fix buffer read-overflow
 - commit-graph, fuzz: add fuzzer for commit-graph

 The codepath to read from the commit-graph file attempted to read
 past the end of it when the file's table-of-contents was corrupt.


* jt/get-reference-with-commit-graph (2018-12-06) 1 commit
 - revision: use commit graph in get_reference()
 (this branch uses sb/more-repo-in-api; is tangled with md/list-objects-filter-by-depth.)

 Micro-optimize the code that prepares commit objects to be walked
 by "git rev-list" when the commit-graph is available.

 Needs to be rebuilt when sb/more-repo-in-api is rewound.


* md/exclude-promisor-objects-fix-cleanup (2018-12-06) 1 commit
 - revision.c: put promisor option in specialized struct

 Code clean-up.

 Will merge to 'next'.


* bw/mailmap (2018-12-09) 1 commit
 - mailmap: update brandon williams's email address

 Will merge to 'next'.


* do/gitweb-strict-export-conf-doc (2018-12-09) 1 commit
 - docs: fix $strict_export text in gitweb.conf.txt

 Doc update.

 Will merge to 'next'.


* en/directory-renames-nothanks-doc-update (2018-12-09) 1 commit
 - git-rebase.txt: update note about directory rename detection and am

 Doc update.

 Will merge to 'next'.


* fd/gitweb-snapshot-conf-doc-fix (2018-12-09) 1 commit
 - docs/gitweb.conf: config variable typo

 Doc update.

 Will merge to 'next'.


* km/rebase-doc-typofix (2018-12-10) 1 commit
 - rebase docs: drop stray word in merge command description

 Doc update.

 Will merge to 'next'.


* nd/indentation-fix (2018-12-09) 1 commit
 - Indent code with TABs

 Code cleanup.

 Will merge to 'next'.


* tb/use-common-win32-pathfuncs-on-cygwin (2018-12-26) 1 commit
 - git clone <url> C:\cygwin\home\USER\repo' is working (again)

 Cygwin update.

 Will merge to 'next'.


* mk/http-backend-kill-children-before-exit (2018-11-26) 1 commit
  (merged to 'next' on 2018-12-28 at 81188d93c3)
 + http-backend: enable cleaning up forked upload/receive-pack on exit

 Originally merged to 'next' on 2018-11-29

 The http-backend CGI process did not correctly clean up the child
 processes it spawns to run upload-pack etc. when it dies itself,
 which has been corrected.

 Will merge to 'master'.


* tb/log-G-binary (2018-12-26) 1 commit
 - log -G: ignore binary files

 "git log -G<regex>" looked for a hunk in the "git log -p" patch
 output that contained a string that matches the given pattern.
 Optimize this code to ignore binary files, which by default will
 not show any hunk that would match any pattern (unless textconv or
 the --text option is in effect, that is).

 Will merge to 'next'.


* dl/merge-cleanup-scissors-fix (2018-11-21) 2 commits
 - merge: add scissors line on merge conflict
 - t7600: clean up 'merge --squash c3 with c7' test

 The list of conflicted paths shown in the editor while concluding a
 conflicted merge was shown above the scissors line when the
 clean-up mode is set to "scissors", even though it was commented
 out just like the list of updated paths and other information to
 help the user explain the merge better.

 Kicked out of 'next', to replace with a newer iteration.
 cf. <cover.1545745331.git.liu.denton@gmail.com>


* aw/pretty-trailers (2018-12-09) 7 commits
 - pretty: add support for separator option in %(trailers)
 - strbuf: separate callback for strbuf_expand:ing literals
 - pretty: add support for "valueonly" option in %(trailers)
 - pretty: allow showing specific trailers
 - pretty: single return path in %(trailers) handling
 - pretty: allow %(trailers) options with explicit value
 - doc: group pretty-format.txt placeholders descriptions

 The %(trailers) formatter in "git log --format=..."  now allows to
 optionally pick trailers selectively by keyword, show only values,
 etc.

 How's the doneness of this one?


* nd/attr-pathspec-in-tree-walk (2018-11-19) 5 commits
 - tree-walk: support :(attr) matching
 - dir.c: move, rename and export match_attrs()
 - pathspec.h: clean up "extern" in function declarations
 - tree-walk.c: make tree_entry_interesting() take an index
 - tree.c: make read_tree*() take 'struct repository *'

 The traversal over tree objects has learned to honor
 ":(attr:label)" pathspec match, which has been implemented only for
 enumerating paths on the filesystem.

 Will merge to 'next'.


* ab/commit-graph-progress-fix (2018-11-20) 1 commit
 - commit-graph: split up close_reachable() progress output

 Will merge to 'next'.


* jn/unknown-index-extensions (2018-11-21) 2 commits
 - index: offer advice for unknown index extensions
 - index: do not warn about unrecognized extensions

 A bit too alarming warning given when unknown index extensions
 exist is getting revamped.

 Expecting a reroll.


* en/fast-export-import (2018-11-17) 11 commits
  (merged to 'next' on 2018-12-28 at a1b09cf515)
 + fast-export: add a --show-original-ids option to show original names
 + fast-import: remove unmaintained duplicate documentation
 + fast-export: add --reference-excluded-parents option
 + fast-export: ensure we export requested refs
 + fast-export: when using paths, avoid corrupt stream with non-existent mark
 + fast-export: move commit rewriting logic into a function for reuse
 + fast-export: avoid dying when filtering by paths and old tags exist
 + fast-export: use value from correct enum
 + git-fast-export.txt: clarify misleading documentation about rev-list args
 + git-fast-import.txt: fix documentation for --quiet option
 + fast-export: convert sha1 to oid

 Originally merged to 'next' on 2018-11-18

 Small fixes and features for fast-export and fast-import, mostly on
 the fast-export side.

 Will merge to 'master'.


* nd/checkout-dwim-fix (2018-11-14) 1 commit
  (merged to 'next' on 2018-12-28 at 3183c9305b)
 + checkout: disambiguate dwim tracking branches and local files

 Originally merged to 'next' on 2018-11-18

 "git checkout frotz" (without any double-dash) avoids ambiguity by
 making sure 'frotz' cannot be interpreted as a revision and as a
 path at the same time.  This safety has been updated to check also
 a unique remote-tracking branch 'frotz' in a remote, when dwimming
 to create a local branch 'frotz' out of a remote-tracking branch
 'frotz' from a remote.

 Will merge to 'master'.


* nd/checkout-noisy (2018-11-20) 2 commits
 - t0027: squelch checkout path run outside test_expect_* block
 - checkout: print something when checking out paths

 "git checkout [<tree-ish>] path..." learned to report the number of
 paths that have been checked out of the index or the tree-ish,
 which gives it the same degree of noisy-ness as the case in which
 the command checks out a branch.

 Will merge to 'next'.


* sg/clone-initial-fetch-configuration (2018-11-16) 3 commits
  (merged to 'next' on 2018-12-28 at 82e104f221)
 + Documentation/clone: document ignored configuration variables
 + clone: respect additional configured fetch refspecs during initial fetch
 + clone: use a more appropriate variable name for the default refspec

 Originally merged to 'next' on 2018-11-18

 Refspecs configured with "git -c var=val clone" did not propagate
 to the resulting repository, which has been corrected.

 Will merge to 'master'.


* en/rebase-merge-on-sequencer (2018-11-08) 2 commits
 - rebase: implement --merge via git-rebase--interactive
 - git-rebase, sequencer: extend --quiet option for the interactive machinery

 "git rebase --merge" as been reimplemented by reusing the internal
 machinery used for "git rebase -i".

 Expecting a reroll.
 cf. <CABPp-BF8RupyfP69iqAVTXxEhBGyzVd-wUgp3y0pf+CbBFAQeg@mail.gmail.com>


* fc/http-version (2018-11-09) 1 commit
  (merged to 'next' on 2018-12-28 at 56bcbb0fa9)
 + http: add support selecting http version

 Originally merged to 'next' on 2018-11-18

 The "http.version" configuration variable can be used with recent
 enough cURL library to force the version of HTTP used to talk when
 fetching and pushing.

 Will merge to 'master'.


* dl/remote-save-to-push (2018-12-11) 1 commit
 - remote: add --save-to-push option to git remote set-url

 "git remote set-url" learned a new option that moves existing value
 of the URL field to pushURL field of the remote before replacing
 the URL field with a new value.

 I am personally not yet quite convinced if this is worth pursuing.


* jk/loose-object-cache (2018-11-24) 10 commits
  (merged to 'next' on 2018-12-28 at 5a5faf384e)
 + odb_load_loose_cache: fix strbuf leak
 + fetch-pack: drop custom loose object cache
 + sha1-file: use loose object cache for quick existence check
 + object-store: provide helpers for loose_objects_cache
 + sha1-file: use an object_directory for the main object dir
 + handle alternates paths the same as the main object dir
 + sha1_file_name(): overwrite buffer instead of appending
 + rename "alternate_object_database" to "object_directory"
 + submodule--helper: prefer strip_suffix() to ends_with()
 + fsck: do not reuse child_process structs

 Originally merged to 'next' on 2018-11-24

 Code clean-up with optimization for the codepath that checks
 (non-)existence of loose objects.

 Will merge to 'master'.


* js/protocol-advertise-multi (2018-11-17) 1 commit
 - protocol: advertise multiple supported versions

 The transport layer has been updated so that the protocol version
 used can be negotiated between the parties, by the initiator
 listing the protocol versions it is willing to talk, and the other
 side choosing from one of them.

 Expecting a reroll.
 cf. <20181218230514.GH37614@google.com>


* js/smart-http-detect-remote-error (2018-12-12) 4 commits
 - lib-httpd, t5551: check server-side HTTP errors
 - remote-curl: tighten "version 2" check for smart-http
 - remote-curl: refactor smart-http discovery
 - pack-protocol.txt: accept error packets in any context

 Some errors from the other side coming over smart HTTP transport
 were not noticed, which has been corrected.

 Will merge to 'next'.


* nb/branch-show-other-worktrees-head (2018-11-12) 2 commits
 - branch: mark and colorize a branch differently if it is checked out in a linked worktree
 - ref-filter: add worktree atom

 "git branch --list" learned to show branches that are checked out
 in other worktrees connected to the same repository prefixed with
 '+', similar to the way the currently checked out branch is shown
 with '*' in front.

 Expecting a reroll.


* nd/the-index (2018-11-12) 22 commits
  (merged to 'next' on 2018-12-28 at 6bbd3befbe)
 + rebase-interactive.c: remove the_repository references
 + rerere.c: remove the_repository references
 + pack-*.c: remove the_repository references
 + pack-check.c: remove the_repository references
 + notes-cache.c: remove the_repository references
 + line-log.c: remove the_repository reference
 + diff-lib.c: remove the_repository references
 + delta-islands.c: remove the_repository references
 + cache-tree.c: remove the_repository references
 + bundle.c: remove the_repository references
 + branch.c: remove the_repository reference
 + bisect.c: remove the_repository reference
 + blame.c: remove implicit dependency the_repository
 + sequencer.c: remove implicit dependency on the_repository
 + sequencer.c: remove implicit dependency on the_index
 + transport.c: remove implicit dependency on the_index
 + notes-merge.c: remove implicit dependency the_repository
 + notes-merge.c: remove implicit dependency on the_index
 + list-objects.c: reduce the_repository references
 + list-objects-filter.c: remove implicit dependency on the_index
 + wt-status.c: remove implicit dependency the_repository
 + wt-status.c: remove implicit dependency on the_index
 (this branch is used by md/list-objects-filter-by-depth.)

 Originally merged to 'next' on 2018-11-18

 More codepaths become aware of working with in-core repository
 instance other than the default "the_repository".

 Will merge to 'master'.


* ot/ref-filter-object-info (2018-11-24) 6 commits
 - ref-filter: replace unportable `%lld` format with %PRIdMAX
 - ref-filter: add docs for new options
 - ref-filter: add tests for deltabase
 - ref-filter: add deltabase option
 - ref-filter: add tests for objectsize:disk
 - ref-filter: add objectsize:disk option

 The "--format=<placeholder>" option of for-each-ref, branch and tag
 learned to show a few more traits of objects that can be learned by
 the object_info API.

 Kicked out of 'next'; will replace with a reroll.
 cf. <01020167e063687c-37a43a09-0a5f-4335-8c21-ec15a0a67882-000000@eu-west-1.amazonses.com>


* sb/diff-color-moved-config-option-fixup (2018-11-14) 1 commit
 - diff: align move detection error handling with other options

 Minor inconsistency fix.

 Will merge to 'next'.


* ab/push-dwim-dst (2018-11-14) 7 commits
  (merged to 'next' on 2018-12-28 at d9f618de10)
 + push doc: document the DWYM behavior pushing to unqualified <dst>
 + push: test that <src> doesn't DWYM if <dst> is unqualified
 + push: add an advice on unqualified <dst> push
 + push: move unqualified refname error into a function
 + push: improve the error shown on unqualified <dst> push
 + i18n: remote.c: mark error(...) messages for translation
 + remote.c: add braces in anticipation of a follow-up change

 Originally merged to 'next' on 2018-11-18

 "git push $there $src:$dst" rejects when $dst is not a fully
 qualified refname and not clear what the end user meant.  The
 codepath has been taught to give a clearer error message, and also
 guess where the push should go by taking the type of the pushed
 object into account (e.g. a tag object would want to go under
 refs/tags/).

 Will merge to 'master'.


* md/list-lazy-objects-fix (2018-12-06) 1 commit
 - list-objects.c: don't segfault for missing cmdline objects

 "git rev-list --exclude-promissor-objects" had to take an object
 that does not exist locally (and is lazily available) from the
 command line without barfing, but the code dereferenced NULL.

 Will merge to 'next'.


* nd/i18n (2018-11-12) 16 commits
  (merged to 'next' on 2018-12-28 at 8e2de8338e)
 + fsck: mark strings for translation
 + fsck: reduce word legos to help i18n
 + parse-options.c: mark more strings for translation
 + parse-options.c: turn some die() to BUG()
 + parse-options: replace opterror() with optname()
 + repack: mark more strings for translation
 + remote.c: mark messages for translation
 + remote.c: turn some error() or die() to BUG()
 + reflog: mark strings for translation
 + read-cache.c: add missing colon separators
 + read-cache.c: mark more strings for translation
 + read-cache.c: turn die("internal error") to BUG()
 + attr.c: mark more string for translation
 + archive.c: mark more strings for translation
 + alias.c: mark split_cmdline_strerror() strings for translation
 + git.c: mark more strings for translation

 Originally merged to 'next' on 2018-11-18

 More _("i18n") markings.

 Will merge to 'master'.


* sb/more-repo-in-api (2018-11-14) 23 commits
 - t/helper/test-repository: celebrate independence from the_repository
 - path.h: make REPO_GIT_PATH_FUNC repository agnostic
 - commit: prepare free_commit_buffer and release_commit_memory for any repo
 - commit-graph: convert remaining functions to handle any repo
 - submodule: don't add submodule as odb for push
 - submodule: use submodule repos for object lookup
 - pretty: prepare format_commit_message to handle arbitrary repositories
 - commit: prepare logmsg_reencode to handle arbitrary repositories
 - commit: prepare repo_unuse_commit_buffer to handle any repo
 - commit: prepare get_commit_buffer to handle any repo
 - commit-reach: prepare in_merge_bases[_many] to handle any repo
 - commit-reach: prepare get_merge_bases to handle any repo
 - commit-reach.c: allow get_merge_bases_many_0 to handle any repo
 - commit-reach.c: allow remove_redundant to handle any repo
 - commit-reach.c: allow merge_bases_many to handle any repo
 - commit-reach.c: allow paint_down_to_common to handle any repo
 - commit: allow parse_commit* to handle any repo
 - object: parse_object to honor its repository argument
 - object-store: prepare has_{sha1, object}_file to handle any repo
 - object-store: prepare read_object_file to deal with any repo
 - object-store: allow read_object_file_extended to read from any repo
 - packfile: allow has_packed_and_bad to handle arbitrary repositories
 - sha1_file: allow read_object to read objects in arbitrary repositories
 (this branch is used by jt/get-reference-with-commit-graph and md/list-objects-filter-by-depth.)

 Originally merged to 'next' on 2018-11-19

 The in-core repository instances are passed through more codepaths.

 Kicked out of 'next'; will replace with a reroll.
 cf. <20181215000942.46033-2-sbeller@google.com>


* en/merge-path-collision (2018-12-01) 11 commits
  (merged to 'next' on 2018-12-28 at b50d3eee25)
 + t6036: avoid non-portable "cp -a"
 + merge-recursive: combine error handling
 + t6036, t6043: increase code coverage for file collision handling
 + merge-recursive: improve rename/rename(1to2)/add[/add] handling
 + merge-recursive: use handle_file_collision for add/add conflicts
 + merge-recursive: improve handling for rename/rename(2to1) conflicts
 + merge-recursive: fix rename/add conflict handling
 + merge-recursive: new function for better colliding conflict resolutions
 + merge-recursive: increase marker length with depth of recursion
 + t6036, t6042: testcases for rename collision of already conflicting files
 + t6042: add tests for consistency in file collision conflict handling

 Originally merged to 'next' on 2018-12-01

 Updates for corner cases in merge-recursive.

 Will merge to 'master'.


* sd/stash-wo-user-name (2018-11-19) 1 commit
  (merged to 'next' on 2018-12-28 at 99197ef5a1)
 + stash: tolerate missing user identity

 Originally merged to 'next' on 2018-11-19

 A properly configured username/email is required under
 user.useConfigOnly in order to create commits; now "git stash"
 (even though it creates commit objects to represent stash entries)
 command is excempt from the requirement.

 Will merge to 'master'.


* bc/sha-256 (2018-11-14) 12 commits
 - hash: add an SHA-256 implementation using OpenSSL
 - sha256: add an SHA-256 implementation using libgcrypt
 - Add a base implementation of SHA-256 support
 - commit-graph: convert to using the_hash_algo
 - t/helper: add a test helper to compute hash speed
 - sha1-file: add a constant for hash block size
 - t: make the sha1 test-tool helper generic
 - t: add basic tests for our SHA-1 implementation
 - cache: make hashcmp and hasheq work with larger hashes
 - hex: introduce functions to print arbitrary hashes
 - sha1-file: provide functions to look up hash algorithms
 - sha1-file: rename algorithm to "sha1"

 Add sha-256 hash and plug it through the code to allow building Git
 with the "NewHash".


* js/vsts-ci (2018-10-16) 13 commits
 . travis: fix skipping tagged releases
 . README: add a build badge (status of the Azure Pipelines build)
 . tests: record more stderr with --write-junit-xml in case of failure
 . tests: include detailed trace logs with --write-junit-xml upon failure
 . git-p4: use `test_atexit` to kill the daemon
 . git-daemon: use `test_atexit` in the tests
 . tests: introduce `test_atexit`
 . ci: add a build definition for Azure DevOps
 . ci/lib.sh: add support for Azure Pipelines
 . tests: optionally write results as JUnit-style .xml
 . test-date: add a subcommand to measure times in shell scripts
 . ci/lib.sh: encapsulate Travis-specific things
 . ci: rename the library of common functions

 Prepare to run test suite on Azure DevOps.

 Ejected out of 'pu', as doing so seems to help other topics get
 tested at TravisCI.

 https://travis-ci.org/git/git/builds/452713184 is a sample of a
 build whose tests on 4 hang (with this series in).  Ejecting it
 gave us https://travis-ci.org/git/git/builds/452778963 which still
 shows breakages from other topics not yet in 'next', but at least
 the tests do not stall.


* du/branch-show-current (2018-10-26) 1 commit
 - branch: introduce --show-current display option

 "git branch" learned a new subcommand "--show-current".

 I am personally not yet quite convinced if this is worth pursuing.


* mk/use-size-t-in-zlib (2018-10-15) 1 commit
 - zlib.c: use size_t for size

 The wrapper to call into zlib followed our long tradition to use
 "unsigned long" for sizes of regions in memory, which have been
 updated to use "size_t".


* ag/sequencer-reduce-rewriting-todo (2018-11-12) 16 commits
 . rebase--interactive: move transform_todo_file() to rebase--interactive.c
 . sequencer: fix a call to error() in transform_todo_file()
 . sequencer: use edit_todo_list() in complete_action()
 . rebase-interactive: rewrite edit_todo_list() to handle the initial edit
 . rebase-interactive: append_todo_help() changes
 . rebase-interactive: use todo_list_write_to_file() in edit_todo_list()
 . sequencer: refactor skip_unnecessary_picks() to work on a todo_list
 . sequencer: change complete_action() to use the refactored functions
 . sequencer: make sequencer_make_script() write its script to a strbuf
 . sequencer: refactor rearrange_squash() to work on a todo_list
 . sequencer: refactor sequencer_add_exec_commands() to work on a todo_list
 . sequencer: refactor check_todo_list() to work on a todo_list
 . sequencer: introduce todo_list_write_to_file()
 . sequencer: refactor transform_todos() to work on a todo_list
 . sequencer: make the todo_list structure public
 . sequencer: changes in parse_insn_buffer()

 The scripted version of "git rebase -i" wrote and rewrote the todo
 list many times during a single step of its operation, and the
 recent C-rewrite made a faithful conversion of the logic to C.  The
 implementation has been updated to carry necessary information
 around in-core to avoid rewriting the same file over and over
 unnecessarily.

 With too many topics in-flight that touch sequencer and rebaser,
 this need to wait giving precedence to other topics that fix bugs.


* sb/submodule-recursive-fetch-gets-the-tip (2018-12-09) 9 commits
 - fetch: ensure submodule objects fetched
 - submodule.c: fetch in submodules git directory instead of in worktree
 - submodule: migrate get_next_submodule to use repository structs
 - repository: repo_submodule_init to take a submodule struct
 - submodule: store OIDs in changed_submodule_names
 - submodule.c: tighten scope of changed_submodule_names struct
 - submodule.c: sort changed_submodule_names before searching it
 - submodule.c: fix indentation
 - sha1-array: provide oid_array_filter

 "git fetch --recurse-submodules" may not fetch the necessary commit
 that is bound to the superproject, which is getting corrected.

 Ready?


* js/add-i-coalesce-after-editing-hunk (2018-08-28) 1 commit
 - add -p: coalesce hunks before testing applicability

 Applicability check after a patch is edited in a "git add -i/p"
 session has been improved.

 Will hold.
 cf. <e5b2900a-0558-d3bf-8ea1-d526b078bbc2@talktalk.net>


* ps/stash-in-c (2018-11-26) 22 commits
 . stash: replace all `write-tree` child processes with API calls
 . stash: optimize `get_untracked_files()` and `check_changes()`
 . stash: convert `stash--helper.c` into `stash.c`
 . stash: convert save to builtin
 . stash: make push -q quiet
 . stash: convert push to builtin
 . stash: convert create to builtin
 . stash: convert store to builtin
 . stash: convert show to builtin
 . stash: convert list to builtin
 . stash: convert pop to builtin
 . stash: convert branch to builtin
 . stash: convert drop and clear to builtin
 . stash: convert apply to builtin
 . stash: mention options in `show` synopsis
 . stash: add tests for `git stash show` config
 . stash: rename test cases to be more descriptive
 . t3903: modernize style
 . stash: improve option parsing test coverage
 . strbuf.c: add `strbuf_insertf()` and `strbuf_vinsertf()`
 . strbuf.c: add `strbuf_join_argv()`
 . sha1-name.c: add `get_oidf()` which acts like `get_oid()`

 "git stash" rewritten in C.

 Expecting a reroll, probably on top of the sd/stash-wo-user-name
 topic after it stabilizes, with an escape hatch like the one in
 "rebase in C".


* pw/add-p-select (2018-07-26) 4 commits
 - add -p: optimize line selection for short hunks
 - add -p: allow line selection to be inverted
 - add -p: select modified lines correctly
 - add -p: select individual hunk lines

 "git add -p" interactive interface learned to let users choose
 individual added/removed lines to be used in the operation, instead
 of accepting or rejecting a whole hunk.

 Will discard.
 No further feedbacks on the topic for quite some time.

 cf. <d622a95b-7302-43d4-4ec9-b2cf3388c653@talktalk.net>
 I found the feature to be hard to explain, and may result in more
 end-user complaints, but let's see.

--------------------------------------------------
[Discarded]

* ab/reject-alias-loop (2018-10-19) 1 commit
 . alias: detect loops in mixed execution mode

 Two (or more) aliases that mutually refer to each other can form an
 infinite loop; we now attempt to notice and stop.

 Discarded.
 Reverted out of 'next'.
 cf. <87sh0slvxm.fsf@evledraar.gmail.com>


* gl/bundle-unlock-before-aborting (2018-11-14) 1 commit
 . bundle: rollback lock file while refusing to create an empty bundle

 Superseded by jk/close-duped-fd-before-unlock-for-bundle


* js/remote-archive-v2 (2018-09-28) 4 commits
 . archive: allow archive over HTTP(S) with proto v2
 . archive: implement protocol v2 archive command
 . archive: use packet_reader for communications
 . archive: follow test standards around assertions

 The original implementation of "git archive --remote" more or less
 bypassed the transport layer and did not work over http(s).  The
 version 2 of the protocol is defined to allow going over http(s) as
 well as Git native transport.

 Retracted; reverted out of next.
 cf. <20181114195142.GI126896@google.com>


* ab/format-patch-rangediff-not-stat (2018-11-24) 1 commit
 . format-patch: don't include --stat with --range-diff output

 The "--rangediff" option recently added to "format-patch"
 interspersed a bogus and useless "--stat" information by mistake,
 which is being corrected.

 Reverted out of 'next'.


* jc/postpone-rebase-in-c (2018-11-26) 1 commit
 . rebase: mark the C reimplementation as an experimental opt-in feature

 People seem to be still finding latent bugs in the "rebase in C"
 reimplementation.  For the upcoming release, use the scripted
 version by default and adjust the documentation accordingly.

 Reverted out of 'next'.

Re: What's cooking in git.git (Dec 2018, #02; Fri, 28)

[Elijah Newren]

On Fri, Dec 28, 2018 at 10:04 AM Junio C Hamano <gitster@pobox.com> wrote:
> Sorry for being slow to recover from the year-end slowness; the 2.20
> release being rather large-ish and needed brown-paper-bag fixes soon
> after it, my temporary trans-pacific migration, or me catching cold
> that I haven't shaken off, did not help X-<.

I hope you feel better soon.

> * en/rebase-merge-on-sequencer (2018-11-08) 2 commits
>  - rebase: implement --merge via git-rebase--interactive
>  - git-rebase, sequencer: extend --quiet option for the interactive machinery
>
>  "git rebase --merge" as been reimplemented by reusing the internal
>  machinery used for "git rebase -i".
>
>  Expecting a reroll.
>  cf. <CABPp-BF8RupyfP69iqAVTXxEhBGyzVd-wUgp3y0pf+CbBFAQeg@mail.gmail.com>

Quick update: Two re-rolls have been sent in[1]; v3 on November 22 and
v4 with only a minor error message tweak on Dec 11.  I think I've
addressed all review comments from v2, but neither v3 nor v4 has
received much review -- Dscho was also heavily busy during the run up
to 2.20 and needed some recovery time afterward.  I was going to
re-ping in early January.  Anyway, it may be worth at least updating
your note to "reroll exists".

Thanks,
Elijah

[1] Latest at https://public-inbox.org/git/20181211161139.31686-1-newren@gmail.com/

Re: What's cooking in git.git (Dec 2018, #02; Fri, 28)

[Johannes Schindelin]

Hi Elijah,

On Fri, 28 Dec 2018, Elijah Newren wrote:

> On Fri, Dec 28, 2018 at 10:04 AM Junio C Hamano <gitster@pobox.com> wrote:
> 
> > * en/rebase-merge-on-sequencer (2018-11-08) 2 commits
> >  - rebase: implement --merge via git-rebase--interactive
> >  - git-rebase, sequencer: extend --quiet option for the interactive machinery
> >
> >  "git rebase --merge" as been reimplemented by reusing the internal
> >  machinery used for "git rebase -i".
> >
> >  Expecting a reroll.
> >  cf. <CABPp-BF8RupyfP69iqAVTXxEhBGyzVd-wUgp3y0pf+CbBFAQeg@mail.gmail.com>
> 
> Quick update: Two re-rolls have been sent in[1]; v3 on November 22 and
> v4 with only a minor error message tweak on Dec 11.  I think I've
> addressed all review comments from v2, but neither v3 nor v4 has
> received much review -- Dscho was also heavily busy during the run up
> to 2.20 and needed some recovery time afterward.

Yep. There have been quite a few problems in the -rc period, and at least
one frantic bug fix of mine introduced another regression, and then there
was the problem with cURL where it would try to use HTTP/2 with NTLM
(which does not work, and probably never will) and as you probably
suspect, NTLM/Kerberos authentication is *quite* common on Windows, so
that would have been a total non-starter if we had shipped Git for Windows
v2.20.0 with an unfixed cURL.

So yes, I was quite exhausted after those weeks.

> I was going to re-ping in early January.  Anyway, it may be worth at
> least updating your note to "reroll exists".

It is early January! ;-)

Ciao,
Dscho

Re: What's cooking in git.git (Dec 2018, #02; Fri, 28)

[Elijah Newren]

Hi Dscho,

On Thu, Jan 3, 2019 at 5:27 AM Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
>
> Hi Elijah,
>
...
> > I was going to re-ping in early January.  Anyway, it may be worth at
> > least updating your note to "reroll exists".
>
> It is early January! ;-)

Indeed...ping incoming.  :-)

>
> Ciao,
> Dscho

ag/sequencer-reduce-rewriting-todo, was Re: What's cooking in git.git (Dec 2018, #02; Fri, 28)

[Alban Gruin]

Hi Junio,

Le 28/12/2018 à 19:04, Junio C Hamano a écrit :
> * ag/sequencer-reduce-rewriting-todo (2018-11-12) 16 commits
>  . rebase--interactive: move transform_todo_file() to rebase--interactive.c
>  . sequencer: fix a call to error() in transform_todo_file()
>  . sequencer: use edit_todo_list() in complete_action()
>  . rebase-interactive: rewrite edit_todo_list() to handle the initial edit
>  . rebase-interactive: append_todo_help() changes
>  . rebase-interactive: use todo_list_write_to_file() in edit_todo_list()
>  . sequencer: refactor skip_unnecessary_picks() to work on a todo_list
>  . sequencer: change complete_action() to use the refactored functions
>  . sequencer: make sequencer_make_script() write its script to a strbuf
>  . sequencer: refactor rearrange_squash() to work on a todo_list
>  . sequencer: refactor sequencer_add_exec_commands() to work on a todo_list
>  . sequencer: refactor check_todo_list() to work on a todo_list
>  . sequencer: introduce todo_list_write_to_file()
>  . sequencer: refactor transform_todos() to work on a todo_list
>  . sequencer: make the todo_list structure public
>  . sequencer: changes in parse_insn_buffer()
> 
>  The scripted version of "git rebase -i" wrote and rewrote the todo
>  list many times during a single step of its operation, and the
>  recent C-rewrite made a faithful conversion of the logic to C.  The
>  implementation has been updated to carry necessary information
>  around in-core to avoid rewriting the same file over and over
>  unnecessarily.
> 
>  With too many topics in-flight that touch sequencer and rebaser,
>  this need to wait giving precedence to other topics that fix bugs.
> 
> 

Most of these topics have reached master and have been released in git
2.20.  Currently, there is four topics actually touching rebase,
interactive rebase and/or the sequencer (js/rebase-i-redo-exec,
nd/backup-log, en/rebase-merge-on-sequencer and nd/the-index).  Among
these, only nd/the-index conflicts with my series.

Should I consider this comment as outdated, and reroll my series
(rebased on top of nd/the-index) in the next few days?

Cheers,
Alban

Re: ag/sequencer-reduce-rewriting-todo, was Re: What's cooking in git.git (Dec 2018, #02; Fri, 28)

[Junio C Hamano]

Alban Gruin <alban.gruin@gmail.com> writes:

>>  With too many topics in-flight that touch sequencer and rebaser,
>>  this need to wait giving precedence to other topics that fix bugs.
>> 
>
> Most of these topics have reached master and have been released in git
> 2.20.  Currently, there is four topics actually touching rebase,
> interactive rebase and/or the sequencer (js/rebase-i-redo-exec,
> nd/backup-log, en/rebase-merge-on-sequencer and nd/the-index).  Among
> these, only nd/the-index conflicts with my series.

OK, so now it's turn for this topic ;-)  Thanks.

Re: What's cooking in git.git (Dec 2018, #02; Fri, 28)

[Denton Liu]

On Fri, Dec 28, 2018 at 10:04:13AM -0800, Junio C Hamano wrote:
> * dl/remote-save-to-push (2018-12-11) 1 commit
>  - remote: add --save-to-push option to git remote set-url
> 
>  "git remote set-url" learned a new option that moves existing value
>  of the URL field to pushURL field of the remote before replacing
>  the URL field with a new value.
> 
>  I am personally not yet quite convinced if this is worth pursuing.

Hi Junio, just wondering from our previous discussions, is this comment
outdated or is it still valid?

Thanks,

-Denton

ps/stash-in-c, was Re: What's cooking in git.git (Dec 2018, #02; Fri, 28)

[Johannes Schindelin]

Hi Junio,

On Fri, 28 Dec 2018, Junio C Hamano wrote:

> * ps/stash-in-c (2018-11-26) 22 commits
>  . stash: replace all `write-tree` child processes with API calls
>  . stash: optimize `get_untracked_files()` and `check_changes()`
>  . stash: convert `stash--helper.c` into `stash.c`
>  . stash: convert save to builtin
>  . stash: make push -q quiet
>  . stash: convert push to builtin
>  . stash: convert create to builtin
>  . stash: convert store to builtin
>  . stash: convert show to builtin
>  . stash: convert list to builtin
>  . stash: convert pop to builtin
>  . stash: convert branch to builtin
>  . stash: convert drop and clear to builtin
>  . stash: convert apply to builtin
>  . stash: mention options in `show` synopsis
>  . stash: add tests for `git stash show` config
>  . stash: rename test cases to be more descriptive
>  . t3903: modernize style
>  . stash: improve option parsing test coverage
>  . strbuf.c: add `strbuf_insertf()` and `strbuf_vinsertf()`
>  . strbuf.c: add `strbuf_join_argv()`
>  . sha1-name.c: add `get_oidf()` which acts like `get_oid()`
> 
>  "git stash" rewritten in C.
> 
>  Expecting a reroll, probably on top of the sd/stash-wo-user-name
>  topic after it stabilizes, with an escape hatch like the one in
>  "rebase in C".

There you go:
https://public-inbox.org/git/cover.1545331726.git.ungureanupaulsebastian@gmail.com/

Happy new year!
Dscho

jk/loose-object-cache

[René Scharfe]

Am 28.12.2018 um 19:04 schrieb Junio C Hamano:
> * jk/loose-object-cache (2018-11-24) 10 commits
>   (merged to 'next' on 2018-12-28 at 5a5faf384e)
>  + odb_load_loose_cache: fix strbuf leak
>  + fetch-pack: drop custom loose object cache
>  + sha1-file: use loose object cache for quick existence check
>  + object-store: provide helpers for loose_objects_cache
>  + sha1-file: use an object_directory for the main object dir
>  + handle alternates paths the same as the main object dir
>  + sha1_file_name(): overwrite buffer instead of appending
>  + rename "alternate_object_database" to "object_directory"
>  + submodule--helper: prefer strip_suffix() to ends_with()
>  + fsck: do not reuse child_process structs
> 
>  Originally merged to 'next' on 2018-11-24
> 
>  Code clean-up with optimization for the codepath that checks
>  (non-)existence of loose objects.
> 
>  Will merge to 'master'.

So this has hit master in the meantime.  We discussed a sort performance
fix in [1]; I'll reply with a short series containing a cleaned-up and
rebased version as a follow-up.

  object-store: factor out odb_loose_cache()
  object-store: factor out odb_clear_loose_cache()
  object-store: use one oid_array per subdirectory for loose cache

 object-store.h | 12 +++++++++++-
 object.c       |  2 +-
 packfile.c     |  7 ++-----
 sha1-file.c    | 24 ++++++++++++++++++++----
 sha1-name.c    | 10 +++++-----
 5 files changed, 39 insertions(+), 16 deletions(-)

[1] https://public-inbox.org/git/221cb2e4-a024-e301-2b3f-e37dcd93795e@web.de/

[PATCH 1/3] object-store: factor out odb_loose_cache()

[René Scharfe]

Add and use a function for loading the entries if a loose object
subdirectory for a given object ID.  It frees callers from deriving the
fanout key; they can use the returned oid_array reference for lookups or
forward range scans.

Suggested-by: Jeff King <peff@peff.net>
Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
 object-store.h |  7 +++++++
 sha1-file.c    | 12 +++++++++---
 sha1-name.c    | 10 +++++-----
 3 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/object-store.h b/object-store.h
index 60758efad8..7236c571c0 100644
--- a/object-store.h
+++ b/object-store.h
@@ -54,6 +54,13 @@ void add_to_alternates_memory(const char *dir);
  */
 void odb_load_loose_cache(struct object_directory *odb, int subdir_nr);
 
+/*
+ * Populate and return the loose object cache array corresponding to the
+ * given object ID.
+ */
+struct oid_array *odb_loose_cache(struct object_directory *odb,
+				  const struct object_id *oid);
+
 struct packed_git {
 	struct packed_git *next;
 	struct list_head mru;
diff --git a/sha1-file.c b/sha1-file.c
index 5a272f70de..cb8583b634 100644
--- a/sha1-file.c
+++ b/sha1-file.c
@@ -924,7 +924,6 @@ static int open_sha1_file(struct repository *r,
 static int quick_has_loose(struct repository *r,
 			   const unsigned char *sha1)
 {
-	int subdir_nr = sha1[0];
 	struct object_id oid;
 	struct object_directory *odb;
 
@@ -932,8 +931,7 @@ static int quick_has_loose(struct repository *r,
 
 	prepare_alt_odb(r);
 	for (odb = r->objects->odb; odb; odb = odb->next) {
-		odb_load_loose_cache(odb, subdir_nr);
-		if (oid_array_lookup(&odb->loose_objects_cache, &oid) >= 0)
+		if (oid_array_lookup(odb_loose_cache(odb, &oid), &oid) >= 0)
 			return 1;
 	}
 	return 0;
@@ -2152,6 +2150,14 @@ static int append_loose_object(const struct object_id *oid, const char *path,
 	return 0;
 }
 
+struct oid_array *odb_loose_cache(struct object_directory *odb,
+				  const struct object_id *oid)
+{
+	int subdir_nr = oid->hash[0];
+	odb_load_loose_cache(odb, subdir_nr);
+	return &odb->loose_objects_cache;
+}
+
 void odb_load_loose_cache(struct object_directory *odb, int subdir_nr)
 {
 	struct strbuf buf = STRBUF_INIT;
diff --git a/sha1-name.c b/sha1-name.c
index b24502811b..a656481c6a 100644
--- a/sha1-name.c
+++ b/sha1-name.c
@@ -87,21 +87,21 @@ static int match_sha(unsigned, const unsigned char *, const unsigned char *);
 
 static void find_short_object_filename(struct disambiguate_state *ds)
 {
-	int subdir_nr = ds->bin_pfx.hash[0];
 	struct object_directory *odb;
 
 	for (odb = the_repository->objects->odb;
 	     odb && !ds->ambiguous;
 	     odb = odb->next) {
 		int pos;
+		struct oid_array *loose_objects;
 
-		odb_load_loose_cache(odb, subdir_nr);
-		pos = oid_array_lookup(&odb->loose_objects_cache, &ds->bin_pfx);
+		loose_objects = odb_loose_cache(odb, &ds->bin_pfx);
+		pos = oid_array_lookup(loose_objects, &ds->bin_pfx);
 		if (pos < 0)
 			pos = -1 - pos;
-		while (!ds->ambiguous && pos < odb->loose_objects_cache.nr) {
+		while (!ds->ambiguous && pos < loose_objects->nr) {
 			const struct object_id *oid;
-			oid = odb->loose_objects_cache.oid + pos;
+			oid = loose_objects->oid + pos;
 			if (!match_sha(ds->len, ds->bin_pfx.hash, oid->hash))
 				break;
 			update_candidates(ds, oid);
-- 
2.20.1

Re: [PATCH 1/3] object-store: factor out odb_loose_cache()

[Jeff King]

On Sun, Jan 06, 2019 at 05:45:30PM +0100, René Scharfe wrote:

> Add and use a function for loading the entries if a loose object
> subdirectory for a given object ID.  It frees callers from deriving the
> fanout key; they can use the returned oid_array reference for lookups or
> forward range scans.

Much nicer.

> diff --git a/object-store.h b/object-store.h
> index 60758efad8..7236c571c0 100644
> --- a/object-store.h
> +++ b/object-store.h
> @@ -54,6 +54,13 @@ void add_to_alternates_memory(const char *dir);
>   */
>  void odb_load_loose_cache(struct object_directory *odb, int subdir_nr);
>  
> +/*
> + * Populate and return the loose object cache array corresponding to the
> + * given object ID.
> + */
> +struct oid_array *odb_loose_cache(struct object_directory *odb,
> +				  const struct object_id *oid);
> +

I think the ugly-interfaced odb_load_loose_cache() can become "static"
now, as the only outside caller (from sha1-name.c) has gone away.

> +struct oid_array *odb_loose_cache(struct object_directory *odb,
> +				  const struct object_id *oid)
> +{
> +	int subdir_nr = oid->hash[0];
> +	odb_load_loose_cache(odb, subdir_nr);
> +	return &odb->loose_objects_cache;
> +}
> +
>  void odb_load_loose_cache(struct object_directory *odb, int subdir_nr)

You'd need to re-order these definitions, of course (or alternatively,
just fold the load function inline into odb_loose_cache()).

-Peff

Re: [PATCH 1/3] object-store: factor out odb_loose_cache()

[René Scharfe]

Am 07.01.2019 um 09:27 schrieb Jeff King:
> On Sun, Jan 06, 2019 at 05:45:30PM +0100, René Scharfe wrote:
>> diff --git a/object-store.h b/object-store.h
>> index 60758efad8..7236c571c0 100644
>> --- a/object-store.h
>> +++ b/object-store.h
>> @@ -54,6 +54,13 @@ void add_to_alternates_memory(const char *dir);
>>   */
>>  void odb_load_loose_cache(struct object_directory *odb, int subdir_nr);
>>  
>> +/*
>> + * Populate and return the loose object cache array corresponding to the
>> + * given object ID.
>> + */
>> +struct oid_array *odb_loose_cache(struct object_directory *odb,
>> +				  const struct object_id *oid);
>> +
> 
> I think the ugly-interfaced odb_load_loose_cache() can become "static"
> now, as the only outside caller (from sha1-name.c) has gone away.
> 
>> +struct oid_array *odb_loose_cache(struct object_directory *odb,
>> +				  const struct object_id *oid)
>> +{
>> +	int subdir_nr = oid->hash[0];
>> +	odb_load_loose_cache(odb, subdir_nr);
>> +	return &odb->loose_objects_cache;
>> +}
>> +
>>  void odb_load_loose_cache(struct object_directory *odb, int subdir_nr)
> 
> You'd need to re-order these definitions, of course (or alternatively,
> just fold the load function inline into odb_loose_cache()).

Yes, the functions are arranged so that odb_load_loose_cache() can be
inlined easily.  I meant to include a patch for that but then quibbled
about keeping the BUG check (which is probably optimized out) or not,
and dropped it for now to get the performance fix in more quickly..

René

Re: [PATCH 1/3] object-store: factor out odb_loose_cache()

[René Scharfe]

Am 07.01.2019 um 14:26 schrieb René Scharfe:
> Yes, the functions are arranged so that odb_load_loose_cache() can be
> inlined easily.  I meant to include a patch for that but then quibbled
> about keeping the BUG check (which is probably optimized out) or not,
> and dropped it for now to get the performance fix in more quickly..

... which is probably just my laziness talking.  Sent a minimal patch
for that now.

René

Re: [PATCH 1/3] object-store: factor out odb_loose_cache()

[Philip Oakley]

On 06/01/2019 16:45, René Scharfe wrote:
> Add and use a function for loading the entries if a loose object
> subdirectory for a given object ID.

The second part of the sentence 'a loose object subdirectory for a given 
object ID' does not scan for me. Is there a word missing?

>    It frees callers from deriving the
> fanout key; they can use the returned oid_array reference for lookups or
> forward range scans.
>
> Suggested-by: Jeff King<peff@peff.net>
> Signed-off-by: Rene Scharfe<l.s.r@web.de>

-- 

Philip

Re: [PATCH 1/3] object-store: factor out odb_loose_cache()

[Jeff King]

On Mon, Jan 07, 2019 at 11:27:06AM +0000, Philip Oakley wrote:

> On 06/01/2019 16:45, René Scharfe wrote:
> > Add and use a function for loading the entries if a loose object
> > subdirectory for a given object ID.
> 
> The second part of the sentence 'a loose object subdirectory for a given
> object ID' does not scan for me. Is there a word missing?

I think s/if/in/ ?

-Peff

Re: [PATCH 1/3] object-store: factor out odb_loose_cache()

[René Scharfe]

Am 07.01.2019 um 13:30 schrieb Jeff King:
> On Mon, Jan 07, 2019 at 11:27:06AM +0000, Philip Oakley wrote:
> 
>> On 06/01/2019 16:45, René Scharfe wrote:
>>> Add and use a function for loading the entries if a loose object
>>> subdirectory for a given object ID.
>>
>> The second part of the sentence 'a loose object subdirectory for a given
>> object ID' does not scan for me. Is there a word missing?
> 
> I think s/if/in/ ?

Good guess and close, but I think I meant s/if/of/.  Anyway, the idea is
that a caller provides an object ID and the function then reads the
corresponding object subdirectory.

René

[PATCH 2/3] object-store: factor out odb_clear_loose_cache()

[René Scharfe]

Add and use a function for emptying the loose object cache, so callers
don't have to know any of its implementation details.

Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
 object-store.h | 3 +++
 object.c       | 2 +-
 packfile.c     | 7 ++-----
 sha1-file.c    | 7 +++++++
 4 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/object-store.h b/object-store.h
index 7236c571c0..709bf856b6 100644
--- a/object-store.h
+++ b/object-store.h
@@ -61,6 +61,9 @@ void odb_load_loose_cache(struct object_directory *odb, int subdir_nr);
 struct oid_array *odb_loose_cache(struct object_directory *odb,
 				  const struct object_id *oid);
 
+/* Empty the loose object cache for the specified object directory. */
+void odb_clear_loose_cache(struct object_directory *odb);
+
 struct packed_git {
 	struct packed_git *next;
 	struct list_head mru;
diff --git a/object.c b/object.c
index 79d636091c..a5c5cf830f 100644
--- a/object.c
+++ b/object.c
@@ -485,7 +485,7 @@ struct raw_object_store *raw_object_store_new(void)
 static void free_object_directory(struct object_directory *odb)
 {
 	free(odb->path);
-	oid_array_clear(&odb->loose_objects_cache);
+	odb_clear_loose_cache(odb);
 	free(odb);
 }
 
diff --git a/packfile.c b/packfile.c
index 8c6b47cc77..0fe9c21bf1 100644
--- a/packfile.c
+++ b/packfile.c
@@ -994,11 +994,8 @@ void reprepare_packed_git(struct repository *r)
 {
 	struct object_directory *odb;
 
-	for (odb = r->objects->odb; odb; odb = odb->next) {
-		oid_array_clear(&odb->loose_objects_cache);
-		memset(&odb->loose_objects_subdir_seen, 0,
-		       sizeof(odb->loose_objects_subdir_seen));
-	}
+	for (odb = r->objects->odb; odb; odb = odb->next)
+		odb_clear_loose_cache(odb);
 
 	r->objects->approximate_object_count_valid = 0;
 	r->objects->packed_git_initialized = 0;
diff --git a/sha1-file.c b/sha1-file.c
index cb8583b634..2f965b2688 100644
--- a/sha1-file.c
+++ b/sha1-file.c
@@ -2178,6 +2178,13 @@ void odb_load_loose_cache(struct object_directory *odb, int subdir_nr)
 	strbuf_release(&buf);
 }
 
+void odb_clear_loose_cache(struct object_directory *odb)
+{
+	oid_array_clear(&odb->loose_objects_cache);
+	memset(&odb->loose_objects_subdir_seen, 0,
+	       sizeof(odb->loose_objects_subdir_seen));
+}
+
 static int check_stream_sha1(git_zstream *stream,
 			     const char *hdr,
 			     unsigned long size,
-- 
2.20.1

[PATCH 3/3] object-store: use one oid_array per subdirectory for loose cache

[René Scharfe]

The loose objects cache is filled one subdirectory at a time as needed.
It is stored in an oid_array, which has to be resorted after each add
operation.  So when querying a wide range of objects, the partially
filled array needs to be resorted up to 255 times, which takes over 100
times longer than sorting once.

Use one oid_array for each subdirectory.  This ensures that entries have
to only be sorted a single time.  It also avoids eight binary search
steps for each cache lookup as a small bonus.

The cache is used for collision checks for the log placeholders %h, %t
and %p, and we can see the change speeding them up in a repository with
ca. 100 objects per subdirectory:

$ git count-objects
26733 objects, 68808 kilobytes

Test                        HEAD^             HEAD
--------------------------------------------------------------------
4205.1: log with %H         0.51(0.47+0.04)   0.51(0.49+0.02) +0.0%
4205.2: log with %h         0.84(0.82+0.02)   0.60(0.57+0.03) -28.6%
4205.3: log with %T         0.53(0.49+0.04)   0.52(0.48+0.03) -1.9%
4205.4: log with %t         0.84(0.80+0.04)   0.60(0.59+0.01) -28.6%
4205.5: log with %P         0.52(0.48+0.03)   0.51(0.50+0.01) -1.9%
4205.6: log with %p         0.85(0.78+0.06)   0.61(0.56+0.05) -28.2%
4205.7: log with %h-%h-%h   0.96(0.92+0.03)   0.69(0.64+0.04) -28.1%

Reported-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
 object-store.h | 2 +-
 sha1-file.c    | 9 ++++++---
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/object-store.h b/object-store.h
index 709bf856b6..2fb6c0e4db 100644
--- a/object-store.h
+++ b/object-store.h
@@ -20,7 +20,7 @@ struct object_directory {
 	 * Be sure to call odb_load_loose_cache() before using.
 	 */
 	char loose_objects_subdir_seen[256];
-	struct oid_array loose_objects_cache;
+	struct oid_array loose_objects_cache[256];
 
 	/*
 	 * Path to the alternative object store. If this is a relative path,
diff --git a/sha1-file.c b/sha1-file.c
index 2f965b2688..c3c6e50704 100644
--- a/sha1-file.c
+++ b/sha1-file.c
@@ -2155,7 +2155,7 @@ struct oid_array *odb_loose_cache(struct object_directory *odb,
 {
 	int subdir_nr = oid->hash[0];
 	odb_load_loose_cache(odb, subdir_nr);
-	return &odb->loose_objects_cache;
+	return &odb->loose_objects_cache[subdir_nr];
 }
 
 void odb_load_loose_cache(struct object_directory *odb, int subdir_nr)
@@ -2173,14 +2173,17 @@ void odb_load_loose_cache(struct object_directory *odb, int subdir_nr)
 	for_each_file_in_obj_subdir(subdir_nr, &buf,
 				    append_loose_object,
 				    NULL, NULL,
-				    &odb->loose_objects_cache);
+				    &odb->loose_objects_cache[subdir_nr]);
 	odb->loose_objects_subdir_seen[subdir_nr] = 1;
 	strbuf_release(&buf);
 }
 
 void odb_clear_loose_cache(struct object_directory *odb)
 {
-	oid_array_clear(&odb->loose_objects_cache);
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(odb->loose_objects_cache); i++)
+		oid_array_clear(&odb->loose_objects_cache[i]);
 	memset(&odb->loose_objects_subdir_seen, 0,
 	       sizeof(odb->loose_objects_subdir_seen));
 }
-- 
2.20.1

Re: [PATCH 3/3] object-store: use one oid_array per subdirectory for loose cache

[Ævar Arnfjörð Bjarmason]

On Sun, Jan 06 2019, René Scharfe wrote:

Thanks. I haven't done my own performance testing but at a glance this
looks good.

> The cache is used for collision checks for the log placeholders %h, %t
> and %p, and we can see the change speeding them up in a repository with
> ca. 100 objects per subdirectory:
>
> $ git count-objects
> 26733 objects, 68808 kilobytes
>
> Test                        HEAD^             HEAD
> --------------------------------------------------------------------
> 4205.1: log with %H         0.51(0.47+0.04)   0.51(0.49+0.02) +0.0%
> 4205.2: log with %h         0.84(0.82+0.02)   0.60(0.57+0.03) -28.6%
> 4205.3: log with %T         0.53(0.49+0.04)   0.52(0.48+0.03) -1.9%
> 4205.4: log with %t         0.84(0.80+0.04)   0.60(0.59+0.01) -28.6%
> 4205.5: log with %P         0.52(0.48+0.03)   0.51(0.50+0.01) -1.9%
> 4205.6: log with %p         0.85(0.78+0.06)   0.61(0.56+0.05) -28.2%
> 4205.7: log with %h-%h-%h   0.96(0.92+0.03)   0.69(0.64+0.04) -28.1%

Can you elaborate on the test setup required to get to the point where
you got these numbers for subsequent comparison, i.e. how you generated
the approx 100 objects per dir, what OS/version & storage type etc.

Re: [PATCH 3/3] object-store: use one oid_array per subdirectory for loose cache

[René Scharfe]

Am 06.01.2019 um 21:38 schrieb Ævar Arnfjörð Bjarmason:
>> $ git count-objects
>> 26733 objects, 68808 kilobytes
>>
>> Test                        HEAD^             HEAD
>> --------------------------------------------------------------------
>> 4205.1: log with %H         0.51(0.47+0.04)   0.51(0.49+0.02) +0.0%
>> 4205.2: log with %h         0.84(0.82+0.02)   0.60(0.57+0.03) -28.6%
>> 4205.3: log with %T         0.53(0.49+0.04)   0.52(0.48+0.03) -1.9%
>> 4205.4: log with %t         0.84(0.80+0.04)   0.60(0.59+0.01) -28.6%
>> 4205.5: log with %P         0.52(0.48+0.03)   0.51(0.50+0.01) -1.9%
>> 4205.6: log with %p         0.85(0.78+0.06)   0.61(0.56+0.05) -28.2%
>> 4205.7: log with %h-%h-%h   0.96(0.92+0.03)   0.69(0.64+0.04) -28.1%
> 
> Can you elaborate on the test setup required to get to the point where
> you got these numbers for subsequent comparison, i.e. how you generated
> the approx 100 objects per dir, what OS/version & storage type etc.

I happened to have that many loose objects lying around.  Numbers are
for Debian Testing on a Hyper-V VM on Windows 10 1893 on an SSD.

You could fake object directory entries with something like this:

    for d in .git/objects/??
    do
        for i in $(seq 0 9)
        do
            >"$d/0000000000000000000000000000000000000$i"
        done
    done

René

[PATCH 0/11] jk/loose-object-cache sha1/object_id fixups

[Jeff King]

On Sun, Jan 06, 2019 at 05:39:14PM +0100, René Scharfe wrote:

> Am 28.12.2018 um 19:04 schrieb Junio C Hamano:
> > * jk/loose-object-cache (2018-11-24) 10 commits
> >   (merged to 'next' on 2018-12-28 at 5a5faf384e)
> >  + odb_load_loose_cache: fix strbuf leak
> >  + fetch-pack: drop custom loose object cache
> >  + sha1-file: use loose object cache for quick existence check
> >  + object-store: provide helpers for loose_objects_cache
> >  + sha1-file: use an object_directory for the main object dir
> >  + handle alternates paths the same as the main object dir
> >  + sha1_file_name(): overwrite buffer instead of appending
> >  + rename "alternate_object_database" to "object_directory"
> >  + submodule--helper: prefer strip_suffix() to ends_with()
> >  + fsck: do not reuse child_process structs
> > 
> >  Originally merged to 'next' on 2018-11-24
> > 
> >  Code clean-up with optimization for the codepath that checks
> >  (non-)existence of loose objects.
> > 
> >  Will merge to 'master'.
> 
> So this has hit master in the meantime.  We discussed a sort performance
> fix in [1]; I'll reply with a short series containing a cleaned-up and
> rebased version as a follow-up.
> 
>   object-store: factor out odb_loose_cache()
>   object-store: factor out odb_clear_loose_cache()
>   object-store: use one oid_array per subdirectory for loose cache

Thanks! With the exception of one tiny nit, this looks good to me.

I also cleaned up my sha1/object_id patch and rebased it on top of what
you have here. Though as I worked on it, it expanded in scope a bit.
Possibly it should be a separate series entirely, but that would create
some annoying textual conflicts on merge.

  [01/11]: sha1-file: fix outdated sha1 comment references
  [02/11]: update comment references to sha1_object_info()
  [03/11]: http: use struct object_id instead of bare sha1
  [04/11]: sha1-file: modernize loose object file functions
  [05/11]: sha1-file: modernize loose header/stream functions
  [06/11]: sha1-file: convert pass-through functions to object_id
  [07/11]: convert has_sha1_file() callers to has_object_file()
  [08/11]: sha1-file: drop has_sha1_file()
  [09/11]: sha1-file: prefer "loose object file" to "sha1 file" in messages
  [10/11]: sha1-file: avoid "sha1 file" for generic use in messages
  [11/11]: prefer "hash mismatch" to "sha1 mismatch"

 Documentation/git-fsck.txt |   6 +-
 apply.c                    |   2 +-
 builtin/cat-file.c         |   6 +-
 builtin/fetch.c            |   7 +-
 builtin/index-pack.c       |   2 +-
 builtin/pack-objects.c     |   4 +-
 builtin/reflog.c           |   2 +-
 builtin/show-ref.c         |   2 +-
 bulk-checkin.c             |   2 +-
 cache-tree.c               |   4 +-
 cache.h                    |   6 +-
 http-push.c                |   2 +-
 http-walker.c              |  10 +-
 http.c                     |  14 +--
 http.h                     |   6 +-
 object-store.h             |  20 ++--
 object.c                   |   4 +-
 refs.c                     |   2 +-
 send-pack.c                |   2 +-
 sha1-file.c                | 219 +++++++++++++++++--------------------
 streaming.c                |  16 +--
 t/t1450-fsck.sh            |   2 +-
 22 files changed, 161 insertions(+), 179 deletions(-)

-Peff

[PATCH 01/11] sha1-file: fix outdated sha1 comment references

[Jeff King]

Commit 17e65451e3 (sha1_file: convert check_sha1_signature to struct
object_id, 2018-03-12) switched to using the name "oid", but forgot to
update the variable name in the comment.

Likewise, b4f5aca40e (sha1_file: convert read_sha1_file to struct
object_id, 2018-03-12) dropped the name read_sha1_file(), but missed a
comment which mentions it.

Signed-off-by: Jeff King <peff@peff.net>
---
 sha1-file.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/sha1-file.c b/sha1-file.c
index c3c6e50704..e86bb28320 100644
--- a/sha1-file.c
+++ b/sha1-file.c
@@ -124,7 +124,7 @@ const char *empty_blob_oid_hex(void)
 
 /*
  * This is meant to hold a *small* number of objects that you would
- * want read_sha1_file() to be able to return, but yet you do not want
+ * want read_object_file() to be able to return, but yet you do not want
  * to write them into the object store (e.g. a browse-only
  * application).
  */
@@ -798,8 +798,8 @@ void *xmmap(void *start, size_t length,
 
 /*
  * With an in-core object data in "map", rehash it to make sure the
- * object name actually matches "sha1" to detect object corruption.
- * With "map" == NULL, try reading the object named with "sha1" using
+ * object name actually matches "oid" to detect object corruption.
+ * With "map" == NULL, try reading the object named with "oid" using
  * the streaming interface and rehash it to do the same.
  */
 int check_object_signature(const struct object_id *oid, void *map,
-- 
2.20.1.470.g640a3e2614

[PATCH 02/11] update comment references to sha1_object_info()

[Jeff King]

Commit abef9020e3 (sha1_file: convert sha1_object_info* to object_id,
2018-03-12) renamed the function to oid_object_info(), but missed some
comments which mention it.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/cat-file.c     | 6 +++---
 builtin/pack-objects.c | 4 ++--
 cache.h                | 2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 2ca56fd086..baaafbdcf3 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -209,14 +209,14 @@ struct expand_data {
 
 	/*
 	 * After a mark_query run, this object_info is set up to be
-	 * passed to sha1_object_info_extended. It will point to the data
+	 * passed to oid_object_info_extended. It will point to the data
 	 * elements above, so you can retrieve the response from there.
 	 */
 	struct object_info info;
 
 	/*
 	 * This flag will be true if the requested batch format and options
-	 * don't require us to call sha1_object_info, which can then be
+	 * don't require us to call oid_object_info, which can then be
 	 * optimized out.
 	 */
 	unsigned skip_object_info : 1;
@@ -490,7 +490,7 @@ static int batch_objects(struct batch_options *opt)
 
 	/*
 	 * Expand once with our special mark_query flag, which will prime the
-	 * object_info to be handed to sha1_object_info_extended for each
+	 * object_info to be handed to oid_object_info_extended for each
 	 * object.
 	 */
 	memset(&data, 0, sizeof(data));
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 24bba8147f..d4c3987f3e 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -1642,7 +1642,7 @@ static void check_object(struct object_entry *entry)
 
 		/*
 		 * No choice but to fall back to the recursive delta walk
-		 * with sha1_object_info() to find about the object type
+		 * with oid_object_info() to find about the object type
 		 * at this point...
 		 */
 		give_up:
@@ -1718,7 +1718,7 @@ static void drop_reused_delta(struct object_entry *entry)
 	if (packed_object_info(the_repository, IN_PACK(entry), entry->in_pack_offset, &oi) < 0) {
 		/*
 		 * We failed to get the info from this pack for some reason;
-		 * fall back to sha1_object_info, which may find another copy.
+		 * fall back to oid_object_info, which may find another copy.
 		 * And if that fails, the error will be recorded in oe_type(entry)
 		 * and dealt with in prepare_pack().
 		 */
diff --git a/cache.h b/cache.h
index ca36b44ee0..587512747b 100644
--- a/cache.h
+++ b/cache.h
@@ -1592,7 +1592,7 @@ extern int odb_mkstemp(struct strbuf *temp_filename, const char *pattern);
 extern int odb_pack_keep(const char *name);
 
 /*
- * Set this to 0 to prevent sha1_object_info_extended() from fetching missing
+ * Set this to 0 to prevent oid_object_info_extended() from fetching missing
  * blobs. This has a difference only if extensions.partialClone is set.
  *
  * Its default value is 1.
-- 
2.20.1.470.g640a3e2614

[PATCH 03/11] http: use struct object_id instead of bare sha1

[Jeff King]

The dumb-http walker code still passes around and stores object ids as
"unsigned char *sha1". Let's modernize it.

There's probably still more work to be done to handle dumb-http fetches
with a new, larger hash. But that can wait; this is enough that we can
now convert some of the low-level object routines that we call into from
here (and in fact, some of the "oid.hash" references added here will be
further improved in the next patch).

Signed-off-by: Jeff King <peff@peff.net>
---
 http-push.c   |  2 +-
 http-walker.c |  6 +++---
 http.c        | 14 +++++++-------
 http.h        |  6 +++---
 4 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/http-push.c b/http-push.c
index cd48590912..0141b0ad53 100644
--- a/http-push.c
+++ b/http-push.c
@@ -255,7 +255,7 @@ static void start_fetch_loose(struct transfer_request *request)
 	struct active_request_slot *slot;
 	struct http_object_request *obj_req;
 
-	obj_req = new_http_object_request(repo->url, request->obj->oid.hash);
+	obj_req = new_http_object_request(repo->url, &request->obj->oid);
 	if (obj_req == NULL) {
 		request->state = ABORTED;
 		return;
diff --git a/http-walker.c b/http-walker.c
index 0a392c85b6..856716c63d 100644
--- a/http-walker.c
+++ b/http-walker.c
@@ -58,7 +58,7 @@ static void start_object_request(struct walker *walker,
 	struct active_request_slot *slot;
 	struct http_object_request *req;
 
-	req = new_http_object_request(obj_req->repo->base, obj_req->oid.hash);
+	req = new_http_object_request(obj_req->repo->base, &obj_req->oid);
 	if (req == NULL) {
 		obj_req->state = ABORTED;
 		return;
@@ -543,11 +543,11 @@ static int fetch_object(struct walker *walker, unsigned char *sha1)
 	} else if (req->zret != Z_STREAM_END) {
 		walker->corrupt_object_found++;
 		ret = error("File %s (%s) corrupt", hex, req->url);
-	} else if (!hasheq(obj_req->oid.hash, req->real_sha1)) {
+	} else if (!oideq(&obj_req->oid, &req->real_oid)) {
 		ret = error("File %s has bad hash", hex);
 	} else if (req->rename < 0) {
 		struct strbuf buf = STRBUF_INIT;
-		loose_object_path(the_repository, &buf, req->sha1);
+		loose_object_path(the_repository, &buf, req->oid.hash);
 		ret = error("unable to write sha1 filename %s", buf.buf);
 		strbuf_release(&buf);
 	}
diff --git a/http.c b/http.c
index 0b6807cef9..8d42154792 100644
--- a/http.c
+++ b/http.c
@@ -2337,9 +2337,9 @@ static size_t fwrite_sha1_file(char *ptr, size_t eltsize, size_t nmemb,
 }
 
 struct http_object_request *new_http_object_request(const char *base_url,
-	unsigned char *sha1)
+						    const struct object_id *oid)
 {
-	char *hex = sha1_to_hex(sha1);
+	char *hex = oid_to_hex(oid);
 	struct strbuf filename = STRBUF_INIT;
 	struct strbuf prevfile = STRBUF_INIT;
 	int prevlocal;
@@ -2350,10 +2350,10 @@ struct http_object_request *new_http_object_request(const char *base_url,
 
 	freq = xcalloc(1, sizeof(*freq));
 	strbuf_init(&freq->tmpfile, 0);
-	hashcpy(freq->sha1, sha1);
+	oidcpy(&freq->oid, oid);
 	freq->localfile = -1;
 
-	loose_object_path(the_repository, &filename, sha1);
+	loose_object_path(the_repository, &filename, oid->hash);
 	strbuf_addf(&freq->tmpfile, "%s.temp", filename.buf);
 
 	strbuf_addf(&prevfile, "%s.prev", filename.buf);
@@ -2495,16 +2495,16 @@ int finish_http_object_request(struct http_object_request *freq)
 	}
 
 	git_inflate_end(&freq->stream);
-	git_SHA1_Final(freq->real_sha1, &freq->c);
+	git_SHA1_Final(freq->real_oid.hash, &freq->c);
 	if (freq->zret != Z_STREAM_END) {
 		unlink_or_warn(freq->tmpfile.buf);
 		return -1;
 	}
-	if (!hasheq(freq->sha1, freq->real_sha1)) {
+	if (!oideq(&freq->oid, &freq->real_oid)) {
 		unlink_or_warn(freq->tmpfile.buf);
 		return -1;
 	}
-	loose_object_path(the_repository, &filename, freq->sha1);
+	loose_object_path(the_repository, &filename, freq->oid.hash);
 	freq->rename = finalize_object_file(freq->tmpfile.buf, filename.buf);
 	strbuf_release(&filename);
 
diff --git a/http.h b/http.h
index d305ca1dc7..66c52b2e1e 100644
--- a/http.h
+++ b/http.h
@@ -224,8 +224,8 @@ struct http_object_request {
 	CURLcode curl_result;
 	char errorstr[CURL_ERROR_SIZE];
 	long http_code;
-	unsigned char sha1[20];
-	unsigned char real_sha1[20];
+	struct object_id oid;
+	struct object_id real_oid;
 	git_SHA_CTX c;
 	git_zstream stream;
 	int zret;
@@ -234,7 +234,7 @@ struct http_object_request {
 };
 
 extern struct http_object_request *new_http_object_request(
-	const char *base_url, unsigned char *sha1);
+	const char *base_url, const struct object_id *oid);
 extern void process_http_object_request(struct http_object_request *freq);
 extern int finish_http_object_request(struct http_object_request *freq);
 extern void abort_http_object_request(struct http_object_request *freq);
-- 
2.20.1.470.g640a3e2614

[PATCH 04/11] sha1-file: modernize loose object file functions

[Jeff King]

The loose object access code in sha1-file.c is some of the oldest in
Git, and could use some modernizing. It mostly uses "unsigned char *"
for object ids, which these days should be "struct object_id".

It also uses the term "sha1_file" in many functions, which is confusing.
The term "loose_objects" is much better. It clearly distinguishes
them from packed objects (which didn't even exist back when the name
"sha1_file" came into being). And it also distinguishes it from the
checksummed-file concept in csum-file.c (which until recently was
actually called "struct sha1file"!).

This patch converts the functions {open,close,map,stat}_sha1_file() into
open_loose_object(), etc, and switches their sha1 arguments for
object_id structs. Similarly, path functions like fill_sha1_path()
become fill_loose_path() and use object_ids.

The function sha1_loose_object_info() already says "loose", so we can
just drop the "sha1" (and teach it to use object_id).

Signed-off-by: Jeff King <peff@peff.net>
---
 http-walker.c  |  2 +-
 http.c         |  4 +--
 object-store.h |  8 +++--
 sha1-file.c    | 81 +++++++++++++++++++++++++-------------------------
 streaming.c    |  4 +--
 5 files changed, 51 insertions(+), 48 deletions(-)

diff --git a/http-walker.c b/http-walker.c
index 856716c63d..29b59e2fe0 100644
--- a/http-walker.c
+++ b/http-walker.c
@@ -547,7 +547,7 @@ static int fetch_object(struct walker *walker, unsigned char *sha1)
 		ret = error("File %s has bad hash", hex);
 	} else if (req->rename < 0) {
 		struct strbuf buf = STRBUF_INIT;
-		loose_object_path(the_repository, &buf, req->oid.hash);
+		loose_object_path(the_repository, &buf, &req->oid);
 		ret = error("unable to write sha1 filename %s", buf.buf);
 		strbuf_release(&buf);
 	}
diff --git a/http.c b/http.c
index 8d42154792..43d06dd074 100644
--- a/http.c
+++ b/http.c
@@ -2353,7 +2353,7 @@ struct http_object_request *new_http_object_request(const char *base_url,
 	oidcpy(&freq->oid, oid);
 	freq->localfile = -1;
 
-	loose_object_path(the_repository, &filename, oid->hash);
+	loose_object_path(the_repository, &filename, oid);
 	strbuf_addf(&freq->tmpfile, "%s.temp", filename.buf);
 
 	strbuf_addf(&prevfile, "%s.prev", filename.buf);
@@ -2504,7 +2504,7 @@ int finish_http_object_request(struct http_object_request *freq)
 		unlink_or_warn(freq->tmpfile.buf);
 		return -1;
 	}
-	loose_object_path(the_repository, &filename, freq->oid.hash);
+	loose_object_path(the_repository, &filename, &freq->oid);
 	freq->rename = finalize_object_file(freq->tmpfile.buf, filename.buf);
 	strbuf_release(&filename);
 
diff --git a/object-store.h b/object-store.h
index 2fb6c0e4db..6b1c408753 100644
--- a/object-store.h
+++ b/object-store.h
@@ -161,11 +161,13 @@ void raw_object_store_clear(struct raw_object_store *o);
 
 /*
  * Put in `buf` the name of the file in the local object database that
- * would be used to store a loose object with the specified sha1.
+ * would be used to store a loose object with the specified oid.
  */
-const char *loose_object_path(struct repository *r, struct strbuf *buf, const unsigned char *sha1);
+const char *loose_object_path(struct repository *r, struct strbuf *buf,
+			      const struct object_id *oid);
 
-void *map_sha1_file(struct repository *r, const unsigned char *sha1, unsigned long *size);
+void *map_loose_object(struct repository *r, const struct object_id *oid,
+		       unsigned long *size);
 
 extern void *read_object_file_extended(const struct object_id *oid,
 				       enum object_type *type,
diff --git a/sha1-file.c b/sha1-file.c
index e86bb28320..cd8e5f005a 100644
--- a/sha1-file.c
+++ b/sha1-file.c
@@ -333,12 +333,12 @@ int raceproof_create_file(const char *path, create_file_fn fn, void *cb)
 	return ret;
 }
 
-static void fill_sha1_path(struct strbuf *buf, const unsigned char *sha1)
+static void fill_loose_path(struct strbuf *buf, const struct object_id *oid)
 {
 	int i;
 	for (i = 0; i < the_hash_algo->rawsz; i++) {
 		static char hex[] = "0123456789abcdef";
-		unsigned int val = sha1[i];
+		unsigned int val = oid->hash[i];
 		strbuf_addch(buf, hex[val >> 4]);
 		strbuf_addch(buf, hex[val & 0xf]);
 		if (!i)
@@ -348,19 +348,19 @@ static void fill_sha1_path(struct strbuf *buf, const unsigned char *sha1)
 
 static const char *odb_loose_path(struct object_directory *odb,
 				  struct strbuf *buf,
-				  const unsigned char *sha1)
+				  const struct object_id *oid)
 {
 	strbuf_reset(buf);
 	strbuf_addstr(buf, odb->path);
 	strbuf_addch(buf, '/');
-	fill_sha1_path(buf, sha1);
+	fill_loose_path(buf, oid);
 	return buf->buf;
 }
 
 const char *loose_object_path(struct repository *r, struct strbuf *buf,
-			      const unsigned char *sha1)
+			      const struct object_id *oid)
 {
-	return odb_loose_path(r->objects->odb, buf, sha1);
+	return odb_loose_path(r->objects->odb, buf, oid);
 }
 
 /*
@@ -721,7 +721,7 @@ static int check_and_freshen_odb(struct object_directory *odb,
 				 int freshen)
 {
 	static struct strbuf path = STRBUF_INIT;
-	odb_loose_path(odb, &path, oid->hash);
+	odb_loose_path(odb, &path, oid);
 	return check_and_freshen_file(path.buf, freshen);
 }
 
@@ -872,22 +872,22 @@ int git_open_cloexec(const char *name, int flags)
 }
 
 /*
- * Find "sha1" as a loose object in the local repository or in an alternate.
+ * Find "oid" as a loose object in the local repository or in an alternate.
  * Returns 0 on success, negative on failure.
  *
  * The "path" out-parameter will give the path of the object we found (if any).
  * Note that it may point to static storage and is only valid until another
- * call to stat_sha1_file().
+ * call to stat_loose_object().
  */
-static int stat_sha1_file(struct repository *r, const unsigned char *sha1,
-			  struct stat *st, const char **path)
+static int stat_loose_object(struct repository *r, const struct object_id *oid,
+			     struct stat *st, const char **path)
 {
 	struct object_directory *odb;
 	static struct strbuf buf = STRBUF_INIT;
 
 	prepare_alt_odb(r);
 	for (odb = r->objects->odb; odb; odb = odb->next) {
-		*path = odb_loose_path(odb, &buf, sha1);
+		*path = odb_loose_path(odb, &buf, oid);
 		if (!lstat(*path, st))
 			return 0;
 	}
@@ -896,11 +896,11 @@ static int stat_sha1_file(struct repository *r, const unsigned char *sha1,
 }
 
 /*
- * Like stat_sha1_file(), but actually open the object and return the
+ * Like stat_loose_object(), but actually open the object and return the
  * descriptor. See the caveats on the "path" parameter above.
  */
-static int open_sha1_file(struct repository *r,
-			  const unsigned char *sha1, const char **path)
+static int open_loose_object(struct repository *r,
+			     const struct object_id *oid, const char **path)
 {
 	int fd;
 	struct object_directory *odb;
@@ -909,7 +909,7 @@ static int open_sha1_file(struct repository *r,
 
 	prepare_alt_odb(r);
 	for (odb = r->objects->odb; odb; odb = odb->next) {
-		*path = odb_loose_path(odb, &buf, sha1);
+		*path = odb_loose_path(odb, &buf, oid);
 		fd = git_open(*path);
 		if (fd >= 0)
 			return fd;
@@ -939,10 +939,10 @@ static int quick_has_loose(struct repository *r,
 
 /*
  * Map the loose object at "path" if it is not NULL, or the path found by
- * searching for a loose object named "sha1".
+ * searching for a loose object named "oid".
  */
-static void *map_sha1_file_1(struct repository *r, const char *path,
-			     const unsigned char *sha1, unsigned long *size)
+static void *map_loose_object_1(struct repository *r, const char *path,
+			     const struct object_id *oid, unsigned long *size)
 {
 	void *map;
 	int fd;
@@ -950,7 +950,7 @@ static void *map_sha1_file_1(struct repository *r, const char *path,
 	if (path)
 		fd = git_open(path);
 	else
-		fd = open_sha1_file(r, sha1, &path);
+		fd = open_loose_object(r, oid, &path);
 	map = NULL;
 	if (fd >= 0) {
 		struct stat st;
@@ -969,10 +969,11 @@ static void *map_sha1_file_1(struct repository *r, const char *path,
 	return map;
 }
 
-void *map_sha1_file(struct repository *r,
-		    const unsigned char *sha1, unsigned long *size)
+void *map_loose_object(struct repository *r,
+		       const struct object_id *oid,
+		       unsigned long *size)
 {
-	return map_sha1_file_1(r, NULL, sha1, size);
+	return map_loose_object_1(r, NULL, oid, size);
 }
 
 static int unpack_sha1_short_header(git_zstream *stream,
@@ -1161,9 +1162,9 @@ int parse_sha1_header(const char *hdr, unsigned long *sizep)
 	return parse_sha1_header_extended(hdr, &oi, 0);
 }
 
-static int sha1_loose_object_info(struct repository *r,
-				  const unsigned char *sha1,
-				  struct object_info *oi, int flags)
+static int loose_object_info(struct repository *r,
+			     const struct object_id *oid,
+			     struct object_info *oi, int flags)
 {
 	int status = 0;
 	unsigned long mapsize;
@@ -1188,15 +1189,15 @@ static int sha1_loose_object_info(struct repository *r,
 		const char *path;
 		struct stat st;
 		if (!oi->disk_sizep && (flags & OBJECT_INFO_QUICK))
-			return quick_has_loose(r, sha1) ? 0 : -1;
-		if (stat_sha1_file(r, sha1, &st, &path) < 0)
+			return quick_has_loose(r, oid->hash) ? 0 : -1;
+		if (stat_loose_object(r, oid, &st, &path) < 0)
 			return -1;
 		if (oi->disk_sizep)
 			*oi->disk_sizep = st.st_size;
 		return 0;
 	}
 
-	map = map_sha1_file(r, sha1, &mapsize);
+	map = map_loose_object(r, oid, &mapsize);
 	if (!map)
 		return -1;
 
@@ -1208,22 +1209,22 @@ static int sha1_loose_object_info(struct repository *r,
 	if ((flags & OBJECT_INFO_ALLOW_UNKNOWN_TYPE)) {
 		if (unpack_sha1_header_to_strbuf(&stream, map, mapsize, hdr, sizeof(hdr), &hdrbuf) < 0)
 			status = error(_("unable to unpack %s header with --allow-unknown-type"),
-				       sha1_to_hex(sha1));
+				       oid_to_hex(oid));
 	} else if (unpack_sha1_header(&stream, map, mapsize, hdr, sizeof(hdr)) < 0)
 		status = error(_("unable to unpack %s header"),
-			       sha1_to_hex(sha1));
+			       oid_to_hex(oid));
 	if (status < 0)
 		; /* Do nothing */
 	else if (hdrbuf.len) {
 		if ((status = parse_sha1_header_extended(hdrbuf.buf, oi, flags)) < 0)
 			status = error(_("unable to parse %s header with --allow-unknown-type"),
-				       sha1_to_hex(sha1));
+				       oid_to_hex(oid));
 	} else if ((status = parse_sha1_header_extended(hdr, oi, flags)) < 0)
-		status = error(_("unable to parse %s header"), sha1_to_hex(sha1));
+		status = error(_("unable to parse %s header"), oid_to_hex(oid));
 
 	if (status >= 0 && oi->contentp) {
 		*oi->contentp = unpack_sha1_rest(&stream, hdr,
-						 *oi->sizep, sha1);
+						 *oi->sizep, oid->hash);
 		if (!*oi->contentp) {
 			git_inflate_end(&stream);
 			status = -1;
@@ -1289,7 +1290,7 @@ int oid_object_info_extended(struct repository *r, const struct object_id *oid,
 			return -1;
 
 		/* Most likely it's a loose object. */
-		if (!sha1_loose_object_info(r, real->hash, oi, flags))
+		if (!loose_object_info(r, real, oi, flags))
 			return 0;
 
 		/* Not a loose object; someone else may have just packed it. */
@@ -1417,7 +1418,7 @@ void *read_object_file_extended(const struct object_id *oid,
 		die(_("replacement %s not found for %s"),
 		    oid_to_hex(repl), oid_to_hex(oid));
 
-	if (!stat_sha1_file(the_repository, repl->hash, &st, &path))
+	if (!stat_loose_object(the_repository, repl, &st, &path))
 		die(_("loose object %s (stored in %s) is corrupt"),
 		    oid_to_hex(repl), path);
 
@@ -1552,7 +1553,7 @@ int hash_object_file(const void *buf, unsigned long len, const char *type,
 }
 
 /* Finalize a file on disk, and close it. */
-static void close_sha1_file(int fd)
+static void close_loose_object(int fd)
 {
 	if (fsync_object_files)
 		fsync_or_die(fd, "sha1 file");
@@ -1617,7 +1618,7 @@ static int write_loose_object(const struct object_id *oid, char *hdr,
 	static struct strbuf tmp_file = STRBUF_INIT;
 	static struct strbuf filename = STRBUF_INIT;
 
-	loose_object_path(the_repository, &filename, oid->hash);
+	loose_object_path(the_repository, &filename, oid);
 
 	fd = create_tmpfile(&tmp_file, filename.buf);
 	if (fd < 0) {
@@ -1665,7 +1666,7 @@ static int write_loose_object(const struct object_id *oid, char *hdr,
 		die(_("confused by unstable object source data for %s"),
 		    oid_to_hex(oid));
 
-	close_sha1_file(fd);
+	close_loose_object(fd);
 
 	if (mtime) {
 		struct utimbuf utb;
@@ -2260,7 +2261,7 @@ int read_loose_object(const char *path,
 
 	*contents = NULL;
 
-	map = map_sha1_file_1(the_repository, path, NULL, &mapsize);
+	map = map_loose_object_1(the_repository, path, NULL, &mapsize);
 	if (!map) {
 		error_errno(_("unable to mmap %s"), path);
 		goto out;
diff --git a/streaming.c b/streaming.c
index ac7c7a22f9..9049146bc1 100644
--- a/streaming.c
+++ b/streaming.c
@@ -338,8 +338,8 @@ static struct stream_vtbl loose_vtbl = {
 
 static open_method_decl(loose)
 {
-	st->u.loose.mapped = map_sha1_file(the_repository,
-					   oid->hash, &st->u.loose.mapsize);
+	st->u.loose.mapped = map_loose_object(the_repository,
+					      oid, &st->u.loose.mapsize);
 	if (!st->u.loose.mapped)
 		return -1;
 	if ((unpack_sha1_header(&st->z,
-- 
2.20.1.470.g640a3e2614

[PATCH 05/11] sha1-file: modernize loose header/stream functions

[Jeff King]

As with the open/map/close functions for loose objects that were
recently converted, the functions for parsing the loose object stream
use the name "sha1" and a bare "unsigned char *". Let's fix that so that
unpack_sha1_header() becomes unpack_loose_header(), etc.

These conversions are less clear-cut than the file access functions.
You could argue that the they are parsing Git's canonical object format
(i.e., "type size\0contents", over which we compute the hash), which is
not strictly tied to loose storage. But in practice these functions are
used only for loose objects, and using the term "loose_header" (instead
of "object_header") distinguishes it from the object header found in
packfiles (which contains the same information in a different format).

Signed-off-by: Jeff King <peff@peff.net>
---
Of course "loose_object_header" would be even more exact, but is quite
long. ;)

 cache.h     |  4 +--
 sha1-file.c | 84 +++++++++++++++++++++++++++--------------------------
 streaming.c | 12 ++++----
 3 files changed, 51 insertions(+), 49 deletions(-)

diff --git a/cache.h b/cache.h
index 587512747b..653c36d0b7 100644
--- a/cache.h
+++ b/cache.h
@@ -1269,8 +1269,8 @@ extern char *xdg_cache_home(const char *filename);
 
 extern int git_open_cloexec(const char *name, int flags);
 #define git_open(name) git_open_cloexec(name, O_RDONLY)
-extern int unpack_sha1_header(git_zstream *stream, unsigned char *map, unsigned long mapsize, void *buffer, unsigned long bufsiz);
-extern int parse_sha1_header(const char *hdr, unsigned long *sizep);
+extern int unpack_loose_header(git_zstream *stream, unsigned char *map, unsigned long mapsize, void *buffer, unsigned long bufsiz);
+extern int parse_loose_header(const char *hdr, unsigned long *sizep);
 
 extern int check_object_signature(const struct object_id *oid, void *buf, unsigned long size, const char *type);
 
diff --git a/sha1-file.c b/sha1-file.c
index cd8e5f005a..4938258ed1 100644
--- a/sha1-file.c
+++ b/sha1-file.c
@@ -976,9 +976,9 @@ void *map_loose_object(struct repository *r,
 	return map_loose_object_1(r, NULL, oid, size);
 }
 
-static int unpack_sha1_short_header(git_zstream *stream,
-				    unsigned char *map, unsigned long mapsize,
-				    void *buffer, unsigned long bufsiz)
+static int unpack_loose_short_header(git_zstream *stream,
+				     unsigned char *map, unsigned long mapsize,
+				     void *buffer, unsigned long bufsiz)
 {
 	/* Get the data stream */
 	memset(stream, 0, sizeof(*stream));
@@ -991,12 +991,12 @@ static int unpack_sha1_short_header(git_zstream *stream,
 	return git_inflate(stream, 0);
 }
 
-int unpack_sha1_header(git_zstream *stream,
-		       unsigned char *map, unsigned long mapsize,
-		       void *buffer, unsigned long bufsiz)
+int unpack_loose_header(git_zstream *stream,
+			unsigned char *map, unsigned long mapsize,
+			void *buffer, unsigned long bufsiz)
 {
-	int status = unpack_sha1_short_header(stream, map, mapsize,
-					      buffer, bufsiz);
+	int status = unpack_loose_short_header(stream, map, mapsize,
+					       buffer, bufsiz);
 
 	if (status < Z_OK)
 		return status;
@@ -1007,13 +1007,13 @@ int unpack_sha1_header(git_zstream *stream,
 	return 0;
 }
 
-static int unpack_sha1_header_to_strbuf(git_zstream *stream, unsigned char *map,
-					unsigned long mapsize, void *buffer,
-					unsigned long bufsiz, struct strbuf *header)
+static int unpack_loose_header_to_strbuf(git_zstream *stream, unsigned char *map,
+					 unsigned long mapsize, void *buffer,
+					 unsigned long bufsiz, struct strbuf *header)
 {
 	int status;
 
-	status = unpack_sha1_short_header(stream, map, mapsize, buffer, bufsiz);
+	status = unpack_loose_short_header(stream, map, mapsize, buffer, bufsiz);
 	if (status < Z_OK)
 		return -1;
 
@@ -1043,7 +1043,9 @@ static int unpack_sha1_header_to_strbuf(git_zstream *stream, unsigned char *map,
 	return -1;
 }
 
-static void *unpack_sha1_rest(git_zstream *stream, void *buffer, unsigned long size, const unsigned char *sha1)
+static void *unpack_loose_rest(git_zstream *stream,
+			       void *buffer, unsigned long size,
+			       const struct object_id *oid)
 {
 	int bytes = strlen(buffer) + 1;
 	unsigned char *buf = xmallocz(size);
@@ -1080,10 +1082,10 @@ static void *unpack_sha1_rest(git_zstream *stream, void *buffer, unsigned long s
 	}
 
 	if (status < 0)
-		error(_("corrupt loose object '%s'"), sha1_to_hex(sha1));
+		error(_("corrupt loose object '%s'"), oid_to_hex(oid));
 	else if (stream->avail_in)
 		error(_("garbage at end of loose object '%s'"),
-		      sha1_to_hex(sha1));
+		      oid_to_hex(oid));
 	free(buf);
 	return NULL;
 }
@@ -1093,8 +1095,8 @@ static void *unpack_sha1_rest(git_zstream *stream, void *buffer, unsigned long s
  * too permissive for what we want to check. So do an anal
  * object header parse by hand.
  */
-static int parse_sha1_header_extended(const char *hdr, struct object_info *oi,
-			       unsigned int flags)
+static int parse_loose_header_extended(const char *hdr, struct object_info *oi,
+				       unsigned int flags)
 {
 	const char *type_buf = hdr;
 	unsigned long size;
@@ -1154,12 +1156,12 @@ static int parse_sha1_header_extended(const char *hdr, struct object_info *oi,
 	return *hdr ? -1 : type;
 }
 
-int parse_sha1_header(const char *hdr, unsigned long *sizep)
+int parse_loose_header(const char *hdr, unsigned long *sizep)
 {
 	struct object_info oi = OBJECT_INFO_INIT;
 
 	oi.sizep = sizep;
-	return parse_sha1_header_extended(hdr, &oi, 0);
+	return parse_loose_header_extended(hdr, &oi, 0);
 }
 
 static int loose_object_info(struct repository *r,
@@ -1207,24 +1209,24 @@ static int loose_object_info(struct repository *r,
 	if (oi->disk_sizep)
 		*oi->disk_sizep = mapsize;
 	if ((flags & OBJECT_INFO_ALLOW_UNKNOWN_TYPE)) {
-		if (unpack_sha1_header_to_strbuf(&stream, map, mapsize, hdr, sizeof(hdr), &hdrbuf) < 0)
+		if (unpack_loose_header_to_strbuf(&stream, map, mapsize, hdr, sizeof(hdr), &hdrbuf) < 0)
 			status = error(_("unable to unpack %s header with --allow-unknown-type"),
 				       oid_to_hex(oid));
-	} else if (unpack_sha1_header(&stream, map, mapsize, hdr, sizeof(hdr)) < 0)
+	} else if (unpack_loose_header(&stream, map, mapsize, hdr, sizeof(hdr)) < 0)
 		status = error(_("unable to unpack %s header"),
 			       oid_to_hex(oid));
 	if (status < 0)
 		; /* Do nothing */
 	else if (hdrbuf.len) {
-		if ((status = parse_sha1_header_extended(hdrbuf.buf, oi, flags)) < 0)
+		if ((status = parse_loose_header_extended(hdrbuf.buf, oi, flags)) < 0)
 			status = error(_("unable to parse %s header with --allow-unknown-type"),
 				       oid_to_hex(oid));
-	} else if ((status = parse_sha1_header_extended(hdr, oi, flags)) < 0)
+	} else if ((status = parse_loose_header_extended(hdr, oi, flags)) < 0)
 		status = error(_("unable to parse %s header"), oid_to_hex(oid));
 
 	if (status >= 0 && oi->contentp) {
-		*oi->contentp = unpack_sha1_rest(&stream, hdr,
-						 *oi->sizep, oid->hash);
+		*oi->contentp = unpack_loose_rest(&stream, hdr,
+						  *oi->sizep, oid);
 		if (!*oi->contentp) {
 			git_inflate_end(&stream);
 			status = -1;
@@ -2189,14 +2191,14 @@ void odb_clear_loose_cache(struct object_directory *odb)
 	       sizeof(odb->loose_objects_subdir_seen));
 }
 
-static int check_stream_sha1(git_zstream *stream,
-			     const char *hdr,
-			     unsigned long size,
-			     const char *path,
-			     const unsigned char *expected_sha1)
+static int check_stream_oid(git_zstream *stream,
+			    const char *hdr,
+			    unsigned long size,
+			    const char *path,
+			    const struct object_id *expected_oid)
 {
 	git_hash_ctx c;
-	unsigned char real_sha1[GIT_MAX_RAWSZ];
+	struct object_id real_oid;
 	unsigned char buf[4096];
 	unsigned long total_read;
 	int status = Z_OK;
@@ -2212,7 +2214,7 @@ static int check_stream_sha1(git_zstream *stream,
 
 	/*
 	 * This size comparison must be "<=" to read the final zlib packets;
-	 * see the comment in unpack_sha1_rest for details.
+	 * see the comment in unpack_loose_rest for details.
 	 */
 	while (total_read <= size &&
 	       (status == Z_OK ||
@@ -2228,19 +2230,19 @@ static int check_stream_sha1(git_zstream *stream,
 	git_inflate_end(stream);
 
 	if (status != Z_STREAM_END) {
-		error(_("corrupt loose object '%s'"), sha1_to_hex(expected_sha1));
+		error(_("corrupt loose object '%s'"), oid_to_hex(expected_oid));
 		return -1;
 	}
 	if (stream->avail_in) {
 		error(_("garbage at end of loose object '%s'"),
-		      sha1_to_hex(expected_sha1));
+		      oid_to_hex(expected_oid));
 		return -1;
 	}
 
-	the_hash_algo->final_fn(real_sha1, &c);
-	if (!hasheq(expected_sha1, real_sha1)) {
+	the_hash_algo->final_fn(real_oid.hash, &c);
+	if (!oideq(expected_oid, &real_oid)) {
 		error(_("sha1 mismatch for %s (expected %s)"), path,
-		      sha1_to_hex(expected_sha1));
+		      oid_to_hex(expected_oid));
 		return -1;
 	}
 
@@ -2267,12 +2269,12 @@ int read_loose_object(const char *path,
 		goto out;
 	}
 
-	if (unpack_sha1_header(&stream, map, mapsize, hdr, sizeof(hdr)) < 0) {
+	if (unpack_loose_header(&stream, map, mapsize, hdr, sizeof(hdr)) < 0) {
 		error(_("unable to unpack header of %s"), path);
 		goto out;
 	}
 
-	*type = parse_sha1_header(hdr, size);
+	*type = parse_loose_header(hdr, size);
 	if (*type < 0) {
 		error(_("unable to parse header of %s"), path);
 		git_inflate_end(&stream);
@@ -2280,10 +2282,10 @@ int read_loose_object(const char *path,
 	}
 
 	if (*type == OBJ_BLOB && *size > big_file_threshold) {
-		if (check_stream_sha1(&stream, hdr, *size, path, expected_oid->hash) < 0)
+		if (check_stream_oid(&stream, hdr, *size, path, expected_oid) < 0)
 			goto out;
 	} else {
-		*contents = unpack_sha1_rest(&stream, hdr, *size, expected_oid->hash);
+		*contents = unpack_loose_rest(&stream, hdr, *size, expected_oid);
 		if (!*contents) {
 			error(_("unable to unpack contents of %s"), path);
 			git_inflate_end(&stream);
diff --git a/streaming.c b/streaming.c
index 9049146bc1..998e6285d7 100644
--- a/streaming.c
+++ b/streaming.c
@@ -342,12 +342,12 @@ static open_method_decl(loose)
 					      oid, &st->u.loose.mapsize);
 	if (!st->u.loose.mapped)
 		return -1;
-	if ((unpack_sha1_header(&st->z,
-				st->u.loose.mapped,
-				st->u.loose.mapsize,
-				st->u.loose.hdr,
-				sizeof(st->u.loose.hdr)) < 0) ||
-	    (parse_sha1_header(st->u.loose.hdr, &st->size) < 0)) {
+	if ((unpack_loose_header(&st->z,
+				 st->u.loose.mapped,
+				 st->u.loose.mapsize,
+				 st->u.loose.hdr,
+				 sizeof(st->u.loose.hdr)) < 0) ||
+	    (parse_loose_header(st->u.loose.hdr, &st->size) < 0)) {
 		git_inflate_end(&st->z);
 		munmap(st->u.loose.mapped, st->u.loose.mapsize);
 		return -1;
-- 
2.20.1.470.g640a3e2614

[PATCH 06/11] sha1-file: convert pass-through functions to object_id

[Jeff King]

These two static functions, read_object() and quick_has_loose(), both
have to hashcpy() their bare-sha1 arguments into object_id structs to
pass them along. Since all of their callers actually have object_id
structs in the first place, we can eliminate the copying by adjusting
their input parameters.

Signed-off-by: Jeff King <peff@peff.net>
---
 sha1-file.c | 20 +++++++-------------
 1 file changed, 7 insertions(+), 13 deletions(-)

diff --git a/sha1-file.c b/sha1-file.c
index 4938258ed1..589a666686 100644
--- a/sha1-file.c
+++ b/sha1-file.c
@@ -922,16 +922,13 @@ static int open_loose_object(struct repository *r,
 }
 
 static int quick_has_loose(struct repository *r,
-			   const unsigned char *sha1)
+			   const struct object_id *oid)
 {
-	struct object_id oid;
 	struct object_directory *odb;
 
-	hashcpy(oid.hash, sha1);
-
 	prepare_alt_odb(r);
 	for (odb = r->objects->odb; odb; odb = odb->next) {
-		if (oid_array_lookup(odb_loose_cache(odb, &oid), &oid) >= 0)
+		if (oid_array_lookup(odb_loose_cache(odb, oid), oid) >= 0)
 			return 1;
 	}
 	return 0;
@@ -1191,7 +1188,7 @@ static int loose_object_info(struct repository *r,
 		const char *path;
 		struct stat st;
 		if (!oi->disk_sizep && (flags & OBJECT_INFO_QUICK))
-			return quick_has_loose(r, oid->hash) ? 0 : -1;
+			return quick_has_loose(r, oid) ? 0 : -1;
 		if (stat_loose_object(r, oid, &st, &path) < 0)
 			return -1;
 		if (oi->disk_sizep)
@@ -1355,19 +1352,16 @@ int oid_object_info(struct repository *r,
 	return type;
 }
 
-static void *read_object(const unsigned char *sha1, enum object_type *type,
+static void *read_object(const struct object_id *oid, enum object_type *type,
 			 unsigned long *size)
 {
-	struct object_id oid;
 	struct object_info oi = OBJECT_INFO_INIT;
 	void *content;
 	oi.typep = type;
 	oi.sizep = size;
 	oi.contentp = &content;
 
-	hashcpy(oid.hash, sha1);
-
-	if (oid_object_info_extended(the_repository, &oid, &oi, 0) < 0)
+	if (oid_object_info_extended(the_repository, oid, &oi, 0) < 0)
 		return NULL;
 	return content;
 }
@@ -1408,7 +1402,7 @@ void *read_object_file_extended(const struct object_id *oid,
 		lookup_replace_object(the_repository, oid) : oid;
 
 	errno = 0;
-	data = read_object(repl->hash, type, size);
+	data = read_object(repl, type, size);
 	if (data)
 		return data;
 
@@ -1748,7 +1742,7 @@ int force_object_loose(const struct object_id *oid, time_t mtime)
 
 	if (has_loose_object(oid))
 		return 0;
-	buf = read_object(oid->hash, &type, &len);
+	buf = read_object(oid, &type, &len);
 	if (!buf)
 		return error(_("cannot read sha1_file for %s"), oid_to_hex(oid));
 	hdrlen = xsnprintf(hdr, sizeof(hdr), "%s %"PRIuMAX , type_name(type), (uintmax_t)len) + 1;
-- 
2.20.1.470.g640a3e2614

[PATCH 07/11] convert has_sha1_file() callers to has_object_file()

[Jeff King]

The only remaining callers of has_sha1_file() actually have an object_id
already. They can use the "object" variant, rather than dereferencing
the hash themselves.

The code changes here were completely generated by the included
coccinelle patch.

Signed-off-by: Jeff King <peff@peff.net>
---
 apply.c                            |  2 +-
 builtin/fetch.c                    |  7 +++----
 builtin/index-pack.c               |  2 +-
 builtin/reflog.c                   |  2 +-
 builtin/show-ref.c                 |  2 +-
 bulk-checkin.c                     |  2 +-
 cache-tree.c                       |  4 ++--
 contrib/coccinelle/object_id.cocci | 32 ++++++++++++++++++++++++++++++
 http-walker.c                      |  4 ++--
 refs.c                             |  2 +-
 send-pack.c                        |  2 +-
 sha1-file.c                        |  2 +-
 12 files changed, 47 insertions(+), 16 deletions(-)

diff --git a/apply.c b/apply.c
index 01793d6126..b8e257ead2 100644
--- a/apply.c
+++ b/apply.c
@@ -3183,7 +3183,7 @@ static int apply_binary(struct apply_state *state,
 		return 0; /* deletion patch */
 	}
 
-	if (has_sha1_file(oid.hash)) {
+	if (has_object_file(&oid)) {
 		/* We already have the postimage */
 		enum object_type type;
 		unsigned long size;
diff --git a/builtin/fetch.c b/builtin/fetch.c
index e0140327aa..57f35c6a0a 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -317,8 +317,7 @@ static void find_non_local_tags(const struct ref *refs,
 			    !has_object_file_with_flags(&ref->old_oid,
 							OBJECT_INFO_QUICK) &&
 			    !will_fetch(head, ref->old_oid.hash) &&
-			    !has_sha1_file_with_flags(item->oid.hash,
-						      OBJECT_INFO_QUICK) &&
+			    !has_object_file_with_flags(&item->oid, OBJECT_INFO_QUICK) &&
 			    !will_fetch(head, item->oid.hash))
 				oidclr(&item->oid);
 			item = NULL;
@@ -332,7 +331,7 @@ static void find_non_local_tags(const struct ref *refs,
 		 * fetch.
 		 */
 		if (item &&
-		    !has_sha1_file_with_flags(item->oid.hash, OBJECT_INFO_QUICK) &&
+		    !has_object_file_with_flags(&item->oid, OBJECT_INFO_QUICK) &&
 		    !will_fetch(head, item->oid.hash))
 			oidclr(&item->oid);
 
@@ -353,7 +352,7 @@ static void find_non_local_tags(const struct ref *refs,
 	 * checked to see if it needs fetching.
 	 */
 	if (item &&
-	    !has_sha1_file_with_flags(item->oid.hash, OBJECT_INFO_QUICK) &&
+	    !has_object_file_with_flags(&item->oid, OBJECT_INFO_QUICK) &&
 	    !will_fetch(head, item->oid.hash))
 		oidclr(&item->oid);
 
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index ac1f4ea9a7..31046c7a0a 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -772,7 +772,7 @@ static void sha1_object(const void *data, struct object_entry *obj_entry,
 	if (startup_info->have_repository) {
 		read_lock();
 		collision_test_needed =
-			has_sha1_file_with_flags(oid->hash, OBJECT_INFO_QUICK);
+			has_object_file_with_flags(oid, OBJECT_INFO_QUICK);
 		read_unlock();
 	}
 
diff --git a/builtin/reflog.c b/builtin/reflog.c
index 64a8df4f25..45e9e15006 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -94,7 +94,7 @@ static int tree_is_complete(const struct object_id *oid)
 	init_tree_desc(&desc, tree->buffer, tree->size);
 	complete = 1;
 	while (tree_entry(&desc, &entry)) {
-		if (!has_sha1_file(entry.oid->hash) ||
+		if (!has_object_file(entry.oid) ||
 		    (S_ISDIR(entry.mode) && !tree_is_complete(entry.oid))) {
 			tree->object.flags |= INCOMPLETE;
 			complete = 0;
diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index ed888ffa48..6a706c02a6 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -23,7 +23,7 @@ static void show_one(const char *refname, const struct object_id *oid)
 	const char *hex;
 	struct object_id peeled;
 
-	if (!has_sha1_file(oid->hash))
+	if (!has_object_file(oid))
 		die("git show-ref: bad ref %s (%s)", refname,
 		    oid_to_hex(oid));
 
diff --git a/bulk-checkin.c b/bulk-checkin.c
index 409ecb566b..39ee7d6107 100644
--- a/bulk-checkin.c
+++ b/bulk-checkin.c
@@ -67,7 +67,7 @@ static int already_written(struct bulk_checkin_state *state, struct object_id *o
 	int i;
 
 	/* The object may already exist in the repository */
-	if (has_sha1_file(oid->hash))
+	if (has_object_file(oid))
 		return 1;
 
 	/* Might want to keep the list sorted */
diff --git a/cache-tree.c b/cache-tree.c
index 190c6e5aa6..47f3464a1f 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -225,7 +225,7 @@ int cache_tree_fully_valid(struct cache_tree *it)
 	int i;
 	if (!it)
 		return 0;
-	if (it->entry_count < 0 || !has_sha1_file(it->oid.hash))
+	if (it->entry_count < 0 || !has_object_file(&it->oid))
 		return 0;
 	for (i = 0; i < it->subtree_nr; i++) {
 		if (!cache_tree_fully_valid(it->down[i]->cache_tree))
@@ -253,7 +253,7 @@ static int update_one(struct cache_tree *it,
 
 	*skip_count = 0;
 
-	if (0 <= it->entry_count && has_sha1_file(it->oid.hash))
+	if (0 <= it->entry_count && has_object_file(&it->oid))
 		return it->entry_count;
 
 	/*
diff --git a/contrib/coccinelle/object_id.cocci b/contrib/coccinelle/object_id.cocci
index 6a7cf3e02d..73886ae583 100644
--- a/contrib/coccinelle/object_id.cocci
+++ b/contrib/coccinelle/object_id.cocci
@@ -147,3 +147,35 @@ expression E1, E2;
 - hashcmp(E1, E2) != 0
 + !hasheq(E1, E2)
   ...>}
+
+@@
+struct object_id OID;
+@@
+- has_sha1_file(OID.hash)
++ has_object_file(&OID)
+
+@@
+identifier f != has_object_file;
+struct object_id *OIDPTR;
+@@
+  f(...) {<...
+- has_sha1_file(OIDPTR->hash)
++ has_object_file(OIDPTR)
+  ...>}
+
+@@
+struct object_id OID;
+expression E;
+@@
+- has_sha1_file_with_flags(OID.hash, E)
++ has_object_file_with_flags(&OID, E)
+
+@@
+identifier f != has_object_file_with_flags;
+struct object_id *OIDPTR;
+expression E;
+@@
+  f(...) {<...
+- has_sha1_file_with_flags(OIDPTR->hash, E)
++ has_object_file_with_flags(OIDPTR, E)
+  ...>}
diff --git a/http-walker.c b/http-walker.c
index 29b59e2fe0..8ae5d76c6a 100644
--- a/http-walker.c
+++ b/http-walker.c
@@ -131,7 +131,7 @@ static int fill_active_slot(struct walker *walker)
 	list_for_each_safe(pos, tmp, head) {
 		obj_req = list_entry(pos, struct object_request, node);
 		if (obj_req->state == WAITING) {
-			if (has_sha1_file(obj_req->oid.hash))
+			if (has_object_file(&obj_req->oid))
 				obj_req->state = COMPLETE;
 			else {
 				start_object_request(walker, obj_req);
@@ -489,7 +489,7 @@ static int fetch_object(struct walker *walker, unsigned char *sha1)
 	if (obj_req == NULL)
 		return error("Couldn't find request for %s in the queue", hex);
 
-	if (has_sha1_file(obj_req->oid.hash)) {
+	if (has_object_file(&obj_req->oid)) {
 		if (obj_req->req != NULL)
 			abort_http_object_request(obj_req->req);
 		abort_object_request(obj_req);
diff --git a/refs.c b/refs.c
index f9936355cd..142888a40a 100644
--- a/refs.c
+++ b/refs.c
@@ -188,7 +188,7 @@ int ref_resolves_to_object(const char *refname,
 {
 	if (flags & REF_ISBROKEN)
 		return 0;
-	if (!has_sha1_file(oid->hash)) {
+	if (!has_object_file(oid)) {
 		error(_("%s does not point to a valid object!"), refname);
 		return 0;
 	}
diff --git a/send-pack.c b/send-pack.c
index f692686770..c673d3ed06 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -40,7 +40,7 @@ int option_parse_push_signed(const struct option *opt,
 
 static void feed_object(const struct object_id *oid, FILE *fh, int negative)
 {
-	if (negative && !has_sha1_file(oid->hash))
+	if (negative && !has_object_file(oid))
 		return;
 
 	if (negative)
diff --git a/sha1-file.c b/sha1-file.c
index 589a666686..449456f2ad 100644
--- a/sha1-file.c
+++ b/sha1-file.c
@@ -1372,7 +1372,7 @@ int pretend_object_file(void *buf, unsigned long len, enum object_type type,
 	struct cached_object *co;
 
 	hash_object_file(buf, len, type_name(type), oid);
-	if (has_sha1_file(oid->hash) || find_cached_object(oid))
+	if (has_object_file(oid) || find_cached_object(oid))
 		return 0;
 	ALLOC_GROW(cached_objects, cached_object_nr + 1, cached_object_alloc);
 	co = &cached_objects[cached_object_nr++];
-- 
2.20.1.470.g640a3e2614

[PATCH 08/11] sha1-file: drop has_sha1_file()

[Jeff King]

There are no callers left of has_sha1_file() or its with_flags()
variant. Let's drop them, and convert has_object_file() from a wrapper
into the "real" function. Ironically, the sha1 variant was just copying
into an object_id internally, so the resulting code is actually shorter!

We can also drop the coccinelle rules for catching has_sha1_file()
callers. Since the function no longer exists, the compiler will do that
for us.

Signed-off-by: Jeff King <peff@peff.net>
---
 contrib/coccinelle/object_id.cocci | 32 ------------------------------
 object-store.h                     | 12 ++++-------
 sha1-file.c                        | 16 ++-------------
 3 files changed, 6 insertions(+), 54 deletions(-)

diff --git a/contrib/coccinelle/object_id.cocci b/contrib/coccinelle/object_id.cocci
index 73886ae583..6a7cf3e02d 100644
--- a/contrib/coccinelle/object_id.cocci
+++ b/contrib/coccinelle/object_id.cocci
@@ -147,35 +147,3 @@ expression E1, E2;
 - hashcmp(E1, E2) != 0
 + !hasheq(E1, E2)
   ...>}
-
-@@
-struct object_id OID;
-@@
-- has_sha1_file(OID.hash)
-+ has_object_file(&OID)
-
-@@
-identifier f != has_object_file;
-struct object_id *OIDPTR;
-@@
-  f(...) {<...
-- has_sha1_file(OIDPTR->hash)
-+ has_object_file(OIDPTR)
-  ...>}
-
-@@
-struct object_id OID;
-expression E;
-@@
-- has_sha1_file_with_flags(OID.hash, E)
-+ has_object_file_with_flags(&OID, E)
-
-@@
-identifier f != has_object_file_with_flags;
-struct object_id *OIDPTR;
-expression E;
-@@
-  f(...) {<...
-- has_sha1_file_with_flags(OIDPTR->hash, E)
-+ has_object_file_with_flags(OIDPTR, E)
-  ...>}
diff --git a/object-store.h b/object-store.h
index 6b1c408753..9e8ac6a1d4 100644
--- a/object-store.h
+++ b/object-store.h
@@ -209,20 +209,16 @@ int read_loose_object(const char *path,
 		      void **contents);
 
 /*
- * Convenience for sha1_object_info_extended() with a NULL struct
+ * Convenience for oid_object_info_extended() with a NULL struct
  * object_info. OBJECT_INFO_SKIP_CACHED is automatically set; pass
  * nonzero flags to also set other flags.
  */
-extern int has_sha1_file_with_flags(const unsigned char *sha1, int flags);
-static inline int has_sha1_file(const unsigned char *sha1)
+int has_object_file_with_flags(const struct object_id *oid, int flags);
+static inline int has_object_file(const struct object_id *oid)
 {
-	return has_sha1_file_with_flags(sha1, 0);
+	return has_object_file_with_flags(oid, 0);
 }
 
-/* Same as the above, except for struct object_id. */
-extern int has_object_file(const struct object_id *oid);
-extern int has_object_file_with_flags(const struct object_id *oid, int flags);
-
 /*
  * Return true iff an alternate object database has a loose object
  * with the specified name.  This function does not respect replace
diff --git a/sha1-file.c b/sha1-file.c
index 449456f2ad..da6d78976f 100644
--- a/sha1-file.c
+++ b/sha1-file.c
@@ -1752,26 +1752,14 @@ int force_object_loose(const struct object_id *oid, time_t mtime)
 	return ret;
 }
 
-int has_sha1_file_with_flags(const unsigned char *sha1, int flags)
+int has_object_file_with_flags(const struct object_id *oid, int flags)
 {
-	struct object_id oid;
 	if (!startup_info->have_repository)
 		return 0;
-	hashcpy(oid.hash, sha1);
-	return oid_object_info_extended(the_repository, &oid, NULL,
+	return oid_object_info_extended(the_repository, oid, NULL,
 					flags | OBJECT_INFO_SKIP_CACHED) >= 0;
 }
 
-int has_object_file(const struct object_id *oid)
-{
-	return has_sha1_file(oid->hash);
-}
-
-int has_object_file_with_flags(const struct object_id *oid, int flags)
-{
-	return has_sha1_file_with_flags(oid->hash, flags);
-}
-
 static void check_tree(const void *buf, size_t size)
 {
 	struct tree_desc desc;
-- 
2.20.1.470.g640a3e2614

[PATCH 09/11] sha1-file: prefer "loose object file" to "sha1 file" in messages

[Jeff King]

When we're reporting an error for a loose object, let's use that term.
It's more consistent with other parts of Git, and it is future-proof
against changes to the hash function.

Signed-off-by: Jeff King <peff@peff.net>
---
 sha1-file.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/sha1-file.c b/sha1-file.c
index da6d78976f..07cc9b548b 100644
--- a/sha1-file.c
+++ b/sha1-file.c
@@ -1552,9 +1552,9 @@ int hash_object_file(const void *buf, unsigned long len, const char *type,
 static void close_loose_object(int fd)
 {
 	if (fsync_object_files)
-		fsync_or_die(fd, "sha1 file");
+		fsync_or_die(fd, "loose object file");
 	if (close(fd) != 0)
-		die_errno(_("error when closing sha1 file"));
+		die_errno(_("error when closing loose object file"));
 }
 
 /* Size of directory component, including the ending '/' */
@@ -1645,7 +1645,7 @@ static int write_loose_object(const struct object_id *oid, char *hdr,
 		ret = git_deflate(&stream, Z_FINISH);
 		the_hash_algo->update_fn(&c, in0, stream.next_in - in0);
 		if (write_buffer(fd, compressed, stream.next_out - compressed) < 0)
-			die(_("unable to write sha1 file"));
+			die(_("unable to write loose object file"));
 		stream.next_out = compressed;
 		stream.avail_out = sizeof(compressed);
 	} while (ret == Z_OK);
-- 
2.20.1.470.g640a3e2614

[PATCH 10/11] sha1-file: avoid "sha1 file" for generic use in messages

[Jeff King]

These error messages say "sha1 file", which is vague and not common in
user-facing documentation. Unlike the conversions from the previous
commit, these do not always refer to loose objects.

In finalize_object_file() we could be dealing with a packfile. Let's
just say "unable to write file" instead; since we include the filename,
the nature of the file is clear from the rest of the message.

In force_object_loose(), we're calling into read_object(), which could
actually be _any_ type of object. Just say "object".

Signed-off-by: Jeff King <peff@peff.net>
---
 sha1-file.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sha1-file.c b/sha1-file.c
index 07cc9b548b..55a4782844 100644
--- a/sha1-file.c
+++ b/sha1-file.c
@@ -1521,7 +1521,7 @@ int finalize_object_file(const char *tmpfile, const char *filename)
 	unlink_or_warn(tmpfile);
 	if (ret) {
 		if (ret != EEXIST) {
-			return error_errno(_("unable to write sha1 filename %s"), filename);
+			return error_errno(_("unable to write file %s"), filename);
 		}
 		/* FIXME!!! Collision check here ? */
 	}
@@ -1744,7 +1744,7 @@ int force_object_loose(const struct object_id *oid, time_t mtime)
 		return 0;
 	buf = read_object(oid, &type, &len);
 	if (!buf)
-		return error(_("cannot read sha1_file for %s"), oid_to_hex(oid));
+		return error(_("cannot read object for %s"), oid_to_hex(oid));
 	hdrlen = xsnprintf(hdr, sizeof(hdr), "%s %"PRIuMAX , type_name(type), (uintmax_t)len) + 1;
 	ret = write_loose_object(oid, hdr, hdrlen, buf, len, mtime);
 	free(buf);
-- 
2.20.1.470.g640a3e2614

[PATCH 11/11] prefer "hash mismatch" to "sha1 mismatch"

[Jeff King]

To future-proof ourselves against a change in the hash, let's use the
more generic "hash mismatch" to refer to integrity problems. Note that
we do advertise this exact string in git-fsck(1). However, the message
itself is marked for translation, meaning we do not expect it to be
machine-readable.

While we're touching that documentation, let's also update it for
grammar and clarity.

Signed-off-by: Jeff King <peff@peff.net>
---
I'm actually a little nervous that we _shouldn't_ have marked the
messages that fsck produces for translation (and nor should we change
them here, but then we're stuck with the word "sha1" forever).

I actually think fsck ought to have a machine-readable output format,
but of course that does not help any existing scripts.

 Documentation/git-fsck.txt | 6 +++---
 object.c                   | 4 ++--
 sha1-file.c                | 4 ++--
 t/t1450-fsck.sh            | 2 +-
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/Documentation/git-fsck.txt b/Documentation/git-fsck.txt
index ab9a93fb9b..55950d9eea 100644
--- a/Documentation/git-fsck.txt
+++ b/Documentation/git-fsck.txt
@@ -140,9 +140,9 @@ dangling <type> <object>::
 	The <type> object <object>, is present in the database but never
 	'directly' used. A dangling commit could be a root node.
 
-sha1 mismatch <object>::
-	The database has an object who's sha1 doesn't match the
-	database value.
+hash mismatch <object>::
+	The database has an object whose hash doesn't match the
+	object database value.
 	This indicates a serious data integrity problem.
 
 Environment Variables
diff --git a/object.c b/object.c
index a5c5cf830f..df72914bdc 100644
--- a/object.c
+++ b/object.c
@@ -263,7 +263,7 @@ struct object *parse_object(struct repository *r, const struct object_id *oid)
 	    (!obj && has_object_file(oid) &&
 	     oid_object_info(r, oid, NULL) == OBJ_BLOB)) {
 		if (check_object_signature(repl, NULL, 0, NULL) < 0) {
-			error(_("sha1 mismatch %s"), oid_to_hex(oid));
+			error(_("hash mismatch %s"), oid_to_hex(oid));
 			return NULL;
 		}
 		parse_blob_buffer(lookup_blob(r, oid), NULL, 0);
@@ -274,7 +274,7 @@ struct object *parse_object(struct repository *r, const struct object_id *oid)
 	if (buffer) {
 		if (check_object_signature(repl, buffer, size, type_name(type)) < 0) {
 			free(buffer);
-			error(_("sha1 mismatch %s"), oid_to_hex(repl));
+			error(_("hash mismatch %s"), oid_to_hex(repl));
 			return NULL;
 		}
 
diff --git a/sha1-file.c b/sha1-file.c
index 55a4782844..a5726c3e73 100644
--- a/sha1-file.c
+++ b/sha1-file.c
@@ -2223,7 +2223,7 @@ static int check_stream_oid(git_zstream *stream,
 
 	the_hash_algo->final_fn(real_oid.hash, &c);
 	if (!oideq(expected_oid, &real_oid)) {
-		error(_("sha1 mismatch for %s (expected %s)"), path,
+		error(_("hash mismatch for %s (expected %s)"), path,
 		      oid_to_hex(expected_oid));
 		return -1;
 	}
@@ -2275,7 +2275,7 @@ int read_loose_object(const char *path,
 		}
 		if (check_object_signature(expected_oid, *contents,
 					 *size, type_name(*type))) {
-			error(_("sha1 mismatch for %s (expected %s)"), path,
+			error(_("hash mismatch for %s (expected %s)"), path,
 			      oid_to_hex(expected_oid));
 			free(*contents);
 			goto out;
diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index 2e5e979336..c61f972141 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -406,7 +406,7 @@ test_expect_success 'rev-list --verify-objects with bad sha1' '
 
 	test_might_fail git rev-list --verify-objects refs/heads/bogus >/dev/null 2>out &&
 	cat out &&
-	test_i18ngrep -q "error: sha1 mismatch 63ffffffffffffffffffffffffffffffffffffff" out
+	test_i18ngrep -q "error: hash mismatch 63ffffffffffffffffffffffffffffffffffffff" out
 '
 
 test_expect_success 'force fsck to ignore double author' '
-- 
2.20.1.470.g640a3e2614

Re: [PATCH 0/11] jk/loose-object-cache sha1/object_id fixups

[René Scharfe]

Am 07.01.2019 um 09:31 schrieb Jeff King:
> I also cleaned up my sha1/object_id patch and rebased it on top of what
> you have here. Though as I worked on it, it expanded in scope a bit.
> Possibly it should be a separate series entirely, but that would create
> some annoying textual conflicts on merge.
> 
>   [01/11]: sha1-file: fix outdated sha1 comment references
>   [02/11]: update comment references to sha1_object_info()
>   [03/11]: http: use struct object_id instead of bare sha1
>   [04/11]: sha1-file: modernize loose object file functions
>   [05/11]: sha1-file: modernize loose header/stream functions
>   [06/11]: sha1-file: convert pass-through functions to object_id
>   [07/11]: convert has_sha1_file() callers to has_object_file()
>   [08/11]: sha1-file: drop has_sha1_file()
>   [09/11]: sha1-file: prefer "loose object file" to "sha1 file" in messages
>   [10/11]: sha1-file: avoid "sha1 file" for generic use in messages
>   [11/11]: prefer "hash mismatch" to "sha1 mismatch"

I skimmed them; they look good to me.  6 and 8 are particularly
satisfying; getting rid of hash copy operations just feels nice. :)

Junio only took 1 to 5 into pu; 6, 7 and its sidekick 8, 10 and 11
conflict with sb/more-repo-in-api; 9 could go in unmodified.

René

Re: [PATCH 0/11] jk/loose-object-cache sha1/object_id fixups

[Junio C Hamano]

René Scharfe <l.s.r@web.de> writes:

> Am 07.01.2019 um 09:31 schrieb Jeff King:
>> I also cleaned up my sha1/object_id patch and rebased it on top of what
>> you have here. Though as I worked on it, it expanded in scope a bit.
>> Possibly it should be a separate series entirely, but that would create
>> some annoying textual conflicts on merge.
>> 
>>   [01/11]: sha1-file: fix outdated sha1 comment references
>>   [02/11]: update comment references to sha1_object_info()
>>   [03/11]: http: use struct object_id instead of bare sha1
>>   [04/11]: sha1-file: modernize loose object file functions
>>   [05/11]: sha1-file: modernize loose header/stream functions
>>   [06/11]: sha1-file: convert pass-through functions to object_id
>>   [07/11]: convert has_sha1_file() callers to has_object_file()
>>   [08/11]: sha1-file: drop has_sha1_file()
>>   [09/11]: sha1-file: prefer "loose object file" to "sha1 file" in messages
>>   [10/11]: sha1-file: avoid "sha1 file" for generic use in messages
>>   [11/11]: prefer "hash mismatch" to "sha1 mismatch"
>
> I skimmed them; they look good to me.  6 and 8 are particularly
> satisfying; getting rid of hash copy operations just feels nice. :)
>
> Junio only took 1 to 5 into pu; 6, 7 and its sidekick 8, 10 and 11
> conflict with sb/more-repo-in-api; 9 could go in unmodified.

I think these later steps are based on something a lot newer than
the result of applying your updates to the jk/loose-object-cache
series they fix.  I think I untangled and backported one of the
earlier commits but then I stopped after 05/11.

I do not think it is important to keep jk/loose-object-cache and
these two follow-up topics mergeable to the maintenance track, so
I'll see if the patches behave better if queued directly on top of
3b2f8a02 ("Merge branch 'jk/loose-object-cache'", 2019-01-04), or
even a yet newer random point on 'master'.

Thanks.

Re: [PATCH 0/11] jk/loose-object-cache sha1/object_id fixups

[Jeff King]

On Tue, Jan 08, 2019 at 09:39:48AM -0800, Junio C Hamano wrote:

> > I skimmed them; they look good to me.  6 and 8 are particularly
> > satisfying; getting rid of hash copy operations just feels nice. :)
> >
> > Junio only took 1 to 5 into pu; 6, 7 and its sidekick 8, 10 and 11
> > conflict with sb/more-repo-in-api; 9 could go in unmodified.
> 
> I think these later steps are based on something a lot newer than
> the result of applying your updates to the jk/loose-object-cache
> series they fix.  I think I untangled and backported one of the
> earlier commits but then I stopped after 05/11.

Sorry, I applied René's patches on top of master and then built on that,
treating it like a new topic. But of course your jk/loose-object-cache
is older.

> I do not think it is important to keep jk/loose-object-cache and
> these two follow-up topics mergeable to the maintenance track, so
> I'll see if the patches behave better if queued directly on top of
> 3b2f8a02 ("Merge branch 'jk/loose-object-cache'", 2019-01-04), or
> even a yet newer random point on 'master'.

Yeah, they should. I think one of them will need René's patch, which
changes the body of quick_has_loose(). I can roll it as a separate topic
if that's easier (or just wait a week or so until René's cleanups
graduate).

-Peff

Re: [PATCH 0/11] jk/loose-object-cache sha1/object_id fixups

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> Yeah, they should. I think one of them will need René's patch, which
> changes the body of quick_has_loose(). I can roll it as a separate topic
> if that's easier (or just wait a week or so until René's cleanups
> graduate).

Nah, what I got is already good to work with.  Both series are
straight-forward and I do not expect them needing long fermentation.

Re: [PATCH 0/11] jk/loose-object-cache sha1/object_id fixups

[Derrick Stolee]

On 1/8/2019 1:07 PM, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
>> Yeah, they should. I think one of them will need René's patch, which
>> changes the body of quick_has_loose(). I can roll it as a separate topic
>> if that's easier (or just wait a week or so until René's cleanups
>> graduate).
> Nah, what I got is already good to work with.  Both series are
> straight-forward and I do not expect them needing long fermentation.

I'm just chiming in to say that this series was a very satisfying read, 
and the changes were clear-cut and mechanical.

Thanks!
-Stolee

Re: [PATCH 0/11] jk/loose-object-cache sha1/object_id fixups

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Jeff King <peff@peff.net> writes:
>
>> Yeah, they should. I think one of them will need René's patch, which
>> changes the body of quick_has_loose(). I can roll it as a separate topic
>> if that's easier (or just wait a week or so until René's cleanups
>> graduate).
>
> Nah, what I got is already good to work with.  Both series are
> straight-forward and I do not expect them needing long fermentation.

Yikes, the conflicts with sb/more-repo-in-api is quite irritating.
I think I'll postpone the later parts of this series and ask this to
be sent after sb/more-repo-in-api matures a bit mroe.

Re: [PATCH 0/11] jk/loose-object-cache sha1/object_id fixups

[Jeff King]

On Tue, Jan 08, 2019 at 10:52:19AM -0800, Junio C Hamano wrote:

> Junio C Hamano <gitster@pobox.com> writes:
> 
> > Jeff King <peff@peff.net> writes:
> >
> >> Yeah, they should. I think one of them will need René's patch, which
> >> changes the body of quick_has_loose(). I can roll it as a separate topic
> >> if that's easier (or just wait a week or so until René's cleanups
> >> graduate).
> >
> > Nah, what I got is already good to work with.  Both series are
> > straight-forward and I do not expect them needing long fermentation.
> 
> Yikes, the conflicts with sb/more-repo-in-api is quite irritating.
> I think I'll postpone the later parts of this series and ask this to
> be sent after sb/more-repo-in-api matures a bit mroe.

There were several conflicts, but it was mostly just tedious textual
fixups. I pushed the result to:

  https://github.com/peff/git.git resolve-oid-more-repo

But I'm happy to wait and rebase if sb/more-repo-in-api is close to
graduating.

-Peff

Re: [PATCH 0/11] jk/loose-object-cache sha1/object_id fixups

[Stefan Beller]

> > Yikes, the conflicts with sb/more-repo-in-api is quite irritating.
> > I think I'll postpone the later parts of this series and ask this to
> > be sent after sb/more-repo-in-api matures a bit mroe.
>
> There were several conflicts, but it was mostly just tedious textual
> fixups. I pushed the result to:
>
>   https://github.com/peff/git.git resolve-oid-more-repo
>
> But I'm happy to wait and rebase if sb/more-repo-in-api is close to
> graduating.

The merge looks good to me, though I just looked quickly.
The series itself is also a pleasant read.

Stefan

Re: [PATCH 0/11] jk/loose-object-cache sha1/object_id fixups

[Stefan Beller]

On Wed, Jan 9, 2019 at 1:37 PM Stefan Beller <sbeller@google.com> wrote:
>
> > > Yikes, the conflicts with sb/more-repo-in-api is quite irritating.
> > > I think I'll postpone the later parts of this series and ask this to
> > > be sent after sb/more-repo-in-api matures a bit mroe.
> >
> > There were several conflicts, but it was mostly just tedious textual
> > fixups. I pushed the result to:
> >
> >   https://github.com/peff/git.git resolve-oid-more-repo
> >
> > But I'm happy to wait and rebase if sb/more-repo-in-api is close to
> > graduating.
>
> The merge looks good to me, though I just looked quickly.
> The series itself is also a pleasant read.

Compiling this leads to:

sha1-file.c:1424:33: error: incompatible pointer types passing 'const
struct object_id *' to parameter of type 'const unsigned char *'
[-Werror,-Wincompatible-pointer-types]
        if ((p = has_packed_and_bad(r, repl)) != NULL)
                                       ^~~~
./packfile.h:149:95: note: passing argument to parameter 'sha1' here
extern const struct packed_git *has_packed_and_bad(struct repository
*r, const unsigned char *sha1);

Re: [PATCH 0/11] jk/loose-object-cache sha1/object_id fixups

[Jeff King]

On Wed, Jan 09, 2019 at 02:42:28PM -0800, Stefan Beller wrote:

> On Wed, Jan 9, 2019 at 1:37 PM Stefan Beller <sbeller@google.com> wrote:
> >
> > > > Yikes, the conflicts with sb/more-repo-in-api is quite irritating.
> > > > I think I'll postpone the later parts of this series and ask this to
> > > > be sent after sb/more-repo-in-api matures a bit mroe.
> > >
> > > There were several conflicts, but it was mostly just tedious textual
> > > fixups. I pushed the result to:
> > >
> > >   https://github.com/peff/git.git resolve-oid-more-repo
> > >
> > > But I'm happy to wait and rebase if sb/more-repo-in-api is close to
> > > graduating.
> >
> > The merge looks good to me, though I just looked quickly.
> > The series itself is also a pleasant read.
> 
> Compiling this leads to:
> 
> sha1-file.c:1424:33: error: incompatible pointer types passing 'const
> struct object_id *' to parameter of type 'const unsigned char *'
> [-Werror,-Wincompatible-pointer-types]
>         if ((p = has_packed_and_bad(r, repl)) != NULL)
>                                        ^~~~
> ./packfile.h:149:95: note: passing argument to parameter 'sha1' here
> extern const struct packed_git *has_packed_and_bad(struct repository
> *r, const unsigned char *sha1);

Eek, sorry about that. I did the merge on a detached HEAD, and my
config.mak relaxes compilation warnings in that case (since I am often
sight-seeing to old versions that have warnings which have since been
fixed). And the result passes the tests since "repl" and "repl->hash"
are effectively the same pointer.

I've pushed up the fix (s/repl/repl->hash/). Thanks for noticing.

-Peff

[PATCH 4/3] object-store: retire odb_load_loose_cache()

[René Scharfe]

Inline odb_load_loose_cache() into its only remaining caller,
odb_loose_cache().  The latter offers a nicer interface for loading the
cache, as it doesn't require callers to deal with fanout directory
numbers directly.

Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
 object-store.h | 7 -------
 sha1-file.c    | 9 ++-------
 2 files changed, 2 insertions(+), 14 deletions(-)

diff --git a/object-store.h b/object-store.h
index 2fb6c0e4db..e16aa38cae 100644
--- a/object-store.h
+++ b/object-store.h
@@ -47,13 +47,6 @@ void add_to_alternates_file(const char *dir);
  */
 void add_to_alternates_memory(const char *dir);
 
-/*
- * Populate an odb's loose object cache for one particular subdirectory (i.e.,
- * the one that corresponds to the first byte of objects you're interested in,
- * from 0 to 255 inclusive).
- */
-void odb_load_loose_cache(struct object_directory *odb, int subdir_nr);
-
 /*
  * Populate and return the loose object cache array corresponding to the
  * given object ID.
diff --git a/sha1-file.c b/sha1-file.c
index c3c6e50704..efcb2cbe74 100644
--- a/sha1-file.c
+++ b/sha1-file.c
@@ -2154,12 +2154,6 @@ struct oid_array *odb_loose_cache(struct object_directory *odb,
 				  const struct object_id *oid)
 {
 	int subdir_nr = oid->hash[0];
-	odb_load_loose_cache(odb, subdir_nr);
-	return &odb->loose_objects_cache[subdir_nr];
-}
-
-void odb_load_loose_cache(struct object_directory *odb, int subdir_nr)
-{
 	struct strbuf buf = STRBUF_INIT;
 
 	if (subdir_nr < 0 ||
@@ -2167,7 +2161,7 @@ void odb_load_loose_cache(struct object_directory *odb, int subdir_nr)
 		BUG("subdir_nr out of range");
 
 	if (odb->loose_objects_subdir_seen[subdir_nr])
-		return;
+		return &odb->loose_objects_cache[subdir_nr];
 
 	strbuf_addstr(&buf, odb->path);
 	for_each_file_in_obj_subdir(subdir_nr, &buf,
@@ -2176,6 +2170,7 @@ void odb_load_loose_cache(struct object_directory *odb, int subdir_nr)
 				    &odb->loose_objects_cache[subdir_nr]);
 	odb->loose_objects_subdir_seen[subdir_nr] = 1;
 	strbuf_release(&buf);
+	return &odb->loose_objects_cache[subdir_nr];
 }
 
 void odb_clear_loose_cache(struct object_directory *odb)
-- 
2.20.1

Re: [PATCH 4/3] object-store: retire odb_load_loose_cache()

[Junio C Hamano]

René Scharfe <l.s.r@web.de> writes:

> Inline odb_load_loose_cache() into its only remaining caller,
> odb_loose_cache().  The latter offers a nicer interface for loading the
> cache, as it doesn't require callers to deal with fanout directory
> numbers directly.
>
> Signed-off-by: Rene Scharfe <l.s.r@web.de>
> ---

OK, that's much better ;-)  Thanks.

>  object-store.h | 7 -------
>  sha1-file.c    | 9 ++-------
>  2 files changed, 2 insertions(+), 14 deletions(-)
>
> diff --git a/object-store.h b/object-store.h
> index 2fb6c0e4db..e16aa38cae 100644
> --- a/object-store.h
> +++ b/object-store.h
> @@ -47,13 +47,6 @@ void add_to_alternates_file(const char *dir);
>   */
>  void add_to_alternates_memory(const char *dir);
>  
> -/*
> - * Populate an odb's loose object cache for one particular subdirectory (i.e.,
> - * the one that corresponds to the first byte of objects you're interested in,
> - * from 0 to 255 inclusive).
> - */
> -void odb_load_loose_cache(struct object_directory *odb, int subdir_nr);
> -
>  /*
>   * Populate and return the loose object cache array corresponding to the
>   * given object ID.
> diff --git a/sha1-file.c b/sha1-file.c
> index c3c6e50704..efcb2cbe74 100644
> --- a/sha1-file.c
> +++ b/sha1-file.c
> @@ -2154,12 +2154,6 @@ struct oid_array *odb_loose_cache(struct object_directory *odb,
>  				  const struct object_id *oid)
>  {
>  	int subdir_nr = oid->hash[0];
> -	odb_load_loose_cache(odb, subdir_nr);
> -	return &odb->loose_objects_cache[subdir_nr];
> -}
> -
> -void odb_load_loose_cache(struct object_directory *odb, int subdir_nr)
> -{
>  	struct strbuf buf = STRBUF_INIT;
>  
>  	if (subdir_nr < 0 ||
> @@ -2167,7 +2161,7 @@ void odb_load_loose_cache(struct object_directory *odb, int subdir_nr)
>  		BUG("subdir_nr out of range");
>  
>  	if (odb->loose_objects_subdir_seen[subdir_nr])
> -		return;
> +		return &odb->loose_objects_cache[subdir_nr];
>  
>  	strbuf_addstr(&buf, odb->path);
>  	for_each_file_in_obj_subdir(subdir_nr, &buf,
> @@ -2176,6 +2170,7 @@ void odb_load_loose_cache(struct object_directory *odb, int subdir_nr)
>  				    &odb->loose_objects_cache[subdir_nr]);
>  	odb->loose_objects_subdir_seen[subdir_nr] = 1;
>  	strbuf_release(&buf);
> +	return &odb->loose_objects_cache[subdir_nr];
>  }
>  
>  void odb_clear_loose_cache(struct object_directory *odb)

Re: jk/loose-object-cache

[Junio C Hamano]

René Scharfe <l.s.r@web.de> writes:

> Am 28.12.2018 um 19:04 schrieb Junio C Hamano:
>> * jk/loose-object-cache (2018-11-24) 10 commits
> ...
> So this has hit master in the meantime.  We discussed a sort performance
> fix in [1]; I'll reply with a short series containing a cleaned-up and
> rebased version as a follow-up.

Thanks.

-- >8 --
Subject: [PATCH 4/3] sha1-file.c: make odb_load_loose_cache() static

Now there is only a single internal caller to this helper function
that knows the implementation detail of the cache too much, hide it
from outside world by making it static.

Suggested-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 object-store.h |  7 -------
 sha1-file.c    | 24 +++++++++++++++---------
 2 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/object-store.h b/object-store.h
index 2fb6c0e4db..e16aa38cae 100644
--- a/object-store.h
+++ b/object-store.h
@@ -47,13 +47,6 @@ void add_to_alternates_file(const char *dir);
  */
 void add_to_alternates_memory(const char *dir);
 
-/*
- * Populate an odb's loose object cache for one particular subdirectory (i.e.,
- * the one that corresponds to the first byte of objects you're interested in,
- * from 0 to 255 inclusive).
- */
-void odb_load_loose_cache(struct object_directory *odb, int subdir_nr);
-
 /*
  * Populate and return the loose object cache array corresponding to the
  * given object ID.
diff --git a/sha1-file.c b/sha1-file.c
index 2adc56cde4..936d216ad4 100644
--- a/sha1-file.c
+++ b/sha1-file.c
@@ -2150,15 +2150,12 @@ static int append_loose_object(const struct object_id *oid, const char *path,
 	return 0;
 }
 
-struct oid_array *odb_loose_cache(struct object_directory *odb,
-				  const struct object_id *oid)
-{
-	int subdir_nr = oid->hash[0];
-	odb_load_loose_cache(odb, subdir_nr);
-	return &odb->loose_objects_cache[subdir_nr];
-}
-
-void odb_load_loose_cache(struct object_directory *odb, int subdir_nr)
+/*
+ * Populate an odb's loose object cache for one particular subdirectory (i.e.,
+ * the one that corresponds to the first byte of objects you're interested in,
+ * from 0 to 255 inclusive).
+ */
+static void odb_load_loose_cache(struct object_directory *odb, int subdir_nr)
 {
 	struct strbuf buf = STRBUF_INIT;
 
@@ -2178,6 +2175,15 @@ void odb_load_loose_cache(struct object_directory *odb, int subdir_nr)
 	strbuf_release(&buf);
 }
 
+struct oid_array *odb_loose_cache(struct object_directory *odb,
+				  const struct object_id *oid)
+{
+	int subdir_nr = oid->hash[0];
+	odb_load_loose_cache(odb, subdir_nr);
+	return &odb->loose_objects_cache[subdir_nr];
+}
+
+
 void odb_clear_loose_cache(struct object_directory *odb)
 {
 	int i;