What's cooking in git.git (Mar 2018, #02; Tue, 6)

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.

I haven't seen enough reviews (nor I had chance to sufficiently review them myself) to comfortably decide what to do with them on a handful of new topics that appeared in the past week or two, so the ones in the new topics section may not have any description yet in this issue.

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/fetch-prune (2018-02-09) 17 commits
(merged to 'next' on 2018-02-27 at eafb648dd9)
+ fetch: make the --prune-tags work with <url>
+ fetch: add a --prune-tags option and fetch.pruneTags config
+ fetch tests: add scaffolding for the new fetch.pruneTags
+ git-fetch & config doc: link to the new PRUNING section
+ git remote doc: correct dangerous lies about what prune does
+ git fetch doc: add a new section to explain the ins & outs of pruning
+ fetch tests: fetch <url> <spec> as well as fetch [<remote>]
+ fetch tests: expand case/esac for later change
+ fetch tests: double quote a variable for interpolation
+ fetch tests: test --prune and refspec interaction
+ fetch tests: add a tag to be deleted to the pruning tests
+ fetch tests: re-arrange arguments for future readability
+ fetch tests: refactor in preparation for testing tag pruning
+ remote: add a macro for "refs/tags/*:refs/tags/*"
+ fetch: stop accessing "remote" variable indirectly
+ fetch: trivially refactor assignment to ref_nr
+ fetch: don't redundantly NULL something calloc() gave us

"git fetch --prune-tags" may be used as a handy short-hand for getting rid of stale tags that are locally held.

* ab/simplify-perl-makefile (2018-02-15) 1 commit
(merged to 'next' on 2018-02-27 at b0d68a2013)
+ Makefile: generate Git(3pm) as dependency of the 'doc' and 'man' targets

Hotfix for a topic already in 'master'.

* bw/c-plus-plus (2018-02-22) 37 commits
(merged to 'next' on 2018-02-27 at daf85c03de)
+ replace: rename 'new' variables
+ trailer: rename 'template' variables
+ tempfile: rename 'template' variables
+ wrapper: rename 'template' variables
+ environment: rename 'namespace' variables
+ diff: rename 'template' variables
+ environment: rename 'template' variables
+ init-db: rename 'template' variables
+ unpack-trees: rename 'new' variables
+ trailer: rename 'new' variables
+ submodule: rename 'new' variables
+ split-index: rename 'new' variables
+ remote: rename 'new' variables
+ ref-filter: rename 'new' variables
+ read-cache: rename 'new' variables
+ line-log: rename 'new' variables
+ imap-send: rename 'new' variables
+ http: rename 'new' variables
+ entry: rename 'new' variables
+ diffcore-delta: rename 'new' variables
+ diff: rename 'new' variables
+ diff-lib: rename 'new' variable
+ commit: rename 'new' variables
+ combine-diff: rename 'new' variables
+ remote: rename 'new' variables
+ reflog: rename 'new' variables
+ pack-redundant: rename 'new' variables
+ help: rename 'new' variables
+ checkout: rename 'new' variables
+ apply: rename 'new' variables
+ apply: rename 'try' variables
+ diff: rename 'this' variables
+ rev-parse: rename 'this' variable
+ pack-objects: rename 'this' variables
+ blame: rename 'this' variables
+ object: rename function 'typename' to 'type_name'
+ object_info: change member name from 'typename' to 'type_name'

We now avoid using identifiers that clash with C++ keywords. Even though it is not a goal to compile Git with C++ compilers, changes like this help use of code analysis tools that targets C++ on our codebase.

* bw/doc-submodule-recurse-config-with-clone (2018-02-21) 1 commit (merged to 'next' on 2018-02-27 at 5b12841508)
+ submodule: indicate that 'submodule.recurse' doesn't apply to clone

Doc update.

* bw/perl-timegm-timelocal-fix (2018-02-23) 1 commit (merged to 'next' on 2018-02-27 at 565a3141ce)
+ perl: call timegm and timelocal with 4-digit year

Y2k20 fix ;-) for our perl scripts.

* jc/allow-ff-merging-kept-tags (2018-02-16) 1 commit (merged to 'next' on 2018-02-27 at 8b03610d2b)
+ merge: allow fast-forward when merging a tracked tag

Since Git 1.7.9, "git merge" defaulted to --no-ff (i.e. even when the side branch being merged is a descendant of the current commit, create a merge commit instead of fast-forwarding) when merging a tag object. This was appropriate default for integrators who pull signed tags from their downstream contributors, but caused an unnecessary merges when used by downstream contributors who habitually "catch up" their topic branches with tagged releases from the upstream. Update "git merge" to default to --no-ff only when merging a tag object that does *not* sit at its usual place in refs/tags/ hierarchy, and allow fast-forwarding otherwise, to mitigate the problem.

* jk/cached-commit-buffer (2018-02-22) 2 commits (merged to 'next' on 2018-02-27 at af791d9a1e)
+ revision: drop --show-all option
+ commit: drop uses of get_cached_commit_buffer()

Code clean-up.

* jk/strbuf-read-file-close-error (2018-02-23) 1 commit (merged to 'next' on 2018-02-27 at c5dfe33335)
+ strbuf_read_file(): preserve errno across close() call

Code clean-up.

* jk/test-helper-v-output-fix (2018-02-22) 1 commit (merged to 'next' on 2018-02-27 at c9109977e8)
+ t: send verbose test-helper output to fd 4

Test framework update.

* ms/non-ascii-ticks (2018-02-22) 1 commit
(merged to 'next' on 2018-02-27 at 41159fc4f0)
+ Documentation/gitsubmodules.txt: avoid non-ASCII apostrophes

Doc markup fix.

* nd/rebase-show-current-patch (2018-02-12) 3 commits (merged to 'next' on 2018-02-27 at 5a4e23a77c)
+ rebase: introduce and use pseudo-ref REBASE_HEAD
+ rebase: add --show-current-patch
+ am: add --show-current-patch

The new "--show-current-patch" option gives an end-user facing way to get the diff being applied when "git rebase" (and "git am") stops with a conflict.

* nm/tag-edit (2018-02-07) 1 commit
(merged to 'next' on 2018-02-27 at 3bc8345213)
+ tag: add --edit option

"git tag" learned an explicit "--edit" option that allows the message given via "-m" and "-F" to be further edited.

* pw/add-p-single (2018-02-13) 3 commits
(merged to 'next' on 2018-02-27 at 0e2bd585e3)
+ add -p: improve error messages
+ add -p: only bind search key if there's more than one hunk
+ add -p: only display help for active keys

