What's cooking in git.git (May 2017, #01; Mon, 1)

What's cooking in git.git (May 2017, #01; Mon, 1)

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

2.13-rc1 was tagged and we are now in a pre-release freeze.  The
topics marked to be merged to 'next' in the list below may be merged
to 'next' as usual, but the ones marked for 'master' may stay in
'next' until the final release, unless they are small bug/doc fixes
or hotfixes to topics that are already in -rc1 from now on.

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

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

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

* ab/align-perf-descriptions (2017-04-23) 1 commit
  (merged to 'next' on 2017-04-26 at 7d66e7019e)
 + t/perf: correctly align non-ASCII descriptions in output

 Output from perf tests have been updated to align their titles.


* jk/complete-checkout-sans-dwim-remote (2017-04-23) 1 commit
  (merged to 'next' on 2017-04-26 at 0c69cff69f)
 + completion: optionally disable checkout DWIM

 Completion for "git checkout <branch>" that auto-creates the branch
 out of a remote tracking branch can now be disabled, as this
 completion often gets in the way when completing to checkout an
 existing local branch that happens to share the same prefix with
 bunch of remote tracking branches.


* jk/prio-queue-avoid-swap-with-self (2017-04-24) 1 commit
  (merged to 'next' on 2017-04-26 at 6a55996734)
 + prio_queue_reverse: don't swap elements with themselves

 Code clean-up.


* jk/submodule-init-segv-fix (2017-04-24) 1 commit
  (merged to 'next' on 2017-04-26 at 9a6eaaff89)
 + submodule_init: die cleanly on submodules without url defined

 Fix a segv in 'submodule init' when url is not given for a submodule.


* ls/travis-stricter-linux32-builds (2017-04-27) 1 commit
  (merged to 'next' on 2017-04-28 at b0a56aec83)
 + travis-ci: set DEVELOPER knob for Linux32 build

 32-bit Linux build on Travis CI uses stricter compilation options.


* ls/travis-win-fix-status (2017-04-26) 1 commit
  (merged to 'next' on 2017-04-28 at 5c5e4d3278)
 + travis-ci: printf $STATUS as string

 Relaying status from Windows build by Travis CI was done with an
 unsafe invocation of printf.


* sh/rebase-i-reread-todo-after-exec (2017-04-27) 1 commit
  (merged to 'next' on 2017-04-28 at 1ea7e62026)
 + rebase -i: reread the todo list if `exec` touched it

 "git rebase -i" failed to re-read the todo list file when the
 command specified with the `exec` instruction updated it.

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

* bw/submodule-with-bs-path (2017-05-01) 1 commit
 - t7400: add !CYGWIN prerequisite to 'add with \\ in path'

 A hotfix to a topic that is already in 'master'.

 Wait for a few days for Acks and then merge to 'master'.


* jk/am-leakfix (2017-04-27) 3 commits
  (merged to 'next' on 2017-04-30 at 78becd7a96)
 + am: shorten ident_split variable name in get_commit_info()
 + am: simplify allocations in get_commit_info()
 + am: fix commit buffer leak in get_commit_info()

 The codepath in "git am" that is used when running "git rebase"
 leaked memory held for the log message of the commits being rebased.

 Will cook in 'next'.


* js/plug-leaks (2017-04-27) 13 commits
 - checkout: fix memory leak
 - cat-file: fix memory leak
 - mailinfo & mailsplit: check for EOF while parsing mails
 - status: close file descriptor after reading git-rebase-todo
 - difftool: close file descriptors after reading
 - http-backend: avoid memory leaks
 - SQUASH???
 - get_mail_commit_oid(): avoid resource leak
 - git_config_rename_section_in_file(): avoid resource leak
 - add_commit_patch_id(): avoid allocating memory unnecessarily
 - winansi: avoid buffer overrun
 - winansi: avoid use of uninitialized value
 - mingw: avoid memory leak when splitting PATH

 An attempt to fix memory leaks pointed out by Coverity.

 A reroll exists and needs to be picked up.  What's queued here is
 just an early parts of the first iteration.


* sk/status-short-branch-color-config (2017-04-28) 2 commits
  (merged to 'next' on 2017-04-30 at 2a7eb05d7b)
 + status: add color config slots for branch info in "--short --branch"
 + status: fix missing newline when comment chars are disabled

 The colors in which "git status --short --branch" showed the names
 of the current branch and its remote-tracking branch are now
 configurable.

 Will cook in 'next'.


* jh/close-index-before-stat (2017-04-28) 1 commit
  (merged to 'next' on 2017-04-30 at 918d4f3378)
 + read-cache: close index.lock in do_write_index

 The timestamp of the index file is now taken after the file is
 closed, to help Windows, on which a stale timestamp is reported by
 fstat() on a file that is opened for writing and data was written
 but not yet closed.

 Will cook in 'next'.


* bw/submodule-has-commits-update (2017-05-01) 6 commits
 - submodule: refactor logic to determine changed submodules
 - submodule: improve submodule_has_commits
 - submodule: change string_list changed_submodule_paths
 - submodule: remove add_oid_to_argv()
 - submodule: rename free_submodules_sha1s()
 - submodule: rename add_sha1_to_array()


* ja/i18n-cleanup (2017-05-01) 2 commits
  (merged to 'next' on 2017-04-30 at 8002e53820)
 + i18n: read-cache: typofix
 + i18n: remove i18n from tag reflog message

 Will merge to 'master'.


* ls/travis-relays-for-windows-ci (2017-05-01) 2 commits
 - travis-ci: handle Git for Windows CI status "failed" explicitly
 - travis-ci: retry if Git for Windows CI returns HTTP error 502 or 503

 Will merge to 'next' after a few CI run in 'pu'.


* mb/diff-default-to-indent-heuristics (2017-05-01) 5 commits
 - add--interactive: drop diff.indentHeuristic handling
 - SQUASH
 - diff: enable indent heuristic by default
 - diff: have the diff-* builtins configure diff before initializing revisions
 - diff: make the indent heuristic part of diff's basic configuration

 Make the "indent" heuristics the default in "diff" and diff.indentHeuristics
 configuration variable an escape hatch for those who do no want it.

 The fix-up needs to be squashed in.


* rg/doc-pull-typofix (2017-05-01) 1 commit
  (merged to 'next' on 2017-04-30 at c2edb4d813)
 + doc: git-pull.txt use US spelling, fix minor typo

 Will merge to 'master'.


* rg/doc-submittingpatches-wordfix (2017-05-01) 1 commit
  (merged to 'next' on 2017-04-30 at 4065036588)
 + doc: update SubmittingPatches

 Will merge to 'master'.


* sr/hooks-cwd-doc (2017-05-01) 1 commit
  (merged to 'next' on 2017-04-30 at 063dd5cb02)
 + githooks.txt: clarify push hooks are always executed in $GIT_DIR

 Will merge to 'master'.


* tb/dedup-crlf-tests (2017-05-01) 1 commit
 - t0027: Some tests are not expensive

 Will be rerolled.

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

* ja/doc-l10n (2017-03-20) 3 commits
 - SQUASH???
 - l10n: add git-add.txt to localized man pages
 - l10n: introduce framework for localizing man pages

 A proposal to use po4a to localize our manual pages.


* sg/clone-refspec-from-command-line-config (2017-02-27) 1 commit
 - clone: respect configured fetch respecs during initial fetch

 Expecting a reroll.
 cf. <20170227211217.73gydlxb2qu2sp3m@sigill.intra.peff.net>
 cf. <CAM0VKj=rsAfKvVccOMOoo5==Q1yW1U0zJBbUV=faKppWFm-u+g@mail.gmail.com>
 Some nits but looks ok.


* sk/dash-is-previous (2017-03-01) 5 commits
 - revert.c: delegate handling of "-" shorthand to setup_revisions
 - sha1_name.c: teach get_sha1_1 "-" shorthand for "@{-1}"
 - revision.c: args starting with "-" might be a revision
 - revision.c: swap if/else blocks
 - revision.c: do not update argv with unknown option

 A dash "-" can be written to mean "the branch that was previously
 checked out" in more places.

 Needs review.
 cf. <1488007487-12965-1-git-send-email-kannan.siddharth12@gmail.com>


* ls/filter-process-delayed (2017-03-06) 1 commit
 - convert: add "status=delayed" to filter process protocol

 What's the status of this one???
 cf. <xmqq60jmnmef.fsf@junio-linux.mtv.corp.google.com>


* pb/bisect (2017-02-18) 28 commits
 . fixup! bisect--helper: `bisect_next_check` & bisect_voc shell function in C
 . bisect--helper: remove the dequote in bisect_start()
 . bisect--helper: retire `--bisect-auto-next` subcommand
 . bisect--helper: retire `--bisect-autostart` subcommand
 . bisect--helper: retire `--bisect-write` subcommand
 . bisect--helper: `bisect_replay` shell function in C
 . bisect--helper: `bisect_log` shell function in C
 . bisect--helper: retire `--write-terms` subcommand
 . bisect--helper: retire `--check-expected-revs` subcommand
 . bisect--helper: `bisect_state` & `bisect_head` shell function in C
 . bisect--helper: `bisect_autostart` shell function in C
 . bisect--helper: retire `--next-all` subcommand
 . bisect--helper: retire `--bisect-clean-state` subcommand
 . bisect--helper: `bisect_next` and `bisect_auto_next` shell function in C
 . t6030: no cleanup with bad merge base
 . bisect--helper: `bisect_start` shell function partially in C
 . bisect--helper: `get_terms` & `bisect_terms` shell function in C
 . bisect--helper: `bisect_next_check` & bisect_voc shell function in C
 . bisect--helper: `check_and_set_terms` shell function in C
 . bisect--helper: `bisect_write` shell function in C
 . bisect--helper: `is_expected_rev` & `check_expected_revs` shell function in C
 . bisect--helper: `bisect_reset` shell function in C
 . wrapper: move is_empty_file() and rename it as is_empty_or_missing_file()
 . t6030: explicitly test for bisection cleanup
 . bisect--helper: `bisect_clean_state` shell function in C
 . bisect--helper: `write_terms` shell function in C
 . bisect: rewrite `check_term_format` shell function in C
 . bisect--helper: use OPT_CMDMODE instead of OPT_BOOL

 Move more parts of "git bisect" to C.

 Ejected from 'pu', as it overlaps and gets in the way of bc/object-id topic.
 Expecting a reroll.
 cf. <CAFZEwPPXPPHi8KiEGS9ggzNHDCGhuqMgH9Z8-Pf9GLshg8+LPA@mail.gmail.com>
 cf. <CAFZEwPM9RSTGN54dzaw9gO9iZmsYjJ_d1SjUD4EzSDDbmh-XuA@mail.gmail.com>
 cf. <CAFZEwPNUXcNY9Qdz=_B7q2kQuaecPzJtTMGdv8YMUPEz2vnp8A@mail.gmail.com>


* sh/grep-tree-obj-tweak-output (2017-01-20) 2 commits
 - grep: use '/' delimiter for paths
 - grep: only add delimiter if there isn't one already

 "git grep", when fed a tree-ish as an input, shows each hit
 prefixed with "<tree-ish>:<path>:<lineno>:".  As <tree-ish> is
 almost always either a commit or a tag that points at a commit, the
 early part of the output "<tree-ish>:<path>" can be used as the
 name of the blob and given to "git show".  When <tree-ish> is a
 tree given in the extended SHA-1 syntax (e.g. "<commit>:", or
 "<commit>:<dir>"), however, this results in a string that does not
 name a blob (e.g. "<commit>::<path>" or "<commit>:<dir>:<path>").
 "git grep" has been taught to be a bit more intelligent about these
 cases and omit a colon (in the former case) or use slash (in the
 latter case) to produce "<commit>:<path>" and
 "<commit>:<dir>/<path>" that can be used as the name of a blob.

 Expecting a reroll?  Is this good enough with known limitations?


* jc/diff-b-m (2015-02-23) 5 commits
 . WIPWIP
 . WIP: diff-b-m
 - diffcore-rename: allow easier debugging
 - diffcore-rename.c: add locate_rename_src()
 - diffcore-break: allow debugging

 "git diff -B -M" produced incorrect patch when the postimage of a
 completely rewritten file is similar to the preimage of a removed
 file; such a resulting file must not be expressed as a rename from
 other place.

 The fix in this patch is broken, unfortunately.

 Will discard.

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

* jc/repack-threads (2017-04-27) 1 commit
 - repack: accept --threads=<n> and pass it down to pack-objects

 "git repack" learned to accept the --threads=<n> option and pass it
 to pack-objects.


* jh/verify-index-checksum-only-in-fsck (2017-04-27) 1 commit
  (merged to 'next' on 2017-04-28 at afb4c70061)
 + t1450: avoid use of "sed" on the index, which is a binary file

 Update an unportable constructin a new test.

 Will cook in 'next'.


* dt/gc-ignore-old-gc-logs (2017-04-24) 3 commits
  (merged to 'next' on 2017-04-26 at 4f4cab8368)
 + test-lib: retire $remove_trash variable
 + test-lib.sh: do not barf under --debug at the end of the test
 + test-lib: abort when can't remove trash directory

 An attempt to allow us notice "fishy" situation where we fail to
 remove the temporary directory used during the test.

 Will cook in 'next'.


* nd/fopen-errors (2017-04-23) 17 commits
 - warn_failure_to_open_read_optional_path(): a new helper
 - t1308: add a test case on open a config directory
 - config.c: handle error on failing to fopen()
 - xdiff-interface.c: report errno on failure to stat() or fopen()
 - wt-status.c: report error on failure to fopen()
 - server-info: report error on failure to fopen()
 - sequencer.c: report error on failure to fopen()
 - rerere.c: report correct errno
 - rerere.c: report error on failure to fopen()
 - remote.c: report error on failure to fopen()
 - commit.c: report error on failure to fopen() the graft file
 - log: report errno on failure to fopen() a file
 - clone: use xfopen() instead of fopen()
 - blame: report error on open if graft_file is a directory
 - bisect: report on fopen() error
 - config.mak.uname: set FREAD_READS_DIRECTORIES for Linux and FreeBSD
 - git_fopen: fix a sparse 'not declared' warning

 We often try to open a file for reading whose existence is
 optional, and silently ignore errors from open/fopen; report such
 errors if they are not due to missing files.

 Expecting a reroll that would be much simplified thanks to a higher
 level helper.

 cf. <xmqqk26e5swj.fsf@gitster.mtv.corp.google.com>
 cf. <CACsJy8D1LuH6qVp15MSkCM_oQphVUUK0r9SeKC5AzX+9Xi2dcw@mail.gmail.com>


* dt/raise-core-packed-git-limit (2017-04-20) 1 commit
  (merged to 'next' on 2017-04-26 at c72dd8c62f)
 + Increase core.packedGitLimit

 Raise the default packed-git limit value on larger platforms to
 avoid "git fetch" from a (recoverable) failure while "gc" is
 running in parallel.

 Will cook in 'next'.


