What's cooking in git.git (May 2019, #01; Thu, 9)

What's cooking in git.git (May 2019, #01; Thu, 9)

[Junio C Hamano]

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

The 8th batch of topics, which Hopefully is the final one before
-rc0, have been pushed out to 'master'.  The "no-extern" topic is
now in 'next', with its merge conflict with many other topics, is
still slowing me down when it is moved earlier in the merge order.
I expect it to need only one more topic shuffling before merged to
'master', so I hope I'd survive.

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

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


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


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


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


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


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


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


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


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


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


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


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


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


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


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


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


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


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


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


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

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

* ab/perf-installed-fix (2019-05-08) 6 commits
 - perf-lib.sh: forbid the use of GIT_TEST_INSTALLED
 - perf tests: add "bindir" prefix to git tree test results
 - perf-lib.sh: remove GIT_TEST_INSTALLED from perf-lib.sh
 - perf-lib.sh: make "./run <revisions>" use the correct gits
 - perf aggregate: remove GIT_TEST_INSTALLED from --codespeed
 - perf README: correct docs for 3c8f12c96c regression

 Performance test framework has been broken and measured the version
 of Git that happens to be on $PATH, not the specified one to
 measure, for a while, which has been corrected.

 Will merge to 'next'.
 cf. <20190507105434.9600-1-avarab@gmail.com>


