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

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

[Junio C Hamano]

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

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/20171110011002.10179-1-sbeller@google.com/#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. <20171204171308.GA13332@sigill.intra.peff.net>


* 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. <xmqqmvakcdqw.fsf@gitster.mtv.corp.google.com>

--------------------------------------------------
[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. <87wozffavp.fsf@evledraar.gmail.com>


* 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. <1519698787-190494-1-git-send-email-dstolee@microsoft.com>


* 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. <20180206150044.1bffbb573c088d38c8e44bf5@google.com>


* 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. <570D707A-DD9E-4397-8155-E8B3C3D09760@gmail.com>


* 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. <58b2bdcd-d621-fd21-ab4d-6a9478319b19@ramsayjones.plus.com>

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

[Johannes Schindelin]

Hi Dan,

On Tue, 6 Mar 2018, Junio C Hamano wrote:

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

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

Ciao,
Johannes

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

[Ævar Arnfjörð Bjarmason]

On Wed, Mar 07 2018, Johannes Schindelin jotted:

> Hi Dan,
>
> On Tue, 6 Mar 2018, Junio C Hamano wrote:
>
>> * 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?
>
> You probably missed this in the huge "What's cooking" mail. Are you game?

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.

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

[Daniel Jacques]

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

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

[Martin Ågren]

On 7 March 2018 at 00:34, Junio C Hamano <gitster@pobox.com> wrote:

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

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

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

[Duy Nguyen]

On Fri, Mar 9, 2018 at 1:15 PM, Martin Ågren <martin.agren@gmail.com> wrote:
> On 7 March 2018 at 00:34, Junio C Hamano <gitster@pobox.com> wrote:
>
>> * 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'?
>
> 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.

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

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

[Junio C Hamano]

Martin Ågren <martin.agren@gmail.com> writes:

>>  Is this ready for 'next'?
>
> 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.

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.

Why don't we symlink libexec/git-core/* to bin/git?

[Ævar Arnfjörð Bjarmason]

On Thu, Mar 08 2018, Daniel Jacques 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!

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?

Re: Why don't we symlink libexec/git-core/* to bin/git?

[Junio C Hamano]

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

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

RE: Why don't we symlink libexec/git-core/* to bin/git?

[Randall S. Becker]

On March 13, 2018 2:37 PM, Junio C Hamano wrote:
> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> 
> > 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.
> > ...
> > 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.

[PATCH 0/3] Makefile: add a INSTALL_SYMLINKS option

[Ævar Arnfjörð Bjarmason]

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

> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>> 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.
>> ...
>> 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.

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

[PATCH 1/3] Makefile: fix broken bindir_relative variable

[Ævar Arnfjörð Bjarmason]

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

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index de4b8f0c02..b2f8f2b171 100644
--- a/Makefile
+++ b/Makefile
@@ -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

[PATCH 2/3] Makefile: add a gitexecdir_relative variable

[Ævar Arnfjörð Bjarmason]

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
<20180108030239.92036-3-dnj@google.com> (https://public-inbox.org/git/20180108030239.92036-3-dnj@google.com/).

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Makefile b/Makefile
index b2f8f2b171..ee0b6c8940 100644
--- a/Makefile
+++ b/Makefile
@@ -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

[PATCH 3/3] Makefile: optionally symlink libexec/git-core binaries to bin/git

[Ævar Arnfjörð Bjarmason]

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

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile | 46 +++++++++++++++++++++++++++++++---------------
 1 file changed, 31 insertions(+), 15 deletions(-)

diff --git a/Makefile b/Makefile
index ee0b6c8940..ac7616422d 100644
--- a/Makefile
+++ b/Makefile
@@ -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

Re: [PATCH 3/3] Makefile: optionally symlink libexec/git-core binaries to bin/git

[Johannes Sixt]

Am 13.03.2018 um 21:39 schrieb Ævar Arnfjörð Bjarmason:
> 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.

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.

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

s/ran/run/

>      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

s/given some//

> 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
> 
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>   Makefile | 46 +++++++++++++++++++++++++++++++---------------
>   1 file changed, 31 insertions(+), 15 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index ee0b6c8940..ac7616422d 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -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

s/ between the two//

> +# 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.

"the final target"? Do you mean "the git executable installed in 
$bindir" or something like this?

> +#
>   # 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; } \

I think that it is unnecessary to place the later options in {} brackets 
because && and || have equal precedence in shell scripts. That is:

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

Of course, it means that when symlinking fails, it falls back to hard 
links (if permitted) or copies, whichever works. But that also happens 
with your version.

(Ditto in the rest of the hunk, which I don't repeat here.)

-- Hannes

Re: [PATCH 3/3] Makefile: optionally symlink libexec/git-core binaries to bin/git

[Ævar Arnfjörð Bjarmason]

On Wed, Mar 14 2018, Johannes Sixt jotted:

> Am 13.03.2018 um 21:39 schrieb Ævar Arnfjörð Bjarmason:
>> 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.
>
> 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.

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.

>>
>> 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
>
> s/ran/run/
>
>>      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
>
> s/given some//
>
>> 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
>>
>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>> ---
>>   Makefile | 46 +++++++++++++++++++++++++++++++---------------
>>   1 file changed, 31 insertions(+), 15 deletions(-)
>>
>> diff --git a/Makefile b/Makefile
>> index ee0b6c8940..ac7616422d 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -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
>
> s/ between the two//

Thanks. Will fix the above in a subsequent submission.

>> +# 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.
>
> "the final target"? Do you mean "the git executable installed in
> $bindir" or something like this?

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.

>> +#
>>   # 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; } \
>
> I think that it is unnecessary to place the later options in {}
> brackets because && and || have equal precedence in shell
> scripts. That is:
>
> 	want symlinks? &&
> 	make symlinks ||
> 	want hard links? &&
> 	make hard links ||
> 	make copies ||
> 	exit
>
> Of course, it means that when symlinking fails, it falls back to hard
> links (if permitted) or copies, whichever works. But that also happens
> with your version.
>
> (Ditto in the rest of the hunk, which I don't repeat here.)

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.

Re: Why don't we symlink libexec/git-core/* to bin/git?

[Ævar Arnfjörð Bjarmason]

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

> On Thu, Mar 08 2018, Daniel Jacques 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!
>
> 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?

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.

Re: Why don't we symlink libexec/git-core/* to bin/git?

[Junio C Hamano]

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

> 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/7vr68b8q9p.fsf@gitster.siamese.dyndns.org/#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).

Re: [PATCH 3/3] Makefile: optionally symlink libexec/git-core binaries to bin/git

[Linus Torvalds]

On Wed, Mar 14, 2018 at 3:14 AM, Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> On Wed, Mar 14 2018, Johannes Sixt jotted:
>>
>> 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.
>
> 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.

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

Re: [PATCH 3/3] Makefile: optionally symlink libexec/git-core binaries to bin/git

[Johannes Schindelin]

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

Hi,

On Wed, 14 Mar 2018, Johannes Sixt wrote:

> Am 13.03.2018 um 21:39 schrieb Ævar Arnfjörð Bjarmason:
> > 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.
> 
> 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.

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

Re: [PATCH 3/3] Makefile: optionally symlink libexec/git-core binaries to bin/git

[Johannes Schindelin]

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

Hi Linus,

On Wed, 14 Mar 2018, Linus Torvalds wrote:

> On Wed, Mar 14, 2018 at 3:14 AM, Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
> > On Wed, Mar 14 2018, Johannes Sixt jotted:
> >>
> >> 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.
> >
> > 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.
> 
> 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.

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

Re: Why don't we symlink libexec/git-core/* to bin/git?

[Johannes Schindelin]

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

Hi Junio,

On Wed, 14 Mar 2018, Junio C Hamano wrote:

> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> 
> > 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/7vr68b8q9p.fsf@gitster.siamese.dyndns.org/#t

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

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

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

Re: [PATCH 3/3] Makefile: optionally symlink libexec/git-core binaries to bin/git

[Linus Torvalds]

On Thu, Mar 15, 2018 at 10:05 AM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> 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.

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

Re: [PATCH 3/3] Makefile: optionally symlink libexec/git-core binaries to bin/git

[Johannes Schindelin]

Hi Linus.

On Thu, 15 Mar 2018, Linus Torvalds wrote:

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

Please do elaborate.

Ciao,
Johannes

Re: [PATCH 3/3] Makefile: optionally symlink libexec/git-core binaries to bin/git

[Ævar Arnfjörð Bjarmason]

On Fri, Mar 16 2018, Johannes Schindelin jotted:

> Hi Linus.
>
> On Thu, 15 Mar 2018, Linus Torvalds wrote:
>
>> 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).
>
> Please do elaborate.

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

Re: Why don't we symlink libexec/git-core/* to bin/git?

[Duy Nguyen]

On Thu, Mar 15, 2018 at 6:16 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> 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 ;-)

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

Re: [PATCH 3/3] Makefile: optionally symlink libexec/git-core binaries to bin/git

[Johannes Schindelin]

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

Hi Ævar,

On Fri, 16 Mar 2018, Ævar Arnfjörð Bjarmason wrote:

> 
> On Fri, Mar 16 2018, Johannes Schindelin jotted:
> 
> > On Thu, 15 Mar 2018, Linus Torvalds wrote:
> >
> >> 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).
> >
> > Please do elaborate.
> 
> 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?

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

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

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

Re: [PATCH 3/3] Makefile: optionally symlink libexec/git-core binaries to bin/git

[Linus Torvalds]

On Mon, Mar 19, 2018, 04:34 Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
>
> This is a real problem.

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 it is our own darned fault because we let an
> implementation detail bleed into a protocol. We could have designed that a
> lot better.

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

> Of course we should fix this, though. There is literally no good reason

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

Re: Why don't we symlink libexec/git-core/* to bin/git?

[Johannes Schindelin]

Hi Duy,

On Fri, 16 Mar 2018, Duy Nguyen wrote:

> On Thu, Mar 15, 2018 at 6:16 PM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> > 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 ;-)
> 
> 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) ?

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.

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

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