"git add -p" used to offer "/" (look for a matching hunk) as a choice, even there was only one hunk, which has been corrected. Also the single-key help is now given only for keys that are enabled (e.g. help for '/' won't be shown when there is only one hunk).

* rs/strbuf-read-file-or-whine (2018-02-22) 1 commit (merged to 'next' on 2018-02-27 at 56017cb5e2)
+ sequencer: factor out strbuf_read_file_or_whine()

Code clean-up.

* sb/color-h-cleanup (2018-02-13) 1 commit
(merged to 'next' on 2018-02-27 at 617345de77)
+ color.h: document and modernize header
(this branch is used by sb/blame-color.)

Devdoc update.

* sg/t6300-modernize (2018-02-13) 1 commit
(merged to 'next' on 2018-02-27 at b6f13b6915)
+ t6300-for-each-ref: fix "more than one quoting style" tests

Test update.

* sm/mv-dry-run-update (2018-02-07) 2 commits
(merged to 'next' on 2018-02-27 at 17eef62ddf)
+ mv: remove unneeded 'if (!show_only)'
+ t7001: add test case for --dry-run

Code clean-up.

* xz/send-email-batch-size (2018-02-12) 1 commit (merged to 'next' on 2018-02-27 at da0247d532)
+ send-email: error out when relogin delay is missing

"git send-email" learned to complain when the batch-size option is not defined when the relogin-delay option is, since these two are mutually required.

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

* bb/git-gui-ssh-key-files (2018-03-02) 2 commits
- Merge branch 'bb/ssh-key-files' of git-gui into bb/git-gui-ssh-key-files
- git-gui: search for all current SSH key types

* bp/git-gui-bind-kp-enter (2018-03-02) 2 commits
- Merge branch 'bp/bind-kp-enter' of git-gui into bp/git-gui-bind-kp-enter
- git-gui: bind CTRL/CMD+numpad ENTER to do_commit

* cb/git-gui-ttk-style (2018-03-05) 2 commits
- Merge branch 'cb/ttk-style' of git-gui into cb/git-gui-ttk-style
- git-gui: workaround ttk:style theme use

* dp/git-el-ls-files-excludes (2018-03-05) 1 commit
- git.el: handle default excludesfile properly

* jk/add-i-diff-filter (2018-03-05) 2 commits
- add--interactive: detect bogus diffFilter output
- t3701: add a test for interactive.diffFilter

* jk/smart-http-protocol-doc-fix (2018-03-05) 1 commit
- smart-http: document flush after "# service" line

* nd/object-allocation-comments (2018-03-06) 2 commits
- object.h: realign object flag allocation comment
- object.h: update flag allocation comment

* nd/pack-objects-pack-struct (2018-03-05) 9 commits
- pack-objects: reorder 'hash' to pack struct object_entry
- pack-objects: refer to delta objects by index instead of pointer
- pack-objects: move in_pack out of struct object_entry
- pack-objects: move in_pack_pos out of struct object_entry
- pack-objects: note about in_pack_header_size
- pack-objects: use bitfield for object_entry::depth
- pack-objects: use bitfield for object_entry::dfs_state
- pack-objects: turn type and in_pack_type to bitfields
- pack-objects: document holes in struct object_entry.h

* nd/repack-keep-pack (2018-03-06) 5 commits
- pack-objects: display progress in get_object_details()
- pack-objects: show some progress when counting kept objects
- gc --auto: exclude base pack if not enough mem to "repack -ad"
- repack: add --keep-pack option
- t7700: have closing quote of a test at the beginning of line

* nd/worktree-prune (2018-03-06) 3 commits
- worktree prune: improve prune logic when worktree is moved
- worktree: delete dead code
- gc.txt: more details about what gc does

* pw/add-p-select (2018-03-06) 3 commits
- add -p: optimize line selection for short hunks
- add -p: allow line selection to be inverted
- add -p: select individual hunk lines (this branch uses pw/add-p-recount.)

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

* sb/blame-color (2018-02-13) 3 commits
- builtin/blame: highlight recently changed lines
- builtin/blame: add option to color metadata fields separately
- builtin/blame: dim uninteresting metadata

Expecting a reroll.
cf. https://public-inbox.org/git/[email protected]/#t
error messages are funny, can segfault, ...

* av/fsmonitor-updates (2018-01-04) 6 commits
- fsmonitor: use fsmonitor data in git diff
- fsmonitor: remove debugging lines from t/t7519-status-fsmonitor.sh
- fsmonitor: make output of test-dump-fsmonitor more concise
- fsmonitor: update helper tool, now that flags are filled later
- fsmonitor: stop inline'ing mark_fsmonitor_valid / _invalid
- dir.c: update comments to match argument name

Code clean-up on fsmonitor integration, plus optional utilization of the fsmonitor data in diff-files.

Waiting for an update.
cf. <alpine.DEB.2.21.1.1801042335130.32@MININT-6BKU6QN.europe.corp.microsoft.com>

* pb/bisect-helper-2 (2017-10-28) 8 commits
- t6030: make various test to pass GETTEXT_POISON tests
- bisect--helper: bisect_start shell function partially in C
- bisect--helper: get_terms & bisect_terms shell function in C
- bisect--helper: bisect_next_check shell function in C
- bisect--helper: check_and_set_terms shell function in C
- wrapper: move is_empty_file() and rename it as is_empty_or_missing_file()
- bisect--helper: bisect_write shell function in C
- bisect--helper: bisect_reset shell function in C

Expecting a reroll.
cf. <0102015f5e5ee171-f30f4868-886f-47a1-a4e4-b4936afc545d-000000@eu-west-1.amazonses.com>

* dj/runtime-prefix (2017-12-05) 4 commits
. exec_cmd: RUNTIME_PREFIX on some POSIX systems . Makefile: add Perl runtime prefix support . Makefile: add support for "perllibdir"
. Makefile: generate Perl header from template file

A build-time option has been added to allow Git to be told to refer to its associated files relative to the main binary, in the same way that has been possible on Windows for quite some time, for Linux, BSDs and Darwin.

Perhaps it is about time to reboot the effort?

* mk/http-backend-content-length (2017-11-27) 4 commits
- SQUASH???
- t5560-http-backend-noserver.sh: add CONTENT_LENGTH cases
- SQUASH???
- http-backend: respect CONTENT_LENGTH as specified by rfc3875

The http-backend (used for smart-http transport) used to slurp the whole input until EOF, without paying attention to CONTENT_LENGTH that is supplied in the environment and instead expecting the Web server to close the input stream. This has been fixed.

Expecting a reroll.
Suggested fixes to be used when rerolling is queued, but I'd prefer _not_ squashing them myself.

Also, it may be too complex solution for the problem. cf. <[email protected]>

* cc/require-tcl-tk-for-build (2017-11-29) 2 commits
- travis-ci: avoid new tcl/tk build requirement
- Makefile: check that tcl/tk is installed

A first-time builder of Git may have installed neither tclsh nor msgfmt, in which case git-gui and gitk part will fail and break the build. As a workaround, refuse to run a build when tclsh is not installed and NO_TCLTK is not set.

Stalled for too long without any response; will discard. I still feel that requring tclsh to be installed, with or without "escape hatch" for experts, may be too heavy-handed.

* mg/merge-base-fork-point (2017-09-17) 3 commits
- merge-base: find fork-point outside partial reflog
- merge-base: return fork-point outside reflog
- t6010: test actual test output

"merge-base --fork-point $branch $commit" is used to guess on which commit among the commits that were once at the tip of the $branch the $commit was built on top of, and it learns these historical tips from the reflog of the $branch. When the true fork-point is lost due to pruning of old reflog entries, the command does not give any output, because it has no way to guess correctly and does not want to mislead the user with a wrong guess.

The command has been updated to give the best but not known to be correct guess, based on a hope that a merge-base between $commit and a virtual merge across all the reflog entries that still are available for $branch may still be a closer to the true fork-point than the merge-base between $commit and the current tip of the $branch.

This may have to be offered by an additional option, to allow the users that are prepared to see a potentially incorrect guess to opt into the feature, without affecting the current callers that may not be prepared to accept a guess that is not known to be correct.

Stalled for too long without any response; will discard.

* jk/drop-ancient-curl (2017-08-09) 5 commits
- http: #error on too-old curl
- curl: remove ifdef'd code never used with curl >=7.19.4
- http: drop support for curl < 7.19.4
- http: drop support for curl < 7.16.0
- http: drop support for curl < 7.11.1

Some code in http.c that has bitrot is being removed.

Expecting a reroll.

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

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

Needs resurrecting by making sure the fix is good and still applies (or adjusted to today's codebase).

* mg/status-in-progress-info (2017-05-10) 2 commits
- status --short --inprogress: spell it as --in-progress
- 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.

Stalled for too long without any response; will discard. cf. <[email protected]>

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

* sg/travis-build-during-script-phase (2018-01-08) 1 commit (merged to 'next' on 2018-03-02 at 29e1585ae7)
+ travis-ci: build Git during the 'script' phase

Build the executable in 'script' phase in Travis CI integration, to follow the established practice, rather than during 'before_script' phase. This allows the CI categorize the failures better ('failed' is project's fault, 'errored' is build environment's).

Will merge to 'master'.

* np/send-email-header-parsing (2017-12-15) 1 commit
- send-email: extract email-parsing code into a subroutine (this branch is used by cl/send-email-reply-to.)

Code refactoring.

* ld/p4-unshelve (2018-02-22) 1 commit
- git-p4: add unshelve command

"git p4" learned to "unshelve" shelved commit from P4.

Will hold, perhaps drop and use format-change that uses a proper "diff". cf. <CAE5ih7_ooDMqVtTMoQ70s5XCkncr04HY0JkqSp1UmKQeG81oaA@mail.gmail.com>

* ps/contains-id-error-message (2018-03-06) 1 commit
- parse-options: squelch usage help on certain errors

"git tag --contains no-such-commit" gave a full list of options after giving an error message.

Rebooted and fixed the root cause of the issue at a lower level.

* rv/grep-cleanup (2018-02-23) 2 commits (merged to 'next' on 2018-03-02 at 4aafca15f9)
+ grep: simplify grep_oid and grep_file
+ grep: move grep_source_init outside critical section

Threaded "git grep" has been optimized to avoid allocation in code section that is covered under a mutex.

Will merge to 'master'.

* sg/subtree-signed-commits (2018-02-23) 1 commit (merged to 'next' on 2018-03-02 at c5f6fd33e6)
+ subtree: fix add and pull for GPG-signed commits

"git subtree" script (in contrib/) scripted around "git log", whose output got affected by end-user configuration like log.showsignature

Will merge to 'master'.

* ds/find-unique-abbrev-optim (2018-02-27) 1 commit (merged to 'next' on 2018-03-02 at 0b6d4f9335)
+ sha1_name: fix uninitialized memory errors

While finding unique object name abbreviation, the code may accidentally have read beyond the end of the array of object names in a pack.

Will merge to 'master'.

* ds/mark-parents-uninteresting-optim (2018-02-27) 1 commit (merged to 'next' on 2018-03-02 at 5a42c79806)
+ revision.c: reduce object database queries

Micro optimization in revision traversal code.

Will merge to 'master'.

* jc/test-must-be-empty (2018-02-27) 1 commit
(merged to 'next' on 2018-03-02 at ec129f1b97)
+ test_must_be_empty: make sure the file exists, not just empty

Test framework tweak to catch developer thinko.

Will merge to 'master'.

* ma/roll-back-lockfiles (2018-02-28) 5 commits
(merged to 'next' on 2018-03-06 at be29bf891c)
+ sequencer: do not roll back lockfile unnecessarily
+ merge: always roll back lock in checkout_fast_forward()
+ merge-recursive: always roll back lock in merge_recursive_generic()
+ sequencer: always roll back lock in do_recursive_merge()
+ sequencer: make lockfiles non-static
(this branch is used by ma/skip-writing-unchanged-index.)

Some codepaths used to take a lockfile and did not roll it back; they are automatically rolled back at program exit, so there is no real "breakage", but it still is a good practice to roll back when you are done with a lockfile.

Will merge to 'master'.

* mk/doc-pretty-fill (2018-02-27) 1 commit (merged to 'next' on 2018-03-02 at 623461b127)
+ docs/pretty-formats: fix typo '% <(<N>)' -> '%<|(<N>)'

Docfix.

Will merge to 'master'.

* nd/remove-ignore-env-field (2018-03-05) 5 commits
- repository: delete ignore_env member
- sha1_file.c: move delayed getenv(altdb) back to setup_git_env()
- repository.c: delete dead functions
- repository.c: move env-related setup code back to environment.c
- repository: initialize the_repository in main()
(this branch is used by sb/object-store and sb/packfiles-in-repository.)

* rj/test-i18ngrep (2018-02-28) 2 commits (merged to 'next' on 2018-03-06 at 7ea1a2352c)
+ t5536: simplify checking of messages output to stderr
+ t4151: consolidate multiple calls to test_i18ngrep

Test updates.

Will merge to 'master'.

* rs/perf-repeat-thrice-by-default (2018-02-27) 1 commit (merged to 'next' on 2018-03-02 at 4898b3c450)
+ perf: use GIT_PERF_REPEAT_COUNT=3 by default even without config file

Perf test regression fix.

Will merge to 'master'.

* sb/object-store (2018-03-05) 27 commits
- sha1_file: allow sha1_loose_object_info to handle arbitrary repositories
- sha1_file: allow map_sha1_file to handle arbitrary repositories
- sha1_file: allow map_sha1_file_1 to handle arbitrary repositories
- sha1_file: allow open_sha1_file to handle arbitrary repositories
- sha1_file: allow stat_sha1_file to handle arbitrary repositories
- sha1_file: allow sha1_file_name to handle arbitrary repositories
- sha1_file: add repository argument to sha1_loose_object_info
- sha1_file: add repository argument to map_sha1_file
- sha1_file: add repository argument to map_sha1_file_1
- sha1_file: add repository argument to open_sha1_file
- sha1_file: add repository argument to stat_sha1_file
- sha1_file: add repository argument to sha1_file_name
- sha1_file: allow prepare_alt_odb to handle arbitrary repositories
- sha1_file: allow link_alt_odb_entries to handle arbitrary repositories
- sha1_file: add repository argument to prepare_alt_odb
- sha1_file: add repository argument to link_alt_odb_entries
- sha1_file: add repository argument to read_info_alternates
- sha1_file: add repository argument to link_alt_odb_entry
- sha1_file: add raw_object_store argument to alt_odb_usable
- pack: move approximate object count to object store
- pack: move prepare_packed_git_run_once to object store
- object-store: close all packs upon clearing the object store
- object-store: move packed_git and packed_git_mru to object store
- object-store: free alt_odb_list
- object-store: move alt_odb_list and alt_odb_tail to object store
- object-store: migrate alternates struct and functions from cache.h
- repository: introduce raw object store field
(this branch is used by sb/packfiles-in-repository; uses nd/remove-ignore-env-field.)

Refactoring the internal global data structure to make it possible to open multiple repositories, work with and then close them.

Rerolled by Duy on top of a separate preliminary clean-up topic. The resulting structure of the topics looked very sensible.

* sb/packfiles-in-repository (2018-03-05) 12 commits
- packfile: keep prepare_packed_git() private
- packfile: allow find_pack_entry to handle arbitrary repositories
- packfile: add repository argument to find_pack_entry
- packfile: allow reprepare_packed_git to handle arbitrary repositories
- packfile: allow prepare_packed_git to handle arbitrary repositories
- packfile: allow prepare_packed_git_one to handle arbitrary repositories
- packfile: add repository argument to reprepare_packed_git
- packfile: add repository argument to prepare_packed_git
- packfile: add repository argument to prepare_packed_git_one
- packfile: allow install_packed_git to handle arbitrary repositories
- packfile: allow rearrange_packed_git to handle arbitrary repositories
- packfile: allow prepare_packed_git_mru to handle arbitrary repositories (this branch uses nd/remove-ignore-env-field and sb/object-store.)

Refactoring of the internal global data structure continues.

* sg/test-x (2018-02-28) 11 commits
(merged to 'next' on 2018-03-06 at ab0684b27c)
+ travis-ci: run tests with '-x' tracing
+ t/README: add a note about don't saving stderr of compound commands
+ t1510-repo-setup: mark as untraceable with '-x'
+ t9903-bash-prompt: don't check the stderr of __git_ps1()
+ t5570-git-daemon: don't check the stderr of a subshell
+ t5526: use $TRASH_DIRECTORY to specify the path of GIT_TRACE log file
+ t5500-fetch-pack: don't check the stderr of a subshell
+ t3030-merge-recursive: don't check the stderr of a subshell
+ t1507-rev-parse-upstream: don't check the stderr of a shell function
+ t: add means to disable '-x' tracing for individual test scripts
+ t: prevent '-x' tracing from interfering with test helpers' stderr

Running test scripts under -x option of the shell is often not a useful way to debug them, because the error messages from the commands tests try to capture and inspect are contaminated by the tracing output by the shell. An earlier work done to make it more pleasant to run tests under -x with recent versions of bash is extended to cover posix shells that do not support BASH_XTRACEFD.

Will merge to 'master'.

* ab/gc-auto-in-commit (2018-03-01) 1 commit (merged to 'next' on 2018-03-02 at 96a5a4d629)
+ commit: run git gc --auto just before the post-commit hook

"git commit" used to run "gc --auto" near the end, which was lost when the command was reimplemented in C by mistake.

Will merge to 'master'.

* ab/pre-auto-gc-battery (2018-02-28) 1 commit (merged to 'next' on 2018-03-06 at ca9cb273cb)
+ hooks/pre-auto-gc-battery: allow gc to run on non-laptops

A sample auto-gc hook (in contrib/) to skip auto-gc while on battery has been updated to almost always allow running auto-gc unless on_ac_power command is absolutely sure that we are on battery power (earlier, it skipped unless the command is sure that we are on ac power).

Will merge to 'master'.

* ag/userdiff-go-funcname (2018-03-01) 1 commit
(merged to 'next' on 2018-03-02 at ea404d1be9)
+ userdiff: add built-in pattern for golang

"git diff" and friends learned funcname patterns for Go language source files.

Will merge to 'master'.

* ma/skip-writing-unchanged-index (2018-03-01) 1 commit
- write_locked_index(): add flag to avoid writing unchanged index (this branch uses ma/roll-back-lockfiles.)

Internal API clean-up to allow write_locked_index() optionally skip writing the in-core index when it is not modified.

May want to merge into ma/roll-back-lockfiles topic before merging to 'next'.

* jh/status-no-ahead-behind (2018-01-24) 4 commits
(merged to 'next' on 2018-03-02 at 68bde8d571)
+ status: support --no-ahead-behind in long format
+ status: update short status to respect --no-ahead-behind
+ status: add --[no-]ahead-behind to status and commit for V2 format.
+ stat_tracking_info: return +1 when branches not equal

"git status" can spend a lot of cycles to compute the relation between the current branch and its upstream, which can now be disabled with "--no-ahead-behind" option.

Will merge to 'master'.

* nd/tilde-expand-opt-file-value (2018-02-14) 2 commits
- init-db: change --template type to OPTION_FILENAME
- parse-options: expand $HOME on filename options

"git cmd --opt=~u/path/to/file" did not tilde-expand "~u" part to the path to the home directory of user 'u'

Will discard.
This may make the resulting whole more confusing, though. cf. <[email protected]>

* ab/perl-fixes (2018-03-05) 13 commits
- perl Git::LoadCPAN: emit better errors under NO_PERL_CPAN_FALLBACKS
- Makefile: add NO_PERL_CPAN_FALLBACKS knob
- perl: move the perl/Git/FromCPAN tree to perl/FromCPAN
- perl: generalize the Git::LoadCPAN facility
- perl: move CPAN loader wrappers to another namespace
- perl: update our copy of Mail::Address
- perl: update our ancient copy of Error.pm
- git-send-email: unconditionally use Net::{SMTP,Domain}
- Git.pm: hard-depend on the File::{Temp,Spec} modules
- gitweb: hard-depend on the Digest::MD5 5.8 module
- Git.pm: add the "use warnings" pragma
- Git.pm: remove redundant "use strict" from sub-package
- perl: *.pm files should not have the executable bit

Clean-up to various pieces of Perl code we have.

WIll merge to 'next'.

* ds/commit-graph (2018-02-20) 13 commits
- commit-graph: build graph from starting commits
- commit-graph: read only from specific pack-indexes
- commit: integrate commit graph with commit parsing
- commit-graph: close under reachability
- commit-graph: add core.commitGraph setting
- commit-graph: implement --delete-expired
- commit-graph: implement --set-latest
- commit-graph: implement git commit-graph read
- commit-graph: implement 'git-commit-graph write'
- commit-graph: implement write_commit_graph()
- commit-graph: create git-commit-graph builtin
- graph: add commit graph design document
- commit-graph: add format document

Precompute and store information necessary for ancestry traversal in a separate file to optimize graph walking.

Reroll exists, but it appears that there will be a further reroll. cf. <[email protected]>

* ot/ref-filter-cleanup (2018-02-21) 2 commits
(merged to 'next' on 2018-03-02 at 3b4c39a4b5)
+ ref-filter: get rid of goto
+ ref-filter: get rid of duplicate code

Code cleanup.

Will merge to 'master'.

* ma/config-page-only-in-list-mode (2018-02-21) 3 commits
- config: change default of pager.config to "on"
- config: respect pager.config in list/get-mode only
- t7006: add tests for how git config paginates

In a way similar to how "git tag" learned to honor the pager setting only in the list mode, "git config" learned to ignore the pager setting when it is used for setting values (i.e. when the purpose of the operation is not to "show").

Is this ready for 'next'?

* pw/add-p-recount (2018-03-05) 9 commits
(merged to 'next' on 2018-03-06 at 68952f9bb0)
+ add -p: don't rely on apply's '--recount' option
+ add -p: fix counting when splitting and coalescing
+ add -p: calculate offset delta for edited patches
+ add -p: adjust offsets of subsequent hunks when one is skipped
+ t3701: add failing test for pathological context lines
+ t3701: don't hard code sha1 hash values
+ t3701: use test_write_lines and write_script
+ t3701: indent here documents
+ add -i: add function to format hunk header
(this branch is used by pw/add-p-select.)

"git add -p" has been lazy in coalescing split patches before passing the result to underlying "git apply", leading to corner case bugs; the logic to prepare the patch to be applied after hunk selections has been tightened.

Will merge to 'master'.

* bp/untracked-cache-noflush (2018-02-28) 2 commits
(merged to 'next' on 2018-03-02 at 709887971b)
+ untracked cache: use git_env_bool() not getenv() for customization
+ dir.c: don't flag the index as dirty for changes to the untracked cache

Writing out the index file when the only thing that changed in it is the untracked cache information is often wasteful, and this has been optimized out.

Will merge to 'master'.

* nd/diff-stat-with-summary (2018-02-27) 2 commits (merged to 'next' on 2018-03-06 at d543f92f5e)
+ diff: add --compact-summary
+ diff.c: refactor pprint_rename() to use strbuf

"git diff" and friends learned "--compact-summary" that shows the information usually given with the "--summary" option on the same line as the diffstat output of the "--stat" option (which saves vertical space and keeps info on a single path at the same place).

Will merge to 'master'.

* nd/parseopt-completion (2018-03-06) 44 commits
- SQUASH???
- completion: simplify _git_notes
- completion: don't set PARSE_OPT_NOCOMPLETE on --rerere-autoupdate (merged to 'next' on 2018-03-02 at d72a6525fd)
+ completion: use __gitcomp_builtin in _git_worktree
+ completion: use __gitcomp_builtin in _git_tag
+ completion: use __gitcomp_builtin in _git_status
+ completion: use __gitcomp_builtin in _git_show_branch
+ completion: use __gitcomp_builtin in _git_rm
+ completion: use __gitcomp_builtin in _git_revert
+ completion: use __gitcomp_builtin in _git_reset
+ completion: use __gitcomp_builtin in _git_replace
+ remote: force completing --mirror= instead of --mirror
+ completion: use __gitcomp_builtin in _git_remote
+ completion: use __gitcomp_builtin in _git_push
+ completion: use __gitcomp_builtin in _git_pull
+ completion: use __gitcomp_builtin in _git_notes
+ completion: use __gitcomp_builtin in _git_name_rev
+ completion: use __gitcomp_builtin in _git_mv
+ completion: use __gitcomp_builtin in _git_merge_base
+ completion: use __gitcomp_builtin in _git_merge
+ completion: use __gitcomp_builtin in _git_ls_remote
+ completion: use __gitcomp_builtin in _git_ls_files
+ completion: use __gitcomp_builtin in _git_init
+ completion: use __gitcomp_builtin in _git_help
+ completion: use __gitcomp_builtin in _git_grep
+ completion: use __gitcomp_builtin in _git_gc
+ completion: use __gitcomp_builtin in _git_fsck
+ completion: use __gitcomp_builtin in _git_fetch
+ completion: use __gitcomp_builtin in _git_difftool
+ completion: use __gitcomp_builtin in _git_describe
+ completion: use __gitcomp_builtin in _git_config
+ completion: use __gitcomp_builtin in _git_commit
+ completion: use __gitcomp_builtin in _git_clone
+ completion: use __gitcomp_builtin in _git_clean
+ completion: use __gitcomp_builtin in _git_cherry_pick
+ completion: use __gitcomp_builtin in _git_checkout
+ completion: use __gitcomp_builtin in _git_branch
+ completion: use __gitcomp_builtin in _git_apply
+ completion: use __gitcomp_builtin in _git_am
+ completion: use __gitcomp_builtin in _git_add
+ git-completion.bash: introduce __gitcomp_builtin
+ parse-options: let OPT__FORCE take optional flags argument
+ parse-options: add OPT_xxx_F() variants
+ parse-options: support --git-completion-helper

Teach parse-options API an option to help the completion script, and make use of the mechanism in command line completion.

Will merge to 'master'.

* pc/submodule-helper-foreach (2018-02-02) 5 commits
- submodule: port submodule subcommand 'foreach' from shell to C
- submodule foreach: document variable '$displaypath'
- submodule foreach: clarify the '$toplevel' variable documentation
- submodule foreach: document '$sm_path' instead of '$path'
- submodule foreach: correct '$path' in nested submodules from a subdirectory

Expecting a response to review comments
e.g. cf. <[email protected]>

* tg/worktree-add-existing-branch (2018-02-05) 3 commits
- worktree: teach "add" to check out existing branches
- worktree: be clearer when "add" dwim-ery kicks in
- worktree: improve message when creating a new worktree

"git worktree add" learned to check out an existing branch.

Expecting a reroll.
cf. <CAPig+cRLohiqR_Drh7P0q3XbvC22WLjNwH0YLZo3dqFzZZuAPw@mail.gmail.com> cf. <CACsJy8BEKYqW+Ne_WY2RBaSbb9OKcjREtrawStj=eJsVsia_Jw@mail.gmail.com> The general idea is good, just end-user facing messages are found suboptimal.

* nd/worktree-move (2018-03-06) 8 commits
(merged to 'next' on 2018-03-06 at a26271e7de)
+ t2028: fix minor error and issues in newly-added "worktree move" tests (merged to 'next' on 2018-03-02 at 5c514dfc92)
+ worktree remove: allow it when $GIT_WORK_TREE is already gone
+ 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.

Will merge to 'master'.

* cl/send-email-reply-to (2018-03-06) 2 commits
- send-email: support separate Reply-To address
- send-email: rename variable for clarity
(this branch uses np/send-email-header-parsing.)

"git send-email" learned "--reply-to=<address>" option.

Will merge to 'next'.

* js/rebase-recreate-merge (2018-02-23) 12 commits
- rebase -i: introduce --recreate-merges=[no-]rebase-cousins
- pull: accept --rebase=recreate to recreate the branch topology
- sequencer: handle post-rewrite for merge commands
- sequencer: make refs generated by the label command worktree-local
- rebase: introduce the --recreate-merges option
- rebase-helper --make-script: introduce a flag to recreate merges
- sequencer: fast-forward merge commits, if possible
- sequencer: introduce the merge command
- sequencer: introduce new commands to reset the revision
- git-rebase--interactive: clarify arguments
- sequencer: make rearrange_squash() a bit more obvious
- sequencer: avoid using errno clobbered by rollback_lock_file()

"git rebase" learned "--recreate-merges" to transplant the whole topology of commit graph elsewhere.

Will merge to 'next'.

* bw/protocol-v2 (2018-03-02) 36 commits
- SQUASH???
- remote-curl: don't request v2 when pushing
- remote-curl: implement stateless-connect command
- http: eliminate "# service" line when using protocol v2
- http: don't always add Git-Protocol header
- http: allow providing extra headers for http requests
- remote-curl: store the protocol version the server responded with
- remote-curl: create copy of the service name
- pkt-line: add packet_buf_write_len function
- transport-helper: introduce stateless-connect
- transport-helper: refactor process_connect_service
- transport-helper: remove name parameter
- connect: don't request v2 when pushing
- connect: refactor git_connect to only get the protocol version once
- fetch-pack: support shallow requests
- fetch-pack: perform a fetch using v2
- upload-pack: introduce fetch server command
- push: pass ref patterns when pushing
- fetch: pass ref patterns when fetching
- ls-remote: pass ref patterns when requesting a remote's refs
- transport: convert transport_get_remote_refs to take a list of ref patterns
- transport: convert get_refs_list to take a list of ref patterns
- connect: request remote refs using v2
- ls-refs: introduce ls-refs server command
- serve: introduce git-serve
- test-pkt-line: introduce a packet-line test helper
- protocol: introduce enum protocol_version value protocol_v2
- transport: store protocol version
- connect: discover protocol version outside of get_remote_heads
- connect: convert get_remote_heads to use struct packet_reader
- transport: use get_refs_via_connect to get refs
- upload-pack: factor out processing lines
- upload-pack: convert to a builtin
- pkt-line: add delim packet support
- pkt-line: allow peeking a packet line without consuming it
- pkt-line: introduce packet_read_with_status

The beginning of the next-gen transfer protocol.

* ls/checkout-encoding (2018-03-06) 8 commits
- convert: add round trip check based on 'core.checkRoundtripEncoding'
- convert: add tracing for 'working-tree-encoding' attribute
- convert: check for detectable errors in UTF encodings
- convert: add 'working-tree-encoding' attribute
- utf8: add function to detect a missing UTF-16/32 BOM
- utf8: add function to detect prohibited UTF-16/32 BOM
- strbuf: add xstrdup_toupper()
- strbuf: remove unnecessary NUL assignment in xstrdup_tolower()

The new "checkout-encoding" attribute can ask Git to convert the contents to the specified encoding when checking out to the working tree (and the other way around when checking in).

Expecting a reroll; it is almost there, though. cf. <[email protected]>

* en/rename-directory-detection (2018-02-27) 29 commits
(merged to 'next' on 2018-03-06 at d42470f86e)
+ merge-recursive: ensure we write updates for directory-renamed file
+ merge-recursive: avoid spurious rename/rename conflict from dir renames
+ directory rename detection: new testcases showcasing a pair of bugs
+ merge-recursive: fix remaining directory rename + dirty overwrite cases
+ merge-recursive: fix overwriting dirty files involved in renames
+ merge-recursive: avoid clobbering untracked files with directory renames
+ merge-recursive: apply necessary modifications for directory renames
+ merge-recursive: when comparing files, don't include trees
+ merge-recursive: check for file level conflicts then get new name
+ merge-recursive: add computation of collisions due to dir rename & merging
+ merge-recursive: check for directory level conflicts
+ merge-recursive: add get_directory_renames()
+ merge-recursive: make a helper function for cleanup for handle_renames
+ merge-recursive: split out code for determining diff_filepairs
+ merge-recursive: make !o->detect_rename codepath more obvious
+ merge-recursive: fix leaks of allocated renames and diff_filepairs
+ merge-recursive: introduce new functions to handle rename logic
+ merge-recursive: move the get_renames() function
+ directory rename detection: tests for handling overwriting dirty files
+ directory rename detection: tests for handling overwriting untracked files
+ directory rename detection: miscellaneous testcases to complete coverage
+ directory rename detection: testcases exploring possibly suboptimal merges
+ directory rename detection: more involved edge/corner testcases
+ directory rename detection: testcases checking which side did the rename
+ directory rename detection: files/directories in the way of some renames
+ directory rename detection: partially renamed directory testcase/discussion
+ directory rename detection: testcases to avoid taking detection too far
+ directory rename detection: directory splitting testcases
+ directory rename detection: basic testcases

Rename detection logic in "diff" family that is used in "merge" has learned to guess when all of x/a, x/b and x/c have moved to z/a, z/b and z/c, it is likely that x/d added in the meantime would also want to move to z/d by taking the hint that the entire directory 'x' moved to 'z'. A bug causing dirty files involved in a rename to be overwritten during merge has also been fixed as part of this work.

Will merge to 'master'.

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

* ot/cat-batch-format (2018-02-12) 23 commits . cat-file: update of docs
. cat-file: tests for new atoms added
. for-each-ref: tests for new atoms added
. ref-filter: unifying formatting of cat-file opts . ref-filter: make populate_value() internal again . cat-file: reuse printing logic from ref-filter . ref-filter: make valid_atom general again . ref-filter: make cat_file_info independent . cat-file: move skip_object_info into ref-filter . ref_filter: add is_atom_used function
. ref-filter: get rid of mark_atom_in_object_info() . cat-file: start reusing populate_value()
. ref-filter: rename field in ref_array_item stuct . ref-filter: make populate_value() global
. cat-file: start use ref_array_item struct . ref-filter: reuse parse_ref_filter_atom() . cat-file: start migrating formatting to ref-filter . cat-file: split expand_atom() into 2 functions . cat-file: move struct expand_data into ref-filter . ref-filter: make valid_atom as function parameter . cat-file: reuse struct ref_format
. ref-filter: add return value to some functions . ref-filter: get rid of goto

Teach "cat-file --batch" to reuse the formatting machinery shared by for-each-ref, branch --list, and tag --list.

Discarded, as a rebooted effort is beginning elsewhere. Allocates flex-array on stack, etc.
cf. <[email protected]>

Hi Dan,

You probably missed this in the huge "What's cooking" mail. Are you game?

Ciao,
Johannes

On Wed, Mar 07 2018, Johannes Schindelin jotted:

It would be great to have this rebooted now that my perl cleanup efforts have un-blocked this. Will be happy to help review & test the next iteration.

Yes, I was just thinking the same thing. I wanted to make sure the Perl changes had landed, and I'm pleased to see that they have. I should have time in the next few days to rebase and put up a new version of the patch series. I'll keep you in the loop, and thanks for pinging!

On Thu, Mar 08 2018, Daniel Jacques jotted:

Related to this, I came across this bug report
https://gitlab.com/gitlab-org/omnibus-gitlab/issues/3265 which is wondering why we're installing N copies of the git binary, presumably they're building with NO_INSTALL_HARDLINKS.

Just doing this:

    diff --git a/Makefile b/Makefile
    index de4b8f0c02..2222319a4f 100644
    --- a/Makefile
    +++ b/Makefile
    @@ -2596,7 +2596,7 @@ endif
              for p in git$X $(filter $(install_bindir_programs),$(ALL_PROGRAMS)); do \
                    $(RM) "$$execdir/$$p" && \
                    test -z "$(NO_INSTALL_HARDLINKS)$(NO_CROSS_DIRECTORY_HARDLINKS)" && \
    -               ln "$$bindir/$$p" "$$execdir/$$p" 2>/dev/null || \
    +               ln -s "$$bindir/$$p" "$$execdir/$$p" 2>/dev/null || \
                    cp "$$bindir/$$p" "$$execdir/$$p" || exit; \
              done; \
            } && \

Seems to work for me, although obviously this would need to be optional, and it'll get in the way of Daniel's patch since it use the absolute path.

But is there any reason anyone can think of for why we shouldn't be figuring out the relative path and symlinking the two?

There is no fundamental reason not to offer such an "install" method as an option; unless you count a more philosophical aversion to use symlinks due to (perceived) additional fragility, that is.

The resulting code may become messier than without, but as long as it is without the reasonable range for usual price we would pay for a new "feature", that would be tolerable, I guess.

A possible (remote) reason for not doing this is in environments using ACLs that somehow want different access permissions on some functions vs. others AND where the platform does not have the ability to separately secure links vs. objects. I don't know of such an environment, but you never know. I know it's a stretch, but I can see security-types being worried about this. I do know of environments where /usr/local/lib is secured differently from /usr/local/bin to prevent inappropriate .so loads on a selective basis, so there's that. Again, this is a stretch. As long as we continue to have a method of forcing the expensive way for the paranoidly inclined ;) -- not meaning offence to those, of course.

Cheers,
Randall

-- Brief whoami:
NonStop developer since approximately 211288444200000000 UNIX developer since approximately 421664400
-- In my real life, I talk too much.

On Tue, Mar 13 2018, Junio C. Hamano jotted:

Cool. I think it makes sense for us to have this. Here's an implementation of it. The 3/3 patch looks a bit scary, but "git show" with --word-diff will show that the change is minimal.

This steals a small piece from Daniel's relocatable series, and doesn't in any way conflict with it. None of this will need to be fixed up to make git relocatable since all the symlinks are relative already.

Ævar Arnfjörð Bjarmason (3):
Makefile: fix broken bindir_relative variable
Makefile: add a gitexecdir_relative variable
Makefile: optionally symlink libexec/git-core binaries to bin/git

 Makefile | 52 +++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 35 insertions(+), 17 deletions(-)

2.15.1.424.g9478a66081

Change the bindir_relative variable to work like the other *_relative variables, which are computed as a function of the absolute path. Before this change, supplying e.g. bindir=/tmp/git/binaries to the Makefile would yield a bindir_relative of just "bin", as opposed to "binaries".

This logic was originally added back in 026fa0d5ad ("Move computation of absolute paths from Makefile to runtime (in preparation for RUNTIME_PREFIX)", 2009-01-18), then later in 971f85388f ("Makefile:
make mandir, htmldir and infodir absolute", 2013-02-24) when more *_relative variables were added those new variables didn't have this bug, but bindir_relative was never fixed.

There is a small change in behavior here, which is that setting bindir_relative as an argument to the Makefile won't work anymore, I think that's fine, since this was always intended as an internal variable (e.g. INSTALL documents bindir=*, not bindir_relative=*).

---
 Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

index de4b8f0c02..b2f8f2b171 100644
@@ -468,8 +468,7 @@ ARFLAGS = rcs
 # This can help installing the suite in a relocatable way.
 
 prefix = $(HOME)
-bindir_relative = bin
-bindir = $(prefix)/$(bindir_relative)
+bindir = $(prefix)/bin
 mandir = $(prefix)/share/man
 infodir = $(prefix)/share/info
 gitexecdir = libexec/git-core

@@ -486,6 +485,7 @@ lib = lib
 # DESTDIR =
 pathsep = :
 
+bindir_relative = $(patsubst $(prefix)/%,%,$(bindir))
 mandir_relative = $(patsubst $(prefix)/%,%,$(mandir))
 infodir_relative = $(patsubst $(prefix)/%,%,$(infodir))
 htmldir_relative = $(patsubst $(prefix)/%,%,$(htmldir))

2.15.1.424.g9478a66081

This variable will be e.g. "libexec/git-core" if gitexecdir=/tmp/git/libexec/git-core is given. It'll be used by a subsequent change.

This is stolen from the yet-to-be integrated (needs resubmission)
"Makefile: add Perl runtime prefix support" patch on the mailing
list. See
<[email protected]> (https://public-inbox.org/git/[email protected]/).

---
 Makefile | 2 ++
 1 file changed, 2 insertions(+)

index b2f8f2b171..ee0b6c8940 100644
@@ -488,6 +488,7 @@ pathsep = :
 bindir_relative = $(patsubst $(prefix)/%,%,$(bindir))
 mandir_relative = $(patsubst $(prefix)/%,%,$(mandir))
 infodir_relative = $(patsubst $(prefix)/%,%,$(infodir))
+gitexecdir_relative = $(patsubst $(prefix)/%,%,$(gitexecdir))
 htmldir_relative = $(patsubst $(prefix)/%,%,$(htmldir))
 
 export prefix bindir sharedir sysconfdir gitwebdir perllibdir localedir

@@ -1735,6 +1736,7 @@ infodir_relative_SQ = $(subst ','\'',$(infodir_relative))
 perllibdir_SQ = $(subst ','\'',$(perllibdir))
 localedir_SQ = $(subst ','\'',$(localedir))
 gitexecdir_SQ = $(subst ','\'',$(gitexecdir))
+gitexecdir_relative_SQ = $(subst ','\'',$(gitexecdir_relative))
 template_dir_SQ = $(subst ','\'',$(template_dir))
 htmldir_relative_SQ = $(subst ','\'',$(htmldir_relative))
 prefix_SQ = $(subst ','\'',$(prefix))

2.15.1.424.g9478a66081

Add a INSTALL_SYMLINKS option which if enabled, changes the default hardlink installation method to one where the relevant binaries in libexec/git-core are symlinked back to ../../bin, instead of being hardlinked.

This new option also overrides the behavior of the existing NO_*_HARDLINKS variables which in some cases would produce symlinks within to libexec/, e.g. "git-add" symlinked to "git" which would be copy of the "git" found in bin/, now "git-add" in libexec/ is always going to be symlinked to the "git" found in the bin/ directory.

This option is being added because:

1) I think it makes what we're doing a lot more obvious. E.g. I'd never noticed that the libexec binaries were really just hardlinks since e.g. ls(1) won't show that in any obvious way. You need to start stat(1)-ing things and look at the inodes to see what's going on.

2) Some tools have very crappy support for hardlinks, e.g. the Git shipped with GitLab is much bigger than it should be because they're using a chef module that doesn't know about hardlinks, see
https://github.com/chef/omnibus/issues/827

I've also ran into other related issues that I think are explained by this, e.g. compiling git with debugging and rpm refusing to install a ~200MB git package with 2GB left on the FS, I think that was because it doesn't consider hardlinks, just the sum of the byte size of everything in the package.

As for the implementation, the "../../bin" noted above will vary given some given some values of "../.." and "bin" depending on the depth of the gitexecdir relative to the destdir, and the "bindir" target, e.g. setting "bindir=/tmp/git/binaries gitexecdir=foo/bar/baz" will do the right thing and produce this result:

    $ file /tmp/git/foo/bar/baz/git-add
    /tmp/git/foo/bar/baz/git-add: symbolic link to ../../../binaries/git

---
 Makefile | 46 +++++++++++++++++++++++++++++++---------------
 1 file changed, 31 insertions(+), 15 deletions(-)

index ee0b6c8940..ac7616422d 100644
@@ -329,6 +329,13 @@ all::
 # when hardlinking a file to another name and unlinking the original file right
 # away (some NTFS drivers seem to zero the contents in that scenario).
 #
+# Define INSTALL_SYMLINKS if you prefer to have everything that can be
+# symlinked between bin/ and libexec/ to use relative symlinks between
+# the two. This option overrides NO_CROSS_DIRECTORY_HARDLINKS and
+# NO_INSTALL_HARDLINKS which will also use symlinking by indirection
+# within the same directory in some cases, INSTALL_SYMLINKS will
+# always symlink to the final target directly.
+#
 # Define NO_CROSS_DIRECTORY_HARDLINKS if you plan to distribute the installed
 # programs as a tar, where bin/ and libexec/ might be on different file systems.
 #

@@ -2594,35 +2601,44 @@ endif
 
 	bindir=$$(cd '$(DESTDIR_SQ)$(bindir_SQ)' && pwd) && \
 	execdir=$$(cd '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' && pwd) && \
+	destdir_from_execdir_SQ=$$(echo '$(gitexecdir_relative_SQ)' | sed -e 's|[^/][^/]*|..|g') && \
 	{ test "$$bindir/" = "$$execdir/" || \
 	  for p in git$X $(filter $(install_bindir_programs),$(ALL_PROGRAMS)); do \
 		$(RM) "$$execdir/$$p" && \
-		test -z "$(NO_INSTALL_HARDLINKS)$(NO_CROSS_DIRECTORY_HARDLINKS)" && \
-		ln "$$bindir/$$p" "$$execdir/$$p" 2>/dev/null || \
-		cp "$$bindir/$$p" "$$execdir/$$p" || exit; \
+		test -n "$(INSTALL_SYMLINKS)" && \
+		ln -s "$$destdir_from_execdir_SQ/$(bindir_relative_SQ)/$$p" "$$execdir/$$p" || \
+		{ test -z "$(NO_INSTALL_HARDLINKS)$(NO_CROSS_DIRECTORY_HARDLINKS)" && \
+		  ln "$$bindir/$$p" "$$execdir/$$p" 2>/dev/null || \
+		  cp "$$bindir/$$p" "$$execdir/$$p" || exit; } \
 	  done; \
 	} && \
 	for p in $(filter $(install_bindir_programs),$(BUILT_INS)); do \
 		$(RM) "$$bindir/$$p" && \
-		test -z "$(NO_INSTALL_HARDLINKS)" && \
-		ln "$$bindir/git$X" "$$bindir/$$p" 2>/dev/null || \
-		ln -s "git$X" "$$bindir/$$p" 2>/dev/null || \
-		cp "$$bindir/git$X" "$$bindir/$$p" || exit; \
+		test -n "$(INSTALL_SYMLINKS)" && \
+		ln -s "git$X" "$$bindir/$$p" || \
+		{ test -z "$(NO_INSTALL_HARDLINKS)" && \
+		  ln "$$bindir/git$X" "$$bindir/$$p" 2>/dev/null || \
+		  ln -s "git$X" "$$bindir/$$p" 2>/dev/null || \
+		  cp "$$bindir/git$X" "$$bindir/$$p" || exit; } \
 	done && \
 	for p in $(BUILT_INS); do \
 		$(RM) "$$execdir/$$p" && \
-		test -z "$(NO_INSTALL_HARDLINKS)" && \
-		ln "$$execdir/git$X" "$$execdir/$$p" 2>/dev/null || \
-		ln -s "git$X" "$$execdir/$$p" 2>/dev/null || \
-		cp "$$execdir/git$X" "$$execdir/$$p" || exit; \
+		test -n "$(INSTALL_SYMLINKS)" && \
+		ln -s "$$destdir_from_execdir_SQ/$(bindir_relative_SQ)/git$X" "$$execdir/$$p" || \
+		{ test -z "$(NO_INSTALL_HARDLINKS)" && \
+		  ln "$$execdir/git$X" "$$execdir/$$p" 2>/dev/null || \
+		  ln -s "git$X" "$$execdir/$$p" 2>/dev/null || \
+		  cp "$$execdir/git$X" "$$execdir/$$p" || exit; } \
 	done && \
 	remote_curl_aliases="$(REMOTE_CURL_ALIASES)" && \
 	for p in $$remote_curl_aliases; do \
 		$(RM) "$$execdir/$$p" && \
-		test -z "$(NO_INSTALL_HARDLINKS)" && \
-		ln "$$execdir/git-remote-http$X" "$$execdir/$$p" 2>/dev/null || \
-		ln -s "git-remote-http$X" "$$execdir/$$p" 2>/dev/null || \
-		cp "$$execdir/git-remote-http$X" "$$execdir/$$p" || exit; \
+		test -n "$(INSTALL_SYMLINKS)" && \
+		ln -s "git-remote-http$X" "$$execdir/$$p" || \
+		{ test -z "$(NO_INSTALL_HARDLINKS)" && \
+		  ln "$$execdir/git-remote-http$X" "$$execdir/$$p" 2>/dev/null || \
+		  ln -s "git-remote-http$X" "$$execdir/$$p" 2>/dev/null || \
+		  cp "$$execdir/git-remote-http$X" "$$execdir/$$p" || exit; } \
 	done && \
 	./check_bindir "z$$bindir" "z$$execdir" "$$bindir/git-add$X"
 

2.15.1.424.g9478a66081

Am 13.03.2018 um 21:39 schrieb Ævar Arnfjörð Bjarmason:

It is important to leave the default at hard-linking the binaries, because on Windows symbolic links are second class citizens (they require special privileges and there is a distinction between link targets being files or directories). Hard links work well.

s/ran/run/

s/given some//

	want symlinks? &&
	make symlinks ||
	want hard links? &&
	make hard links ||
	make copies ||
	exit

On Wed, Mar 14 2018, Johannes Sixt jotted:

Yeah makes sense. I just want to add this as an option, and think if it's proven to be un-buggy we could probably turn it on by default on the *nix's if people prefer that, but yeah, we'll definitely need the uname detection.

Thanks. Will fix the above in a subsequent submission.

I'm not explaining this well, but what I mean is that right now if you supply NO_INSTALL_HARDLINKS you end up with this:

    bin/git
    libexec/git
    libexec/git-add -> git

I.e. we make two copies of the "git" binary, and then just symlink within the libexec dir, whereas with this change:

    bin/git
    libexec/git -> ../bin/git
    libexec/git-add -> ../bin/git

I.e. we'll only install one "git" and never copy it, and to the extent that we need symlinking we're always going to symlink directly to the binary, i.e. not:

    bin/git
    libexec/git -> ../bin/git
    libexec/git-add -> git

Even though that would also work, I just don't think it makes sense.

Yes. This is shitty, I'll change it. I'm going to inject a patch series earlier in this series so we'll do:

want symlinks? &&
make symlinks ||
want hard links? &&
make hard links ||
want copy fallback? &&
make copies ||
exit

And then turn on the copy fallback by default. Right now we'll silently hide errors during "make install", which isn't nice. I'll leave the fallback on by default (for now), but I'd like to declare that I want to install symlinks, and if that fails I should get an error, not a silent fallback to copy.

I definitely would prefer to make symlinks the default on unix.

It's what we used to do (long long ago), and as you pointed out, it's a lot clearer what's going on too when you don't have to look at inode numbers and link counts.

Forcing hardlinking everywhere by default just because Windows filesystems suck donkey ass through a straw is not the right thing either.

Linus

Hi Linus,

The most sensible thing, of course, would be to *not* link the builtins at all. I mean, we deprecated the dashed form (which was a design mistake, whether you admit it or not) a long time ago.

Ciao,
Johannes

That's probably not a bad idea for the builtin commands. At least as an option.

We do end up still using the dashed form for certain things, but they are already special-cased (ie things like "git-receive-pack" and "git-shell" that very much get executed directly, and for fundamental reasons).

As to it being a design mistake? No, not really. It made a lot of sense at the time. The fact is, the problem is Windows, not git. I know you have your hangups, but that's your problem.

Linus

Hi Linus.

Please do elaborate.

Ciao,
Johannes

On Fri, Mar 16 2018, Johannes Schindelin jotted:

If you were to set set "/bin/git shell" in /etc/password it would not do the right thing as far as I know. Is that a shell name with a space in it, or the "shell" argument to /bin/git?

There's also the fully dashed forms of stuff like git-receive-pack is part of the over-ssh convention, i.e.:

ssh <host> git-upload-pack ...

That being said I think Linus is conflating two things here. If we still had just the dashed forms on *nix we'd still have the issue of what it does to shell completion, which is one thing that got brought up in the discussion to create the "git" wrapper at the time. There were also other reasons IIRC.

That's an entirely separate discussion from how we go about either hard- or symlinking some stuff git is using, whether or not that's ever directly exposed to the user.

Having said that I have a WIP re-roll which where I'm aiming to:

* Add a NO_INSTALL_CP_FALLBACK flag, so we won't implicitly fall back to cp silently (unless told so)

* Remove the 2>/dev/null we're doing on everything. That pre-dates the NO_*_*HARDLINKS flags and we shouldn't be doing that anymore.

* Add an option where we optionally won't install the majority of these dashed forms, regardless of whether we choose hardlinks or symlinks. We'll still need some linking as some dashed forms we can't remove, as noted above.

I didn't expect Junio to merge this down to next so fast, so I'll wait until INSTALL_SYMLINKS lands. As far as I know the code as-is in next isn't buggy, I'd just like to improve it a bit more, so I'll need to rebase what I have on top of that (which is fine).

Hi Ævar,

True. And git-shell is not a builtin, so it does not even matter with regards to this discussion.

Even if upload-pack is not a builtin (and thus still has to be its own executable), receive-pack *is*, so this does affect our current discussion.

This is a real problem. And it is our own darned fault because we let an implementation detail bleed into a protocol. We could have designed that a lot better.

Of course we should fix this, though. There is literally no good reason that I can think of why we should not change this to ssh <host> git upload-pack ... (of course with an insanely long deprecation period).

Ciao,
Dscho

No it isn't.

We already handle those special cases specially, and install them in the bin directory (as opposed to libexec). And it all works fine.

Look into the bin directory some day. You'll find things like

git-cvsserver
gitk
git-receive-pack
git-shell
git-upload-archive
git-upload-pack

there, and the fact that a couple of them happen to be built-ins is an IMPLEMENTATION DETAIL, not a "Oh we should have used just 'git' for them".

The design of having separate programs is the *good* conceptual design. And we damn well should keep it for these things that are used for special purposes.

The fact that two of them have become built-ins as part of the git binary is incidental. It shouldn't be visible in the names, because it really is just an internal implementation thing, not anything fundamental.

And by "we" you clearly mean "not you", and by "we could have designed that a lot better" you must mean "and it was very well designed by competent people who didn't use bad operating systems".

Go away.

We shouldn't fix it, it's all fine as-is, and there were tons of f*cking good reasons for why git did what it did. The main one being "it's a collection of scripts", which was what git _was_, for chrissake. And using spaces and running some idiotic and hard-to-verify script de-multiplexer is the WRONG THING for things like "git-shell" and "git-receive-pack" and friends.

Right now you can actually verify exactly what "git-shell" does. Or you could replace - or remove - it entirely if you don't like it. And never have to worry about running "git" with some "shell" subcommand.

And you know that it's not an alias, for example. Because "git-xyz" simply does not look up aliases.

So really. Go away, Johannes. Your concerns are complete and utter BS.

The real problem is that Windows is badly designed, but since it's easy to work around (by using hard-linking on Windows), nobody sane cares.

The solution is simple, and was already suggested: use symlinks (like we used to!) on non-windows systems. End of story.

And for the libexec thing, we might want to deprecate those names, if somebody wants to, but it's not like it actually hurts, and it gives backwards compatibility.

Btw, real Windows people know all about backwards compatibility. Ask around competent people inside MS whether it's an important thing.

So stop this idiotic "bad design" crap. Somebody working on Windows simply can't afford your attitude.

Somebody who didn't design it in the first place can't afford your attitude.

Linus

I think up to patch 4 here should be near a state that's ready for inclusion.

Although I'm on the fence with the approach in 1/5. Should this be a giant getopt switch statement like that in a helper script? An alternative would be to write out a shell file similar to GIT-BUILD-OPTIONS and source that from this thing. I don't know, what do you all think?

The idea with 4/5 was to make this symlink mode the default in config.mak.uname and have a blacklist of systems like Windows that couldn't deal with it.

Since my ad874608d8 ("Makefile: optionally symlink libexec/git-core binaries to bin/git", 2018-03-13) I see that e.g. Debian and GitLab have started shipping with the INSTALL_SYMLINKS flag, so making that unconditional is the next logical step.

The 5th one is more radical. See
https://public-inbox.org/git/[email protected]/ from back in March for context.

I'd like to say it's ready, but I've spotted some fallout:

* Help like "git ninit" suggesting "git init" doesn't work, this is because load_command_list() in help.c doesn't look out our in-memory idea of builtins, it reads the libexecdir, so if we don't have the programs there it doesn't know about it.

* GIT_TEST_INSTALLED breaks entirely under this, as early as the heuristic for "are we built?" being "do we have git-init in libexecdir?". I tried a bit to make this work, but there's a lot of dependencies there.

* We still (and this is also true of my ad874608d8) hardlink everything in the build dir via a different part of the Makefile, ideally we should do exactly the same thing there so also normal tests and not just GIT_TEST_INSTALLED (if that worked) would test in the same mode.

I gave making that work a bit of a try and gave up in the Makefile jungle.

Ævar Arnfjörð Bjarmason (5):
Makefile: move long inline shell loops in "install" into helper
Makefile: conform some of the code to our coding standards
Makefile: stop hiding failures during "install"
Makefile: add NO_INSTALL_SYMLINKS_FALLBACK switch
Makefile: Add a NO_INSTALL_BUILTIN_EXECDIR_ALIASES flag

 Makefile         |  65 +++++++++++--------------
 install_programs | 124 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 151 insertions(+), 38 deletions(-)
 create mode 100755 install_programs

2.19.1.930.g4563a0d9d0

Not really. Why do we iterate over these in a shell loop, rather than having make to figure them out, just like we do when we "loop over the source files and turn them into object files" without using a shell loop? What's so special about enumerating the installation targets and iterating over the enumeration to perform an action on each of them? I think that is the first question we should be asking before patch 1/5, which already assumes that it has been decided that it must be done with a shell loop.

I think "first install 'git' itself, and then make these other things derived from it" should let $(MAKE) install things in parallel just like it can naturally do many things in parallel, and the dependency rule to do so should not be so bad, I suspect.

This is a tangent, but I have long been wishing that somebody would notice that output during install and (dist)clean without V=1 is so different from the normal targets and do something about it, and hoped that that somebody finally turned out to be you doing so in this series X-<.

I still have not recovered from the trauma I suffered post 1.6.0 era, so I would rather *not* engage in a long discussion like this one (it is a long thread; reserve a solid hour to read it through if you are interested),

https://public-inbox.org/git/[email protected]/

which would be needed to defend the choice, if we decide to omit installing the git-foo on disk in a released version.

I personally have no objection to offer a knob that can e used to force installation of symlinks without falling back to other methods. I think it would be ideal to do so without special casing symbolic links---rather, it would be ideal if it were a single knob INSTALL_GIT_FOO_METHOD=(symlinks|hardlinks|copies) that says "I want them to be installed as (symlinks|hardlinks|copies), no fallbacks".

I'm all for this, but don't have enough Make skills to make it happen. Can you or someone else post a WIP patch showing how to do this?

What would the targets look like? Something that's a build target for the target file in the installation directory, so e.g. if you ran "all install" and had modified just one file (and no recursive rebuilds) you'd install just that one file?

Early on in the "install" target we install many of these programs, and then some of these for-loops re-install on top of them. Actually now that I think of this this is one of the reasons for the 2>/dev/null probably, i.e. run "install" twice and you don't want to get errors.

Anyway, regardless of how the for-loop looks like (shell or make-powered) I split this up because it was getting really hard to maintain the *inner* part of those loops. I.e. needing to specially quote everything, end lines with \ etc.

But reading on...

Thanks. I'll read that later.

... If you're happy to accept a patch that rips out this whole conditional fallback logic and just makes it an if/elsif/elsif for symlink/hardlink/copy this makes things a lot easier.

Hi Ævar,

I have a couple of patches in the pipeline to improve GIT_TEST_INSTALLED, as I needed it to work without hardlinked copies of the built-ins. These patches might help this here isue.

Yep.

Ciao,
Dscho

A follow-up on this: We should really fix this for other reasons. I.e. compile in some "this is stuff we ourselves think is in git".

There's other manifestations of this, e.g.:

git-sizer --help # => shows you help
git sizer --help # => says it doesn't have a manpage

Because we aren't aware that git-sizer is some external tool, and that we should route --help to it.

Non-withstanding the arguable bug that things like git-sizer shouldn't be allowing themselves to be invoked by "git" like that without guaranteeing that it can consume all the options 'git' expects. When I had to deal with a similar problem in an external git-* command I was maintaining I simply made it an error to invoke it as "git mything" instead of "git-mything".

That would be nice. This has been an annoying for several tools named git-foo that I have worked on (e.g., git-sizer, git-imerge, git-when-merged, plus many internal tools).

Hmmm, I always thought that it was intended and supported that an external tool can name itself git-foo so that it can be invoked as git foo.

Which git options do you think that such a tool should be expected to support? Many useful ones, like -C <path>, --git-dir=<path>, --work-tree=<path>, -c <name>=<value>, and --no-replace-objects, work pretty much for free if the external tool uses git to interact with the repository. I use such options regularly with external tools. IMO it would be a regression for these tools to refuse to run when invoked as, say, git -C path/to/repo foo.

Michael

I don't mean we should forbid it, just that if you have an external git-foo tool meant to be invoked like "git-foo" that and not "git foo" it might be worth to save yourself the trouble and not support the latter. I thought git-sizer was one such tool, since it docs just mention "git-sizer".

But yeah, "-c" etc. are useful, and we definitely want to support this in the general case. E.g. for "git-annex" and others that are meant to be used like that.

So maybe we should just document this interface better. It seems one implicit dependency is that we expect a manpage for the tool to exist for --help.

Or, keep some list of internal git tools and treat them specially. But I see now that would break "git annex --help" in the other direction, but maybe that would be better. I don't know.

As I recall I stopped supporting "git" invocation for the tool I was fixing a long time ago because of some funny interaction with terminals & signals. I.e. git itself would eat some of them, but the tool wanted to handle it instead.

But I don't remember the details, and can't reproduce it now with:

    $ cat ~/bin/git-sigint 
    #!/usr/bin/env perl
    $SIG{INT} = sub { warn localtime . " " . $$ };
    sleep 1 while 1;
    $ git sigint # or git-sigint
    [... behaves the same either way ...]

So maybe it was something else, or I'm misremembering...

Yeah, I think this really the only problematic assumption. The rest of "-c", "--git-dir", etc, are just manipulating the environment, and that gets passed through to sub-invocations of Git (so if I have a script which calls git-config, it will pick up the "-c" config).

It would be nice if there was a way to ask "is there a manpage?", and fallback to running "git-cmd --help". But:

1. I don't think there is a portable way to query that via man. And anyway, depending platform and config, it may be opening a browser to show HTML documentation (or I think even texinfo?).

2. We can just ask whether "man git-sizer" (or whatever help display command) returned a non-zero exit code, and fall back to "git-sizer --help". But there's an infinite-loop possibility here: running "git-sizer --help" does what we want. But if "man git-log" failed, we'd run "git-log --help", which in turn runs "git help log", which runs "man git-log", and so on.

You can break that loop with an environment variable for "I already tried to show the manpage", which would I guess convert "--help" to "-h".

So it's maybe do-able, but not quite as trivial as one might hope.

I think that generally works because hitting ^C is going to send SIGINT to the whole process group. A more interesting case is:

git sigint &
kill -INT $!

There $! is a parent "git" process that is just waiting on git-sigint to die. But that works, too, because the parent relays common signals due to 10c6cddd92 (dashed externals: kill children on exit, 2012-01-08). So you might have been remembering issues prior to that commit (or uncommon signals; these come from sigchain_push_common).

-Peff

Hi Peff,

A trivial alternative would be to recommend adding a man page for 3rd-party git-<tool>s.

In other words, as soon as git-sizer is accompanied by git-sizer.1 in one of the appropriate locations, it is set.

(Actually, it is not: on Windows, it would have to add git-sizer.html in the appropriate location, but we can deal with this if needed.)

FWIW I do have a couple of scripts I use that I install into $HOME/bin/git-<tool>. Although, granted, I essentially switched to aliases for most of them, where the aliases still call a script that is checked out in some folder in my home directory. The reason: this works in more circumstances, as I do not have to add $HOME/bin to the PATH, say, in PowerShell.

So YMMV with git-<tool>s. My rule of thumb is: if I want to use this myself only, I'll make it an alias. If I want to ship it (e.g. with Git for Windows), I'll make it a git-<tool>.

Ciao,
Dscho

Yes, it would be nice if everything did ship with a manpage. Unfortunately that complicates installation, where the installation for many such tools is "save this file to your $PATH".

Tools like git-sizer may be getting big and popular enough to merit the extra infrastructure, but I think there are many smaller scripts which don't.

I have a handful of personal git-* scripts: mostly ones that are big enough to be unwieldy as an alias. But then, $PATH management is pretty straightforward on my platforms, so it's easier to drop a script there than to point an alias to a non-git-* script.

-Peff

Hi Peff,

Oh, my Git aliases pretty much *all* point to a single script that takes subcommands.

Ciao,
Dscho

Move a 37 line for-loop mess out of "install" and into a helper script. This started out fairly innocent but over the years has grown into a hard-to-maintain monster, and my recent ad874608d8 ("Makefile:
optionally symlink libexec/git-core binaries to bin/git", 2018-03-13) certainly didn't help.

The shell code is ported pretty much as-is (with getopts added), it'll be fixed & prettified in subsequent commits.

---
 Makefile         | 52 ++++++++--------------------
 install_programs | 89 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 103 insertions(+), 38 deletions(-)
 create mode 100755 install_programs

index bbfbb4292d..aa6ca1fa68 100644
@@ -2808,44 +2808,20 @@ endif
 	bindir=$$(cd '$(DESTDIR_SQ)$(bindir_SQ)' && pwd) && \
 	execdir=$$(cd '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' && pwd) && \
 	destdir_from_execdir_SQ=$$(echo '$(gitexecdir_relative_SQ)' | sed -e 's|[^/][^/]*|..|g') && \
-	{ test "$$bindir/" = "$$execdir/" || \
-	  for p in git$X $(filter $(install_bindir_programs),$(ALL_PROGRAMS)); do \
-		$(RM) "$$execdir/$$p" && \
-		test -n "$(INSTALL_SYMLINKS)" && \
-		ln -s "$$destdir_from_execdir_SQ/$(bindir_relative_SQ)/$$p" "$$execdir/$$p" || \
-		{ test -z "$(NO_INSTALL_HARDLINKS)$(NO_CROSS_DIRECTORY_HARDLINKS)" && \
-		  ln "$$bindir/$$p" "$$execdir/$$p" 2>/dev/null || \
-		  cp "$$bindir/$$p" "$$execdir/$$p" || exit; } \
-	  done; \
-	} && \
-	for p in $(filter $(install_bindir_programs),$(BUILT_INS)); do \
-		$(RM) "$$bindir/$$p" && \
-		test -n "$(INSTALL_SYMLINKS)" && \
-		ln -s "git$X" "$$bindir/$$p" || \
-		{ test -z "$(NO_INSTALL_HARDLINKS)" && \
-		  ln "$$bindir/git$X" "$$bindir/$$p" 2>/dev/null || \
-		  ln -s "git$X" "$$bindir/$$p" 2>/dev/null || \
-		  cp "$$bindir/git$X" "$$bindir/$$p" || exit; } \
-	done && \
-	for p in $(BUILT_INS); do \
-		$(RM) "$$execdir/$$p" && \
-		test -n "$(INSTALL_SYMLINKS)" && \
-		ln -s "$$destdir_from_execdir_SQ/$(bindir_relative_SQ)/git$X" "$$execdir/$$p" || \
-		{ test -z "$(NO_INSTALL_HARDLINKS)" && \
-		  ln "$$execdir/git$X" "$$execdir/$$p" 2>/dev/null || \
-		  ln -s "git$X" "$$execdir/$$p" 2>/dev/null || \
-		  cp "$$execdir/git$X" "$$execdir/$$p" || exit; } \
-	done && \
-	remote_curl_aliases="$(REMOTE_CURL_ALIASES)" && \
-	for p in $$remote_curl_aliases; do \
-		$(RM) "$$execdir/$$p" && \
-		test -n "$(INSTALL_SYMLINKS)" && \
-		ln -s "git-remote-http$X" "$$execdir/$$p" || \
-		{ test -z "$(NO_INSTALL_HARDLINKS)" && \
-		  ln "$$execdir/git-remote-http$X" "$$execdir/$$p" 2>/dev/null || \
-		  ln -s "git-remote-http$X" "$$execdir/$$p" 2>/dev/null || \
-		  cp "$$execdir/git-remote-http$X" "$$execdir/$$p" || exit; } \
-	done && \
+	./install_programs \
+		--X="$$X" \
+		--RM="$(RM)" \
+		--bindir="$$bindir" \
+		--bindir-relative="$(bindir_relative_SQ)" \
+		--execdir="$$execdir" \
+		--destdir-from-execdir="$$destdir_from_execdir_SQ" \
+		--flag-install-symlinks="$(INSTALL_SYMLINKS)" \
+		--flag-no-install-hardlinks="$(NO_INSTALL_HARDLINKS)" \
+		--flag-no-cross-directory-hardlinks="$(NO_CROSS_DIRECTORY_HARDLINKS)" \
+		--list-bindir-standalone="git$X $(filter $(install_bindir_programs),$(ALL_PROGRAMS))" \
+		--list-bindir-git-dashed="$(filter $(install_bindir_programs),$(BUILT_INS))" \
+		--list-execdir-git-dashed="$(BUILT_INS)" \
+		--list-execdir-curl-aliases="$(REMOTE_CURL_ALIASES)" && \
 	./check_bindir "z$$bindir" "z$$execdir" "$$bindir/git-add$X"
 
 .PHONY: install-gitweb install-doc install-man install-man-perl install-html install-info install-pdf

2.19.1.930.g4563a0d9d0

'sb/filenames-with-dashes'On Fri, Nov 2, 2018 at 6:38 PM Ævar Arnfjörð

Pure nitpick: Earlier this year, Stefan made an effort[1] to eradicate filenames with underscores and replace them with hyphenated filenames. Perhaps name this "install-programs", instead.

[1]: sb/filenames-with-dashes

    X=
    RM=
    bindir=
    ...

Hi,

	#!/bin/sh

	from=
	while case "$1" in
	--no-hardlinks)
		NO_INSTALL_HARDLINKS=t
		;;
	--from=*)
		from="${1#*=}"
		;;
	*)
		break
		;;
	esac; do
		shift
	done

	test $# -gt 3 || {
		echo "Usage: $0 [--no-hardlinks] <from-dir> <to-dir> <file>..." >&2
		exit 1
	}

	fromdir="$1"
	todir="$2"
	shift
	shift

	for p in "$@"
	do
		$RM "$todir/$p" &&
		test -n "$INSTALL_SYMLINKS" &&
		ln -s "$fromdir/${from:-$p}" "$todir/$p" ||
		{ test -z "$NO_INSTALL_HARDLINKS" &&
		  ln "$fromdir/${from:-$p}" "$todir/$p" ||
		  ln -s "$fromdir/${from:-$p}" "$todir/$p" ||
		  cp "$fromdir/${from:-$p}" "$todir/$p" || exit; }
	done

	test "$bindir/" = "$execdir/" ||
	link-or-copy ${NO_CROSS_DIRECTORY_HARDLINKS:+--no-hardlinks} \
		"$bindir" "$execdir" $list_bindir_standalone
	link-or-copy --from=git$X "$bindir" "$bindir" $list_bindir_git_dashed
	link-or-copy --from=git$X "$bindir" "$execdir" $list_bindir_git_dashed
	link-or-copy --from=git-remote-http$X "$bindir" "$execdir" $list_execdir_curl_aliases