* js/larger-timestamps (2017-04-27) 8 commits
  (merged to 'next' on 2017-04-28 at b56a0d38cd)
 + use uintmax_t for timestamps
 + date.c: abort if the system time cannot handle one of our timestamps
 + timestamp_t: a new data type for timestamps
 + PRItime: introduce a new "printf format" for timestamps
 + parse_timestamp(): specify explicitly where we parse timestamps
 + t0006 & t5000: skip "far in the future" test when time_t is too limited
 + t0006 & t5000: prepare for 64-bit timestamps
 + ref-filter: avoid using `unsigned long` for catch-all data type

 Some platforms have ulong that is smaller than time_t, and our
 historical use of ulong for timestamp would mean they cannot
 represent some timestamp that the platform allows.  Invent a
 separate and dedicated timestamp_t (so that we can distingiuish
 timestamps and a vanilla ulongs, which along is already a good
 move), and then declare uintmax_t is the type to be used as the
 timestamp_t

 Will cook in 'next'.


* ab/grep-pcre-v2 (2017-04-25) 20 commits
 - SQUASH???
 - Makefile & configure: make PCRE v2 the default PCRE implementation
 - grep: remove support for concurrent use of both PCRE v1 & v2
 - grep: add support for PCRE v2
 - grep: add support for the PCRE v1 JIT API
 - perf: add a performance comparison test of grep -E and -P
 - grep: change the internal PCRE code & header names to be PCRE1
 - grep: change the internal PCRE macro names to be PCRE1
 - test-lib: rename the LIBPCRE prerequisite to PCRE
 - grep: make grep.patternType=[pcre|pcre1] a synonym for "perl"
 - grep & rev-list doc: stop promising libpcre for --perl-regexp
 - log: add -P as a synonym for --perl-regexp
 - log: add exhaustive tests for pattern style options & config
 - grep: add a test for backreferences in PCRE patterns
 - Makefile & configure: reword outdated comment about PCRE
 - grep: remove redundant `regflags &= ~REG_EXTENDED` assignments
 - grep: remove redundant regflags assignment under PCRE
 - grep: submodule-related case statements should die if new fields are added
 - grep: add tests for grep pattern types being passed to submodules
 - grep: amend submodule recursion test in preparation for rx engine testing

 PCRE2, which has an API different from and incompatible with PCRE,
 can now be chosen to support "grep -P -e '<pattern>'" and friends.


* bc/object-id (2017-04-23) 53 commits
 - object: convert parse_object* to take struct object_id
 - tree: convert parse_tree_indirect to struct object_id
 - sequencer: convert do_recursive_merge to struct object_id
 - diff-lib: convert do_diff_cache to struct object_id
 - builtin/ls-tree: convert to struct object_id
 - merge: convert checkout_fast_forward to struct object_id
 - sequencer: convert fast_forward_to to struct object_id
 - builtin/ls-files: convert overlay_tree_on_cache to object_id
 - builtin/read-tree: convert to struct object_id
 - sha1_name: convert internals of peel_onion to object_id
 - upload-pack: convert remaining parse_object callers to object_id
 - revision: convert remaining parse_object callers to object_id
 - revision: rename add_pending_sha1 to add_pending_oid
 - http-push: convert process_ls_object and descendants to object_id
 - refs/files-backend: convert many internals to struct object_id
 - refs: convert struct ref_update to use struct object_id
 - ref-filter: convert some static functions to struct object_id
 - Convert struct ref_array_item to struct object_id
 - Convert the verify_pack callback to struct object_id
 - Convert lookup_tag to struct object_id
 - log-tree: convert to struct object_id
 - Convert lookup_tree to struct object_id
 - builtin/reflog: convert tree_is_complete to take struct object_id
 - tree: convert read_tree_1 to use struct object_id internally
 - Convert lookup_blob to struct object_id
 - Convert remaining callers of lookup_blob to object_id
 - builtin/unpack-objects: convert to struct object_id
 - pack: convert struct pack_idx_entry to struct object_id
 - Convert lookup_commit* to struct object_id
 - Convert remaining callers of lookup_commit_reference* to object_id
 - builtin/tag: convert to struct object_id
 - sequencer: convert some functions to struct object_id
 - shallow: convert shallow registration functions to object_id
 - revision: convert prepare_show_merge to struct object_id
 - notes-utils: convert internals to struct object_id
 - http-push: convert some static functions to struct object_id
 - tag: convert parse_tag_buffer to struct object_id
 - builtin/verify-commit: convert to struct object_id
 - reflog_expire: convert to struct object_id
 - parse-options-cb: convert to struct object_id
 - notes-cache: convert to struct object_id
 - submodule: convert merge_submodule to use struct object_id
 - fast-import: convert to struct object_id
 - fast-import: convert internal structs to struct object_id
 - builtin/rev-parse: convert to struct object_id
 - builtin/blame: convert static function to struct object_id
 - branch: convert to struct object_id
 - bundle: convert to struct object_id
 - builtin/prune: convert to struct object_id
 - builtin/name-rev: convert to struct object_id
 - Convert struct cache_tree to use struct object_id
 - Clean up outstanding object_id transforms.
 - fetch-pack: convert to struct object_id

 Conversion from uchar[20] to struct object_id continues.

 Reroll exists and needs to be picked up.


* jk/no-null-sha1-in-cache-tree (2017-04-23) 1 commit
  (merged to 'next' on 2017-04-26 at 45fbe9d57d)
 + cache-tree: reject entries with null sha1

 Tighten code to update cache-tree so that we won't accidentally
 write out any 0{40} entry in the tree object.

 Will cook in 'next'.


* rg/a-the-typo (2017-05-01) 1 commit
  (merged to 'next' on 2017-04-30 at a648cf2961)
 + fix minor typos

 Typofix.

 Will merge to 'master'.


* rs/large-zip (2017-05-01) 7 commits
 - t5004: require 64-bit support for big ZIP tests
 - archive-zip: set version field for big files correctly
  (merged to 'next' on 2017-04-26 at a6beab60f2)
 + archive-zip: support files bigger than 4GB
 + archive-zip: support archives bigger than 4GB
 + archive-zip: write ZIP dir entry directly to strbuf
 + archive-zip: use strbuf for ZIP directory
 + archive-zip: add tests for big ZIP archives

 "git archive --format=zip" learned to use zip64 extension when
 necessary to go beyond the 4GB limit.

 Will cook in 'next'.


* jc/checkout-working-tree-only (2017-04-27) 1 commit
 - checkout: add --working-tree-only option

 "git checkout <tree-ish> <pathspec>" learned a variant that does
 not update the index when doing its thing.


* js/rebase-i-final (2017-04-27) 9 commits
 - rebase -i: rearrange fixup/squash lines using the rebase--helper
 - t3415: test fixup with wrapped oneline
 - rebase -i: skip unnecessary picks using the rebase--helper
 - rebase -i: check for missing commits in the rebase--helper
 - t3404: relax rebase.missingCommitsCheck tests
 - rebase -i: also expand/collapse the SHA-1s via the rebase--helper
 - rebase -i: do not invent onelines when expanding/collapsing SHA-1s
 - rebase -i: remove useless indentation
 - rebase -i: generate the script via rebase--helper

 The final batch to "git rebase -i" updates to move more code from
 the shell script to C.

 Needs review.


* jt/use-trailer-api-in-commands (2017-04-26) 1 commit
  (merged to 'next' on 2017-04-30 at 006c8d7ebd)
 + sequencer: add newline before adding footers

 "git cherry-pick" and other uses of the sequencer machinery
 mishandled a trailer block whose last line is an incomplete line.
 This has been fixed so that an additional sign-off etc. are added
 after completing the existing incomplete line.

 Will cook in 'next'.


* nd/worktree-move (2017-04-20) 6 commits
 - worktree remove: new command
 - worktree move: refuse to move worktrees with submodules
 - worktree move: accept destination as directory
 - worktree move: new command
 - worktree.c: add update_worktree_location()
 - worktree.c: add validate_worktree()

 "git worktree" learned move and remove subcommands.

 Expecting a reroll.
 cf. <20170420101024.7593-1-pclouds@gmail.com>
 cf. <20170421145916.mknekgqzhxffu7di@sigill.intra.peff.net>
 cf. <d0e81b1e-5869-299e-f462-4d43dc997bd1@ramsayjones.plus.com>


* jk/parse-options-no-no-no (2017-04-25) 2 commits
 - SQUASH???
 - parse-options: disallow double-negations of options starting with no-

 Command line options that begin with "--no-" (e.g. "--no-checkout"
 option of "git clone") can be negated by removing "--no-"; we
 historically also allowed prefixing an extra "no" to the option
 (e.g. "--no-no-checkout"), which made the command line look ugly
 and unusual.  This proposes to forbid it.

 While I agree there is no need to support "--no-no-checkout", this
 looks more like "if it looks ugly and unusual, you do not have to
 use it".  Perhaps we can drop it?

 Will discard.


* df/dir-iter-remove-subtree (2017-04-19) 5 commits
 - remove_subtree(): reimplement using iterators
 - dir_iterator: rewrite state machine model
 - dir_iterator: refactor dir_iterator_advance
 - remove_subtree(): test removing nested directories
 - dir_iterator: add tests for dir_iterator API

 Update the dir-iterator API and use it to reimplement
 remove_subtree().

 A reroll exists that is based on the updated 'master', but I ran
 out of time trying to get it to work with other topics in flight
 in 'pu'.
 GSoC microproject.


* ab/clone-no-tags (2017-05-01) 3 commits
  (merged to 'next' on 2017-04-30 at 601649896a)
 + tests: rename a test having to do with shallow submodules
 + clone: add a --no-tags option to clone without tags
 + tests: change "cd ... && git fetch" to "cd &&\n\tgit fetch"

 "git clone" learned the "--no-tags" option not to fetch all tags
 initially, and also set up the tagopt not to follow any tags in
 subsequent fetches.

 Will cook in 'next'.


* bw/forking-and-threading (2017-04-25) 13 commits
  (merged to 'next' on 2017-04-26 at 89c495e37f)
 + run-command: restrict PATH search to executable files
 + run-command: expose is_executable function
  (merged to 'next' on 2017-04-23 at 7754b5ebc3)
 + run-command: block signals between fork and execve
 + run-command: add note about forking and threading
 + run-command: handle dup2 and close errors in child
 + run-command: eliminate calls to error handling functions in child
 + run-command: don't die in child when duping /dev/null
 + run-command: prepare child environment before forking
 + string-list: add string_list_remove function
 + run-command: use the async-signal-safe execv instead of execvp
 + run-command: prepare command before forking
 + t0061: run_command executes scripts without a #! line
 + t5550: use write_script to generate post-update hook

 The "run-command" API implementation has been made more robust
 against dead-locking in a threaded environment.

 Will cook in 'next'.


* sb/reset-recurse-submodules (2017-04-23) 4 commits
 - builtin/reset: add --recurse-submodules switch
 - submodule.c: submodule_move_head works with broken submodules
 - submodule.c: uninitialized submodules are ignored in recursive commands
 - entry.c: submodule recursing: respect force flag correctly

 "git reset" learned "--recurse-submodules" option.


* ls/travis-doc-asciidoctor (2017-04-26) 4 commits
 - travis-ci: check AsciiDoc/AsciiDoctor stderr output
  (merged to 'next' on 2017-04-19 at 359c32953b)
 + travis-ci: unset compiler for jobs that do not need one
 + travis-ci: parallelize documentation build
 + travis-ci: build documentation with AsciiDoc and Asciidoctor

 Have Travis CI format the documentation with both AsciiDoc and
 AsciiDoctor.

 Will merge to 'next' after a few CI run in 'pu'.


* mg/status-in-progress-info (2017-04-14) 1 commit
 - status: show in-progress info for short status

 "git status" learns an option to report various operations
 (e.g. "merging") that the user is in the middle of.

 It is still unclear if the participants decided that it is OK to
 spell "--inprogress" as a single word.


* ab/grep-threading-cleanup (2017-04-16) 8 commits
 - grep: given --threads with NO_PTHREADS=YesPlease, warn
 - pack-objects: fix buggy warning about threads under NO_PTHREADS=YesPlease
 - pack-object & index-pack: add test for --threads warning under NO_PTHREADS
 - tests: add a PTHREADS prerequisite
 - grep: skip pthreads overhead when using one thread
 - grep: don't redundantly compile throwaway patterns under threading
 - grep: add tests for --threads=N and grep.threads
 - grep: assert that threading is enabled when calling grep_{lock,unlock}

 Code cleanup.

 Needs review.


* bp/sub-process-convert-filter (2017-04-23) 8 commits
 - convert: update subprocess_read_status() to not die on EOF
 - sub-process: move sub-process functions into separate files
 - convert: rename reusable sub-process functions
 - convert: update generic functions to only use generic data structures
 - convert: separate generic structures and variables from the filter specific ones
 - convert: split start_multi_file_filter into two separate functions
 - convert: move packet_write_list() into pkt-line as packet_writel()
 - pkt-line: add packet_read_line_gently()

 Code from "conversion using external process" codepath has been
 extracted to a separate sub-process.[ch] module.

 Reroll exists but needs a bit more work.
 cf. <20170407120354.17736-1-benpeart@microsoft.com>


* mg/name-rev-debug (2017-03-31) 4 commits
 - describe: pass --debug down to name-rev
 - name-rev: provide debug output
 - name-rev: favor describing with tags and use committer date to tiebreak
 - name-rev: refactor logic to see if a new candidate is a better name

 "git describe --debug --contains" did not add any meaningful
 information, even though without "--contains" it did.

 Expecting a reroll of the tip two.
 cf. <xmqqshltxnwt.fsf@gitster.mtv.corp.google.com>


* nd/prune-in-worktree (2017-04-24) 12 commits
 - rev-list: expose and document --single-worktree
 - revision.c: --reflog add HEAD reflog from all worktrees
 - files-backend: make reflog iterator go through per-worktree reflog
 - revision.c: --all adds HEAD from all worktrees
 - refs: remove dead for_each_*_submodule()
 - revision.c: use refs_for_each*() instead of for_each_*_submodule()
 - refs: add refs_head_ref()
 - refs: move submodule slash stripping code to get_submodule_ref_store
 - refs.c: refactor get_submodule_ref_store(), share common free block
 - revision.c: --indexed-objects add objects from all worktrees
 - revision.c: refactor add_index_objects_to_pending()
 - revision.h: new flag in struct rev_info wrt. worktree-related refs
 (this branch uses nd/worktree-kill-parse-ref.)

 "git gc" and friends when multiple worktrees are used off of a
 single repository did not consider the index and per-worktree refs
 of other worktrees as the root for reachability traversal, making
 objects that are in use only in other worktrees to be subject to
 garbage collection.

 Expecting a reroll.
 Waiting for nd/worktree-kill-parse-ref to settle.
 cf. <CACsJy8ADCVBiLoPg_Tz0L6CMdh_eFmK4RYzfQ-PmUgBK7w9e=A@mail.gmail.com>