* an/ignore-doc-update (2019-05-08) 1 commit
 - gitignore.txt: make slash-rules more readable

 The description about slashes in gitignore patterns (used to
 indicate things like "anchored to this level only" and "only
 matches directories") has been revamped.

 Almost there.
 cf. <20190507104507.18735-1-admin@in-ici.net>


* bl/t4253-exit-code-from-format-patch (2019-05-07) 1 commit
 - t4253-am-keep-cr-dos: avoid using pipes

 Avoid patterns to pipe output from a git command to feed another
 command in tests.

 Will merge to 'next'.


* cm/notes-comment-fix (2019-05-07) 1 commit
 - notes: correct documentation of format_display_notes()

 A stale in-code comment has been updated.

 Will merge to 'next'.


* dl/branch-from-3dot-merge-base (2019-05-07) 2 commits
 - branch: make create_branch accept a merge base rev
 - t2018: cleanup in current test

 "git branch new A...B" and "git checkout -b new A...B" have been
 taught that in their contexts, the notation A...B means "the merge
 base between these two commits", just like "git checkout A...B"
 detaches HEAD at that commit.

 Will merge to 'next'.
 cf. <cover.1556366347.git.liu.denton@gmail.com>


* ds/cvsexportcommit-force-text (2019-05-07) 1 commit
 - cvsexportcommit: force crlf translation

 "git cvsexportcommit" running on msys did not expect cvsnt showed
 "cvs status" output with CRLF line endings.

 Will merge to 'next'.


* en/fast-export-encoding (2019-05-07) 5 commits
 - fast-export: do automatic reencoding of commit messages only if requested
 - fast-export: differentiate between explicitly utf-8 and implicitly utf-8
 - fast-export: avoid stripping encoding header if we cannot reencode
 - fast-import: support 'encoding' commit header
 - t9350: fix encoding test to actually test reencoding

 The "git fast-export/import" pair has been taught to handle commits
 with log messages in encoding other than UTF-8 better.

 Will merge to 'next'.


* jh/trace2 (2019-04-26) 1 commit
  (merged to 'next' on 2019-05-09 at e1bba8aeac)
 + trace2: fix incorrect function pointer check

 An embarrassing bugfix.

 Will merge to 'master'.


* jk/apache-lsan (2019-05-08) 1 commit
 - t/lib-httpd: pass LSAN_OPTIONS through apache

 Allow tests that involve httpd to be run under leak sanitizer, just
 like we can already do so under address sanitizer.

 Will merge to 'next'.


* jk/cocci-batch (2019-05-08) 2 commits
 - coccicheck: make batch size of 0 mean "unlimited"
 - coccicheck: optionally batch spatch invocations

 Optionally "make coccicheck" can feed multiple source files to
 spatch, gaining performance while spending more memory.

 Will merge to 'next'.


* js/commit-graph-parse-leakfix (2019-05-07) 1 commit
 - commit-graph: fix memory leak

 Leakfix.

 Will merge to 'next'.


* js/fsmonitor-refresh-after-discarding-index (2019-05-08) 2 commits
 - fsmonitor: force a refresh after the index was discarded
 - fsmonitor: demonstrate that it is not refreshed after discard_index()

 The fsmonitor interface got out of sync after the in-core index
 file gets discarded, which has been corrected.

 Will merge to 'next'.


* js/t5580-unc-alternate-test (2019-05-07) 1 commit
 - t5580: verify that alternates can be UNC paths

 An additional test for MinGW

 Will merge to 'next'.


* js/t6500-use-windows-pid-on-mingw (2019-05-08) 1 commit
 - t6500(mingw): use the Windows PID of the shell

 Future-proof a test against an update to MSYS2 runtime v3.x series.

 Will merge to 'next'.
 cf. <pull.185.git.gitgitgadget@gmail.com>
 It might be helpful in the longer term to encapsulate the code that
 uses /proc/$$/winpid into a helper function and use it anywhere $$
 is referenced, but let's defer it until we see such a callsite that
 would be helped by such a move.


* mh/http-fread-api-fix (2019-05-08) 1 commit
 - Make fread/fwrite-like functions in http.c more like fread/fwrite.

 A pair of private functions in http.c that had names similar to
 fread/fwrite did not return the number of elements, which was found
 to be confusing.

 Will merge to 'next'.


* nd/merge-quit (2019-05-07) 2 commits
 - merge: add --quit
 - merge: remove drop_save() in favor of remove_merge_branch_state()

 "git merge" learned "--quit" option that cleans up the in-progress
 merge while leaving the working tree and the index still in a mess.

 Hmph, why is this a good idea?


* nd/parse-options-aliases (2019-05-07) 1 commit
 - parse-options: don't emit "ambiguous option" for aliases

 Attempt to use an abbreviated option in "git clone --recurs" is
 responded by a request to disambiguate between --recursive and
 --recurse-submodules, which is bad because these two are synonyms.
 The parse-options API has been extended to define such synonyms
 more easily and not produce an unnecessary failure.

 Will merge to 'next'.


* pw/rebase-abort-clean-rewritten (2019-05-08) 1 commit
 - rebase --abort: cleanup refs/rewritten
 (this branch uses pw/rebase-i-internal.)

 "git rebase --abort" used to leave refs/rewritten/ when concluding
 "git rebase -r", which has been corrected.

 Will merge to 'next'.


* sg/ci-libsvn-perl (2019-05-07) 1 commit
 - ci: install 'libsvn-perl' instead of 'git-svn'

 To run tests for Git SVN, our scripts for CI used to install the
 git-svn package (in the hope that it would bring in the right
 dependencies).  This has been updated to install the more direct
 dependency, namely, libsvn-perl.

 Will merge to 'next'.


* tt/no-ipv6-fallback-for-winxp (2019-05-07) 1 commit
 - mingw: remove obsolete IPv6-related code

 Code cleanup.

 Will merge to 'next'.


* jc/send-email-transferencoding-fix (2019-05-08) 2 commits
 - send-email: honor transferencoding config option again
 - send-email: update the mechanism to set default configuration values

 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.

 cf. <20190508105607.178244-1-gitster@pobox.com> (v2)

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

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


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


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

* nb/branch-show-other-worktrees-head (2019-05-07) 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.


* jc/make-dedup-ls-files-output (2019-04-22) 1 commit
  (merged to 'next' on 2019-05-09 at e3d5825003)
 + 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 'master'.
 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
  (merged to 'next' on 2019-05-09 at a68fe0ae72)
 + doc/ls-files: put nested list for "-t" option into block

 Docfix.

 Will merge to 'master'.


* jk/p5302-avoid-collision-check-cost (2019-04-23) 1 commit
  (merged to 'next' on 2019-05-09 at 8dc92cad96)
 + 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 'master'.


* dl/rev-tilde-doc-clarify (2019-05-08) 4 commits
  (merged to 'next' on 2019-05-09 at 6efd564b11)
 + revisions.txt: remove ambibuity between <rev>:<path> and :<path>
 + revisions.txt: mention <rev>~ form
 + revisions.txt: mark optional rev arguments with []
 + revisions.txt: change "rev" to "<rev>"

 Docfix.

 Will merge to 'master'.


* en/unicode-in-refnames (2019-04-26) 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
  (merged to 'next' on 2019-05-09 at e697c1993b)
 + 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 'master'.


* cc/access-on-aix-workaround (2019-04-25) 1 commit
  (merged to 'next' on 2019-05-09 at 79b25b1954)
 + git-compat-util: work around for access(X_OK) under root

 Workaround for standard-compliant but less-than-useful behaviour of
 access(2) for the root user.

 Will merge to 'master'.


* 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

 Update "git difftool" and "git mergetool" so that the combinations
 of {diff,merge}.{tool,guitool} configuration variables serve as
 fallback settings of each other in a sensible order.

 Will merge to 'next'.


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

 Ready for next?
 cf. <20190424151428.170316-1-dstolee@microsoft.com> (v5)


* es/first-contrib-tutorial (2019-05-08) 2 commits
 - documentation: add anchors to MyFirstContribution
 - documentation: add tutorial for first contribution

 A new tutorial targetting specifically aspiring git-core
 developers.

 I think we are almost there.
 cf. <20190507213040.151799-1-emilyshaffer@google.com> (v5)


* pw/clean-sequencer-state-upon-final-commit (2019-04-17) 2 commits
  (merged to 'next' on 2019-05-09 at cf6cce8ca7)
 + 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 'master'.


* dl/no-extern-in-func-decl (2019-05-05) 3 commits
  (merged to 'next' on 2019-05-09 at d165ac4cee)
 + *.[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 'master'.


* js/partial-clone-connectivity-check (2019-05-05) 2 commits
 - t/perf: add perf script for partial clones
  (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 'next' and then 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-05-07) 43 commits
 - Declare both git-switch and git-restore experimental
 - help: move git-diff and git-reset to different groups
 - 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. <20190425094600.15673-1-pclouds@gmail.com> (restore v3)


* ew/repack-with-bitmaps-by-default (2019-03-18) 3 commits
  (merged to 'next' on 2019-05-09 at 4f8e8b01c8)
 + 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 'master'.
 cf. <87a7g2iuem.fsf@evledraar.gmail.com>


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


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

 On hold.
 cf. <20190508001252.15752-1-avarab@gmail.com>
 cf. <xmqqa7fxionx.fsf@gitster-ct.c.googlers.com>


* jh/trace2-sid-fix (2019-05-07) 11 commits
 - trace2: fixup access problem on /etc/gitconfig in read_very_early_config
  (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 'next' and then to 'master'.
 cf. <pull.169.v4.git.gitgitgadget@gmail.com> (v4)


* pw/rebase-i-internal (2019-04-19) 13 commits
  (merged to 'next' on 2019-05-09 at 1206aa6865)
 + 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
 (this branch is used by pw/rebase-abort-clean-rewritten.)

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

 Will merge to 'master'.


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


* ds/commit-graph-format-v2 (2019-05-08) 6 commits
 - commit-graph: remove Future Work section
 - commit-graph: implement file format version 2
 - commit-graph: add --version=<n> option
 - commit-graph: create new version parameter
 - 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.

 Still actively discussed.
 cf. <pull.112.v3.git.gitgitgadget@gmail.com> (v3)


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

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

* 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/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()

 Dropped.
 cf. <CACsJy8DcaxBLCa2vK=MfoxkaHS0gDmdUsmamyqE2yOaoG_Esog@mail.gmail.com>


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

 Can be safely discarded.
 cf. <CACsJy8BcyD199L4qGv6-TP-8HD+GS+ZDNN5jspkh5uVaWekkoQ@mail.gmail.com>


* 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>
 Replaced by jc/send-email-transferencoding-fix

Re: What's cooking in git.git (May 2019, #01; Thu, 9)

[Denton Liu]

Hi Junio,

On Thu, May 09, 2019 at 02:23:24AM +0900, Junio C Hamano wrote:
> Here are the topics that have been cooking.  Commits prefixed with
> '-' are only in 'pu' (proposed updates) while commits prefixed with
> '+' are in 'next'.  The ones marked with '.' do not appear in any of
> the integration branches, but I am still holding onto them.
> 
> The 8th batch of topics, which Hopefully is the final one before
> -rc0, have been pushed out to 'master'.  The "no-extern" topic is
> now in 'next', with its merge conflict with many other topics, is
> still slowing me down when it is moved earlier in the merge order.
> I expect it to need only one more topic shuffling before merged to
> 'master', so I hope I'd survive.

Thanks for taking on the hard work for this!

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

[snip]

> * 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
> 
>  Update "git difftool" and "git mergetool" so that the combinations
>  of {diff,merge}.{tool,guitool} configuration variables serve as
>  fallback settings of each other in a sensible order.
> 
>  Will merge to 'next'.

A reroll of this was sent at
<cover.1556518203.git.liu.denton@gmail.com> based on some comments that
David Aguilar made.

Thanks,

Denton

Re: What's cooking in git.git (May 2019, #01; Thu, 9)

[Elijah Newren]

On Wed, May 8, 2019 at 10:23 AM Junio C Hamano <gitster@pobox.com> wrote:

> * en/unicode-in-refnames (2019-04-26) 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?

I believe so.  en/unicode-in-refnames represents v2; the last "What's
cooking?" email went out with v1 and I responded that a small commit
message cleanup was needed as per Torsten's review, but that was the
only issue I was aware of.

However, I think your summary of this topic still needs some minor
wording/spelling corrections; here's your summary with my suggested
revisions:

On a filesystem like HFS+, 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 get "renamed".  Work around the
mismatch by paying attention to the core.precomposeUnicode
configuration.

Re: What's cooking in git.git (May 2019, #01; Thu, 9)

[Phillip Wood]

On 08/05/2019 18:23, Junio C Hamano wrote:
> * pw/rebase-abort-clean-rewritten (2019-05-08) 1 commit
>   - rebase --abort: cleanup refs/rewritten
>   (this branch uses pw/rebase-i-internal.)
> 
>   "git rebase --abort" used to leave refs/rewritten/ when concluding
>   "git rebase -r", which has been corrected.
> 
>   Will merge to 'next'.

Can you hold off on this one please, I think we should clean up 
refs/rewritten/ on --quit as well (I'm not sure I've convinced dscho 
about that yet though [1])

Thanks

Phillip

[1] 
https://public-inbox.org/git/4d486504-7f64-95fb-b7eb-51d424f3e6cc@gmail.com/#t

Re: What's cooking in git.git (May 2019, #01; Thu, 9)

[Johannes Schindelin]

Hi Phillip,

On Thu, 9 May 2019, Phillip Wood wrote:

> On 08/05/2019 18:23, Junio C Hamano wrote:
> > * pw/rebase-abort-clean-rewritten (2019-05-08) 1 commit
> >   - rebase --abort: cleanup refs/rewritten
> >   (this branch uses pw/rebase-i-internal.)
> >
> >   "git rebase --abort" used to leave refs/rewritten/ when concluding
> >   "git rebase -r", which has been corrected.
> >
> >   Will merge to 'next'.
>
> Can you hold off on this one please, I think we should clean up
> refs/rewritten/ on --quit as well (I'm not sure I've convinced dscho about
> that yet though [1])

You have convinced me ;-)

Thanks for your consideration,
Dscho

> Thanks
>
> Phillip
>
> [1]
> https://public-inbox.org/git/4d486504-7f64-95fb-b7eb-51d424f3e6cc@gmail.com/#t
>
>

Re: What's cooking in git.git (May 2019, #01; Thu, 9)

[Phillip Wood]

Hi Dscho

On 10/05/2019 14:49, Johannes Schindelin wrote:
> Hi Phillip,
> 
> On Thu, 9 May 2019, Phillip Wood wrote:
> 
>> On 08/05/2019 18:23, Junio C Hamano wrote:
>>> * pw/rebase-abort-clean-rewritten (2019-05-08) 1 commit
>>>    - rebase --abort: cleanup refs/rewritten
>>>    (this branch uses pw/rebase-i-internal.)
>>>
>>>    "git rebase --abort" used to leave refs/rewritten/ when concluding
>>>    "git rebase -r", which has been corrected.
>>>
>>>    Will merge to 'next'.
>>
>> Can you hold off on this one please, I think we should clean up
>> refs/rewritten/ on --quit as well (I'm not sure I've convinced dscho about
>> that yet though [1])
> 
> You have convinced me ;-)

Thanks, I'll try and send a re-roll tomorrow

Best Wishes

Phillip

> Thanks for your consideration,
> Dscho
> 
>> Thanks
>>
>> Phillip
>>
>> [1]
>> https://public-inbox.org/git/4d486504-7f64-95fb-b7eb-51d424f3e6cc@gmail.com/#t
>>
>>

Re: What's cooking in git.git (May 2019, #01; Thu, 9)

[Duy Nguyen]

On Thu, May 9, 2019 at 12:23 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> * nd/merge-quit (2019-05-07) 2 commits
>  - merge: add --quit
>  - merge: remove drop_save() in favor of remove_merge_branch_state()
>
>  "git merge" learned "--quit" option that cleans up the in-progress
>  merge while leaving the working tree and the index still in a mess.
>
>  Hmph, why is this a good idea?

The first reason is consistency, besides bisect, all other in-progress
commands have both --abort and --quit.

The second is because it's an escape hatch suggestion in git-switch.
And this goes back to the first reason because git-checkout will just
let you switch and do stuff. If you forget about the merge, at some
point continue the merge won't even make sense.

> * nd/switch-and-restore (2019-05-07) 43 commits
>  - Declare both git-switch and git-restore experimental
>  - help: move git-diff and git-reset to different groups
>  - 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. <20190425094600.15673-1-pclouds@gmail.com> (restore v3)

No opinon here.

I do have three small patches to refine git-switch. But I think we can
do it on top. Probably best that way anyway to keep the justification
for the changes in commit message.

git-restore works quite nicely for me, by my opinion in this area does
not count. Besides Emily's usability issue with shell wildcard
expansion, intent-to-add support is still not there. And Phillip might
want to change --discard-changes behavior. That's all that's left.
-- 
Duy

en/fast-export-encoding, was Re: What's cooking in git.git (May 2019, #01; Thu, 9)

[Johannes Schindelin]

Hi Junio & Elijah,

On Thu, 9 May 2019, Junio C Hamano wrote:

> * en/fast-export-encoding (2019-05-07) 5 commits
>  - fast-export: do automatic reencoding of commit messages only if requested
>  - fast-export: differentiate between explicitly utf-8 and implicitly utf-8
>  - fast-export: avoid stripping encoding header if we cannot reencode
>  - fast-import: support 'encoding' commit header
>  - t9350: fix encoding test to actually test reencoding
>
>  The "git fast-export/import" pair has been taught to handle commits
>  with log messages in encoding other than UTF-8 better.

This breaks on Windows, see
https://dev.azure.com/gitgitgadget/git/_build/results?buildId=8298&view=ms.vss-test-web.build-test-results-tab

Sadly, I ran out of time looking at it in detail.

Ciao,
Dscho

Re: en/fast-export-encoding, was Re: What's cooking in git.git (May 2019, #01; Thu, 9)

[Elijah Newren]

Hi Johannes,

On Thu, May 9, 2019 at 1:46 PM Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
>
> Hi Junio & Elijah,
>
> On Thu, 9 May 2019, Junio C Hamano wrote:
>
> > * en/fast-export-encoding (2019-05-07) 5 commits
> >  - fast-export: do automatic reencoding of commit messages only if requested
> >  - fast-export: differentiate between explicitly utf-8 and implicitly utf-8
> >  - fast-export: avoid stripping encoding header if we cannot reencode
> >  - fast-import: support 'encoding' commit header
> >  - t9350: fix encoding test to actually test reencoding
> >
> >  The "git fast-export/import" pair has been taught to handle commits
> >  with log messages in encoding other than UTF-8 better.
>
> This breaks on Windows, see
> https://dev.azure.com/gitgitgadget/git/_build/results?buildId=8298&view=ms.vss-test-web.build-test-results-tab
>
> Sadly, I ran out of time looking at it in detail.

Thanks for the heads up, and for taking some time to check it out.
The error doesn't seem obvious from the log.  Does Azure Pipelines
have anything like CircleCI's "Debug with SSH" feature[1]?  (Where one
can click a "Rerun job with SSH", and it'll restart the pipeline but
also print out an ssh command someone can use to directly access the
box on which the test is running, in order to be able to investigate.)
 Failing that, assuming I can find a Windows system somewhere, is