[RFC/PATCH 0/5] stop installing old libexec aliases like "git-init"

[Ævar Arnfjörð Bjarmason]

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/87woyfdkoi.fsf@evledraar.gmail.com/ 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

[RFC/PATCH 1/5] Makefile: move long inline shell loops in "install" into helper

[Ævar Arnfjörð Bjarmason]

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.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile         | 52 ++++++++--------------------
 install_programs | 89 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 103 insertions(+), 38 deletions(-)
 create mode 100755 install_programs

diff --git a/Makefile b/Makefile
index bbfbb4292d..aa6ca1fa68 100644
--- a/Makefile
+++ b/Makefile
@@ -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
diff --git a/install_programs b/install_programs
new file mode 100755
index 0000000000..e287108112
--- /dev/null
+++ b/install_programs
@@ -0,0 +1,89 @@
+#!/bin/sh
+
+while test $# != 0
+do
+	case "$1" in
+	--X=*)
+		X="${1#--X=}"
+		;;
+	--RM=*)
+		RM="${1#--RM=}"
+		;;
+	--bindir=*)
+		bindir="${1#--bindir=}"
+		;;
+	--bindir-relative=*)
+		bindir_relative="${1#--bindir-relative=}"
+		;;
+	--execdir=*)
+		execdir="${1#--execdir=}"
+		;;
+	--destdir-from-execdir=*)
+		destdir_from_execdir="${1#--destdir-from-execdir=}"
+		;;
+	--flag-install-symlinks=*)
+		INSTALL_SYMLINKS="${1#--flag-install-symlinks=}"
+		;;
+	--flag-no-install-hardlinks=*)
+		NO_INSTALL_HARDLINKS="${1#--flag-no-install-hardlinks=}"
+		;;
+	--flag-no-cross-directory-hardlinks=*)
+		NO_CROSS_DIRECTORY_HARDLINKS="${1#--flag-no-cross-directory-hardlinks=}"
+		;;
+	--list-bindir-standalone=*)
+		list_bindir_standalone="${1#--list-bindir-standalone=}"
+		;;
+	--list-bindir-git-dashed=*)
+		list_bindir_git_dashed="${1#--list-bindir-git-dashed=}"
+		;;
+	--list-execdir-git-dashed=*)
+		list_execdir_git_dashed="${1#--list-execdir-git-dashed=}"
+		;;
+	--list-execdir-curl-aliases=*)
+		list_execdir_curl_aliases="${1#--list-execdir-curl-aliases=}"
+		;;
+
+	*)
+		echo "Unknown option $1"
+		exit 1
+		;;
+	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
+	$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 $list_execdir_git_dashed; do
+	$RM "$execdir/$p" &&
+	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 ||
+	  cp "$execdir/git$X" "$execdir/$p" || exit; }
+done &&
+for p in $list_execdir_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
-- 
2.19.1.930.g4563a0d9d0