* nd/worktree-kill-parse-ref (2017-04-24) 6 commits
  (merged to 'next' on 2017-04-26 at b8e40da709)
 + refs: kill set_worktree_head_symref()
 + worktree.c: kill parse_ref() in favor of refs_resolve_ref_unsafe()
 + refs: introduce get_worktree_ref_store()
 + refs: add REFS_STORE_ALL_CAPS
 + refs.c: make submodule ref store hashmap generic
 + environment.c: fix potential segfault by get_git_common_dir()
 (this branch is used by nd/prune-in-worktree.)

 "git gc" did not interact well with "git worktree"-managed
 per-worktree refs.

 Will cook in 'next'.


* jc/bundle (2016-03-03) 6 commits
 - index-pack: --clone-bundle option
 - Merge branch 'jc/index-pack' into jc/bundle
 - bundle v3: the beginning
 - bundle: keep a copy of bundle file name in the in-core bundle header
 - bundle: plug resource leak
 - bundle doc: 'verify' is not about verifying the bundle

 The beginning of "split bundle", which could be one of the
 ingredients to allow "git clone" traffic off of the core server
 network to CDN.

Re: What's cooking in git.git (May 2017, #01; Mon, 1)

[Ævar Arnfjörð Bjarmason]

On Mon, May 1, 2017 at 7:35 AM, Junio C Hamano <gitster@pobox.com> wrote:
> * ab/clone-no-tags (2017-05-01) 3 commits
>   (merged to 'next' on 2017-04-30 at 601649896a)
>  + tests: rename a test having to do with shallow submodules
>  + clone: add a --no-tags option to clone without tags
>  + tests: change "cd ... && git fetch" to "cd &&\n\tgit fetch"
>
>  "git clone" learned the "--no-tags" option not to fetch all tags
>  initially, and also set up the tagopt not to follow any tags in
>  subsequent fetches.
>
>  Will cook in 'next'.

Thanks for trimming off the top 2 patches. I've dropped those myself,
if someone (Brandon || Stefan) more interested in working on
submodules wants to pick them up that would be neat, but I don't need
it myself & doing it differently than the existing submodule options
would take too much of my time.

> * ab/grep-pcre-v2 (2017-04-25) 20 commits
>  - SQUASH???
>  - Makefile & configure: make PCRE v2 the default PCRE implementation
>  - grep: remove support for concurrent use of both PCRE v1 & v2
>  - grep: add support for PCRE v2
>  - grep: add support for the PCRE v1 JIT API
>  - perf: add a performance comparison test of grep -E and -P
>  - grep: change the internal PCRE code & header names to be PCRE1
>  - grep: change the internal PCRE macro names to be PCRE1
>  - test-lib: rename the LIBPCRE prerequisite to PCRE
>  - grep: make grep.patternType=[pcre|pcre1] a synonym for "perl"
>  - grep & rev-list doc: stop promising libpcre for --perl-regexp
>  - log: add -P as a synonym for --perl-regexp
>  - log: add exhaustive tests for pattern style options & config
>  - grep: add a test for backreferences in PCRE patterns
>  - Makefile & configure: reword outdated comment about PCRE
>  - grep: remove redundant `regflags &= ~REG_EXTENDED` assignments
>  - grep: remove redundant regflags assignment under PCRE
>  - grep: submodule-related case statements should die if new fields are added
>  - grep: add tests for grep pattern types being passed to submodules
>  - grep: amend submodule recursion test in preparation for rx engine testing
>
>  PCRE2, which has an API different from and incompatible with PCRE,
>  can now be chosen to support "grep -P -e '<pattern>'" and friends.

That squash looks good to me.

> * ab/grep-threading-cleanup (2017-04-16) 8 commits
>  - grep: given --threads with NO_PTHREADS=YesPlease, warn
>  - pack-objects: fix buggy warning about threads under NO_PTHREADS=YesPlease
>  - pack-object & index-pack: add test for --threads warning under NO_PTHREADS
>  - tests: add a PTHREADS prerequisite
>  - grep: skip pthreads overhead when using one thread
>  - grep: don't redundantly compile throwaway patterns under threading
>  - grep: add tests for --threads=N and grep.threads
>  - grep: assert that threading is enabled when calling grep_{lock,unlock}
>
>  Code cleanup.
>
>  Needs review.

Between these two series there's 27 patches, and I understand it's a
bit of a PITA to review/get comments on it.

Anything I should be doing differently here other than just waiting
for 2.13 to come out so they can be cooked further & merged down to
next & then master if there's no objections?

Re: What's cooking in git.git (May 2017, #01; Mon, 1)

[Brandon Williams]

On 05/01, Ævar Arnfjörð Bjarmason wrote:
> On Mon, May 1, 2017 at 7:35 AM, Junio C Hamano <gitster@pobox.com> wrote:
> > * ab/clone-no-tags (2017-05-01) 3 commits
> >   (merged to 'next' on 2017-04-30 at 601649896a)
> >  + tests: rename a test having to do with shallow submodules
> >  + clone: add a --no-tags option to clone without tags
> >  + tests: change "cd ... && git fetch" to "cd &&\n\tgit fetch"
> >
> >  "git clone" learned the "--no-tags" option not to fetch all tags
> >  initially, and also set up the tagopt not to follow any tags in
> >  subsequent fetches.
> >
> >  Will cook in 'next'.
> 
> Thanks for trimming off the top 2 patches. I've dropped those myself,
> if someone (Brandon || Stefan) more interested in working on
> submodules wants to pick them up that would be neat, but I don't need
> it myself & doing it differently than the existing submodule options
> would take too much of my time.

Yeah we can add it to our backlog, though I'm not sure how quickly we'll
be able to get to it.  If you end up needing this sooner just let me
know.

-- 
Brandon Williams

Re: What's cooking in git.git (May 2017, #01; Mon, 1)

[Ævar Arnfjörð Bjarmason]

On Mon, May 1, 2017 at 6:21 PM, Brandon Williams <bmwill@google.com> wrote:
> On 05/01, Ęvar Arnfjörš Bjarmason wrote:
>> On Mon, May 1, 2017 at 7:35 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> > * ab/clone-no-tags (2017-05-01) 3 commits
>> >   (merged to 'next' on 2017-04-30 at 601649896a)
>> >  + tests: rename a test having to do with shallow submodules
>> >  + clone: add a --no-tags option to clone without tags
>> >  + tests: change "cd ... && git fetch" to "cd &&\n\tgit fetch"
>> >
>> >  "git clone" learned the "--no-tags" option not to fetch all tags
>> >  initially, and also set up the tagopt not to follow any tags in
>> >  subsequent fetches.
>> >
>> >  Will cook in 'next'.
>>
>> Thanks for trimming off the top 2 patches. I've dropped those myself,
>> if someone (Brandon || Stefan) more interested in working on
>> submodules wants to pick them up that would be neat, but I don't need
>> it myself & doing it differently than the existing submodule options
>> would take too much of my time.
>
> Yeah we can add it to our backlog, though I'm not sure how quickly we'll
> be able to get to it.  If you end up needing this sooner just let me
> know.

I don't need it at all, I just thought it was prudent to add the
submodule part of passing clone options / config when adding a new
option since there were some existing options that did that.

Since there's outstanding debate about the interface for that sort of
thing I'll just leave that to people who care more about submodules if
they'd like to pursue it.

Re: What's cooking in git.git (May 2017, #01; Mon, 1)

[Jeff King]

On Mon, May 01, 2017 at 04:25:14PM +0200, Ævar Arnfjörð Bjarmason wrote:

> > * ab/grep-pcre-v2 (2017-04-25) 20 commits
> [...]
> > * ab/grep-threading-cleanup (2017-04-16) 8 commits
> [...]
> >
> >  Needs review.
> 
> Between these two series there's 27 patches, and I understand it's a
> bit of a PITA to review/get comments on it.
> 
> Anything I should be doing differently here other than just waiting
> for 2.13 to come out so they can be cooked further & merged down to
> next & then master if there's no objections?

These are both on my todo list to review. That's not a promise of
timeliness, but at least they didn't get dropped into a void. :) If you
haven't heard anything post-2.13, I think reposting them then would be
reasonable.

-Peff

Re: What's cooking in git.git (May 2017, #01; Mon, 1)

[Junio C Hamano]

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

>> * ab/grep-pcre-v2 (2017-04-25) 20 commits
>>  - SQUASH???
>>  - Makefile & configure: make PCRE v2 the default PCRE implementation
>>  - grep: remove support for concurrent use of both PCRE v1 & v2
>>  - grep: add support for PCRE v2
>>  - grep: add support for the PCRE v1 JIT API
>>  - perf: add a performance comparison test of grep -E and -P
>>  - grep: change the internal PCRE code & header names to be PCRE1
>>  - grep: change the internal PCRE macro names to be PCRE1
>>  - test-lib: rename the LIBPCRE prerequisite to PCRE
>>  - grep: make grep.patternType=[pcre|pcre1] a synonym for "perl"
>>  - grep & rev-list doc: stop promising libpcre for --perl-regexp
>>  - log: add -P as a synonym for --perl-regexp
>>  - log: add exhaustive tests for pattern style options & config
>>  - grep: add a test for backreferences in PCRE patterns
>>  - Makefile & configure: reword outdated comment about PCRE
>>  - grep: remove redundant `regflags &= ~REG_EXTENDED` assignments
>>  - grep: remove redundant regflags assignment under PCRE
>>  - grep: submodule-related case statements should die if new fields are added
>>  - grep: add tests for grep pattern types being passed to submodules
>>  - grep: amend submodule recursion test in preparation for rx engine testing
>>
>>  PCRE2, which has an API different from and incompatible with PCRE,
>>  can now be chosen to support "grep -P -e '<pattern>'" and friends.
>
> That squash looks good to me.

Thanks.

That is not a particulary helpful comment, by the way.  I can help
topics by contributors by queuing emergency fix at the tip to make
ones that do not build correctly buildable and testable (which is
what the "SQUASH???" commits are about), but I'd rather not see me
forced to find among 19 commits which one is broken and needs the
hotfix squashed in myself.

>> * ab/grep-threading-cleanup (2017-04-16) 8 commits
>>  - grep: given --threads with NO_PTHREADS=YesPlease, warn
>>  - pack-objects: fix buggy warning about threads under NO_PTHREADS=YesPlease
>>  - pack-object & index-pack: add test for --threads warning under NO_PTHREADS
>>  - tests: add a PTHREADS prerequisite
>>  - grep: skip pthreads overhead when using one thread
>>  - grep: don't redundantly compile throwaway patterns under threading
>>  - grep: add tests for --threads=N and grep.threads
>>  - grep: assert that threading is enabled when calling grep_{lock,unlock}
>>
>>  Code cleanup.
>>
>>  Needs review.
>
> Between these two series there's 27 patches, and I understand it's a
> bit of a PITA to review/get comments on it.
>
> Anything I should be doing differently here other than just waiting
> for 2.13 to come out so they can be cooked further & merged down to
> next & then master if there's no objections?

There are topics that need fresh eyes to be reviewed by other
contributors, so perhaps you can help unblock them by reviewing,
while they pick lints from yours?

Re: What's cooking in git.git (May 2017, #01; Mon, 1)

[Ævar Arnfjörð Bjarmason]

On Tue, May 2, 2017 at 1:21 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>>> * ab/grep-pcre-v2 (2017-04-25) 20 commits
>>>  - SQUASH???
>>>  - Makefile & configure: make PCRE v2 the default PCRE implementation
>>>  - grep: remove support for concurrent use of both PCRE v1 & v2
>>>  - grep: add support for PCRE v2
>>>  - grep: add support for the PCRE v1 JIT API
>>>  - perf: add a performance comparison test of grep -E and -P
>>>  - grep: change the internal PCRE code & header names to be PCRE1
>>>  - grep: change the internal PCRE macro names to be PCRE1
>>>  - test-lib: rename the LIBPCRE prerequisite to PCRE
>>>  - grep: make grep.patternType=[pcre|pcre1] a synonym for "perl"
>>>  - grep & rev-list doc: stop promising libpcre for --perl-regexp
>>>  - log: add -P as a synonym for --perl-regexp
>>>  - log: add exhaustive tests for pattern style options & config
>>>  - grep: add a test for backreferences in PCRE patterns
>>>  - Makefile & configure: reword outdated comment about PCRE
>>>  - grep: remove redundant `regflags &= ~REG_EXTENDED` assignments
>>>  - grep: remove redundant regflags assignment under PCRE
>>>  - grep: submodule-related case statements should die if new fields are added
>>>  - grep: add tests for grep pattern types being passed to submodules
>>>  - grep: amend submodule recursion test in preparation for rx engine testing
>>>
>>>  PCRE2, which has an API different from and incompatible with PCRE,
>>>  can now be chosen to support "grep -P -e '<pattern>'" and friends.
>>
>> That squash looks good to me.
>
> Thanks.
>
> That is not a particulary helpful comment, by the way.  I can help
> topics by contributors by queuing emergency fix at the tip to make
> ones that do not build correctly buildable and testable (which is
> what the "SQUASH???" commits are about), but I'd rather not see me
> forced to find among 19 commits which one is broken and needs the
> hotfix squashed in myself.

I'm happy to change what I'm doing to be more helpful, but it's not
clear to me from this & the context what that would be.

* I sent a v4 that had this bug in <20170425210548.24612-6-avarab@gmail.com>
* You pointed out that initialization bug in response
* I sent a v5 of just that patch (not the rest of the series) in
response to that in <20170426074856.29903-1-avarab@gmail.com>
* You replied in <xmqq1sser7ty.fsf@gitster.mtv.corp.google.com> in a
reply I (probably mis-)read as "no worries, I'll just squash the fix
in"

So now a ~week later in WCIG you've taken the series but added that
squash instead of using my v5 of that one patch, that looks good to me
(i.e. your hotfix does the same thing as my v5) but you don't think
that's a helpful comment.

So what would you like to have happen instead? If it's easier I could
just re-sent a v6 of the whole thing and we could do away with this
squash/replace-one-patch dance.

>>> * ab/grep-threading-cleanup (2017-04-16) 8 commits
>>>  - grep: given --threads with NO_PTHREADS=YesPlease, warn
>>>  - pack-objects: fix buggy warning about threads under NO_PTHREADS=YesPlease
>>>  - pack-object & index-pack: add test for --threads warning under NO_PTHREADS
>>>  - tests: add a PTHREADS prerequisite
>>>  - grep: skip pthreads overhead when using one thread
>>>  - grep: don't redundantly compile throwaway patterns under threading
>>>  - grep: add tests for --threads=N and grep.threads
>>>  - grep: assert that threading is enabled when calling grep_{lock,unlock}
>>>
>>>  Code cleanup.
>>>
>>>  Needs review.
>>
>> Between these two series there's 27 patches, and I understand it's a
>> bit of a PITA to review/get comments on it.
>>
>> Anything I should be doing differently here other than just waiting
>> for 2.13 to come out so they can be cooked further & merged down to
>> next & then master if there's no objections?
>
> There are topics that need fresh eyes to be reviewed by other
> contributors, so perhaps you can help unblock them by reviewing,
> while they pick lints from yours?