I'll try for a non-RFC re-submission of this which won't have the 5/5 NO_INSTALL_BUILTIN_EXECDIR_ALIASES (for now) which'll hopefully be ready for inclusion.

I tried to fold up some of these special cases into one thing before, but managing the exceptions gets messier to read in my opinion than just having some duplication.

But more to the point, between your suggestion and Junio's to do all of this with some make construct: Yeah we should make it awesome, but I think a logical first step for a patch series like this is to just as-is use the existing logic we have now with some minor fixes for bugs.

We can refactor this later, I'm mainly interested in moving this over to a *.sh first so that process is less of a mess, fixing some minor bugs, and not getting a first iteration of improvements stuck on a much bigger review for "I rewrote the way the whole install process works" series.

Hi Ævar,

Did you seen any mistake in my version? Because if you didn't, then it would be unfortunate to leave this technical debt unaddressed. Just moving the mess really does nothing to address it, and my version should be almost there to actually do address the mess.

Ciao,
Dscho

It's not that I found some bug, and I'm not saying it should be left unaddressed. Just pointing out that saying "let's make this even better while we're at it" can make perfect the enemy of the good, particularly in this case since we have no tests for this build procedure and there's a myriad of option combinations that need to be tested.

This code is still very much unlike our usual style since it was lifted from the Makefile, but we can at least make some of it use the usual style and line spacing.