[RFC/PATCH 2/5] Makefile: conform some of the code to our coding standards

[Ævar Arnfjörð Bjarmason]

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.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 install_programs | 28 +++++++++++++++++-----------
 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/install_programs b/install_programs
index e287108112..d3333cd25f 100755
--- a/install_programs
+++ b/install_programs
@@ -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

[RFC/PATCH 3/5] Makefile: stop hiding failures during "install"

[Ævar Arnfjörð Bjarmason]

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.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 install_programs | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/install_programs b/install_programs
index d3333cd25f..367e9a6cdf 100755
--- a/install_programs
+++ b/install_programs
@@ -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

[RFC/PATCH 4/5] Makefile: add NO_INSTALL_SYMLINKS_FALLBACK switch

[Ævar Arnfjörð Bjarmason]

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.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile         |  5 ++++
 install_programs | 69 ++++++++++++++++++++++++++++++++----------------
 2 files changed, 51 insertions(+), 23 deletions(-)

diff --git a/Makefile b/Makefile
index aa6ca1fa68..07c8b74353 100644
--- a/Makefile
+++ b/Makefile
@@ -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)" \
diff --git a/install_programs b/install_programs
index 367e9a6cdf..51e08019dd 100755
--- a/install_programs
+++ b/install_programs
@@ -30,6 +30,9 @@ do
 	--flag-no-cross-directory-hardlinks=*)
 		NO_CROSS_DIRECTORY_HARDLINKS="${1#--flag-no-cross-directory-hardlinks=}"
 		;;
+	--flag-no-install-symlinks-fallback=*)
+		NO_INSTALL_SYMLINKS_FALLBACK="${1#--flag-no-install-symlinks-fallback=}"
+		;;
 	--list-bindir-standalone=*)
 		list_bindir_standalone="${1#--list-bindir-standalone=}"
 		;;