*nod*

Re: What's cooking in git.git (May 2017, #01; Mon, 1)

[Junio C Hamano]

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

>>> That squash looks good to me.
>>
>> Thanks.
>>
>> That is not a particulary helpful comment, by the way.  I can help
>> topics by contributors by queuing emergency fix at the tip to make
>> ones that do not build correctly buildable and testable (which is
>> what the "SQUASH???" commits are about), but I'd rather not see me
>> forced to find among 19 commits which one is broken and needs the
>> hotfix squashed in myself.
>
> I'm happy to change what I'm doing to be more helpful, but it's not
> clear to me from this & the context what that would be.
>
> * I sent a v4 that had this bug in <20170425210548.24612-6-avarab@gmail.com>
> * You pointed out that initialization bug in response
> * I sent a v5 of just that patch (not the rest of the series) in
> response to that in <20170426074856.29903-1-avarab@gmail.com>
> * You replied in <xmqq1sser7ty.fsf@gitster.mtv.corp.google.com> in a
> reply I (probably mis-)read as "no worries, I'll just squash the fix
> in"

Sorry, I completely forgot about our exchange around your v5.  If
your comment were "squash is good but you've seen a replacement sent
as v5 that is not there yet", I wouldn't have made such a silly
comment, but given that I've already responded to your v5 saying
I'll handle it, that is asking too much from you.

What I pushed out a few hours ago should already have the fix in.
Thanks for clarifying the situation, and sorry again.

Re: What's cooking in git.git (May 2017, #01; Mon, 1)

[Ævar Arnfjörð Bjarmason]

On Tue, May 2, 2017 at 11:35 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>>>> That squash looks good to me.
>>>
>>> Thanks.
>>>
>>> That is not a particulary helpful comment, by the way.  I can help
>>> topics by contributors by queuing emergency fix at the tip to make
>>> ones that do not build correctly buildable and testable (which is
>>> what the "SQUASH???" commits are about), but I'd rather not see me
>>> forced to find among 19 commits which one is broken and needs the
>>> hotfix squashed in myself.
>>
>> I'm happy to change what I'm doing to be more helpful, but it's not
>> clear to me from this & the context what that would be.
>>
>> * I sent a v4 that had this bug in <20170425210548.24612-6-avarab@gmail.com>
>> * You pointed out that initialization bug in response
>> * I sent a v5 of just that patch (not the rest of the series) in
>> response to that in <20170426074856.29903-1-avarab@gmail.com>
>> * You replied in <xmqq1sser7ty.fsf@gitster.mtv.corp.google.com> in a
>> reply I (probably mis-)read as "no worries, I'll just squash the fix
>> in"
>
> Sorry, I completely forgot about our exchange around your v5.  If
> your comment were "squash is good but you've seen a replacement sent
> as v5 that is not there yet", I wouldn't have made such a silly
> comment, but given that I've already responded to your v5 saying
> I'll handle it, that is asking too much from you.
>
> What I pushed out a few hours ago should already have the fix in.
> Thanks for clarifying the situation, and sorry again.

Okey, no worries. Was just a bit confused & wondering if I could make
something easier for you in the future.

Re: What's cooking in git.git (May 2017, #01; Mon, 1)

[Johannes Schindelin]

Hi Junio,

On Sun, 30 Apr 2017, Junio C Hamano wrote:

> * js/rebase-i-final (2017-04-27) 9 commits
>  - rebase -i: rearrange fixup/squash lines using the rebase--helper
>  - t3415: test fixup with wrapped oneline
>  - rebase -i: skip unnecessary picks using the rebase--helper
>  - rebase -i: check for missing commits in the rebase--helper
>  - t3404: relax rebase.missingCommitsCheck tests
>  - rebase -i: also expand/collapse the SHA-1s via the rebase--helper
>  - rebase -i: do not invent onelines when expanding/collapsing SHA-1s
>  - rebase -i: remove useless indentation
>  - rebase -i: generate the script via rebase--helper
> 
>  The final batch to "git rebase -i" updates to move more code from
>  the shell script to C.

This is not the final batch, hopefully. It is the last batch of my
original work to accelerate rebase -i, and it is a blocker for more work
toward making rebase -i a builtin. Once this patch series is in, I plan on
doing more work on that front.

Ciao,
Dscho

PCRE v2 compile error, was Re: What's cooking in git.git (May 2017, #01; Mon, 1)

[Johannes Schindelin]

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

Hi Ævar,

On Sun, 30 Apr 2017, Junio C Hamano wrote:

> * ab/grep-pcre-v2 (2017-04-25) 20 commits
>  - SQUASH???
>  - Makefile & configure: make PCRE v2 the default PCRE implementation
>  - grep: remove support for concurrent use of both PCRE v1 & v2
>  - grep: add support for PCRE v2
>  - grep: add support for the PCRE v1 JIT API
>  - perf: add a performance comparison test of grep -E and -P
>  - grep: change the internal PCRE code & header names to be PCRE1
>  - grep: change the internal PCRE macro names to be PCRE1
>  - test-lib: rename the LIBPCRE prerequisite to PCRE
>  - grep: make grep.patternType=[pcre|pcre1] a synonym for "perl"
>  - grep & rev-list doc: stop promising libpcre for --perl-regexp
>  - log: add -P as a synonym for --perl-regexp
>  - log: add exhaustive tests for pattern style options & config
>  - grep: add a test for backreferences in PCRE patterns
>  - Makefile & configure: reword outdated comment about PCRE
>  - grep: remove redundant `regflags &= ~REG_EXTENDED` assignments
>  - grep: remove redundant regflags assignment under PCRE
>  - grep: submodule-related case statements should die if new fields are added
>  - grep: add tests for grep pattern types being passed to submodules
>  - grep: amend submodule recursion test in preparation for rx engine testing
> 
>  PCRE2, which has an API different from and incompatible with PCRE,
>  can now be chosen to support "grep -P -e '<pattern>'" and friends.

FWIW for quite a couple of recent builds, `pu` fails on Windows with a
variation of this error:

	    CC blob.o
	In file included from revision.h:5:0,
			 from bisect.c:4:
	grep.h:16:19: fatal error: pcre2.h: No such file or directory
	 #include <pcre2.h>
			   ^
	compilation terminated.

Maybe this can be fixed before hitting `next`?

Ciao,
Dscho

Re: PCRE v2 compile error, was Re: What's cooking in git.git (May 2017, #01; Mon, 1)

[Ævar Arnfjörð Bjarmason]

On Tue, May 2, 2017 at 2:09 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi Ævar,
>
> On Sun, 30 Apr 2017, Junio C Hamano wrote:
>
>> * ab/grep-pcre-v2 (2017-04-25) 20 commits
>>  - SQUASH???
>>  - Makefile & configure: make PCRE v2 the default PCRE implementation
>>  - grep: remove support for concurrent use of both PCRE v1 & v2
>>  - grep: add support for PCRE v2
>>  - grep: add support for the PCRE v1 JIT API
>>  - perf: add a performance comparison test of grep -E and -P
>>  - grep: change the internal PCRE code & header names to be PCRE1
>>  - grep: change the internal PCRE macro names to be PCRE1
>>  - test-lib: rename the LIBPCRE prerequisite to PCRE
>>  - grep: make grep.patternType=[pcre|pcre1] a synonym for "perl"
>>  - grep & rev-list doc: stop promising libpcre for --perl-regexp
>>  - log: add -P as a synonym for --perl-regexp
>>  - log: add exhaustive tests for pattern style options & config
>>  - grep: add a test for backreferences in PCRE patterns
>>  - Makefile & configure: reword outdated comment about PCRE
>>  - grep: remove redundant `regflags &= ~REG_EXTENDED` assignments
>>  - grep: remove redundant regflags assignment under PCRE
>>  - grep: submodule-related case statements should die if new fields are added
>>  - grep: add tests for grep pattern types being passed to submodules
>>  - grep: amend submodule recursion test in preparation for rx engine testing
>>
>>  PCRE2, which has an API different from and incompatible with PCRE,
>>  can now be chosen to support "grep -P -e '<pattern>'" and friends.
>
> FWIW for quite a couple of recent builds, `pu` fails on Windows with a
> variation of this error:
>
>             CC blob.o
>         In file included from revision.h:5:0,
>                          from bisect.c:4:
>         grep.h:16:19: fatal error: pcre2.h: No such file or directory
>          #include <pcre2.h>
>                            ^
>         compilation terminated.
>
> Maybe this can be fixed before hitting `next`?

This will be due to a combination of the build machine not having pcre
v2 (but having v1) & my "Makefile & configure: make PCRE v2 the
default PCRE implementation" patch, which makes v2 the default for
USE_LIBPCRE=YesPlease.

Is it easy to install v2 on these build machines? Alternatively that
patch could be ejected out of pu, or you could USE_LIBPCRE1=YesPlease
to use v1, but as explained in that commit I think it makes sense to
make v2 the default.

Re: PCRE v2 compile error, was Re: What's cooking in git.git (May 2017, #01; Mon, 1)

[Johannes Schindelin]

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

Hi Ævar,

On Tue, 2 May 2017, Ævar Arnfjörð Bjarmason wrote:

> On Tue, May 2, 2017 at 2:09 PM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> >
> > On Sun, 30 Apr 2017, Junio C Hamano wrote:
> >
> >> * ab/grep-pcre-v2 (2017-04-25) 20 commits
> >>  - SQUASH???
> >>  - Makefile & configure: make PCRE v2 the default PCRE implementation
> >>  - grep: remove support for concurrent use of both PCRE v1 & v2
> >>  - grep: add support for PCRE v2
> >>  - grep: add support for the PCRE v1 JIT API
> >>  - perf: add a performance comparison test of grep -E and -P
> >>  - grep: change the internal PCRE code & header names to be PCRE1
> >>  - grep: change the internal PCRE macro names to be PCRE1
> >>  - test-lib: rename the LIBPCRE prerequisite to PCRE
> >>  - grep: make grep.patternType=[pcre|pcre1] a synonym for "perl"
> >>  - grep & rev-list doc: stop promising libpcre for --perl-regexp
> >>  - log: add -P as a synonym for --perl-regexp
> >>  - log: add exhaustive tests for pattern style options & config
> >>  - grep: add a test for backreferences in PCRE patterns
> >>  - Makefile & configure: reword outdated comment about PCRE
> >>  - grep: remove redundant `regflags &= ~REG_EXTENDED` assignments
> >>  - grep: remove redundant regflags assignment under PCRE
> >>  - grep: submodule-related case statements should die if new fields are added
> >>  - grep: add tests for grep pattern types being passed to submodules
> >>  - grep: amend submodule recursion test in preparation for rx engine testing
> >>
> >>  PCRE2, which has an API different from and incompatible with PCRE,
> >>  can now be chosen to support "grep -P -e '<pattern>'" and friends.
> >
> > FWIW for quite a couple of recent builds, `pu` fails on Windows with a
> > variation of this error:
> >
> >             CC blob.o
> >         In file included from revision.h:5:0,
> >                          from bisect.c:4:
> >         grep.h:16:19: fatal error: pcre2.h: No such file or directory
> >          #include <pcre2.h>
> >                            ^
> >         compilation terminated.
> >
> > Maybe this can be fixed before hitting `next`?
> 
> This will be due to a combination of the build machine not having pcre
> v2 (but having v1) & my "Makefile & configure: make PCRE v2 the
> default PCRE implementation" patch, which makes v2 the default for
> USE_LIBPCRE=YesPlease.
> 
> Is it easy to install v2 on these build machines? Alternatively that
> patch could be ejected out of pu, or you could USE_LIBPCRE1=YesPlease
> to use v1, but as explained in that commit I think it makes sense to
> make v2 the default.

Let me put it this way: Installing PCRE v1 in MSYS2 is as easy as

	pacman -Sy mingw-w64-x86_64-pcre

To install PCRE v2, you would have to copy-edit
https://github.com/Alexpux/MINGW-packages/blob/master/mingw-w64-pcre/PKGBUILD,
make sure that it builds by running it through

	makepkg-mingw -s

possibly initializing a Git repository in the
mingw-w64-pcre/src/${_realname}-${pkgver}/ directory, patching the source
until it builds, committing the changes, adding them as patch files to the
same directory as the new PKGBUILD, adjusting the PKGBUILD file to list
the patch files with their checksums and to add the commands to apply
them.

Then (and this is a one time cost, fortunately) initializing two packages
on BinTray (which we use to serve the Pacman packages of Git for Windows),
then build and upload the packages.

In short, PCRE v2 would be slightly (ahem, ahem) more involved than PCRE
v1.

I cannot imagine that MSYS2 is the only environment with that issue.

Ciao,
Dscho

Re: PCRE v2 compile error, was Re: What's cooking in git.git (May 2017, #01; Mon, 1)

[Ævar Arnfjörð Bjarmason]

On Tue, May 2, 2017 at 6:05 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi Ævar,
>
> On Tue, 2 May 2017, Ævar Arnfjörð Bjarmason wrote:
>
>> On Tue, May 2, 2017 at 2:09 PM, Johannes Schindelin
>> <Johannes.Schindelin@gmx.de> wrote:
>> >
>> > On Sun, 30 Apr 2017, Junio C Hamano wrote:
>> >
>> >> * ab/grep-pcre-v2 (2017-04-25) 20 commits
>> >>  - SQUASH???
>> >>  - Makefile & configure: make PCRE v2 the default PCRE implementation
>> >>  - grep: remove support for concurrent use of both PCRE v1 & v2
>> >>  - grep: add support for PCRE v2
>> >>  - grep: add support for the PCRE v1 JIT API
>> >>  - perf: add a performance comparison test of grep -E and -P
>> >>  - grep: change the internal PCRE code & header names to be PCRE1
>> >>  - grep: change the internal PCRE macro names to be PCRE1
>> >>  - test-lib: rename the LIBPCRE prerequisite to PCRE
>> >>  - grep: make grep.patternType=[pcre|pcre1] a synonym for "perl"
>> >>  - grep & rev-list doc: stop promising libpcre for --perl-regexp
>> >>  - log: add -P as a synonym for --perl-regexp
>> >>  - log: add exhaustive tests for pattern style options & config
>> >>  - grep: add a test for backreferences in PCRE patterns
>> >>  - Makefile & configure: reword outdated comment about PCRE
>> >>  - grep: remove redundant `regflags &= ~REG_EXTENDED` assignments
>> >>  - grep: remove redundant regflags assignment under PCRE
>> >>  - grep: submodule-related case statements should die if new fields are added
>> >>  - grep: add tests for grep pattern types being passed to submodules
>> >>  - grep: amend submodule recursion test in preparation for rx engine testing
>> >>
>> >>  PCRE2, which has an API different from and incompatible with PCRE,
>> >>  can now be chosen to support "grep -P -e '<pattern>'" and friends.
>> >
>> > FWIW for quite a couple of recent builds, `pu` fails on Windows with a
>> > variation of this error:
>> >
>> >             CC blob.o
>> >         In file included from revision.h:5:0,
>> >                          from bisect.c:4:
>> >         grep.h:16:19: fatal error: pcre2.h: No such file or directory
>> >          #include <pcre2.h>
>> >                            ^
>> >         compilation terminated.
>> >
>> > Maybe this can be fixed before hitting `next`?
>>
>> This will be due to a combination of the build machine not having pcre
>> v2 (but having v1) & my "Makefile & configure: make PCRE v2 the
>> default PCRE implementation" patch, which makes v2 the default for
>> USE_LIBPCRE=YesPlease.
>>
>> Is it easy to install v2 on these build machines? Alternatively that
>> patch could be ejected out of pu, or you could USE_LIBPCRE1=YesPlease
>> to use v1, but as explained in that commit I think it makes sense to
>> make v2 the default.
>
> Let me put it this way: Installing PCRE v1 in MSYS2 is as easy as
>
>         pacman -Sy mingw-w64-x86_64-pcre
>
> To install PCRE v2, you would have to copy-edit
> https://github.com/Alexpux/MINGW-packages/blob/master/mingw-w64-pcre/PKGBUILD,
> make sure that it builds by running it through
>
>         makepkg-mingw -s
>
> possibly initializing a Git repository in the
> mingw-w64-pcre/src/${_realname}-${pkgver}/ directory, patching the source
> until it builds, committing the changes, adding them as patch files to the
> same directory as the new PKGBUILD, adjusting the PKGBUILD file to list
> the patch files with their checksums and to add the commands to apply
> them.
>
> Then (and this is a one time cost, fortunately) initializing two packages
> on BinTray (which we use to serve the Pacman packages of Git for Windows),
> then build and upload the packages.
>
> In short, PCRE v2 would be slightly (ahem, ahem) more involved than PCRE
> v1.
>
> I cannot imagine that MSYS2 is the only environment with that issue.