there a list of steps for setting up a development environment and
building git on Windows?

Elijah

[1] https://circleci.com/docs/2.0/ssh-access-jobs/

Re: en/fast-export-encoding, was Re: What's cooking in git.git (May 2019, #01; Thu, 9)

[Johannes Sixt]

Am 10.05.19 um 02:14 schrieb Elijah Newren:
> Hi Johannes,
> 
> On Thu, May 9, 2019 at 1:46 PM Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
>>
>> Hi Junio & Elijah,
>>
>> On Thu, 9 May 2019, Junio C Hamano wrote:
>>
>>> * en/fast-export-encoding (2019-05-07) 5 commits
>>>  - fast-export: do automatic reencoding of commit messages only if requested
>>>  - fast-export: differentiate between explicitly utf-8 and implicitly utf-8
>>>  - fast-export: avoid stripping encoding header if we cannot reencode
>>>  - fast-import: support 'encoding' commit header
>>>  - t9350: fix encoding test to actually test reencoding
>>>
>>>  The "git fast-export/import" pair has been taught to handle commits
>>>  with log messages in encoding other than UTF-8 better.
>>
>> This breaks on Windows, see
>> https://dev.azure.com/gitgitgadget/git/_build/results?buildId=8298&view=ms.vss-test-web.build-test-results-tab
>>
>> Sadly, I ran out of time looking at it in detail.
> 
> Thanks for the heads up, and for taking some time to check it out.
> The error doesn't seem obvious from the log.  Does Azure Pipelines
> have anything like CircleCI's "Debug with SSH" feature[1]?  (Where one
> can click a "Rerun job with SSH", and it'll restart the pipeline but
> also print out an ssh command someone can use to directly access the
> box on which the test is running, in order to be able to investigate.)
>  Failing that, assuming I can find a Windows system somewhere, is
> there a list of steps for setting up a development environment and
> building git on Windows?

I'll just tell you why things go wrong here:

In these cases, a byte that is intended to be an ISO8859-something
characters is passed via the command line. This cannot work as intended
on Windows, because the command line is not just a stream of bytes, but
a string of characters. On Windows (and presumably also on macOS), the
command line bytes are interpreted as UTF-8. As such, the bytes undergo
some encoding conversions between UTF-8 and UTF-16LE. That cannot work
when the bytes are not correct UTF-8 characters.

To make the tests pass you have to pass the ISO8859-something characters
via a file.

-- Hannes

Re: en/fast-export-encoding, was Re: What's cooking in git.git (May 2019, #01; Thu, 9)

[Johannes Schindelin]

Hi Hannes & Elijah,

On Fri, 10 May 2019, Johannes Sixt wrote:

> Am 10.05.19 um 02:14 schrieb Elijah Newren:
> > Hi Johannes,
> >
> > On Thu, May 9, 2019 at 1:46 PM Johannes Schindelin
> > <Johannes.Schindelin@gmx.de> wrote:
> >>
> >> Hi Junio & Elijah,
> >>
> >> On Thu, 9 May 2019, Junio C Hamano wrote:
> >>
> >>> * en/fast-export-encoding (2019-05-07) 5 commits
> >>>  - fast-export: do automatic reencoding of commit messages only if requested
> >>>  - fast-export: differentiate between explicitly utf-8 and implicitly utf-8
> >>>  - fast-export: avoid stripping encoding header if we cannot reencode
> >>>  - fast-import: support 'encoding' commit header
> >>>  - t9350: fix encoding test to actually test reencoding
> >>>
> >>>  The "git fast-export/import" pair has been taught to handle commits
> >>>  with log messages in encoding other than UTF-8 better.
> >>
> >> This breaks on Windows, see
> >> https://dev.azure.com/gitgitgadget/git/_build/results?buildId=8298&view=ms.vss-test-web.build-test-results-tab
> >>
> >> Sadly, I ran out of time looking at it in detail.
> >
> > Thanks for the heads up, and for taking some time to check it out.
> > The error doesn't seem obvious from the log.  Does Azure Pipelines
> > have anything like CircleCI's "Debug with SSH" feature[1]?  (Where one
> > can click a "Rerun job with SSH", and it'll restart the pipeline but
> > also print out an ssh command someone can use to directly access the
> > box on which the test is running, in order to be able to investigate.)
> >  Failing that, assuming I can find a Windows system somewhere, is
> > there a list of steps for setting up a development environment and
> > building git on Windows?
>
> I'll just tell you why things go wrong here:
>
> In these cases, a byte that is intended to be an ISO8859-something
> characters is passed via the command line. This cannot work as intended
> on Windows, because the command line is not just a stream of bytes, but
> a string of characters. On Windows (and presumably also on macOS), the
> command line bytes are interpreted as UTF-8. As such, the bytes undergo
> some encoding conversions between UTF-8 and UTF-16LE. That cannot work
> when the bytes are not correct UTF-8 characters.
>
> To make the tests pass you have to pass the ISO8859-something characters
> via a file.

Thanks for the explanation. Yes, we cannot rely on command-lines (or for
that matter, environment variables) being opaque byte sequences, as that
does not work on Windows: byte sequences *always* have an encoding, and
are pretty much always converted into UTF-16 before continuing.

As to Debug with SSH: this is not possible in Azure Pipelines. What I
frequently do is to edit azure-pipelines.yml (usually restricting to one
particular job, e.g. Windows build, and to one particular test script) and
ci/ and t/ heavily, to get tons of debug information, then open a PR on
GitGitGadget to start a build.

That's how I investigated the macOS Mojave breakages, for example.

Ciao,
Dscho

nd/merge-quit, was Re: What's cooking in git.git (May 2019, #01; Thu, 9)

[Johannes Schindelin]

Hi Junio & Duy,

On Thu, 9 May 2019, Junio C Hamano wrote:

> * nd/merge-quit (2019-05-07) 2 commits
>  - merge: add --quit
>  - merge: remove drop_save() in favor of remove_merge_branch_state()
>
>  "git merge" learned "--quit" option that cleans up the in-progress
>  merge while leaving the working tree and the index still in a mess.
>
>  Hmph, why is this a good idea?

It also seems to work *only* on Linux. At least the tests break on macOS
and on Windows:

https://dev.azure.com/gitgitgadget/git/_build/results?buildId=8313&view=ms.vss-test-web.build-test-results-tab

Sadly, I ran out of time do look into this (I am pretty busy preparing Git
for Windows for v2.22.0-rc0).

Ciao,
Dscho

Re: nd/merge-quit, was Re: What's cooking in git.git (May 2019, #01; Thu, 9)

[Duy Nguyen]

On Fri, May 10, 2019 at 3:54 AM Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
>
> Hi Junio & Duy,
>
> On Thu, 9 May 2019, Junio C Hamano wrote:
>
> > * nd/merge-quit (2019-05-07) 2 commits
> >  - merge: add --quit
> >  - merge: remove drop_save() in favor of remove_merge_branch_state()
> >
> >  "git merge" learned "--quit" option that cleans up the in-progress
> >  merge while leaving the working tree and the index still in a mess.
> >
> >  Hmph, why is this a good idea?
>
> It also seems to work *only* on Linux. At least the tests break on macOS
> and on Windows:
>
> https://dev.azure.com/gitgitgadget/git/_build/results?buildId=8313&view=ms.vss-test-web.build-test-results-tab

Sorry I have no idea what the problem is. That's basically the same as
the 'merge detects mod-256 conflicts (recursive)' test earlier but
with rerere enabled. It does not even look like some leftover rerere
records accidentally fix the conflict.

I tried with a case-insensitive filesytem (on linux) and with
--valgrind, no problem found. Travis on pu seemed ok with t7600 on
mac.

One difference I notice is the the failed test looks like it found the
wrong merge base

found 1 common ancestor:
c4c4222 commit 1

while my tests have "commit 0" as the base. "git log --graph
--oneline" indicates "commit 1" is the wrong base.

Something is wrong with the merge code (this has not even reached the
new --quit code). I could change the setup steps to be more stable,
using a simpler commit history, but this looks like something we
should find and fix.

> Sadly, I ran out of time do look into this (I am pretty busy preparing Git
> for Windows for v2.22.0-rc0).
>
> Ciao,
> Dscho



--
Duy

Re: nd/merge-quit, was Re: What's cooking in git.git (May 2019, #01; Thu, 9)

[Johannes Schindelin]

Hi Duy,

On Fri, 10 May 2019, Duy Nguyen wrote:

> On Fri, May 10, 2019 at 3:54 AM Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> >
> > Hi Junio & Duy,
> >
> > On Thu, 9 May 2019, Junio C Hamano wrote:
> >
> > > * nd/merge-quit (2019-05-07) 2 commits
> > >  - merge: add --quit
> > >  - merge: remove drop_save() in favor of remove_merge_branch_state()
> > >
> > >  "git merge" learned "--quit" option that cleans up the in-progress
> > >  merge while leaving the working tree and the index still in a mess.
> > >
> > >  Hmph, why is this a good idea?
> >
> > It also seems to work *only* on Linux. At least the tests break on macOS
> > and on Windows:
> >
> > https://dev.azure.com/gitgitgadget/git/_build/results?buildId=8313&view=ms.vss-test-web.build-test-results-tab
>
> Sorry I have no idea what the problem is. That's basically the same as
> the 'merge detects mod-256 conflicts (recursive)' test earlier but
> with rerere enabled. It does not even look like some leftover rerere
> records accidentally fix the conflict.
>
> I tried with a case-insensitive filesytem (on linux) and with
> --valgrind, no problem found. Travis on pu seemed ok with t7600 on
> mac.
>
> One difference I notice is the the failed test looks like it found the
> wrong merge base
>
> found 1 common ancestor:
> c4c4222 commit 1
>
> while my tests have "commit 0" as the base. "git log --graph
> --oneline" indicates "commit 1" is the wrong base.
>
> Something is wrong with the merge code (this has not even reached the
> new --quit code). I could change the setup steps to be more stable,
> using a simpler commit history, but this looks like something we
> should find and fix.

Yeah... someone should look at this... Someone. But who?

:-)