@@ -55,41 +58,61 @@ 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" ||
-		  cp "$bindir/$p" "$execdir/$p" || exit; }
+		if test -n "$INSTALL_SYMLINKS" -a -n "$NO_INSTALL_SYMLINKS_FALLBACK"
+		then
+			ln -s "$destdir_from_execdir/$bindir_relative/$p" "$execdir/$p"
+		else
+			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" ||
+			  cp "$bindir/$p" "$execdir/$p" || exit; }
+		fi
 	done
 fi &&
 
 for p in $list_bindir_git_dashed
 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" ||
-	  ln -s "git$X" "$bindir/$p" ||
-	  cp "$bindir/git$X" "$bindir/$p" || exit; }
+	if test -n "$INSTALL_SYMLINKS" -a -n "$NO_INSTALL_SYMLINKS_FALLBACK"
+	then
+		ln -s "git$X" "$bindir/$p"
+	else
+		test -n "$INSTALL_SYMLINKS" &&
+		ln -s "git$X" "$bindir/$p" ||
+		{ test -z "$NO_INSTALL_HARDLINKS" &&
+		  ln "$bindir/git$X" "$bindir/$p" ||
+		  ln -s "git$X" "$bindir/$p" ||
+		  cp "$bindir/git$X" "$bindir/$p" || exit; }
+	fi
 done &&
 
 for p in $list_execdir_git_dashed; do
 	$RM "$execdir/$p" &&
-	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; }
+	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 &&
 
 for p in $list_execdir_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" ||
-	  ln -s "git-remote-http$X" "$execdir/$p" ||
-	  cp "$execdir/git-remote-http$X" "$execdir/$p" || exit; }
+	if test -n "$INSTALL_SYMLINKS" -a -n "$NO_INSTALL_SYMLINKS_FALLBACK"
+	then
+		ln -s "git-remote-http$X" "$execdir/$p"
+	else
+		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" ||
+		  ln -s "git-remote-http$X" "$execdir/$p" ||
+		  cp "$execdir/git-remote-http$X" "$execdir/$p" || exit; }
+	fi
 done
-- 
2.19.1.930.g4563a0d9d0

[RFC/PATCH 5/5] Makefile: Add a NO_INSTALL_BUILTIN_EXECDIR_ALIASES flag

[Ævar Arnfjörð Bjarmason]

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/87woyfdkoi.fsf@evledraar.gmail.com/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile         |  8 ++++++++
 install_programs | 36 +++++++++++++++++++++---------------
 2 files changed, 29 insertions(+), 15 deletions(-)

diff --git a/Makefile b/Makefile
index 07c8b74353..a849a7b6d1 100644
--- a/Makefile
+++ b/Makefile
@@ -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)" \
diff --git a/install_programs b/install_programs
index 51e08019dd..8d89cd9984 100755
--- a/install_programs
+++ b/install_programs
@@ -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

Re: [RFC/PATCH 0/5] stop installing old libexec aliases like "git-init"

[Junio C Hamano]

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

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

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'd like to say it's ready, but I've spotted some fallout:

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/alpine.LFD.1.10.0808261435470.3363@nehalem.linux-foundation.org/

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

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

Re: [RFC/PATCH 4/5] Makefile: add NO_INSTALL_SYMLINKS_FALLBACK switch

[Eric Sunshine]

On Fri, Nov 2, 2018 at 6:38 PM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
> 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).
> [...]
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
> diff --git a/Makefile b/Makefile
> @@ -342,6 +342,10 @@ all::
> +# Define NO_INSTALL_SYMLINKS_FALLBACK if in conjunction with

s/if in/in/

> +# INSTALL_SYMLINKS if you'd prefer not to have the install procedure
> +# fallack on hardlinking or copying if "ln -s" fails.
> +#

Re: [RFC/PATCH 5/5] Makefile: Add a NO_INSTALL_BUILTIN_EXECDIR_ALIASES flag

[Eric Sunshine]

On Fri, Nov 2, 2018 at 6:38 PM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
> 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).
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
> diff --git a/Makefile b/Makefile
> @@ -346,6 +346,13 @@ all::
> +# 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

s/returns/return/
s/something/&,/

Although, it's not clear what "return something" means. Perhaps rephrase it as:

   ...git-init is expected to exist, set this.

> +# use the likes of "git init" for ages now, these programs were only
> +# provided for legacy compatibility.
> +#

Re: [RFC/PATCH 1/5] Makefile: move long inline shell loops in "install" into helper

[Eric Sunshine]

'sb/filenames-with-dashes'On Fri, Nov 2, 2018 at 6:38 PM Ævar Arnfjörð
Bjarmason <avarab@gmail.com> wrote:
> 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.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  install_programs | 89 ++++++++++++++++++++++++++++++++++++++++++++++++

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