I think it's worth it to copy/paste the commit message where I made
this change, since we're having this discussion in this thread:

    Makefile & configure: make PCRE v2 the default PCRE implementation

    Change the USE_LIBPCRE=YesPlease & --with-libpcre flags to the
    Makefile & configure script, respectively, to mean use PCRE v2, not
    PCRE v1.

    The legacy library previously available via those options is still
    available on request via USE_LIBPCRE1=YesPlease or
    --with-libpcre1. The existing USE_LIBPCRE2=YesPlease & --with-libpcre2
    still explicitly ask for v2.

    The v2 PCRE is stable & end-user compatible, all this change does is
    change the default. Someone building a new git is likely to also have
    packaged PCRE v2 sometime in the last 2 years since it was released.
    If not they can choose to use the legacy v2 library by making the
    trivial s/USE_LIBPCRE/USE_LIBPCRE1/ change, or package up PCRE v2.

    New releases of PCRE v2 are already faster than PCRE v1, and not only
    is all significant development is happening on v2, but bugs in
    reported against v1 have started getting WONTFIX'd asking users to
    just upgrade to v2.

    So it makes sense to give our downstream distributors a nudge to
    switch over to it.

So that's why I did it. I also looked at several Linux distros & BSDs
/ Solaris and they have it packaged, although some not for their
current stable release but for their "next" release, but that's likely
what they're going to be building a new git for.

But, problems with this:

 * Debian testing has ~600 projects that rdepend on pcre1, with ~15
that rdepend on pcre2. Git would definitely be the most prominent
project using pcre2, it seems 2 years after release few have bothered
to switch over.

  Thus even though it came out 2 years ago it's likely that a few
platforms like MSYS2 simply haven't bothered to package it yet
(although packaging it is trivial if you already have a recipe for
v1).

 * There's no set of compilation flags that work before & after my
patch to request v1. I.e. before we only have v1 via USE_LIBPCRE=1,
now we have USE_LIBPCRE1=1 for v1, and either (or both) USE_LIBPCRE=1
& USE_LIBPCRE2=1 for v2.

  I can't think of a good way out of this that doesn't both make sense
Makefile UI-wise and allows the same CI script to Just Work as this
change migrates from pu->next->master.

 * Due to the bizarro existing semantics of the configure script noted
upthread if you have a git build script that does --with-libpcre & you
have libpcre1 installed, it'll link to it, but now since
--with-libpcre defaults to libpcre2 it'll silently skip linking to it
if you don't have it installed.

  Arguably the first part of this series should be to change this
behavior so we don't silently subject distributors to this bad
behavior when and if we migrate to v2 by default, i.e. we should
detect & use a PCRE we find if it's there, and if the user explicitly
asks for --with-libpcre and we can't find whatever the default is we
should error out.

Re: PCRE v2 compile error, was Re: What's cooking in git.git (May 2017, #01; Mon, 1)

[Kevin Daudt]

On Tue, May 02, 2017 at 08:52:21PM +0200, Ævar Arnfjörð Bjarmason wrote:
> 
>  * Due to the bizarro existing semantics of the configure script noted
> upthread if you have a git build script that does --with-libpcre & you
> have libpcre1 installed, it'll link to it, but now since
> --with-libpcre defaults to libpcre2 it'll silently skip linking to it
> if you don't have it installed.
> 

Case in point: The Archlinux git-git aur package[0] (community maintained,
latest git version) does run ./configure without --with-libpcre, but
 requests it from make with USE_LIBPCRE=1.

I noticed when trying git grep -P which then failed.

[0]: https://aur.archlinux.org/cgit/aur.git/tree/PKGBUILD?h=git-git

Re: PCRE v2 compile error, was Re: What's cooking in git.git (May 2017, #01; Mon, 1)

[Ævar Arnfjörð Bjarmason]

On Tue, May 2, 2017 at 10:51 PM, Kevin Daudt <me@ikke.info> wrote:
> On Tue, May 02, 2017 at 08:52:21PM +0200, Ęvar Arnfjörš Bjarmason wrote:
>>
>>  * Due to the bizarro existing semantics of the configure script noted
>> upthread if you have a git build script that does --with-libpcre & you
>> have libpcre1 installed, it'll link to it, but now since
>> --with-libpcre defaults to libpcre2 it'll silently skip linking to it
>> if you don't have it installed.
>>
>
> Case in point: The Archlinux git-git aur package[0] (community maintained,
> latest git version) does run ./configure without --with-libpcre, but
>  requests it from make with USE_LIBPCRE=1.
>
> I noticed when trying git grep -P which then failed.
>
> [0]: https://aur.archlinux.org/cgit/aur.git/tree/PKGBUILD?h=git-git

Whatever's going on there is unrelated to the issue I'm talking about,
but if that's producing a package where -P doesn't work that looks
like a bug in the Makefile.

The way --with-libpcre works now is that it second guesses you, i.e.
it'll put USE_LIBPCRE=YesPlease into config.mak.autogen (sourced by
the Makefile) only if you supply --with-libpcre *and* it can find a
working libpcre on the system, otherwise it silently ignores you.

Whatever you supply to the configure script it'll be overridden by
Makefile arguments if present, but even if there was another bug where
./configure arguments took precedence over Makefile arguments I don't
see how it would apply here, in this case nothing libpcre related will
be written to config.mak.autogen.

So I must be missing something. I don't see how that package build
doesn't either fail at compile time because it doesn't have pcre, or
work, in which case the -P option will work.

Re: PCRE v2 compile error, was Re: What's cooking in git.git (May 2017, #01; Mon, 1)

[Johannes Schindelin]

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

Hi Ævar,

On Tue, 2 May 2017, Ævar Arnfjörð Bjarmason wrote:

> On Tue, May 2, 2017 at 6:05 PM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
>
> > [... complaint about the abrupt change that makes Git for Windows fail
> >  to build because pcre2.h is missing ...]
>
> I think it's worth it to copy/paste the commit message where I made
> this change, since we're having this discussion in this thread:
> 
>     Makefile & configure: make PCRE v2 the default PCRE implementation
> 
>     Change the USE_LIBPCRE=YesPlease & --with-libpcre flags to the
>     Makefile & configure script, respectively, to mean use PCRE v2, not
>     PCRE v1.
> 
>     The legacy library previously available via those options is still
>     available on request via USE_LIBPCRE1=YesPlease or
>     --with-libpcre1. The existing USE_LIBPCRE2=YesPlease & --with-libpcre2
>     still explicitly ask for v2.
> 
>     The v2 PCRE is stable & end-user compatible, all this change does is
>     change the default. Someone building a new git is likely to also have
>     packaged PCRE v2 sometime in the last 2 years since it was released.
>     If not they can choose to use the legacy v2 library by making the
>     trivial s/USE_LIBPCRE/USE_LIBPCRE1/ change, or package up PCRE v2.
> 
>     New releases of PCRE v2 are already faster than PCRE v1, and not only
>     is all significant development is happening on v2, but bugs in
>     reported against v1 have started getting WONTFIX'd asking users to
>     just upgrade to v2.
> 
>     So it makes sense to give our downstream distributors a nudge to
>     switch over to it.

I see where you are coming from.

At this point, I feel that someone should recall into our collective
memory what happened when we made a change similar in nature that broke
existing build setups: by requiring REG_STARTEND all of a sudden ("you can
easily flip the NO_REGEX switch", as if everybody should know about those
Makefile flags we have).

I hate to be that someone, but it has to be said: this is a disruptive
change, and it would be a lot better to make it an opt-in at first, and
when the dust settled about this option and many distributions have opted
in already because of the benefits and tested this thoroughly in practice,
then we can think about making this an opt-out option, potentially putting
some serious thought into trying to make it painless to upgrade for
users casually compiling Git (and not, woohoo, being subscribed to the Git
mailing list, let alone knowing about all those Makefile knobs).

In short: it would be a mistake to force PCRE v2 support on by default.

Ciao,
Dscho

Re: PCRE v2 compile error, was Re: What's cooking in git.git (May 2017, #01; Mon, 1)

[Duy Nguyen]

On Wed, May 3, 2017 at 4:45 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
>>     So it makes sense to give our downstream distributors a nudge to
>>     switch over to it.

Some contributor (i.e. me) was not happy with this nudging though. The
other day I switched to some branch (probably 'pu') and the build
failed because, guess what, I didn't have pcre2 installed. If I set
USE_LIBPCRE1 then I lose pcre support when switching to other
branches. And no, I don't want to install libpcre2, not when I'm
forced to this way.

188 packages on Gentoo optionally depend on libpcre, 6 packages on
libpcre2. Chances that a Gentoo user has libpcre2 already are rather
low. I'll revisit my installation when the level of libpcre2 support
grows a bit more than that. You can nudge distributors directly,
probably more efficient too.

> ...
>
> I hate to be that someone, but it has to be said: this is a disruptive
> change, and it would be a lot better to make it an opt-in at first, and
> when the dust settled about this option and many distributions have opted
> in already because of the benefits and tested this thoroughly in practice,

Agreed.
-- 
Duy

Re: PCRE v2 compile error, was Re: What's cooking in git.git (May 2017, #01; Mon, 1)

[Ævar Arnfjörð Bjarmason]

[Just replying to you & Duy in the same mail, easier]

On Wed, May 3, 2017 at 11:45 AM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi Ævar,
>
> On Tue, 2 May 2017, Ævar Arnfjörð Bjarmason wrote:
>
>> On Tue, May 2, 2017 at 6:05 PM, Johannes Schindelin
>> <Johannes.Schindelin@gmx.de> wrote:
>>
>> > [... complaint about the abrupt change that makes Git for Windows fail
>> >  to build because pcre2.h is missing ...]
>>
>> I think it's worth it to copy/paste the commit message where I made
>> this change, since we're having this discussion in this thread:
>>
>>     Makefile & configure: make PCRE v2 the default PCRE implementation
>>
>>     Change the USE_LIBPCRE=YesPlease & --with-libpcre flags to the
>>     Makefile & configure script, respectively, to mean use PCRE v2, not
>>     PCRE v1.
>>
>>     The legacy library previously available via those options is still
>>     available on request via USE_LIBPCRE1=YesPlease or
>>     --with-libpcre1. The existing USE_LIBPCRE2=YesPlease & --with-libpcre2
>>     still explicitly ask for v2.
>>
>>     The v2 PCRE is stable & end-user compatible, all this change does is
>>     change the default. Someone building a new git is likely to also have
>>     packaged PCRE v2 sometime in the last 2 years since it was released.
>>     If not they can choose to use the legacy v2 library by making the
>>     trivial s/USE_LIBPCRE/USE_LIBPCRE1/ change, or package up PCRE v2.
>>
>>     New releases of PCRE v2 are already faster than PCRE v1, and not only
>>     is all significant development is happening on v2, but bugs in
>>     reported against v1 have started getting WONTFIX'd asking users to
>>     just upgrade to v2.
>>
>>     So it makes sense to give our downstream distributors a nudge to
>>     switch over to it.
>
> I see where you are coming from.
>
> At this point, I feel that someone should recall into our collective
> memory what happened when we made a change similar in nature that broke
> existing build setups: by requiring REG_STARTEND all of a sudden ("you can
> easily flip the NO_REGEX switch", as if everybody should know about those
> Makefile flags we have).

And as a result grep/log -G got faster by default, and more
importantly since v2.10.1 which includes your 2f8952250a and made a
REG_STARTEND engine a hard requirement nobody using git is
mysteriously going to miss grep results because of some stray \0 in
the string being matched.

I agree that I should drop the patch to make v2 the default from this
series for now. Clearly it's controversial, and can be considered on
its own merits once the supporting code is in. I'll do that in the
next submission, which I'm planning to send after v2.13.0 comes out.

But aside from that, I must say I completely disagree with this line
of argument. I think to the extent that we did anything wrong with
NO_REGEX it's that I should have noticed that we could use
REG_STARTEND after I imported gawk's engine in compat/regex &
submitted a patch like yours back in 2010. Instead we got 6 years of
git releases where

Similarly, if PCRE v2 is less buggy & faster then we're looking at man
days/weeks and possibly months/years saved given how many people use
git v.s. how many are developing it & write build scripts for it.

Leaving user benefits on the table to optimize for reducing the
one-time inconvenience of packagers is the wrong move.

> [...]
On Wed, May 3, 2017 at 1:15 PM, Duy Nguyen <pclouds@gmail.com> wrote:
> On Wed, May 3, 2017 at 4:45 PM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
>>>     So it makes sense to give our downstream distributors a nudge to
>>>     switch over to it.
>
> Some contributor (i.e. me) was not happy with this nudging though. The
> other day I switched to some branch (probably 'pu') and the build
> failed because, guess what, I didn't have pcre2 installed. If I set
> USE_LIBPCRE1 then I lose pcre support when switching to other
> branches. And no, I don't want to install libpcre2, not when I'm
> forced to this way.

Assuming that we'd want to change the default to v2, can you think of
a better way to handle this transition? As pointed out in
<CACBZZX6-qZLEGob6CEwpJ7jtEBG6WLPdHQsO4DsbkNZ8di5mjg@mail.gmail.com> I
can't.