Well, since you seemed quite reluctant to figure out why your patches fail
the test suite, and since we're about to enter the -rc0 phase (where we
all spend all of our time to hammer out the next version, right? Right?),
I figured out I better look into it before nobody does.

Turns out that the culprit is not even hard to figure out. All I had to do
is to compare, carefully, the logs from the Azure Pipelines and from a
local run in a local Ubuntu.

It has nothing to do with our merge code. There might be bugs, but this
breakage is safely in this here patch series: the test case you introduced
relies on side effects.

Namely, when test cases 51 and 52 are skipped because of a missing GPG
prerequisite [*1*], and those two are obviously required to run for the
`git merge to fail in your test case, as you can very easily verify by
downloading the artifact containing the `trash directory.t7600-merge`
directory and re-running the last steps on Linux (where the `git -c
rerere.enabled=true merge master` *succeeds*).

In fact, you can very, very easily emulate the whole situation on your box
by running:

	sh t7600-merge.sh -i -v -x --run=1-50,53-59

And then you can fix your test case so that it does not need to rely on
test cases that may, or may not, have run previously.

Ciao,
Johannes

Footnote *1*: GNU Privacy Guard is not actually missing from Git for
Windows' SDK, quite to the contrary. But it fails to start a gpg-agent due
to the fact that we pass a `--homedir` that contains a colon, something
that is totally expected on Windows, and at the same something that GNU
Privacy Guard totally cannot handle.

Re: nd/merge-quit, was Re: What's cooking in git.git (May 2019, #01; Thu, 9)

[Johannes Schindelin]

Hi,

On Mon, 13 May 2019, Johannes Schindelin wrote:

> On Fri, 10 May 2019, Duy Nguyen wrote:
>
> > On Fri, May 10, 2019 at 3:54 AM Johannes Schindelin
> > <Johannes.Schindelin@gmx.de> wrote:
> > >
> > > Hi Junio & Duy,
> > >
> > > On Thu, 9 May 2019, Junio C Hamano wrote:
> > >
> > > > * nd/merge-quit (2019-05-07) 2 commits
> > > >  - merge: add --quit
> > > >  - merge: remove drop_save() in favor of remove_merge_branch_state()
> > > >
> > > >  "git merge" learned "--quit" option that cleans up the in-progress
> > > >  merge while leaving the working tree and the index still in a mess.
> > > >
> > > >  Hmph, why is this a good idea?
> > >
> > > It also seems to work *only* on Linux. At least the tests break on macOS
> > > and on Windows:
> > >
> > > https://dev.azure.com/gitgitgadget/git/_build/results?buildId=8313&view=ms.vss-test-web.build-test-results-tab
> >
> > Sorry I have no idea what the problem is. That's basically the same as
> > the 'merge detects mod-256 conflicts (recursive)' test earlier but
> > with rerere enabled. It does not even look like some leftover rerere
> > records accidentally fix the conflict.
> >
> > I tried with a case-insensitive filesytem (on linux) and with
> > --valgrind, no problem found. Travis on pu seemed ok with t7600 on
> > mac.
> >
> > One difference I notice is the the failed test looks like it found the
> > wrong merge base
> >
> > found 1 common ancestor:
> > c4c4222 commit 1
> >
> > while my tests have "commit 0" as the base. "git log --graph
> > --oneline" indicates "commit 1" is the wrong base.
> >
> > Something is wrong with the merge code (this has not even reached the
> > new --quit code). I could change the setup steps to be more stable,
> > using a simpler commit history, but this looks like something we
> > should find and fix.
>
> Yeah... someone should look at this... Someone. But who?
>
> :-)
>
> Well, since you seemed quite reluctant to figure out why your patches fail
> the test suite, and since we're about to enter the -rc0 phase (where we
> all spend all of our time to hammer out the next version, right? Right?),
> I figured out I better look into it before nobody does.
>
> Turns out that the culprit is not even hard to figure out. All I had to do
> is to compare, carefully, the logs from the Azure Pipelines and from a
> local run in a local Ubuntu.
>
> It has nothing to do with our merge code. There might be bugs, but this
> breakage is safely in this here patch series: the test case you introduced
> relies on side effects.
>
> Namely, when test cases 51 and 52 are skipped because of a missing GPG
> prerequisite [*1*], and those two are obviously required to run for the
> `git merge to fail in your test case, as you can very easily verify by
> downloading the artifact containing the `trash directory.t7600-merge`
> directory and re-running the last steps on Linux (where the `git -c
> rerere.enabled=true merge master` *succeeds*).

I should have posted the link, as it may not be totally obvious where you
can download artifacts:

https://dev.azure.com/mseng/AzureDevOps/_build/results?buildId=9464474&view=artifacts

Ciao,
Johannes

> In fact, you can very, very easily emulate the whole situation on your box
> by running:
>
> 	sh t7600-merge.sh -i -v -x --run=1-50,53-59
>
> And then you can fix your test case so that it does not need to rely on
> test cases that may, or may not, have run previously.
>
> Ciao,
> Johannes
>
> Footnote *1*: GNU Privacy Guard is not actually missing from Git for
> Windows' SDK, quite to the contrary. But it fails to start a gpg-agent due
> to the fact that we pass a `--homedir` that contains a colon, something
> that is totally expected on Windows, and at the same something that GNU
> Privacy Guard totally cannot handle.
>
>

Re: nd/merge-quit, was Re: What's cooking in git.git (May 2019, #01; Thu, 9)

[Eric Sunshine]

On Mon, May 13, 2019 at 10:06 AM Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> I should have posted the link, as it may not be totally obvious where you
> can download artifacts:
>
> https://dev.azure.com/mseng/AzureDevOps/_build/results?buildId=9464474&view=artifacts

This link leads to a Login page requiring some sort of Microsoft ID.

Re: nd/merge-quit, was Re: What's cooking in git.git (May 2019, #01; Thu, 9)

[Johannes Schindelin]

Hi Eric,

On Mon, 13 May 2019, Eric Sunshine wrote:

> On Mon, May 13, 2019 at 10:06 AM Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> > I should have posted the link, as it may not be totally obvious where you
> > can download artifacts:
> >
> > https://dev.azure.com/mseng/AzureDevOps/_build/results?buildId=9464474&view=artifacts
>
> This link leads to a Login page requiring some sort of Microsoft ID.

Ah, whoops, that is not the build that I was looking for (it is for an
internal fork of Git which I use extensively to test various things).

And the public Azure Pipeline that actually publishes those artifacts only
runs for the four integration branches.

Still, you should be able to replicate my findings with the
`failed-test-artifacts` artifact from
https://dev.azure.com/git/git/_build/results?buildId=569&view=results

Ciao,
Dscho

[PATCH] tests: add a special setup where prerequisites fail

[Ævar Arnfjörð Bjarmason]

As discussed in [1] there's a regression in the "pu" branch now
because a new test implicitly assumed that a previous test guarded by
a prerequisite had been run. Add a "GIT_TEST_FAIL_PREREQS" special
test setup where we'll skip (nearly) all tests guarded by
prerequisites, allowing us to easily emulate those platform where we
don't run these tests.

As noted in the documentation I'm adding I'm whitelisting the SYMLINKS
prerequisite for now. A lot of tests started failing if we lied about
not supporting symlinks. It's also unlikely that we'll have a failing
test due to a hard dependency on symlinks without that being the
obvious cause, so for now it's not worth the effort to make it work.

1. https://public-inbox.org/git/nycvar.QRO.7.76.6.1905131531000.44@tvgsbejvaqbjf.bet/

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

On Mon, May 13 2019, Johannes Schindelin wrote:

> [...]
> Namely, when test cases 51 and 52 are skipped because of a missing GPG
> prerequisite [*1*], and those two are obviously required to run for the
> `git merge to fail in your test case, as you can very easily verify by
> downloading the artifact containing the `trash directory.t7600-merge`
> directory and re-running the last steps on Linux (where the `git -c
> rerere.enabled=true merge master` *succeeds*).
>
> In fact, you can very, very easily emulate the whole situation on your box
> by running:
>
> 	sh t7600-merge.sh -i -v -x --run=1-50,53-59
>
> And then you can fix your test case so that it does not need to rely on
> test cases that may, or may not, have run previously.

I think it would be better to more pro-actively spot this sort of
thing in the future, so here's a patch to do that. It passes on
"master", but fails on "pu" due to the issue with the one test being
discussed here.

 t/README                   |  9 +++++++++
 t/t0000-basic.sh           | 10 +++++-----
 t/t4202-log.sh             |  2 +-
 t/t7405-submodule-merge.sh |  2 +-
 t/t7810-grep.sh            |  6 +++---
 t/test-lib-functions.sh    | 20 ++++++++++++++++++++
 t/test-lib.sh              |  4 ++++
 7 files changed, 43 insertions(+), 10 deletions(-)

diff --git a/t/README b/t/README
index 6404f33e19..9747971d58 100644
--- a/t/README
+++ b/t/README
@@ -334,6 +334,15 @@ that cannot be easily covered by a few specific test cases. These
 could be enabled by running the test suite with correct GIT_TEST_
 environment set.
 
+GIT_TEST_FAIL_PREREQS<non-empty?> fails all prerequisites. This is
+useful for discovering issues with the tests where say a later test
+implicitly depends on an optional earlier test.
+
+There's a "FAIL_PREREQS" prerequisite that can be used to test for
+whether this mode is active, and e.g. skip some tests that are hard to
+refactor to deal with it. The "SYMLINKS" prerequisite is currently
+excluded as so much relies on it, but this might change in the future.
+
 GIT_TEST_GETTEXT_POISON=<non-empty?> turns all strings marked for
 translation into gibberish if non-empty (think "test -n"). Used for
 spotting those tests that need to be marked with a C_LOCALE_OUTPUT
diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index c03054c538..31de7e90f3 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -726,7 +726,7 @@ donthaveit=yes
 test_expect_success DONTHAVEIT 'unmet prerequisite causes test to be skipped' '
 	donthaveit=no
 '
-if test $haveit$donthaveit != yesyes
+if test -z "$GIT_TEST_FAIL_PREREQS" -a $haveit$donthaveit != yesyes
 then
 	say "bug in test framework: prerequisite tags do not work reliably"
 	exit 1
@@ -747,7 +747,7 @@ donthaveiteither=yes
 test_expect_success DONTHAVEIT,HAVEIT 'unmet prerequisites causes test to be skipped' '
 	donthaveiteither=no
 '
-if test $haveit$donthaveit$donthaveiteither != yesyesyes
+if test -z "$GIT_TEST_FAIL_PREREQS" -a $haveit$donthaveit$donthaveiteither != yesyesyes
 then
 	say "bug in test framework: multiple prerequisite tags do not work reliably"
 	exit 1
@@ -763,7 +763,7 @@ test_expect_success !LAZY_TRUE 'missing lazy prereqs skip tests' '
 	donthavetrue=no
 '
 
-if test "$havetrue$donthavetrue" != yesyes
+if test -z "$GIT_TEST_FAIL_PREREQS" -a "$havetrue$donthavetrue" != yesyes
 then
 	say 'bug in test framework: lazy prerequisites do not work'
 	exit 1
@@ -779,7 +779,7 @@ test_expect_success LAZY_FALSE 'missing negative lazy prereqs will skip' '
 	havefalse=no
 '
 
-if test "$nothavefalse$havefalse" != yesyes
+if test -z "$GIT_TEST_FAIL_PREREQS" -a "$nothavefalse$havefalse" != yesyes
 then
 	say 'bug in test framework: negative lazy prerequisites do not work'
 	exit 1
@@ -790,7 +790,7 @@ test_expect_success 'tests clean up after themselves' '
 	test_when_finished clean=yes
 '
 
-if test $clean != yes
+if test -z "$GIT_TEST_FAIL_PREREQS" -a $clean != yes
 then
 	say "bug in test framework: basic cleanup command does not work reliably"
 	exit 1
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 819c24d10e..c20209324c 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -352,7 +352,7 @@ test_expect_success 'log with grep.patternType configuration and command line' '
 	test_cmp expect actual
 '
 
-test_expect_success 'log with various grep.patternType configurations & command-lines' '
+test_expect_success !FAIL_PREREQS 'log with various grep.patternType configurations & command-lines' '
 	git init pattern-type &&
 	(
 		cd pattern-type &&
diff --git a/t/t7405-submodule-merge.sh b/t/t7405-submodule-merge.sh
index 7855bd8648..aa33978ed2 100755
--- a/t/t7405-submodule-merge.sh
+++ b/t/t7405-submodule-merge.sh
@@ -417,7 +417,7 @@ test_expect_failure 'directory/submodule conflict; keep submodule clean' '
 	)
 '
 
-test_expect_failure 'directory/submodule conflict; should not treat submodule files as untracked or in the way' '
+test_expect_failure !FAIL_PREREQS 'directory/submodule conflict; should not treat submodule files as untracked or in the way' '
 	test_when_finished "git -C directory-submodule/path reset --hard" &&
 	test_when_finished "git -C directory-submodule reset --hard" &&
 	(
diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index 2e1bb61b41..7d7b396c23 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -412,7 +412,7 @@ do
 		test_cmp expected actual
 	'
 
-	test_expect_success !PCRE "grep $L with grep.patterntype=perl errors without PCRE" '
+	test_expect_success !FAIL_PREREQS,!PCRE "grep $L with grep.patterntype=perl errors without PCRE" '
 		test_must_fail git -c grep.patterntype=perl grep "foo.*bar"
 	'
 
@@ -1234,7 +1234,7 @@ test_expect_success PCRE 'grep --perl-regexp pattern' '
 	test_cmp expected actual
 '
 
-test_expect_success !PCRE 'grep --perl-regexp pattern errors without PCRE' '
+test_expect_success !FAIL_PREREQS,!PCRE 'grep --perl-regexp pattern errors without PCRE' '
 	test_must_fail git grep --perl-regexp "foo.*bar"
 '
 
@@ -1249,7 +1249,7 @@ test_expect_success LIBPCRE2 "grep -P with (*NO_JIT) doesn't error out" '
 
 '
 
-test_expect_success !PCRE 'grep -P pattern errors without PCRE' '
+test_expect_success !FAIL_PREREQS,!PCRE 'grep -P pattern errors without PCRE' '
 	test_must_fail git grep -P "foo.*bar"
 '
 
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 8270de74be..0367cec5fd 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -309,6 +309,26 @@ test_unset_prereq () {
 }
 
 test_set_prereq () {
+	if test -n "$GIT_TEST_FAIL_PREREQS"
+	then
+		case "$1" in
+		# The "!" case is handled below with
+		# test_unset_prereq()
+		!*)
+			;;
+		# (Temporary?) whitelist of things we can't easily
+		# pretend not to support
+		SYMLINKS)
+			;;
+		# Inspecting whether GIT_TEST_FAIL_PREREQS is on
+		# should be unaffected.
+		FAIL_PREREQS)
+			;;
+		*)
+			return
+		esac
+	fi
+
 	case "$1" in
 	!*)
 		test_unset_prereq "${1#!}"
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 908ddb9c46..6fabafebb3 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -1607,3 +1607,7 @@ test_lazy_prereq SHA1 '
 test_lazy_prereq REBASE_P '
 	test -z "$GIT_TEST_SKIP_REBASE_P"
 '
+
+test_lazy_prereq FAIL_PREREQS '
+	test -n "$GIT_TEST_FAIL_PREREQS"
+'
-- 
2.21.0.1020.gf2820cf01a

Re: [PATCH] tests: add a special setup where prerequisites fail

[Johannes Schindelin]

[-- Attachment #1: Type: text/plain, Size: 8443 bytes --]

Hi Ævar,

On Mon, 13 May 2019, Ævar Arnfjörð Bjarmason wrote:

> As discussed in [1] there's a regression in the "pu" branch now
> because a new test implicitly assumed that a previous test guarded by
> a prerequisite had been run. Add a "GIT_TEST_FAIL_PREREQS" special
> test setup where we'll skip (nearly) all tests guarded by
> prerequisites, allowing us to easily emulate those platform where we
> don't run these tests.
>
> As noted in the documentation I'm adding I'm whitelisting the SYMLINKS
> prerequisite for now. A lot of tests started failing if we lied about
> not supporting symlinks. It's also unlikely that we'll have a failing
> test due to a hard dependency on symlinks without that being the
> obvious cause, so for now it's not worth the effort to make it work.

I don't know... In Git for Windows, the SYMLINKS prereq is not met.

(Side note: Windows 10 already supports symlinks for quite some time, even
for non-admin developers, but the fact that Git's test suite is
implemented in shell script bites us yet one more time: MSYS2 has a
completely different idea what symlinks are. It uses the "system file" bit
that only exists on Windows, and if that is set, reads the beginning of
the file, and if that reads "!<symlink>" (interpreted as ASCII), then the
rest of that system file is interpreted as the symlink target.)

So it makes me worry if you say that you had to exclude the SYMLINK
prereq. Maybe all the dependent tests have different prereqs that just so
happen *also* not to be met on Windows?

> 1. https://public-inbox.org/git/nycvar.QRO.7.76.6.1905131531000.44@tvgsbejvaqbjf.bet/
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>
> On Mon, May 13 2019, Johannes Schindelin wrote:
>
> > [...]
> > Namely, when test cases 51 and 52 are skipped because of a missing GPG
> > prerequisite [*1*], and those two are obviously required to run for the
> > `git merge to fail in your test case, as you can very easily verify by
> > downloading the artifact containing the `trash directory.t7600-merge`
> > directory and re-running the last steps on Linux (where the `git -c
> > rerere.enabled=true merge master` *succeeds*).
> >
> > In fact, you can very, very easily emulate the whole situation on your box
> > by running:
> >
> > 	sh t7600-merge.sh -i -v -x --run=1-50,53-59
> >
> > And then you can fix your test case so that it does not need to rely on
> > test cases that may, or may not, have run previously.
>
> I think it would be better to more pro-actively spot this sort of
> thing in the future, so here's a patch to do that. It passes on
> "master", but fails on "pu" due to the issue with the one test being
> discussed here.

It does drive me nuts that the `--run=<N>` option exists (thank you,
Slavica, for teaching me!) and is so poorly supported by our test suite.

For example, if t7600.59 fails, it would make a ton of sense to run

	sh t7600-merge.sh -i -v -x --run=59

right?

Except that we frequently have at least one "test case" whose only purpose
is to set things up.

But we never declare explicitly "test case 59 requires test case 51 to run
first".

We do not even declare the test cases: we execute them immediately. So we
would not even be able to juggle them about, e.g. run them in reverse
(which would otherwise be the easiest way to get rid of almost all side
effects).