---
 install_programs | 28 +++++++++++++++++-----------
 1 file changed, 17 insertions(+), 11 deletions(-)

index e287108112..d3333cd25f 100755
@@ -50,17 +50,21 @@ do
 	esac
 	shift
 done &&
-{ test "$bindir/" = "$execdir/" ||
-  for p in $list_bindir_standalone; do
-	$RM "$execdir/$p" &&
-	test -n "$INSTALL_SYMLINKS" &&
-	ln -s "$destdir_from_execdir/$bindir_relative/$p" "$execdir/$p" ||
-	{ test -z "$NO_INSTALL_HARDLINKS$NO_CROSS_DIRECTORY_HARDLINKS" &&
-	  ln "$bindir/$p" "$execdir/$p" 2>/dev/null ||
-	  cp "$bindir/$p" "$execdir/$p" || exit; }
-  done;
-} &&
-for p in $list_bindir_git_dashed; do
+
+if test "$bindir/" != "$execdir/"
+then
+	for p in $list_bindir_standalone; do
+		$RM "$execdir/$p" &&
+		test -n "$INSTALL_SYMLINKS" &&
+		ln -s "$destdir_from_execdir/$bindir_relative/$p" "$execdir/$p" ||
+		{ test -z "$NO_INSTALL_HARDLINKS$NO_CROSS_DIRECTORY_HARDLINKS" &&
+		  ln "$bindir/$p" "$execdir/$p" 2>/dev/null ||
+		  cp "$bindir/$p" "$execdir/$p" || exit; }
+	done
+fi &&
+
+for p in $list_bindir_git_dashed
+do
 	$RM "$bindir/$p" &&
 	test -n "$INSTALL_SYMLINKS" &&
 	ln -s "git$X" "$bindir/$p" ||