Perhaps the best way would be to migrate this change directly to
master if there's consensus to move to v2 by default, that would avoid
the pain for developers related to switching between concurrent
branches while the change is cooking.

> 188 packages on Gentoo optionally depend on libpcre, 6 packages on
> libpcre2. Chances that a Gentoo user has libpcre2 already are rather
> low.

It sounds like two things are being conflated here, surely "a Gentoo
user", i.e. someone who's not actively contributing to git.git will
just get this via `emerge git` if this change were released, since the
ebuild will depend on libpcre2?

> I'll revisit my installation when the level of libpcre2 support
> grows a bit more than that.

There being few existing reverse dependencies seems like a strange
reason not to run `sudo emerge libpcre2`, but whatever, your dev env,
and we won't be breaking v1 support any time soon.

> You can nudge distributors directly, probably more efficient too.

My assumption in making this change this way is that there's a
decreasing amount of git users who'll get a change if:

* 1. It's made mandatory to compile git (100%)
* 2. It's made a default flag that must be turned off (e.g. our HTTP
support via curl)
* 3. It's something users actually notice being missing, e.g. PCRE
support in general via -P
* 4. It's the default mode of some opt-in flag, e.g. this change where
USE_LIBPCRE=YesPlease gives you v2, not v1
* 5. it's advertised prominently in release notes / the installation
guide / on-list.

Your suggestion is some variant of doing #5, whereas I'm currently
trying to aim for #4 (and in the future I think we should probably go
for #2 for PCRE, but that's another discussion).

You can E-Mail some distributors directly, but there's a big long tail
of git builders who maintain build scripts for their company, e.g. I
maintain one of those. If it's just advertised as #5 I'd probably miss
it unless I was paying close attention, particularly since there's no
#3 since I'd be building with v1 already.

Which is why I think if we're going to nudge distributors to move to
v2 it makes sense to do so as this patch is doing it, i.e. if they're
asking for PCRE already make the build fail until they declare
explicitly whether they'd like v1 or v2.

Re: PCRE v2 compile error, was Re: What's cooking in git.git (May 2017, #01; Mon, 1)

[Johannes Schindelin]

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

Hi Ævar,

On Wed, 3 May 2017, Ævar Arnfjörð Bjarmason wrote:

> [Just replying to you & Duy in the same mail, easier]

It makes it harder on everybody else, though, as two slightly different
discussion points are conflated now. Also, no single online mail archive
will be able to render the thread correctly (assuming that you edited in
the In-Reply-To header to loop back to Duy's mail).

> On Wed, May 3, 2017 at 11:45 AM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> >
> > At this point, I feel that someone should recall into our collective
> > memory what happened when we made a change similar in nature that
> > broke existing build setups: by requiring REG_STARTEND all of a sudden
> > ("you can easily flip the NO_REGEX switch", as if everybody should
> > know about those Makefile flags we have).
> 
> And as a result grep/log -G got faster by default,

Sure. For those developers where the build was not broken.

Software maintenance is always a trade-off, and with software as popular
as Git, maintainers bear a special responsibility to *not* break builds
easily, as it is more likely than not that anybody who wants to build Git
is *unfamiliar* with the specifics.

That is the main reason why we have a configure, even if we try hard to
make things work with a straight `make`: people who are happily oblivious
of our discussions on this here high-volume mailing list will be able to
build Git without even consulting the documentation, just by doing what
they would do with any Unix-based software: ./configure && make

> and more importantly since v2.10.1 which includes your 2f8952250a and
> made a REG_STARTEND engine a hard requirement nobody using git is
> mysteriously going to miss grep results because of some stray \0 in the
> string being matched.

That is a misinterpretation of what the REG_STARTEND flag is supposed to
do. In *some* implementations, REG_STARTEND allows NULs in the haystack.
Some other implementations do not allow that. It is ill-defined.

> I agree that I should drop the patch to make v2 the default from this
> series for now. Clearly it's controversial, and can be considered on
> its own merits once the supporting code is in. I'll do that in the
> next submission, which I'm planning to send after v2.13.0 comes out.

Good. I am really glad that we agree that the move to v2 should be a
two-step process, with the uncontroversial "optionally use PCRE v2 for
speed and robustness" first.

Once enough users like it (and speaking for myself, once I heard from
enough users how good it is so that I can justify to set aside enough time
to support PCRE v2 in MSYS2), you will find it much smoother sailing to go
to phase 2.

Ciao,
Dscho

Re: PCRE v2 compile error, was Re: What's cooking in git.git (May 2017, #01; Mon, 1)

[Ævar Arnfjörð Bjarmason]

On Thu, May 4, 2017 at 11:11 AM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi Ævar,
>
> On Wed, 3 May 2017, Ævar Arnfjörð Bjarmason wrote:
>
>> [Just replying to you & Duy in the same mail, easier]
>
> It makes it harder on everybody else, though, as two slightly different
> discussion points are conflated now. Also, no single online mail archive
> will be able to render the thread correctly (assuming that you edited in
> the In-Reply-To header to loop back to Duy's mail).
>
>> On Wed, May 3, 2017 at 11:45 AM, Johannes Schindelin
>> <Johannes.Schindelin@gmx.de> wrote:
>> >
>> > At this point, I feel that someone should recall into our collective
>> > memory what happened when we made a change similar in nature that
>> > broke existing build setups: by requiring REG_STARTEND all of a sudden
>> > ("you can easily flip the NO_REGEX switch", as if everybody should
>> > know about those Makefile flags we have).
>>
>> And as a result grep/log -G got faster by default,
>
> Sure. For those developers where the build was not broken.
>
> Software maintenance is always a trade-off, and with software as popular
> as Git, maintainers bear a special responsibility to *not* break builds
> easily, as it is more likely than not that anybody who wants to build Git
> is *unfamiliar* with the specifics.
>
> That is the main reason why we have a configure, even if we try hard to
> make things work with a straight `make`: people who are happily oblivious
> of our discussions on this here high-volume mailing list will be able to
> build Git without even consulting the documentation, just by doing what
> they would do with any Unix-based software: ./configure && make

And they still will. That sort of invocation will not build with
PCRE[1]. This change to v2 by default only impacts builders who are
already going out of their way to customize their build with
USE_LIBPCRE=YesPlease.

I don't think your comments make sense in that context. We're not even
talking about the advanced user/newbie developer who wants to build
git for the first time, we're talking about the user who's already
involved enough to start scouring the Makefile & tweaking the various
flags there.

Changing the current advice from "if you want PCRE enable this" to "if
you want PCRE enable this, we use v2 then, turn this other thing on if
you want v1" is a *very* small imposition on people already deeply
customizing their build to the extent of turning on the non-default
PCRE in the first place.

Yes that's a trade-off, and I'm not denying that some advanced users
will notice / be annoyed by that change. I just think the end-user
benefit of bringing v2 to the attention of distributors who are doing
such customized builds outweighs that.

>> and more importantly since v2.10.1 which includes your 2f8952250a and
>> made a REG_STARTEND engine a hard requirement nobody using git is
>> mysteriously going to miss grep results because of some stray \0 in the
>> string being matched.
>
> That is a misinterpretation of what the REG_STARTEND flag is supposed to
> do. In *some* implementations, REG_STARTEND allows NULs in the haystack.
> Some other implementations do not allow that. It is ill-defined.

We have a test in t7008-grep-binary.sh that'll fail if REG_STARTEND
doesn't behave as I described. Actually I think I got the history a
bit wrong here, I marked that test as succeeding in commit 7e36de5859
("t/t7008-grep-binary.sh: un-TODO a test that needs REG_STARTEND",
2010-08-17).

So I think since 2010 either distributors without REG_STARTEND have
been enabling NO_REGEX=YesPlease giving users the right behavior with
a \0 in their haystack, or their tests have been consistently failing
for 7 years, hopefully the former is the case.

>> I agree that I should drop the patch to make v2 the default from this
>> series for now. Clearly it's controversial, and can be considered on
>> its own merits once the supporting code is in. I'll do that in the
>> next submission, which I'm planning to send after v2.13.0 comes out.
>
> Good. I am really glad that we agree that the move to v2 should be a
> two-step process, with the uncontroversial "optionally use PCRE v2 for
> speed and robustness" first.
>
> Once enough users like it (and speaking for myself, once I heard from
> enough users how good it is so that I can justify to set aside enough time
> to support PCRE v2 in MSYS2), you will find it much smoother sailing to go
> to phase 2.

The v2 API is not a user-visible change. It'll help performance, and
over the long term subject users to fewer internal PCRE bugs in v1
that'll never get fixed.

That's pretty much a textbook example of the sort of thing users will
never even notice, but which is a good thing to do anyway, users don't
notice most of the incremental performance improvements made to git,
but they matter to them in the aggregate.

So I think if your criteria for working on integrating v2 is users
noticing it elsewhere and asking you for it you'll likely never switch
to it. I think that's a bit unfortunate & a strange way to evaluate
whether you should turn on incremental performance improvements, but
whatever, your build & your users.

Regardless of that, I think it's a different question whether most
other distributors of git feel the same way. I'd hope that most of
them would be happy to take a performance/reliability improvement like
that, most of them likely have v2 packaged already so it would be
trivial for them, and if not they'd either be happy to package it up
for such an improvement, or alternatively change USE_LIBPCRE=Y to
USE_LIBPCRE1=Y.

1.  Which I think is a bug as noted upthread in
<CACBZZX5_45KnU7qzW2ojJiznmfkef44YGL8-CYkHFLOvhLSASg@mail.gmail.com>,
but this behavior is what users use/rely on now.

Re: PCRE v2 compile error, was Re: What's cooking in git.git (May 2017, #01; Mon, 1)

[Johannes Schindelin]

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

Hi Ævar,

On Thu, 4 May 2017, Ævar Arnfjörð Bjarmason wrote:

> So I think if your criteria for working on integrating v2 is users
> noticing it elsewhere and asking you for it you'll likely never switch
> to it.

Speaking for myself, my biggest problem with your patch series is to find
the time to work on packaging PCRE v2 as an MSYS2 package.

If you force me to make that time *now*, by forcing the switch to v2 in
`pu`, you will disimprove my mood. That's all.

If, however, you are gently with people like me, offering this as an
opt-in, so that I can play with it when I find some time to build PCRE v2
in MSYS2's context, I will be pretty happy, and of course will ship Git
for Windows with the faster PCRE support as soon as I am satisfied that it
works correctly.

That should make you happy, too, as you will get quite a bit of testers
that way. Gently.

Ciao,
Dscho

Re: PCRE v2 compile error, was Re: What's cooking in git.git (May 2017, #01; Mon, 1)

[Ævar Arnfjörð Bjarmason]

On Thu, May 4, 2017 at 1:32 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi Ævar,
>
> On Thu, 4 May 2017, Ævar Arnfjörð Bjarmason wrote:
>
>> So I think if your criteria for working on integrating v2 is users
>> noticing it elsewhere and asking you for it you'll likely never switch
>> to it.
>
> Speaking for myself, my biggest problem with your patch series is to find
> the time to work on packaging PCRE v2 as an MSYS2 package.
>
> If you force me to make that time *now*, by forcing the switch to v2 in
> `pu`, you will disimprove my mood. That's all.
>
> If, however, you are gently with people like me, offering this as an
> opt-in, so that I can play with it when I find some time to build PCRE v2
> in MSYS2's context, I will be pretty happy, and of course will ship Git
> for Windows with the faster PCRE support as soon as I am satisfied that it
> works correctly.
>
> That should make you happy, too, as you will get quite a bit of testers
> that way. Gently.

Sure, all makes sense. As noted I'll keep it from being the default for now.

The only reason I even replied & kept this thread going now was
because I noticed "make" not enabling it, REG_STARTEND making \0
searches work in practice (even if not specified by the NetBSD
extension) etc.

All stuff I would surely forget by the time we have the discussion
again down the road, so I wanted to get that E-Mail out now for future
reference.

Re: PCRE v2 compile error, was Re: What's cooking in git.git (May 2017, #01; Mon, 1)

[Ævar Arnfjörð Bjarmason]

On Thu, May 4, 2017 at 1:32 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi Ævar,
>
> On Thu, 4 May 2017, Ævar Arnfjörð Bjarmason wrote:
>
>> So I think if your criteria for working on integrating v2 is users
>> noticing it elsewhere and asking you for it you'll likely never switch
>> to it.
>
> Speaking for myself, my biggest problem with your patch series is to find
> the time to work on packaging PCRE v2 as an MSYS2 package.

This won't be in my next PCRE submission, but I have a path locally to
simply import PCRE into git.git as compat/pcre2, so it can be compiled
with NO_PCRE=Y similar to how NO_REGEX=Y works.

This will hopefully address your concerns partially, i.e. when you do
want to try it out it'll be easier.

Re: PCRE v2 compile error, was Re: What's cooking in git.git (May 2017, #01; Mon, 1)

[Junio C Hamano]

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

> This won't be in my next PCRE submission, but I have a path locally to
> simply import PCRE into git.git as compat/pcre2, so it can be compiled
> with NO_PCRE=Y similar to how NO_REGEX=Y works.
>
> This will hopefully address your concerns partially, i.e. when you do
> want to try it out it'll be easier.

Eek, please don't.  

Until pcre2 becomes _so_ stable that all reasonable distros give
choice to the end-users to install it easily in a packaged form,
such a "not a fork, but a copy" will be a moving target that I'd
rather not to have in compat/.  IOW, our compat/$pkg should be a
last resort to help those on distros that are so hard to convince to
carry the version/variant of $pkg we would like to use.

Re: PCRE v2 compile error, was Re: What's cooking in git.git (May 2017, #01; Mon, 1)

[brian m. carlson]

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

On Mon, May 08, 2017 at 04:10:41PM +0900, Junio C Hamano wrote:
> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> 
> > This won't be in my next PCRE submission, but I have a path locally to
> > simply import PCRE into git.git as compat/pcre2, so it can be compiled
> > with NO_PCRE=Y similar to how NO_REGEX=Y works.
> >
> > This will hopefully address your concerns partially, i.e. when you do
> > want to try it out it'll be easier.
> 
> Eek, please don't.
> 
> Until pcre2 becomes _so_ stable that all reasonable distros give
> choice to the end-users to install it easily in a packaged form,
> such a "not a fork, but a copy" will be a moving target that I'd
> rather not to have in compat/.  IOW, our compat/$pkg should be a
> last resort to help those on distros that are so hard to convince to
> carry the version/variant of $pkg we would like to use.

PCRE and PCRE2 also tend to have a lot of security updates, so I would
prefer if we didn't import them into the tree.  It is far better for
users to use their distro's packages for PCRE, as it means they get
automatic security updates even if they're using an old Git.

We shouldn't consider shipping anything with a remotely frequent history
of security updates in our tree, since people very frequently run old or
ancient versions of Git.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]

Re: PCRE v2 compile error, was Re: What's cooking in git.git (May 2017, #01; Mon, 1)

[Ævar Arnfjörð Bjarmason]

On Tue, May 9, 2017 at 1:32 AM, brian m. carlson
<sandals@crustytoothpaste.net> wrote:
> On Mon, May 08, 2017 at 04:10:41PM +0900, Junio C Hamano wrote:
>> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>>
>> > This won't be in my next PCRE submission, but I have a path locally to
>> > simply import PCRE into git.git as compat/pcre2, so it can be compiled
>> > with NO_PCRE=Y similar to how NO_REGEX=Y works.
>> >
>> > This will hopefully address your concerns partially, i.e. when you do
>> > want to try it out it'll be easier.
>>
>> Eek, please don't.
>>
>> Until pcre2 becomes _so_ stable that all reasonable distros give
>> choice to the end-users to install it easily in a packaged form,
>> such a "not a fork, but a copy" will be a moving target that I'd
>> rather not to have in compat/.  IOW, our compat/$pkg should be a
>> last resort to help those on distros that are so hard to convince to
>> carry the version/variant of $pkg we would like to use.

The reason I'm looking into this is because the WIP part of my PCRE
branch has changes which start to use PCRE as a general matching
engine in git, even. I.e.:

* git grep -F will be powered by it rather than kwset (which'll be git rm'd)
* Long standing limitations with \0s in regexes go away.
* grep -G and -E will use PCRE via a WIP POSIX -> PCRE  pattern
translator (https://bugs.exim.org/show_bug.cgi?id=2106)
* Perhaps we can remove compat/regex/ entirely & use PCRE via its
POSIX emulation mode + pattern translator (we use regcomp/regexec a
lot for non-grep/log), I'm not sure yet.

I have messy but working code for this in a WIP branch, here's the
performance improvement against linux.git:

Test                                           v2.13.0-rc2       HEAD
---------------------------------------------------------------------------------------
7820.1: basic grep how.to                      0.31(1.20+0.46)
0.19(0.33+0.55) -38.7%
7820.2: extended grep how.to                   0.31(1.19+0.46)
0.19(0.33+0.55) -38.7%
7820.3: perl grep how.to                       0.30(1.12+0.46)
0.19(0.28+0.62) -36.7%
7820.5: basic grep ^how to                     0.31(1.24+0.39)
0.19(0.32+0.56) -38.7%
7820.6: extended grep ^how to                  0.30(1.18+0.44)
0.19(0.22+0.66) -36.7%
7820.7: perl grep ^how to                      0.55(2.68+0.41)
0.19(0.32+0.56) -65.5%
7820.9: basic grep [how] to                    0.47(2.17+0.44)
0.22(0.39+0.54) -53.2%
7820.10: extended grep [how] to                0.47(2.21+0.40)
0.22(0.39+0.55) -53.2%
7820.11: perl grep [how] to                    0.53(2.64+0.39)
0.21(0.37+0.58) -60.4%
7820.13: basic grep \(e.t[^ ]*\|v.ry\) rare    0.63(3.16+0.48)
0.21(0.48+0.53) -66.7%
7820.14: extended grep (e.t[^ ]*|v.ry) rare    0.64(3.28+0.38)
0.21(0.45+0.57) -67.2%
7820.15: perl grep (e.t[^ ]*|v.ry) rare        1.00(5.77+0.37)
0.21(0.50+0.53) -79.0%
7820.17: basic grep m\(ú\|u\)ult.b\(æ\|y\)te   0.31(1.23+0.51)
0.19(0.30+0.58) -38.7%
7820.18: extended grep m(ú|u)ult.b(æ|y)te      0.32(1.26+0.47)
0.19(0.27+0.61) -40.6%
7820.19: perl grep m(ú|u)ult.b(æ|y)te          0.36(1.61+0.37)
0.19(0.30+0.57) -47.2%
7821.1: fixed grep int                         0.52(1.71+0.64)
0.43(1.11+0.72) -17.3%
7821.2: basic grep int                         0.54(1.62+0.70)
0.42(1.14+0.62) -22.2%
7821.3: extended grep int                      0.53(1.67+0.64)
0.51(1.17+0.62) -3.8%
7821.4: perl grep int                          0.53(1.71+0.59)
0.72(1.13+0.63) +35.8%
7821.6: fixed grep -i int                      0.58(1.86+0.67)
0.47(1.32+0.62) -19.0%
7821.7: basic grep -i int                      0.62(1.94+0.61)
0.57(1.25+0.72) -8.1%
7821.8: extended grep -i int                   0.82(1.86+0.68)
0.50(1.41+0.56) -39.0%
7821.9: perl grep -i int                       0.70(1.88+0.68)
0.56(1.25+0.70) -20.0%
7821.11: fixed grep æ                          0.33(1.30+0.43)
0.19(0.22+0.64) -42.4%
7821.12: basic grep æ                          0.33(1.35+0.38)
0.18(0.26+0.59) -45.5%
7821.13: extended grep æ                       0.33(1.20+0.52)
0.18(0.32+0.53) -45.5%
7821.14: perl grep æ                           0.33(1.31+0.40)
0.18(0.28+0.56) -45.5%
7821.16: fixed grep -i æ                       0.25(0.87+0.50)
0.18(0.24+0.60) -28.0%
7821.17: basic grep -i æ                       0.26(0.88+0.48)
0.18(0.24+0.60) -30.8%
7821.18: extended grep -i æ                    0.26(0.92+0.44)
0.18(0.24+0.61) -30.8%
7821.19: perl grep -i æ                        0.25(0.79+0.45)
0.19(0.32+0.56) -24.0%

In case that comes out misformatted it's also available at
https://github.com/avar/git/commit/ee5b2040e2c697e22a73c7b5f07f1b1e591f07e3

I.e. grepping is sped up by 50% and more in many cases, even for -G
and -E patterns which now get translated internally into PCRE
patterns.

> PCRE and PCRE2 also tend to have a lot of security updates, so I would
> prefer if we didn't import them into the tree.  It is far better for
> users to use their distro's packages for PCRE, as it means they get
> automatic security updates even if they're using an old Git.
>
> We shouldn't consider shipping anything with a remotely frequent history
> of security updates in our tree, since people very frequently run old or
> ancient versions of Git.

I'm aware of its security record[1], but I wonder what threat model
you have in mind here. I'm not aware of any parts of git (except maybe
gitweb?) where we take regexes from untrusted sources.

I.e. yes there have been DoS's & even some overflow bugs leading code
execution in PCRE, but in the context of powering git-grep & git-log
with PCRE this falls into the "stop hitting yourself" category.

1. https://www.cvedetails.com/vendor/3265/Pcre.html

Re: PCRE v2 compile error, was Re: What's cooking in git.git (May 2017, #01; Mon, 1)

[brian m. carlson]

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

On Tue, May 09, 2017 at 02:00:18AM +0200, Ævar Arnfjörð Bjarmason wrote:
> On Tue, May 9, 2017 at 1:32 AM, brian m. carlson
> <sandals@crustytoothpaste.net> wrote:
> > PCRE and PCRE2 also tend to have a lot of security updates, so I would
> > prefer if we didn't import them into the tree.  It is far better for
> > users to use their distro's packages for PCRE, as it means they get
> > automatic security updates even if they're using an old Git.
> >
> > We shouldn't consider shipping anything with a remotely frequent history
> > of security updates in our tree, since people very frequently run old or
> > ancient versions of Git.
> 
> I'm aware of its security record[1], but I wonder what threat model
> you have in mind here. I'm not aware of any parts of git (except maybe
> gitweb?) where we take regexes from untrusted sources.
> 
> I.e. yes there have been DoS's & even some overflow bugs leading code
> execution in PCRE, but in the context of powering git-grep & git-log
> with PCRE this falls into the "stop hitting yourself" category.