I think in the long run, we'll have to drag Git's test suite into the 21st
century (kicking and screaming, I'm sure), to have a more declarative
style, with those features that one might know from Mocha, Jest, JUnit,
xUnit.NET, etc.

Back to your patch: it only catches prereq problems. But the `--run=59`
thing would still not be addressed.

What would you think about a mode where random test cases are skipped? It
would have to make sure to provide a way to recreate the problem, e.g.
giving a string that defines exactly which test cases were skipped.

I am *sure* that tons of test scripts would fail with that, and we would
probably have to special-case the `setup` "test cases", and we would have
to clean up quite a few scripts to *not* execute random stuff outside of
`test_expect_*`...

> diff --git a/t/README b/t/README
> index 6404f33e19..9747971d58 100644
> --- a/t/README
> +++ b/t/README
> @@ -334,6 +334,15 @@ that cannot be easily covered by a few specific test cases. These
>  could be enabled by running the test suite with correct GIT_TEST_
>  environment set.
>
> +GIT_TEST_FAIL_PREREQS<non-empty?> fails all prerequisites. This is

Did you mean to insert `=` after `GIT_TEST_FAIL_PREREQS`?

> diff --git a/t/t4202-log.sh b/t/t4202-log.sh
> index 819c24d10e..c20209324c 100755
> --- a/t/t4202-log.sh
> +++ b/t/t4202-log.sh
> @@ -352,7 +352,7 @@ test_expect_success 'log with grep.patternType configuration and command line' '
>  	test_cmp expect actual
>  '
>
> -test_expect_success 'log with various grep.patternType configurations & command-lines' '
> +test_expect_success !FAIL_PREREQS 'log with various grep.patternType configurations & command-lines' '

Is this an indication of a bug in this test case?

>  	git init pattern-type &&
>  	(
>  		cd pattern-type &&
> diff --git a/t/t7405-submodule-merge.sh b/t/t7405-submodule-merge.sh
> index 7855bd8648..aa33978ed2 100755
> --- a/t/t7405-submodule-merge.sh
> +++ b/t/t7405-submodule-merge.sh
> @@ -417,7 +417,7 @@ test_expect_failure 'directory/submodule conflict; keep submodule clean' '
>  	)
>  '
>
> -test_expect_failure 'directory/submodule conflict; should not treat submodule files as untracked or in the way' '
> +test_expect_failure !FAIL_PREREQS 'directory/submodule conflict; should not treat submodule files as untracked or in the way' '

Same here?

>  	test_when_finished "git -C directory-submodule/path reset --hard" &&
>  	test_when_finished "git -C directory-submodule reset --hard" &&
>  	(
> diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
> index 2e1bb61b41..7d7b396c23 100755
> --- a/t/t7810-grep.sh
> +++ b/t/t7810-grep.sh
> @@ -412,7 +412,7 @@ do
>  		test_cmp expected actual
>  	'
>
> -	test_expect_success !PCRE "grep $L with grep.patterntype=perl errors without PCRE" '
> +	test_expect_success !FAIL_PREREQS,!PCRE "grep $L with grep.patterntype=perl errors without PCRE" '

And here?

>  		test_must_fail git -c grep.patterntype=perl grep "foo.*bar"
>  	'
>
> @@ -1234,7 +1234,7 @@ test_expect_success PCRE 'grep --perl-regexp pattern' '
>  	test_cmp expected actual
>  '
>
> -test_expect_success !PCRE 'grep --perl-regexp pattern errors without PCRE' '
> +test_expect_success !FAIL_PREREQS,!PCRE 'grep --perl-regexp pattern errors without PCRE' '

And here?

>  	test_must_fail git grep --perl-regexp "foo.*bar"
>  '
>
> @@ -1249,7 +1249,7 @@ test_expect_success LIBPCRE2 "grep -P with (*NO_JIT) doesn't error out" '
>
>  '
>
> -test_expect_success !PCRE 'grep -P pattern errors without PCRE' '
> +test_expect_success !FAIL_PREREQS,!PCRE 'grep -P pattern errors without PCRE' '

And here?

>  	test_must_fail git grep -P "foo.*bar"
>  '
>
> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> index 8270de74be..0367cec5fd 100644
> --- a/t/test-lib-functions.sh
> +++ b/t/test-lib-functions.sh
> @@ -309,6 +309,26 @@ test_unset_prereq () {
>  }
>
>  test_set_prereq () {
> +	if test -n "$GIT_TEST_FAIL_PREREQS"
> +	then
> +		case "$1" in
> +		# The "!" case is handled below with
> +		# test_unset_prereq()
> +		!*)
> +			;;
> +		# (Temporary?) whitelist of things we can't easily
> +		# pretend not to support
> +		SYMLINKS)
> +			;;
> +		# Inspecting whether GIT_TEST_FAIL_PREREQS is on
> +		# should be unaffected.
> +		FAIL_PREREQS)
> +			;;
> +		*)
> +			return
> +		esac
> +	fi
> +

I would probably have done that on the reading side rather than the
writing side ;-)

Thanks for starting this!
Dscho

>  	case "$1" in
>  	!*)
>  		test_unset_prereq "${1#!}"
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 908ddb9c46..6fabafebb3 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -1607,3 +1607,7 @@ test_lazy_prereq SHA1 '
>  test_lazy_prereq REBASE_P '
>  	test -z "$GIT_TEST_SKIP_REBASE_P"
>  '
> +
> +test_lazy_prereq FAIL_PREREQS '
> +	test -n "$GIT_TEST_FAIL_PREREQS"
> +'
> --
> 2.21.0.1020.gf2820cf01a
>
>

Re: [PATCH] tests: add a special setup where prerequisites fail

[Ævar Arnfjörð Bjarmason]

On Tue, May 14 2019, Johannes Schindelin wrote:

> Hi Ævar,
>
> On Mon, 13 May 2019, Ævar Arnfjörð Bjarmason wrote:
>
>> As discussed in [1] there's a regression in the "pu" branch now
>> because a new test implicitly assumed that a previous test guarded by
>> a prerequisite had been run. Add a "GIT_TEST_FAIL_PREREQS" special
>> test setup where we'll skip (nearly) all tests guarded by
>> prerequisites, allowing us to easily emulate those platform where we
>> don't run these tests.
>>
>> As noted in the documentation I'm adding I'm whitelisting the SYMLINKS
>> prerequisite for now. A lot of tests started failing if we lied about
>> not supporting symlinks. It's also unlikely that we'll have a failing
>> test due to a hard dependency on symlinks without that being the
>> obvious cause, so for now it's not worth the effort to make it work.
>
> I don't know... In Git for Windows, the SYMLINKS prereq is not met.
>
> (Side note: Windows 10 already supports symlinks for quite some time, even
> for non-admin developers, but the fact that Git's test suite is
> implemented in shell script bites us yet one more time: MSYS2 has a
> completely different idea what symlinks are. It uses the "system file" bit
> that only exists on Windows, and if that is set, reads the beginning of
> the file, and if that reads "!<symlink>" (interpreted as ASCII), then the
> rest of that system file is interpreted as the symlink target.)
>
> So it makes me worry if you say that you had to exclude the SYMLINK
> prereq. Maybe all the dependent tests have different prereqs that just so
> happen *also* not to be met on Windows?

I should have clarified this. What I mean is that it leaves SYMLINK
alone, i.e. what was breaking is that on Linux we don't deal with
pretending that we don't support symlinks, these tests should still work
just fine on Windows (or anywhere SYMLINKS is genuinely false), since
there we actually back up our promise not to support them.

