What's cooking in git.git (Apr 2019, #05; Thu, 25)

What's cooking in git.git (Apr 2019, #05; Thu, 25)

[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.

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"]

* ab/commit-graph-fixes (2019-04-01) 8 commits
  (merged to 'next' on 2019-04-16 at 97f4ba02f0)
 + commit-graph: improve & i18n error messages
 + commit-graph write: don't die if the existing graph is corrupt
 + commit-graph verify: detect inability to read the graph
 + commit-graph: don't pass filename to load_commit_graph_one_fd_st()
 + commit-graph: don't early exit(1) on e.g. "git status"
 + commit-graph: fix segfault on e.g. "git status"
 + commit-graph tests: test a graph that's too small
 + commit-graph tests: split up corrupt_graph_and_verify()
 (this branch is used by ds/commit-graph-format-v2.)

 Code cleanup with more careful error checking before using data
 read from the commit-graph file.


* ab/gc-docs (2019-04-08) 11 commits
  (merged to 'next' on 2019-04-22 at 02785d40f5)
 + gc docs: remove incorrect reference to gc.auto=0
 + gc docs: clarify that "gc" doesn't throw away referenced objects
 + gc docs: note "gc --aggressive" in "fast-import"
 + gc docs: downplay the usefulness of --aggressive
 + gc docs: note how --aggressive impacts --window & --depth
 + gc docs: fix formatting for "gc.writeCommitGraph"
 + gc docs: re-flow the "gc.*" section in "config"
 + gc docs: include the "gc.*" section from "config" in "gc"
 + gc docs: clean grammar for "gc.bigPackThreshold"
 + gc docs: stop noting "repack" flags
 + gc docs: modernize the advice for manually running "gc"

 Update docs around "gc".


* ab/gc-reflog (2019-04-01) 7 commits
  (merged to 'next' on 2019-04-16 at aa27f951a8)
 + gc: handle & check gc.reflogExpire config
 + reflog tests: assert lack of early exit with expiry="never"
 + reflog tests: test for the "points nowhere" warning
 + reflog tests: make use of "test_config" idiom
 + gc: refactor a "call me once" pattern
 + gc: convert to using the_hash_algo
 + gc: remove redundant check for gc_auto_threshold

 Fix various glitches in "git gc" around reflog handling.


* ab/test-lib-pass-trace2-env (2019-04-01) 1 commit
  (merged to 'next' on 2019-04-16 at 4dad6d6d7a)
 + test-lib: whitelist GIT_TR2_* in the environment

 Allow tracing of Git executable while running the testsuite.


* ag/sequencer-reduce-rewriting-todo (2019-03-07) 18 commits
  (merged to 'next' on 2019-04-10 at 7eab7c7800)
 + rebase--interactive: move 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
 + rebase--interactive: move rearrange_squash_in_todo_file()
 + rebase--interactive: move sequencer_add_exec_commands()
 + 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: remove the 'arg' field from todo_item
 + sequencer: make the todo_list structure public
 + sequencer: changes in parse_insn_buffer()
 (this branch is used by pw/rebase-i-internal.)

 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.
 cf. <20190305191805.13561-1-alban.gruin@gmail.com> (v8)


* bc/hash-transition-16 (2019-04-01) 35 commits
  (merged to 'next' on 2019-04-16 at 8227fea6fa)
 + gitweb: make hash size independent
 + Git.pm: make hash size independent
 + read-cache: read data in a hash-independent way
 + dir: make untracked cache extension hash size independent
 + builtin/difftool: use parse_oid_hex
 + refspec: make hash size independent
 + archive: convert struct archiver_args to object_id
 + builtin/get-tar-commit-id: make hash size independent
 + get-tar-commit-id: parse comment record
 + hash: add a function to lookup hash algorithm by length
 + remote-curl: make hash size independent
 + http: replace sha1_to_hex
 + http: compute hash of downloaded objects using the_hash_algo
 + http: replace hard-coded constant with the_hash_algo
 + http-walker: replace sha1_to_hex
 + http-push: remove remaining uses of sha1_to_hex
 + http-backend: allow 64-character hex names
 + http-push: convert to use the_hash_algo
 + builtin/pull: make hash-size independent
 + builtin/am: make hash size independent
 + fast-import: replace sha1_to_hex
 + fast-import: make hash-size independent
 + builtin/name-rev: make hash-size independent
 + object-store: rename and expand packed_git's sha1 member
 + notes: replace sha1_to_hex
 + notes: make hash size independent
 + notes-merge: switch to use the_hash_algo
 + submodule: avoid hard-coded constants
 + pack-bitmap: switch hash tables to use struct object_id
 + pack-bitmap: switch hard-coded constants to the_hash_algo
 + pack-bitmap: replace sha1_to_hex
 + pack-bitmap: convert struct stored_bitmap to object_id
 + pack-bitmap: make bitmap header handling hash agnostic
 + khash: move oid hash table definition
 + t/lib-submodule-update: use appropriate length constant

 Conversion from unsigned char[20] to struct object_id continues.


* bc/send-email-qp-cr (2019-04-14) 1 commit
  (merged to 'next' on 2019-04-22 at 69398b0ea8)
 + send-email: default to quoted-printable when CR is present

 "git send-email" has been taught to use quoted-printable when the
 payload contains carriage-return.  The use of the mechanism is in
 line with the design originally added the codepath that chooses QP
 when the payload has overly long lines.


* bp/post-index-change-hook (2019-02-15) 1 commit
  (merged to 'next' on 2019-03-11 at cb96d1d7c4)
 + read-cache: add post-index-change hook

 Originally merged to 'next' on 2019-02-23

 A new hook "post-index-change" is called when the on-disk index
 file changes, which can help e.g. a virtualized working tree
 implementation.


* bs/sendemail-tighten-anything-by (2019-04-04) 1 commit
  (merged to 'next' on 2019-04-22 at 0a0680f234)
 + send-email: don't cc *-by lines with '-' prefix

 The recently added feature to add addresses that are on
 anything-by: trailers in 'git send-email' was found to be way too
 eager and considered nonsense strings as if they can be legitimate
 beginning of *-by: trailer.  This has been tightened.


* dk/blame-keep-origin-blob (2019-04-03) 1 commit
  (merged to 'next' on 2019-04-16 at 39679dde8b)
 + blame.c: don't drop origin blobs as eagerly

 Performance fix around "git blame", especially in a linear history
 (which is the norm we should optimize for).


* dl/submodule-set-branch (2019-04-10) 3 commits
  (merged to 'next' on 2019-04-22 at 3b640715ae)
 + submodule: teach set-branch subcommand
 + submodule--helper: teach config subcommand --unset
 + git-submodule.txt: "--branch <branch>" option defaults to 'master'

 "git submodule" learns "set-branch" subcommand that allows the
 submodule.*.branch settings to be modified.


* en/fast-import-parsing-fix (2019-04-01) 5 commits
  (merged to 'next' on 2019-04-16 at b318831bde)
 + fast-import: fix erroneous handling of get-mark with empty orphan commits
 + fast-import: only allow cat-blob requests where it makes sense
 + fast-import: check most prominent commands first
 + git-fast-import.txt: fix wording about where ls command can appear
 + t9300: demonstrate bug with get-mark and empty orphan commits

 "git fast-import" update.


* jc/gettext-test-fix (2019-04-15) 1 commit
  (merged to 'next' on 2019-04-22 at 7c57deeb04)
 + gettext tests: export the restored GIT_TEST_GETTEXT_POISON

 The GETTEXT_POISON test option has been quite broken ever since it
 was made runtime-tunable, which has been fixed.


* jk/fetch-reachability-error-fix (2019-04-15) 7 commits
  (merged to 'next' on 2019-04-22 at b4ce8375c0)
 + fetch: do not consider peeled tags as advertised tips
 + remote.c: make singular free_ref() public
 + fetch: use free_refs()
 + pkt-line: prepare buffer before handling ERR packets
 + upload-pack: send ERR packet for non-tip objects
 + t5530: check protocol response for "not our ref"
 + t5516: drop ok=sigpipe from unreachable-want tests

 Code clean-up and a fix for "git fetch" by an explicit object name
 (as opposed to fetching refs by name).


* jk/revision-rewritten-parents-in-prio-queue (2019-04-04) 1 commit
  (merged to 'next' on 2019-04-16 at bdc1465128)
 + revision: use a prio_queue to hold rewritten parents

 Performance fix for "rev-list --parents -- pathspec".


* jk/server-info-rabbit-hole (2019-04-16) 13 commits
  (merged to 'next' on 2019-04-16 at 3dded8be9a)
 + update_info_refs(): drop unused force parameter
 + server-info: drop objdirlen pointer arithmetic
 + server-info: drop nr_alloc struct member
 + server-info: use strbuf to read old info/packs file
 + server-info: simplify cleanup in parse_pack_def()
 + server-info: fix blind pointer arithmetic
 + http: simplify parsing of remote objects/info/packs
 + packfile: fix pack basename computation
 + midx: check both pack and index names for containment
 + t5319: drop useless --buffer from cat-file
 + t5319: fix bogus cat-file argument
 + pack-revindex: open index if necessary
 + packfile.h: drop extern from function declarations

 Code clean-up around a much-less-important-than-it-used-to-be
 update_server_info() funtion.


* jk/unused-params-even-more (2019-03-21) 13 commits
  (merged to 'next' on 2019-04-10 at 12edf8872f)
 + parse_opt_ref_sorting: always use with NONEG flag
 + pretty: drop unused strbuf from parse_padding_placeholder()
 + pretty: drop unused "type" parameter in needs_rfc2047_encoding()
 + parse-options: drop unused ctx parameter from show_gitcomp()
 + fetch_pack(): drop unused parameters
 + report_path_error(): drop unused prefix parameter
 + unpack-trees: drop unused error_type parameters
 + unpack-trees: drop name_entry from traverse_by_cache_tree()
 + test-date: drop unused "now" parameter from parse_dates()
 + update-index: drop unused prefix_length parameter from do_reupdate()
 + log: drop unused "len" from show_tagger()
 + log: drop unused rev_info from early output
 + revision: drop some unused "revs" parameters

 Code cleanup.


* jk/xmalloc (2019-04-12) 4 commits
  (merged to 'next' on 2019-04-22 at 1a907289fa)
 + progress: use xmalloc/xcalloc
 + xdiff: use xmalloc/xrealloc
 + xdiff: use git-compat-util
 + test-prio-queue: use xmalloc

 The code is updated to check the result of memory allocation before
 it is used in more places, by using xmalloc and/or xcalloc calls.


* js/difftool-no-index (2019-03-18) 3 commits
  (merged to 'next' on 2019-04-16 at 7313f9ff18)
 + difftool: allow running outside Git worktrees with --no-index
 + parse-options: make OPT_ARGUMENT() more useful
 + difftool: remove obsolete (and misleading) comment

 "git difftool" can now run outside a repository.


* js/iso8895-test-on-apfs (2019-04-15) 1 commit
  (merged to 'next' on 2019-04-22 at c2fadead33)
 + t9822: skip tests if file names cannot be ISO-8859-1 encoded

 Test fix on APFS that is incapable of store paths in Latin-1.


* js/macos-gettext-build (2019-04-15) 1 commit
  (merged to 'next' on 2019-04-22 at de4cbb1431)
 + macOS: make sure that gettext is found

 Build with gettext breaks on recent macOS w/ Homebrew when
 /usr/local/bin is not on PATH, which has been corrected.


* js/t3301-unbreak-notes-test (2019-04-09) 1 commit
  (merged to 'next' on 2019-04-22 at a015b00bd9)
 + t3301: fix false negative

 Test fix.


* js/untracked-cache-allocfix (2019-04-12) 1 commit
  (merged to 'next' on 2019-04-22 at 004a544075)
 + untracked cache: fix off-by-one
 (this branch is used by jk/untracked-cache-more-fixes.)

 An underallocation in the code to read the untracked cache
 extension has been corrected.


* jt/batch-fetch-blobs-in-diff (2019-04-08) 2 commits
  (merged to 'next' on 2019-04-22 at 0598bae567)
 + diff: batch fetching of missing blobs
 + sha1-file: support OBJECT_INFO_FOR_PREFETCH
 (this branch is used by cc/multi-promisor.)

 While running "git diff" in a lazy clone, we can upfront know which
 missing blobs we will need, instead of waiting for the on-demand
 machinery to discover them one by one.  Aim to achieve better
 performance by batching the request for these promised blobs.


* jt/fetch-no-update-shallow-in-proto-v2 (2019-04-01) 3 commits
  (merged to 'next' on 2019-04-16 at 05c5ebe471)
 + fetch-pack: respect --no-update-shallow in v2
 + fetch-pack: call prepare_shallow_info only if v0
 + Merge branch 'jt/test-protocol-version' into jt/fetch-no-update-shallow-in-proto-v2

 Fix for protocol v2 support in "git fetch-pack" of shallow clones.


* jt/fetch-pack-wanted-refs-optim (2019-04-01) 1 commit
  (merged to 'next' on 2019-04-16 at 051f6bd38a)
 + fetch-pack: binary search when storing wanted-refs

 Performance fix around "git fetch" that grabs many refs.


* km/t3000-retitle (2019-04-12) 1 commit
  (merged to 'next' on 2019-04-22 at 2d5aa01ca6)
 + t3000 (ls-files -o): widen description to reflect current tests

 A test update.


* nd/checkout-m (2019-03-24) 4 commits
  (merged to 'next' on 2019-04-16 at 4d7c322bed)
 + checkout: prevent losing staged changes with --merge
 + read-tree: add --quiet
 + unpack-trees: rename "gently" flag to "quiet"
 + unpack-trees: keep gently check inside add_rejected_path

 "git checkout -m <other>" was about carrying the differences
 between HEAD and the working-tree files forward while checking out
 another branch, and ignored the differences between HEAD and the
 index.  The command has been taught to abort when the index and the
 HEAD are different.


* nd/commit-a-with-paths-msg-update (2019-03-22) 1 commit
  (merged to 'next' on 2019-04-16 at a36c712b39)
 + commit: improve error message in "-a <paths>" case

 The message given when "git commit -a <paths>" errors out has been
 updated.


* nd/diff-parseopt-4 (2019-03-24) 20 commits
  (merged to 'next' on 2019-04-10 at 893b135f10)
 + am: avoid diff_opt_parse()
 + diff --no-index: use parse_options() instead of diff_opt_parse()
 + range-diff: use parse_options() instead of diff_opt_parse()
 + diff.c: allow --no-color-moved-ws
 + diff-parseopt: convert --color-moved-ws
 + diff-parseopt: convert --[no-]color-moved
 + diff-parseopt: convert --inter-hunk-context
 + diff-parseopt: convert --no-prefix
 + diff-parseopt: convert --line-prefix
 + diff-parseopt: convert --[src|dst]-prefix
 + diff-parseopt: convert --[no-]abbrev
 + diff-parseopt: convert --diff-filter
 + diff-parseopt: convert --find-object
 + diff-parseopt: convert -O
 + diff-parseopt: convert --pickaxe-all|--pickaxe-regex
 + diff-parseopt: convert -S|-G
 + diff-parseopt: convert -l
 + diff-parseopt: convert -z
 + diff-parseopt: convert --ita-[in]visible-in-index
 + diff-parseopt: convert --ws-error-highlight

 Fourth batch to teach the diff machinery to use the parse-options
 API.


* nd/submodule-foreach-quiet (2019-04-15) 1 commit
  (merged to 'next' on 2019-04-22 at bf982bca7b)
 + submodule foreach: fix "<command> --quiet" not being respected

 "git submodule foreach <command> --quiet" did not pass the option
 down correctly, which has been corrected.


* po/describe-not-necessarily-7 (2019-04-08) 1 commit
  (merged to 'next' on 2019-04-22 at 65b47ca73f)
 + describe doc: remove '7-char' abbreviation reference

 Docfix.


* po/rerere-doc-fmt (2019-04-08) 1 commit
  (merged to 'next' on 2019-04-22 at 780c0d2450)
 + rerere doc: quote `rerere.enabled`

 Docfix.


* pw/cherry-pick-continue (2019-03-18) 3 commits
  (merged to 'next' on 2019-04-16 at 1bfd7a7179)
 + cherry-pick --continue: remember options
 + cherry-pick: demonstrate option amnesia
 + sequencer: break some long lines

 "git cherry-pick --options A..B", after giving control back to the
 user to ask help resolving a conflicted step, did not honor the
 options it originally received, which has been corrected.


* sg/blame-in-bare-start-at-head (2019-04-08) 1 commit
  (merged to 'next' on 2019-04-22 at 159777c280)
 + blame: default to HEAD in a bare repo when no start commit is given

 "git blame -- path" in a non-bare repository starts blaming from
 the working tree, and the same command in a bare repository errors
 out because there is no working tree by definition.  The command
 has been taught to instead start blaming from the commit at HEAD,
 which is more useful.


* sg/index-pack-progress (2019-04-01) 1 commit
  (merged to 'next' on 2019-04-16 at a10bfdd950)
 + index-pack: show progress while checking objects

 A progress indicator has been added to the "index-pack" step, which
 often makes users wait for completion during "git clone".


* sg/overlong-progress-fix (2019-04-15) 4 commits
  (merged to 'next' on 2019-04-22 at 69921cdf09)
 + progress: break too long progress bar lines
 + progress: clear previous progress update dynamically
 + progress: assemble percentage and counters in a strbuf before printing
 + progress: make display_progress() return void

 Updating the display with progress message has been cleaned up to
 deal better with overlong messages.


* sg/test-atexit (2019-03-14) 11 commits
  (merged to 'next' on 2019-04-10 at 7839135291)
 + t9811-git-p4-label-import: fix pipeline negation
 + git p4 test: disable '-x' tracing in the p4d watchdog loop
 + git p4 test: simplify timeout handling
 + git p4 test: clean up the p4d cleanup functions
 + git p4 test: use 'test_atexit' to kill p4d and the watchdog process
 + t0301-credential-cache: use 'test_atexit' to stop the credentials helper
 + tests: use 'test_atexit' to stop httpd
 + git-daemon: use 'test_atexit` to stop 'git-daemon'
 + test-lib: introduce 'test_atexit'
 + t/lib-git-daemon: make sure to kill the 'git-daemon' process
 + test-lib: fix interrupt handling with 'dash' and '--verbose-log -x'

 Test framework update to more robustly clean up leftover files and
 processes after tests are done.


* tg/ls-files-debug-format-fix (2019-04-08) 1 commit
  (merged to 'next' on 2019-04-22 at a5ac1ca49f)
 + ls-files: use correct format string

 Debugging code fix.


* tz/doc-apostrophe-no-longer-needed (2019-04-10) 1 commit
  (merged to 'next' on 2019-04-22 at 8ff03863ce)
 + Documentation/git-show-branch: avoid literal {apostrophe}

 Doc formatting fix.

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

* jc/make-dedup-ls-files-output (2019-04-22) 1 commit
 - Makefile: dedup list of files obtained from ls-files

 A "ls-files" that emulates "find" to enumerate files in the working
 tree resulted in duplicated Makefile rules that caused the build to
 issue an unnecessary warning during a trial build after merge
 conflicts are resolved in working tree *.h files but before the
 resolved results are added to the index.  This has been corrected.

 Will merge to 'next'.
 A not-so-low hanging fruit to teach ls-files to dedup either optionally
 or always has also been discussed, which probably is a good idea,
 to prevent mistakes similar to the bug this topic fixes in the future.


* jk/ls-files-doc-markup-fix (2019-04-23) 1 commit
 - doc/ls-files: put nested list for "-t" option into block

 Docfix.

 Will merge to 'next'.


* jk/p5302-avoid-collision-check-cost (2019-04-23) 1 commit
 - p5302: create the repo in each index-pack test

 Fix index-pack perf test so that the repeated invocations always
 run in an empty repository, which emulates the initial clone
 situation better.

 Will merge to 'next'.


* dl/rev-tilde-doc-clarify (2019-04-24) 1 commit
 - revisions.txt: mention <rev>~ form

 Docfix.

 Will merge to 'next'.


* en/unicode-in-refnames (2019-04-24) 1 commit
 - Honor core.precomposeUnicode in more places

 The names of the refs stored as filesystem entities may become
 different from what the end-user expects, just like files in the
 working tree gets "renamed", on a filesystem like HFS+.  Work it
 around by paying attemption to the core.precomposeUnicode
 configuration.

 Looked sensible.  Ready for next?


* jk/perf-aggregate-wo-libjson (2019-04-24) 1 commit
 - t/perf: depend on perl JSON only when using --codespeed

 The script to aggregate perf result unconditionally depended on
 libjson-perl even though it did not have to, which has been
 corrected.

 Will merge to 'next'.


* cc/access-on-aix-workaround (2019-04-25) 1 commit
 - git-compat-util: work around for access(X_OK) under root
 (this branch uses cc/aix-has-fileno-as-a-macro.)


* dl/difftool-mergetool (2019-04-25) 6 commits
 - difftool: fallback on merge.guitool
 - difftool: make --gui, --tool and --extcmd mutually exclusive
 - mergetool: fallback to tool when guitool unavailable
 - mergetool: use get_merge_tool function
 - t7610: add mergetool --gui tests
 - t7610: unsuppress output

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

* nb/branch-show-other-worktrees-head (2019-03-18) 3 commits
 - branch: add worktree info on verbose output
 - branch: update output to include worktree info
 - ref-filter: add worktreepath 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.

 Getting there...
 cf. <20190316013807.38756-1-nbelakovski@gmail.com> (v9)
 cf. <20190318121054.GC24175@szeder.dev>


* jt/fetch-cdn-offload (2019-03-12) 9 commits
 - SQUASH???
 - upload-pack: send part of packfile response as uri
 - fetch-pack: support more than one pack lockfile
 - upload-pack: refactor reading of pack-objects out
 - Documentation: add Packfile URIs design doc
 - Documentation: order protocol v2 sections
 - http-fetch: support fetching packfiles by URL
 - http: improve documentation of http_pack_request
 - http: use --stdin when getting dumb HTTP pack

 WIP for allowing a response to "git fetch" to instruct the bulk of
 the pack contents to be instead taken from elsewhere (aka CDN).

 Still being discussed.


* 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>


* js/protocol-advertise-multi (2018-12-28) 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. <CANq=j3u-zdb_FvNJGPCmygNMScseav63GhVvBX3NcVS4f7TejA@mail.gmail.com>


* 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".


* 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.

 Anybody who wants to champion this topic?
 I am personally not yet quite convinced if this is worth pursuing.

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

* ds/midx-expire-repack (2019-04-25) 11 commits
 - t5319-multi-pack-index.sh: test batch size zero
 - midx: add test that 'expire' respects .keep files
 - multi-pack-index: test expire while adding packs
 - midx: implement midx_repack()
 - multi-pack-index: prepare 'repack' subcommand
 - multi-pack-index: implement 'expire' subcommand
 - midx: refactor permutation logic and pack sorting
 - midx: simplify computation of pack name lengths
 - multi-pack-index: prepare for 'expire' subcommand
 - Docs: rearrange subcommands for multi-pack-index
 - repack: refactor pack deletion for future use

 "git multi-pack-index expire/repack" are new subcommands that
 consult midx file and are used to drop unused pack files and
 coalesce small pack files that are still in use.

 cf. <20190424151428.170316-1-dstolee@microsoft.com> (v5)


* es/first-contrib-tutorial (2019-04-25) 1 commit
 - documentation: add tutorial for first contribution

 A new tutorial targetting specifically aspiring git-core
 developers.

 cf. <20190423193410.101803-1-emilyshaffer@google.com> (v4)


* pw/clean-sequencer-state-upon-final-commit (2019-04-17) 2 commits
 - fix cherry-pick/revert status after commit
 - commit/reset: try to clean up sequencer state

 "git chery-pick" (and "revert" that shares the same runtime engine)
 that deals with multiple commits got confused when the final step
 gets stopped with a conflict and the user concluded the sequence
 with "git commit".  Attempt to fix it by cleaning up the state
 files used by these commands in such a situation.

 Will merge to 'next'.


* pw/sequencer-cleanup-with-signoff-x-fix (2019-04-18) 1 commit
  (merged to 'next' on 2019-04-25 at cc587fb2b9)
 + sequencer: fix cleanup with --signoff and -x
 (this branch is used by dl/merge-cleanup-scissors-fix.)

 "git cherry-pick" run with the "-x" or the "--signoff" option used
 to (and more importantly, ought to) clean up the commit log message
 with the --cleanup=space option by default, but this has been
 broken since late 2017.  This has been fixed.

 Will merge to 'master'.


* dr/ref-filter-push-track-fix (2019-04-18) 1 commit
  (merged to 'next' on 2019-04-25 at 07db067adc)
 + ref-filter: use correct branch for %(push:track)

 %(push:track) token used in the --format option to "git
 for-each-ref" and friends was not showing the right branch, which
 has been fixed.

 Will merge to 'master'.


* cc/aix-has-fileno-as-a-macro (2019-04-25) 1 commit
  (merged to 'next' on 2019-04-25 at f1d6464f98)
 + Makefile: use fileno macro work around on AIX
 (this branch is used by cc/access-on-aix-workaround.)

 AIX shared the same build issues with other BSDs around fileno(fp),
 which has been corrected.

 Will merge to 'master'.


* jk/prune-optim (2019-04-19) 1 commit
  (merged to 'next' on 2019-04-25 at c50353b27f)
 + t5304: add a test for pruning with bitmaps

 A follow-up test for an earlier "git prune" improvements.

 Will merge to 'master'.


* jk/untracked-cache-more-fixes (2019-04-19) 3 commits
  (merged to 'next' on 2019-04-25 at a6037ddd54)
 + untracked-cache: simplify parsing by dropping "len"
 + untracked-cache: simplify parsing by dropping "next"
 + untracked-cache: be defensive about missing NULs in index

 Code clean-up.

 Will merge to 'master'.


* jt/submodule-repo-is-with-worktree (2019-04-21) 1 commit
  (merged to 'next' on 2019-04-25 at da2c6d684d)
 + worktree: update is_bare heuristics

 The logic to tell if a Git repository has a working tree protects
 "git branch -D" from removing the branch that is currently checked
 out by mistake.  The implementation of this logic was broken for
 repositories with unusual name, which unfortunately is the norm for
 submodules these days.  This has been fixed.

 Will merge to 'master'.


* vk/autoconf-gettext (2019-04-19) 1 commit
  (merged to 'next' on 2019-04-25 at 918870cbc2)
 + autoconf: #include <libintl.h> when checking for gettext()

 The autoconf generated configure script failed to use the right
 gettext() implementations from -libintl by ignoring useless stub
 implementations shipped in some C library, which has been
 corrected.

 Will merge to 'master'.


* dl/no-extern-in-func-decl (2019-04-24) 4 commits
 - cocci: prevent extern function declarations
 - *.[ch]: manually align parameter lists
 - *.[ch]: remove extern from function declarations using sed
 - *.[ch]: remove extern from function declarations using spatch

 Mechanically and systematically drop "extern" from function
 declarlation.

 Will merge to 'next'.

 This is the kind of code churn patch that causes heavy conflicts
 with multiple topics in flight, which causes conflicts in slightly
 different shape when it gets merged with them in different order.
 I've queued it moderately early in 'pu' for the night, and I am
 reasonably sure the merge of the topic itself is OK, but I do not
 have much confidence in the resolutions of conflicts with other
 topics later merged to 'pu'.


* js/partial-clone-connectivity-check (2019-04-21) 1 commit
  (merged to 'next' on 2019-04-25 at ebd8b4bffd)
 + clone: do faster object check for partial clones

 During an initial "git clone --depth=..." partial clone, it is
 pointless to spend cycles for a large portion of the connectivity
 check that enumerates and skips promisor objects (which by
 definition is all objects fetched from the other side).  This has
 been optimized out.

 Will merge to 'master'.


* hs/send-email-transferencoding-fix (2019-04-10) 1 commit
 - send-email: honor transferencoding config option again

 Since "git send-email" learned to take 'auto' as the value for the
 transfer-encoding, it by mistake stopped honoring the values given
 to the configuration variables sendemail.transferencoding and/or
 sendemail.<ident>.transferencoding.  Attempt to correct this.

 Not quite.
 cf. <xmqq8swi34h5.fsf@gitster-ct.c.googlers.com>


* jk/pack-objects-reports-num-objects-to-trace2 (2019-04-12) 1 commit
  (merged to 'next' on 2019-04-25 at e79464c054)
 + pack-objects: write objects packed to trace2

 The "git pack-objects" command learned to report the number of
 objects it packed via the trace2 mechanism.

 Will merge to 'master'.


* js/misc-doc-fixes (2019-04-19) 8 commits
  (merged to 'next' on 2019-04-25 at 6898f709d0)
 + Turn `git serve` into a test helper
 + test-tool: handle the `-C <directory>` option just like `git`
 + check-docs: do not bother checking for legacy scripts' documentation
 + docs: exclude documentation for commands that have been excluded
 + check-docs: allow command-list.txt to contain excluded commands
 + help -a: do not list commands that are excluded from the build
 + Makefile: drop the NO_INSTALL variable
 + remote-testgit: move it into the support directory for t5801

 "make check-docs", "git help -a", etc. did not account for cases
 where a particular build may deliberately omit some subcommands,
 which has been corrected.

 Will merge to 'master'.


* dl/warn-tagging-a-tag (2019-04-12) 2 commits
  (merged to 'next' on 2019-04-25 at 8b966d7fe8)
 + tag: advise on nested tags
 + tag: fix formatting

 "git tag" learned to give an advice suggesting it might be a
 mistake when creating an annotated or signed tag that points at
 another tag.

 Will merge to 'master'.


* jc/format-patch-delay-message-id (2019-04-05) 1 commit
 - format-patch: move message-id and related headers to the end

 The location "git format-patch --thread" adds the Message-Id:
 header in the series of header fields has been moved down, which
 may help working around a suspected bug in GMail MSA, reported at
 <CAHk-=whP1stFZNAaJiMi5eZ9rj0MRt20Y_yHVczZPH+O01d+sA@mail.gmail.com>

 Waiting for feedback to see if it truly helps.
 Needs tests.


* tb/unexpected (2019-04-10) 7 commits
  (merged to 'next' on 2019-04-25 at c49927fca0)
 + rev-list: detect broken root trees
 + rev-list: let traversal die when --missing is not in use
 + get_commit_tree(): return NULL for broken tree
 + list-objects.c: handle unexpected non-tree entries
 + list-objects.c: handle unexpected non-blob entries
 + t: introduce tests for unexpected object types
 + t: move 'hex2oct' into test-lib-functions.sh

 Code tightening against a "wrong" object appearing where an object
 of a different type is expected, instead of blindly assuming that
 the connection between objects are correctly made.

 Will merge to 'master'.


* en/merge-directory-renames (2019-04-08) 15 commits
  (merged to 'next' on 2019-04-25 at fd5b4f57b4)
 + merge-recursive: switch directory rename detection default
 + merge-recursive: give callers of handle_content_merge() access to contents
 + merge-recursive: track information associated with directory renames
 + t6043: fix copied test description to match its purpose
 + merge-recursive: switch from (oid,mode) pairs to a diff_filespec
 + merge-recursive: cleanup handle_rename_* function signatures
 + merge-recursive: track branch where rename occurred in rename struct
 + merge-recursive: remove ren[12]_other fields from rename_conflict_info
 + merge-recursive: shrink rename_conflict_info
 + merge-recursive: move some struct declarations together
 + merge-recursive: use 'ci' for rename_conflict_info variable name
 + merge-recursive: rename locals 'o' and 'a' to 'obuf' and 'abuf'
 + merge-recursive: rename diff_filespec 'one' to 'o'
 + merge-recursive: rename merge_options argument from 'o' to 'opt'
 + Use 'unsigned short' for mode, like diff_filespec does

 "git merge-recursive" backend recently learned a new heuristics to
 infer file movement based on how other files in the same directory
 moved.  As this is inherently less robust heuristics than the one
 based on the content similarity of the file itself (rather than
 based on what its neighbours are doing), it sometimes gives an
 outcome unexpected by the end users.  This has been toned down to
 leave the renamed paths in higher/conflicted stages in the index so
 that the user can examine and confirm the result.

 Will merge to 'master'.


* jt/clone-server-option (2019-04-18) 2 commits
  (merged to 'next' on 2019-04-25 at 21f07cc85d)
 + clone: send server options when using protocol v2
 + transport: die if server options are unsupported

 "git clone" learned a new --server-option option when talking over
 the protocol version 2.

 Will merge to 'master'.


* tz/git-svn-doc-markup-fix (2019-04-10) 1 commit
  (merged to 'next' on 2019-04-25 at 3efaa6285c)
 + Documentation/git-svn: improve asciidoctor compatibility

 Doc formatting fix.

 Will merge to 'master'.


* ss/msvc-path-utils-fix (2019-04-09) 1 commit
  (merged to 'next' on 2019-04-25 at ee2850da18)
 + MSVC: include compat/win32/path-utils.h for MSVC, too, for real_path()

 An earlier update for MinGW and Cygwin accidentally broke MSVC build,
 which has been fixed.

 Will merge to 'master'.


* cc/multi-promisor (2019-04-15) 17 commits
 - Move core_partial_clone_filter_default to promisor-remote.c
 - Move repository_format_partial_clone to promisor-remote.c
 - Remove fetch-object.{c,h} in favor of promisor-remote.{c,h}
 - remote: add promisor and partial clone config to the doc
 - partial-clone: add multiple remotes in the doc
 - t0410: test fetching from many promisor remotes
 - builtin/fetch: remove unique promisor remote limitation
 - promisor-remote: parse remote.*.partialclonefilter
 - diff: use promisor-remote.h instead of fetch-object.h
 - Use promisor_remote_get_direct() and has_promisor_remote()
 - promisor-remote: use repository_format_partial_clone
 - promisor-remote: add promisor_remote_reinit()
 - promisor-remote: implement promisor_remote_get_direct()
 - Add initial support for many promisor remotes
 - fetch-object: make functions return an error code
 - t0410: remove pipes after git commands
 - Merge branch 'jt/batch-fetch-blobs-in-diff' into cc/multi-promisor

 Teach the lazy clone machinery that there can be more than one
 promisor remote and consult them in order when downloading missing
 objects on demand.

 Needs review.


* nd/switch-and-restore (2019-04-12) 43 commits
 - help: move git-diff and git-reset to different groups
 - rm: add --staged as alias for --cached
 - doc: promote "git restore"
 - user-manual.txt: prefer 'merge --abort' over 'reset --hard'
 - completion: support restore
 - t: add tests for restore
 - restore: support --patch
 - restore: replace --force with --ignore-unmerged
 - restore: default to --source=HEAD when only --staged is specified
 - restore: reject invalid combinations with --staged
 - restore: add --worktree and --staged
 - checkout: factor out worktree checkout code
 - restore: disable overlay mode by default
 - restore: make pathspec mandatory
 - restore: take tree-ish from --source option instead
 - checkout: split part of it to new command 'restore'
 - doc: promote "git switch"
 - completion: support switch
 - t: add tests for switch
 - switch: make --orphan switch to an empty tree
 - switch: reject if some operation is in progress
 - switch: no worktree status unless real branch switch happens
 - switch: implicit dwim, use --no-guess to disable it
 - switch: add short option for --detach
 - switch: only allow explicit detached HEAD
 - switch: reject "do nothing" case
 - switch: stop accepting pathspec
 - switch: remove -l
 - switch: add --discard-changes
 - switch: better names for -b and -B
 - checkout: split part of it to new command 'switch'
 - checkout: split options[] array in three pieces
 - checkout: move 'confict_style' and 'dwim_..' to checkout_opts
 - checkout: make "opts" in cmd_checkout() a pointer
 - checkout: factor out some code in parse_branchname_arg()
 - checkout: keep most #include sorted
 - checkout: inform the user when removing branch state
 - checkout: advice how to get out of detached HEAD mode
 - t: rename t2014-switch.sh to t2014-checkout-switch.sh
 - git-checkout.txt: fix monospace typeset
 - doc: document --overwrite-ignore
 - git-checkout.txt: fix one syntax line
 - git-checkout.txt: spell out --no-option

 Two new commands "git switch" and "git restore" are introduced to
 split "checking out a branch to work on advancing its history" and
 "checking out paths out of the index and/or a tree-ish to work on
 advancing the current history" out of the single "git checkout"
 command.

 The "switch" part seems more or less ready for testing.  Perhaps
 we should split this back into two topics and merge it to 'next'.
 cf. <20190329103919.15642-1-pclouds@gmail.com> (switch v6)
 cf. <20190411131218.19195-1-pclouds@gmail.com> (restore v2)


* ew/repack-with-bitmaps-by-default (2019-03-18) 3 commits
 - pack-objects: default to writing bitmap hash-cache
 - t5310: correctly remove bitmaps for jgit test
 - repack: enable bitmaps by default on bare repos

 The connectivity bitmaps are created by default in bare
 repositories now; also the pathname hash-cache is created by
 default to avoid making crappy deltas when repacking.

 Will merge to 'next'.


* jc/format-patch-noclobber (2019-02-22) 1 commit
 - format-patch: --no-clobber refrains from overwriting output files

 "git format-patch" used to overwrite an existing patch/cover-letter
 file.  A new "--no-clobber" option stops it.

 Undecided but inclined to discard.


* am/p4-branches-excludes (2019-04-02) 8 commits
 - git-p4: respect excluded paths when detecting branches
 - git-p4: add failing test for "git-p4: respect excluded paths when detecting branches"
 - git-p4: don't exclude other files with same prefix
 - git-p4: add failing test for "don't exclude other files with same prefix"
 - git-p4: don't groom exclude path list on every commit
 - git-p4: match branches case insensitively if configured
 - git-p4: add failing test for "git-p4: match branches case insensitively if configured"
 - git-p4: detect/prevent infinite loop in gitCommitByP4Change()

 "git p4" update.

 Is this ready for 'next'?


* cc/replace-graft-peel-tags (2019-04-15) 4 commits
  (merged to 'next' on 2019-04-25 at f8d0db27ba)
 + replace: peel tag when passing a tag first to --graft
 + replace: peel tag when passing a tag as parent to --graft
 + t6050: redirect expected error output to a file
 + t6050: use test_line_count instead of wc -l

 When given a tag that points at a commit-ish, "git replace --graft"
 failed to peel the tag before writing a replace ref, which did not
 make sense because the old graft mechanism the feature wants to
 mimick only allowed to replace one commit object with another.
 This has been fixed.

 Will merge to 'master'.


* dl/rebase-i-keep-base (2019-04-25) 6 commits
 - rebase: teach rebase --keep-base
 - rebase: fast-forward --fork-point in more cases
 - rebase: fast-forward --onto in more cases
 - rebase: refactor can_fast_forward into goto tower
 - t3432: test rebase fast-forward behavior
 - t3431: add rebase --fork-point tests

 "git rebase --keep-base <upstream>" tries to find the original base
 of the topic being rebased and rebase on top of that same base,
 which is useful when running the "git rebase -i" (and its limited
 variant "git rebase -x").

 The command also has learned to fast-forward in more cases where it
 can instead of replaying to recreate identical commits.

 Will merge to 'next'.


* jh/trace2-sid-fix (2019-04-16) 10 commits
  (merged to 'next' on 2019-04-25 at a5c08f1226)
 + trace2: update docs to describe system/global config settings
 + trace2: make SIDs more unique
 + trace2: clarify UTC datetime formatting
 + trace2: report peak memory usage of the process
 + trace2: use system/global config for default trace2 settings
 + config: add read_very_early_config()
 + trace2: find exec-dir before trace2 initialization
 + trace2: add absolute elapsed time to start event
 + trace2: refactor setting process starting time
 + config: initialize opts structure in repo_read_config()

 Polishing of the new trace2 facility continues.  The system-level
 configuration can specify site-wide trace2 settings, which can be
 overridden with per-user configuration and environment variables.

 Will merge to 'master'.
 cf. <pull.169.v4.git.gitgitgadget@gmail.com> (v4)


* js/trace2-to-directory (2019-03-22) 1 commit
  (merged to 'next' on 2019-04-25 at 53adf71c41)
 + trace2: write to directory targets

 The trace2 tracing facility learned to auto-generate a filename
 when told to log to a directory.

 Will merge to 'master'.


* km/empty-repo-is-still-a-repo (2019-04-10) 3 commits
  (merged to 'next' on 2019-04-25 at bb3d4406a5)
 + add: error appropriately on repository with no commits
 + dir: do not traverse repositories with no commits
 + submodule: refuse to add repository with no commits

 Running "git add" on a repository created inside the current
 repository is an explicit indication that the user wants to add it
 as a submodule, but when the HEAD of the inner repository is on an
 unborn branch, it cannot be added as a submodule.  Worse, the files
 in its working tree can be added as if they are a part of the outer
 repository, which is not what the user wants.  These problems are
 being addressed.

 Will merge to 'master'.


* nd/precious (2019-04-09) 1 commit
 - Introduce "precious" file concept

 "git clean" learned to pay attention to the 'precious' attributes
 and keep untracked paths with the attribute instead of removing
 when the "--keep-precious" is given.

 Retracted.
 cf. <CACsJy8AEZ-Lz6zgEsuNukvphB9TTa9FAC1gK05fhnie2xtfc9w@mail.gmail.com>

 I am not sure what aspect of this longer-term "precious" vision,
 which gets taught to various commands and use cases individually
 and incrementally, Ævar finds problematic, which I understand is
 the reason of redtraction.


* nd/sha1-name-c-wo-the-repository (2019-04-16) 34 commits
  (merged to 'next' on 2019-04-25 at d826918329)
 + sha1-name.c: remove the_repo from get_oid_mb()
 + sha1-name.c: remove the_repo from other get_oid_*
 + sha1-name.c: remove the_repo from maybe_die_on_misspelt_object_name
 + submodule-config.c: use repo_get_oid for reading .gitmodules
 + sha1-name.c: add repo_get_oid()
 + sha1-name.c: remove the_repo from get_oid_with_context_1()
 + sha1-name.c: remove the_repo from resolve_relative_path()
 + sha1-name.c: remove the_repo from diagnose_invalid_index_path()
 + sha1-name.c: remove the_repo from handle_one_ref()
 + sha1-name.c: remove the_repo from get_oid_1()
 + sha1-name.c: remove the_repo from get_oid_basic()
 + sha1-name.c: remove the_repo from get_describe_name()
 + sha1-name.c: remove the_repo from get_oid_oneline()
 + sha1-name.c: add repo_interpret_branch_name()
 + sha1-name.c: remove the_repo from interpret_branch_mark()
 + sha1-name.c: remove the_repo from interpret_nth_prior_checkout()
 + sha1-name.c: remove the_repo from get_short_oid()
 + sha1-name.c: add repo_for_each_abbrev()
 + sha1-name.c: store and use repo in struct disambiguate_state
 + sha1-name.c: add repo_find_unique_abbrev_r()
 + sha1-name.c: remove the_repo from find_abbrev_len_packed()
 + sha1-name.c: remove the_repo from sort_ambiguous()
 + commit.c: add repo_get_commit_tree()
 + commit.cocci: refactor code, avoid double rewrite
 + refs.c: remove the_repo from read_ref_at()
 + refs.c: add repo_dwim_log()
 + refs.c: add repo_dwim_ref()
 + refs.c: remove the_repo from expand_ref()
 + refs.c: remove the_repo from substitute_branch_name()
 + refs.c: add refs_shorten_unambiguous_ref()
 + refs.c: add refs_ref_exists()
 + packfile.c: add repo_approximate_object_count()
 + builtin rebase: use oideq()
 + builtin rebase: use FREE_AND_NULL

 Further code clean-up to allow the lowest level of name-to-object
 mapping layer to work with a passed-in repository other than the
 default one.

 Will merge to 'master'.


* pw/rebase-i-internal (2019-04-19) 13 commits
 - rebase -i: run without forking rebase--interactive
 - rebase: use a common action enum
 - rebase -i: use struct rebase_options in do_interactive_rebase()
 - rebase -i: use struct rebase_options to parse args
 - rebase -i: use struct object_id for squash_onto
 - rebase -i: use struct commit when parsing options
 - rebase -i: remove duplication
 - rebase -i: combine rebase--interactive.c with rebase.c
 - rebase: use OPT_RERERE_AUTOUPDATE()
 - rebase: rename write_basic_state()
 - rebase: don't translate trace strings
 - sequencer: always discard index after checkout
 - Merge branch 'ag/sequencer-reduce-rewriting-todo' into pw/rebase-i-internal

 The internal implementation of "git rebase -i" has been updated to
 avoid forking a separate "rebase--interactive" process.

 Will merge to 'next'.


* nd/worktree-name-sanitization (2019-03-20) 2 commits
 - SQUASH???
 - worktree add: sanitize worktree names

 In recent versions of Git, per-worktree refs are exposed in
 refs/worktrees/<wtname>/ hierarchy, which means that worktree names
 must be a valid refname component.  The code now sanitizes the names
 given to worktrees, to make sure these refs are well-formed.

 I am inclined to squash the fix at the tip in and merge the result
 to 'next'.  Opinions?


* dm/some-stdio-functions-are-macro-on-freebsd (2019-02-01) 1 commit
 - http: cast result to FILE *

 Variants of BSD define fileno(fh) as a macro, breaking a program
 that passes a "void *" to it.

 Expecting a reroll.
 cf. <49B9198C-53E5-42BD-8834-B1EDEB3332CB@usask.ca>


* ds/commit-graph-format-v2 (2019-04-25) 5 commits
 - commit-graph: implement file format version 2
 - commit-graph: add --version=<n> option
 - commit-graph: create new version flags
 - commit-graph: collapse parameters into flags
 - commit-graph: return with errors during write

 Introduce version 2 of the commit-graph format to correct
 deficiency in the initial version.

 cf. <pull.112.v2.git.gitgitgadget@gmail.com> (v2)


* br/blame-ignore (2019-04-14) 6 commits
 - blame: use a fingerprint heuristic to match ignored lines
 - blame: optionally track line fingerprints during fill_blame_origin()
 - blame: add config options to handle output for ignored lines
 - blame: add the ability to ignore commits and their changes
 - blame: use a helper function in blame_chunk()
 - Move init_skiplist() outside of fsck

 "git blame" learned to "ignore" commits in the history, whose
 effects (as well as their presence) get ignored.

 Expecting a reroll.
 cf. <20190410162409.117264-1-brho@google.com> (v6)
 cf. <a742dd62-c84e-1f85-0663-4a3aa4d14989@google.com>
 cf. <3db6bad3-e7a5-af1d-3fe2-321bd17db2c6@google.com>


* nd/config-move-to (2019-01-14) 7 commits
 - config.h: fix hdr-check warnings
 - config: add --move-to
 - config: factor out set_config_source_file()
 - config: use OPT_FILENAME()
 - config.c: add repo_config_set_worktree_gently()
 - worktree.c: add get_worktree_config()
 - config.c: avoid git_path() in do_git_config_sequence()

 Needs review.


* dl/merge-cleanup-scissors-fix (2019-04-19) 10 commits
  (merged to 'next' on 2019-04-25 at 2014eef6b1)
 + cherry-pick/revert: add scissors line on merge conflict
 + sequencer.c: save and restore cleanup mode
 + merge: add scissors line on merge conflict
 + merge: cleanup messages like commit
 + parse-options.h: extract common --cleanup option
 + commit: extract cleanup_mode functions to sequencer
 + t7502: clean up style
 + t7604: clean up style
 + t3507: clean up style
 + t7600: clean up style
 (this branch uses pw/sequencer-cleanup-with-signoff-x-fix.)

 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.

 Will merge to 'master'.


* 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.

Re: What's cooking in git.git (Apr 2019, #05; Thu, 25)

[Elijah Newren]

On Thu, Apr 25, 2019 at 7:16 AM Junio C Hamano <gitster@pobox.com> wrote
> * en/unicode-in-refnames (2019-04-24) 1 commit
>  - Honor core.precomposeUnicode in more places
>
>  The names of the refs stored as filesystem entities may become
>  different from what the end-user expects, just like files in the
>  working tree gets "renamed", on a filesystem like HFS+.  Work it
>  around by paying attemption to the core.precomposeUnicode
>  configuration.
>
>  Looked sensible.  Ready for next?

Almost.  Torsten pointed out that I left some commentary in the commit
message that I should remove.  I'll send a v2 with it excised.

Also, s/it around/around it/ and s/attemption/attention/ in your summary?

> * nd/switch-and-restore (2019-04-12) 43 commits
>  - help: move git-diff and git-reset to different groups
>  - rm: add --staged as alias for --cached
>  - doc: promote "git restore"
>  - user-manual.txt: prefer 'merge --abort' over 'reset --hard'
>  - completion: support restore
>  - t: add tests for restore
>  - restore: support --patch
>  - restore: replace --force with --ignore-unmerged
>  - restore: default to --source=HEAD when only --staged is specified
>  - restore: reject invalid combinations with --staged
>  - restore: add --worktree and --staged
>  - checkout: factor out worktree checkout code
>  - restore: disable overlay mode by default
>  - restore: make pathspec mandatory
>  - restore: take tree-ish from --source option instead
>  - checkout: split part of it to new command 'restore'
>  - doc: promote "git switch"
>  - completion: support switch
>  - t: add tests for switch
>  - switch: make --orphan switch to an empty tree
>  - switch: reject if some operation is in progress
>  - switch: no worktree status unless real branch switch happens
>  - switch: implicit dwim, use --no-guess to disable it
>  - switch: add short option for --detach
>  - switch: only allow explicit detached HEAD
>  - switch: reject "do nothing" case
>  - switch: stop accepting pathspec
>  - switch: remove -l
>  - switch: add --discard-changes
>  - switch: better names for -b and -B
>  - checkout: split part of it to new command 'switch'
>  - checkout: split options[] array in three pieces
>  - checkout: move 'confict_style' and 'dwim_..' to checkout_opts
>  - checkout: make "opts" in cmd_checkout() a pointer
>  - checkout: factor out some code in parse_branchname_arg()
>  - checkout: keep most #include sorted
>  - checkout: inform the user when removing branch state
>  - checkout: advice how to get out of detached HEAD mode
>  - t: rename t2014-switch.sh to t2014-checkout-switch.sh
>  - git-checkout.txt: fix monospace typeset
>  - doc: document --overwrite-ignore
>  - git-checkout.txt: fix one syntax line
>  - git-checkout.txt: spell out --no-option
>
>  Two new commands "git switch" and "git restore" are introduced to
>  split "checking out a branch to work on advancing its history" and
>  "checking out paths out of the index and/or a tree-ish to work on
>  advancing the current history" out of the single "git checkout"
>  command.
>
>  The "switch" part seems more or less ready for testing.  Perhaps
>  we should split this back into two topics and merge it to 'next'.
>  cf. <20190329103919.15642-1-pclouds@gmail.com> (switch v6)
>  cf. <20190411131218.19195-1-pclouds@gmail.com> (restore v2)

Splitting apart and advancing "switch" makes sense to me.  I'm behind
on stuff but really need to take a look at the new restore series;
I'll try to get to that soon.

js/partial-clone-connectivity-check (was: What's cooking in git.git (Apr 2019, #05; Thu, 25))

[Josh Steadmon]

On 2019.04.25 19:15, Junio C Hamano wrote:
> * js/partial-clone-connectivity-check (2019-04-21) 1 commit
>   (merged to 'next' on 2019-04-25 at ebd8b4bffd)
>  + clone: do faster object check for partial clones
> 
>  During an initial "git clone --depth=..." partial clone, it is
>  pointless to spend cycles for a large portion of the connectivity
>  check that enumerates and skips promisor objects (which by
>  definition is all objects fetched from the other side).  This has
>  been optimized out.
> 
>  Will merge to 'master'.

Peff asked for a perf test for this [1], but I haven't had time to write one
yet. I can do that in a separate patch if you still want to merge this
as-is.

[1]: https://public-inbox.org/git/20190422213113.GB4728@sigill.intra.peff.net/

Re: What's cooking in git.git (Apr 2019, #05; Thu, 25)

[Taylor Blau]

Hi Junio,

On Thu, Apr 25, 2019 at 07:15:06PM +0900, Junio C Hamano wrote:
> * tb/unexpected (2019-04-10) 7 commits
>   (merged to 'next' on 2019-04-25 at c49927fca0)
>  + rev-list: detect broken root trees
>  + rev-list: let traversal die when --missing is not in use
>  + get_commit_tree(): return NULL for broken tree
>  + list-objects.c: handle unexpected non-tree entries
>  + list-objects.c: handle unexpected non-blob entries
>  + t: introduce tests for unexpected object types
>  + t: move 'hex2oct' into test-lib-functions.sh
>
>  Code tightening against a "wrong" object appearing where an object
>  of a different type is expected, instead of blindly assuming that
>  the connection between objects are correctly made.
>
>  Will merge to 'master'.

Thanks for picking this up. Before you merge to master, I want to make
sure that the whole series was taken in.

I can see the first four of these landed on 'next' (in
5c07647d98...b49e74eac4), but I'm having some difficulty finding the
later three.

Did you pick these up as well?

Thanks,
Taylor

Re: What's cooking in git.git (Apr 2019, #05; Thu, 25)

[Junio C Hamano]

Taylor Blau <me@ttaylorr.com> writes:

> On Thu, Apr 25, 2019 at 07:15:06PM +0900, Junio C Hamano wrote:
>> * tb/unexpected (2019-04-10) 7 commits
>>   (merged to 'next' on 2019-04-25 at c49927fca0)
>>  + rev-list: detect broken root trees
>>  + rev-list: let traversal die when --missing is not in use
>>  + get_commit_tree(): return NULL for broken tree
>>  + list-objects.c: handle unexpected non-tree entries
>>  + list-objects.c: handle unexpected non-blob entries
>>  + t: introduce tests for unexpected object types
>>  + t: move 'hex2oct' into test-lib-functions.sh
>>
>>  Code tightening against a "wrong" object appearing where an object
>>  of a different type is expected, instead of blindly assuming that
>>  the connection between objects are correctly made.
>>
>>  Will merge to 'master'.
>
> Thanks for picking this up. Before you merge to master, I want to make
> sure that the whole series was taken in.
>
> I can see the first four of these landed on 'next' (in
> 5c07647d98...b49e74eac4), but I'm having some difficulty finding the
> later three.
>
> Did you pick these up as well?

Sorry, but I do not follow.

$ git log --oneline master..c49927fca0^2
97dd512af7 rev-list: detect broken root trees
ee4dfee227 rev-list: let traversal die when --missing is not in use
834876630b get_commit_tree(): return NULL for broken tree
b49e74eac4 list-objects.c: handle unexpected non-tree entries
23c204455b list-objects.c: handle unexpected non-blob entries
0616617c7e t: introduce tests for unexpected object types
5c07647d98 t: move 'hex2oct' into test-lib-functions.sh

Do you mean you do not have b49e74eac4..c49927fca0?  I do not think
the topic was merged in multiple steps (iow, at c49927fca0^ the
whole 7 commits were not in 'next', and after c49927fca0, all 7 are
in).  So I am not sure what you are asking.  IOW, if you have b49e74
in your copy of 'next', it is impossible not to have the others.

The merge itself can be seen at one of the authoritative repositories

https://git.kernel.org/pub/scm/git/git.git/commit/?h=next&id=c49927fca0de4c213ae9b21dcb7eafb80e453d27

Unfortunately, I do not know how to ask GitHub web UI to give us a
simple "log --oneline" equivalent of list like gitweb did (sadly
cgit is not much better wrt this), but I think clicking on the
parent link starting from here

https://github.com/git/git/commit/c49927fca0de4c213ae9b21dcb7eafb80e453d27

and remembering (or writing down X-<) the commit names would
eventually give us the equivalent.

In any case, thanks for working on this topic.

Re: What's cooking in git.git (Apr 2019, #05; Thu, 25)

[Taylor Blau]

On Fri, Apr 26, 2019 at 02:41:38PM +0900, Junio C Hamano wrote:
> Taylor Blau <me@ttaylorr.com> writes:
>
> > On Thu, Apr 25, 2019 at 07:15:06PM +0900, Junio C Hamano wrote:
> >> * tb/unexpected (2019-04-10) 7 commits
> >>   (merged to 'next' on 2019-04-25 at c49927fca0)
> >>  + rev-list: detect broken root trees
> >>  + rev-list: let traversal die when --missing is not in use
> >>  + get_commit_tree(): return NULL for broken tree
> >>  + list-objects.c: handle unexpected non-tree entries
> >>  + list-objects.c: handle unexpected non-blob entries
> >>  + t: introduce tests for unexpected object types
> >>  + t: move 'hex2oct' into test-lib-functions.sh
> >>
> >>  Code tightening against a "wrong" object appearing where an object
> >>  of a different type is expected, instead of blindly assuming that
> >>  the connection between objects are correctly made.
> >>
> >>  Will merge to 'master'.
> >
> > Thanks for picking this up. Before you merge to master, I want to make
> > sure that the whole series was taken in.
> >
> > I can see the first four of these landed on 'next' (in
> > 5c07647d98...b49e74eac4), but I'm having some difficulty finding the
> > later three.
> >
> > Did you pick these up as well?
>
> Sorry, but I do not follow.
>
> $ git log --oneline master..c49927fca0^2
> 97dd512af7 rev-list: detect broken root trees
> ee4dfee227 rev-list: let traversal die when --missing is not in use
> 834876630b get_commit_tree(): return NULL for broken tree
> b49e74eac4 list-objects.c: handle unexpected non-tree entries
> 23c204455b list-objects.c: handle unexpected non-blob entries
> 0616617c7e t: introduce tests for unexpected object types
> 5c07647d98 t: move 'hex2oct' into test-lib-functions.sh
>
> Do you mean you do not have b49e74eac4..c49927fca0?  I do not think
> the topic was merged in multiple steps (iow, at c49927fca0^ the
> whole 7 commits were not in 'next', and after c49927fca0, all 7 are
> in).  So I am not sure what you are asking.  IOW, if you have b49e74
> in your copy of 'next', it is impossible not to have the others.

Ah, I _can_ see the merge in my local copy (fetched from
https://github.com/git/git) as c49927fca0 (Merge branch 'tb/unexpected'
into next, 2019-04-25).

I looked for the commits themselves with:

  $ git log --author=Taylor

on 'next', but only found the first four. I'm sure there's a logical
explanation of why this happened, but I'm not sure what it is :-).

> The merge itself can be seen at one of the authoritative repositories
>
> https://git.kernel.org/pub/scm/git/git.git/commit/?h=next&id=c49927fca0de4c213ae9b21dcb7eafb80e453d27
>
> Unfortunately, I do not know how to ask GitHub web UI to give us a
> simple "log --oneline" equivalent of list like gitweb did (sadly
> cgit is not much better wrt this), but I think clicking on the
> parent link starting from here
>
> https://github.com/git/git/commit/c49927fca0de4c213ae9b21dcb7eafb80e453d27
>
> and remembering (or writing down X-<) the commit names would
> eventually give us the equivalent.
>
> In any case, thanks for working on this topic.

Thanks for explaining, and it was my pleasure.

Thanks,
Taylor

Re: What's cooking in git.git (Apr 2019, #05; Thu, 25)

[Junio C Hamano]

Taylor Blau <me@ttaylorr.com> writes:

> Ah, I _can_ see the merge in my local copy (fetched from
> https://github.com/git/git) as c49927fca0 (Merge branch 'tb/unexpected'
> into next, 2019-04-25).
>
> I looked for the commits themselves with:
>
>   $ git log --author=Taylor
>
> on 'next', but only found the first four. I'm sure there's a logical
> explanation of why this happened, but I'm not sure what it is :-).

The last three patches were made out of these messages from you

 Message-Id: <e0bd479e822ce81de280ee6fdd07f608a96b7836.1554861974.git.me@ttaylorr.com>
 Message-Id: <88ca5dfe68c4cdb558aff6e90a525ec06f15dbd6.1554861974.git.me@ttaylorr.com>
 Message-Id: <e9400a9f773adb62315f75347ad7231b4476a2f4.1554861974.git.me@ttaylorr.com>

These were you relaying Peff's commits, for example, the topmost one

https://public-inbox.org/git/e0bd479e822ce81de280ee6fdd07f608a96b7836.1554861974.git.me@ttaylorr.com/

begins with an in-body header From: with a corresponding sign-off,
both by Peff.

Re: What's cooking in git.git (Apr 2019, #05; Thu, 25)

[Kaartic Sivaraam]

Hi Junio,



On 26 April 2019 11:11:38 GMT+05:30, Junio C Hamano <gitster@pobox.com> wrote:
>
>Unfortunately, I do not know how to ask GitHub web UI to give us a
>simple "log --oneline" equivalent of list like gitweb did (sadly
>cgit is not much better wrt this), but I think clicking on the
>parent link starting from here
>
>https://github.com/git/git/commit/c49927fca0de4c213ae9b21dcb7eafb80e453d27
>
>and remembering (or writing down X-<) the commit names would
>eventually give us the equivalent.
>

May be you are searching for the following view which lists (similar to git log --one-line) the commits starting from  97dd512 which is the last in the tb/unexpected series?

https://github.com/git/git/commits/97dd512af7ce4afb4f638ef73b4770921c8ca3aa


-- 
Sivaraam

Sent from my Android device with K-9 Mail. Please excuse my brevity.

Re: What's cooking in git.git (Apr 2019, #05; Thu, 25)

[Junio C Hamano]

2019年4月27日(土) 2:58 Kaartic Sivaraam <kaartic.sivaraam@gmail.com>:
>
> May be you are searching for the following view which lists (similar to git log --one-line) the commits starting from  97dd512 which is the last in the tb/unexpected series?
>
> https://github.com/git/git/commits/97dd512af7ce4afb4f638ef73b4770921c8ca3aa

Yes, exactly. The problem I have is it is not obvious how to get there
starting from
https://github.com/git/git/

From that starting point, I can get to
https://github.com/git/git/commits/next with
two clicks, and get a "git log --oneline next" equivalent). In there,
I can eyeball
to find "Merge branch 'tb/unexpected' into next".

I can click it and reach https://github.com/git/git/commit/c49927fca0d but then
that is more like "git show c49927fca". I can even go to its second parent with
one click, but that is equivalent to "git show 97dd512".  The page you showed,
which is a rough equivalent to "git log --oneline 97dd512", is what I
want to get
to from there, but I am not sure how to get there X-<.

Re: What's cooking in git.git (Apr 2019, #05; Thu, 25)

[Kaartic Sivaraam]

Hi Junio,



On 27 April 2019 10:36:55 GMT+05:30, Junio C Hamano <gitster@pobox.com> wrote:
>2019年4月27日(土) 2:58 Kaartic Sivaraam <kaartic.sivaraam@gmail.com>:
>>
>
>I can click it and reach https://github.com/git/git/commit/c49927fca0d
>but then
>that is more like "git show c49927fca". I can even go to its second
>parent with
>one click, but that is equivalent to "git show 97dd512a".  The page you
>showed,
>which is a rough equivalent to "git log --oneline 97dd512", is what I
>want to get
>to from there, but I am not sure how to get there X-<.

I seem to know of two ways to get to where you want to get to (git log --oneline 97dd512) from the page similar to "git show 97dd512" [1].

1. A direct way that requires two clicks:
a. You should click on the 'Browse files' button you see in the page that shows output like git show [1]. Then you'll get to the page where you see the files in the repository just like when you checkout 97dd512 [2].
b. From that page you could click on the number of commits to get to the page you want to see (page which shows output like "git log --oneline 97dd512") [3].

2. A simple but indirect way is to edit the URL when you are in page [1], and replace the 'commit' part in it with 'commits'. This is not a reliable way as there's a possibility that they could change the URL format but it seems to be working well for me so far :-)

Hope that helps.

References:

[1]: https://github.com/git/git/commit/97dd512af7ce4afb4f638ef73b4770921c8ca3aa

[2]: https://github.com/git/git/tree/97dd512af7ce4afb4f638ef73b4770921c8ca3aa

[3]: https://github.com/git/git/commits/97dd512af7ce4afb4f638ef73b4770921c8ca3aa

-- 
Sivaraam

Sent from my Android device with K-9 Mail. Please excuse my brevity.

js/macos-gettext-build, was Re: What's cooking in git.git (Apr 2019, #05; Thu, 25)

[Johannes Schindelin]

Hi Junio,

On Thu, 25 Apr 2019, Junio C Hamano wrote:

> * js/macos-gettext-build (2019-04-15) 1 commit
>   (merged to 'next' on 2019-04-22 at de4cbb1431)
>  + macOS: make sure that gettext is found
>
>  Build with gettext breaks on recent macOS w/ Homebrew when
>  /usr/local/bin is not on PATH, which has been corrected.

This description is almost correct: the problem fixed in this patch is
that `/usr/local/include` seems not to be in the header search path of
Homebrew's `gcc` on Mojave.

The `PATH` does contain `/usr/local/bin` alright.

Ciao,
Dscho

Re: js/partial-clone-connectivity-check (was: What's cooking in git.git (Apr 2019, #05; Thu, 25))

[Jeff King]

On Thu, Apr 25, 2019 at 03:16:57PM -0700, Josh Steadmon wrote:

> On 2019.04.25 19:15, Junio C Hamano wrote:
> > * js/partial-clone-connectivity-check (2019-04-21) 1 commit
> >   (merged to 'next' on 2019-04-25 at ebd8b4bffd)
> >  + clone: do faster object check for partial clones
> > 
> >  During an initial "git clone --depth=..." partial clone, it is
> >  pointless to spend cycles for a large portion of the connectivity
> >  check that enumerates and skips promisor objects (which by
> >  definition is all objects fetched from the other side).  This has
> >  been optimized out.
> > 
> >  Will merge to 'master'.
> 
> Peff asked for a perf test for this [1], but I haven't had time to write one
> yet. I can do that in a separate patch if you still want to merge this
> as-is.

I won't die without one, but it would be nice. It may also be that an
existing perf test, but I don't think we cover partial clones in t/perf
at all. Might be worth just a straight-up "git clone --filter=blob:none"
test.

Also, in the proposed merge message above, it should be --filter, not
--depth, right?

-Peff

Re: What's cooking in git.git (Apr 2019, #05; Thu, 25)

[Jeff King]

On Sat, Apr 27, 2019 at 02:06:55PM +0900, Junio C Hamano wrote:

> 2019年4月27日(土) 2:58 Kaartic Sivaraam <kaartic.sivaraam@gmail.com>:
> >
> > May be you are searching for the following view which lists (similar to git log --one-line) the commits starting from  97dd512 which is the last in the tb/unexpected series?
> >
> > https://github.com/git/git/commits/97dd512af7ce4afb4f638ef73b4770921c8ca3aa
> 
> Yes, exactly. The problem I have is it is not obvious how to get there
> starting from
> https://github.com/git/git/

Not exactly in git.git, but I'd probably do something like:

  https://github.com/git/git/compare/master...gitster:tb/unexpected

You can get there by going to the "branches" tab in gitster/git, then
"tb/unexpected", and then "compare" (either from the branch selection
page, or if you've already clicked through from the tree view of that
branch).

From _just_ inside git.git it's trickier, since it doesn't have that
branch. I don't think there's an easy way to click through to a
compare-view there, since arbitrary commits that people are looking at
might or might not be useful tips for the compare-view (which is also
really the pull request view).

I'd probably just generate this URL myself:

  https://github.com/git/git/compare/master...c49927fca0de4c213ae9b21dcb7eafb80e453d27^2

since it's stable and pretty readable.

Probably more than you wanted to know, but I feel like I ought to be
able to drop at least a little GitHub wisdom. ;)

-Peff

Re: js/partial-clone-connectivity-check (was: What's cooking in git.git (Apr 2019, #05; Thu, 25))

[Josh Steadmon]

On 2019.05.01 22:52, Jeff King wrote:
> On Thu, Apr 25, 2019 at 03:16:57PM -0700, Josh Steadmon wrote:
> 
> > On 2019.04.25 19:15, Junio C Hamano wrote:
> > > * js/partial-clone-connectivity-check (2019-04-21) 1 commit
> > >   (merged to 'next' on 2019-04-25 at ebd8b4bffd)
> > >  + clone: do faster object check for partial clones
> > > 
> > >  During an initial "git clone --depth=..." partial clone, it is
> > >  pointless to spend cycles for a large portion of the connectivity
> > >  check that enumerates and skips promisor objects (which by
> > >  definition is all objects fetched from the other side).  This has
> > >  been optimized out.
> > > 
> > >  Will merge to 'master'.
> > 
> > Peff asked for a perf test for this [1], but I haven't had time to write one
> > yet. I can do that in a separate patch if you still want to merge this
> > as-is.
> 
> I won't die without one, but it would be nice. It may also be that an
> existing perf test, but I don't think we cover partial clones in t/perf
> at all. Might be worth just a straight-up "git clone --filter=blob:none"
> test.
> 
> Also, in the proposed merge message above, it should be --filter, not
> --depth, right?

Yes, thanks for the catch.

Re: js/partial-clone-connectivity-check (was: What's cooking in git.git (Apr 2019, #05; Thu, 25))

[Jeff King]

On Wed, May 01, 2019 at 10:52:06PM -0400, Jeff King wrote:

> > > * js/partial-clone-connectivity-check (2019-04-21) 1 commit
> > >   (merged to 'next' on 2019-04-25 at ebd8b4bffd)
> > >  + clone: do faster object check for partial clones
> > > 
> > >  During an initial "git clone --depth=..." partial clone, it is
> > >  pointless to spend cycles for a large portion of the connectivity
> > >  check that enumerates and skips promisor objects (which by
> > >  definition is all objects fetched from the other side).  This has
> > >  been optimized out.
> > > 
> > >  Will merge to 'master'.
> > 
> > Peff asked for a perf test for this [1], but I haven't had time to write one
> > yet. I can do that in a separate patch if you still want to merge this
> > as-is.
> 
> I won't die without one, but it would be nice. It may also be that an
> existing perf test, but I don't think we cover partial clones in t/perf
> at all. Might be worth just a straight-up "git clone --filter=blob:none"
> test.

Here's what I came up with. Note that there's a bug in 'master' right
now which causes perf to produce nonsense results. It's due to my
0baf78e7bc (perf-lib.sh: rely on test-lib.sh for --tee handling,
2019-03-15). I'll fix that separately (the timing below is done with
that commit reverted).

-- >8 --
Subject: [PATCH] t/perf: add perf script for partial clones

We don't cover the partial clone feature at all in t/perf. Let's at
least run a few basic tests so that we'll notice any regressions.

We'll do a no-blob clone, and split it into two parts: the actual object
transfer, and the subsequent checkout (which will of course require
another transfer to get the blobs). That will help us more clearly
assess the performance of each.

There are obviously a lot more possibilities besides just a no-blob
partial clone, but this should serve as a canary that alerts us to any
generic slow-downs (and we can add more tests later for cases that
aren't exercised here).

There are a few non-ideal things here that make this not an entirely
accurate test, but are probably OK for our purposes:

  1. We have to do some extra prep/cleanup work inside the timing tests,
     since they impact the on-disk state and the perf harness may run
     each one multiple times.

     In practice this is probably OK, since these bits should be much
     less expensive than the operations we are measuring.

  2. The clone time is likely to be dominated by the server's object
     enumeration. In the real world, a repo large enough to drive people
     to partial clones is likely to have reachability bitmaps enabled.

     And in the opposite direction, our object transfer is happening at
     the speed of a local pipe, whereas in the real world it would
     bottle-neck on the network.

     So any percentage speedups should be taken with a grain of salt.
     But hopefully any regressions will produce enough of an effect to
     be noticeable.

This script also demonstrates the recent improvement from dfa33a298d
(clone: do faster object check for partial clones, 2019-04-19):

  Test                          dfa33a298d^         dfa33a298d
  -------------------------------------------------------------------------
  5600.2: clone without blobs   18.41(22.72+1.09)   6.83(11.65+0.50) -62.9%
  5600.3: checkout of result    1.82(3.24+0.26)     1.84(3.24+0.26) +1.1%

Signed-off-by: Jeff King <peff@peff.net>
---
The speedup from dfa33a298d was larger than I expected. Doing a full
`rev-list --objects --all` on a non-partial clone is only 3-4s. So how
did we manage to save 11s by removing it? The answer is that the
is_promisor check is super expensive: we have to walk all of those trees
in the partial pack to find which blobs are mentioned, only to then walk
them all again for the actual traversal and say "yep, this is in our
promisor list". Yikes.

Your patch makes all of that go away, but I do think there's room for us
to be more clever here (but that leads us back down the rabbit hole you
explored earlier, so I think it makes sense to take your patch now and
deal with other optimizations separately).

 t/perf/p5600-partial-clone.sh | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)
 create mode 100755 t/perf/p5600-partial-clone.sh

diff --git a/t/perf/p5600-partial-clone.sh b/t/perf/p5600-partial-clone.sh
new file mode 100755
index 0000000000..3e04bd2ae1
--- /dev/null
+++ b/t/perf/p5600-partial-clone.sh
@@ -0,0 +1,26 @@
+#!/bin/sh
+
+test_description='performance of partial clones'
+. ./perf-lib.sh
+
+test_perf_default_repo
+
+test_expect_success 'enable server-side config' '
+	git config uploadpack.allowFilter true &&
+	git config uploadpack.allowAnySHA1InWant true
+'
+
+test_perf 'clone without blobs' '
+	rm -rf bare.git &&
+	git clone --no-local --bare --filter=blob:none . bare.git
+'
+
+test_perf 'checkout of result' '
+	rm -rf worktree &&
+	mkdir -p worktree/.git &&
+	tar -C bare.git -cf - . | tar -C worktree/.git -xf - &&
+	git -C worktree config core.bare false &&
+	git -C worktree checkout -f
+'
+
+test_done
-- 
2.21.0.1314.g224b191707

Re: js/partial-clone-connectivity-check (was: What's cooking in git.git (Apr 2019, #05; Thu, 25))

[Jeff King]

On Thu, May 02, 2019 at 05:45:09PM -0400, Jeff King wrote:

> Here's what I came up with. Note that there's a bug in 'master' right
> now which causes perf to produce nonsense results. It's due to my
> 0baf78e7bc (perf-lib.sh: rely on test-lib.sh for --tee handling,
> 2019-03-15). I'll fix that separately (the timing below is done with
> that commit reverted).

And here's the fix for that. It's rather subtle, so I hope I explained
it sufficiently. I didn't notice it while working on the original
because everything _appears_ to run fine, but you just get timings from
the wrong version of Git. Which is only noticeable if you're literally
testing two versions that you expect to differ.

-- >8 --
Subject: [PATCH] t/perf: set GIT_TEST_INSTALLED before including test-lib.sh

Commit 0baf78e7bc (perf-lib.sh: rely on test-lib.sh for --tee handling,
2019-03-15) introduced a bug which causes perf test runs to always find
git in the $PATH, rather than actually testing $GIT_TEST_INSTALLED
(which is used by the "run" script).

That commit bumped the conversion of $GIT_TEST_INSTALLED to an absolute
path until after test-lib.sh was been included. That solved the original
problem of generating a bogus $perf_results_prefix value from the
absolute path. But it introduced a new one: test-lib.sh needs to see
that absolute value because it will put it at the front of the $PATH.

The root of the issue is that we need both the relative and the absolute
paths available. That earlier commit chose to stuff the absolute path
into $ABSOLUTE_GIT_TEST_INSTALLED, keep the relative one in
$GIT_TEST_INSTALLED, and then resolve the two after test-lib.sh has been
loaded. We can fix it by reversing our strategy: we'll keep the absolute
path in $GIT_TEST_INSTALLED, drop our now-useless ABSOLUTE variable, and
introduce a new $TEST_INSTALLED_ORIG variable to hold the original value
(from which we will later derive the prefix).

Note that we need to export our ORIG variable because the whole point of
0baf78e7bc is that test-lib.sh may actually re-exec a new copy of the
script to handle tee-ing the output.

I'd hoped to add a test to p0000-perf-lib-sanity to cover this, but the
bug is actually outside that scope. It triggers if you do something
like:

  cat >t/perf/p1234-foo.sh <<-\EOF
  #!/bin/sh
  test_description=debugging
  . ./perf-lib.sh
  test_expect_success 'version' 'which git && git version'
  test_perf 'fake timing' 'true'
  test_done
  EOF

  chmod +x t/perf/p1234-foo.sh
  make GIT_TEST_OPTS=--verbose-log
  cd t/perf
  ./run origin p1234-foo.sh
  cat test-results/p1234-foo.out

Without this patch, you'll see that we're running whatever git is in
your $PATH, not the one from "origin". With this patch, we should see:

  - the correct git is run

  - we get timing results out of ./run, showing that the prefix bug from
    0baf78e7bc was not regressed

  - our values make it across the script re-exec due to --verbose-log

Signed-off-by: Jeff King <peff@peff.net>
---
 t/perf/perf-lib.sh | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/t/perf/perf-lib.sh b/t/perf/perf-lib.sh
index 169f92eae3..8317dbef44 100644
--- a/t/perf/perf-lib.sh
+++ b/t/perf/perf-lib.sh
@@ -21,7 +21,11 @@
 # because it will change our working directory.
 TEST_DIRECTORY=$(pwd)/..
 TEST_OUTPUT_DIRECTORY=$(pwd)
-ABSOLUTE_GIT_TEST_INSTALLED=$(
+
+# Likewise we must turn GIT_TEST_INSTALLED into an absolute path; but remember
+# and export the original value, since we'll later generate our prefix from it.
+: ${TEST_INSTALLED_ORIG=$GIT_TEST_INSTALLED}; export TEST_INSTALLED_ORIG
+GIT_TEST_INSTALLED=$(
 	test -n "$GIT_TEST_INSTALLED" && cd "$GIT_TEST_INSTALLED" && pwd)
 
 TEST_NO_CREATE_REPO=t
@@ -32,8 +36,7 @@ TEST_NO_MALLOC_CHECK=t
 if test -z "$GIT_TEST_INSTALLED"; then
 	perf_results_prefix=
 else
-	perf_results_prefix=$(printf "%s" "${GIT_TEST_INSTALLED%/bin-wrappers}" | tr -c "[a-zA-Z0-9]" "[_*]")"."
-	GIT_TEST_INSTALLED=$ABSOLUTE_GIT_TEST_INSTALLED
+	perf_results_prefix=$(printf "%s" "${TEST_INSTALLED_ORIG%/bin-wrappers}" | tr -c "[a-zA-Z0-9]" "[_*]")"."
 fi
 
 # Variables from test-lib that are normally internal to the tests; we
-- 
2.21.0.1314.g224b191707

Re: js/macos-gettext-build, was Re: What's cooking in git.git (Apr 2019, #05; Thu, 25)

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Hi Junio,
>
> On Thu, 25 Apr 2019, Junio C Hamano wrote:
>
>> * js/macos-gettext-build (2019-04-15) 1 commit
>>   (merged to 'next' on 2019-04-22 at de4cbb1431)
>>  + macOS: make sure that gettext is found
>>
>>  Build with gettext breaks on recent macOS w/ Homebrew when
>>  /usr/local/bin is not on PATH, which has been corrected.
>
> This description is almost correct: the problem fixed in this patch is
> that `/usr/local/include` seems not to be in the header search path of
> Homebrew's `gcc` on Mojave.

Ack.  Thanks.  Will update the draft release notes.

Re: What's cooking in git.git (Apr 2019, #05; Thu, 25)

[Duy Nguyen]

On Thu, Apr 25, 2019 at 8:15 PM Junio C Hamano <gitster@pobox.com> wrote:
> * nd/config-move-to (2019-01-14) 7 commits
>  - config.h: fix hdr-check warnings
>  - config: add --move-to
>  - config: factor out set_config_source_file()
>  - config: use OPT_FILENAME()
>  - config.c: add repo_config_set_worktree_gently()
>  - worktree.c: add get_worktree_config()
>  - config.c: avoid git_path() in do_git_config_sequence()
>
>  Needs review.

Please drop this for now. I still want to make it work (because I need
it). But I'll try again with a full series of submodule/worktree
support. Hopefully that will catch some reviewer's attention.
-- 
Duy

[PATCH] perf-lib.sh: make "./run <revisions>" use the correct gits

[Ævar Arnfjörð Bjarmason]

Fix a really bad regression in 0baf78e7bc ("perf-lib.sh: rely on
test-lib.sh for --tee handling", 2019-03-15). Since that change all
runs of different <revisions> of git have used the git found in the
user's $PATH, e.g. /usr/bin/git instead of the <revision> we just
built and wanted to performance test.

The problem starts with GIT_TEST_INSTALLED not working like our
non-perf tests with the "run" script. I.e. you can't run performance
tests against a given installed git. Instead we expect to use it
ourselves to point GIT_TEST_INSTALLED to the <revision> we just built.

However, we had been relying on '$(cd "$GIT_TEST_INSTALLED" && pwd)'
to resolve that relative $GIT_TEST_INSTALLED to an absolute
path *before* test-lib.sh was loaded, in cases where it was
e.g. "build/<rev>/bin-wrappers" and we wanted "<abs_path>build/...".

Perhaps there's some better way to fix this, but it seems to me that
the best solution is to just make this behavior less magical. We know
in run_dirs_helper() that we're about to run performance tests on a
given <revision>, so let's just set GIT_TEST_INSTALLED to an absolute
path there, and then make getting logging target from a previously
relative path less magical, we'll just explicitly pass down the
relative path as a variable.

This makes e.g. these cases all work:

    ./run . $PWD/../../ origin/master origin/next HEAD -- <tests>

As well as just a plain one-off:

    ./run <tests>

And, since we're passing down the new GIT_PERF_DIR_MYDIR_REL we make
sure the bug relating to aggregate.perl not finding our files as
described in 0baf78e7bc doesn't happen again.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---

On Fri, May 03 2019, Jeff King wrote:

> On Thu, May 02, 2019 at 05:45:09PM -0400, Jeff King wrote:
>
>> Here's what I came up with. Note that there's a bug in 'master' right
>> now which causes perf to produce nonsense results. It's due to my
>> 0baf78e7bc (perf-lib.sh: rely on test-lib.sh for --tee handling,
>> 2019-03-15). I'll fix that separately (the timing below is done with
>> that commit reverted).
>
> And here's the fix for that. It's rather subtle, so I hope I explained
> it sufficiently. I didn't notice it while working on the original
> because everything _appears_ to run fine, but you just get timings from
> the wrong version of Git. Which is only noticeable if you're literally
> testing two versions that you expect to differ.

I ran into this today and it took me an embarrasingly long time to
figure out why my code wasn't making things faster.

So I wrote this up before seeing your patch, since it wasn't queued in
"pu" and my naïve ML search didn't include inline patches (again,
*sigh*).

Anyway, I wonder if something closer to this patch, or some sort of
merge of the two (e.g. to still get rid of the
ABSOLUTE_GIT_TEST_INSTALLED variable) is better. I.e. why try to
magically detect all of this in perf-lib.sh itself, we know we're
going to invoke it like this in the "run" script, so we can just set
the appropriate variables there instead of this hard-to-explain magic
of $GIT_TEST_INSTALLED being one value the first time, but another one
the second time around.

 t/perf/perf-lib.sh | 4 ++++
 t/perf/run         | 8 ++++++--
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/t/perf/perf-lib.sh b/t/perf/perf-lib.sh
index 169f92eae3..b15ee1d262 100644
--- a/t/perf/perf-lib.sh
+++ b/t/perf/perf-lib.sh
@@ -32,6 +32,10 @@ TEST_NO_MALLOC_CHECK=t
 if test -z "$GIT_TEST_INSTALLED"; then
 	perf_results_prefix=
 else
+	if test -n "$GIT_PERF_DIR_MYDIR_REL"
+	then
+		GIT_TEST_INSTALLED=$GIT_PERF_DIR_MYDIR_REL
+	fi
 	perf_results_prefix=$(printf "%s" "${GIT_TEST_INSTALLED%/bin-wrappers}" | tr -c "[a-zA-Z0-9]" "[_*]")"."
 	GIT_TEST_INSTALLED=$ABSOLUTE_GIT_TEST_INSTALLED
 fi
diff --git a/t/perf/run b/t/perf/run
index 9aaa733c77..0a7c8744ab 100755
--- a/t/perf/run
+++ b/t/perf/run
@@ -91,10 +91,14 @@ run_dirs_helper () {
 	if test "$mydir" = .; then
 		unset GIT_TEST_INSTALLED
 	else
-		GIT_TEST_INSTALLED="$mydir/bin-wrappers"
+		GIT_PERF_DIR_MYDIR_REL=$mydir
+		GIT_PERF_DIR_MYDIR_ABS=$(cd $mydir && pwd)
+		export GIT_PERF_DIR_MYDIR_REL GIT_PERF_DIR_MYDIR_ABS
+
+		GIT_TEST_INSTALLED="$GIT_PERF_DIR_MYDIR_ABS/bin-wrappers"
 		# Older versions of git lacked bin-wrappers; fallback to the
 		# files in the root.
-		test -d "$GIT_TEST_INSTALLED" || GIT_TEST_INSTALLED=$mydir
+		test -d "$GIT_TEST_INSTALLED" || GIT_TEST_INSTALLED=$GIT_PERF_DIR_MYDIR_ABS
 		export GIT_TEST_INSTALLED
 	fi
 	run_one_dir "$@"
-- 
2.21.0.593.g511ec345e18

Re: [PATCH] perf-lib.sh: make "./run <revisions>" use the correct gits

[Jeff King]

On Mon, May 06, 2019 at 09:16:11PM +0200, Ævar Arnfjörð Bjarmason wrote:

> Perhaps there's some better way to fix this, but it seems to me that
> the best solution is to just make this behavior less magical. We know
> in run_dirs_helper() that we're about to run performance tests on a
> given <revision>, so let's just set GIT_TEST_INSTALLED to an absolute
> path there, and then make getting logging target from a previously
> relative path less magical, we'll just explicitly pass down the
> relative path as a variable.
> 
> This makes e.g. these cases all work:
> 
>     ./run . $PWD/../../ origin/master origin/next HEAD -- <tests>
> 
> As well as just a plain one-off:
> 
>     ./run <tests>

Doing this naively would break anybody doing:

  GIT_TEST_INSTALLED=some-relative-path ./p1234-foo.sh

but I doubt that actually matters in practice (notably this already does
not work with non-perf tests, as test-lib.sh does not do any
normalization).

I don't think your patch does, because it leaves the extra absolutizing
in perf-lib.sh. But then it doesn't feel like it's really simplified
anything. ;)

> diff --git a/t/perf/perf-lib.sh b/t/perf/perf-lib.sh
> index 169f92eae3..b15ee1d262 100644
> --- a/t/perf/perf-lib.sh
> +++ b/t/perf/perf-lib.sh
> @@ -32,6 +32,10 @@ TEST_NO_MALLOC_CHECK=t
>  if test -z "$GIT_TEST_INSTALLED"; then
>  	perf_results_prefix=
>  else
> +	if test -n "$GIT_PERF_DIR_MYDIR_REL"
> +	then
> +		GIT_TEST_INSTALLED=$GIT_PERF_DIR_MYDIR_REL
> +	fi
>  	perf_results_prefix=$(printf "%s" "${GIT_TEST_INSTALLED%/bin-wrappers}" | tr -c "[a-zA-Z0-9]" "[_*]")"."
>  	GIT_TEST_INSTALLED=$ABSOLUTE_GIT_TEST_INSTALLED
>  fi

So we reset GIT_TEST_INSTALLED to the relative path here (ignoring
what's in it!), and then afterwards set it to the absolute path. That
still seems rather magical. :)

What if instead we:

  - taught test-lib.sh to make GIT_TEST_INSTALLED absolute (since after
    all it is the one who is planning to chdir and wreck the relative
    path)

  - let callers pass in $GIT_PERF_RESULTS_PREFIX instead of guessing at
    it ourselves from the path name. Then the "run" script could quite
    reasonably just pass in the tree oid it already has instead of us
    trying to decode it. And nobody would care about whether
    $GIT_TEST_INSTALLED has been mangled.

I thought about going this route for my original patch, but I wanted to
fix the regression (which I agree is quite serious and embarrassing) as
quickly and simply as possible.

-Peff

[PATCH v2 0/2] perf-lib.sh: fix 0baf78e7bc regression, refactor & fix bugs

[Ævar Arnfjörð Bjarmason]

On Mon, May 06 2019, Jeff King wrote:

> On Mon, May 06, 2019 at 09:16:11PM +0200, Ævar Arnfjörð Bjarmason wrote:
>
>> Perhaps there's some better way to fix this, but it seems to me that
>> the best solution is to just make this behavior less magical. We know
>> in run_dirs_helper() that we're about to run performance tests on a
>> given <revision>, so let's just set GIT_TEST_INSTALLED to an absolute
>> path there, and then make getting logging target from a previously
>> relative path less magical, we'll just explicitly pass down the
>> relative path as a variable.
>> 
>> This makes e.g. these cases all work:
>> 
>>     ./run . $PWD/../../ origin/master origin/next HEAD -- <tests>
>> 
>> As well as just a plain one-off:
>> 
>>     ./run <tests>
>
> Doing this naively would break anybody doing:
>
>   GIT_TEST_INSTALLED=some-relative-path ./p1234-foo.sh
>
> but I doubt that actually matters in practice (notably this already does
> not work with non-perf tests, as test-lib.sh does not do any
> normalization).

Yeah this has never worked and GIT_TEST_INSTALLED has been documented
to take an absolute path when used with the normal test suite.

I've noted as much in the 1/2 here.

> I don't think your patch does, because it leaves the extra absolutizing
> in perf-lib.sh. But then it doesn't feel like it's really simplified
> anything. ;)

Yeah, v1 didn't because I was trying for a minimal regression fix
without digging us fully out of that GIT_TEST_INSTALLED hole, but as
2/2 here shows we can fully get rid of it from perf-lib.sh, which I
think makes things much simpler.

>> diff --git a/t/perf/perf-lib.sh b/t/perf/perf-lib.sh
>> index 169f92eae3..b15ee1d262 100644
>> --- a/t/perf/perf-lib.sh
>> +++ b/t/perf/perf-lib.sh
>> @@ -32,6 +32,10 @@ TEST_NO_MALLOC_CHECK=t
>>  if test -z "$GIT_TEST_INSTALLED"; then
>>  	perf_results_prefix=
>>  else
>> +	if test -n "$GIT_PERF_DIR_MYDIR_REL"
>> +	then
>> +		GIT_TEST_INSTALLED=$GIT_PERF_DIR_MYDIR_REL
>> +	fi
>>  	perf_results_prefix=$(printf "%s" "${GIT_TEST_INSTALLED%/bin-wrappers}" | tr -c "[a-zA-Z0-9]" "[_*]")"."
>>  	GIT_TEST_INSTALLED=$ABSOLUTE_GIT_TEST_INSTALLED
>>  fi
>
> So we reset GIT_TEST_INSTALLED to the relative path here (ignoring
> what's in it!), and then afterwards set it to the absolute path. That
> still seems rather magical. :)
>
> What if instead we:
>
>   - taught test-lib.sh to make GIT_TEST_INSTALLED absolute (since after
>     all it is the one who is planning to chdir and wreck the relative
>     path)
>
>   - let callers pass in $GIT_PERF_RESULTS_PREFIX instead of guessing at
>     it ourselves from the path name. Then the "run" script could quite
>     reasonably just pass in the tree oid it already has instead of us
>     trying to decode it. And nobody would care about whether
>     $GIT_TEST_INSTALLED has been mangled.
>
> I thought about going this route for my original patch, but I wanted to
> fix the regression (which I agree is quite serious and embarrassing) as
> quickly and simply as possible.

It seems simplest after the changes I've made here to just make a
relative GIT_TEST_INSTALLED be an error in test-lib.sh, why bend over
backwards to support it?

Re GIT_PERF_RESULTS_PREFIX: Depending on what you mean we now have
that in 2/2 as PERF_RESULTS_PREFIX.

But if you mean the user can pass it in that doesn't make sense, since
we need to pick a different prefix revision we test, so it's a
many-to-many relationship.

As seen in 2/2 modifying some of the shell & associated Perl code it's
a bit nasty that we need to duplicate the logic for picking these
PERF_RESULTS_PREFIXes in the shell code and Perl code. Ideally the
shell code would pick it, and pass the mapping to the Perl code
somehow. But that's another "has sucked since forever, future TODO"
item.

Ævar Arnfjörð Bjarmason (2):
  perf-lib.sh: make "./run <revisions>" use the correct gits
  perf-lib.sh: remove GIT_TEST_INSTALLED from perf-lib.sh

 t/perf/aggregate.perl | 13 ++++++-------
 t/perf/perf-lib.sh    | 11 +----------
 t/perf/run            | 41 +++++++++++++++++++++++++++++++----------
 3 files changed, 38 insertions(+), 27 deletions(-)

-- 
2.21.0.593.g511ec345e18

[PATCH v2 1/2] perf-lib.sh: make "./run <revisions>" use the correct gits

[Ævar Arnfjörð Bjarmason]

Fix a really bad regression in 0baf78e7bc ("perf-lib.sh: rely on
test-lib.sh for --tee handling", 2019-03-15). Since that change all
runs of different <revisions> of git have used the git found in the
user's $PATH, e.g. /usr/bin/git instead of the <revision> we just
built and wanted to performance test.

The problem starts with GIT_TEST_INSTALLED not working like our
non-perf tests with the "run" script. I.e. you can't run performance
tests against a given installed git. Instead we expect to use it
ourselves to point GIT_TEST_INSTALLED to the <revision> we just built.

However, we had been relying on '$(cd "$GIT_TEST_INSTALLED" && pwd)'
to resolve that relative $GIT_TEST_INSTALLED to an absolute
path *before* test-lib.sh was loaded, in cases where it was
e.g. "build/<rev>/bin-wrappers" and we wanted "<abs_path>build/...".

This change post-dates another proposed solution by a few days[1], I
didn't notice that version when I initially wrote this. I'm doing the
most minimal thing to solve the regression here, a follow-up change
will move this result prefix selection logic entirely into the "run"
script.

This makes e.g. these cases all work:

    ./run . $PWD/../../ origin/master origin/next HEAD -- <tests>

As well as just a plain one-off:

    ./run <tests>

And, since we're passing down the new GIT_PERF_DIR_MYDIR_REL we make
sure the bug relating to aggregate.perl not finding our files as
described in 0baf78e7bc doesn't happen again.

What *doesn't* work is setting GIT_TEST_INSTALLED to a relative path,
this will subtly fail in test-lib.sh. This has always been the case
even before 0baf78e7bc, and as documented in t/README the
GIT_TEST_INSTALLED variable should be set to an absolute path (needs
to be set "to the bindir", which is always absolute). Perhaps that
should be dealt with in the future, but I'm leaving that alone for
now.

1. https://public-inbox.org/git/20190502222409.GA15631@sigill.intra.peff.net/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/perf/perf-lib.sh | 4 ++++
 t/perf/run         | 8 ++++++--
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/t/perf/perf-lib.sh b/t/perf/perf-lib.sh
index 169f92eae3..b15ee1d262 100644
--- a/t/perf/perf-lib.sh
+++ b/t/perf/perf-lib.sh
@@ -32,6 +32,10 @@ TEST_NO_MALLOC_CHECK=t
 if test -z "$GIT_TEST_INSTALLED"; then
 	perf_results_prefix=
 else
+	if test -n "$GIT_PERF_DIR_MYDIR_REL"
+	then
+		GIT_TEST_INSTALLED=$GIT_PERF_DIR_MYDIR_REL
+	fi
 	perf_results_prefix=$(printf "%s" "${GIT_TEST_INSTALLED%/bin-wrappers}" | tr -c "[a-zA-Z0-9]" "[_*]")"."
 	GIT_TEST_INSTALLED=$ABSOLUTE_GIT_TEST_INSTALLED
 fi
diff --git a/t/perf/run b/t/perf/run
index 9aaa733c77..0a7c8744ab 100755
--- a/t/perf/run
+++ b/t/perf/run
@@ -91,10 +91,14 @@ run_dirs_helper () {
 	if test "$mydir" = .; then
 		unset GIT_TEST_INSTALLED
 	else
-		GIT_TEST_INSTALLED="$mydir/bin-wrappers"
+		GIT_PERF_DIR_MYDIR_REL=$mydir
+		GIT_PERF_DIR_MYDIR_ABS=$(cd $mydir && pwd)
+		export GIT_PERF_DIR_MYDIR_REL GIT_PERF_DIR_MYDIR_ABS
+
+		GIT_TEST_INSTALLED="$GIT_PERF_DIR_MYDIR_ABS/bin-wrappers"
 		# Older versions of git lacked bin-wrappers; fallback to the
 		# files in the root.
-		test -d "$GIT_TEST_INSTALLED" || GIT_TEST_INSTALLED=$mydir
+		test -d "$GIT_TEST_INSTALLED" || GIT_TEST_INSTALLED=$GIT_PERF_DIR_MYDIR_ABS
 		export GIT_TEST_INSTALLED
 	fi
 	run_one_dir "$@"
-- 
2.21.0.593.g511ec345e18

[PATCH v2 2/2] perf-lib.sh: remove GIT_TEST_INSTALLED from perf-lib.sh

[Ævar Arnfjörð Bjarmason]

Follow-up my preceding change which fixed the immediate "./run
<revisions>" regression in 0baf78e7bc ("perf-lib.sh: rely on
test-lib.sh for --tee handling", 2019-03-15) and entirely get rid of
GIT_TEST_INSTALLED from perf-lib.sh (and aggregate.perl).

As noted in that change the dance we're doing with GIT_TEST_INSTALLED
perf-lib.sh isn't necessary, but there I was doing the most minimal
set of changes to quickly fix a regression.

But it's much simpler to never deal with the "GIT_TEST_INSTALLED" we
were setting in perf-lib.sh at all. Instead the run_dirs_helper() sets
the previously inferred $PERF_RESULTS_PREFIX directly.

Setting this at the callsite that's already best positioned to
exhaustively know about all the different cases we need to handle
where PERF_RESULTS_PREFIX isn't what we want already (the empty
string) makes the most sense. In one-off cases like:

    ./run ./p0000-perf-lib-sanity.sh
    ./p0000-perf-lib-sanity.sh

We'll just do the right thing because PERF_RESULTS_PREFIX will be
empty, and test-lib.sh takes care of finding where our git is.

Refactoring this revealed a few bugs, e.g. while a relative git path
was supported via e.g.:

    ./run ../../ -- <test>

We'd just print out ".." as the header, since we'd always take the
content after the last slash. Now we'll always resolve the absolute
path to something we detect to be be a manually supplied bindir, and
print the full path in the aggregation.

There was also a long-standing bug in the codespeed output where the
"environment" for N number of tests would be whatever our
GIT_TEST_INSTALLED had been set to by the last of those N runs. Let's
instead just fall back to "uname -r", which is a more sensible
"environment" than some random build directory path, even for the N=1
case.

Also simplify the "[_*]" on the RHS of "tr -c", we're trimming
everything to "_", so we don't need that.

https://public-inbox.org/git/20190502222409.GA15631@sigill.intra.peff.net/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/perf/aggregate.perl | 13 ++++++-------
 t/perf/perf-lib.sh    | 15 +--------------
 t/perf/run            | 45 +++++++++++++++++++++++++++++--------------
 3 files changed, 38 insertions(+), 35 deletions(-)

diff --git a/t/perf/aggregate.perl b/t/perf/aggregate.perl
index 494907a892..c8f4a78903 100755
--- a/t/perf/aggregate.perl
+++ b/t/perf/aggregate.perl
@@ -6,6 +6,7 @@
 use JSON;
 use Getopt::Long;
 use Git;
+use Cwd qw(realpath);
 
 sub get_times {
 	my $name = shift;
@@ -103,13 +104,14 @@ sub format_size {
 	if (! -d $arg) {
 		my $rev = Git::command_oneline(qw(rev-parse --verify), $arg);
 		$dir = "build/".$rev;
+	} elsif ($arg eq '.') {
+		$dir = '.';
 	} else {
-		$arg =~ s{/*$}{};
-		$dir = $arg;
-		$dirabbrevs{$dir} = $dir;
+		$dir = realpath($arg);
+		$dirnames{$dir} = $dir;
 	}
 	push @dirs, $dir;
-	$dirnames{$dir} = $arg;
+	$dirnames{$dir} ||= $arg;
 	my $prefix = $dir;
 	$prefix =~ tr/^a-zA-Z0-9/_/c;
 	$prefixes{$dir} = $prefix . '.';
@@ -312,9 +314,6 @@ sub print_codespeed_results {
 		$environment = $reponame;
 	} elsif (exists $ENV{GIT_PERF_REPO_NAME} and $ENV{GIT_PERF_REPO_NAME} ne "") {
 		$environment = $ENV{GIT_PERF_REPO_NAME};
-	} elsif (exists $ENV{GIT_TEST_INSTALLED} and $ENV{GIT_TEST_INSTALLED} ne "") {
-		$environment = $ENV{GIT_TEST_INSTALLED};
-		$environment =~ s|/bin-wrappers$||;
 	} else {
 		$environment = `uname -r`;
 		chomp $environment;
diff --git a/t/perf/perf-lib.sh b/t/perf/perf-lib.sh
index b15ee1d262..9cdccba222 100644
--- a/t/perf/perf-lib.sh
+++ b/t/perf/perf-lib.sh
@@ -21,25 +21,12 @@
 # because it will change our working directory.
 TEST_DIRECTORY=$(pwd)/..
 TEST_OUTPUT_DIRECTORY=$(pwd)
-ABSOLUTE_GIT_TEST_INSTALLED=$(
-	test -n "$GIT_TEST_INSTALLED" && cd "$GIT_TEST_INSTALLED" && pwd)
 
 TEST_NO_CREATE_REPO=t
 TEST_NO_MALLOC_CHECK=t
 
 . ../test-lib.sh
 
-if test -z "$GIT_TEST_INSTALLED"; then
-	perf_results_prefix=
-else
-	if test -n "$GIT_PERF_DIR_MYDIR_REL"
-	then
-		GIT_TEST_INSTALLED=$GIT_PERF_DIR_MYDIR_REL
-	fi
-	perf_results_prefix=$(printf "%s" "${GIT_TEST_INSTALLED%/bin-wrappers}" | tr -c "[a-zA-Z0-9]" "[_*]")"."
-	GIT_TEST_INSTALLED=$ABSOLUTE_GIT_TEST_INSTALLED
-fi
-
 # Variables from test-lib that are normally internal to the tests; we
 # need to export them for test_perf subshells
 export TEST_DIRECTORY TRASH_DIRECTORY GIT_BUILD_DIR GIT_TEST_CMP
@@ -183,7 +170,7 @@ test_wrapper_ () {
 		base=$(basename "$0" .sh)
 		echo "$test_count" >>"$perf_results_dir"/$base.subtests
 		echo "$1" >"$perf_results_dir"/$base.$test_count.descr
-		base="$perf_results_dir"/"$perf_results_prefix$(basename "$0" .sh)"."$test_count"
+		base="$perf_results_dir"/"$PERF_RESULTS_PREFIX$(basename "$0" .sh)"."$test_count"
 		"$test_wrapper_func_" "$@"
 	fi
 
diff --git a/t/perf/run b/t/perf/run
index 0a7c8744ab..85b7bd31d5 100755
--- a/t/perf/run
+++ b/t/perf/run
@@ -70,6 +70,22 @@ build_git_rev () {
 	) || die "failed to build revision '$mydir'"
 }
 
+set_git_test_installed () {
+	mydir=$1
+
+	mydir_abs=$(cd $mydir && pwd)
+	mydir_abs_wrappers="$mydir_abs_wrappers/bin-wrappers"
+	if test -d "$mydir_abs_wrappers"
+	then
+		GIT_TEST_INSTALLED=$mydir_abs_wrappers
+	else
+		# Older versions of git lacked bin-wrappers;
+		# fallback to the files in the root.
+		GIT_TEST_INSTALLED=$mydir_abs
+	fi
+	export GIT_TEST_INSTALLED
+}
+
 run_dirs_helper () {
 	mydir=${1%/}
 	shift
@@ -79,7 +95,16 @@ run_dirs_helper () {
 	if test $# -gt 0 -a "$1" = --; then
 		shift
 	fi
-	if [ ! -d "$mydir" ]; then
+
+	PERF_RESULTS_PREFIX=
+	if test "$mydir" = "."
+	then
+		unset GIT_TEST_INSTALLED
+	elif test -d "$mydir"
+	then
+		PERF_RESULTS_PREFIX=$(cd $mydir && printf "%s" "$(pwd)" | tr -c "[a-zA-Z0-9]" "_").
+		set_git_test_installed "$mydir"
+	else
 		rev=$(git rev-parse --verify "$mydir" 2>/dev/null) ||
 		die "'$mydir' is neither a directory nor a valid revision"
 		if [ ! -d build/$rev ]; then
@@ -87,20 +112,12 @@ run_dirs_helper () {
 		fi
 		build_git_rev $rev "$mydir"
 		mydir=build/$rev
+
+		PERF_RESULTS_PREFIX=build_$rev.
+		set_git_test_installed "$mydir"
 	fi
-	if test "$mydir" = .; then
-		unset GIT_TEST_INSTALLED
-	else
-		GIT_PERF_DIR_MYDIR_REL=$mydir
-		GIT_PERF_DIR_MYDIR_ABS=$(cd $mydir && pwd)
-		export GIT_PERF_DIR_MYDIR_REL GIT_PERF_DIR_MYDIR_ABS
-
-		GIT_TEST_INSTALLED="$GIT_PERF_DIR_MYDIR_ABS/bin-wrappers"
-		# Older versions of git lacked bin-wrappers; fallback to the
-		# files in the root.
-		test -d "$GIT_TEST_INSTALLED" || GIT_TEST_INSTALLED=$GIT_PERF_DIR_MYDIR_ABS
-		export GIT_TEST_INSTALLED
-	fi
+	export PERF_RESULTS_PREFIX
+
 	run_one_dir "$@"
 }
 
-- 
2.21.0.593.g511ec345e18

Re: What's cooking in git.git (Apr 2019, #05; Thu, 25)

[Junio C Hamano]

Duy Nguyen <pclouds@gmail.com> writes:

> On Thu, Apr 25, 2019 at 8:15 PM Junio C Hamano <gitster@pobox.com> wrote:
>> * nd/config-move-to (2019-01-14) 7 commits
>>  - config.h: fix hdr-check warnings
>>  - config: add --move-to
>>  - config: factor out set_config_source_file()
>>  - config: use OPT_FILENAME()
>>  - config.c: add repo_config_set_worktree_gently()
>>  - worktree.c: add get_worktree_config()
>>  - config.c: avoid git_path() in do_git_config_sequence()
>>
>>  Needs review.
>
> Please drop this for now. I still want to make it work (because I need
> it). But I'll try again with a full series of submodule/worktree
> support. Hopefully that will catch some reviewer's attention.

Will do; thanks.

Re: [PATCH v2 0/2] perf-lib.sh: fix 0baf78e7bc regression, refactor & fix bugs

[Jeff King]

On Tue, May 07, 2019 at 01:23:07AM +0200, Ævar Arnfjörð Bjarmason wrote:

> > What if instead we:
> >
> >   - taught test-lib.sh to make GIT_TEST_INSTALLED absolute (since after
> >     all it is the one who is planning to chdir and wreck the relative
> >     path)
> >
> >   - let callers pass in $GIT_PERF_RESULTS_PREFIX instead of guessing at
> >     it ourselves from the path name. Then the "run" script could quite
> >     reasonably just pass in the tree oid it already has instead of us
> >     trying to decode it. And nobody would care about whether
> >     $GIT_TEST_INSTALLED has been mangled.
> >
> > I thought about going this route for my original patch, but I wanted to
> > fix the regression (which I agree is quite serious and embarrassing) as
> > quickly and simply as possible.
> 
> It seems simplest after the changes I've made here to just make a
> relative GIT_TEST_INSTALLED be an error in test-lib.sh, why bend over
> backwards to support it?

I'm OK with that, though _somebody_ has to generate the full path. My
thinking was that it would be a little nicer if we did at the lowest
level, but it's pretty easy to do it directly in the "run" script.

> Re GIT_PERF_RESULTS_PREFIX: Depending on what you mean we now have
> that in 2/2 as PERF_RESULTS_PREFIX.
> 
> But if you mean the user can pass it in that doesn't make sense, since
> we need to pick a different prefix revision we test, so it's a
> many-to-many relationship.

I couldn't quite parse this. What I meant was that any caller of the
perf scripts would pass in two pieces of data: the path to the git we
want to test (GIT_TEST_INSTALLED) and the unique name under which it
should be saved (GIT_PERF_PREFIX, which would generally be either empty
or the tree oid).

I see that perf-lib.sh also has support for running git out of some
arbitrary installed directory. I'm not sure if anybody actually uses
that, but it's not too hard to support either (the run script just
generates the prefix).

Something like (just for illustration on top of master; this doesn't
have the actual bugfix):

diff --git a/t/perf/perf-lib.sh b/t/perf/perf-lib.sh
index 169f92eae3..e2b342f216 100644
--- a/t/perf/perf-lib.sh
+++ b/t/perf/perf-lib.sh
@@ -29,10 +29,7 @@ TEST_NO_MALLOC_CHECK=t
 
 . ../test-lib.sh
 
-if test -z "$GIT_TEST_INSTALLED"; then
-	perf_results_prefix=
-else
-	perf_results_prefix=$(printf "%s" "${GIT_TEST_INSTALLED%/bin-wrappers}" | tr -c "[a-zA-Z0-9]" "[_*]")"."
+if test -n "$GIT_TEST_INSTALLED"; then
 	GIT_TEST_INSTALLED=$ABSOLUTE_GIT_TEST_INSTALLED
 fi
 
@@ -179,7 +176,8 @@ test_wrapper_ () {
 		base=$(basename "$0" .sh)
 		echo "$test_count" >>"$perf_results_dir"/$base.subtests
 		echo "$1" >"$perf_results_dir"/$base.$test_count.descr
-		base="$perf_results_dir"/"$perf_results_prefix$(basename "$0" .sh)"."$test_count"
+		prefix=$PERF_RESULTS_PREFIX${PERF_RESULTS_PREFIX:+.}
+		base="$perf_results_dir"/"$prefix$(basename "$0" .sh)"."$test_count"
 		"$test_wrapper_func_" "$@"
 	fi
 
diff --git a/t/perf/run b/t/perf/run
index 9aaa733c77..e5695ec8de 100755
--- a/t/perf/run
+++ b/t/perf/run
@@ -87,15 +87,20 @@ run_dirs_helper () {
 		fi
 		build_git_rev $rev "$mydir"
 		mydir=build/$rev
+		PERF_RESULTS_PREFIX=$rev
+	else
+		PERF_RESULTS_PREFIX=$(echo mydir | tr -c "[a-zA-Z0-9]" "[_*]")"."
 	fi
 	if test "$mydir" = .; then
 		unset GIT_TEST_INSTALLED
+		unset PERF_RESULTS_PREFIX
 	else
 		GIT_TEST_INSTALLED="$mydir/bin-wrappers"
 		# Older versions of git lacked bin-wrappers; fallback to the
 		# files in the root.
 		test -d "$GIT_TEST_INSTALLED" || GIT_TEST_INSTALLED=$mydir
 		export GIT_TEST_INSTALLED
+		export PERF_RESULTS_PREFIX
 	fi
 	run_one_dir "$@"
 }

It does mean that anybody calling "GIT_TEST_INSTALLED=whatever
./p1234-foo.sh" has to pass in PERF_RESULTS_PREFIX, but I think that's
OK (they should be using the "run" script anyway).

> As seen in 2/2 modifying some of the shell & associated Perl code it's
> a bit nasty that we need to duplicate the logic for picking these
> PERF_RESULTS_PREFIXes in the shell code and Perl code. Ideally the
> shell code would pick it, and pass the mapping to the Perl code
> somehow. But that's another "has sucked since forever, future TODO"
> item.

Yeah, I didn't realize that aggregate.perl has the duplicate logic.
Gross. But I agree that's not a topic for now. It would continue to
compute the same PERF_RESULTS_PREFIX that "run" does, except it doesn't
need to bother to pass it to anybody else.

(Note that in my patch above, I dropped the pointless "build_" from the
rev-based names, so aggregate.perl would need to do the same).

-Peff

Re: [PATCH v2 2/2] perf-lib.sh: remove GIT_TEST_INSTALLED from perf-lib.sh

[Jeff King]

On Tue, May 07, 2019 at 01:23:09AM +0200, Ævar Arnfjörð Bjarmason wrote:

> @@ -79,7 +95,16 @@ run_dirs_helper () {
>  	if test $# -gt 0 -a "$1" = --; then
>  		shift
>  	fi
> -	if [ ! -d "$mydir" ]; then
> +
> +	PERF_RESULTS_PREFIX=
> +	if test "$mydir" = "."
> +	then
> +		unset GIT_TEST_INSTALLED
> +	elif test -d "$mydir"
> +	then
> +		PERF_RESULTS_PREFIX=$(cd $mydir && printf "%s" "$(pwd)" | tr -c "[a-zA-Z0-9]" "_").
> +		set_git_test_installed "$mydir"
> +	else
>  		rev=$(git rev-parse --verify "$mydir" 2>/dev/null) ||
>  		die "'$mydir' is neither a directory nor a valid revision"
>  		if [ ! -d build/$rev ]; then

OK, so this is basically the same cleanup I came up with. The big
difference is that you pushed the shared code into a function, rather
than the funky double-conditional in the original. I'm OK with that.

> diff --git a/t/perf/aggregate.perl b/t/perf/aggregate.perl
> index 494907a892..c8f4a78903 100755
> --- a/t/perf/aggregate.perl
> +++ b/t/perf/aggregate.perl
> @@ -6,6 +6,7 @@
>  use JSON;
>  use Getopt::Long;
>  use Git;
> +use Cwd qw(realpath);
>  
>  sub get_times {
>  	my $name = shift;
> @@ -103,13 +104,14 @@ sub format_size {
>  	if (! -d $arg) {
>  		my $rev = Git::command_oneline(qw(rev-parse --verify), $arg);
>  		$dir = "build/".$rev;
> +	} elsif ($arg eq '.') {
> +		$dir = '.';
>  	} else {
> -		$arg =~ s{/*$}{};
> -		$dir = $arg;
> -		$dirabbrevs{$dir} = $dir;
> +		$dir = realpath($arg);
> +		$dirnames{$dir} = $dir;
>  	}
>  	push @dirs, $dir;
> -	$dirnames{$dir} = $arg;
> +	$dirnames{$dir} ||= $arg;

I'm not sure I get what's going on here. Why do we need the realpath in
aggregate.perl? We'd want to generate the same filename that "run"
decided to store things in, which we'd generate from the command-line
arguments (either passed on to us by "run", or direct from the user if
they're printing a previous run).

> @@ -312,9 +314,6 @@ sub print_codespeed_results {
>  		$environment = $reponame;
>  	} elsif (exists $ENV{GIT_PERF_REPO_NAME} and $ENV{GIT_PERF_REPO_NAME} ne "") {
>  		$environment = $ENV{GIT_PERF_REPO_NAME};
> -	} elsif (exists $ENV{GIT_TEST_INSTALLED} and $ENV{GIT_TEST_INSTALLED} ne "") {
> -		$environment = $ENV{GIT_TEST_INSTALLED};
> -		$environment =~ s|/bin-wrappers$||;
>  	} else {
>  		$environment = `uname -r`;
>  		chomp $environment;

Is this codespeed thing a totally separate bug? Should it go into its
own patch?

-Peff

Re: [PATCH v2 1/2] perf-lib.sh: make "./run <revisions>" use the correct gits

[Jeff King]

On Tue, May 07, 2019 at 01:23:08AM +0200, Ævar Arnfjörð Bjarmason wrote:

> diff --git a/t/perf/perf-lib.sh b/t/perf/perf-lib.sh
> index 169f92eae3..b15ee1d262 100644
> --- a/t/perf/perf-lib.sh
> +++ b/t/perf/perf-lib.sh
> @@ -32,6 +32,10 @@ TEST_NO_MALLOC_CHECK=t
>  if test -z "$GIT_TEST_INSTALLED"; then
>  	perf_results_prefix=
>  else
> +	if test -n "$GIT_PERF_DIR_MYDIR_REL"
> +	then
> +		GIT_TEST_INSTALLED=$GIT_PERF_DIR_MYDIR_REL
> +	fi
>  	perf_results_prefix=$(printf "%s" "${GIT_TEST_INSTALLED%/bin-wrappers}" | tr -c "[a-zA-Z0-9]" "[_*]")"."
>  	GIT_TEST_INSTALLED=$ABSOLUTE_GIT_TEST_INSTALLED
>  fi

I still like my fix better, as it works completely within perf-lib.sh,
and so is a more direct fix.  And doesn't have this weird exported
MYDIR_ABS that nobody actually uses.

But I don't actually think this topic is worth spending too many brain
cycles on. So I am fine with either fix, as long as we do something.

-Peff

Re: [PATCH v2 2/2] perf-lib.sh: remove GIT_TEST_INSTALLED from perf-lib.sh

[Ævar Arnfjörð Bjarmason]

On Tue, May 07 2019, Jeff King wrote:

> On Tue, May 07, 2019 at 01:23:09AM +0200, Ævar Arnfjörð Bjarmason wrote:
>
>> @@ -79,7 +95,16 @@ run_dirs_helper () {
>>  	if test $# -gt 0 -a "$1" = --; then
>>  		shift
>>  	fi
>> -	if [ ! -d "$mydir" ]; then
>> +
>> +	PERF_RESULTS_PREFIX=
>> +	if test "$mydir" = "."
>> +	then
>> +		unset GIT_TEST_INSTALLED
>> +	elif test -d "$mydir"
>> +	then
>> +		PERF_RESULTS_PREFIX=$(cd $mydir && printf "%s" "$(pwd)" | tr -c "[a-zA-Z0-9]" "_").
>> +		set_git_test_installed "$mydir"
>> +	else
>>  		rev=$(git rev-parse --verify "$mydir" 2>/dev/null) ||
>>  		die "'$mydir' is neither a directory nor a valid revision"
>>  		if [ ! -d build/$rev ]; then
>
> OK, so this is basically the same cleanup I came up with. The big
> difference is that you pushed the shared code into a function, rather
> than the funky double-conditional in the original. I'm OK with that.

Yup. It can be done either way, but this way allows for a many/many
mapping where we set the GIT_TEST_INSTALLED & PERF_RESULTS_PREFIX
depending on the different cases, as opposed to in perf-lib.sh where
we'd need to set GIT_TEST_INSTALLED and then infer a PERF_RESULTS_PREFIX
(or pass flags marking each case, which would amount to the same
thing...).

>> diff --git a/t/perf/aggregate.perl b/t/perf/aggregate.perl
>> index 494907a892..c8f4a78903 100755
>> --- a/t/perf/aggregate.perl
>> +++ b/t/perf/aggregate.perl
>> @@ -6,6 +6,7 @@
>>  use JSON;
>>  use Getopt::Long;
>>  use Git;
>> +use Cwd qw(realpath);
>>
>>  sub get_times {
>>  	my $name = shift;
>> @@ -103,13 +104,14 @@ sub format_size {
>>  	if (! -d $arg) {
>>  		my $rev = Git::command_oneline(qw(rev-parse --verify), $arg);
>>  		$dir = "build/".$rev;
>> +	} elsif ($arg eq '.') {
>> +		$dir = '.';
>>  	} else {
>> -		$arg =~ s{/*$}{};
>> -		$dir = $arg;
>> -		$dirabbrevs{$dir} = $dir;
>> +		$dir = realpath($arg);
>> +		$dirnames{$dir} = $dir;
>>  	}
>>  	push @dirs, $dir;
>> -	$dirnames{$dir} = $arg;
>> +	$dirnames{$dir} ||= $arg;
>
> I'm not sure I get what's going on here. Why do we need the realpath in
> aggregate.perl? We'd want to generate the same filename that "run"
> decided to store things in, which we'd generate from the command-line
> arguments (either passed on to us by "run", or direct from the user if
> they're printing a previous run).

So this is part of the "has sucked since forever, future TODO" mentioned
in 0/2.

I.e. if you pass "../.." as a path to "run" we'll try to discover a
built/installed "git" in a "bindir" there, and then we need to do two
things:

 1. Figure out a way to turn that into a filename sensible for the
    *.times files.
 2. Print some header showing that path in the aggregate output.

The "run" script will discover #1 for itself, that's what that "pwd &&
tr -c ..." command is doing, but then we just pass "../.." again to
aggregate.perl and have it figure it out again on its own, so it needs
to duplicate the logic.

Just having both discover the absolute path all the time for #1 made
things a lot simpler, e.g. if you do ../.. on v2.21.0 you'll get things
like:

    _____.p0000-perf-lib-sanity.1.times

And with $PWD/../../ you'd get:

    _home_avar_g_git_t_perf______.p0000-perf-lib-sanity.1.times

Now this is all pretty & consistent. Any path to a "git" will always be
turned into the absolute path, e.g.:

    _home_avar_g_git.p0000-perf-lib-sanity.1.times

And instead of "git" or ".." being printed in the aggregate header we
print the path, e.g. "/home/avar/g/git".

>> @@ -312,9 +314,6 @@ sub print_codespeed_results {
>>  		$environment = $reponame;
>>  	} elsif (exists $ENV{GIT_PERF_REPO_NAME} and $ENV{GIT_PERF_REPO_NAME} ne "") {
>>  		$environment = $ENV{GIT_PERF_REPO_NAME};
>> -	} elsif (exists $ENV{GIT_TEST_INSTALLED} and $ENV{GIT_TEST_INSTALLED} ne "") {
>> -		$environment = $ENV{GIT_TEST_INSTALLED};
>> -		$environment =~ s|/bin-wrappers$||;
>>  	} else {
>>  		$environment = `uname -r`;
>>  		chomp $environment;
>
> Is this codespeed thing a totally separate bug? Should it go into its
> own patch?

I could split it up, but figured any change like this would have had to
deal with and refactor with any existing uses of GIT_TEST_INSTALLED,
that there was a bug in some of that code and it could be removed was
just luck...

[PATCH v3 0/6] perf-lib.sh: fix 0baf78e7bc regression, refactor & fix bugs

[Ævar Arnfjörð Bjarmason]

So here as a 6-parter given some of the feedback on v2. Maybe Jeff
still hates it :), but this time around some of the changes are split
up and should be easier to understand in isolation, as well as some
more "noticed while I was at it" things fixed.

This series ends with outright forbidding the user from directly
setting GIT_TEST_INSTALLED. As discussed it makes things easier for
us, and as noted in 1/6 the demand for that in the wild seems
non-existent, since the way we've been documenting how you could do
that with an environment variable has been broken since 2012.

Ævar Arnfjörð Bjarmason (6):
  perf README: correct docs for 3c8f12c96c regression
  perf aggregate: remove GIT_TEST_INSTALLED from --codespeed
  perf-lib.sh: make "./run <revisions>" use the correct gits
  perf-lib.sh: remove GIT_TEST_INSTALLED from perf-lib.sh
  perf tests: add "bindir" prefix to git tree test results
  perf-lib.sh: forbid the use of GIT_TEST_INSTALLED

 t/perf/README         |  2 +-
 t/perf/aggregate.perl | 17 +++++++++--------
 t/perf/perf-lib.sh    | 18 ++++++++++--------
 t/perf/run            | 43 +++++++++++++++++++++++++++++++++----------
 4 files changed, 53 insertions(+), 27 deletions(-)

Range-diff:
-:  ---------- > 1:  6684dca042 perf README: correct docs for 3c8f12c96c regression
-:  ---------- > 2:  c4e903d898 perf aggregate: remove GIT_TEST_INSTALLED from --codespeed
1:  22a132ed64 ! 3:  9d2d162c64 perf-lib.sh: make "./run <revisions>" use the correct gits
    @@ -40,9 +40,13 @@
         this will subtly fail in test-lib.sh. This has always been the case
         even before 0baf78e7bc, and as documented in t/README the
         GIT_TEST_INSTALLED variable should be set to an absolute path (needs
    -    to be set "to the bindir", which is always absolute). Perhaps that
    -    should be dealt with in the future, but I'm leaving that alone for
    -    now.
    +    to be set "to the bindir", which is always absolute), and the "perf"
    +    framework expects to munge it itself.
    +
    +    Perhaps that should be dealt with in the future to allow manually
    +    setting GIT_TEST_INSTALLED, but as a preceding commit showed the user
    +    can just use the "run" script, which'll also pick the right output
    +    directory for the test results as expected by aggregate.perl.
     
         1. https://public-inbox.org/git/20190502222409.GA15631@sigill.intra.peff.net/
     
2:  f43beb6450 ! 4:  58f1dd3f6f perf-lib.sh: remove GIT_TEST_INSTALLED from perf-lib.sh
    @@ -26,28 +26,28 @@
         We'll just do the right thing because PERF_RESULTS_PREFIX will be
         empty, and test-lib.sh takes care of finding where our git is.
     
    -    Refactoring this revealed a few bugs, e.g. while a relative git path
    -    was supported via e.g.:
    +    Any refactoring of this code needs to change both the shell code and
    +    the Perl code in aggregate.perl, because when running e.g.:
     
             ./run ../../ -- <test>
     
    -    We'd just print out ".." as the header, since we'd always take the
    -    content after the last slash. Now we'll always resolve the absolute
    -    path to something we detect to be be a manually supplied bindir, and
    -    print the full path in the aggregation.
    +    The "../../" path to a relative bindir needs to be munged to a
    +    filename containing the results, and critically aggregate.perl does
    +    not get passed the path to those aggregations, just "../..".
     
    -    There was also a long-standing bug in the codespeed output where the
    -    "environment" for N number of tests would be whatever our
    -    GIT_TEST_INSTALLED had been set to by the last of those N runs. Let's
    -    instead just fall back to "uname -r", which is a more sensible
    -    "environment" than some random build directory path, even for the N=1
    -    case.
    +    Let's fix cases where aggregate.perl would print e.g. ".." in its
    +    report output for this, and "git" for "/home/avar/g/git", i.e. it
    +    would always pick the last element. Now'll always print the full path
    +    instead.
    +
    +    This also makes the code sturdier, e.g. you can feed "../.."  to
    +    "./run" and then an absolute path to the aggregate.perl script, as
    +    long as the absolute path and "../.." resolved to the same directory
    +    printing the aggregation will work.
     
         Also simplify the "[_*]" on the RHS of "tr -c", we're trimming
         everything to "_", so we don't need that.
     
    -    https://public-inbox.org/git/20190502222409.GA15631@sigill.intra.peff.net/
    -
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
      diff --git a/t/perf/aggregate.perl b/t/perf/aggregate.perl
    @@ -80,16 +80,6 @@
      	my $prefix = $dir;
      	$prefix =~ tr/^a-zA-Z0-9/_/c;
      	$prefixes{$dir} = $prefix . '.';
    -@@
    - 		$environment = $reponame;
    - 	} elsif (exists $ENV{GIT_PERF_REPO_NAME} and $ENV{GIT_PERF_REPO_NAME} ne "") {
    - 		$environment = $ENV{GIT_PERF_REPO_NAME};
    --	} elsif (exists $ENV{GIT_TEST_INSTALLED} and $ENV{GIT_TEST_INSTALLED} ne "") {
    --		$environment = $ENV{GIT_TEST_INSTALLED};
    --		$environment =~ s|/bin-wrappers$||;
    - 	} else {
    - 		$environment = `uname -r`;
    - 		chomp $environment;
     
      diff --git a/t/perf/perf-lib.sh b/t/perf/perf-lib.sh
      --- a/t/perf/perf-lib.sh
-:  ---------- > 5:  64df9157a4 perf tests: add "bindir" prefix to git tree test results
-:  ---------- > 6:  21307f1f2d perf-lib.sh: forbid the use of GIT_TEST_INSTALLED
-- 
2.21.0.593.g511ec345e18

[PATCH v3 1/6] perf README: correct docs for 3c8f12c96c regression

[Ævar Arnfjörð Bjarmason]

Since 3c8f12c96c ("test-lib: reorder and include GIT-BUILD-OPTIONS a
lot earlier", 2012-06-24) the suggested advice of overriding
GIT_BUILD_DIR has not worked. We've printed a hard error like this
given e.g. GIT_BUILD_DIR=/home/avar/g/git:

    /bin-wrappers/git is not executable; using GIT_EXEC_PATH
    error: You haven't built things yet, have you?

Let's just suggest that the user run other gits via the "run"
script. That'll do the right thing for setting the path to the other
git, and running the "aggregate.perl" scripts afterwards will work.

As an aside, if setting GIT_BUILD_DIR had still worked, then the
MODERN_GIT feature/fix added in 1a0962dee5 ("t/perf: fix regression in
testing older versions of git", 2016-06-22) would have broke.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/perf/README | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/perf/README b/t/perf/README
index be12090c38..c7b70e2d28 100644
--- a/t/perf/README
+++ b/t/perf/README
@@ -45,7 +45,7 @@ call the aggregation script to summarize the results:
 
     $ ./p0001-rev-list.sh
     [...]
-    $ GIT_BUILD_DIR=/path/to/other/git ./p0001-rev-list.sh
+    $ ./run /path/to/other/git -- ./p0001-rev-list.sh
     [...]
     $ ./aggregate.perl . /path/to/other/git ./p0001-rev-list.sh
 
-- 
2.21.0.593.g511ec345e18

[PATCH v3 2/6] perf aggregate: remove GIT_TEST_INSTALLED from --codespeed

[Ævar Arnfjörð Bjarmason]

Remove the setting of the "environment" from the --codespeed output. I
don't think this is useful, and it helps with a later refactoring
where we GIT_TEST_INSTALLED stop munging/reading GIT_TEST_INSTALLED in
the perf tests in so many places.

This was added in 05eb1c37ed ("perf/aggregate: implement codespeed
JSON output", 2018-01-05), but since the "run" scripts uses
"GIT_TEST_INSTALLED" internally this was only ever useful for one-off
runs of a single revision as all the "environment" values would be
ones for whatever directory the "run" script ran last.

Let's instead fall back on the "uname -r" case, which is the sort of
thing the environment should be set to, not something that duplicates
other parts of the codpseed output. For setting the "environment" to
something custom the perf.repoName variable can be used. See
19cf57a92e ("perf/run: read GIT_PERF_REPO_NAME from perf.repoName",
2018-01-05).

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/perf/aggregate.perl | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/t/perf/aggregate.perl b/t/perf/aggregate.perl
index 494907a892..f6518339dc 100755
--- a/t/perf/aggregate.perl
+++ b/t/perf/aggregate.perl
@@ -312,9 +312,6 @@ sub print_codespeed_results {
 		$environment = $reponame;
 	} elsif (exists $ENV{GIT_PERF_REPO_NAME} and $ENV{GIT_PERF_REPO_NAME} ne "") {
 		$environment = $ENV{GIT_PERF_REPO_NAME};
-	} elsif (exists $ENV{GIT_TEST_INSTALLED} and $ENV{GIT_TEST_INSTALLED} ne "") {
-		$environment = $ENV{GIT_TEST_INSTALLED};
-		$environment =~ s|/bin-wrappers$||;
 	} else {
 		$environment = `uname -r`;
 		chomp $environment;
-- 
2.21.0.593.g511ec345e18

[PATCH v3 3/6] perf-lib.sh: make "./run <revisions>" use the correct gits

[Ævar Arnfjörð Bjarmason]

Fix a really bad regression in 0baf78e7bc ("perf-lib.sh: rely on
test-lib.sh for --tee handling", 2019-03-15). Since that change all
runs of different <revisions> of git have used the git found in the
user's $PATH, e.g. /usr/bin/git instead of the <revision> we just
built and wanted to performance test.

The problem starts with GIT_TEST_INSTALLED not working like our
non-perf tests with the "run" script. I.e. you can't run performance
tests against a given installed git. Instead we expect to use it
ourselves to point GIT_TEST_INSTALLED to the <revision> we just built.

However, we had been relying on '$(cd "$GIT_TEST_INSTALLED" && pwd)'
to resolve that relative $GIT_TEST_INSTALLED to an absolute
path *before* test-lib.sh was loaded, in cases where it was
e.g. "build/<rev>/bin-wrappers" and we wanted "<abs_path>build/...".

This change post-dates another proposed solution by a few days[1], I
didn't notice that version when I initially wrote this. I'm doing the
most minimal thing to solve the regression here, a follow-up change
will move this result prefix selection logic entirely into the "run"
script.

This makes e.g. these cases all work:

    ./run . $PWD/../../ origin/master origin/next HEAD -- <tests>

As well as just a plain one-off:

    ./run <tests>

And, since we're passing down the new GIT_PERF_DIR_MYDIR_REL we make
sure the bug relating to aggregate.perl not finding our files as
described in 0baf78e7bc doesn't happen again.

What *doesn't* work is setting GIT_TEST_INSTALLED to a relative path,
this will subtly fail in test-lib.sh. This has always been the case
even before 0baf78e7bc, and as documented in t/README the
GIT_TEST_INSTALLED variable should be set to an absolute path (needs
to be set "to the bindir", which is always absolute), and the "perf"
framework expects to munge it itself.

Perhaps that should be dealt with in the future to allow manually
setting GIT_TEST_INSTALLED, but as a preceding commit showed the user
can just use the "run" script, which'll also pick the right output
directory for the test results as expected by aggregate.perl.

1. https://public-inbox.org/git/20190502222409.GA15631@sigill.intra.peff.net/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/perf/perf-lib.sh | 4 ++++
 t/perf/run         | 8 ++++++--
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/t/perf/perf-lib.sh b/t/perf/perf-lib.sh
index 169f92eae3..b15ee1d262 100644
--- a/t/perf/perf-lib.sh
+++ b/t/perf/perf-lib.sh
@@ -32,6 +32,10 @@ TEST_NO_MALLOC_CHECK=t
 if test -z "$GIT_TEST_INSTALLED"; then
 	perf_results_prefix=
 else
+	if test -n "$GIT_PERF_DIR_MYDIR_REL"
+	then
+		GIT_TEST_INSTALLED=$GIT_PERF_DIR_MYDIR_REL
+	fi
 	perf_results_prefix=$(printf "%s" "${GIT_TEST_INSTALLED%/bin-wrappers}" | tr -c "[a-zA-Z0-9]" "[_*]")"."
 	GIT_TEST_INSTALLED=$ABSOLUTE_GIT_TEST_INSTALLED
 fi
diff --git a/t/perf/run b/t/perf/run
index 9aaa733c77..0a7c8744ab 100755
--- a/t/perf/run
+++ b/t/perf/run
@@ -91,10 +91,14 @@ run_dirs_helper () {
 	if test "$mydir" = .; then
 		unset GIT_TEST_INSTALLED
 	else
-		GIT_TEST_INSTALLED="$mydir/bin-wrappers"
+		GIT_PERF_DIR_MYDIR_REL=$mydir
+		GIT_PERF_DIR_MYDIR_ABS=$(cd $mydir && pwd)
+		export GIT_PERF_DIR_MYDIR_REL GIT_PERF_DIR_MYDIR_ABS
+
+		GIT_TEST_INSTALLED="$GIT_PERF_DIR_MYDIR_ABS/bin-wrappers"
 		# Older versions of git lacked bin-wrappers; fallback to the
 		# files in the root.
-		test -d "$GIT_TEST_INSTALLED" || GIT_TEST_INSTALLED=$mydir
+		test -d "$GIT_TEST_INSTALLED" || GIT_TEST_INSTALLED=$GIT_PERF_DIR_MYDIR_ABS
 		export GIT_TEST_INSTALLED
 	fi
 	run_one_dir "$@"
-- 
2.21.0.593.g511ec345e18

[PATCH v3 4/6] perf-lib.sh: remove GIT_TEST_INSTALLED from perf-lib.sh

[Ævar Arnfjörð Bjarmason]

Follow-up my preceding change which fixed the immediate "./run
<revisions>" regression in 0baf78e7bc ("perf-lib.sh: rely on
test-lib.sh for --tee handling", 2019-03-15) and entirely get rid of
GIT_TEST_INSTALLED from perf-lib.sh (and aggregate.perl).

As noted in that change the dance we're doing with GIT_TEST_INSTALLED
perf-lib.sh isn't necessary, but there I was doing the most minimal
set of changes to quickly fix a regression.

But it's much simpler to never deal with the "GIT_TEST_INSTALLED" we
were setting in perf-lib.sh at all. Instead the run_dirs_helper() sets
the previously inferred $PERF_RESULTS_PREFIX directly.

Setting this at the callsite that's already best positioned to
exhaustively know about all the different cases we need to handle
where PERF_RESULTS_PREFIX isn't what we want already (the empty
string) makes the most sense. In one-off cases like:

    ./run ./p0000-perf-lib-sanity.sh
    ./p0000-perf-lib-sanity.sh

We'll just do the right thing because PERF_RESULTS_PREFIX will be
empty, and test-lib.sh takes care of finding where our git is.

Any refactoring of this code needs to change both the shell code and
the Perl code in aggregate.perl, because when running e.g.:

    ./run ../../ -- <test>

The "../../" path to a relative bindir needs to be munged to a
filename containing the results, and critically aggregate.perl does
not get passed the path to those aggregations, just "../..".

Let's fix cases where aggregate.perl would print e.g. ".." in its
report output for this, and "git" for "/home/avar/g/git", i.e. it
would always pick the last element. Now'll always print the full path
instead.

This also makes the code sturdier, e.g. you can feed "../.."  to
"./run" and then an absolute path to the aggregate.perl script, as
long as the absolute path and "../.." resolved to the same directory
printing the aggregation will work.

Also simplify the "[_*]" on the RHS of "tr -c", we're trimming
everything to "_", so we don't need that.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/perf/aggregate.perl | 10 ++++++----
 t/perf/perf-lib.sh    | 15 +--------------
 t/perf/run            | 45 +++++++++++++++++++++++++++++--------------
 3 files changed, 38 insertions(+), 32 deletions(-)

diff --git a/t/perf/aggregate.perl b/t/perf/aggregate.perl
index f6518339dc..c8f4a78903 100755
--- a/t/perf/aggregate.perl
+++ b/t/perf/aggregate.perl
@@ -6,6 +6,7 @@
 use JSON;
 use Getopt::Long;
 use Git;
+use Cwd qw(realpath);
 
 sub get_times {
 	my $name = shift;
@@ -103,13 +104,14 @@ sub format_size {
 	if (! -d $arg) {
 		my $rev = Git::command_oneline(qw(rev-parse --verify), $arg);
 		$dir = "build/".$rev;
+	} elsif ($arg eq '.') {
+		$dir = '.';
 	} else {
-		$arg =~ s{/*$}{};
-		$dir = $arg;
-		$dirabbrevs{$dir} = $dir;
+		$dir = realpath($arg);
+		$dirnames{$dir} = $dir;
 	}
 	push @dirs, $dir;
-	$dirnames{$dir} = $arg;
+	$dirnames{$dir} ||= $arg;
 	my $prefix = $dir;
 	$prefix =~ tr/^a-zA-Z0-9/_/c;
 	$prefixes{$dir} = $prefix . '.';
diff --git a/t/perf/perf-lib.sh b/t/perf/perf-lib.sh
index b15ee1d262..9cdccba222 100644
--- a/t/perf/perf-lib.sh
+++ b/t/perf/perf-lib.sh
@@ -21,25 +21,12 @@
 # because it will change our working directory.
 TEST_DIRECTORY=$(pwd)/..
 TEST_OUTPUT_DIRECTORY=$(pwd)
-ABSOLUTE_GIT_TEST_INSTALLED=$(
-	test -n "$GIT_TEST_INSTALLED" && cd "$GIT_TEST_INSTALLED" && pwd)
 
 TEST_NO_CREATE_REPO=t
 TEST_NO_MALLOC_CHECK=t
 
 . ../test-lib.sh
 
-if test -z "$GIT_TEST_INSTALLED"; then
-	perf_results_prefix=
-else
-	if test -n "$GIT_PERF_DIR_MYDIR_REL"
-	then
-		GIT_TEST_INSTALLED=$GIT_PERF_DIR_MYDIR_REL
-	fi
-	perf_results_prefix=$(printf "%s" "${GIT_TEST_INSTALLED%/bin-wrappers}" | tr -c "[a-zA-Z0-9]" "[_*]")"."
-	GIT_TEST_INSTALLED=$ABSOLUTE_GIT_TEST_INSTALLED
-fi
-
 # Variables from test-lib that are normally internal to the tests; we
 # need to export them for test_perf subshells
 export TEST_DIRECTORY TRASH_DIRECTORY GIT_BUILD_DIR GIT_TEST_CMP
@@ -183,7 +170,7 @@ test_wrapper_ () {
 		base=$(basename "$0" .sh)
 		echo "$test_count" >>"$perf_results_dir"/$base.subtests
 		echo "$1" >"$perf_results_dir"/$base.$test_count.descr
-		base="$perf_results_dir"/"$perf_results_prefix$(basename "$0" .sh)"."$test_count"
+		base="$perf_results_dir"/"$PERF_RESULTS_PREFIX$(basename "$0" .sh)"."$test_count"
 		"$test_wrapper_func_" "$@"
 	fi
 
diff --git a/t/perf/run b/t/perf/run
index 0a7c8744ab..85b7bd31d5 100755
--- a/t/perf/run
+++ b/t/perf/run
@@ -70,6 +70,22 @@ build_git_rev () {
 	) || die "failed to build revision '$mydir'"
 }
 
+set_git_test_installed () {
+	mydir=$1
+
+	mydir_abs=$(cd $mydir && pwd)
+	mydir_abs_wrappers="$mydir_abs_wrappers/bin-wrappers"
+	if test -d "$mydir_abs_wrappers"
+	then
+		GIT_TEST_INSTALLED=$mydir_abs_wrappers
+	else
+		# Older versions of git lacked bin-wrappers;
+		# fallback to the files in the root.
+		GIT_TEST_INSTALLED=$mydir_abs
+	fi
+	export GIT_TEST_INSTALLED
+}
+
 run_dirs_helper () {
 	mydir=${1%/}
 	shift
@@ -79,7 +95,16 @@ run_dirs_helper () {
 	if test $# -gt 0 -a "$1" = --; then
 		shift
 	fi
-	if [ ! -d "$mydir" ]; then
+
+	PERF_RESULTS_PREFIX=
+	if test "$mydir" = "."
+	then
+		unset GIT_TEST_INSTALLED
+	elif test -d "$mydir"
+	then
+		PERF_RESULTS_PREFIX=$(cd $mydir && printf "%s" "$(pwd)" | tr -c "[a-zA-Z0-9]" "_").
+		set_git_test_installed "$mydir"
+	else
 		rev=$(git rev-parse --verify "$mydir" 2>/dev/null) ||
 		die "'$mydir' is neither a directory nor a valid revision"
 		if [ ! -d build/$rev ]; then
@@ -87,20 +112,12 @@ run_dirs_helper () {
 		fi
 		build_git_rev $rev "$mydir"
 		mydir=build/$rev
+
+		PERF_RESULTS_PREFIX=build_$rev.
+		set_git_test_installed "$mydir"
 	fi
-	if test "$mydir" = .; then
-		unset GIT_TEST_INSTALLED
-	else
-		GIT_PERF_DIR_MYDIR_REL=$mydir
-		GIT_PERF_DIR_MYDIR_ABS=$(cd $mydir && pwd)
-		export GIT_PERF_DIR_MYDIR_REL GIT_PERF_DIR_MYDIR_ABS
-
-		GIT_TEST_INSTALLED="$GIT_PERF_DIR_MYDIR_ABS/bin-wrappers"
-		# Older versions of git lacked bin-wrappers; fallback to the
-		# files in the root.
-		test -d "$GIT_TEST_INSTALLED" || GIT_TEST_INSTALLED=$GIT_PERF_DIR_MYDIR_ABS
-		export GIT_TEST_INSTALLED
-	fi
+	export PERF_RESULTS_PREFIX
+
 	run_one_dir "$@"
 }
 
-- 
2.21.0.593.g511ec345e18

[PATCH v3 5/6] perf tests: add "bindir" prefix to git tree test results

[Ævar Arnfjörð Bjarmason]

Change the output file names in test-results/ to be
"test-results/bindir_<munged dir>" rather than just
"test-results/<munged dir>".

This is for consistency with the "build_" directories we have for
built revisions, i.e. "test-results/build_<SHA-1>".

There's no user-visible functional changes here, it just makes it
easier to see at a glance what "test-results" files are of what "type"
as they're all explicitly grouped together now, and to grep this code
to find both the run_dirs_helper() implementation and its
corresponding aggregate.perl code.

Note that we already guarantee that the rest of the
PERF_RESULTS_PREFIX is an absolute path, and since it'll start with
e.g. "/" which we munge to "_" we'll up with a readable string like
"bindir_home_avar[...]".

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/perf/aggregate.perl | 4 +++-
 t/perf/run            | 2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/t/perf/aggregate.perl b/t/perf/aggregate.perl
index c8f4a78903..b951747e08 100755
--- a/t/perf/aggregate.perl
+++ b/t/perf/aggregate.perl
@@ -100,6 +100,7 @@ sub format_size {
 while (scalar @ARGV) {
 	my $arg = $ARGV[0];
 	my $dir;
+	my $prefix = '';
 	last if -f $arg or $arg eq "--";
 	if (! -d $arg) {
 		my $rev = Git::command_oneline(qw(rev-parse --verify), $arg);
@@ -109,10 +110,11 @@ sub format_size {
 	} else {
 		$dir = realpath($arg);
 		$dirnames{$dir} = $dir;
+		$prefix .= 'bindir';
 	}
 	push @dirs, $dir;
 	$dirnames{$dir} ||= $arg;
-	my $prefix = $dir;
+	$prefix .= $dir;
 	$prefix =~ tr/^a-zA-Z0-9/_/c;
 	$prefixes{$dir} = $prefix . '.';
 	shift @ARGV;
diff --git a/t/perf/run b/t/perf/run
index 85b7bd31d5..cd3882b117 100755
--- a/t/perf/run
+++ b/t/perf/run
@@ -102,7 +102,7 @@ run_dirs_helper () {
 		unset GIT_TEST_INSTALLED
 	elif test -d "$mydir"
 	then
-		PERF_RESULTS_PREFIX=$(cd $mydir && printf "%s" "$(pwd)" | tr -c "[a-zA-Z0-9]" "_").
+		PERF_RESULTS_PREFIX=bindir$(cd $mydir && printf "%s" "$(pwd)" | tr -c "[a-zA-Z0-9]" "_").
 		set_git_test_installed "$mydir"
 	else
 		rev=$(git rev-parse --verify "$mydir" 2>/dev/null) ||
-- 
2.21.0.593.g511ec345e18

[PATCH v3 6/6] perf-lib.sh: forbid the use of GIT_TEST_INSTALLED

[Ævar Arnfjörð Bjarmason]

As noted in preceding commits setting GIT_TEST_INSTALLED has never
been supported or documented, and as noted in an earlier t/perf/README
change to the extent that it's been documented nobody's notices that
the example hasn't worked since 3c8f12c96c ("test-lib: reorder and
include GIT-BUILD-OPTIONS a lot earlier", 2012-06-24).

We could directly support GIT_TEST_INSTALLED for invocations without
the "run" script, such as:

    GIT_TEST_INSTALLED=../../ ./p0000-perf-lib-sanity.sh
    GIT_TEST_INSTALLED=/home/avar/g/git ./p0000-perf-lib-sanity.sh

But while not having this "error" will "work", it won't write the the
resulting "test-results/*" files to the right place, and thus a
subsequent call to aggregate.perl won't work as expected.

Let's just tell the user that they need to use the "run" script,
which'll correctly deal with this and set the right
PERF_RESULTS_PREFIX.

If someone's in desperate need of bypassing "run" for whatever reason
they can trivially do so by setting "PERF_SET_GIT_TEST_INSTALLED", but
not we won't have people who expect GIT_TEST_INSTALLED to just work
wondering why their aggregation doesn't work, even though they're
running the right "git".

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/perf/perf-lib.sh | 11 +++++++++++
 t/perf/run         |  2 ++
 2 files changed, 13 insertions(+)

diff --git a/t/perf/perf-lib.sh b/t/perf/perf-lib.sh
index 9cdccba222..b58a43ea43 100644
--- a/t/perf/perf-lib.sh
+++ b/t/perf/perf-lib.sh
@@ -27,6 +27,17 @@ TEST_NO_MALLOC_CHECK=t
 
 . ../test-lib.sh
 
+if test -n "$GIT_TEST_INSTALLED" -a -z "$PERF_SET_GIT_TEST_INSTALLED"
+then
+	error "Do not use GIT_TEST_INSTALLED with the perf tests.
+
+Instead use:
+
+    ./run <path-to-git> -- <tests>
+
+See t/perf/README for details."
+fi
+
 # Variables from test-lib that are normally internal to the tests; we
 # need to export them for test_perf subshells
 export TEST_DIRECTORY TRASH_DIRECTORY GIT_BUILD_DIR GIT_TEST_CMP
diff --git a/t/perf/run b/t/perf/run
index cd3882b117..c7b86104e1 100755
--- a/t/perf/run
+++ b/t/perf/run
@@ -84,6 +84,8 @@ set_git_test_installed () {
 		GIT_TEST_INSTALLED=$mydir_abs
 	fi
 	export GIT_TEST_INSTALLED
+	PERF_SET_GIT_TEST_INSTALLED=true
+	export PERF_SET_GIT_TEST_INSTALLED
 }
 
 run_dirs_helper () {
-- 
2.21.0.593.g511ec345e18

Re: [PATCH v2 2/2] perf-lib.sh: remove GIT_TEST_INSTALLED from perf-lib.sh

[Jeff King]

On Tue, May 07, 2019 at 10:31:23AM +0200, Ævar Arnfjörð Bjarmason wrote:

> > I'm not sure I get what's going on here. Why do we need the realpath in
> > aggregate.perl? We'd want to generate the same filename that "run"
> > decided to store things in, which we'd generate from the command-line
> > arguments (either passed on to us by "run", or direct from the user if
> > they're printing a previous run).
> 
> So this is part of the "has sucked since forever, future TODO" mentioned
> in 0/2.
> 
> I.e. if you pass "../.." as a path to "run" we'll try to discover a
> built/installed "git" in a "bindir" there, and then we need to do two
> things:
> 
>  1. Figure out a way to turn that into a filename sensible for the
>     *.times files.
>  2. Print some header showing that path in the aggregate output.
> 
> The "run" script will discover #1 for itself, that's what that "pwd &&
> tr -c ..." command is doing, but then we just pass "../.." again to
> aggregate.perl and have it figure it out again on its own, so it needs
> to duplicate the logic.
> 
> Just having both discover the absolute path all the time for #1 made
> things a lot simpler, e.g. if you do ../.. on v2.21.0 you'll get things
> like:
> 
>     _____.p0000-perf-lib-sanity.1.times
> 
> And with $PWD/../../ you'd get:
> 
>     _home_avar_g_git_t_perf______.p0000-perf-lib-sanity.1.times
> 
> Now this is all pretty & consistent. Any path to a "git" will always be
> turned into the absolute path, e.g.:
> 
>     _home_avar_g_git.p0000-perf-lib-sanity.1.times
> 
> And instead of "git" or ".." being printed in the aggregate header we
> print the path, e.g. "/home/avar/g/git".

OK. I sort of assumed we'd be sticking with the crappy "_____" for both
cases after your cleanup. But it really is changing behavior to name
things after the absolute path. I'd probably have split that out into
its own change, but I don't think it's worth revisiting at this point.

Thanks for explaining.

-Peff

Re: [PATCH v3 0/6] perf-lib.sh: fix 0baf78e7bc regression, refactor & fix bugs

[Jeff King]

On Tue, May 07, 2019 at 12:54:28PM +0200, Ævar Arnfjörð Bjarmason wrote:

> So here as a 6-parter given some of the feedback on v2. Maybe Jeff
> still hates it :), but this time around some of the changes are split
> up and should be easier to understand in isolation, as well as some
> more "noticed while I was at it" things fixed.

Thanks, the split did make things easier to understand.

These all look reasonable to me, and I confirmed by re-running a few of
the problematic perf tests that everything is behaving as expected after
the patches.

> This series ends with outright forbidding the user from directly
> setting GIT_TEST_INSTALLED. As discussed it makes things easier for
> us, and as noted in 1/6 the demand for that in the wild seems
> non-existent, since the way we've been documenting how you could do
> that with an environment variable has been broken since 2012.

I think this is reasonable, especially with the feature in patch 6 to
tell the caller they're doing it wrong, instead of just silently
producing nonsense results.

Junio, if you haven't been following closely, this can replace my patch
from jk/perf-installed-fix.

-Peff

Re: [PATCH v3 0/6] perf-lib.sh: fix 0baf78e7bc regression, refactor & fix bugs

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> I think this is reasonable, especially with the feature in patch 6 to
> tell the caller they're doing it wrong, instead of just silently
> producing nonsense results.
>
> Junio, if you haven't been following closely, this can replace my patch
> from jk/perf-installed-fix.

I did not pick up v2 or earlier as it wasn't clear what the
resolution would be, but I agree that this round looks sensible.

Thanks for the hint; very very much appreciated.