Just because you don't drive Git with untrusted regexes doesn't mean
other people don't.  It's not a good idea to require a stronger security
model than we absolutely have to, since people can and will violate it.
Think how devastating Shellshock was even though technically nobody
should provide insecure environment variables to the shell.

And, yes, gitweb does in fact call git grep.  That means that git grep
must in fact be secure against untrusted regexes, or you have a remote
code execution vulnerability.

Furthermore, at work we distribute Git with all releases of our product.
We normally only do non-security updates to the last couple of releases,
but we provide security updates to all supported versions.  I'm not
comfortable shipping the entirety of PCRE or PCRE2 to customers without
providing security updates, so you're going to make my job (and my
coworkers') a lot harder by shipping it.  Please don't.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]

Re: PCRE v2 compile error, was Re: What's cooking in git.git (May 2017, #01; Mon, 1)

[Johannes Schindelin]

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

Hi,

On Tue, 9 May 2017, brian m. carlson wrote:

> On Tue, May 09, 2017 at 02:00:18AM +0200, Ævar Arnfjörð Bjarmason wrote:
> > On Tue, May 9, 2017 at 1:32 AM, brian m. carlson
> > <sandals@crustytoothpaste.net> wrote:
> > > PCRE and PCRE2 also tend to have a lot of security updates, so I
> > > would prefer if we didn't import them into the tree.  It is far
> > > better for users to use their distro's packages for PCRE, as it
> > > means they get automatic security updates even if they're using an
> > > old Git.
> > >
> > > We shouldn't consider shipping anything with a remotely frequent
> > > history of security updates in our tree, since people very
> > > frequently run old or ancient versions of Git.
> > 
> > I'm aware of its security record[1], but I wonder what threat model
> > you have in mind here. I'm not aware of any parts of git (except maybe
> > gitweb?) where we take regexes from untrusted sources.
> > 
> > I.e. yes there have been DoS's & even some overflow bugs leading code
> > execution in PCRE, but in the context of powering git-grep & git-log
> > with PCRE this falls into the "stop hitting yourself" category.
> 
> Just because you don't drive Git with untrusted regexes doesn't mean
> other people don't.

Or other applications.

> It's not a good idea to require a stronger security model than we
> absolutely have to, since people can and will violate it.  Think how
> devastating Shellshock was even though technically nobody should provide
> insecure environment variables to the shell.
> 
> And, yes, gitweb does in fact call git grep. That means that git grep
> must in fact be secure against untrusted regexes, or you have a remote
> code execution vulnerability.

And not only grep is affected. Think HEAD^{/<regex>}. There are plenty of
sites where you are allowed to specify revs in a freer form than SHA-1s.

Having said that, I do like the prospect of a faster git grep.

Hopefully there will be a way to make use of PCRE that can be switched
off? Like, a compile-time replacement of the regex API backed by PCRE v2
*iff* PCRE v2 is used for building?

Ciao,
Dscho

Re: PCRE v2 compile error, was Re: What's cooking in git.git (May 2017, #01; Mon, 1)

[Ævar Arnfjörð Bjarmason]

On Tue, May 9, 2017 at 12:40 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi,
>
> On Tue, 9 May 2017, brian m. carlson wrote:
>
>> On Tue, May 09, 2017 at 02:00:18AM +0200, Ævar Arnfjörð Bjarmason wrote:
>> > On Tue, May 9, 2017 at 1:32 AM, brian m. carlson
>> > <sandals@crustytoothpaste.net> wrote:
>> > > PCRE and PCRE2 also tend to have a lot of security updates, so I
>> > > would prefer if we didn't import them into the tree.  It is far
>> > > better for users to use their distro's packages for PCRE, as it
>> > > means they get automatic security updates even if they're using an
>> > > old Git.
>> > >
>> > > We shouldn't consider shipping anything with a remotely frequent
>> > > history of security updates in our tree, since people very
>> > > frequently run old or ancient versions of Git.
>> >
>> > I'm aware of its security record[1], but I wonder what threat model
>> > you have in mind here. I'm not aware of any parts of git (except maybe
>> > gitweb?) where we take regexes from untrusted sources.
>> >
>> > I.e. yes there have been DoS's & even some overflow bugs leading code
>> > execution in PCRE, but in the context of powering git-grep & git-log
>> > with PCRE this falls into the "stop hitting yourself" category.
>>
>> Just because you don't drive Git with untrusted regexes doesn't mean
>> other people don't.
>
> Or other applications.
>
>> It's not a good idea to require a stronger security model than we
>> absolutely have to, since people can and will violate it.  Think how
>> devastating Shellshock was even though technically nobody should provide
>> insecure environment variables to the shell.
>>
>> And, yes, gitweb does in fact call git grep. That means that git grep
>> must in fact be secure against untrusted regexes, or you have a remote
>> code execution vulnerability.
>
> And not only grep is affected. Think HEAD^{/<regex>}. There are plenty of
> sites where you are allowed to specify revs in a freer form than SHA-1s.