> diff --git a/install_programs b/install_programs
> @@ -0,0 +1,89 @@
> +while test $# != 0
> +do
> +       case "$1" in
> +       --X=*)
> +               X="${1#--X=}"
> +               ;;
> +       --RM=*)
> +               RM="${1#--RM=}"
> +               ;;
> +       --bindir=*)
> +               bindir="${1#--bindir=}"
> +               ;;

Is the intention that the user might have X, RM, 'bindir', etc.
already in the environment, and the switches in this script merely
override those values? Or is the intention that X, RM, 'bindir, etc.
should all start out unset? If the latter, perhaps start the script
with an initialization block which clears all these variables first:

    X=
    RM=
    bindir=
    ...

Re: [RFC/PATCH 0/5] stop installing old libexec aliases like "git-init"

[Ævar Arnfjörð Bjarmason]

On Sat, Nov 03 2018, Junio C Hamano wrote:

> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
>
>> 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?
>
> 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'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...

>> I'd like to say it's ready, but I've spotted some fallout:
>
> 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/alpine.LFD.1.10.0808261435470.3363@nehalem.linux-foundation.org/
>
> which would be needed to defend the choice, if we decide to omit
> installing the git-foo on disk in a released version.

Thanks. I'll read that later.

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

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

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

Re: [RFC/PATCH 0/5] stop installing old libexec aliases like "git-init"

[Johannes Schindelin]

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

Hi Ævar,

On Fri, 2 Nov 2018, Ævar Arnfjörð Bjarmason wrote:

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

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.

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

Yep.

Ciao,
Dscho

Re: [RFC/PATCH 1/5] Makefile: move long inline shell loops in "install" into helper

[Johannes Schindelin]

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

Hi,

On Fri, 2 Nov 2018, Ævar Arnfjörð Bjarmason wrote:

> 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.
> 
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  Makefile         | 52 ++++++++--------------------
>  install_programs | 89 ++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 103 insertions(+), 38 deletions(-)
>  create mode 100755 install_programs
> 
> diff --git a/Makefile b/Makefile
> index bbfbb4292d..aa6ca1fa68 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -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 && \

This indeed looks like a mess...

> +	./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
> diff --git a/install_programs b/install_programs
> new file mode 100755
> index 0000000000..e287108112
> --- /dev/null
> +++ b/install_programs
> @@ -0,0 +1,89 @@
> +#!/bin/sh
> +
> +while test $# != 0
> +do
> +	case "$1" in
> +	--X=*)
> +		X="${1#--X=}"
> +		;;
> +	--RM=*)
> +		RM="${1#--RM=}"
> +		;;
> +	--bindir=*)
> +		bindir="${1#--bindir=}"
> +		;;
> +	--bindir-relative=*)
> +		bindir_relative="${1#--bindir-relative=}"
> +		;;
> +	--execdir=*)
> +		execdir="${1#--execdir=}"
> +		;;
> +	--destdir-from-execdir=*)
> +		destdir_from_execdir="${1#--destdir-from-execdir=}"
> +		;;
> +	--flag-install-symlinks=*)
> +		INSTALL_SYMLINKS="${1#--flag-install-symlinks=}"
> +		;;
> +	--flag-no-install-hardlinks=*)
> +		NO_INSTALL_HARDLINKS="${1#--flag-no-install-hardlinks=}"
> +		;;
> +	--flag-no-cross-directory-hardlinks=*)
> +		NO_CROSS_DIRECTORY_HARDLINKS="${1#--flag-no-cross-directory-hardlinks=}"
> +		;;
> +	--list-bindir-standalone=*)
> +		list_bindir_standalone="${1#--list-bindir-standalone=}"
> +		;;
> +	--list-bindir-git-dashed=*)
> +		list_bindir_git_dashed="${1#--list-bindir-git-dashed=}"
> +		;;
> +	--list-execdir-git-dashed=*)
> +		list_execdir_git_dashed="${1#--list-execdir-git-dashed=}"
> +		;;
> +	--list-execdir-curl-aliases=*)
> +		list_execdir_curl_aliases="${1#--list-execdir-curl-aliases=}"
> +		;;
> +
> +	*)
> +		echo "Unknown option $1"
> +		exit 1
> +		;;
> +	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
> +	$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 $list_execdir_git_dashed; do
> +	$RM "$execdir/$p" &&
> +	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 ||
> +	  cp "$execdir/git$X" "$execdir/$p" || exit; }
> +done &&
> +for p in $list_execdir_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

... but so does this. I would be very surprised if these four very
similar-looking constructs could not be refactored into a single shell
script that is then called four times with different parameters.

Something like

	#!/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

and then calling it using

	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

That would at least DRY up this mess a bit.

Ciao,
Dscho

> -- 
> 2.19.1.930.g4563a0d9d0
> 
> 

Re: [RFC/PATCH 5/5] Makefile: Add a NO_INSTALL_BUILTIN_EXECDIR_ALIASES flag