@@ -69,6 +73,7 @@ for p in $list_bindir_git_dashed; do
 	  ln -s "git$X" "$bindir/$p" 2>/dev/null ||
 	  cp "$bindir/git$X" "$bindir/$p" || exit; }
 done &&
+
 for p in $list_execdir_git_dashed; do
 	$RM "$execdir/$p" &&
 	test -n "$INSTALL_SYMLINKS" &&

@@ -78,6 +83,7 @@ for p in $list_execdir_git_dashed; do
 	  ln -s "git$X" "$execdir/$p" 2>/dev/null ||
 	  cp "$execdir/git$X" "$execdir/$p" || exit; }
 done &&
+
 for p in $list_execdir_curl_aliases; do
 	$RM "$execdir/$p" &&
 	test -n "$INSTALL_SYMLINKS" &&

2.19.1.930.g4563a0d9d0

Change the fallback mechanism where we try to create hardlinks and ultimately fall back on a plain copy to emit the errors it encounters instead of hiding them away and silently falling back to copying.

Hiding these errors dates back to 3e073dc561 ("Makefile: always provide a fallback when hardlinks fail", 2008-08-25) when the existing "hardlink or copy" logic was amended to hide the errors.

At that time "make install" hadn't yet been taught any of the NO_*_HARDLINK options, that happened later in 3426e34fed ("Add NO_CROSS_DIRECTORY_HARDLINKS support to the Makefile", 2009-05-11) and was finally finished to roughly the current form in 70de5e65e8 ("Makefile: NO_INSTALL_HARDLINKS", 2012-05-02).