That will still use reg(comp|exec) for the foreseeable future. We have
plenty of manual use of that all over the place:

    $ git grep 'reg(comp|exec)\(' *.[ch] builtin/*.[ch]

And the ^{/rx} feature is powered by the one in sha1_name.c

> Having said that, I do like the prospect of a faster git grep.
>
> Hopefully there will be a way to make use of PCRE that can be switched
> off? Like, a compile-time replacement of the regex API backed by PCRE v2
> *iff* PCRE v2 is used for building?

Yup, see my just-sent
<CACBZZX6V8qbnrZAdhRvPthy5Z91iEG8rrJ=Sf9tdkOt52M9j1Q@mail.gmail.com>.
It'll be optional for now, as it's been for a while.

Aside from that I do think given these numbers it's worth considering
making PCRE a default dependency, and possibly getting rid of stuff
like kwset because a) it reduces the many codepaths we have now of
either doing fixed/basic/extended/pcre into one b) since the numbers
suggest pcre can support all of that faster that seems like a sensible
thing to do.

But anything like that will be a few patch series's down the road, for
now I'm just making it all optional.

Re: PCRE v2 compile error, was Re: What's cooking in git.git (May 2017, #01; Mon, 1)

[Ævar Arnfjörð Bjarmason]

On Tue, May 9, 2017 at 2:37 AM, brian m. carlson
<sandals@crustytoothpaste.net> wrote:
> On Tue, May 09, 2017 at 02:00:18AM +0200, Ævar Arnfjörð Bjarmason wrote:
>> On Tue, May 9, 2017 at 1:32 AM, brian m. carlson
>> <sandals@crustytoothpaste.net> wrote:
>> > PCRE and PCRE2 also tend to have a lot of security updates, so I would
>> > prefer if we didn't import them into the tree.  It is far better for
>> > users to use their distro's packages for PCRE, as it means they get
>> > automatic security updates even if they're using an old Git.
>> >
>> > We shouldn't consider shipping anything with a remotely frequent history
>> > of security updates in our tree, since people very frequently run old or
>> > ancient versions of Git.
>>
>> I'm aware of its security record[1], but I wonder what threat model
>> you have in mind here. I'm not aware of any parts of git (except maybe
>> gitweb?) where we take regexes from untrusted sources.
>>
>> I.e. yes there have been DoS's & even some overflow bugs leading code
>> execution in PCRE, but in the context of powering git-grep & git-log
>> with PCRE this falls into the "stop hitting yourself" category.
>
> Just because you don't drive Git with untrusted regexes doesn't mean
> other people don't.  It's not a good idea to require a stronger security
> model than we absolutely have to, since people can and will violate it.
> Think how devastating Shellshock was even though technically nobody
> should provide insecure environment variables to the shell.

Yes this is definitely something we should keep in mind. I see my
comment could be read as dismissing your concerns, I didn't mean that.
This is definitely something to be concerned about.

I was trying to solicit feedback on whether there were any other parts
of stock git that could take external input as regexes than gitweb,
I'm not aware of any.

I thought that gitweb wouldn't take regexes by default, but I see now
that the 'search' feature is on by default, and that allows you to
grep / pickaxe with regexes. Either -E or -F for grep, or
--pickaxe-regex (which implies -E) for log.

> And, yes, gitweb does in fact call git grep.  That means that git grep
> must in fact be secure against untrusted regexes, or you have a remote
> code execution vulnerability.

Indeed, but it's not as clear-cut as turning on on PCRE making you vulnerable.

* gitweb is vulnerable to CPU DoS now in its default configuration.
It's easy to provide an ERE that ends up slurping up 100% CPU for
several seconds on any non-trivial sized repo, do that in parallel &
you have a DoS vector.

* Obviously something that's 2x as fast or more (which my WIP code is)
makes this better.

* PCRE tends to be worse at pathological patterns, but this is because
it has really large limits by default and will try rather insanely
hard to match your pattern.

                -DHEAP_LIMIT=20000000 \
                -DMATCH_LIMIT=10000000 \
                -DMATCH_LIMIT_DEPTH=10000000 \
                -DMAX_NAME_COUNT=10000 \
                -DMAX_NAME_SIZE=32 \
                -DPARENS_NEST_LIMIT=250 \

This can be reduced dynamically via the API (and the patterns can't
change it, except to reduce it). For example on my system 2.11.0
(before any of my changes) on linux.git:

    $ time git grep -E -- '-?-?-?-?-?-?-?-?-?-?-?-----------$' |wc -l
    12434
    real    0m0.444s
    $ time git grep -P -- '-?-?-?-?-?-?-?-?-?-?-?-----------$' | wc -l
    12434
    real    1m20.849s

With my JIT changes:

    $ time ~/g/git/git --exec-path=/home/avar/g/git grep -P --
'-?-?-?-?-?-?-?-?-?-?-?-----------$' | wc -l
    12434
    real    0m5.334s

However for user-supplied patterns we can just turn on really
conservative settings:

    $ time ~/g/git/git --exec-path=/home/avar/g/git grep -P --
'(*LIMIT_RECURSION=1000)(*LIMIT_MATCH=1000)-?-?-?-?-?-?-?-?-?-?-?-----------$'
| wc -l
    fatal: pcre2_jit_match failed with error code -47: match limit exceeded
    0
    real    0m0.021s

When I locally compile git with something like this:

-               -DMATCH_LIMIT=10000000 \
-               -DMATCH_LIMIT_DEPTH=10000000 \
-               -DMATCH_LIMIT_RECURSION=10000000 \
-               -DMAX_NAME_COUNT=10000 \
+               -DMATCH_LIMIT=1000 \
+               -DMATCH_LIMIT_DEPTH=1000 \
+               -DMATCH_LIMIT_RECURSION=1000 \
+               -DMAX_NAME_COUNT=100 \
                -DMAX_NAME_SIZE=32 \
-               -DPARENS_NEST_LIMIT=250 \
+               -DPARENS_NEST_LIMIT=10 \

All tests still pass, and from my own testing I can't find any
non-pathological patterns this would break. So we might actually
consider turning this down for git by default.

* Much of the rest of the patterns PCRE has CVEs for can similarly be
mitigated by simply turning various features off.

> Furthermore, at work we distribute Git with all releases of our product.
> We normally only do non-security updates to the last couple of releases,
> but we provide security updates to all supported versions.  I'm not
> comfortable shipping the entirety of PCRE or PCRE2 to customers without
> providing security updates, so you're going to make my job (and my
> coworkers') a lot harder by shipping it.  Please don't.

I'm not going to make your job harder. This WIP patch I have for
embedding PCRE2 as compat/pcre2 is for use by those who'd want to get
a faster grep, but for whatever reason don't have a handy PCRE v2
package already, or wouldn't mind supporting a git that comes bundled
with it.

My comments about "git rm"-ing kwset & compat/regex were meant in the
sense of "I have a branch where we don't use that anymore, and it's
faster", not that you couldn't compile a git without PCRE for the
forseeable future.

My WIP branch does remove much of the non-PCRE code, but that's just
because it was easier to hack up like that and not support a bunch of
if pcre/else branches, I'll amend that and keep PCRE optional when I
end up submitting those patches.

Re: PCRE v2 compile error, was Re: What's cooking in git.git (May 2017, #01; Mon, 1)

[demerphq]

On 9 May 2017 at 13:12, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
> On Tue, May 9, 2017 at 2:37 AM, brian m. carlson
> <sandals@crustytoothpaste.net> wrote:
>> On Tue, May 09, 2017 at 02:00:18AM +0200, Ævar Arnfjörð Bjarmason wrote:
> * gitweb is vulnerable to CPU DoS now in its default configuration.
> It's easy to provide an ERE that ends up slurping up 100% CPU for
> several seconds on any non-trivial sized repo, do that in parallel &
> you have a DoS vector.

Does one need an ERE? Can't one do that now to many parts of git just
with a glob?

Yves

Re: PCRE v2 compile error, was Re: What's cooking in git.git (May 2017, #01; Mon, 1)

[Ævar Arnfjörð Bjarmason]

On Tue, May 9, 2017 at 4:22 PM, demerphq <demerphq@gmail.com> wrote:
> On 9 May 2017 at 13:12, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
>> On Tue, May 9, 2017 at 2:37 AM, brian m. carlson
>> <sandals@crustytoothpaste.net> wrote:
>>> On Tue, May 09, 2017 at 02:00:18AM +0200, Ævar Arnfjörð Bjarmason wrote:
>> * gitweb is vulnerable to CPU DoS now in its default configuration.
>> It's easy to provide an ERE that ends up slurping up 100% CPU for
>> several seconds on any non-trivial sized repo, do that in parallel &
>> you have a DoS vector.
>
> Does one need an ERE? Can't one do that now to many parts of git just
> with a glob?

in practice I don't think so because:

1) I'm now aware of any place where we expose globbing over the wire.

2) AFAICT for the issue detailed in [1] to trigger you also need a
pathological filename in the repo, e.g. I can't get git-ls-files to go
quadratic on either git.git or linux.git, whereas it's pretty easy to
come up with a really expensive regex since there's more content to
choose from when matching file content than filenames.

1. https://public-inbox.org/git/20170424211249.28553-1-avarab@gmail.com/

Re: PCRE v2 compile error, was Re: What's cooking in git.git (May 2017, #01; Mon, 1)

[Brandon Williams]

On 05/02, Ævar Arnfjörð Bjarmason wrote:
> On Tue, May 2, 2017 at 2:09 PM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> > Hi Ævar,
> >
> > On Sun, 30 Apr 2017, Junio C Hamano wrote:
> >
> >> * ab/grep-pcre-v2 (2017-04-25) 20 commits
> >>  - SQUASH???
> >>  - Makefile & configure: make PCRE v2 the default PCRE implementation
> >>  - grep: remove support for concurrent use of both PCRE v1 & v2
> >>  - grep: add support for PCRE v2
> >>  - grep: add support for the PCRE v1 JIT API
> >>  - perf: add a performance comparison test of grep -E and -P
> >>  - grep: change the internal PCRE code & header names to be PCRE1
> >>  - grep: change the internal PCRE macro names to be PCRE1
> >>  - test-lib: rename the LIBPCRE prerequisite to PCRE
> >>  - grep: make grep.patternType=[pcre|pcre1] a synonym for "perl"
> >>  - grep & rev-list doc: stop promising libpcre for --perl-regexp
> >>  - log: add -P as a synonym for --perl-regexp
> >>  - log: add exhaustive tests for pattern style options & config
> >>  - grep: add a test for backreferences in PCRE patterns
> >>  - Makefile & configure: reword outdated comment about PCRE
> >>  - grep: remove redundant `regflags &= ~REG_EXTENDED` assignments
> >>  - grep: remove redundant regflags assignment under PCRE
> >>  - grep: submodule-related case statements should die if new fields are added
> >>  - grep: add tests for grep pattern types being passed to submodules
> >>  - grep: amend submodule recursion test in preparation for rx engine testing
> >>
> >>  PCRE2, which has an API different from and incompatible with PCRE,
> >>  can now be chosen to support "grep -P -e '<pattern>'" and friends.
> >
> > FWIW for quite a couple of recent builds, `pu` fails on Windows with a
> > variation of this error:
> >
> >             CC blob.o
> >         In file included from revision.h:5:0,
> >                          from bisect.c:4:
> >         grep.h:16:19: fatal error: pcre2.h: No such file or directory
> >          #include <pcre2.h>
> >                            ^
> >         compilation terminated.
> >
> > Maybe this can be fixed before hitting `next`?
> 
> This will be due to a combination of the build machine not having pcre
> v2 (but having v1) & my "Makefile & configure: make PCRE v2 the
> default PCRE implementation" patch, which makes v2 the default for
> USE_LIBPCRE=YesPlease.
> 
> Is it easy to install v2 on these build machines? Alternatively that
> patch could be ejected out of pu, or you could USE_LIBPCRE1=YesPlease
> to use v1, but as explained in that commit I think it makes sense to
> make v2 the default.

Shouldn't the Makefile check for the existence of PCREv2 before making
it the default?

-- 
Brandon Williams

Re: PCRE v2 compile error, was Re: What's cooking in git.git (May 2017, #01; Mon, 1)

[Ævar Arnfjörð Bjarmason]

On Tue, May 2, 2017 at 7:43 PM, Brandon Williams <bmwill@google.com> wrote:
> On 05/02, Ævar Arnfjörð Bjarmason wrote:
>> On Tue, May 2, 2017 at 2:09 PM, Johannes Schindelin
>> <Johannes.Schindelin@gmx.de> wrote:
>> > Hi Ævar,
>> >
>> > On Sun, 30 Apr 2017, Junio C Hamano wrote:
>> >
>> >> * ab/grep-pcre-v2 (2017-04-25) 20 commits
>> >>  - SQUASH???
>> >>  - Makefile & configure: make PCRE v2 the default PCRE implementation
>> >>  - grep: remove support for concurrent use of both PCRE v1 & v2
>> >>  - grep: add support for PCRE v2
>> >>  - grep: add support for the PCRE v1 JIT API
>> >>  - perf: add a performance comparison test of grep -E and -P
>> >>  - grep: change the internal PCRE code & header names to be PCRE1
>> >>  - grep: change the internal PCRE macro names to be PCRE1
>> >>  - test-lib: rename the LIBPCRE prerequisite to PCRE
>> >>  - grep: make grep.patternType=[pcre|pcre1] a synonym for "perl"
>> >>  - grep & rev-list doc: stop promising libpcre for --perl-regexp
>> >>  - log: add -P as a synonym for --perl-regexp
>> >>  - log: add exhaustive tests for pattern style options & config
>> >>  - grep: add a test for backreferences in PCRE patterns
>> >>  - Makefile & configure: reword outdated comment about PCRE
>> >>  - grep: remove redundant `regflags &= ~REG_EXTENDED` assignments
>> >>  - grep: remove redundant regflags assignment under PCRE
>> >>  - grep: submodule-related case statements should die if new fields are added
>> >>  - grep: add tests for grep pattern types being passed to submodules
>> >>  - grep: amend submodule recursion test in preparation for rx engine testing
>> >>
>> >>  PCRE2, which has an API different from and incompatible with PCRE,
>> >>  can now be chosen to support "grep -P -e '<pattern>'" and friends.
>> >
>> > FWIW for quite a couple of recent builds, `pu` fails on Windows with a
>> > variation of this error:
>> >
>> >             CC blob.o
>> >         In file included from revision.h:5:0,
>> >                          from bisect.c:4:
>> >         grep.h:16:19: fatal error: pcre2.h: No such file or directory
>> >          #include <pcre2.h>
>> >                            ^
>> >         compilation terminated.
>> >
>> > Maybe this can be fixed before hitting `next`?
>>
>> This will be due to a combination of the build machine not having pcre
>> v2 (but having v1) & my "Makefile & configure: make PCRE v2 the
>> default PCRE implementation" patch, which makes v2 the default for
>> USE_LIBPCRE=YesPlease.
>>
>> Is it easy to install v2 on these build machines? Alternatively that
>> patch could be ejected out of pu, or you could USE_LIBPCRE1=YesPlease
>> to use v1, but as explained in that commit I think it makes sense to
>> make v2 the default.
>
> Shouldn't the Makefile check for the existence of PCREv2 before making
> it the default?

We can't check for the existence of anything in the Makefile, that's
what the configure script is for.

The configure script does the "right" thing with pcre v2.

The "right" thing being the bizarro pre-existing behavior of "oh you
asked for PCRE? Well let me check if it's on the system, if it's not
I'll just silently undo what you asked for". Which is crazy, but was
there before my patches, so I didn't change it.

I.e. instead of the more sane behavior of opting the user into an
optional library if it's found on the system, and producing an error
if you supply --with-that-library but don't have thatlibrary.so.

Re: What's cooking in git.git (May 2017, #01; Mon, 1)

[Ramsay Jones]

On 01/05/17 06:35, Junio C Hamano wrote:
> 
> * nd/fopen-errors (2017-04-23) 17 commits
>  - warn_failure_to_open_read_optional_path(): a new helper
>  - t1308: add a test case on open a config directory
>  - config.c: handle error on failing to fopen()
>  - xdiff-interface.c: report errno on failure to stat() or fopen()
>  - wt-status.c: report error on failure to fopen()
>  - server-info: report error on failure to fopen()
>  - sequencer.c: report error on failure to fopen()
>  - rerere.c: report correct errno
>  - rerere.c: report error on failure to fopen()
>  - remote.c: report error on failure to fopen()
>  - commit.c: report error on failure to fopen() the graft file
>  - log: report errno on failure to fopen() a file
>  - clone: use xfopen() instead of fopen()
>  - blame: report error on open if graft_file is a directory
>  - bisect: report on fopen() error
>  - config.mak.uname: set FREAD_READS_DIRECTORIES for Linux and FreeBSD
>  - git_fopen: fix a sparse 'not declared' warning
> 
>  We often try to open a file for reading whose existence is
>  optional, and silently ignore errors from open/fopen; report such
>  errors if they are not due to missing files.
> 
>  Expecting a reroll that would be much simplified thanks to a higher
>  level helper.

Since this "What's cooking", this has been re-rolled, but the
"git_fopen: fix a sparse 'not declared' warning" patch has been
dropped, so I'm seeing the sparse warning again.

Also, this series removes every external caller of the
'warn_on_inaccessible()' function, so this could be marked
static in wrapper.c. (I would move the definition of that
function above/before warn_on_fopen_errors(), in order not
to require a forward declaration).

ATB,
Ramsay Jones