[Johannes Schindelin]

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

Hi Ævar,

On Fri, 2 Nov 2018, Ævar Arnfjörð Bjarmason wrote:

> 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/87woyfdkoi.fsf@evledraar.gmail.com/
> 
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---

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

>  Makefile         |  8 ++++++++
>  install_programs | 36 +++++++++++++++++++++---------------
>  2 files changed, 29 insertions(+), 15 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 07c8b74353..a849a7b6d1 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -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)" \
> diff --git a/install_programs b/install_programs
> index 51e08019dd..8d89cd9984 100755
> --- a/install_programs
> +++ b/install_programs
> @@ -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
> 
> 

Re: [RFC/PATCH 1/5] Makefile: move long inline shell loops in "install" into helper

[Ævar Arnfjörð Bjarmason]

On Mon, Nov 12 2018, Johannes Schindelin wrote:

> Hi,
>
> On Fri, 2 Nov 2018, Ævar Arnfjörð Bjarmason wrote:
>
>> 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.
>>
>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>> ---
>>  Makefile         | 52 ++++++++--------------------
>>  install_programs | 89 ++++++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 103 insertions(+), 38 deletions(-)
>>  create mode 100755 install_programs
>>
>> diff --git a/Makefile b/Makefile
>> index bbfbb4292d..aa6ca1fa68 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -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 && \
>
> This indeed looks like a mess...
>
>> +	./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
>> diff --git a/install_programs b/install_programs
>> new file mode 100755
>> index 0000000000..e287108112
>> --- /dev/null
>> +++ b/install_programs
>> @@ -0,0 +1,89 @@
>> +#!/bin/sh
>> +
>> +while test $# != 0
>> +do
>> +	case "$1" in
>> +	--X=*)
>> +		X="${1#--X=}"
>> +		;;
>> +	--RM=*)
>> +		RM="${1#--RM=}"
>> +		;;
>> +	--bindir=*)
>> +		bindir="${1#--bindir=}"
>> +		;;
>> +	--bindir-relative=*)
>> +		bindir_relative="${1#--bindir-relative=}"
>> +		;;
>> +	--execdir=*)
>> +		execdir="${1#--execdir=}"
>> +		;;
>> +	--destdir-from-execdir=*)
>> +		destdir_from_execdir="${1#--destdir-from-execdir=}"
>> +		;;
>> +	--flag-install-symlinks=*)
>> +		INSTALL_SYMLINKS="${1#--flag-install-symlinks=}"
>> +		;;
>> +	--flag-no-install-hardlinks=*)
>> +		NO_INSTALL_HARDLINKS="${1#--flag-no-install-hardlinks=}"
>> +		;;
>> +	--flag-no-cross-directory-hardlinks=*)
>> +		NO_CROSS_DIRECTORY_HARDLINKS="${1#--flag-no-cross-directory-hardlinks=}"
>> +		;;
>> +	--list-bindir-standalone=*)
>> +		list_bindir_standalone="${1#--list-bindir-standalone=}"
>> +		;;
>> +	--list-bindir-git-dashed=*)
>> +		list_bindir_git_dashed="${1#--list-bindir-git-dashed=}"
>> +		;;
>> +	--list-execdir-git-dashed=*)
>> +		list_execdir_git_dashed="${1#--list-execdir-git-dashed=}"
>> +		;;
>> +	--list-execdir-curl-aliases=*)
>> +		list_execdir_curl_aliases="${1#--list-execdir-curl-aliases=}"
>> +		;;
>> +
>> +	*)
>> +		echo "Unknown option $1"
>> +		exit 1
>> +		;;
>> +	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
>> +	$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 $list_execdir_git_dashed; do
>> +	$RM "$execdir/$p" &&
>> +	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 ||
>> +	  cp "$execdir/git$X" "$execdir/$p" || exit; }
>> +done &&
>> +for p in $list_execdir_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
>
> ... but so does this. I would be very surprised if these four very
> similar-looking constructs could not be refactored into a single shell
> script that is then called four times with different parameters.
>
> Something like
>
> 	#!/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
>
> and then calling it using
>
> 	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
>
> That would at least DRY up this mess a bit.

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.

Re: [RFC/PATCH 1/5] Makefile: move long inline shell loops in "install" into helper

[Johannes Schindelin]

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

Hi Ævar,

On Mon, 12 Nov 2018, Ævar Arnfjörð Bjarmason wrote:

> On Mon, Nov 12 2018, Johannes Schindelin wrote:
> 
> > On Fri, 2 Nov 2018, Ævar Arnfjörð Bjarmason wrote:
> >
> >> 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.
> >>
> >> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> >> ---
> >>  Makefile         | 52 ++++++++--------------------
> >>  install_programs | 89 ++++++++++++++++++++++++++++++++++++++++++++++++
> >>  2 files changed, 103 insertions(+), 38 deletions(-)
> >>  create mode 100755 install_programs
> >>
> >> diff --git a/Makefile b/Makefile
> >> index bbfbb4292d..aa6ca1fa68 100644
> >> --- a/Makefile
> >> +++ b/Makefile
> >> @@ -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 && \
> >
> > This indeed looks like a mess...
> >
> >> +	./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
> >> diff --git a/install_programs b/install_programs
> >> new file mode 100755
> >> index 0000000000..e287108112
> >> --- /dev/null
> >> +++ b/install_programs
> >> @@ -0,0 +1,89 @@
> >> +#!/bin/sh
> >> +
> >> +while test $# != 0
> >> +do
> >> +	case "$1" in
> >> +	--X=*)
> >> +		X="${1#--X=}"
> >> +		;;
> >> +	--RM=*)
> >> +		RM="${1#--RM=}"
> >> +		;;
> >> +	--bindir=*)
> >> +		bindir="${1#--bindir=}"
> >> +		;;
> >> +	--bindir-relative=*)
> >> +		bindir_relative="${1#--bindir-relative=}"
> >> +		;;
> >> +	--execdir=*)
> >> +		execdir="${1#--execdir=}"
> >> +		;;
> >> +	--destdir-from-execdir=*)
> >> +		destdir_from_execdir="${1#--destdir-from-execdir=}"
> >> +		;;
> >> +	--flag-install-symlinks=*)
> >> +		INSTALL_SYMLINKS="${1#--flag-install-symlinks=}"
> >> +		;;
> >> +	--flag-no-install-hardlinks=*)
> >> +		NO_INSTALL_HARDLINKS="${1#--flag-no-install-hardlinks=}"
> >> +		;;
> >> +	--flag-no-cross-directory-hardlinks=*)
> >> +		NO_CROSS_DIRECTORY_HARDLINKS="${1#--flag-no-cross-directory-hardlinks=}"
> >> +		;;
> >> +	--list-bindir-standalone=*)
> >> +		list_bindir_standalone="${1#--list-bindir-standalone=}"
> >> +		;;
> >> +	--list-bindir-git-dashed=*)
> >> +		list_bindir_git_dashed="${1#--list-bindir-git-dashed=}"
> >> +		;;
> >> +	--list-execdir-git-dashed=*)
> >> +		list_execdir_git_dashed="${1#--list-execdir-git-dashed=}"
> >> +		;;
> >> +	--list-execdir-curl-aliases=*)
> >> +		list_execdir_curl_aliases="${1#--list-execdir-curl-aliases=}"
> >> +		;;
> >> +
> >> +	*)
> >> +		echo "Unknown option $1"
> >> +		exit 1
> >> +		;;
> >> +	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
> >> +	$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 $list_execdir_git_dashed; do
> >> +	$RM "$execdir/$p" &&
> >> +	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 ||
> >> +	  cp "$execdir/git$X" "$execdir/$p" || exit; }
> >> +done &&
> >> +for p in $list_execdir_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
> >
> > ... but so does this. I would be very surprised if these four very
> > similar-looking constructs could not be refactored into a single shell
> > script that is then called four times with different parameters.
> >
> > Something like
> >
> > 	#!/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
> >
> > and then calling it using
> >
> > 	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
> >
> > That would at least DRY up this mess a bit.
> 
> 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.

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

Re: [RFC/PATCH 1/5] Makefile: move long inline shell loops in "install" into helper

[Ævar Arnfjörð Bjarmason]

On Mon, Nov 12 2018, Johannes Schindelin wrote:

> Hi Ævar,
>
> On Mon, 12 Nov 2018, Ævar Arnfjörð Bjarmason wrote:
>
>> On Mon, Nov 12 2018, Johannes Schindelin wrote:
>>
>> > On Fri, 2 Nov 2018, Ævar Arnfjörð Bjarmason wrote:
>> >
>> >> 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.
>> >>
>> >> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>> >> ---
>> >>  Makefile         | 52 ++++++++--------------------
>> >>  install_programs | 89 ++++++++++++++++++++++++++++++++++++++++++++++++
>> >>  2 files changed, 103 insertions(+), 38 deletions(-)
>> >>  create mode 100755 install_programs
>> >>
>> >> diff --git a/Makefile b/Makefile
>> >> index bbfbb4292d..aa6ca1fa68 100644
>> >> --- a/Makefile
>> >> +++ b/Makefile
>> >> @@ -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 && \
>> >
>> > This indeed looks like a mess...
>> >
>> >> +	./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
>> >> diff --git a/install_programs b/install_programs
>> >> new file mode 100755
>> >> index 0000000000..e287108112
>> >> --- /dev/null
>> >> +++ b/install_programs
>> >> @@ -0,0 +1,89 @@
>> >> +#!/bin/sh
>> >> +
>> >> +while test $# != 0
>> >> +do
>> >> +	case "$1" in
>> >> +	--X=*)
>> >> +		X="${1#--X=}"
>> >> +		;;
>> >> +	--RM=*)
>> >> +		RM="${1#--RM=}"
>> >> +		;;
>> >> +	--bindir=*)
>> >> +		bindir="${1#--bindir=}"
>> >> +		;;
>> >> +	--bindir-relative=*)
>> >> +		bindir_relative="${1#--bindir-relative=}"
>> >> +		;;
>> >> +	--execdir=*)
>> >> +		execdir="${1#--execdir=}"
>> >> +		;;
>> >> +	--destdir-from-execdir=*)
>> >> +		destdir_from_execdir="${1#--destdir-from-execdir=}"
>> >> +		;;
>> >> +	--flag-install-symlinks=*)
>> >> +		INSTALL_SYMLINKS="${1#--flag-install-symlinks=}"
>> >> +		;;
>> >> +	--flag-no-install-hardlinks=*)
>> >> +		NO_INSTALL_HARDLINKS="${1#--flag-no-install-hardlinks=}"
>> >> +		;;
>> >> +	--flag-no-cross-directory-hardlinks=*)
>> >> +		NO_CROSS_DIRECTORY_HARDLINKS="${1#--flag-no-cross-directory-hardlinks=}"
>> >> +		;;
>> >> +	--list-bindir-standalone=*)
>> >> +		list_bindir_standalone="${1#--list-bindir-standalone=}"
>> >> +		;;
>> >> +	--list-bindir-git-dashed=*)
>> >> +		list_bindir_git_dashed="${1#--list-bindir-git-dashed=}"
>> >> +		;;
>> >> +	--list-execdir-git-dashed=*)
>> >> +		list_execdir_git_dashed="${1#--list-execdir-git-dashed=}"
>> >> +		;;
>> >> +	--list-execdir-curl-aliases=*)
>> >> +		list_execdir_curl_aliases="${1#--list-execdir-curl-aliases=}"
>> >> +		;;
>> >> +
>> >> +	*)
>> >> +		echo "Unknown option $1"
>> >> +		exit 1
>> >> +		;;
>> >> +	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
>> >> +	$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 $list_execdir_git_dashed; do
>> >> +	$RM "$execdir/$p" &&
>> >> +	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 ||
>> >> +	  cp "$execdir/git$X" "$execdir/$p" || exit; }
>> >> +done &&
>> >> +for p in $list_execdir_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
>> >
>> > ... but so does this. I would be very surprised if these four very
>> > similar-looking constructs could not be refactored into a single shell
>> > script that is then called four times with different parameters.
>> >
>> > Something like
>> >
>> > 	#!/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
>> >
>> > and then calling it using
>> >
>> > 	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
>> >
>> > That would at least DRY up this mess a bit.
>>
>> 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.
>
> 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.

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.

Re: [RFC/PATCH 0/5] stop installing old libexec aliases like "git-init"

[Ævar Arnfjörð Bjarmason]

On Fri, Nov 02 2018, Ævar Arnfjörð Bjarmason wrote:

> 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/87woyfdkoi.fsf@evledraar.gmail.com/ 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.

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

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

Re: [RFC/PATCH 0/5] stop installing old libexec aliases like "git-init"

[Michael Haggerty]

On Fri, Nov 16, 2018 at 11:38 AM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> [...]
> 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.

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

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

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

Re: [RFC/PATCH 0/5] stop installing old libexec aliases like "git-init"

[Ævar Arnfjörð Bjarmason]

On Fri, Nov 16 2018, Michael Haggerty wrote:

> On Fri, Nov 16, 2018 at 11:38 AM Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
>> [...]
>> 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.
>
> 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).
>
>> 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".
>
> 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`.

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

Re: [RFC/PATCH 0/5] stop installing old libexec aliases like "git-init"

[Jeff King]

On Fri, Nov 16, 2018 at 08:22:11PM +0100, Ævar Arnfjörð Bjarmason wrote:

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

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.

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

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

Re: [RFC/PATCH 0/5] stop installing old libexec aliases like "git-init"

[Johannes Schindelin]

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

Hi Peff,

On Sat, 17 Nov 2018, Jeff King wrote:

> On Fri, Nov 16, 2018 at 08:22:11PM +0100, Ævar Arnfjörð Bjarmason wrote:
> 
> > 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.
> 
> 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.

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

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

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

Re: [RFC/PATCH 0/5] stop installing old libexec aliases like "git-init"

[Jeff King]

On Thu, Nov 22, 2018 at 01:48:53PM +0100, Johannes Schindelin wrote:

> > So it's maybe do-able, but not quite as trivial as one might hope.
> 
> 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.

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.

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

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

Re: [RFC/PATCH 0/5] stop installing old libexec aliases like "git-init"

[Johannes Schindelin]

Hi Peff,

On Thu, 22 Nov 2018, Jeff King wrote:

> On Thu, Nov 22, 2018 at 01:48:53PM +0100, Johannes Schindelin wrote:
> 
> > 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>.
> 
> 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.

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

Ciao,
Dscho