If someone is building a git in an environment where hard linking fails, they can now specify some combination of NO_INSTALL_HARDLINKS=YesPlease and NO_INSTALL_HARDLINKS=YesPlease, it doesn't make sense anymore to not only implicitly fall back to copying, but to do so silently.

This change leaves no way to not get errors spewed if we're trying and failing to e.g. make symlinks and having to fall back on "cp". I think that's OK.

---
 install_programs | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

index d3333cd25f..367e9a6cdf 100755
@@ -58,7 +58,7 @@ then
 		test -n "$INSTALL_SYMLINKS" &&
 		ln -s "$destdir_from_execdir/$bindir_relative/$p" "$execdir/$p" ||
 		{ test -z "$NO_INSTALL_HARDLINKS$NO_CROSS_DIRECTORY_HARDLINKS" &&
-		  ln "$bindir/$p" "$execdir/$p" 2>/dev/null ||
+		  ln "$bindir/$p" "$execdir/$p" ||
 		  cp "$bindir/$p" "$execdir/$p" || exit; }
 	done
 fi &&

@@ -69,8 +69,8 @@ do
 	test -n "$INSTALL_SYMLINKS" &&
 	ln -s "git$X" "$bindir/$p" ||
 	{ test -z "$NO_INSTALL_HARDLINKS" &&
-	  ln "$bindir/git$X" "$bindir/$p" 2>/dev/null ||
-	  ln -s "git$X" "$bindir/$p" 2>/dev/null ||
+	  ln "$bindir/git$X" "$bindir/$p" ||
+	  ln -s "git$X" "$bindir/$p" ||
 	  cp "$bindir/git$X" "$bindir/$p" || exit; }
 done &&
 