>> 1. https://public-inbox.org/git/nycvar.QRO.7.76.6.1905131531000.44@tvgsbejvaqbjf.bet/
>>
>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>> ---
>>
>> On Mon, May 13 2019, Johannes Schindelin wrote:
>>
>> > [...]
>> > Namely, when test cases 51 and 52 are skipped because of a missing GPG
>> > prerequisite [*1*], and those two are obviously required to run for the
>> > `git merge to fail in your test case, as you can very easily verify by
>> > downloading the artifact containing the `trash directory.t7600-merge`
>> > directory and re-running the last steps on Linux (where the `git -c
>> > rerere.enabled=true merge master` *succeeds*).
>> >
>> > In fact, you can very, very easily emulate the whole situation on your box
>> > by running:
>> >
>> > 	sh t7600-merge.sh -i -v -x --run=1-50,53-59
>> >
>> > And then you can fix your test case so that it does not need to rely on
>> > test cases that may, or may not, have run previously.
>>
>> I think it would be better to more pro-actively spot this sort of
>> thing in the future, so here's a patch to do that. It passes on
>> "master", but fails on "pu" due to the issue with the one test being
>> discussed here.
>
> It does drive me nuts that the `--run=<N>` option exists (thank you,
> Slavica, for teaching me!) and is so poorly supported by our test suite.
>
> For example, if t7600.59 fails, it would make a ton of sense to run
>
> 	sh t7600-merge.sh -i -v -x --run=59
>
> right?
>
> Except that we frequently have at least one "test case" whose only purpose
> is to set things up.
>
> But we never declare explicitly "test case 59 requires test case 51 to run
> first".
>
> We do not even declare the test cases: we execute them immediately. So we
> would not even be able to juggle them about, e.g. run them in reverse
> (which would otherwise be the easiest way to get rid of almost all side
> effects).
>
> I think in the long run, we'll have to drag Git's test suite into the 21st
> century (kicking and screaming, I'm sure), to have a more declarative
> style, with those features that one might know from Mocha, Jest, JUnit,
> xUnit.NET, etc.
>
> Back to your patch: it only catches prereq problems. But the `--run=59`
> thing would still not be addressed.

Yeah that would be a nice-to-have but unrelated thing (and a much bigger
task). We'd need to split up "this is setup code" v.s. "this is a test",
or make each individual test declare its dependency graph, which could
get quite verbose.

> What would you think about a mode where random test cases are skipped? It
> would have to make sure to provide a way to recreate the problem, e.g.
> giving a string that defines exactly which test cases were skipped.
>
> I am *sure* that tons of test scripts would fail with that, and we would
> probably have to special-case the `setup` "test cases", and we would have
> to clean up quite a few scripts to *not* execute random stuff outside of
> `test_expect_*`...

I think it would be neat, but unrelated to and overkill for spotting the
practical problem we have now, which is that we *know* we skip some of
this now on some platforms/setups due to prereqs.

>> diff --git a/t/README b/t/README
>> index 6404f33e19..9747971d58 100644
>> --- a/t/README
>> +++ b/t/README
>> @@ -334,6 +334,15 @@ that cannot be easily covered by a few specific test cases. These
>>  could be enabled by running the test suite with correct GIT_TEST_
>>  environment set.
>>
>> +GIT_TEST_FAIL_PREREQS<non-empty?> fails all prerequisites. This is
>
> Did you mean to insert `=` after `GIT_TEST_FAIL_PREREQS`?

Yeah, will fix.

>> diff --git a/t/t4202-log.sh b/t/t4202-log.sh
>> index 819c24d10e..c20209324c 100755
>> --- a/t/t4202-log.sh
>> +++ b/t/t4202-log.sh
>> @@ -352,7 +352,7 @@ test_expect_success 'log with grep.patternType configuration and command line' '
>>  	test_cmp expect actual
>>  '
>>
>> -test_expect_success 'log with various grep.patternType configurations & command-lines' '
>> +test_expect_success !FAIL_PREREQS 'log with various grep.patternType configurations & command-lines' '
>
> Is this an indication of a bug in this test case?
>
>>  	git init pattern-type &&
>>  	(
>>  		cd pattern-type &&
>> diff --git a/t/t7405-submodule-merge.sh b/t/t7405-submodule-merge.sh
>> index 7855bd8648..aa33978ed2 100755
>> --- a/t/t7405-submodule-merge.sh
>> +++ b/t/t7405-submodule-merge.sh
>> @@ -417,7 +417,7 @@ test_expect_failure 'directory/submodule conflict; keep submodule clean' '
>>  	)
>>  '
>>
>> -test_expect_failure 'directory/submodule conflict; should not treat submodule files as untracked or in the way' '
>> +test_expect_failure !FAIL_PREREQS 'directory/submodule conflict; should not treat submodule files as untracked or in the way' '
>
> Same here?
>
>>  	test_when_finished "git -C directory-submodule/path reset --hard" &&
>>  	test_when_finished "git -C directory-submodule reset --hard" &&
>>  	(
>> diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
>> index 2e1bb61b41..7d7b396c23 100755
>> --- a/t/t7810-grep.sh
>> +++ b/t/t7810-grep.sh
>> @@ -412,7 +412,7 @@ do
>>  		test_cmp expected actual
>>  	'
>>
>> -	test_expect_success !PCRE "grep $L with grep.patterntype=perl errors without PCRE" '
>> +	test_expect_success !FAIL_PREREQS,!PCRE "grep $L with grep.patterntype=perl errors without PCRE" '
>
> And here?
>
>>  		test_must_fail git -c grep.patterntype=perl grep "foo.*bar"
>>  	'
>>
>> @@ -1234,7 +1234,7 @@ test_expect_success PCRE 'grep --perl-regexp pattern' '
>>  	test_cmp expected actual
>>  '
>>
>> -test_expect_success !PCRE 'grep --perl-regexp pattern errors without PCRE' '
>> +test_expect_success !FAIL_PREREQS,!PCRE 'grep --perl-regexp pattern errors without PCRE' '
>
> And here?
>
>>  	test_must_fail git grep --perl-regexp "foo.*bar"
>>  '
>>
>> @@ -1249,7 +1249,7 @@ test_expect_success LIBPCRE2 "grep -P with (*NO_JIT) doesn't error out" '
>>
>>  '
>>
>> -test_expect_success !PCRE 'grep -P pattern errors without PCRE' '
>> +test_expect_success !FAIL_PREREQS,!PCRE 'grep -P pattern errors without PCRE' '
>
> And here?

To all of the above: Not a bug, I guess it could be refactored as an
unrelated fix, but it's tests of the form "without this prereq we should
die", so of course if I lie about not having it where I *do* have it it
won't die.

>>  	test_must_fail git grep -P "foo.*bar"
>>  '
>>
>> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
>> index 8270de74be..0367cec5fd 100644
>> --- a/t/test-lib-functions.sh
>> +++ b/t/test-lib-functions.sh
>> @@ -309,6 +309,26 @@ test_unset_prereq () {
>>  }
>>
>>  test_set_prereq () {
>> +	if test -n "$GIT_TEST_FAIL_PREREQS"
>> +	then
>> +		case "$1" in
>> +		# The "!" case is handled below with
>> +		# test_unset_prereq()
>> +		!*)
>> +			;;
>> +		# (Temporary?) whitelist of things we can't easily
>> +		# pretend not to support
>> +		SYMLINKS)
>> +			;;
>> +		# Inspecting whether GIT_TEST_FAIL_PREREQS is on
>> +		# should be unaffected.
>> +		FAIL_PREREQS)
>> +			;;
>> +		*)
>> +			return
>> +		esac
>> +	fi
>> +
>
> I would probably have done that on the reading side rather than the
> writing side ;-)

I started out by doing that (I'll note so in the v2 commit message), but
found it much harier, in that function we need to deal with the lazy
prereqs v.s. non-lazy, so I'd need to change multiple branches of that
or refactor it, all that stuff calls test_set_prereq() after it
discovers what the prereq is, so doing it here is much easier &
straightforward.

>>  	case "$1" in
>>  	!*)
>>  		test_unset_prereq "${1#!}"
>> diff --git a/t/test-lib.sh b/t/test-lib.sh
>> index 908ddb9c46..6fabafebb3 100644
>> --- a/t/test-lib.sh
>> +++ b/t/test-lib.sh
>> @@ -1607,3 +1607,7 @@ test_lazy_prereq SHA1 '
>>  test_lazy_prereq REBASE_P '
>>  	test -z "$GIT_TEST_SKIP_REBASE_P"
>>  '
>> +
>> +test_lazy_prereq FAIL_PREREQS '
>> +	test -n "$GIT_TEST_FAIL_PREREQS"
>> +'
>> --
>> 2.21.0.1020.gf2820cf01a
>>
>>

Re: [PATCH] tests: add a special setup where prerequisites fail

[Johannes Schindelin]

[-- Attachment #1: Type: text/plain, Size: 1288 bytes --]

Hi Ævar,

On Tue, 14 May 2019, Ævar Arnfjörð Bjarmason wrote:

> On Tue, May 14 2019, Johannes Schindelin wrote:
>
> > What would you think about a mode where random test cases are skipped?
> > It would have to make sure to provide a way to recreate the problem,
> > e.g. giving a string that defines exactly which test cases were
> > skipped.
> >
> > I am *sure* that tons of test scripts would fail with that, and we
> > would probably have to special-case the `setup` "test cases", and we
> > would have to clean up quite a few scripts to *not* execute random
> > stuff outside of `test_expect_*`...
>
> I think it would be neat, but unrelated to and overkill for spotting the
> practical problem we have now, which is that we *know* we skip some of
> this now on some platforms/setups due to prereqs.

I understand, but I am still worried that this is a lot of work for an
incomplete fix.

For example, the t7600-merge.sh test script that set off this conversation
has two prereqs that are unmet on Windows: GPG and EXECKEEPSPID. On Azure
Pipelines' macOS agents, it is only GPG that is unmet. So switching off
all prereqs would not help macOS with e.g. a bug where the GPG test cases
are skipped but the EXECKEEPSPID test case is not.

Ciao,
Dscho

Re: [PATCH] tests: add a special setup where prerequisites fail

[Ævar Arnfjörð Bjarmason]

On Tue, May 14 2019, Johannes Schindelin wrote:

> Hi Ævar,
>
> On Tue, 14 May 2019, Ævar Arnfjörð Bjarmason wrote:
>
>> On Tue, May 14 2019, Johannes Schindelin wrote:
>>
>> > What would you think about a mode where random test cases are skipped?
>> > It would have to make sure to provide a way to recreate the problem,
>> > e.g. giving a string that defines exactly which test cases were
>> > skipped.
>> >
>> > I am *sure* that tons of test scripts would fail with that, and we
>> > would probably have to special-case the `setup` "test cases", and we
>> > would have to clean up quite a few scripts to *not* execute random
>> > stuff outside of `test_expect_*`...
>>
>> I think it would be neat, but unrelated to and overkill for spotting the
>> practical problem we have now, which is that we *know* we skip some of
>> this now on some platforms/setups due to prereqs.
>
> I understand, but I am still worried that this is a lot of work for an
> incomplete fix.
>
> For example, the t7600-merge.sh test script that set off this conversation
> has two prereqs that are unmet on Windows: GPG and EXECKEEPSPID. On Azure
> Pipelines' macOS agents, it is only GPG that is unmet. So switching off
> all prereqs would not help macOS with e.g. a bug where the GPG test cases
> are skipped but the EXECKEEPSPID test case is not.

It won't catch such cases, but will catch cases where a later new test
assumes that whatever the state of the test repo it gets is what's
always going to be there. In practice I think that'll catch most such
issues.

The other GIT_TEST_* modes assume similar non-combinatorial explosion
failure scenarios.

I haven't gone back through the test suite's commit history to try to
dig for other cases, so perhaps this mode is premature etc.

Re: [PATCH] tests: add a special setup where prerequisites fail

[Johannes Schindelin]

[-- Attachment #1: Type: text/plain, Size: 1938 bytes --]

Hi Ævar,

On Tue, 14 May 2019, Ævar Arnfjörð Bjarmason wrote:

> On Tue, May 14 2019, Johannes Schindelin wrote:
>
> > On Tue, 14 May 2019, Ævar Arnfjörð Bjarmason wrote:
> >
> >> On Tue, May 14 2019, Johannes Schindelin wrote:
> >>
> >> > What would you think about a mode where random test cases are
> >> > skipped? It would have to make sure to provide a way to recreate
> >> > the problem, e.g. giving a string that defines exactly which test
> >> > cases were skipped.
> >> >
> >> > I am *sure* that tons of test scripts would fail with that, and we
> >> > would probably have to special-case the `setup` "test cases", and
> >> > we would have to clean up quite a few scripts to *not* execute
> >> > random stuff outside of `test_expect_*`...
> >>
> >> I think it would be neat, but unrelated to and overkill for spotting
> >> the practical problem we have now, which is that we *know* we skip
> >> some of this now on some platforms/setups due to prereqs.
> >
> > I understand, but I am still worried that this is a lot of work for an
> > incomplete fix.
> >
> > For example, the t7600-merge.sh test script that set off this
> > conversation has two prereqs that are unmet on Windows: GPG and
> > EXECKEEPSPID. On Azure Pipelines' macOS agents, it is only GPG that is
> > unmet. So switching off all prereqs would not help macOS with e.g. a
> > bug where the GPG test cases are skipped but the EXECKEEPSPID test
> > case is not.
>
> It won't catch such cases, but will catch cases where a later new test
> assumes that whatever the state of the test repo it gets is what's
> always going to be there. In practice I think that'll catch most such
> issues.

That's fair. It will also raise awareness of these issues, which should
also have a quite beneficial effect.

Given that your patch is not even large, I think you're right, it *is* a
lot of bang for the buck.

Ciao,
Dscho

[PATCH] tests: mark two failing tests under FAIL_PREREQS

[Ævar Arnfjörð Bjarmason]

Fix a couple of tests that would potentially fail under
GIT_TEST_FAIL_PREREQS=true.

I missed these when annotating other tests in dfe1a17df9 ("tests: add
a special setup where prerequisites fail", 2019-05-13) because on my
system I can only reproduce this failure when I run the tests as
"root", since the tests happen to depend on whether we can fall back
on GECOS info or not. I.e. they'd usually fail to look up the ident
info anyway, but not always.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/t0007-git-var.sh          | 2 +-
 t/t7502-commit-porcelain.sh | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t0007-git-var.sh b/t/t0007-git-var.sh
index 5868a87352..1f600e2cae 100755
--- a/t/t0007-git-var.sh
+++ b/t/t0007-git-var.sh
@@ -17,7 +17,7 @@ test_expect_success 'get GIT_COMMITTER_IDENT' '
 	test_cmp expect actual
 '
 
-test_expect_success !AUTOIDENT 'requested identites are strict' '
+test_expect_success !FAIL_PREREQS,!AUTOIDENT 'requested identites are strict' '
 	(
 		sane_unset GIT_COMMITTER_NAME &&
 		sane_unset GIT_COMMITTER_EMAIL &&
diff --git a/t/t7502-commit-porcelain.sh b/t/t7502-commit-porcelain.sh
index 5733d9cd34..14c92e4c25 100755
--- a/t/t7502-commit-porcelain.sh
+++ b/t/t7502-commit-porcelain.sh
@@ -402,7 +402,7 @@ echo editor started >"$(pwd)/.git/result"
 exit 0
 EOF
 
-test_expect_success !AUTOIDENT 'do not fire editor when committer is bogus' '
+test_expect_success !FAIL_PREREQS,!AUTOIDENT 'do not fire editor when committer is bogus' '
 	>.git/result &&
 
 	echo >>negative &&
-- 
2.22.0.455.g172b71a6c5

Re: [PATCH] tests: mark two failing tests under FAIL_PREREQS

[Johannes Schindelin]

[-- Attachment #1: Type: text/plain, Size: 2496 bytes --]

Hi Ævar,

On Thu, 20 Jun 2019, Ævar Arnfjörð Bjarmason wrote:

> Fix a couple of tests that would potentially fail under
> GIT_TEST_FAIL_PREREQS=true.
>
> I missed these when annotating other tests in dfe1a17df9 ("tests: add
> a special setup where prerequisites fail", 2019-05-13) because on my
> system I can only reproduce this failure when I run the tests as
> "root", since the tests happen to depend on whether we can fall back
> on GECOS info or not. I.e. they'd usually fail to look up the ident
> info anyway, but not always.

I had to read the commit message (in particular the oneline) a couple of
times, and I have to admit that I wish it was a bit clearer...

From the explanation, I would have assumed that those two test cases fail
often, anyway, so they shouldn't care whether `FAIL_PREREQS` is in effect.

The only reason why they should be exempt from the `FAIL_PREREQS` mode
that I can think of is that later test cases would depend on them, but how
can they? Those test cases would also have to have the `AUTOIDENT` prereq,
and they would be skipped under `FAIL_PREREQS`, too, no?

In other words, I struggle to understand why this patch is necessary.

Could you help me understand?

Ciao,
Dscho

>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  t/t0007-git-var.sh          | 2 +-
>  t/t7502-commit-porcelain.sh | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/t/t0007-git-var.sh b/t/t0007-git-var.sh
> index 5868a87352..1f600e2cae 100755
> --- a/t/t0007-git-var.sh
> +++ b/t/t0007-git-var.sh
> @@ -17,7 +17,7 @@ test_expect_success 'get GIT_COMMITTER_IDENT' '
>  	test_cmp expect actual
>  '
>
> -test_expect_success !AUTOIDENT 'requested identites are strict' '
> +test_expect_success !FAIL_PREREQS,!AUTOIDENT 'requested identites are strict' '
>  	(
>  		sane_unset GIT_COMMITTER_NAME &&
>  		sane_unset GIT_COMMITTER_EMAIL &&
> diff --git a/t/t7502-commit-porcelain.sh b/t/t7502-commit-porcelain.sh
> index 5733d9cd34..14c92e4c25 100755
> --- a/t/t7502-commit-porcelain.sh
> +++ b/t/t7502-commit-porcelain.sh
> @@ -402,7 +402,7 @@ echo editor started >"$(pwd)/.git/result"
>  exit 0
>  EOF
>
> -test_expect_success !AUTOIDENT 'do not fire editor when committer is bogus' '
> +test_expect_success !FAIL_PREREQS,!AUTOIDENT 'do not fire editor when committer is bogus' '
>  	>.git/result &&
>
>  	echo >>negative &&
> --
> 2.22.0.455.g172b71a6c5
>
>

Re: [PATCH] tests: mark two failing tests under FAIL_PREREQS

[Ævar Arnfjörð Bjarmason]

On Fri, Jun 21 2019, Johannes Schindelin wrote:

> Hi Ævar,
>
> On Thu, 20 Jun 2019, Ævar Arnfjörð Bjarmason wrote:
>
>> Fix a couple of tests that would potentially fail under
>> GIT_TEST_FAIL_PREREQS=true.
>>
>> I missed these when annotating other tests in dfe1a17df9 ("tests: add
>> a special setup where prerequisites fail", 2019-05-13) because on my
>> system I can only reproduce this failure when I run the tests as
>> "root", since the tests happen to depend on whether we can fall back
>> on GECOS info or not. I.e. they'd usually fail to look up the ident
>> info anyway, but not always.
>
> I had to read the commit message (in particular the oneline) a couple of
> times, and I have to admit that I wish it was a bit clearer...
>
> From the explanation, I would have assumed that those two test cases fail
> often, anyway, so they shouldn't care whether `FAIL_PREREQS` is in effect.
>
> The only reason why they should be exempt from the `FAIL_PREREQS` mode
> that I can think of is that later test cases would depend on them, but how
> can they? Those test cases would also have to have the `AUTOIDENT` prereq,
> and they would be skipped under `FAIL_PREREQS`, too, no?

The test doesn't depend on "AUTOIDENT", but "!AUTOIDENT", i.e. the
negated version. The effect of the FAIL_PREREQS mode is to set all
prereqs to false, and therefore "test_have_prereq AUTOIDENT" is false,
but "test_have_prereq !AUTOIDENT" is true.

So this test that would otherwise get skipped gets run.

I honestly didn't think much about these cases when I wrote dfe1a17df9
("tests: add a special setup where prerequisites fail", 2019-05-13), and
now I'm not quite sure whether it should be considered a bug or a
feature, but in the meantime this un-breaks the test suite under this
mode.

> In other words, I struggle to understand why this patch is necessary.
>
> Could you help me understand?
>
> Ciao,
> Dscho
>
>>
>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>> ---
>>  t/t0007-git-var.sh          | 2 +-
>>  t/t7502-commit-porcelain.sh | 2 +-
>>  2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/t/t0007-git-var.sh b/t/t0007-git-var.sh
>> index 5868a87352..1f600e2cae 100755
>> --- a/t/t0007-git-var.sh
>> +++ b/t/t0007-git-var.sh
>> @@ -17,7 +17,7 @@ test_expect_success 'get GIT_COMMITTER_IDENT' '
>>  	test_cmp expect actual
>>  '
>>
>> -test_expect_success !AUTOIDENT 'requested identites are strict' '
>> +test_expect_success !FAIL_PREREQS,!AUTOIDENT 'requested identites are strict' '
>>  	(
>>  		sane_unset GIT_COMMITTER_NAME &&
>>  		sane_unset GIT_COMMITTER_EMAIL &&
>> diff --git a/t/t7502-commit-porcelain.sh b/t/t7502-commit-porcelain.sh
>> index 5733d9cd34..14c92e4c25 100755
>> --- a/t/t7502-commit-porcelain.sh
>> +++ b/t/t7502-commit-porcelain.sh
>> @@ -402,7 +402,7 @@ echo editor started >"$(pwd)/.git/result"
>>  exit 0
>>  EOF
>>
>> -test_expect_success !AUTOIDENT 'do not fire editor when committer is bogus' '
>> +test_expect_success !FAIL_PREREQS,!AUTOIDENT 'do not fire editor when committer is bogus' '
>>  	>.git/result &&
>>
>>  	echo >>negative &&
>> --
>> 2.22.0.455.g172b71a6c5
>>
>>

Re: [PATCH] tests: mark two failing tests under FAIL_PREREQS

[Junio C Hamano]

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> ... The effect of the FAIL_PREREQS mode is to set all
> prereqs to false, and therefore "test_have_prereq AUTOIDENT" is false,
> but "test_have_prereq !AUTOIDENT" is true.
>
> So this test that would otherwise get skipped gets run.
>
> I honestly didn't think much about these cases when I wrote dfe1a17df9
> ("tests: add a special setup where prerequisites fail", 2019-05-13), and
> now I'm not quite sure whether it should be considered a bug or a
> feature, but in the meantime this un-breaks the test suite under this
> mode.

Yeah, reading the above alone, anybody's knee-jerk reaction would be
that fail-prereqs is buggy, but then I am not sure how we can "fix"
that, short of forbidding a prereq like this !AUTOIDENT one from the
test suite.  The test "predicts" how 'git' used in the test body
would behave, and skips it if we know that the predicted behaviour
breaks the test.  Forcing the prereq using FAIL_PREREQS without
actually making a matching change to the behaviour of 'git' has no
chance of producing sane results.

>>> diff --git a/t/t0007-git-var.sh b/t/t0007-git-var.sh
>>> index 5868a87352..1f600e2cae 100755
>>> --- a/t/t0007-git-var.sh
>>> +++ b/t/t0007-git-var.sh
>>> @@ -17,7 +17,7 @@ test_expect_success 'get GIT_COMMITTER_IDENT' '
>>>  	test_cmp expect actual
>>>  '
>>>
>>> -test_expect_success !AUTOIDENT 'requested identites are strict' '
>>> +test_expect_success !FAIL_PREREQS,!AUTOIDENT 'requested identites are strict' '
>>>  	(
>>>  		sane_unset GIT_COMMITTER_NAME &&
>>>  		sane_unset GIT_COMMITTER_EMAIL &&

For this particular one, given the test used to set/unset AUTOIDENT
lazily, I am not sure if it even makes sense to keep this test.  If
we break the underlying ident machinery that would be caught by this
particular test, it seems to me that it is very likely that the test
used to lazily set up AUTOIDENT prereq would fail the same way, so
I am not sure protecting it with !AUTOIDENT would still keep this
one useful.

>>> diff --git a/t/t7502-commit-porcelain.sh b/t/t7502-commit-porcelain.sh
>>> index 5733d9cd34..14c92e4c25 100755
>>> --- a/t/t7502-commit-porcelain.sh
>>> +++ b/t/t7502-commit-porcelain.sh
>>> @@ -402,7 +402,7 @@ echo editor started >"$(pwd)/.git/result"
>>>  exit 0
>>>  EOF
>>>
>>> -test_expect_success !AUTOIDENT 'do not fire editor when committer is bogus' '
>>> +test_expect_success !FAIL_PREREQS,!AUTOIDENT 'do not fire editor when committer is bogus' '
>>>  	>.git/result &&
>>>
>>>  	echo >>negative &&
>>> --
>>> 2.22.0.455.g172b71a6c5
>>>
>>>