@@ -79,8 +79,8 @@ for p in $list_execdir_git_dashed; do
 	test -n "$INSTALL_SYMLINKS" &&
 	ln -s "$destdir_from_execdir/$bindir_relative/git$X" "$execdir/$p" ||
 	{ test -z "$NO_INSTALL_HARDLINKS" &&
-	  ln "$execdir/git$X" "$execdir/$p" 2>/dev/null ||
-	  ln -s "git$X" "$execdir/$p" 2>/dev/null ||
+	  ln "$execdir/git$X" "$execdir/$p" ||
+	  ln -s "git$X" "$execdir/$p" ||
 	  cp "$execdir/git$X" "$execdir/$p" || exit; }
 done &&
 

@@ -89,7 +89,7 @@ for p in $list_execdir_curl_aliases; do
 	test -n "$INSTALL_SYMLINKS" &&
 	ln -s "git-remote-http$X" "$execdir/$p" ||
 	{ test -z "$NO_INSTALL_HARDLINKS" &&
-	  ln "$execdir/git-remote-http$X" "$execdir/$p" 2>/dev/null ||
-	  ln -s "git-remote-http$X" "$execdir/$p" 2>/dev/null ||
+	  ln "$execdir/git-remote-http$X" "$execdir/$p" ||
+	  ln -s "git-remote-http$X" "$execdir/$p" ||
 	  cp "$execdir/git-remote-http$X" "$execdir/$p" || exit; }
 done

2.19.1.930.g4563a0d9d0

Add a switch for use in conjunction with the INSTALL_SYMLINKS flag added in ad874608d8 ("Makefile: optionally symlink libexec/git-core binaries to bin/git", 2018-03-13).

Now it's possible to install git with:

INSTALL_SYMLINKS=YesPlease NO_INSTALL_SYMLINKS_FALLBACK=YesPlease

And know for sure that there's not going to be any silent fallbacks on hardlinks or copying of files if symlinking fails.

---
 Makefile         |  5 ++++
 install_programs | 69 ++++++++++++++++++++++++++++++++----------------
 2 files changed, 51 insertions(+), 23 deletions(-)

index aa6ca1fa68..07c8b74353 100644
@@ -342,6 +342,10 @@ all::
 # within the same directory in some cases, INSTALL_SYMLINKS will
 # always symlink to the final target directly.
 #
+# Define NO_INSTALL_SYMLINKS_FALLBACK if in conjunction with
+# INSTALL_SYMLINKS if you'd prefer not to have the install procedure
+# fallack on hardlinking or copying if "ln -s" fails.
+#
 # Define NO_CROSS_DIRECTORY_HARDLINKS if you plan to distribute the installed
 # programs as a tar, where bin/ and libexec/ might be on different file systems.
 #

@@ -2818,6 +2822,7 @@ endif
 		--flag-install-symlinks="$(INSTALL_SYMLINKS)" \
 		--flag-no-install-hardlinks="$(NO_INSTALL_HARDLINKS)" \
 		--flag-no-cross-directory-hardlinks="$(NO_CROSS_DIRECTORY_HARDLINKS)" \
+		--flag-no-install-symlinks-fallback="$(NO_INSTALL_SYMLINKS_FALLBACK)" \
 		--list-bindir-standalone="git$X $(filter $(install_bindir_programs),$(ALL_PROGRAMS))" \
 		--list-bindir-git-dashed="$(filter $(install_bindir_programs),$(BUILT_INS))" \
 		--list-execdir-git-dashed="$(BUILT_INS)" \

2.19.1.930.g4563a0d9d0

Back when git was initially written the likes of "git-add", "git-init" etc. were installed in the user's $PATH. A few years later everything, with a few exceptions like git-upload-pack and git-receive-pack, was expected to be invoked as "git $cmd".

Now something like a decade later we're still installing these old commands in gitexecdir. This is so someone with a shellscript that still targets e.g. "git-init" can add $(git --exec-path) to their $PATH and not have to change their script.

Let's add an option to break this backwards compatibility. Now with NO_INSTALL_BUILTIN_EXECDIR_ALIASES=YesPlease there's only 3 programs in the bindir that are hardlinked to "git" (receive-pack, upload-archive & upload-pack), and 3 in the
gitexecdir (git-remote-{ftp,ftps,https} linked to git-remote-http).

There's no cross-directory links anymore, so the "NO_CROSS_DIRECTORY_HARDLINKS" flag becomes redundant under this new option.

1. https://public-inbox.org/git/[email protected]/

---
 Makefile         |  8 ++++++++
 install_programs | 36 +++++++++++++++++++++---------------
 2 files changed, 29 insertions(+), 15 deletions(-)

index 07c8b74353..a849a7b6d1 100644
@@ -346,6 +346,13 @@ all::
 # INSTALL_SYMLINKS if you'd prefer not to have the install procedure
 # fallack on hardlinking or copying if "ln -s" fails.
 #
+# Define NO_INSTALL_BUILTIN_EXECDIR_ALIASES if you'd like to skip
+# installing legacy such as "git-init" and "git-add" in the
+# gitexecdir. Unless you're on a system where "which git-init" is
+# expected to returns something set this. Users have been expected to
+# use the likes of "git init" for ages now, these programs were only
+# provided for legacy compatibility.
+#
 # Define NO_CROSS_DIRECTORY_HARDLINKS if you plan to distribute the installed
 # programs as a tar, where bin/ and libexec/ might be on different file systems.
 #

@@ -2823,6 +2830,7 @@ endif
 		--flag-no-install-hardlinks="$(NO_INSTALL_HARDLINKS)" \
 		--flag-no-cross-directory-hardlinks="$(NO_CROSS_DIRECTORY_HARDLINKS)" \
 		--flag-no-install-symlinks-fallback="$(NO_INSTALL_SYMLINKS_FALLBACK)" \
+		--flag-no-install-builtin-execdir-aliases="$(NO_INSTALL_BUILTIN_EXECDIR_ALIASES)" \
 		--list-bindir-standalone="git$X $(filter $(install_bindir_programs),$(ALL_PROGRAMS))" \
 		--list-bindir-git-dashed="$(filter $(install_bindir_programs),$(BUILT_INS))" \
 		--list-execdir-git-dashed="$(BUILT_INS)" \

index 51e08019dd..8d89cd9984 100755
@@ -33,6 +33,9 @@ do
 	--flag-no-install-symlinks-fallback=*)
 		NO_INSTALL_SYMLINKS_FALLBACK="${1#--flag-no-install-symlinks-fallback=}"
 		;;
+	--flag-no-install-builtin-execdir-aliases=*)
+		NO_INSTALL_BUILTIN_EXECDIR_ALIASES="${1#--flag-no-install-builtin-execdir-aliases=}"
+		;;
 	--list-bindir-standalone=*)
 		list_bindir_standalone="${1#--list-bindir-standalone=}"
 		;;

@@ -54,7 +57,7 @@ do
 	shift
 done &&
 
-if test "$bindir/" != "$execdir/"
+if test "$bindir/" != "$execdir/" -a -z "$NO_INSTALL_BUILTIN_EXECDIR_ALIASES"
 then
 	for p in $list_bindir_standalone; do
 		$RM "$execdir/$p" &&

@@ -87,20 +90,23 @@ do
 	fi
 done &&
 
-for p in $list_execdir_git_dashed; do
-	$RM "$execdir/$p" &&
-	if test -n "$INSTALL_SYMLINKS" -a -n "$NO_INSTALL_SYMLINKS_FALLBACK"
-	then
-		ln -s "$destdir_from_execdir/$bindir_relative/git$X" "$execdir/$p"
-	else
-		test -n "$INSTALL_SYMLINKS" &&
-		ln -s "$destdir_from_execdir/$bindir_relative/git$X" "$execdir/$p" ||
-		{ test -z "$NO_INSTALL_HARDLINKS" &&
-		  ln "$execdir/git$X" "$execdir/$p" ||
-		  ln -s "git$X" "$execdir/$p" ||
-		  cp "$execdir/git$X" "$execdir/$p" || exit; }
-	fi
-done &&
+if test -z "$NO_INSTALL_BUILTIN_EXECDIR_ALIASES"
+then
+	for p in $list_execdir_git_dashed; do
+		$RM "$execdir/$p" &&
+		if test -n "$INSTALL_SYMLINKS" -a -n "$NO_INSTALL_SYMLINKS_FALLBACK"
+		then
+			ln -s "$destdir_from_execdir/$bindir_relative/git$X" "$execdir/$p"
+		else
+			test -n "$INSTALL_SYMLINKS" &&
+			ln -s "$destdir_from_execdir/$bindir_relative/git$X" "$execdir/$p" ||
+			{ test -z "$NO_INSTALL_HARDLINKS" &&
+			  ln "$execdir/git$X" "$execdir/$p" ||
+			  ln -s "git$X" "$execdir/$p" ||
+			  cp "$execdir/git$X" "$execdir/$p" || exit; }
+		fi
+	done
+fi &&
 
 for p in $list_execdir_curl_aliases; do
 	$RM "$execdir/$p" &&

2.19.1.930.g4563a0d9d0

Hi Ævar,

I like the idea.

With my suggested refactoring that avoids the non-DRY code, this patch would also become much simpler (as would 2/5 -- 4/5).

However, I would not call these "aliases". That's just confusing. Maybe NO_INSTALL_DASHED_BUILTINS would be better? It certainly would not have confused me.

Ciao,
Dscho

Hi,

To clarify: symbolic links do not exist in Windows Vista and earlier. (There exists a concept called Junction points, but it has subtly different semantics than symbolic links, different enough that we cannot emulate symbolic links via Junctions).

Windows 7 and later do have symbolic links, but they require elevated privileges to be created, as Hannes pointed out.

Since Windows 10 version 1703 (Creators Update), enabling Developer Mode will disable this restriction and allow creating symlinks without UAC elevation. See
https://blogs.windows.com/buildingapps/2016/12/02/symlinks-windows-10/ for details.

In Git for Windows, I originally missed the memo and forgot to add support for the special flag, but since Git for Windows v2.13.1, users can create symbolic links without administrators' privileges on Windows 10 (Creators Update or later) in Developer Mode.

Of course, we still support Windows all the way back to Vista, so the default is still: no symbolic links.

Thanks for your attention,
Dscho

On Tue, Mar 13 2018, Ævar Arnfjörð Bjarmason jotted:

Also, as another follow-up question. we have stuff like "git-add" in the libexec/ directory, but when you run "git add" the bin/git binary just handles that internally, it's not dispatching to libexec/git-add.

Is the only reason we're still installing these binaries like git-add in libexec for compatibility with some old installation where that was added to the $PATH, shouldn't we (and I can write this patch) also have a toggle for "I want the modern install method" which would not install any of these binaries like git-add at all?

Then the libexec/ dir would only contain things that we really do need the bin/git to dispatch to, like git-svn, git-bisect etc.

Removing them by default was proposed and failed; see this thread for example:

https://public-inbox.org/git/[email protected]/#t

If a packager ships Git without these copies in libexec, that is not the Git that promised users that prepending the $(git --exec-path) aka GIT_EXEC_PATH to your $PATH is a valid way to preserve their older script.

I do not think anybody actually minds to have an option to omit them as long as the users understand the consequence (i.e. old promises broken) and know they are not affected (i.e. they do not have scripts that rely on the old promise).

Hi Junio,

Let's add a very, very important piece of information that was missing:
this thread is from late August 2008. We had deprecated the dashed form "only for a couple of months" by then (we removed the dashed form from the completions end of April 2008 in 799596a5d06 (completion: remove use of dashed git commands, 2008-04-20) for example).

I am glad that you changed your stance from "without dashed builtins, your Git is broken" to this much more tenable position to state that it may break super-old promises whose use we discouraged already a full decade ago.

To add some interesting information to this: in MinGit (the light-weight "Git for applications" we bundle to avoid adding a hefty 230MB to any application that wants to bundle Git for Windows), we simply ignored that old promise. We do support hooks written as Unix shell scripts in MinGit, and we have not had a single report since offering MinGit with v2.9.2 on July 16th, 2016, that it broke anybody's scripts, so it seems that users are more sensible than our promises ;-)

Not requiring Git to install any type of link makes it even possible to bundle it as .zip file (which, let's face it, is the de facto standard for cross-platform archiving).

Ciao,
Dscho

That's very good to hear. Perhaps we could slowly move away from symlinking (or even hard linking) these builtin commands (with a couple exception like receive-pack and stuff) ? We don't have to do it right now but we can start announcing that we will drop it in maybe 2 or 3 releases. We do provide a new make target to recreate these links so that packagers can make a "compat" package that contains just these links if they want to. But by default a git package will have no links.

Duy

Hi Duy,

I would hope so. As I said before: the fact that Git started out with everything as dashed subcommands is an implementation detail that unfortunately leaked into many parts of Git's UI. We can fix this.

I think that makes a *ton* of sense. Let's get to work after v2.17.0? (Same for your excellent work on t/helper/test-tool)

Ciao,
Dscho

I am not aware of any open questions or issues. You thought out loud about how the series was structured, in particular about introducing a successful test, then redefining it, as opposed to introducing it as a failing test, then making it succeed. I hope I managed to motivate my choice better in v2 (which is what you have picked up).

Duy wondered if it was sane to use a pager when we know that we are "--get"-ing at most one config item. In v2, I addressed this by turning on paging for a more careful selection of "--get"-ters.

Martin

Yeah I got busy with stuff and didn't look at it. I've just checked what's in 'pu'. Looks good to me.

Duy

Yeah, I am aware of these exchanges, and they are resolved nicely, I think. I was mostly asking if other people have concerns we haven't thought of yet.

Let's merge this to 'next', then.

Thanks.