All of lore.kernel.org
 help / color / mirror / Atom feed
* What's cooking in git.git (Sep 2021, #08; Mon, 27)
@ 2021-09-28  0:52 Junio C Hamano
  2021-09-28  1:57 ` Ævar Arnfjörð Bjarmason
                   ` (6 more replies)
  0 siblings, 7 replies; 40+ messages in thread
From: Junio C Hamano @ 2021-09-28  0:52 UTC (permalink / raw)
  To: git

Here are the topics that have been cooking in my tree.  Commits
prefixed with '+' are in 'next' (being in 'next' is a sign that a
topic is stable enough to be used and are candidate to be in a
future release).  Commits prefixed with '-' are only in 'seen',
which means nothing more than that I have found them of interest for
some reason (like "it may have hard-to-resolve conflicts with
another topic already in flight" or "this may turn out to be
useful").  Do not read too much into a topic being in (or not in)
'seen'.  The ones marked with '.' do not appear in any of the
integration branches, but I am still holding onto them.



Copies of the source code to Git live in many repositories, and the
following is a list of the ones I push into or their mirrors.  Some
repositories have only a subset of branches.

With maint, master, next, seen, todo:

	git://git.kernel.org/pub/scm/git/git.git/
	git://repo.or.cz/alt-git.git/
	https://kernel.googlesource.com/pub/scm/git/git/
	https://github.com/git/git/
	https://gitlab.com/git-vcs/git/

With all the integration branches and topics broken out:

	https://github.com/gitster/git/

Even though the preformatted documentation in HTML and man format
are not sources, they are published in these repositories for
convenience (replace "htmldocs" with "manpages" for the manual
pages):

	git://git.kernel.org/pub/scm/git/git-htmldocs.git/
	https://github.com/gitster/git-htmldocs.git/

Release tarballs are available at:

	https://www.kernel.org/pub/software/scm/git/

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

* ab/bundle-remove-verbose-option (2021-09-23) 1 commit
 - bundle: remove ignored & undocumented "--verbose" flag

 Doc update.

 Will merge to 'next'.


* ab/config-based-hooks-1 (2021-09-27) 8 commits
 - hook-list.h: add a generated list of hooks, like config-list.h
 - hook.c users: use "hook_exists()" instead of "find_hook()"
 - hook.c: add a hook_exists() wrapper and use it in bugreport.c
 - hook.[ch]: move find_hook() from run-command.c to hook.c
 - Makefile: remove an out-of-date comment
 - Makefile: don't perform "mv $@+ $@" dance for $(GENERATED_H)
 - Makefile: stop hardcoding {command,config}-list.h
 - Makefile: mark "check" target as .PHONY


* ab/http-pinned-public-key-mismatch (2021-09-27) 1 commit
 - http: check CURLE_SSL_PINNEDPUBKEYNOTMATCH when emitting errors

 HTTPS error handling updates.

 Will merge to 'next'?


* ab/make-compdb-fix (2021-09-27) 1 commit
 - Makefile: pass -Wno-pendantic under GENERATE_COMPILATION_DATABASE=yes

 Build update.

 Will merge to 'next'.


* jk/ref-paranoia (2021-09-27) 16 commits
 - refs: drop "broken" flag from for_each_fullref_in()
 - ref-filter: drop broken-ref code entirely
 - ref-filter: stop setting FILTER_REFS_INCLUDE_BROKEN
 - repack, prune: drop GIT_REF_PARANOIA settings
 - refs: turn on GIT_REF_PARANOIA by default
 - refs: omit dangling symrefs when using GIT_REF_PARANOIA
 - refs: add DO_FOR_EACH_OMIT_DANGLING_SYMREFS flag
 - refs-internal.h: reorganize DO_FOR_EACH_* flag documentation
 - refs-internal.h: move DO_FOR_EACH_* flags next to each other
 - t5312: be more assertive about command failure
 - t5312: test non-destructive repack
 - t5312: create bogus ref as necessary
 - t5312: drop "verbose" helper
 - t5600: provide detached HEAD for corruption failures
 - t5516: don't use HEAD ref for invalid ref-deletion tests
 - t7900: clean up some more broken refs

 The ref iteration code used to optionally allow dangling refs to be
 shown, which has been tightened up.

 Will merge to 'next'?


* pw/rebase-reread-todo-after-editing (2021-09-24) 2 commits
 - rebase: fix todo-list rereading
 - sequencer.c: factor out a function

 The code to re-read the edited todo list in "git rebase -i" was
 made more robust.

 Will merge to 'next'.


* rs/close-pack-leakfix (2021-09-24) 1 commit
 - packfile: release bad_objects in close_pack()

 Leakfix.

 Will merge to 'next'.

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

* ab/pack-objects-stdin (2021-07-09) 5 commits
 - pack-objects.c: make use of REV_INFO_STDIN_LINE_PROCESS
 - pack-objects.c: do stdin parsing via revision.c's API
 - revision.[ch]: add a "handle_stdin_line" API
 - revision.h: refactor "disable_stdin" and "read_from_stdin"
 - upload-pack: run is_repository_shallow() before setup_revisions()

 Introduce handle_stdin_line callback to revision API and uses it.

 Expecting a reroll.


* es/superproject-aware-submodules (2021-08-19) 5 commits
 . fixup! introduce submodule.superprojectGitDir record
 . submodule: record superproject gitdir during 'update'
 . submodule: record superproject gitdir during absorbgitdirs
 . introduce submodule.superprojectGitDir record
 . t7400-submodule-basic: modernize inspect() helper

 A configuration variable in a submodule points at the location of
 the superproject it is bound to (RFC).

 Kicked out of 'seen' tentatively to see how well a conflicting
 ar/submodule-update topic fares.


* ab/fsck-unexpected-type (2021-09-22) 17 commits
 - fsck: report invalid object type-path combinations
 - fsck: don't hard die on invalid object types
 - object-file.c: stop dying in parse_loose_header()
 - object-file.c: return ULHR_TOO_LONG on "header too long"
 - object-file.c: use "enum" return type for unpack_loose_header()
 - object-file.c: simplify unpack_loose_short_header()
 - object-file.c: make parse_loose_header_extended() public
 - object-file.c: return -1, not "status" from unpack_loose_header()
 - object-file.c: don't set "typep" when returning non-zero
 - cat-file tests: test for current --allow-unknown-type behavior
 - cat-file tests: add corrupt loose object test
 - cat-file tests: test for missing/bogus object with -t, -s and -p
 - cat-file tests: move bogus_* variable declarations earlier
 - fsck tests: test for garbage appended to a loose object
 - fsck tests: test current hash/type mismatch behavior
 - fsck tests: refactor one test to use a sub-repo
 - fsck tests: add test for fsck-ing an unknown type

 "git fsck" has been taught to report mismatch between expected and
 actual types of an object better.

 Needs review.

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

* ab/make-clean-depend-dirs (2021-09-22) 1 commit
 - Makefile: clean .depend dirs under COMPUTE_HEADER_DEPENDENCIES != yes

 "make clean" has been updated to remove leftover .depend/
 directories, even when it is not told to use them to compute header
 dependencies.

 Will merge to 'next'.


* bs/ls-files-opt-help-text-update (2021-09-22) 1 commit
 - ls-files: use imperative mood for -X and -z option description

 Help test for "ls-files" options have been updated.

 Will merge to 'next'.


* ds/perf-test-built-path-fix (2021-09-22) 1 commit
 - t/perf/run: fix bin-wrappers computation

 Perf test fix.

 Will merge to 'next'.


* gc/doc-first-contribution-reroll (2021-09-22) 1 commit
 - MyFirstContribution: Document --range-diff option when writing v2

 Doc update.

 Will merge to 'next'.


* jk/grep-haystack-is-read-only (2021-09-22) 5 commits
 - grep: store grep_source buffer as const
 - grep: mark "haystack" buffers as const
 - grep: stop modifying buffer in grep_source_1()
 - grep: stop modifying buffer in show_line()
 - grep: stop modifying buffer in strip_timestamp
 (this branch is used by hm/paint-hits-in-log-grep.)

 Code clean-up in the "grep" machinery.

 Will merge to 'next'.


* jk/http-redact-fix (2021-09-22) 1 commit
 - http: match headers case-insensitively when redacting

 Sensitive data in the HTTP trace were supposed to be redacted, but
 we failed to do so in HTTP/2 requests.

 Will merge to 'next'.


* js/win-lazyload-buildfix (2021-09-27) 2 commits
 - lazyload.h: use an even more generic function pointer than FARPROC
 - lazyload.h: fix warnings about mismatching function pointer types

 Compilation fix.

 Will merge to 'next'.


* ab/auto-depend-with-pedantic (2021-09-22) 1 commit
 - Makefile: make COMPUTE_HEADER_DEPENDENCIES=auto work with DEVOPTS=pedantic

 Improve build procedure for developers.

 Will merge to 'next'.


* ab/make-sparse-for-real (2021-09-22) 1 commit
 - Makefile: make the "sparse" target non-.PHONY

 Prevent "make sparse" from running for the source files that
 haven't been modified.

 Will merge to 'next'?


* bs/difftool-msg-tweak (2021-09-22) 1 commit
 - difftool: fix word spacing in the usage strings

 Message tweak.

 Will merge to 'next'.


* ew/midx-doc-update (2021-09-24) 1 commit
 - doc/technical: update note about core.multiPackIndex

 Doc tweak.

 Will merge to 'next'.


* jt/no-abuse-alternate-odb-for-submodules (2021-09-22) 10 commits
 . submodule: trace adding submodule ODB as alternate
 . refs: change refs_for_each_ref_in() to take repo
 . submodule: pass repo to check_has_commit()
 . object-file: only register submodule ODB if needed
 . merge-{ort,recursive}: remove add_submodule_odb()
 . refs: teach refs_for_each_ref() arbitrary repos
 . refs iterator: support non-the_repository advance
 . refs: add repo paramater to _iterator_peel()
 . refs: make _advance() check struct repo, not flag
 . Merge branch 'jt/add-submodule-odb-clean-up' into jt/no-abuse-alternate-odb-for-submodules
 (this branch uses jt/add-submodule-odb-clean-up.)

 Follow through the work to use the repo interface to access
 submodule objects in-process, instead of abusing the alternate
 object database interface.

 Expecting a reroll on top of the jk/ref-paranoia topic.


* tp/send-email-completion (2021-09-22) 3 commits
 - send-email docs: add format-patch options
 - send-email: move bash completions to core script
 - send-email: terminate --git-completion-helper with LF

 The command line complation for "git send-email" options have been
 tweaked to make it easier to keep it in sync with the command itself.


* hm/paint-hits-in-log-grep (2021-09-24) 3 commits
 - pretty: colorize pattern matches in commit messages
 - grep: refactor next_match() and match_one_pattern() for external use
 - Merge branch 'jk/grep-haystack-is-read-only' into hm/paint-hits-in-log-grep
 (this branch uses jk/grep-haystack-is-read-only.)

 "git log --grep=string --author=name" learns to highlight hits just
 like "git grep string" does.


* ab/repo-settings-cleanup (2021-09-22) 5 commits
 - repository.h: don't use a mix of int and bitfields
 - repo-settings.c: simplify the setup
 - read-cache & fetch-negotiator: check "enum" values in switch()
 - environment.c: remove test-specific "ignore_untracked..." variable
 - wrapper.c: add x{un,}setenv(), and use xsetenv() in environment.c

 Code cleanup.

 Will merge to 'next'.


* ah/connect-parse-feature-v0-fix (2021-09-27) 1 commit
 - connect: also update offset for features without values

 Protocol v0 clients can get stuck parsing a malformed feature line.

 Will merge to 'next'.


* ah/unreak-revisions (2021-09-20) 2 commits
 - log: UNLEAK original pending objects
 - log: UNLEAK rev to silence a large number of leaks

 Mark a few structures with UNLEAK() to help leak detection CI jobs.

 Will merge to 'next'.


* ar/submodule-update (2021-09-20) 8 commits
 - submodule--helper: rename helper functions
 - submodule--helper: remove unused helpers
 - submodule--helper: remove update-clone subcommand
 - submodule: move core cmd_update() logic to C
 - submodule--helper: refactor get_submodule_displaypath()
 - submodule--helper: rename helpers for update-clone
 - submodule--helper: get remote names from any repository
 - submodule--helper: split up ensure_core_worktree()

 Rewrite of "git submodule update" in C.

 Expecting a reroll.
 Stomps on a handful of other topics and comes with an overly large step.


* da/difftool (2021-09-23) 4 commits
 - difftool: add a missing space to the run_dir_diff() comments
 - difftool: create a tmpdir path without repeated slashes
 - Merge branch 'da/difftool-dir-diff-symlink-fix' into da/difftool
 - Merge branch 'ab/retire-option-argument' into da/difftool
 (this branch uses da/difftool-dir-diff-symlink-fix.)

 Code clean-up in "git difftool".

 Will merge to 'next'?


* da/difftool-dir-diff-symlink-fix (2021-09-23) 1 commit
 - difftool: fix symlink-file writing in dir-diff mode
 (this branch is used by da/difftool.)

 "git difftool --dir-diff" mishandled symbolic links.

 Will merge to 'next'.


* en/removing-untracked-fixes (2021-09-27) 12 commits
 - Documentation: call out commands that nuke untracked files/directories
 - Comment important codepaths regarding nuking untracked files/dirs
 - unpack-trees: avoid nuking untracked dir in way of locally deleted file
 - unpack-trees: avoid nuking untracked dir in way of unmerged file
 - Change unpack_trees' 'reset' flag into an enum
 - Remove ignored files by default when they are in the way
 - unpack-trees: make dir an internal-only struct
 - unpack-trees: introduce preserve_ignored to unpack_trees_options
 - read-tree, merge-recursive: overwrite ignored files by default
 - checkout, read-tree: fix leak of unpack_trees_options.dir
 - t2500: add various tests for nuking untracked files
 - Merge branch 'en/am-abort-fix' into en/removing-untracked-fixes

 Various fixes in code paths that move untracked files away to make room.

 Will merge to 'next'?


* en/typofixes (2021-09-20) 2 commits
  (merged to 'next' on 2021-09-20 at 48648dafa3)
 + merge-ort: fix completely wrong comment
 + trace2.h: fix trivial comment typo

 Typofixes.

 Will merge to 'master'.


* ks/submodule-add-message-fix (2021-09-20) 1 commit
 - submodule--helper: fix incorrect newlines in an error message

 Message regression fix.

 Will merge to 'next'.


* tb/commit-graph-usage-fix (2021-09-22) 2 commits
 - builtin/multi-pack-index.c: disable top-level --[no-]progress
 - builtin/commit-graph.c: don't accept common --[no-]progress

 Regression fix for "git commit-graph" command line parsing.

 Will merge to 'next'.


* ws/refer-to-forkpoint-config-in-rebase-doc (2021-09-20) 1 commit
 - Document `rebase.forkpoint` in rebase man page

 Doc update.

 Will merge to 'next'?


* jk/clone-unborn-head-in-bare (2021-09-20) 1 commit
  (merged to 'next' on 2021-09-20 at 93c93b8d51)
 + clone: handle unborn branch in bare repos

 "git clone" from a repository whose HEAD is unborn into a bare
 repository didn't follow the branch name the other side used, which
 is corrected.

 Will merge to 'master'.


* jk/reduce-malloc-in-v2-servers (2021-09-15) 12 commits
  (merged to 'next' on 2021-09-16 at 40cfe41efc)
 + ls-refs: reject unknown arguments
 + serve: reject commands used as capabilities
 + serve: reject bogus v2 "command=ls-refs=foo"
 + docs/protocol-v2: clarify some ls-refs ref-prefix details
 + ls-refs: ignore very long ref-prefix counts
 + serve: drop "keys" strvec
 + serve: provide "receive" function for session-id capability
 + serve: provide "receive" function for object-format capability
 + serve: add "receive" method for v2 capabilities table
 + serve: return capability "value" from get_capability()
 + serve: rename is_command() to parse_command()
 + Merge branch 'ab/serve-cleanup' into jk/reduce-malloc-in-v2-servers

 Code cleanup to limit memory consumption and tighten protocol
 message parsing.

 Will merge to 'master'.


* ns/batched-fsync (2021-09-27) 8 commits
 - core.fsyncobjectfiles: performance tests for add and stash
 - core.fsyncobjectfiles: tests for batch mode
 - unpack-objects: use the bulk-checkin infrastructure
 - update-index: use the bulk-checkin infrastructure
 - core.fsyncobjectfiles: add windows support for batch mode
 - core.fsyncobjectfiles: batched disk flushes
 - bulk-checkin: rename 'state' variable and separate 'plugged' boolean
 - object-file.c: do not rename in a temp odb

 The "core.fsyncobjectfiles" configuration variable can now be set
 to "batch" for improved performance.

 Will merge to 'next'?


* jh/builtin-fsmonitor-part1 (2021-09-20) 7 commits
 - t/helper/simple-ipc: convert test-simple-ipc to use start_bg_command
 - run-command: create start_bg_command
 - simple-ipc/ipc-win32: add Windows ACL to named pipe
 - simple-ipc/ipc-win32: add trace2 debugging
 - simple-ipc: move definition of ipc_active_state outside of ifdef
 - simple-ipc: preparations for supporting binary messages.
 - trace2: add trace2_child_ready() to report on background children

 Built-in fsmonitor (part 1).


* cb/cvsserver (2021-09-16) 3 commits
  (merged to 'next' on 2021-09-23 at 98f5f3f9cf)
 + Documentation: cleanup git-cvsserver
 + git-cvsserver: protect against NULL in crypt(3)
 + git-cvsserver: use crypt correctly to compare password hashes

 "git cvsserver" had a long-standing bug in its authentication code,
 which has finally been corrected (it is unclear and is a separate
 question if anybody is seriously using it, though).

 Will merge to 'master'.


* cb/unicode-14 (2021-09-17) 1 commit
  (merged to 'next' on 2021-09-20 at 7873b310ad)
 + unicode: update the width tables to Unicode 14

 The unicode character width table (used for output alignment) has
 been updated.

 Will merge to 'master'.


* ab/align-parse-options-help (2021-09-22) 4 commits
 - parse-options: properly align continued usage output
 - git rev-parse --parseopt tests: add more usagestr tests
 - send-pack: properly use parse_options() API for usage string
 - parse-options API users: align usage output in C-strings

 When "git cmd -h" shows more than one line of usage text (e.g.
 the cmd subcommand may take sub-sub-command), parse-options API
 learned to align these lines, even across i18n/l10n.

 Will merge to 'next'?


* ab/help-config-vars (2021-09-23) 9 commits
 - help: move column config discovery to help.c library
 - help / completion: make "git help" do the hard work
 - help tests: test --config-for-completion option & output
 - help: simplify by moving to OPT_CMDMODE()
 - help: correct logic error in combining --all and --guides
 - help: correct logic error in combining --all and --config
 - help tests: add test for --config output
 - help: correct usage & behavior of "git help --guides"
 - help: correct the usage string in -h and documentation

 Teach "git help -c" into helping the command line completion of
 configuration variables.

 Will merge to 'next'?


* tb/repack-write-midx (2021-09-16) 8 commits
 - builtin/repack.c: pass `--refs-snapshot` when writing bitmaps
 - builtin/repack.c: make largest pack preferred
 - builtin/repack.c: support writing a MIDX while repacking
 - builtin/repack.c: extract showing progress to a variable
 - builtin/repack.c: keep track of existing packs unconditionally
 - midx: preliminary support for `--refs-snapshot`
 - builtin/multi-pack-index.c: support `--stdin-packs` mode
 - midx: expose `write_midx_file_only()` publicly

 "git repack" has been taught to generate multi-pack reachability
 bitmaps.

 Under review.
 cf. <YU0TS+KsWY36oeKU@nand.local>


* ds/add-rm-with-sparse-index (2021-09-24) 13 commits
 - advice: update message to suggest '--sparse'
 - mv: refuse to move sparse paths
 - rm: skip sparse paths with missing SKIP_WORKTREE
 - rm: add --sparse option
 - add: update --renormalize to skip sparse paths
 - add: update --chmod to skip sparse paths
 - add: implement the --sparse option
 - add: skip tracked paths outside sparse-checkout cone
 - add: fail when adding an untracked sparse file
 - dir: fix pattern matching on dirs
 - dir: select directories correctly
 - t1092: behavior for adding sparse files
 - t3705: test that 'sparse_entry' is unstaged

 "git add", "git mv", and "git rm" have been adjusted to avoid
 updating paths outside of the sparse-checkout definition unless
 the user specifies a "--sparse" option.

 Will merge to 'next'?


* tb/midx-write-propagate-namehash (2021-09-17) 7 commits
 - t5326: test propagating hashcache values
 - p5326: generate pack bitmaps before writing the MIDX bitmap
 - p5326: don't set core.multiPackIndex unnecessarily
 - p5326: create missing 'perf-tag' tag
 - midx.c: respect 'pack.writeBitmapHashcache' when writing bitmaps
 - pack-bitmap.c: propagate namehash values from existing bitmaps
 - t/helper/test-bitmap.c: add 'dump-hashes' mode

 "git multi-pack-index write --bitmap" learns to propagate the
 hashcache from original bitmap to resulting bitmap.

 Will merge to 'next'.


* pw/rebase-of-a-tag-fix (2021-09-22) 10 commits
 - rebase: dereference tags
 - rebase: use lookup_commit_reference_by_name()
 - rebase: use our standard error return value
 - t3407: rework rebase --quit tests
 - t3407: strengthen rebase --abort tests
 - t3407: use test_path_is_missing
 - t3407: rename a variable
 - t3407: use test_cmp_rev
 - t3407: use test_commit
 - t3407: run tests in $TEST_DIRECTORY

 "git rebase <upstream> <tag>" failed when aborted in the middle, as
 it mistakenly tried to write the tag object instead of peeling it
 to HEAD.

 Will merge to 'next'.


* en/zdiff3 (2021-09-20) 2 commits
 - update documentation for new zdiff3 conflictStyle
 - xdiff: implement a zealous diff3, or "zdiff3"

 "Zealous diff3" style of merge conflict presentation has been added.


* en/stash-df-fix (2021-09-10) 3 commits
  (merged to 'next' on 2021-09-20 at 513c11fb11)
 + stash: restore untracked files AFTER restoring tracked files
 + stash: avoid feeding directories to update-index
 + t3903: document a pair of directory/file bugs

 Fix "git stash" corner case where the tentative change involves
 changing a directory to a file (or vice versa).

 Will merge to 'master'.


* jt/add-submodule-odb-clean-up (2021-09-09) 3 commits
 - revision: remove "submodule" from opt struct
 - repository: support unabsorbed in repo_submodule_init
 - submodule: remove unnecessary unabsorbed fallback
 (this branch is used by jt/no-abuse-alternate-odb-for-submodules.)

 More code paths that uses the hack to add submodule's object
 database to the set of alternate object store have been cleaned up.

 Will merge to 'next'.


* jx/ci-l10n (2021-09-09) 1 commit
  (merged to 'next' on 2021-09-23 at b2d7f5eecb)
 + ci: new github-action for git-l10n code review

 CI help for l10n.

 Will merge to 'master'.


* js/scalar (2021-09-14) 15 commits
 - scalar: accept -C and -c options before the subcommand
 - scalar: implement the `version` command
 - scalar: implement the `delete` command
 - scalar: teach 'reconfigure' to optionally handle all registered enlistments
 - scalar: allow reconfiguring an existing enlistment
 - scalar: implement the `run` command
 - scalar: teach 'clone' to support the --single-branch option
 - scalar: implement the `clone` subcommand
 - scalar: implement 'scalar list'
 - scalar: let 'unregister' handle a deleted enlistment directory gracefully
 - scalar: 'unregister' stops background maintenance
 - scalar: 'register' sets recommended config and starts maintenance
 - scalar: create test infrastructure
 - scalar: start documenting the command
 - scalar: create a rudimentary executable

 Add pieces from "scalar" to contrib/.

 Will merge to 'next'?


* ab/sanitize-leak-ci (2021-09-23) 2 commits
 - tests: add a test mode for SANITIZE=leak, run it in CI
 - Makefile: add SANITIZE=leak flag to GIT-BUILD-OPTIONS

 CI learns to run the leak sanitizer builds.

 Will merge to 'next'?


* ms/customizable-ident-expansion (2021-09-01) 1 commit
 - keyword expansion: make "$Id$" string configurable

 Instead of "$Id$", user-specified string (like $FreeBSD$) can be
 used as an in-blob placeholder for keyword expansion.


* js/retire-preserve-merges (2021-09-07) 11 commits
  (merged to 'next' on 2021-09-10 at f645ffd7a3)
 + sequencer: restrict scope of a formerly public function
 + rebase: remove a no-longer-used function
 + rebase: stop mentioning the -p option in comments
 + rebase: remove obsolete code comment
 + rebase: drop the internal `rebase--interactive` command
 + git-svn: drop support for `--preserve-merges`
 + rebase: drop support for `--preserve-merges`
 + pull: remove support for `--rebase=preserve`
 + tests: stop testing `git rebase --preserve-merges`
 + remote: warn about unhandled branch.<name>.rebase values
 + t5520: do not use `pull.rebase=preserve`

 The "--preserve-merges" option of "git rebase" has been removed.

 Will cook in 'next'.


* en/remerge-diff (2021-08-31) 7 commits
 - doc/diff-options: explain the new --remerge-diff option
 - show, log: provide a --remerge-diff capability
 - tmp-objdir: new API for creating and removing primary object dirs
 - merge-ort: capture and print ll-merge warnings in our preferred fashion
 - ll-merge: add API for capturing warnings in a strbuf instead of stderr
 - merge-ort: add ability to record conflict messages in a file
 - merge-ort: mark a few more conflict messages as omittable

 A new presentation for two-parent merge "--remerge-diff" can be
 used to show the difference between mechanical (and possibly
 conflicted) merge results and the recorded resolution.

 Will merge to 'next'?


* sg/test-split-index-fix (2021-09-07) 7 commits
 - read-cache: fix GIT_TEST_SPLIT_INDEX
 - tests: disable GIT_TEST_SPLIT_INDEX for sparse index tests
 - read-cache: look for shared index files next to the index, too
 - t1600-index: disable GIT_TEST_SPLIT_INDEX
 - t1600-index: don't run git commands upstream of a pipe
 - t1600-index: remove unnecessary redirection
 - Merge branch 'ds/sparse-index-ignored-files' into sg/test-split-index-fix

 Test updates.

 Will merge to 'next'?


* ab/refs-errno-cleanup (2021-08-25) 4 commits
 - refs: make errno output explicit for refs_resolve_ref_unsafe
 - refs: explicitly return failure_errno from parse_loose_ref_contents
 - branch tests: test for errno propagating on failing read
 - refs: add failure_errno to refs_read_raw_ref() signature
 (this branch uses ab/refs-files-cleanup and hn/refs-errno-cleanup.)

 The "remainder" of hn/refs-errno-cleanup topic.


* ab/lib-subtest (2021-09-22) 9 commits
 - test-lib tests: get rid of copy/pasted mock test code
 - test-lib tests: assert 1 exit code, not non-zero
 - test-lib tests: refactor common part of check_sub_test_lib_test*()
 - test-lib tests: avoid subshell for "test_cmp" for readability
 - test-lib tests: don't provide a description for the sub-tests
 - test-lib tests: split up "write and run" into two functions
 - test-lib tests: move "run_sub_test" to a new lib-subtest.sh
 - Merge branch 'ps/t0000-output-directory-fix' into ab/lib-subtest
 - Merge branch 'jk/t0000-subtests-fix' into ab/lib-subtest

 Updates to the tests in t0000 to test the test framework.

 Will merge to 'next'?


* ab/only-single-progress-at-once (2021-09-22) 8 commits
 - progress.c: add & assert a "global_progress" variable
 - pack-bitmap-write.c: add a missing stop_progress()
 - progress.c: add temporary variable from progress struct
 - progress.c: stop eagerly fflush(stderr) when not a terminal
 - progress.c: call progress_interval() from progress_test_force_update()
 - progress.c: move signal handler functions lower
 - progress.c tests: test some invalid usage
 - progress.c tests: make start/stop verbs on stdin

 Further tweaks on progress API.


* fs/ssh-signing (2021-09-10) 9 commits
 - ssh signing: test that gpg fails for unknown keys
 - ssh signing: tests for logs, tags & push certs
 - ssh signing: duplicate t7510 tests for commits
 - ssh signing: verify signatures using ssh-keygen
 - ssh signing: provide a textual signing_key_id
 - ssh signing: retrieve a default key from ssh-agent
 - ssh signing: add ssh key format and signing code
 - ssh signing: add test prereqs
 - ssh signing: preliminary refactoring and clean-up

 Use ssh public crypto for object and push-cert signing.

 On hold.
 cf. <pull.1041.v8.git.git.1631304462.gitgitgadget@gmail.com>
 cf. <532d97e7-8c91-df6a-6d90-70668256f513@gigacodes.de>


* cf/fetch-set-upstream-while-detached (2021-07-06) 1 commit
 - fetch: fix segfault on --set-upstream while on a detached HEAD

 "git fetch --set-upstream" while on detached HEAD segfaulted
 instead of noticing that such an operation did not make sense.

 Expecting a reroll.
 cf. <xmqqsg0ri5mq.fsf@gitster.g>


* pw/diff-color-moved-fix (2021-08-05) 13 commits
 - diff: drop unused options parameter from cmp_in_block_with_wsd()
 - diff --color-moved: intern strings
 - diff: use designated initializers for emitted_diff_symbol
 - diff --color-moved-ws=allow-indentation-change: improve hash lookups
 - diff --color-moved: stop clearing potential moved blocks
 - diff --color-moved: shrink potential moved blocks as we go
 - diff --color-moved: unify moved block growth functions
 - diff --color-moved: call comparison function directly
 - diff --color-moved-ws=allow-indentation-change: simplify and optimize
 - diff: simplify allow-indentation-change delta calculation
 - diff --color-moved: avoid false short line matches and bad zerba coloring
 - diff --color-moved=zebra: fix alternate coloring
 - diff --color-moved: add perf tests

 Originally merged to 'next' on 2021-08-05

 Long-overdue correctness and performance update to "diff
 --color-moved" feature.

 Expecting a reroll.
 cf. <8bec1a6d-5052-50c3-4100-e6348289d581@gmail.com>


* hn/reftable (2021-09-10) 20 commits
 - fixup! reftable: implement stack, a mutable database of reftable files.
 - Add "test-tool dump-reftable" command.
 - reftable: add dump utility
 - reftable: implement stack, a mutable database of reftable files.
 - reftable: implement refname validation
 - reftable: add merged table view
 - reftable: add a heap-based priority queue for reftable records
 - reftable: reftable file level tests
 - reftable: read reftable files
 - reftable: generic interface to tables
 - reftable: write reftable files
 - reftable: a generic binary tree implementation
 - reftable: reading/writing blocks
 - Provide zlib's uncompress2 from compat/zlib-compat.c
 - reftable: (de)serialization for the polymorphic record type.
 - reftable: add blocksource, an abstraction for random access reads
 - reftable: utility functions
 - reftable: add error related functionality
 - reftable: RFC: add LICENSE
 - hash.h: provide constants for the hash IDs

 The "reftable" backend for the refs API, without integrating into
 the refs subsystem.

 Will merge to 'next'?


* ab/refs-files-cleanup (2021-08-25) 13 commits
  (merged to 'next' on 2021-09-23 at eb5668523f)
 + refs/files: remove unused "errno != ENOTDIR" condition
 + refs/files: remove unused "errno == EISDIR" code
 + refs/files: remove unused "oid" in lock_ref_oid_basic()
 + refs API: remove OID argument to reflog_expire()
 + reflog expire: don't lock reflogs using previously seen OID
 + refs/files: add a comment about refs_reflog_exists() call
 + refs: make repo_dwim_log() accept a NULL oid
 + refs/debug: re-indent argument list for "prepare"
 + refs/files: remove unused "skip" in lock_raw_ref() too
 + refs/files: remove unused "extras/skip" in lock_ref_oid_basic()
 + refs: drop unused "flags" parameter to lock_ref_oid_basic()
 + refs/files: remove unused REF_DELETING in lock_ref_oid_basic()
 + refs/packet: add missing BUG() invocations to reflog callbacks
 (this branch is used by ab/refs-errno-cleanup and hn/refs-errno-cleanup.)

 Continued work on top of the hn/refs-errno-cleanup topic.

 Will merge to 'master'.


* hn/refs-errno-cleanup (2021-08-25) 4 commits
  (merged to 'next' on 2021-09-23 at 502e6b6b08)
 + refs: make errno output explicit for read_raw_ref_fn
 + refs/files-backend: stop setting errno from lock_ref_oid_basic
 + refs: remove EINVAL errno output from specification of read_raw_ref_fn
 + refs file backend: move raceproof_create_file() here
 (this branch is used by ab/refs-errno-cleanup; uses ab/refs-files-cleanup.)

 Futz with the way 'errno' is relied on in the refs API to carry the
 failure modes up the call chain.

 Will merge to 'master'.

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

* ao/p4-avoid-decoding (2021-04-12) 2 commits
 . git-p4: do not decode data from perforce by default
 . git-p4: avoid decoding more data from perforce

 "git p4" in Python-2 days used to accept a lot more kinds of data
 from Perforce server as uninterrupted byte sequence, but after
 switching to Python-3, too many things are expected to be in UTF-8,
 which broke traditional use cases.

 Have been stalled for too long.
 cf. <20210504220153.1d9f0cb2@ado-tr>


* tv/p4-fallback-encoding (2021-04-30) 1 commit
 . git-p4: git-p4.fallbackEncoding to specify non UTF-8 charset

 "git p4" learns the fallbackEncoding configuration variable to
 safely accept changeset descriptions that aren't written in UTF-8.

 Have been stalled for too long.
 cf. <CAKu1iLUaLuAZWqjNK4tfhhR=YaSt4MdQ+90ZY-JcEh_SeHyYCw@mail.gmail.com>


* jh/builtin-fsmonitor (2021-09-03) 37 commits
 . fixup! fsmonitor--daemon: implement handle_client callback
 . SQUASH??? https://github.com/git/git/runs/3438543601?check_suite_focus=true#step:5:136
 . BANDAID: sparse fixes
 . t7527: test FS event reporing on MacOS WRT case and Unicode
 . fsmonitor: handle shortname for .git
 . t7527: test status with untracked-cache and fsmonitor--daemon
 . fsmonitor: force update index after large responses
 . fsmonitor: enhance existing comments
 . fsmonitor--daemon: use a cookie file to sync with file system
 . fsmonitor--daemon: periodically truncate list of modified files
 . t7527: create test for fsmonitor--daemon
 . t/perf/p7519: add fsmonitor--daemon test cases
 . t/perf: avoid copying builtin fsmonitor files into test repo
 . t/perf/p7519: speed up test using "test-tool touch"
 . t/helper/test-touch: add helper to touch a series of files
 . fsmonitor--daemon: implement handle_client callback
 . fsmonitor-fs-listen-macos: implement FSEvent listener on MacOS
 . fsmonitor-fs-listen-macos: add macos header files for FSEvent
 . fsmonitor-fs-listen-win32: implement FSMonitor backend on Windows
 . fsmonitor--daemon: create token-based changed path cache
 . fsmonitor--daemon: define token-ids
 . fsmonitor--daemon: add pathname classification
 . fsmonitor: do not try to operate on bare repos
 . fsmonitor--daemon: implement 'start' command
 . fsmonitor--daemon: implement 'run' command
 . fsmonitor-fs-listen-macos: stub in backend for MacOS
 . fsmonitor-fs-listen-win32: stub in backend for Windows
 . t/helper/fsmonitor-client: create IPC client to talk to FSMonitor Daemon
 . fsmonitor--daemon: implement 'stop' and 'status' commands
 . fsmonitor--daemon: add a built-in fsmonitor daemon
 . fsmonitor: use IPC to query the builtin FSMonitor daemon
 . fsmonitor: config settings are repository-specific
 . help: include fsmonitor--daemon feature flag in version info
 . fsmonitor-ipc: create client routines for git-fsmonitor--daemon
 . fsmonitor--daemon: update fsmonitor documentation
 . fsmonitor--daemon: man page
 . simple-ipc: preparations for supporting binary messages.

 An attempt to write and ship with a watchman equivalent tailored
 for our use.

 Will be rerolled in pieces.


* ab/config-based-hooks-base (2021-09-09) 36 commits
 . hooks: fix a TOCTOU in "did we run a hook?" heuristic
 . receive-pack: convert receive hooks to hook.h
 . post-update: use hook.h library
 . receive-pack: convert 'update' hook to hook.h
 . hooks: allow callers to capture output
 . run-command: allow capturing of collated output
 . reference-transaction: use hook.h to run hooks
 . hook tests: use a modern style for "pre-push" tests
 . hook tests: test for exact "pre-push" hook input
 . transport: convert pre-push hook to hook.h
 . hook: convert 'post-rewrite' hook in sequencer.c to hook.h
 . hook: provide stdin by string_list or callback
 . run-command: add stdin callback for parallelization
 . am: convert 'post-rewrite' hook to hook.h
 . hook: support passing stdin to hooks
 . run-command: allow stdin for run_processes_parallel
 . run-command: remove old run_hook_{le,ve}() hook API
 . receive-pack: convert push-to-checkout hook to hook.h
 . read-cache: convert post-index-change to use hook.h
 . commit: convert {pre-commit,prepare-commit-msg} hook to hook.h
 . git-p4: use 'git hook' to run hooks
 . send-email: use 'git hook run' for 'sendemail-validate'
 . git hook run: add an --ignore-missing flag
 . merge: convert post-merge to use hook.h
 . hooks: convert 'post-checkout' hook to hook library
 . am: convert applypatch to use hook.h
 . rebase: convert pre-rebase to use hook.h
 . gc: use hook library for pre-auto-gc hook
 . hook: add 'run' subcommand
 . hook-list.h: add a generated list of hooks, like config-list.h
 . hook.c users: use "hook_exists()" instead of "find_hook()"
 . hook.c: add a hook_exists() wrapper and use it in bugreport.c
 . hook.[ch]: move find_hook() from run-command.c to hook.c
 . Makefile: remove an out-of-date comment
 . Makefile: stop hardcoding {command,config}-list.h
 . Makefile: mark "check" target as .PHONY
 (this branch is used by es/config-based-hooks.)

 Restructuring of (a subset of) Emily's config-based-hooks series,
 to demonstrate that a series can be presented as a more logical and
 focused progression.

 Will be rerolled in pieces.


* es/config-based-hooks (2021-09-09) 6 commits
 . hook: allow out-of-repo 'git hook' invocations
 . hook: include hooks from the config
 . hook: introduce "git hook list"
 . hook: allow parallel hook execution
 . fixup! hook: run a list of hooks instead
 . hook: run a list of hooks instead
 (this branch uses ab/config-based-hooks-base.)

 Revamp the hooks subsystem to allow multiple of them to trigger
 upon the same event and control via the configuration variables.


* cb/make-compdb-fix (2021-09-22) 1 commit
 . Makefile: avoid breaking compilation database generation with DEVELOPER

 Adjust to recent change to use -pedantic for developer builds.

 Replaced by the ab/make-compdb-fix topic that uses the same
 approach as the ab/auto-depend-with-pedantic topic.


^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: What's cooking in git.git (Sep 2021, #08; Mon, 27)
  2021-09-28  0:52 What's cooking in git.git (Sep 2021, #08; Mon, 27) Junio C Hamano
@ 2021-09-28  1:57 ` Ævar Arnfjörð Bjarmason
  2021-09-28 20:52   ` Junio C Hamano
  2021-09-28  6:46 ` Elijah Newren
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 40+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-09-28  1:57 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, René Scharfe, Jeff King,
	Carlo Marcelo Arenas Belón, SZEDER Gábor, Taylor Blau


On Mon, Sep 27 2021, Junio C Hamano wrote:

Updates on my topics, CC'd the people I mentioned.

> * ab/config-based-hooks-1 (2021-09-27) 8 commits
>  - hook-list.h: add a generated list of hooks, like config-list.h
>  - hook.c users: use "hook_exists()" instead of "find_hook()"
>  - hook.c: add a hook_exists() wrapper and use it in bugreport.c
>  - hook.[ch]: move find_hook() from run-command.c to hook.c
>  - Makefile: remove an out-of-date comment
>  - Makefile: don't perform "mv $@+ $@" dance for $(GENERATED_H)
>  - Makefile: stop hardcoding {command,config}-list.h
>  - Makefile: mark "check" target as .PHONY

Thanks for picking this up. Looks like my split-up plan is working in
that this has received a lot of careful review.

I think per René's the v2 should be good-to-go, barring any new
undiscovered issues:
https://lore.kernel.org/git/0e07cee4-079a-af12-880f-d4a99300df28@web.de/

> * ab/http-pinned-public-key-mismatch (2021-09-27) 1 commit
>  - http: check CURLE_SSL_PINNEDPUBKEYNOTMATCH when emitting errors
>
>  HTTPS error handling updates.
>
>  Will merge to 'next'?

I'd like it merged. I think a fair summary of Jeff King's
https://lore.kernel.org/git/YU5CJC9XJvQITfr8@coredump.intra.peff.net/ is
"meh" though :)

I think the translation improvement + mentioning the specific git config
variable in play is a pretty minor but still worthwhile change.

> * ab/make-compdb-fix (2021-09-27) 1 commit
>  - Makefile: pass -Wno-pendantic under GENERATE_COMPILATION_DATABASE=yes
>
>  Build update.
>
>  Will merge to 'next'.

Thanks, FWIW I thought Carlos's cb/make-compdb-fix was better, but see
from this E-Mail & other context that you'd looked at both and preferred
the consistency with my COMPUTE_HEADER_DEPENDENCIES fix.

In any case both fix the immediate "pedantic" fallout.

> * ab/pack-objects-stdin (2021-07-09) 5 commits
>  - pack-objects.c: make use of REV_INFO_STDIN_LINE_PROCESS
>  - pack-objects.c: do stdin parsing via revision.c's API
>  - revision.[ch]: add a "handle_stdin_line" API
>  - revision.h: refactor "disable_stdin" and "read_from_stdin"
>  - upload-pack: run is_repository_shallow() before setup_revisions()
>
>  Introduce handle_stdin_line callback to revision API and uses it.
>
>  Expecting a reroll.

I said I'd re-roll this into the larger "git bundle --stdin" topic I had
in mind, but when doing so found one memory leak, which I really wanted
the SANITIZE=leak interface to fix.

I think it's best to eject/discard this given the amount of other
outstanding stuff I've got. I don't really *need* this for anything
else, I'll loop back to it sometime after the SANITIZE=leak testing has
landed.

> * ab/fsck-unexpected-type (2021-09-22) 17 commits
> [...]
>  "git fsck" has been taught to report mismatch between expected and
>  actual types of an object better.
>
>  Needs review.

The "needs review" note here pre-dates v6, which got a thorough review,
and Taylor seemed happy with the v7, aside from a minor issue I just
re-rolled v8 with a fix for:

https://lore.kernel.org/git/cover-v8-00.17-00000000000-20210928T021616Z-avarab@gmail.com/

> * ab/make-clean-depend-dirs (2021-09-22) 1 commit
> [...]
>  Will merge to 'next'.

Thanks!

> * ab/auto-depend-with-pedantic (2021-09-22) 1 commit
>  - Makefile: make COMPUTE_HEADER_DEPENDENCIES=auto work with DEVOPTS=pedantic
>
>  Improve build procedure for developers.
>
>  Will merge to 'next'.

Thanks!

> * ab/make-sparse-for-real (2021-09-22) 1 commit
>  - Makefile: make the "sparse" target non-.PHONY
>
>  Prevent "make sparse" from running for the source files that
>  haven't been modified.
>
>  Will merge to 'next'?

I just submitted a hopefully final v4, please ignore the v3 :)
https://lore.kernel.org/git/patch-v4-1.1-f31fa3e8282-20210928T014023Z-avarab@gmail.com/T/#u

That leaves "sparse" as the bug-for-bug compatible old target, with a
new "sparse-incr" for those (like me) who want the incremental
version. The feedback on v2 was mainly about the change in behavior
breaking existing workflows, so v4 should get around that nicely.

> * ab/repo-settings-cleanup (2021-09-22) 5 commits
> [...]
>  Will merge to 'next'.

Thanks!

> * ab/align-parse-options-help (2021-09-22) 4 commits
>  - parse-options: properly align continued usage output
>  - git rev-parse --parseopt tests: add more usagestr tests
>  - send-pack: properly use parse_options() API for usage string
>  - parse-options API users: align usage output in C-strings
>
>  When "git cmd -h" shows more than one line of usage text (e.g.
>  the cmd subcommand may take sub-sub-command), parse-options API
>  learned to align these lines, even across i18n/l10n.
>
>  Will merge to 'next'?

I think it's ready for that.

> * ab/help-config-vars (2021-09-23) 9 commits
>  - help: move column config discovery to help.c library
>  - help / completion: make "git help" do the hard work
>  - help tests: test --config-for-completion option & output
>  - help: simplify by moving to OPT_CMDMODE()
>  - help: correct logic error in combining --all and --guides
>  - help: correct logic error in combining --all and --config
>  - help tests: add test for --config output
>  - help: correct usage & behavior of "git help --guides"
>  - help: correct the usage string in -h and documentation
>
>  Teach "git help -c" into helping the command line completion of
>  configuration variables.
>
>  Will merge to 'next'?

I think so too, you had some feedback on the v3, I think partially
addressed by the "simplify by moving to OPT_CMDMODE()" step.

For further improving the usage info in the rare case of combining
e.g. --all and --config I'd prefer to just leave it for a future
improvement.

> * ab/sanitize-leak-ci (2021-09-23) 2 commits
>  - tests: add a test mode for SANITIZE=leak, run it in CI
>  - Makefile: add SANITIZE=leak flag to GIT-BUILD-OPTIONS
>
>  CI learns to run the leak sanitizer builds.
>
>  Will merge to 'next'?

Yes please! As noted in some other threads I've got more memory leak
fixes waiting on this, and since the v8 to avoid testing the split index
it doesn't seem to be causing any trouble:
https://lore.kernel.org/git/cover-v8-0.2-00000000000-20210923T091819Z-avarab@gmail.com/

> * ab/refs-errno-cleanup (2021-08-25) 4 commits
>  - refs: make errno output explicit for refs_resolve_ref_unsafe
>  - refs: explicitly return failure_errno from parse_loose_ref_contents
>  - branch tests: test for errno propagating on failing read
>  - refs: add failure_errno to refs_read_raw_ref() signature
>  (this branch uses ab/refs-files-cleanup and hn/refs-errno-cleanup.)
>
>  The "remainder" of hn/refs-errno-cleanup topic.

I think this should be ready too, but if you'd prefer to wait a bit
after ab/refs-files-cleanup and ab/refs-files-cleanup land I understand,
it's a lot of ref code changes...

> * ab/lib-subtest (2021-09-22) 9 commits
>  - test-lib tests: get rid of copy/pasted mock test code
>  - test-lib tests: assert 1 exit code, not non-zero
>  - test-lib tests: refactor common part of check_sub_test_lib_test*()
>  - test-lib tests: avoid subshell for "test_cmp" for readability
>  - test-lib tests: don't provide a description for the sub-tests
>  - test-lib tests: split up "write and run" into two functions
>  - test-lib tests: move "run_sub_test" to a new lib-subtest.sh
>  - Merge branch 'ps/t0000-output-directory-fix' into ab/lib-subtest
>  - Merge branch 'jk/t0000-subtests-fix' into ab/lib-subtest
>
>  Updates to the tests in t0000 to test the test framework.
>
>  Will merge to 'next'?

Yes please, it was waiting on review for a long time. Carlo had some
feedback on it that I re-rolled v4 addressing, resolving what I think
was the main sticking point all along. I.e. conflating a test assertion
mode with this t0000 cleanup/split-up:

https://lore.kernel.org/git/cover-v4-0.7-00000000000-20210922T111734Z-avarab@gmail.com/

> * ab/only-single-progress-at-once (2021-09-22) 8 commits
>  - progress.c: add & assert a "global_progress" variable
>  - pack-bitmap-write.c: add a missing stop_progress()
>  - progress.c: add temporary variable from progress struct
>  - progress.c: stop eagerly fflush(stderr) when not a terminal
>  - progress.c: call progress_interval() from progress_test_force_update()
>  - progress.c: move signal handler functions lower
>  - progress.c tests: test some invalid usage
>  - progress.c tests: make start/stop verbs on stdin
>
>  Further tweaks on progress API.

I think per
https://lore.kernel.org/git/cover-v2-0.8-00000000000-20210920T225701Z-avarab@gmail.com/
this should be ready to progress.

I.e. it was held off due to comment from SZEDER that once clarified I
understand to be a general concern that we should test the 8/8 step. As
noted in the re-rolled 8/8 I've tested this thoroughly in a way that
would break if that BUG() was going to bite us.

> * ab/refs-files-cleanup (2021-08-25) 13 commits
> [...]
>
>  Continued work on top of the hn/refs-errno-cleanup topic.
>
>  Will merge to 'master'.

[...]

> * hn/refs-errno-cleanup (2021-08-25) 4 commits
> [...]
>
>  Will merge to 'master'.

Yay, thanks!

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: What's cooking in git.git (Sep 2021, #08; Mon, 27)
  2021-09-28  0:52 What's cooking in git.git (Sep 2021, #08; Mon, 27) Junio C Hamano
  2021-09-28  1:57 ` Ævar Arnfjörð Bjarmason
@ 2021-09-28  6:46 ` Elijah Newren
  2021-09-28  7:45   ` Ævar Arnfjörð Bjarmason
                     ` (4 more replies)
  2021-09-28  8:22 ` da/difftool (was: Re: What's cooking in git.git (Sep 2021, #08; Mon, 27)) Ævar Arnfjörð Bjarmason
                   ` (4 subsequent siblings)
  6 siblings, 5 replies; 40+ messages in thread
From: Elijah Newren @ 2021-09-28  6:46 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List, Jeff King, Jonathan Nieder

[Did some slight re-ordering of topics]

On Mon, Sep 27, 2021 at 5:53 PM Junio C Hamano <gitster@pobox.com> wrote:

> * ds/add-rm-with-sparse-index (2021-09-24) 13 commits
>  - advice: update message to suggest '--sparse'
>  - mv: refuse to move sparse paths
>  - rm: skip sparse paths with missing SKIP_WORKTREE
>  - rm: add --sparse option
>  - add: update --renormalize to skip sparse paths
>  - add: update --chmod to skip sparse paths
>  - add: implement the --sparse option
>  - add: skip tracked paths outside sparse-checkout cone
>  - add: fail when adding an untracked sparse file
>  - dir: fix pattern matching on dirs
>  - dir: select directories correctly
>  - t1092: behavior for adding sparse files
>  - t3705: test that 'sparse_entry' is unstaged
>
>  "git add", "git mv", and "git rm" have been adjusted to avoid
>  updating paths outside of the sparse-checkout definition unless
>  the user specifies a "--sparse" option.
>
>  Will merge to 'next'?

It would be nice to see the --diff-filter=u change, which you also
seemed to like[1]; but after that, yeah this is ready to merge down.

[1] https://lore.kernel.org/git/xmqq35pppwsm.fsf@gitster.g/

> * js/scalar (2021-09-14) 15 commits
>  - scalar: accept -C and -c options before the subcommand
>  - scalar: implement the `version` command
>  - scalar: implement the `delete` command
>  - scalar: teach 'reconfigure' to optionally handle all registered enlistments
>  - scalar: allow reconfiguring an existing enlistment
>  - scalar: implement the `run` command
>  - scalar: teach 'clone' to support the --single-branch option
>  - scalar: implement the `clone` subcommand
>  - scalar: implement 'scalar list'
>  - scalar: let 'unregister' handle a deleted enlistment directory gracefully
>  - scalar: 'unregister' stops background maintenance
>  - scalar: 'register' sets recommended config and starts maintenance
>  - scalar: create test infrastructure
>  - scalar: start documenting the command
>  - scalar: create a rudimentary executable
>
>  Add pieces from "scalar" to contrib/.
>
>  Will merge to 'next'?

I feel bad for taking so long to take a look, but I finally responded
with a few comments.  Mostly, I'm glad it's a contrib thing; a lot of
the config makes sense to me (even if some of it is 'meh' for my
repository sizes or setup), but two of the config settings would be
very objectionable if they were a core git command.  Since it's
contrib, though, it's probably fine to be very opinionated...and
perhaps even excessively so.  ;-)

However, since Johannes has been away for a couple weeks, maybe give
him a chance to return and respond to myself and others (and perhaps
push any updates that occurred to him while on vacation) before
merging down?

> * en/remerge-diff (2021-08-31) 7 commits
>  - doc/diff-options: explain the new --remerge-diff option
>  - show, log: provide a --remerge-diff capability
>  - tmp-objdir: new API for creating and removing primary object dirs
>  - merge-ort: capture and print ll-merge warnings in our preferred fashion
>  - ll-merge: add API for capturing warnings in a strbuf instead of stderr
>  - merge-ort: add ability to record conflict messages in a file
>  - merge-ort: mark a few more conflict messages as omittable
>
>  A new presentation for two-parent merge "--remerge-diff" can be
>  used to show the difference between mechanical (and possibly
>  conflicted) merge results and the recorded resolution.
>
>  Will merge to 'next'?

It has been a month that it's been cooking with no issues brought up,
and it's been in production for nearly a year...

But just this morning I pinged peff and jrnieder if they might have
time to respectively look at the tmp-objdir stuff (patch 5, plus its
integration into log-tree.c in patch 7) and the ll-merge.[ch] changes
(patch 3).  I don't know if either will have time to do it, but
perhaps wait half a week or so to see if they'll mention they have
time?  Otherwise, yeah, it's probably time to merge this down.

> * en/removing-untracked-fixes (2021-09-27) 12 commits
>  - Documentation: call out commands that nuke untracked files/directories
>  - Comment important codepaths regarding nuking untracked files/dirs
>  - unpack-trees: avoid nuking untracked dir in way of locally deleted file
>  - unpack-trees: avoid nuking untracked dir in way of unmerged file
>  - Change unpack_trees' 'reset' flag into an enum
>  - Remove ignored files by default when they are in the way
>  - unpack-trees: make dir an internal-only struct
>  - unpack-trees: introduce preserve_ignored to unpack_trees_options
>  - read-tree, merge-recursive: overwrite ignored files by default
>  - checkout, read-tree: fix leak of unpack_trees_options.dir
>  - t2500: add various tests for nuking untracked files
>  - Merge branch 'en/am-abort-fix' into en/removing-untracked-fixes
>
>  Various fixes in code paths that move untracked files away to make room.
>
>  Will merge to 'next'?

I just sent out v3 this morning with five new patches (included in
your list above).  While I think my patches are good, and I'd like to
see them merged down to next so I can send my
current-working-directory-deletion fixes that build on top of it, I'm
a little surprised you're proposing to merge this series down this
quickly instead of waiting a little longer for review of the new
patches.  I'm not complaining...but was that intentional?

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: What's cooking in git.git (Sep 2021, #08; Mon, 27)
  2021-09-28  6:46 ` Elijah Newren
@ 2021-09-28  7:45   ` Ævar Arnfjörð Bjarmason
  2021-09-28 17:25     ` Junio C Hamano
  2021-09-28  8:07   ` Ævar Arnfjörð Bjarmason
                     ` (3 subsequent siblings)
  4 siblings, 1 reply; 40+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-09-28  7:45 UTC (permalink / raw)
  To: Elijah Newren
  Cc: Junio C Hamano, Git Mailing List, Jeff King, Jonathan Nieder,
	Neeraj Singh


On Mon, Sep 27 2021, Elijah Newren wrote:

> [Did some slight re-ordering of topics]
>
> On Mon, Sep 27, 2021 at 5:53 PM Junio C Hamano <gitster@pobox.com> wrote:
> [...]
>> * en/remerge-diff (2021-08-31) 7 commits
>>  - doc/diff-options: explain the new --remerge-diff option
>>  - show, log: provide a --remerge-diff capability
>>  - tmp-objdir: new API for creating and removing primary object dirs
>>  - merge-ort: capture and print ll-merge warnings in our preferred fashion
>>  - ll-merge: add API for capturing warnings in a strbuf instead of stderr
>>  - merge-ort: add ability to record conflict messages in a file
>>  - merge-ort: mark a few more conflict messages as omittable
>>
>>  A new presentation for two-parent merge "--remerge-diff" can be
>>  used to show the difference between mechanical (and possibly
>>  conflicted) merge results and the recorded resolution.
>>
>>  Will merge to 'next'?
>
> It has been a month that it's been cooking with no issues brought up,
> and it's been in production for nearly a year...
>
> But just this morning I pinged peff and jrnieder if they might have
> time to respectively look at the tmp-objdir stuff (patch 5, plus its
> integration into log-tree.c in patch 7) and the ll-merge.[ch] changes
> (patch 3).  I don't know if either will have time to do it, but
> perhaps wait half a week or so to see if they'll mention they have
> time?  Otherwise, yeah, it's probably time to merge this down.

I haven't poked at it much, but haven't you and Neeraj Singh (CC'd)
independently come up with two slightly different changes in
tmp-objdir.c to do the same thing? See the tmp-objdir.c part of:

http://lore.kernel.org/git/543ea3569342165363c1602ce36683a54dce7a0b.1632527609.git.gitgitgadget@gmail.com

And your:

http://lore.kernel.org/git/67d3b2b09f9ddda616cdd0d1b12ab7afc73670ed.1630376800.git.gitgitgadget@gmail.com

I.e. yours has the object database managed outside, his has it added to
"struct tmp_objdir", but it's the same objdir dance isn't it?

I started reading the combined code in "seen" and found it quite
confusing until I saw what was going on.

For both, if you can agree on some common API: I'd prefer if the
"%s/incoming-XXXXXX" part of the "tmp_objdir_create()" was passed as
some template, perhaps just the string "incoming" as a prefix?

I.e. this was for receive-pack.c originally, now it's for bulk-checkin.c
and log.c, if either of those segfault or is long-running it's quite
confusing to have something called "incoming" if you're manually
inspecting it.  Perhaps "core-fsyncObjectFiles-batch" (or even
"core.fsyncObjectFiles=batch") and "log-remerge-diff" as prefixes for
the two, and "incoming" for the one existing caller in "master"?

>> * en/removing-untracked-fixes (2021-09-27) 12 commits
>>  - Documentation: call out commands that nuke untracked files/directories
>>  - Comment important codepaths regarding nuking untracked files/dirs
>>  - unpack-trees: avoid nuking untracked dir in way of locally deleted file
>>  - unpack-trees: avoid nuking untracked dir in way of unmerged file
>>  - Change unpack_trees' 'reset' flag into an enum
>>  - Remove ignored files by default when they are in the way
>>  - unpack-trees: make dir an internal-only struct
>>  - unpack-trees: introduce preserve_ignored to unpack_trees_options
>>  - read-tree, merge-recursive: overwrite ignored files by default
>>  - checkout, read-tree: fix leak of unpack_trees_options.dir
>>  - t2500: add various tests for nuking untracked files
>>  - Merge branch 'en/am-abort-fix' into en/removing-untracked-fixes
>>
>>  Various fixes in code paths that move untracked files away to make room.
>>
>>  Will merge to 'next'?
>
> I just sent out v3 this morning with five new patches (included in
> your list above).  While I think my patches are good, and I'd like to
> see them merged down to next so I can send my
> current-working-directory-deletion fixes that build on top of it, I'm
> a little surprised you're proposing to merge this series down this
> quickly instead of waiting a little longer for review of the new
> patches.  I'm not complaining...but was that intentional?


^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: What's cooking in git.git (Sep 2021, #08; Mon, 27)
  2021-09-28  6:46 ` Elijah Newren
  2021-09-28  7:45   ` Ævar Arnfjörð Bjarmason
@ 2021-09-28  8:07   ` Ævar Arnfjörð Bjarmason
  2021-09-28 17:27     ` Junio C Hamano
  2021-09-28 13:31   ` Derrick Stolee
                     ` (2 subsequent siblings)
  4 siblings, 1 reply; 40+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-09-28  8:07 UTC (permalink / raw)
  To: Elijah Newren
  Cc: Junio C Hamano, Git Mailing List, Jeff King, Jonathan Nieder


On Mon, Sep 27 2021, Elijah Newren wrote:

[Aside: I quite liked the method of replying to What's Cooking per-topic
in Johannes's replies to
https://lore.kernel.org/git/xmqqsfyf5b74.fsf@gitster.g/; I'm never quite
sure if I should have one E-Mail with all of my comments on other
in-flight work, use existing commentary as a springboard etc. Doing the
latter here]

>> * js/scalar (2021-09-14) 15 commits
>>  - scalar: accept -C and -c options before the subcommand
>>  - scalar: implement the `version` command
>>  - scalar: implement the `delete` command
>>  - scalar: teach 'reconfigure' to optionally handle all registered enlistments
>>  - scalar: allow reconfiguring an existing enlistment
>>  - scalar: implement the `run` command
>>  - scalar: teach 'clone' to support the --single-branch option
>>  - scalar: implement the `clone` subcommand
>>  - scalar: implement 'scalar list'
>>  - scalar: let 'unregister' handle a deleted enlistment directory gracefully
>>  - scalar: 'unregister' stops background maintenance
>>  - scalar: 'register' sets recommended config and starts maintenance
>>  - scalar: create test infrastructure
>>  - scalar: start documenting the command
>>  - scalar: create a rudimentary executable
>>
>>  Add pieces from "scalar" to contrib/.
>>
>>  Will merge to 'next'?
>
> I feel bad for taking so long to take a look, but I finally responded
> with a few comments.  Mostly, I'm glad it's a contrib thing; a lot of
> the config makes sense to me (even if some of it is 'meh' for my
> repository sizes or setup), but two of the config settings would be
> very objectionable if they were a core git command.  Since it's
> contrib, though, it's probably fine to be very opinionated...and
> perhaps even excessively so.  ;-)
>
> However, since Johannes has been away for a couple weeks, maybe give
> him a chance to return and respond to myself and others (and perhaps
> push any updates that occurred to him while on vacation) before
> merging down?

Since the thread at
https://lore.kernel.org/git/87ilz44kdk.fsf@evledraar.gmail.com/ I've
been running with an altered version of Johannes's patches that does the
Makefile integration differently, per my
https://lore.kernel.org/git/87ilz44kdk.fsf@evledraar.gmail.com/.

I've been trickling in some of the Makefile changes that make that
easier (still quite possible on top of "master", I just had some
cleanups). It builds[1], tests[2], installs[3] and participates in any
auxiliary targets like "check-docs", "sparse"[4] (and now my
"sparse-incr") etc.

AFAIKT Johannes's version is so far just doing [1] and [2], and just
optionally if you "make" in the contrib subdirectory.

Junio seemed positive on that direction in [1][2]. Re your comments in
[3]: Right now "scalar" doesn't define any of its own config, but one
way out of some of your comments seems to be to do that.

If it's using its own build system in contrib/scalar that'll happen how
exactly? We keep the main manpage in contrib/scalar/scalar.txt but have
a Documentation/config/scalar.txt? Have "git help --config" grow some
optional mode for scalar-specific config? Add a "scalar help-config"?

In case my opinion on it isn't painfully obvious at this point I think
the answer for both current & future integration reasons should be to
just have its assets & build logic intergrated with existing locations
and logic.

1. https://lore.kernel.org/git/xmqqilz32hhr.fsf@gitster.g/
2. https://lore.kernel.org/git/xmqq1r5qzv35.fsf@gitster.g/
3. https://lore.kernel.org/git/CABPp-BG_wupp1o5bBSYOJSvF3eJjf=TbX0RBHqqKuD+3F8s6hw@mail.gmail.com/

^ permalink raw reply	[flat|nested] 40+ messages in thread

* da/difftool (was: Re: What's cooking in git.git (Sep 2021, #08; Mon, 27))
  2021-09-28  0:52 What's cooking in git.git (Sep 2021, #08; Mon, 27) Junio C Hamano
  2021-09-28  1:57 ` Ævar Arnfjörð Bjarmason
  2021-09-28  6:46 ` Elijah Newren
@ 2021-09-28  8:22 ` Ævar Arnfjörð Bjarmason
  2021-09-28  8:23 ` ns/batched-fsync & en/remerge-diff (was " Ævar Arnfjörð Bjarmason
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 40+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-09-28  8:22 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git


On Mon, Sep 27 2021, Junio C Hamano wrote:

> * da/difftool (2021-09-23) 4 commits
>  - difftool: add a missing space to the run_dir_diff() comments
>  - difftool: create a tmpdir path without repeated slashes
>  - Merge branch 'da/difftool-dir-diff-symlink-fix' into da/difftool
>  - Merge branch 'ab/retire-option-argument' into da/difftool
>  (this branch uses da/difftool-dir-diff-symlink-fix.)
>
>  Code clean-up in "git difftool".
>
>  Will merge to 'next'?

I think so, I looked it over and left a comment about a weird edge case
in the pre-image to do with negative return codes, but all of the logic
seems OK to me.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* ns/batched-fsync & en/remerge-diff (was Re: What's cooking in git.git (Sep 2021, #08; Mon, 27))
  2021-09-28  0:52 What's cooking in git.git (Sep 2021, #08; Mon, 27) Junio C Hamano
                   ` (2 preceding siblings ...)
  2021-09-28  8:22 ` da/difftool (was: Re: What's cooking in git.git (Sep 2021, #08; Mon, 27)) Ævar Arnfjörð Bjarmason
@ 2021-09-28  8:23 ` Ævar Arnfjörð Bjarmason
  2021-09-28  8:31 ` sg/test-split-index-fix " Ævar Arnfjörð Bjarmason
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 40+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-09-28  8:23 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Neeraj K. Singh, Elijah Newren


On Mon, Sep 27 2021, Junio C Hamano wrote:

> * ns/batched-fsync (2021-09-27) 8 commits
>  - core.fsyncobjectfiles: performance tests for add and stash
>  - core.fsyncobjectfiles: tests for batch mode
>  - unpack-objects: use the bulk-checkin infrastructure
>  - update-index: use the bulk-checkin infrastructure
>  - core.fsyncobjectfiles: add windows support for batch mode
>  - core.fsyncobjectfiles: batched disk flushes
>  - bulk-checkin: rename 'state' variable and separate 'plugged' boolean
>  - object-file.c: do not rename in a temp odb
>
>  The "core.fsyncobjectfiles" configuration variable can now be set
>  to "batch" for improved performance.
>
>  Will merge to 'next'?

This version was significantly re-rolled in response to my feedback, I
haven't had time to look at this latest version in detail, but will try
to do so soon.

I did note in
https://lore.kernel.org/git/87v92lxhh4.fsf@evledraar.gmail.com/ that it
one of Elijah's in-flight seem to have independently come up with
slightly incompatible versions of the same tmp-objdir.c API....

> * en/remerge-diff (2021-08-31) 7 commits
>  - doc/diff-options: explain the new --remerge-diff option
>  - show, log: provide a --remerge-diff capability
>  - tmp-objdir: new API for creating and removing primary object dirs
>  - merge-ort: capture and print ll-merge warnings in our preferred fashion
>  - ll-merge: add API for capturing warnings in a strbuf instead of stderr
>  - merge-ort: add ability to record conflict messages in a file
>  - merge-ort: mark a few more conflict messages as omittable
>
>  A new presentation for two-parent merge "--remerge-diff" can be
>  used to show the difference between mechanical (and possibly
>  conflicted) merge results and the recorded resolution.
>
>  Will merge to 'next'?

...i.e. this series

^ permalink raw reply	[flat|nested] 40+ messages in thread

* sg/test-split-index-fix (was Re: What's cooking in git.git (Sep 2021, #08; Mon, 27))
  2021-09-28  0:52 What's cooking in git.git (Sep 2021, #08; Mon, 27) Junio C Hamano
                   ` (3 preceding siblings ...)
  2021-09-28  8:23 ` ns/batched-fsync & en/remerge-diff (was " Ævar Arnfjörð Bjarmason
@ 2021-09-28  8:31 ` Ævar Arnfjörð Bjarmason
  2021-09-28  8:35 ` hn/reftable (Re: " Ævar Arnfjörð Bjarmason
  2021-09-29  8:12 ` What's cooking in git.git (Sep 2021, #08; Mon, 27) Fabian Stelzer
  6 siblings, 0 replies; 40+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-09-28  8:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git


On Mon, Sep 27 2021, Junio C Hamano wrote:

> * sg/test-split-index-fix (2021-09-07) 7 commits
>  - read-cache: fix GIT_TEST_SPLIT_INDEX
>  - tests: disable GIT_TEST_SPLIT_INDEX for sparse index tests
>  - read-cache: look for shared index files next to the index, too
>  - t1600-index: disable GIT_TEST_SPLIT_INDEX
>  - t1600-index: don't run git commands upstream of a pipe
>  - t1600-index: remove unnecessary redirection
>  - Merge branch 'ds/sparse-index-ignored-files' into sg/test-split-index-fix
>
>  Test updates.
>
>  Will merge to 'next'?

It LGTM. It introduces a "new" memory leak that I re-rolled
ab/sanitize-leak-ci for, but on closer inspection it's a leak that was
there all along, but we didn't see due to the issue the series fixes:
The GIT_TEST_SPLIT_INDEX test mode was broken.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* hn/reftable (Re: What's cooking in git.git (Sep 2021, #08; Mon, 27))
  2021-09-28  0:52 What's cooking in git.git (Sep 2021, #08; Mon, 27) Junio C Hamano
                   ` (4 preceding siblings ...)
  2021-09-28  8:31 ` sg/test-split-index-fix " Ævar Arnfjörð Bjarmason
@ 2021-09-28  8:35 ` Ævar Arnfjörð Bjarmason
  2021-09-28 12:18   ` Han-Wen Nienhuys
  2021-09-29  8:12 ` What's cooking in git.git (Sep 2021, #08; Mon, 27) Fabian Stelzer
  6 siblings, 1 reply; 40+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-09-28  8:35 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Han-Wen Nienhuys, Carlo Marcelo Arenas Belón


On Mon, Sep 27 2021, Junio C Hamano wrote:

> * hn/reftable (2021-09-10) 20 commits
>  - fixup! reftable: implement stack, a mutable database of reftable files.
>  - Add "test-tool dump-reftable" command.
>  - reftable: add dump utility
>  - reftable: implement stack, a mutable database of reftable files.
>  - reftable: implement refname validation
>  - reftable: add merged table view
>  - reftable: add a heap-based priority queue for reftable records
>  - reftable: reftable file level tests
>  - reftable: read reftable files
>  - reftable: generic interface to tables
>  - reftable: write reftable files
>  - reftable: a generic binary tree implementation
>  - reftable: reading/writing blocks
>  - Provide zlib's uncompress2 from compat/zlib-compat.c
>  - reftable: (de)serialization for the polymorphic record type.
>  - reftable: add blocksource, an abstraction for random access reads
>  - reftable: utility functions
>  - reftable: add error related functionality
>  - reftable: RFC: add LICENSE
>  - hash.h: provide constants for the hash IDs
>
>  The "reftable" backend for the refs API, without integrating into
>  the refs subsystem.
>
>  Will merge to 'next'?

I think we've reached approximately "good enough" with this for the next
steps, and can hopefully fix any remaining nits (such as my [1])
post-merge.

Maybe hold off until Han-Wen gets a chance to ack it, and whether he's
ok with the proposed fixup(s)?

Wasn't there an outstanding "some of this in reftable/* should be static
functions" from someone, Carlo? (CC'd). In any case that sort of thing
could also be a post-cleanup, I couldn't find a reference to that
discussion in anything except my vague memory of it as I wrote this.

1. https://lore.kernel.org/git/87wnn62nhp.fsf@evledraar.gmail.com/

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: hn/reftable (Re: What's cooking in git.git (Sep 2021, #08; Mon, 27))
  2021-09-28  8:35 ` hn/reftable (Re: " Ævar Arnfjörð Bjarmason
@ 2021-09-28 12:18   ` Han-Wen Nienhuys
  2021-09-30  5:06     ` Carlo Arenas
  0 siblings, 1 reply; 40+ messages in thread
From: Han-Wen Nienhuys @ 2021-09-28 12:18 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason
  Cc: Junio C Hamano, git, Carlo Marcelo Arenas Belón

On Tue, Sep 28, 2021 at 10:38 AM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> > * hn/reftable (2021-09-10) 20 commits
> >  - fixup! reftable: implement stack, a mutable database of reftable files.
> >  - Add "test-tool dump-reftable" command.
> >  - reftable: add dump utility
> >  - reftable: implement stack, a mutable database of reftable files.
> >  - reftable: implement refname validation
> >  - reftable: add merged table view
> >  - reftable: add a heap-based priority queue for reftable records
> >  - reftable: reftable file level tests
> >  - reftable: read reftable files
> >  - reftable: generic interface to tables
> >  - reftable: write reftable files
> >  - reftable: a generic binary tree implementation
> >  - reftable: reading/writing blocks
> >  - Provide zlib's uncompress2 from compat/zlib-compat.c
> >  - reftable: (de)serialization for the polymorphic record type.
> >  - reftable: add blocksource, an abstraction for random access reads
> >  - reftable: utility functions
> >  - reftable: add error related functionality
> >  - reftable: RFC: add LICENSE
> >  - hash.h: provide constants for the hash IDs
> >
> >  The "reftable" backend for the refs API, without integrating into
> >  the refs subsystem.
> >
> >  Will merge to 'next'?
>
> I think we've reached approximately "good enough" with this for the next
> steps, and can hopefully fix any remaining nits (such as my [1])
> post-merge.

There is still an RFC in 02/19. Maybe we can get agreement that this
is OK and drop the RFC ?

> Maybe hold off until Han-Wen gets a chance to ack it, and whether he's
> ok with the proposed fixup(s)?

regarding uncompress2: I thought it was functionality best left to
zlib to implement, but I imagine git.git offers something similar.
Sounds good to use that.

> Wasn't there an outstanding "some of this in reftable/* should be static
> functions" from someone, Carlo? (CC'd). In any case that sort of thing
> could also be a post-cleanup, I couldn't find a reference to that
> discussion in anything except my vague memory of it as I wrote this.

I think I have addressed all outstanding issues in my github PR, and
I'll send it out once I see CI pass.

-- 
Han-Wen Nienhuys - Google Munich
I work 80%. Don't expect answers from me on Fridays.
--

Google Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich

Registergericht und -nummer: Hamburg, HRB 86891

Sitz der Gesellschaft: Hamburg

Geschäftsführer: Paul Manicle, Halimah DeLaine Prado

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: What's cooking in git.git (Sep 2021, #08; Mon, 27)
  2021-09-28  6:46 ` Elijah Newren
  2021-09-28  7:45   ` Ævar Arnfjörð Bjarmason
  2021-09-28  8:07   ` Ævar Arnfjörð Bjarmason
@ 2021-09-28 13:31   ` Derrick Stolee
  2021-09-28 17:33     ` Junio C Hamano
  2021-09-28 17:16   ` Junio C Hamano
  2021-09-28 23:40   ` Jeff King
  4 siblings, 1 reply; 40+ messages in thread
From: Derrick Stolee @ 2021-09-28 13:31 UTC (permalink / raw)
  To: Elijah Newren, Junio C Hamano
  Cc: Git Mailing List, Jeff King, Jonathan Nieder

On 9/28/2021 2:46 AM, Elijah Newren wrote:
> [Did some slight re-ordering of topics]
> 
> On Mon, Sep 27, 2021 at 5:53 PM Junio C Hamano <gitster@pobox.com> wrote:
> 
>> * ds/add-rm-with-sparse-index (2021-09-24) 13 commits
>>  - advice: update message to suggest '--sparse'
>>  - mv: refuse to move sparse paths
>>  - rm: skip sparse paths with missing SKIP_WORKTREE
>>  - rm: add --sparse option
>>  - add: update --renormalize to skip sparse paths
>>  - add: update --chmod to skip sparse paths
>>  - add: implement the --sparse option
>>  - add: skip tracked paths outside sparse-checkout cone
>>  - add: fail when adding an untracked sparse file
>>  - dir: fix pattern matching on dirs
>>  - dir: select directories correctly
>>  - t1092: behavior for adding sparse files
>>  - t3705: test that 'sparse_entry' is unstaged
>>
>>  "git add", "git mv", and "git rm" have been adjusted to avoid
>>  updating paths outside of the sparse-checkout definition unless
>>  the user specifies a "--sparse" option.
>>
>>  Will merge to 'next'?
> 
> It would be nice to see the --diff-filter=u change, which you also
> seemed to like[1]; but after that, yeah this is ready to merge down.
> 
> [1] https://lore.kernel.org/git/xmqq35pppwsm.fsf@gitster.g/

Yes, I agree. I was waiting to see if more comments trickled in, but
it seems stable now. Do you want me to re-roll the whole series, or
do you want to apply the fixup below?

Thanks,
-Stolee


---- >8 ----

From d279bd580ad3b66187f9a8c0370acc5bca7cc5b6 Mon Sep 17 00:00:00 2001
From: Derrick Stolee <dstolee@microsoft.com>
Date: Tue, 28 Sep 2021 09:30:10 -0400
Subject: [PATCH] fixup! t1092: behavior for adding sparse files

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
 t/t1092-sparse-checkout-compatibility.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh
index a00e42fa233..ca91c6a67f8 100755
--- a/t/t1092-sparse-checkout-compatibility.sh
+++ b/t/t1092-sparse-checkout-compatibility.sh
@@ -192,7 +192,7 @@ test_sparse_unstaged () {
 	for repo in sparse-checkout sparse-index
 	do
 		# Skip "unmerged" paths
-		git -C $repo diff --staged --diff-filter=ACDMRTXB -- "$file" >diff &&
+		git -C $repo diff --staged --diff-filter=u -- "$file" >diff &&
 		test_must_be_empty diff || return 1
 	done
 }
-- 
2.33.0.vfs.0.0



^ permalink raw reply related	[flat|nested] 40+ messages in thread

* Re: What's cooking in git.git (Sep 2021, #08; Mon, 27)
  2021-09-28  6:46 ` Elijah Newren
                     ` (2 preceding siblings ...)
  2021-09-28 13:31   ` Derrick Stolee
@ 2021-09-28 17:16   ` Junio C Hamano
  2021-09-29  6:42     ` Elijah Newren
  2021-09-28 23:40   ` Jeff King
  4 siblings, 1 reply; 40+ messages in thread
From: Junio C Hamano @ 2021-09-28 17:16 UTC (permalink / raw)
  To: Elijah Newren; +Cc: Git Mailing List, Jeff King, Jonathan Nieder

Elijah Newren <newren@gmail.com> writes:

>> * js/scalar (2021-09-14) 15 commits
> ...
> However, since Johannes has been away for a couple weeks, maybe give
> him a chance to return and respond to myself and others (and perhaps
> push any updates that occurred to him while on vacation) before
> merging down?

Fair enough.

>> * en/remerge-diff (2021-08-31) 7 commits
>>  - doc/diff-options: explain the new --remerge-diff option
>>  - show, log: provide a --remerge-diff capability
>>  - tmp-objdir: new API for creating and removing primary object dirs
>>  - merge-ort: capture and print ll-merge warnings in our preferred fashion
>>  - ll-merge: add API for capturing warnings in a strbuf instead of stderr
>>  - merge-ort: add ability to record conflict messages in a file
>>  - merge-ort: mark a few more conflict messages as omittable
>>
>>  A new presentation for two-parent merge "--remerge-diff" can be
>>  used to show the difference between mechanical (and possibly
>>  conflicted) merge results and the recorded resolution.
>>
>>  Will merge to 'next'?
>
> It has been a month that it's been cooking with no issues brought up,
> and it's been in production for nearly a year...

Please do not read that much for being in "seen".  Until a topic
hits 'next', where some orgs package and ship to their internal
audience, I am not sure if it can be called "cooking".

But your using it on your folks in the production (how big is your
audience, I don't know) does count ;-)

> But just this morning I pinged peff and jrnieder if they might have
> time to respectively look at the tmp-objdir stuff (patch 5, plus its
> integration into log-tree.c in patch 7) and the ll-merge.[ch] changes
> (patch 3).  I don't know if either will have time to do it, but
> perhaps wait half a week or so to see if they'll mention they have
> time?

Sure.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: What's cooking in git.git (Sep 2021, #08; Mon, 27)
  2021-09-28  7:45   ` Ævar Arnfjörð Bjarmason
@ 2021-09-28 17:25     ` Junio C Hamano
  2021-09-28 21:00       ` Neeraj Singh
  0 siblings, 1 reply; 40+ messages in thread
From: Junio C Hamano @ 2021-09-28 17:25 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason
  Cc: Elijah Newren, Git Mailing List, Jeff King, Jonathan Nieder,
	Neeraj Singh

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

> I haven't poked at it much, but haven't you and Neeraj Singh (CC'd)
> independently come up with two slightly different changes in
> tmp-objdir.c to do the same thing? See the tmp-objdir.c part of:
>
> http://lore.kernel.org/git/543ea3569342165363c1602ce36683a54dce7a0b.1632527609.git.gitgitgadget@gmail.com
>
> And your:
>
> http://lore.kernel.org/git/67d3b2b09f9ddda616cdd0d1b12ab7afc73670ed.1630376800.git.gitgitgadget@gmail.com
>
> I.e. yours has the object database managed outside, his has it added to
> "struct tmp_objdir", but it's the same objdir dance isn't it?

They touch the same tmp-objdir, but unlike the original use in the
receive-pack (i.e. responding to "git push") with the intention to
add the objects collected in them back to the primary, remerge-diff
wants to discard what was added there at the end.  I do not think it
would directly help the bulk-fsync stuff (but I didn't quite see why
bulk-fsync stuff needed to _add_ new functions to tmp-objdir API,
instead of just being a customer of tmp-objdir API), where it wants
to do the same _migrate() dance in the end.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: What's cooking in git.git (Sep 2021, #08; Mon, 27)
  2021-09-28  8:07   ` Ævar Arnfjörð Bjarmason
@ 2021-09-28 17:27     ` Junio C Hamano
  0 siblings, 0 replies; 40+ messages in thread
From: Junio C Hamano @ 2021-09-28 17:27 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason
  Cc: Elijah Newren, Git Mailing List, Jeff King, Jonathan Nieder

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

> On Mon, Sep 27 2021, Elijah Newren wrote:
>
> [Aside: I quite liked the method of replying to What's Cooking per-topic
> in Johannes's replies to
> https://lore.kernel.org/git/xmqqsfyf5b74.fsf@gitster.g/; I'm never quite
> sure if I should have one E-Mail with all of my comments on other
> in-flight work, use existing commentary as a springboard etc. Doing the
> latter here]

Yup.  Splitting them out may be more work to produce comments, but
it lets you Cc relevant folks and would be easier for those who
participate in the discussion.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: What's cooking in git.git (Sep 2021, #08; Mon, 27)
  2021-09-28 13:31   ` Derrick Stolee
@ 2021-09-28 17:33     ` Junio C Hamano
  2021-09-28 20:16       ` Derrick Stolee
  0 siblings, 1 reply; 40+ messages in thread
From: Junio C Hamano @ 2021-09-28 17:33 UTC (permalink / raw)
  To: Derrick Stolee
  Cc: Elijah Newren, Git Mailing List, Jeff King, Jonathan Nieder

Derrick Stolee <stolee@gmail.com> writes:

> Yes, I agree. I was waiting to see if more comments trickled in, but
> it seems stable now. Do you want me to re-roll the whole series, or
> do you want to apply the fixup below?

Let us wait for a bit more trickling in, but in the meantime, here
is what I'd keep.  Thanks.

[ds/add-rm-with-sparse-index] $ git range-diff @{1}...
 1:  f1309dd7be !  1:  edd2cd345f t1092: behavior for adding sparse files
    @@ t/t1092-sparse-checkout-compatibility.sh: test_sparse_match () {
     +	for repo in sparse-checkout sparse-index
     +	do
     +		# Skip "unmerged" paths
    -+		git -C $repo diff --staged --diff-filter=ACDMRTXB -- "$file" >diff &&
    ++		git -C $repo diff --staged --diff-filter=u -- "$file" >diff &&
     +		test_must_be_empty diff || return 1
     +	done
     +}
 2:  9a8c340809 =  2:  f6526728f9 dir: select directories correctly
 3:  2a4e495cd4 =  3:  ed4958477b dir: fix pattern matching on dirs
 ...


^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: What's cooking in git.git (Sep 2021, #08; Mon, 27)
  2021-09-28 17:33     ` Junio C Hamano
@ 2021-09-28 20:16       ` Derrick Stolee
  0 siblings, 0 replies; 40+ messages in thread
From: Derrick Stolee @ 2021-09-28 20:16 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Elijah Newren, Git Mailing List, Jeff King, Jonathan Nieder

On 9/28/2021 1:33 PM, Junio C Hamano wrote:
> Derrick Stolee <stolee@gmail.com> writes:
> 
>> Yes, I agree. I was waiting to see if more comments trickled in, but
>> it seems stable now. Do you want me to re-roll the whole series, or
>> do you want to apply the fixup below?
> 
> Let us wait for a bit more trickling in, but in the meantime, here
> is what I'd keep.  Thanks.
> 
> [ds/add-rm-with-sparse-index] $ git range-diff @{1}...
>  1:  f1309dd7be !  1:  edd2cd345f t1092: behavior for adding sparse files
>     @@ t/t1092-sparse-checkout-compatibility.sh: test_sparse_match () {
>      +	for repo in sparse-checkout sparse-index
>      +	do
>      +		# Skip "unmerged" paths
>     -+		git -C $repo diff --staged --diff-filter=ACDMRTXB -- "$file" >diff &&
>     ++		git -C $repo diff --staged --diff-filter=u -- "$file" >diff &&
>      +		test_must_be_empty diff || return 1
>      +	done
>      +}
>  2:  9a8c340809 =  2:  f6526728f9 dir: select directories correctly
>  3:  2a4e495cd4 =  3:  ed4958477b dir: fix pattern matching on dirs

That's exactly the diff I was hoping for. I'll keep an eye for more
feedback.

Thanks,
-Stolee

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: What's cooking in git.git (Sep 2021, #08; Mon, 27)
  2021-09-28  1:57 ` Ævar Arnfjörð Bjarmason
@ 2021-09-28 20:52   ` Junio C Hamano
  0 siblings, 0 replies; 40+ messages in thread
From: Junio C Hamano @ 2021-09-28 20:52 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason
  Cc: git, René Scharfe, Jeff King,
	Carlo Marcelo Arenas Belón, SZEDER Gábor, Taylor Blau

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

>> * ab/http-pinned-public-key-mismatch (2021-09-27) 1 commit
>>  - http: check CURLE_SSL_PINNEDPUBKEYNOTMATCH when emitting errors
>>
>>  HTTPS error handling updates.
>>
>>  Will merge to 'next'?
>
> I'd like it merged. I think a fair summary of Jeff King's
> https://lore.kernel.org/git/YU5CJC9XJvQITfr8@coredump.intra.peff.net/ is
> "meh" though :)

Mine is also "I do not mind seeing it in or out."

>> * ab/pack-objects-stdin (2021-07-09) 5 commits
>>  - pack-objects.c: make use of REV_INFO_STDIN_LINE_PROCESS
>>  - pack-objects.c: do stdin parsing via revision.c's API
>>  - revision.[ch]: add a "handle_stdin_line" API
>>  - revision.h: refactor "disable_stdin" and "read_from_stdin"
>>  - upload-pack: run is_repository_shallow() before setup_revisions()
>>
>>  Introduce handle_stdin_line callback to revision API and uses it.
>>
>>  Expecting a reroll.
>
> I said I'd re-roll this into the larger "git bundle --stdin" topic I had
> in mind, but when doing so found one memory leak, which I really wanted
> the SANITIZE=leak interface to fix.
>
> I think it's best to eject/discard this given the amount of other
> outstanding stuff I've got. I don't really *need* this for anything
> else, I'll loop back to it sometime after the SANITIZE=leak testing has
> landed.

OK, will mark as "Retracted for now".

>> * ab/fsck-unexpected-type (2021-09-22) 17 commits
>> [...]
>>  "git fsck" has been taught to report mismatch between expected and
>>  actual types of an object better.
>>
>>  Needs review.
>
> The "needs review" note here pre-dates v6, which got a thorough review,
> and Taylor seemed happy with the v7, aside from a minor issue I just
> re-rolled v8 with a fix for:
>
> https://lore.kernel.org/git/cover-v8-00.17-00000000000-20210928T021616Z-avarab@gmail.com/

Thanks.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: What's cooking in git.git (Sep 2021, #08; Mon, 27)
  2021-09-28 17:25     ` Junio C Hamano
@ 2021-09-28 21:00       ` Neeraj Singh
  2021-09-28 23:34         ` Junio C Hamano
  0 siblings, 1 reply; 40+ messages in thread
From: Neeraj Singh @ 2021-09-28 21:00 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Ævar Arnfjörð Bjarmason, Elijah Newren,
	Git Mailing List, Jeff King, Jonathan Nieder, Neeraj Singh

On Tue, Sep 28, 2021 at 10:25:08AM -0700, Junio C Hamano wrote:
> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> 
> > I haven't poked at it much, but haven't you and Neeraj Singh (CC'd)
> > independently come up with two slightly different changes in
> > tmp-objdir.c to do the same thing? See the tmp-objdir.c part of:
> >
> > http://lore.kernel.org/git/543ea3569342165363c1602ce36683a54dce7a0b.1632527609.git.gitgitgadget@gmail.com
> >
> > And your:
> >
> > http://lore.kernel.org/git/67d3b2b09f9ddda616cdd0d1b12ab7afc73670ed.1630376800.git.gitgitgadget@gmail.com
> >
> > I.e. yours has the object database managed outside, his has it added to
> > "struct tmp_objdir", but it's the same objdir dance isn't it?
> 
> They touch the same tmp-objdir, but unlike the original use in the
> receive-pack (i.e. responding to "git push") with the intention to
> add the objects collected in them back to the primary, remerge-diff
> wants to discard what was added there at the end.  I do not think it
> would directly help the bulk-fsync stuff (but I didn't quite see why
> bulk-fsync stuff needed to _add_ new functions to tmp-objdir API,
> instead of just being a customer of tmp-objdir API), where it wants
> to do the same _migrate() dance in the end.

Both Elijah and I needed the same functionality of having a writable
ODB in the current process, which wasn't previously provided by tmp-objdir.

I'm making a new patch which is an amalgamation of Elijah's version and mine.
I'll rebase and resend my patch series with that version and I'll also make
a modified version of Elijah's branch available on github.

Thanks,
Neeraj

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: What's cooking in git.git (Sep 2021, #08; Mon, 27)
  2021-09-28 21:00       ` Neeraj Singh
@ 2021-09-28 23:34         ` Junio C Hamano
  2021-09-28 23:53           ` Neeraj Singh
  0 siblings, 1 reply; 40+ messages in thread
From: Junio C Hamano @ 2021-09-28 23:34 UTC (permalink / raw)
  To: Neeraj Singh
  Cc: Ævar Arnfjörð Bjarmason, Elijah Newren,
	Git Mailing List, Jeff King, Jonathan Nieder, Neeraj Singh

Neeraj Singh <nksingh85@gmail.com> writes:

> Both Elijah and I needed the same functionality of having a writable
> ODB in the current process, which wasn't previously provided by tmp-objdir.
>
> I'm making a new patch which is an amalgamation of Elijah's version and mine.
> I'll rebase and resend my patch series with that version and I'll also make
> a modified version of Elijah's branch available on github.

Sounds good.  Thanks.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: What's cooking in git.git (Sep 2021, #08; Mon, 27)
  2021-09-28  6:46 ` Elijah Newren
                     ` (3 preceding siblings ...)
  2021-09-28 17:16   ` Junio C Hamano
@ 2021-09-28 23:40   ` Jeff King
  2021-09-28 23:49     ` Jeff King
                       ` (2 more replies)
  4 siblings, 3 replies; 40+ messages in thread
From: Jeff King @ 2021-09-28 23:40 UTC (permalink / raw)
  To: Elijah Newren; +Cc: Junio C Hamano, Git Mailing List, Jonathan Nieder

On Mon, Sep 27, 2021 at 11:46:40PM -0700, Elijah Newren wrote:

> > * en/remerge-diff (2021-08-31) 7 commits
> >  - doc/diff-options: explain the new --remerge-diff option
> >  - show, log: provide a --remerge-diff capability
> >  - tmp-objdir: new API for creating and removing primary object dirs
> >  - merge-ort: capture and print ll-merge warnings in our preferred fashion
> >  - ll-merge: add API for capturing warnings in a strbuf instead of stderr
> >  - merge-ort: add ability to record conflict messages in a file
> >  - merge-ort: mark a few more conflict messages as omittable
> >
> >  A new presentation for two-parent merge "--remerge-diff" can be
> >  used to show the difference between mechanical (and possibly
> >  conflicted) merge results and the recorded resolution.
> >
> >  Will merge to 'next'?
> 
> It has been a month that it's been cooking with no issues brought up,
> and it's been in production for nearly a year...
> 
> But just this morning I pinged peff and jrnieder if they might have
> time to respectively look at the tmp-objdir stuff (patch 5, plus its
> integration into log-tree.c in patch 7) and the ll-merge.[ch] changes
> (patch 3).  I don't know if either will have time to do it, but
> perhaps wait half a week or so to see if they'll mention they have
> time?  Otherwise, yeah, it's probably time to merge this down.

Sorry to take so long. I think this is a very exciting topic, and I
appreciate being called into to look at tmp-objdir stuff, because it can
be quite subtle.

I just left a rather long-ish mail in the thread, but the gist of it is
that I'm worried that there's some possibility of corruption there if
the diff code writes objects. I didn't do a proof-of-concept there, but
I worked one up just now. Try this:

  # make a repo
  git init repo
  cd repo
  echo base >file
  git add file
  git commit -m base

  # two diverging branches
  echo main >file
  git commit -am main
  git checkout -b side HEAD^
  echo side >file
  git commit -am side

  # we get a conflict, and we resolve
  git merge main
  echo resolved >file
  git commit -am merged

  # now imagine we had a file with a diff driver. I stuffed it
  # in here after the fact, but it could have been here all along,
  # or come as part of the merge, etc.
  echo whatever >unrelated
  echo "unrelated diff=foo" >.gitattributes
  git add .
  git commit --amend --no-edit

  # set up the diff driver not just to do a textconv, but to cache the
  # result. This will require writing out new objects for the cache
  # as part of the diff operation.
  git config diff.foo.textconv "$PWD/upcase"
  git config diff.foo.cachetextconv true
  cat >upcase <<\EOF &&
  #!/bin/sh
  tr a-z A-Z <$1
  EOF
  chmod +x upcase

  # now show the diff
  git log -1 --remerge-diff

  # and make sure the repo is still OK
  git fsck

The remerge diff will correctly show the textconv'd content (because
it's not in either parent, and hence an evil merge):

  diff --git a/unrelated b/unrelated
  new file mode 100644
  index 0000000..982793c
  --- /dev/null
  +++ b/unrelated
  @@ -0,0 +1 @@
  +WHATEVER

but then fsck shows that our cache is corrupted:

  Checking object directories: 100% (256/256), done.
  error: refs/notes/textconv/foo: invalid sha1 pointer 1e9b3ecffca73411001043fe914a7ec0e7291043
  error: refs/notes/textconv/foo: invalid reflog entry 1e9b3ecffca73411001043fe914a7ec0e7291043
  dangling tree 569b6e414203d2f50bdaafc1585eae11e28afc37

Now I'll admit the textconv-cache is a pretty seldom-used feature. And
that there _probably_ aren't a lot of other ways that the diff code
would try to write objects or references. But I think it speaks to the
kind of subtle interactions we should worry about (and certainly
corrupting the repository is a pretty bad outcome, though at least in
this case it's "just" a cache and we could blow away that ref manually).

-Peff

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: What's cooking in git.git (Sep 2021, #08; Mon, 27)
  2021-09-28 23:40   ` Jeff King
@ 2021-09-28 23:49     ` Jeff King
  2021-09-29 18:43     ` Neeraj Singh
  2021-10-04 13:45     ` Elijah Newren
  2 siblings, 0 replies; 40+ messages in thread
From: Jeff King @ 2021-09-28 23:49 UTC (permalink / raw)
  To: Elijah Newren
  Cc: Neeraj Singh, Junio C Hamano, Git Mailing List, Jonathan Nieder

On Tue, Sep 28, 2021 at 07:40:14PM -0400, Jeff King wrote:

> but then fsck shows that our cache is corrupted:
> 
>   Checking object directories: 100% (256/256), done.
>   error: refs/notes/textconv/foo: invalid sha1 pointer 1e9b3ecffca73411001043fe914a7ec0e7291043
>   error: refs/notes/textconv/foo: invalid reflog entry 1e9b3ecffca73411001043fe914a7ec0e7291043
>   dangling tree 569b6e414203d2f50bdaafc1585eae11e28afc37
> 
> Now I'll admit the textconv-cache is a pretty seldom-used feature. And
> that there _probably_ aren't a lot of other ways that the diff code
> would try to write objects or references. But I think it speaks to the
> kind of subtle interactions we should worry about (and certainly
> corrupting the repository is a pretty bad outcome, though at least in
> this case it's "just" a cache and we could blow away that ref manually).

I haven't looked at the way the bulk-fsync code uses tmp_objdir at all
yet, but this is the same kind of thing to be watching out for. I've
added Neeraj to the cc. The full reproduction showing the problem with
Elijah's patch is in the replied-to mail:

  https://lore.kernel.org/git/YVOn3hDsb5pnxR53@coredump.intra.peff.net/

The batch-fsync stuff might be OK in that the goal there is presumably
not to throw away the result, but to eventually migrate it into the odb.
So the unintended interaction to look out for there might be more like:

  - some code foo() wants to write a bunch of objects; it opens up a
    tmp_objdir to batch them

  - some other code bar() deep in the call-stack wants to write an
    object for whatever reason (maybe it's a caching textconv). Its
    object ends up in the tmp_objdir, too.

  - bar() writes a ref pointing to the object's oid, thinking it has
    fully written the object.

  - foo() encounters an error and aborts, rolling back the tmp_objdir,
    including the object written by bar(). Now our ref is corrupt.

As I said, I haven't looked at how the bulk-fsync stuff uses
tmp_objdir. But if it's doing the same "globally all object writes go to
the tmpdir" then it's at risk of this sort of thing.

-Peff

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: What's cooking in git.git (Sep 2021, #08; Mon, 27)
  2021-09-28 23:34         ` Junio C Hamano
@ 2021-09-28 23:53           ` Neeraj Singh
  2021-10-07 22:01             ` Junio C Hamano
  0 siblings, 1 reply; 40+ messages in thread
From: Neeraj Singh @ 2021-09-28 23:53 UTC (permalink / raw)
  To: Junio C Hamano, Elijah Newren
  Cc: Ævar Arnfjörð Bjarmason, Git Mailing List,
	Jeff King, Jonathan Nieder, Neeraj Singh

On Tue, Sep 28, 2021 at 4:34 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Neeraj Singh <nksingh85@gmail.com> writes:
>
> > Both Elijah and I needed the same functionality of having a writable
> > ODB in the current process, which wasn't previously provided by tmp-objdir.
> >
> > I'm making a new patch which is an amalgamation of Elijah's version and mine.
> > I'll rebase and resend my patch series with that version and I'll also make
> > a modified version of Elijah's branch available on github.
>
> Sounds good.  Thanks.

I just posted the new series to the list.

Elijah,

Here's a branch of your changes based on the amalgamated tmp-objdir code:
https://github.com/neerajsi-msft/git/commits/neerajsi/remerge-diff

This commit adapts your code to use the amalgamated API:
https://github.com/neerajsi-msft/git/commit/725328fe1d8be8326d2ddef78e164ca21450b100

Thanks,
Neeraj

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: What's cooking in git.git (Sep 2021, #08; Mon, 27)
  2021-09-28 17:16   ` Junio C Hamano
@ 2021-09-29  6:42     ` Elijah Newren
  0 siblings, 0 replies; 40+ messages in thread
From: Elijah Newren @ 2021-09-29  6:42 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List, Jeff King, Jonathan Nieder

On Tue, Sep 28, 2021 at 10:16 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> Elijah Newren <newren@gmail.com> writes:
>
> >> * js/scalar (2021-09-14) 15 commits
> > ...
> > However, since Johannes has been away for a couple weeks, maybe give
> > him a chance to return and respond to myself and others (and perhaps
> > push any updates that occurred to him while on vacation) before
> > merging down?
>
> Fair enough.
>
> >> * en/remerge-diff (2021-08-31) 7 commits
> >>  - doc/diff-options: explain the new --remerge-diff option
> >>  - show, log: provide a --remerge-diff capability
> >>  - tmp-objdir: new API for creating and removing primary object dirs
> >>  - merge-ort: capture and print ll-merge warnings in our preferred fashion
> >>  - ll-merge: add API for capturing warnings in a strbuf instead of stderr
> >>  - merge-ort: add ability to record conflict messages in a file
> >>  - merge-ort: mark a few more conflict messages as omittable
> >>
> >>  A new presentation for two-parent merge "--remerge-diff" can be
> >>  used to show the difference between mechanical (and possibly
> >>  conflicted) merge results and the recorded resolution.
> >>
> >>  Will merge to 'next'?
> >
> > It has been a month that it's been cooking with no issues brought up,
> > and it's been in production for nearly a year...
>
> Please do not read that much for being in "seen".  Until a topic
> hits 'next', where some orgs package and ship to their internal
> audience, I am not sure if it can be called "cooking".

As a heads up, I found this topic in an email titled "What's
Cooking...", and in particular it was listed in a section of that
email that was titled "[Cooking]".  ;-)

Sorry, I couldn't resist.

But yeah, I know "seen" doesn't mean much.  My comment was reflecting
that while leaving a topic in seen for a while sometimes results in
more information coming forth to inform the decision about whether to
merge the series to next, or drop it, or ask the submitter for
changes, there comes a point where further waiting is unlikely to
provide any additional information that would help inform that
decision.  I was starting to wonder whether we'd get any (more)
feedback.  But I'm glad we waited a little longer.  :-)

> But your using it on your folks in the production (how big is your
> audience, I don't know) does count ;-)

~100 opt-in users (apparently shot up in the past few months; used to
be ~50).  I have a hard time gauging how heavily this particular
feature was/is used by them, however:

  * The feature was explicitly announced, but I did it because I
thought it was cool and a nice demonstration of merge-ort features,
not because there were explicit requests for it
  * I did make "-p" imply "--remerge-diff" for git-log in our internal
opt-in distribution, so that should increase usage significantly.
  * A bug in an internal tool for a while was accidentally running
`git log -p --name-status $COMMIT --not
--remotes=ACCIDENTALLY_INVALID_REMOTE_NAME` for a while, including in
big repos; in combination with previous point, this increased usage
and scope of remerge-diff usage a fair amount as that command would
run on all commits in history.
  * Most merges here are done by either GitHub (Enterprise) or Gerrit,
both of which require the user to rebase and re-push if there are any
conflicts.  This makes remerge-diff boring for most merges.  There are
some notable exceptions where merges are done differently, they're
just much less common.  (--remerge-diff-only for cherry-picks and
reverts is a bit more interesting, but that's not part of this
series...and I also haven't gotten much feedback on it.)
  * I have only received feedback from two users about the
remerge-diff feature, one only indirectly, and both reports actually
ended up being about underlying bugs in early versions of merge-ort[+]

[+] The bugs were (1) my QSORT in write_tree() was not originally
using tree_entry_order() and thus was writing trees with the wrong
ordering of elements; this was a bug that was fixed before
write_tree() was ever submitted upstream, and (2) and the bug fixed by
72b3091040f8 (merge-ort: use STABLE_QSORT instead of QSORT where
required, 2021-03-20)

> > But just this morning I pinged peff and jrnieder if they might have
> > time to respectively look at the tmp-objdir stuff (patch 5, plus its
> > integration into log-tree.c in patch 7) and the ll-merge.[ch] changes
> > (patch 3).  I don't know if either will have time to do it, but
> > perhaps wait half a week or so to see if they'll mention they have
> > time?
>
> Sure.

Apparently that was a good idea; Peff and Ævar have both chimed in
with feedback.  :-)

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: What's cooking in git.git (Sep 2021, #08; Mon, 27)
  2021-09-28  0:52 What's cooking in git.git (Sep 2021, #08; Mon, 27) Junio C Hamano
                   ` (5 preceding siblings ...)
  2021-09-28  8:35 ` hn/reftable (Re: " Ævar Arnfjörð Bjarmason
@ 2021-09-29  8:12 ` Fabian Stelzer
  2021-09-30 21:26   ` Junio C Hamano
  6 siblings, 1 reply; 40+ messages in thread
From: Fabian Stelzer @ 2021-09-29  8:12 UTC (permalink / raw)
  To: Junio C Hamano, git

On 28.09.21 02:52, Junio C Hamano wrote > * fs/ssh-signing (2021-09-10)
9 commits
>  - ssh signing: test that gpg fails for unknown keys
>  - ssh signing: tests for logs, tags & push certs
>  - ssh signing: duplicate t7510 tests for commits
>  - ssh signing: verify signatures using ssh-keygen
>  - ssh signing: provide a textual signing_key_id
>  - ssh signing: retrieve a default key from ssh-agent
>  - ssh signing: add ssh key format and signing code
>  - ssh signing: add test prereqs
>  - ssh signing: preliminary refactoring and clean-up
> 
>  Use ssh public crypto for object and push-cert signing.
> 
>  On hold.
>  cf. <pull.1041.v8.git.git.1631304462.gitgitgadget@gmail.com>
>  cf. <532d97e7-8c91-df6a-6d90-70668256f513@gigacodes.de>
> 
>
Openssh 8.8 has been released a few days ago and includes the needed fix
for the find-principal segfault.
I ran the full git testsuite against it without issues.

Also, we (~30developers) have been running this patch with
openssh-portable (2d678c5e3bdc2f5c99f7af5122e9d054925d560d / post 8.7 -
pre 8.8) in our organization for the last 2 weeks without problems.

The only issues we saw with our users are related to some misleading
openssh error messages.
For example if you configure a public key and the private key is not
available via the ssh-agent the error message is: "invalid format".
Or if the public key contains a typo (forgot a char in copy&pase) it
will error with "no such file or directory".
I will need to dig a bit deeper into openssh to see if we can make these
more specific without breaking any compatibility. Both errors originate
from some lower level lib functions which i don't want to change.

But vverall i think this is ready for some broader usage/testing via next.

I'd suggest to send the additional patches for valid-before/after
functionality in a new patchset for review after.

Best regards,
Fabian

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: What's cooking in git.git (Sep 2021, #08; Mon, 27)
  2021-09-28 23:40   ` Jeff King
  2021-09-28 23:49     ` Jeff King
@ 2021-09-29 18:43     ` Neeraj Singh
  2021-09-30  8:16       ` Jeff King
  2021-10-04 13:45     ` Elijah Newren
  2 siblings, 1 reply; 40+ messages in thread
From: Neeraj Singh @ 2021-09-29 18:43 UTC (permalink / raw)
  To: Jeff King
  Cc: Elijah Newren, Junio C Hamano, Git Mailing List, Jonathan Nieder

On Tue, Sep 28, 2021 at 07:40:14PM -0400, Jeff King wrote:
> On Mon, Sep 27, 2021 at 11:46:40PM -0700, Elijah Newren wrote:
> 
> > > * en/remerge-diff (2021-08-31) 7 commits
> > >  - doc/diff-options: explain the new --remerge-diff option
> > >  - show, log: provide a --remerge-diff capability
> > >  - tmp-objdir: new API for creating and removing primary object dirs
> > >  - merge-ort: capture and print ll-merge warnings in our preferred fashion
> > >  - ll-merge: add API for capturing warnings in a strbuf instead of stderr
> > >  - merge-ort: add ability to record conflict messages in a file
> > >  - merge-ort: mark a few more conflict messages as omittable
> > >
> > >  A new presentation for two-parent merge "--remerge-diff" can be
> > >  used to show the difference between mechanical (and possibly
> > >  conflicted) merge results and the recorded resolution.
> > >
> > >  Will merge to 'next'?
> > 
> > It has been a month that it's been cooking with no issues brought up,
> > and it's been in production for nearly a year...
> > 
> > But just this morning I pinged peff and jrnieder if they might have
> > time to respectively look at the tmp-objdir stuff (patch 5, plus its
> > integration into log-tree.c in patch 7) and the ll-merge.[ch] changes
> > (patch 3).  I don't know if either will have time to do it, but
> > perhaps wait half a week or so to see if they'll mention they have
> > time?  Otherwise, yeah, it's probably time to merge this down.
> 
> Sorry to take so long. I think this is a very exciting topic, and I
> appreciate being called into to look at tmp-objdir stuff, because it can
> be quite subtle.
> 
> I just left a rather long-ish mail in the thread, but the gist of it is
> that I'm worried that there's some possibility of corruption there if
> the diff code writes objects. I didn't do a proof-of-concept there, but
> I worked one up just now. Try this:
> 
>   # make a repo
>   git init repo
>   cd repo
>   echo base >file
>   git add file
>   git commit -m base
> 
>   # two diverging branches
>   echo main >file
>   git commit -am main
>   git checkout -b side HEAD^
>   echo side >file
>   git commit -am side
> 
>   # we get a conflict, and we resolve
>   git merge main
>   echo resolved >file
>   git commit -am merged
> 
>   # now imagine we had a file with a diff driver. I stuffed it
>   # in here after the fact, but it could have been here all along,
>   # or come as part of the merge, etc.
>   echo whatever >unrelated
>   echo "unrelated diff=foo" >.gitattributes
>   git add .
>   git commit --amend --no-edit
> 
>   # set up the diff driver not just to do a textconv, but to cache the
>   # result. This will require writing out new objects for the cache
>   # as part of the diff operation.
>   git config diff.foo.textconv "$PWD/upcase"
>   git config diff.foo.cachetextconv true
>   cat >upcase <<\EOF &&
>   #!/bin/sh
>   tr a-z A-Z <$1
>   EOF
>   chmod +x upcase
> 
>   # now show the diff
>   git log -1 --remerge-diff
> 
>   # and make sure the repo is still OK
>   git fsck
> 
> The remerge diff will correctly show the textconv'd content (because
> it's not in either parent, and hence an evil merge):
> 
>   diff --git a/unrelated b/unrelated
>   new file mode 100644
>   index 0000000..982793c
>   --- /dev/null
>   +++ b/unrelated
>   @@ -0,0 +1 @@
>   +WHATEVER
> 
> but then fsck shows that our cache is corrupted:
> 
>   Checking object directories: 100% (256/256), done.
>   error: refs/notes/textconv/foo: invalid sha1 pointer 1e9b3ecffca73411001043fe914a7ec0e7291043
>   error: refs/notes/textconv/foo: invalid reflog entry 1e9b3ecffca73411001043fe914a7ec0e7291043
>   dangling tree 569b6e414203d2f50bdaafc1585eae11e28afc37
> 
> Now I'll admit the textconv-cache is a pretty seldom-used feature. And
> that there _probably_ aren't a lot of other ways that the diff code
> would try to write objects or references. But I think it speaks to the
> kind of subtle interactions we should worry about (and certainly
> corrupting the repository is a pretty bad outcome, though at least in
> this case it's "just" a cache and we could blow away that ref manually).
> 
> -Peff

It seems to me that one problem is that the new "primary" objdir code doesn't
disable ref updates since the GIT_QUARANTINE_ENVIRONMENT setting isn't set.
If we fix that we should be forbidding ref updates.

I've included a path that fixes my test case. This is on top of:
https://lore.kernel.org/git/CANQDOddqwVtWfC4eEP3fJB4sUiszGX8bLqoEVLcMf=v+jzx19g@mail.gmail.com/

From 38be7f6d31da8df3f205b35d25dd0a505aa75a8a Mon Sep 17 00:00:00 2001
From: Neeraj Singh <neerajsi@microsoft.com>
Date: Wed, 29 Sep 2021 11:24:05 -0700
Subject: [PATCH] tmp-objdir: disable ref updates when replacing the primary
 odb

When creating a subprocess with a temporary ODB, we set the
GIT_QUARANTINE_ENVIRONMENT env var to tell child Git processes not
to update refs, since the tmp-objdir may go away.

Introduce a similar mechanism for in-process temporary ODBs when
we call tmp_objdir_replace_primary_odb.

Peff's test case was invoking ref updates via the cachetextconv
setting. That particular code silently does nothing when a ref
update is forbidden

Reported-by: Jeff King <peff@peff.net>

Signed-off-by: Neeraj Singh <neerajsi@microsoft.com>
---
 object-file.c |  7 +++++++
 refs.c        | 22 +++++++++++++++++++++-
 refs.h        |  5 +++++
 3 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/object-file.c b/object-file.c
index 990381abee..e3e5cec3c8 100644
--- a/object-file.c
+++ b/object-file.c
@@ -770,6 +770,12 @@ struct object_directory *set_temporary_primary_odb(const char *dir, int will_des
 	new_odb->will_destroy = will_destroy;
 	new_odb->next = the_repository->objects->odb;
 	the_repository->objects->odb = new_odb;
+
+	/*
+	 * Disable ref updates while a temporary odb is active, since
+	 * the objects in the database may roll back.
+	 */
+	refs_disable_updates();
 	return new_odb->next;
 }
 
@@ -786,6 +792,7 @@ void restore_primary_odb(struct object_directory *restore_odb, const char *old_p
 
 	the_repository->objects->odb = restore_odb;
 	free_object_directory(cur_odb);
+	refs_enable_updates();
 }
 
 /*
diff --git a/refs.c b/refs.c
index 8b9f7c3a80..98026b7341 100644
--- a/refs.c
+++ b/refs.c
@@ -19,6 +19,8 @@
 #include "repository.h"
 #include "sigchain.h"
 
+static int ref_update_disabled_count;
+
 /*
  * List of all available backends
  */
@@ -1045,6 +1047,24 @@ void ref_transaction_free(struct ref_transaction *transaction)
 	free(transaction);
 }
 
+void refs_disable_updates(void)
+{
+	++ref_update_disabled_count;
+}
+
+void refs_enable_updates(void)
+{
+	if (!ref_update_disabled_count)
+		BUG("ref updates are not disabled");
+
+	--ref_update_disabled_count;
+}
+
+int ref_updates_are_enabled(void)
+{
+	return !ref_update_disabled_count;
+}
+
 struct ref_update *ref_transaction_add_update(
 		struct ref_transaction *transaction,
 		const char *refname, unsigned int flags,
@@ -2126,7 +2146,7 @@ int ref_transaction_prepare(struct ref_transaction *transaction,
 		break;
 	}
 
-	if (getenv(GIT_QUARANTINE_ENVIRONMENT)) {
+	if (getenv(GIT_QUARANTINE_ENVIRONMENT) || !ref_updates_are_enabled()) {
 		strbuf_addstr(err,
 			      _("ref updates forbidden inside quarantine environment"));
 		return -1;
diff --git a/refs.h b/refs.h
index 48970dfc7e..acd7e275c5 100644
--- a/refs.h
+++ b/refs.h
@@ -840,6 +840,11 @@ int ref_storage_backend_exists(const char *name);
 
 struct ref_store *get_main_ref_store(struct repository *r);
 
+/* Helpers to mark updates to the refs as forbidden */
+void refs_disable_updates(void);
+void refs_enable_updates(void);
+int ref_updates_are_enabled(void);
+
 /**
  * Submodules
  * ----------
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* Re: hn/reftable (Re: What's cooking in git.git (Sep 2021, #08; Mon, 27))
  2021-09-28 12:18   ` Han-Wen Nienhuys
@ 2021-09-30  5:06     ` Carlo Arenas
  0 siblings, 0 replies; 40+ messages in thread
From: Carlo Arenas @ 2021-09-30  5:06 UTC (permalink / raw)
  To: Han-Wen Nienhuys
  Cc: Ævar Arnfjörð Bjarmason, Junio C Hamano, git, ramsay

On Tue, Sep 28, 2021 at 5:19 AM Han-Wen Nienhuys <hanwen@google.com> wrote:
> On Tue, Sep 28, 2021 at 10:38 AM Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
> > > * hn/reftable (2021-09-10) 20 commits
> > >  - fixup! reftable: implement stack, a mutable database of reftable files.
> > >  - Add "test-tool dump-reftable" command.
> > >  - reftable: add dump utility
> > >  - reftable: implement stack, a mutable database of reftable files.
> > >  - reftable: implement refname validation
> > >  - reftable: add merged table view
> > >  - reftable: add a heap-based priority queue for reftable records
> > >  - reftable: reftable file level tests
> > >  - reftable: read reftable files
> > >  - reftable: generic interface to tables
> > >  - reftable: write reftable files
> > >  - reftable: a generic binary tree implementation
> > >  - reftable: reading/writing blocks
> > >  - Provide zlib's uncompress2 from compat/zlib-compat.c
> > >  - reftable: (de)serialization for the polymorphic record type.
> > >  - reftable: add blocksource, an abstraction for random access reads
> > >  - reftable: utility functions
> > >  - reftable: add error related functionality
> > >  - reftable: RFC: add LICENSE
> > >  - hash.h: provide constants for the hash IDs
> > >
> > >  The "reftable" backend for the refs API, without integrating into
> > >  the refs subsystem.
> > >
> > >  Will merge to 'next'?
> >
> > I think we've reached approximately "good enough" with this for the next
> > steps, and can hopefully fix any remaining nits (such as my [1])
> > post-merge.
>
> There is still an RFC in 02/19. Maybe we can get agreement that this
> is OK and drop the RFC ?
>
> > Maybe hold off until Han-Wen gets a chance to ack it, and whether he's
> > ok with the proposed fixup(s)?
>
> regarding uncompress2: I thought it was functionality best left to
> zlib to implement, but I imagine git.git offers something similar.
> Sounds good to use that.
>
> > Wasn't there an outstanding "some of this in reftable/* should be static
> > functions" from someone, Carlo? (CC'd). In any case that sort of thing
> > could also be a post-cleanup, I couldn't find a reference to that
> > discussion in anything except my vague memory of it as I wrote this.
>
> I think I have addressed all outstanding issues in my github PR, and
> I'll send it out once I see CI pass.

It was actually raised by Ramsay[1], and might have been a red herring,
but there was for sure no follow up.

I have a few minor fixups that will post for discussion, and might be even
ok to merge to next without squashing if included on top, but I am not
sure if all those issues could be addressed post-cleanup, since IMHO
once this gets into master, then it will be considered a stable API
and it can't be changed.

For example. while looking at Ramsay message noticed the API for "reader"
in libreftable might be better with s/init_reader/reader_init/g

Carlo

[1] https://lore.kernel.org/git/bc387e32-321e-4726-2a02-2e6cf6ed5250@ramsayjones.plus.com/

CC +Ramsay

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: What's cooking in git.git (Sep 2021, #08; Mon, 27)
  2021-09-29 18:43     ` Neeraj Singh
@ 2021-09-30  8:16       ` Jeff King
  2021-10-01  7:50         ` Elijah Newren
  0 siblings, 1 reply; 40+ messages in thread
From: Jeff King @ 2021-09-30  8:16 UTC (permalink / raw)
  To: Neeraj Singh
  Cc: Elijah Newren, Junio C Hamano, Git Mailing List, Jonathan Nieder

On Wed, Sep 29, 2021 at 11:43:39AM -0700, Neeraj Singh wrote:

> It seems to me that one problem is that the new "primary" objdir code doesn't
> disable ref updates since the GIT_QUARANTINE_ENVIRONMENT setting isn't set.
> If we fix that we should be forbidding ref updates.
> 
> I've included a path that fixes my test case. This is on top of:
> https://lore.kernel.org/git/CANQDOddqwVtWfC4eEP3fJB4sUiszGX8bLqoEVLcMf=v+jzx19g@mail.gmail.com/

Ah, right, I forgot we had that "forbid ref updates in quarantine" magic
(despite being the one who added it).

I do think this improves the safety, but things are still a bit more
dangerous because we're handling it all in a single process, which sees
both the quarantine and non-quarantine states. I wrote more details in
this reply a few minutes ago:

  https://lore.kernel.org/git/YVVmssXlaFM6yD5W@coredump.intra.peff.net/

(sorry, it's long; search for the paragraph starting with "Whereas doing
it in-process").

> When creating a subprocess with a temporary ODB, we set the
> GIT_QUARANTINE_ENVIRONMENT env var to tell child Git processes not
> to update refs, since the tmp-objdir may go away.
> 
> Introduce a similar mechanism for in-process temporary ODBs when
> we call tmp_objdir_replace_primary_odb.
> 
> Peff's test case was invoking ref updates via the cachetextconv
> setting. That particular code silently does nothing when a ref
> update is forbidden

Oh good. I was worried that it would generate ugly errors, rather than
silently skipping the update.

> @@ -2126,7 +2146,7 @@ int ref_transaction_prepare(struct ref_transaction *transaction,
>  		break;
>  	}
>  
> -	if (getenv(GIT_QUARANTINE_ENVIRONMENT)) {
> +	if (getenv(GIT_QUARANTINE_ENVIRONMENT) || !ref_updates_are_enabled()) {
>  		strbuf_addstr(err,
>  			      _("ref updates forbidden inside quarantine environment"));
>  		return -1;

I think the overall goal of this patch makes sense, but this
conditional, along with tmp-objdir reaching out to the refs code, feels
funny. Should we perhaps have a single:

  int tmp_objdir_is_primary(struct repository *r)
  {
	if (the_tmp_objdir &&
	    !strcmp(the_tmp_objdir->path.buf, r->objects->odb->path))
		return 1; /* our temporary is the primary */

	if (getenv(GIT_QUARANTINE_PATH))
		return 1; /* our caller put us in quarantine */

	return 0;
  }

Then it's all nicely abstracted and the ref code does not have to know
the details of GIT_QUARANTINE_PATH in the first place.

If you do got that route, the strcmp() might need to be a little more
careful about whether r->objects can be NULL (I didn't check).
Alternatively, I kind of wonder if "struct object_directory" should just
have a flag that says "is_temporary".

-Peff

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: What's cooking in git.git (Sep 2021, #08; Mon, 27)
  2021-09-29  8:12 ` What's cooking in git.git (Sep 2021, #08; Mon, 27) Fabian Stelzer
@ 2021-09-30 21:26   ` Junio C Hamano
  0 siblings, 0 replies; 40+ messages in thread
From: Junio C Hamano @ 2021-09-30 21:26 UTC (permalink / raw)
  To: Fabian Stelzer; +Cc: git

Fabian Stelzer <fs@gigacodes.de> writes:

> On 28.09.21 02:52, Junio C Hamano wrote > * fs/ssh-signing (2021-09-10)
> 9 commits
>>  - ssh signing: test that gpg fails for unknown keys
>>  - ssh signing: tests for logs, tags & push certs
>>  - ssh signing: duplicate t7510 tests for commits
>>  - ssh signing: verify signatures using ssh-keygen
>>  - ssh signing: provide a textual signing_key_id
>>  - ssh signing: retrieve a default key from ssh-agent
>>  - ssh signing: add ssh key format and signing code
>>  - ssh signing: add test prereqs
>>  - ssh signing: preliminary refactoring and clean-up
>> 
>>  Use ssh public crypto for object and push-cert signing.
>> 
>>  On hold.
>>  cf. <pull.1041.v8.git.git.1631304462.gitgitgadget@gmail.com>
>>  cf. <532d97e7-8c91-df6a-6d90-70668256f513@gigacodes.de>
>> 
>>
> Openssh 8.8 has been released a few days ago and includes the needed fix
> for the find-principal segfault.
> I ran the full git testsuite against it without issues.
>
> Also, we (~30developers) have been running this patch with
> openssh-portable (2d678c5e3bdc2f5c99f7af5122e9d054925d560d / post 8.7 -
> pre 8.8) in our organization for the last 2 weeks without problems.
> ...
> But vverall i think this is ready for some broader usage/testing via next.
>
> I'd suggest to send the additional patches for valid-before/after
> functionality in a new patchset for review after.

OK, so with the new OpenSSH, there wasn't anything that contradicts
what the above set of patches wanted to do and the series is good to
go?  Let's merge it to 'next' for wider audience, then.

Thanks.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: What's cooking in git.git (Sep 2021, #08; Mon, 27)
  2021-09-30  8:16       ` Jeff King
@ 2021-10-01  7:50         ` Elijah Newren
  2021-10-01 17:02           ` Junio C Hamano
  0 siblings, 1 reply; 40+ messages in thread
From: Elijah Newren @ 2021-10-01  7:50 UTC (permalink / raw)
  To: Jeff King; +Cc: Neeraj Singh, Junio C Hamano, Git Mailing List, Jonathan Nieder

On Thu, Sep 30, 2021 at 1:16 AM Jeff King <peff@peff.net> wrote:
>
> On Wed, Sep 29, 2021 at 11:43:39AM -0700, Neeraj Singh wrote:
>
> > It seems to me that one problem is that the new "primary" objdir code doesn't
> > disable ref updates since the GIT_QUARANTINE_ENVIRONMENT setting isn't set.
> > If we fix that we should be forbidding ref updates.
> >
> > I've included a path that fixes my test case. This is on top of:
> > https://lore.kernel.org/git/CANQDOddqwVtWfC4eEP3fJB4sUiszGX8bLqoEVLcMf=v+jzx19g@mail.gmail.com/
>
> Ah, right, I forgot we had that "forbid ref updates in quarantine" magic
> (despite being the one who added it).
>
> I do think this improves the safety, but things are still a bit more
> dangerous because we're handling it all in a single process, which sees
> both the quarantine and non-quarantine states. I wrote more details in
> this reply a few minutes ago:
>
>   https://lore.kernel.org/git/YVVmssXlaFM6yD5W@coredump.intra.peff.net/
>
> (sorry, it's long; search for the paragraph starting with "Whereas doing
> it in-process").
>
> > When creating a subprocess with a temporary ODB, we set the
> > GIT_QUARANTINE_ENVIRONMENT env var to tell child Git processes not
> > to update refs, since the tmp-objdir may go away.
> >
> > Introduce a similar mechanism for in-process temporary ODBs when
> > we call tmp_objdir_replace_primary_odb.
> >
> > Peff's test case was invoking ref updates via the cachetextconv
> > setting. That particular code silently does nothing when a ref
> > update is forbidden
>
> Oh good. I was worried that it would generate ugly errors, rather than
> silently skipping the update.
>
> > @@ -2126,7 +2146,7 @@ int ref_transaction_prepare(struct ref_transaction *transaction,
> >               break;
> >       }
> >
> > -     if (getenv(GIT_QUARANTINE_ENVIRONMENT)) {
> > +     if (getenv(GIT_QUARANTINE_ENVIRONMENT) || !ref_updates_are_enabled()) {
> >               strbuf_addstr(err,
> >                             _("ref updates forbidden inside quarantine environment"));
> >               return -1;
>
> I think the overall goal of this patch makes sense, but this
> conditional, along with tmp-objdir reaching out to the refs code, feels
> funny. Should we perhaps have a single:
>
>   int tmp_objdir_is_primary(struct repository *r)
>   {
>         if (the_tmp_objdir &&
>             !strcmp(the_tmp_objdir->path.buf, r->objects->odb->path))
>                 return 1; /* our temporary is the primary */
>
>         if (getenv(GIT_QUARANTINE_PATH))
>                 return 1; /* our caller put us in quarantine */
>
>         return 0;
>   }
>
> Then it's all nicely abstracted and the ref code does not have to know
> the details of GIT_QUARANTINE_PATH in the first place.
>
> If you do got that route, the strcmp() might need to be a little more
> careful about whether r->objects can be NULL (I didn't check).
> Alternatively, I kind of wonder if "struct object_directory" should just
> have a flag that says "is_temporary".

Actually, wouldn't this be the safest approach, for my particular
usecase?  By having the quarantine in place, no refs will be updated,
which removes the risk of new refs mentioning objects that will go
away.  The only issue that could arise would be from new objects
referencing objects that will go away.  But the new objects are also
written to the temporary object store when it remains the primary, and
thus those new objects will also be deleted when the temporary object
store is.

In contrast, moving the temporary object store to the end of the
alternates list would allow new objects to be written that reference
the objects that will go away.  Using pretend_object_file() will also
allow new objects to be written that reference the objects that will
go away.

Said another way, I don't think anything should be writing a critical
file that needs to be durable when we're in the middle of a
"read-only" process like git-log.  The only things written should be
temporary stuff, like the automatic remerge calculation from
merge-ort, the textconv cache optimization stuff, or perhaps future
gitattributes transformation caching.  All that stuff can safely be
blown away at the completion of each merge.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: What's cooking in git.git (Sep 2021, #08; Mon, 27)
  2021-10-01  7:50         ` Elijah Newren
@ 2021-10-01 17:02           ` Junio C Hamano
  2021-10-01 17:39             ` Neeraj Singh
  2021-10-01 18:12             ` Elijah Newren
  0 siblings, 2 replies; 40+ messages in thread
From: Junio C Hamano @ 2021-10-01 17:02 UTC (permalink / raw)
  To: Elijah Newren; +Cc: Jeff King, Neeraj Singh, Git Mailing List, Jonathan Nieder

Elijah Newren <newren@gmail.com> writes:

> Said another way, I don't think anything should be writing a critical
> file that needs to be durable when we're in the middle of a
> "read-only" process like git-log.  The only things written should be
> temporary stuff, like the automatic remerge calculation from
> merge-ort, the textconv cache optimization stuff, or perhaps future
> gitattributes transformation caching.  All that stuff can safely be
> blown away at the completion of each merge.

The textconv cache can be populated/written during "git log -p" into
the object store to persist.  With or without "--remerge-diff", we
can make design decision to either 

 - use temporary object store to discard everything we create at the
   end in one-go, or

 - write them into the object store to let later gc to handle the
   crufts.

The former will disable persistent write access to the cache.  It
still allows accesses the cached data during the same process,
though.  We so far deemed that textconv cache, when the user enables
it, is valuable enough to make persistent.  Perhaps remerge-diff's
tentative merge result may fall into the same category?  Some folks
may want to cache, others may not.

If we were to use the same notes-cache mechanism, we record the tree
object (perhaps the object name) as the cached value for the merge
commit in question.  Hopefully most of the merges are clean merges,
and "caching" the results of the recreation of the merge would cost
almost nothing.  We need objects to record the fact that "this merge
has cached result" in the notes-cache, but the tree object that
represents the cached result is already in the history the merge
belongs to.


^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: What's cooking in git.git (Sep 2021, #08; Mon, 27)
  2021-10-01 17:02           ` Junio C Hamano
@ 2021-10-01 17:39             ` Neeraj Singh
  2021-10-01 18:15               ` Elijah Newren
  2021-10-01 18:12             ` Elijah Newren
  1 sibling, 1 reply; 40+ messages in thread
From: Neeraj Singh @ 2021-10-01 17:39 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Elijah Newren, Jeff King, Git Mailing List, Jonathan Nieder

On Fri, Oct 1, 2021 at 10:02 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> Elijah Newren <newren@gmail.com> writes:
>
> > Said another way, I don't think anything should be writing a critical
> > file that needs to be durable when we're in the middle of a
> > "read-only" process like git-log.  The only things written should be
> > temporary stuff, like the automatic remerge calculation from
> > merge-ort, the textconv cache optimization stuff, or perhaps future
> > gitattributes transformation caching.  All that stuff can safely be
> > blown away at the completion of each merge.
>
> The textconv cache can be populated/written during "git log -p" into
> the object store to persist.  With or without "--remerge-diff", we
> can make design decision to either
>
>  - use temporary object store to discard everything we create at the
>    end in one-go, or
>
>  - write them into the object store to let later gc to handle the
>    crufts.
>
> The former will disable persistent write access to the cache.  It
> still allows accesses the cached data during the same process,
> though.  We so far deemed that textconv cache, when the user enables
> it, is valuable enough to make persistent.  Perhaps remerge-diff's
> tentative merge result may fall into the same category?  Some folks
> may want to cache, others may not.
>
> If we were to use the same notes-cache mechanism, we record the tree
> object (perhaps the object name) as the cached value for the merge
> commit in question.  Hopefully most of the merges are clean merges,
> and "caching" the results of the recreation of the merge would cost
> almost nothing.  We need objects to record the fact that "this merge
> has cached result" in the notes-cache, but the tree object that
> represents the cached result is already in the history the merge
> belongs to.
>

Elijah,
To Junio's point, I'm also curious about why it's so important to
aggressively purge the
mechanical merge cache that you want to do it after each diff
operation rather than once
at the end.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: What's cooking in git.git (Sep 2021, #08; Mon, 27)
  2021-10-01 17:02           ` Junio C Hamano
  2021-10-01 17:39             ` Neeraj Singh
@ 2021-10-01 18:12             ` Elijah Newren
  2021-10-01 22:02               ` Junio C Hamano
  1 sibling, 1 reply; 40+ messages in thread
From: Elijah Newren @ 2021-10-01 18:12 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, Neeraj Singh, Git Mailing List, Jonathan Nieder

On Fri, Oct 1, 2021 at 10:02 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> Elijah Newren <newren@gmail.com> writes:
>
> > Said another way, I don't think anything should be writing a critical
> > file that needs to be durable when we're in the middle of a
> > "read-only" process like git-log.  The only things written should be
> > temporary stuff, like the automatic remerge calculation from
> > merge-ort, the textconv cache optimization stuff, or perhaps future
> > gitattributes transformation caching.  All that stuff can safely be
> > blown away at the completion of each merge.
>
> The textconv cache can be populated/written during "git log -p" into
> the object store to persist.  With or without "--remerge-diff", we
> can make design decision to either
>
>  - use temporary object store to discard everything we create at the
>    end in one-go, or
>
>  - write them into the object store to let later gc to handle the
>    crufts.
>
> The former will disable persistent write access to the cache.  It
> still allows accesses the cached data during the same process,
> though.  We so far deemed that textconv cache, when the user enables
> it, is valuable enough to make persistent.  Perhaps remerge-diff's
> tentative merge result may fall into the same category?  Some folks
> may want to cache, others may not.
>
> If we were to use the same notes-cache mechanism, we record the tree
> object (perhaps the object name) as the cached value for the merge
> commit in question.  Hopefully most of the merges are clean merges,
> and "caching" the results of the recreation of the merge would cost
> almost nothing.  We need objects to record the fact that "this merge
> has cached result" in the notes-cache, but the tree object that
> represents the cached result is already in the history the merge
> belongs to.

I'm not sure this performance caching for remerge-diff makes any
sense, for multiple reasons:

1) Does the cache become invalidated when the merge algorithm changes
or when config options change (--remerge-diff shows the difference
between what an automatic merge would do, but an automatic merge's
results depend on the current algorithm and various merge.*, diff.*,
etc. config options the user can set).  Do we re-cache each variant?
Do we expire the cache at some point?

2) The performance aspect seems suspect at best, and likely to backfire.  Badly:

2a) Let's say there's 100,000 merge commits in your history (so small
history roughly the size of linux.git).  `git log --remerge-diff` thus
repeats 100,000 merges, creating loose objects for them.  Perhaps only
1 in 10 merges needs any loose objects created (because the rest of
the merges were clean).  But those that do have conflicts will need to
create both new blobs and any necessary new trees that use those new
blobs.  Who knows how many blobs and trees are needed, but let's just
use 10 total blobs+trees per merge that has conflicts as a guesstimate
of the average.  Thus `git log --remerge-diff` will need to create
100,000 * 1/10 * 10 = 100,000 loose objects while it runs.  Preserving
all those files to cache the results slows things down
considerably...until they are gc'ed.  If we do the notes-cache thing,
then yes these objects would become packed.  Perhaps you're suggested
doing an automatic gc (or several) as part of running --remerge-diff,
but that's super slow itself.  When I implemented that, it felt worse
to me than just letting loose objects pile up.

2b) Note that my implementation didn't just clear out the tmp-objdir
at the _end_ of the remerge-diff (which was the easiest to implement),
but cleared it out after each merge, because that was much faster with
as many objects as can accumulate.

2c) Is --remerge-diff slow enough to even merit the complicated effort
of caching results?  --remerge-diff is faster than --cc in my testing,
so why is it even worth the pain to cache?



So with that out of the way, let's return to discussing the textconv
cache.  If the remerge-diff results aren't cached, isn't it unsafe to
allow the textconv cache to persist anything while remerge-diff is
running because it could create corruption?  We could still let the
textconv cache persist results when remerge-diff is not in use, but I
think we _want_ anything written by the textconv cache during
remerge-diff to be thrown away because there's a risk it references a
temporary object created by remerge-diff that will be deleted.

With all that in mind, to me, it seems like using the tmp-objdir as
the primary object store in combination with the refs quarantine is
actually the safest solution for this usecase.  Am I missing
something?

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: What's cooking in git.git (Sep 2021, #08; Mon, 27)
  2021-10-01 17:39             ` Neeraj Singh
@ 2021-10-01 18:15               ` Elijah Newren
  0 siblings, 0 replies; 40+ messages in thread
From: Elijah Newren @ 2021-10-01 18:15 UTC (permalink / raw)
  To: Neeraj Singh; +Cc: Junio C Hamano, Jeff King, Git Mailing List, Jonathan Nieder

On Fri, Oct 1, 2021 at 10:39 AM Neeraj Singh <nksingh85@gmail.com> wrote:
>
> On Fri, Oct 1, 2021 at 10:02 AM Junio C Hamano <gitster@pobox.com> wrote:
> >
> > Elijah Newren <newren@gmail.com> writes:
> >
> > > Said another way, I don't think anything should be writing a critical
> > > file that needs to be durable when we're in the middle of a
> > > "read-only" process like git-log.  The only things written should be
> > > temporary stuff, like the automatic remerge calculation from
> > > merge-ort, the textconv cache optimization stuff, or perhaps future
> > > gitattributes transformation caching.  All that stuff can safely be
> > > blown away at the completion of each merge.
> >
> > The textconv cache can be populated/written during "git log -p" into
> > the object store to persist.  With or without "--remerge-diff", we
> > can make design decision to either
> >
> >  - use temporary object store to discard everything we create at the
> >    end in one-go, or
> >
> >  - write them into the object store to let later gc to handle the
> >    crufts.
> >
> > The former will disable persistent write access to the cache.  It
> > still allows accesses the cached data during the same process,
> > though.  We so far deemed that textconv cache, when the user enables
> > it, is valuable enough to make persistent.  Perhaps remerge-diff's
> > tentative merge result may fall into the same category?  Some folks
> > may want to cache, others may not.
> >
> > If we were to use the same notes-cache mechanism, we record the tree
> > object (perhaps the object name) as the cached value for the merge
> > commit in question.  Hopefully most of the merges are clean merges,
> > and "caching" the results of the recreation of the merge would cost
> > almost nothing.  We need objects to record the fact that "this merge
> > has cached result" in the notes-cache, but the tree object that
> > represents the cached result is already in the history the merge
> > belongs to.
> >
>
> Elijah,
> To Junio's point, I'm also curious about why it's so important to
> aggressively purge the
> mechanical merge cache that you want to do it after each diff
> operation rather than once
> at the end.

Performance; I don't want to accumulate hundreds of thousands of loose
objects and pay the penalty from them sitting around, especially when
I know they're cruft.  (My original implementation from a year ago did
just clean out once at the end.)

For more details, see
https://lore.kernel.org/git/CABPp-BGsjq3ts4A6wKLYcopD9rknM+LXXi8qR_SLEpmU+x7KNQ@mail.gmail.com/,
starting with "It'd be _much_ slower".  Or the email I just sent to
Junio.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: What's cooking in git.git (Sep 2021, #08; Mon, 27)
  2021-10-01 18:12             ` Elijah Newren
@ 2021-10-01 22:02               ` Junio C Hamano
  2021-10-01 23:05                 ` Elijah Newren
  0 siblings, 1 reply; 40+ messages in thread
From: Junio C Hamano @ 2021-10-01 22:02 UTC (permalink / raw)
  To: Elijah Newren; +Cc: Jeff King, Neeraj Singh, Git Mailing List, Jonathan Nieder

Elijah Newren <newren@gmail.com> writes:

> So with that out of the way, let's return to discussing the textconv
> cache.  If the remerge-diff results aren't cached, isn't it unsafe to
> allow the textconv cache to persist anything while remerge-diff is
> running because it could create corruption?

I do not think anybody involved in this thread thinks it is
practical to annotate each write_object_file() with "this is
temporary" vs "this is to persist", so it is a given that it would
be all-or-none.  If we want write_object_file() called while we are
running remerge-diff to write to a temporary object store, we have
to accept any other write_object_file() called by somebody else,
like textconv cache, must become temporary.

It may be sufficient to plug ref updates (which would cover the
finialization of notes-cache used by the textconv cache) to avoid
corruption, but that might give us a pointless and unpleasant error
message, so it may be necessary to teach the notes-cache stuff to
allow getting existing cached data while disabling it to accept
cache updates.

Thanks.


^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: What's cooking in git.git (Sep 2021, #08; Mon, 27)
  2021-10-01 22:02               ` Junio C Hamano
@ 2021-10-01 23:05                 ` Elijah Newren
  0 siblings, 0 replies; 40+ messages in thread
From: Elijah Newren @ 2021-10-01 23:05 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, Neeraj Singh, Git Mailing List, Jonathan Nieder

On Fri, Oct 1, 2021 at 3:02 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Elijah Newren <newren@gmail.com> writes:
>
> > So with that out of the way, let's return to discussing the textconv
> > cache.  If the remerge-diff results aren't cached, isn't it unsafe to
> > allow the textconv cache to persist anything while remerge-diff is
> > running because it could create corruption?
>
> I do not think anybody involved in this thread thinks it is
> practical to annotate each write_object_file() with "this is
> temporary" vs "this is to persist", so it is a given that it would
> be all-or-none.  If we want write_object_file() called while we are
> running remerge-diff to write to a temporary object store, we have
> to accept any other write_object_file() called by somebody else,
> like textconv cache, must become temporary.

Ok, good.

But is this write_object_file() specific?  I think
pretend_object_file() has the same problem where the textconv cache
could reference a pretend_object_file() and thus write objects and/or
refs that become corrupt.  (In particular, if the userdiff fires on
any of the new files created by the three-way content merges, then I
think we hit the exact same problem)  So, I think as long as there are
any "temporary" objects being fed to diff, the textconv cache needs to
also be considered temporary.  Or am I misunderstanding something?

> It may be sufficient to plug ref updates (which would cover the
> finialization of notes-cache used by the textconv cache) to avoid
> corruption, but that might give us a pointless and unpleasant error
> message, so it may be necessary to teach the notes-cache stuff to
> allow getting existing cached data while disabling it to accept
> cache updates.

All we need to do here is set the quarantine environment, as per
Neeraj's report[1], it already handles all of this:

"""
Peff's test case was invoking ref updates via the cachetextconv
setting. That particular code silently does nothing when a ref
update is forbidden
"""

Looking at the code, this appears to happen because
ref_transaction_prepare() doesn't print an error in the quarantine
environment, but returns one.  That error is passed all the way up the
stack to notes_cache_write() and then to fill_textconv().
fill_textconv() is the first level in the stack that ignores the
return code of what it's calling, namely notes_cache_write(), which
means that things work fine.  There's even a comment a few lines
stating, "ignore errors, as we might be in a readonly repository".
So, I think we're good as long as we ensure the quarantine environment
is setup.

[1] https://lore.kernel.org/git/20210929184339.GA19712@neerajsi-x1.localdomain/

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: What's cooking in git.git (Sep 2021, #08; Mon, 27)
  2021-09-28 23:40   ` Jeff King
  2021-09-28 23:49     ` Jeff King
  2021-09-29 18:43     ` Neeraj Singh
@ 2021-10-04 13:45     ` Elijah Newren
  2 siblings, 0 replies; 40+ messages in thread
From: Elijah Newren @ 2021-10-04 13:45 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, Git Mailing List, Jonathan Nieder

On Tue, Sep 28, 2021 at 4:40 PM Jeff King <peff@peff.net> wrote:
>
> I just left a rather long-ish mail in the thread, but the gist of it is
> that I'm worried that there's some possibility of corruption there if
> the diff code writes objects. I didn't do a proof-of-concept there, but
> I worked one up just now. Try this:

Thanks for the testcase; it's very helpful.

Let's make it a little more interesting, though...

>
>   # make a repo
>   git init repo
>   cd repo
>   echo base >file
>   git add file
>   git commit -m base
>
>   # two diverging branches
>   echo main >file
>   git commit -am main
>   git checkout -b side HEAD^
>   echo side >file
>   git commit -am side
>
>   # we get a conflict, and we resolve
>   git merge main
>   echo resolved >file
>   git commit -am merged
>
>   # now imagine we had a file with a diff driver. I stuffed it
>   # in here after the fact, but it could have been here all along,
>   # or come as part of the merge, etc.
>   echo whatever >unrelated
>   echo "unrelated diff=foo" >.gitattributes

If we change this line to:

  echo "file diff=foo" >.gitattributes

Then the userdiff will fire on the file involved in the remerge-diff,
including on the file included in the automatic 3-way content
conflict, giving us a diff that looks like this:

diff --git a/file b/file
index 5c66bc5..2ab19ae 100644
--- a/file
+++ b/file
@@ -1,7 +1 @@
-<<<<<<< 7D4147C (SIDE)
-SIDE
-||||||| BE54C04
-BASE
-=======
-MAIN
->>>>>>> 03FC4E3 (MAIN)
+RESOLVED

>   git add .
>   git commit --amend --no-edit
>
>   # set up the diff driver not just to do a textconv, but to cache the
>   # result. This will require writing out new objects for the cache
>   # as part of the diff operation.
>   git config diff.foo.textconv "$PWD/upcase"
>   git config diff.foo.cachetextconv true
>   cat >upcase <<\EOF &&
>   #!/bin/sh
>   tr a-z A-Z <$1
>   EOF
>   chmod +x upcase
>
>   # now show the diff
>   git log -1 --remerge-diff
>
>   # and make sure the repo is still OK
>   git fsck
>
> The remerge diff will correctly show the textconv'd content (because
> it's not in either parent, and hence an evil merge):
>
>   diff --git a/unrelated b/unrelated
>   new file mode 100644
>   index 0000000..982793c
>   --- /dev/null
>   +++ b/unrelated
>   @@ -0,0 +1 @@
>   +WHATEVER
>
> but then fsck shows that our cache is corrupted:
>
>   Checking object directories: 100% (256/256), done.
>   error: refs/notes/textconv/foo: invalid sha1 pointer 1e9b3ecffca73411001043fe914a7ec0e7291043
>   error: refs/notes/textconv/foo: invalid reflog entry 1e9b3ecffca73411001043fe914a7ec0e7291043
>   dangling tree 569b6e414203d2f50bdaafc1585eae11e28afc37


> Now I'll admit the textconv-cache is a pretty seldom-used feature. And
> that there _probably_ aren't a lot of other ways that the diff code
> would try to write objects or references. But I think it speaks to the
> kind of subtle interactions we should worry about (and certainly
> corrupting the repository is a pretty bad outcome, though at least in
> this case it's "just" a cache and we could blow away that ref manually).


I implemented the pretend_object_file() solution on Saturday and tried
this out, and found with the above change that it has corruption
problems as well.  This really doesn't feel much safer too me.

The move-the-tmp-objdir-to-just-being-an-alternate-instead-of-a-primary
solution, as suggested at
https://lore.kernel.org/git/YVOiggCWAdZcxAb6@coredump.intra.peff.net/
has the same problem.

But, more importantly, neither of these solutions can be made safe.
Even if we adopt the GIT_QUARANTINE_PATH suggestion from Neeraj
(https://lore.kernel.org/git/20210929184339.GA19712@neerajsi-x1.localdomain/),
we still have objects referencing now-missing objects.

*However*, if we use the tmp-objdir-as-primary solution, and leave it
as a primary, and use your modification to the GIT_QUARANTINE_PATH
idea from Neeraj (i.e. create a "the-tmp-objdir" and have the refs
code check for it and turn on the quarantine automatically when one
exists), then we prevent adding or modifying refs to point to bad
objects, and any new objects that textconv might write that could
depend on transient objects, will themselves be transient and go away.
So, this solution is safe from corruption.  I'll be implementing it.

(not sure how quickly I'll get it done, as my time is mostly limited
to early mornings or evenings or Saturdays right now...)

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* Re: What's cooking in git.git (Sep 2021, #08; Mon, 27)
  2021-09-28 23:53           ` Neeraj Singh
@ 2021-10-07 22:01             ` Junio C Hamano
  2021-10-08  6:51               ` Elijah Newren
  0 siblings, 1 reply; 40+ messages in thread
From: Junio C Hamano @ 2021-10-07 22:01 UTC (permalink / raw)
  To: Neeraj Singh, Elijah Newren
  Cc: Ævar Arnfjörð Bjarmason, Git Mailing List,
	Jeff King, Jonathan Nieder, Neeraj Singh

Neeraj Singh <nksingh85@gmail.com> writes:

> Elijah,
>
> Here's a branch of your changes based on the amalgamated tmp-objdir code:
> https://github.com/neerajsi-msft/git/commits/neerajsi/remerge-diff
>
> This commit adapts your code to use the amalgamated API:
> https://github.com/neerajsi-msft/git/commit/725328fe1d8be8326d2ddef78e164ca21450b100

It seems that the discussion petered out at this point.

Right now I have a version of ns/remerge-diff before this adjustment
in 'seen', and Neeraj's latest version is kept out of 'seen' as they
do not play well together without an adjustment like that.

What's the good way forward?  I do not deeply care which one goes
first, but I have a feeling that the need by remerge-diff that wants
to discard temporary objects would involve more work to make it safe
than the need by batched fsync where newly created objects will not
be discarded but merely moved to the primary store before the end of
the operation, so from that point of view, it seems simpler and
safer to queue ns/batched-fsync topic first (especially given that
it is a no-op until the end-user opts into the experiment), and have
a remerge-diff that uses the infrastructure from Neeraj's topic.

What's your take on the rebase Neeraj made, Elijah (at the URL
above)?

Thanks.





^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: What's cooking in git.git (Sep 2021, #08; Mon, 27)
  2021-10-07 22:01             ` Junio C Hamano
@ 2021-10-08  6:51               ` Elijah Newren
  2021-10-08 22:30                 ` Neeraj Singh
  2021-10-08 23:01                 ` Junio C Hamano
  0 siblings, 2 replies; 40+ messages in thread
From: Elijah Newren @ 2021-10-08  6:51 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Neeraj Singh, Ævar Arnfjörð Bjarmason,
	Git Mailing List, Jeff King, Jonathan Nieder, Neeraj Singh

On Thu, Oct 7, 2021 at 3:01 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Neeraj Singh <nksingh85@gmail.com> writes:
>
> > Elijah,
> >
> > Here's a branch of your changes based on the amalgamated tmp-objdir code:
> > https://github.com/neerajsi-msft/git/commits/neerajsi/remerge-diff
> >
> > This commit adapts your code to use the amalgamated API:
> > https://github.com/neerajsi-msft/git/commit/725328fe1d8be8326d2ddef78e164ca21450b100
>
> It seems that the discussion petered out at this point.
>
> Right now I have a version of ns/remerge-diff before this adjustment
> in 'seen', and Neeraj's latest version is kept out of 'seen' as they
> do not play well together without an adjustment like that.
>
> What's the good way forward?  I do not deeply care which one goes
> first, but I have a feeling that the need by remerge-diff that wants
> to discard temporary objects would involve more work to make it safe
> than the need by batched fsync where newly created objects will not
> be discarded but merely moved to the primary store before the end of
> the operation, so from that point of view, it seems simpler and
> safer to queue ns/batched-fsync topic first (especially given that
> it is a no-op until the end-user opts into the experiment), and have
> a remerge-diff that uses the infrastructure from Neeraj's topic.
>
> What's your take on the rebase Neeraj made, Elijah (at the URL
> above)?

I meant to dig further, but nearly all my git time in the last week
and a half was attempting to keep up with other patch reviews.  My git
time is fast disappearing in the near term, and it's not clear how
much, if any, time I'll have to work on patches (or even continued
reviewing) before, say, mid-November.  I most likely won't be able to
do any discussion-prep work in advance of the Git Contributor's
Summit, and might not even be able to attend anymore.

I had looked over Neeraj's patches and they looked reasonable.  I
thought there might be some tweaks that I could try out, but at this
point, just take what he has and keep my topic as expecting an update.
I'll circle back eventually.

Sorry for the delay.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: What's cooking in git.git (Sep 2021, #08; Mon, 27)
  2021-10-08  6:51               ` Elijah Newren
@ 2021-10-08 22:30                 ` Neeraj Singh
  2021-10-08 23:01                 ` Junio C Hamano
  1 sibling, 0 replies; 40+ messages in thread
From: Neeraj Singh @ 2021-10-08 22:30 UTC (permalink / raw)
  To: Elijah Newren
  Cc: Junio C Hamano, Ævar Arnfjörð Bjarmason,
	Git Mailing List, Jeff King, Jonathan Nieder, Neeraj Singh

Thanks Junio.  I'd be happy to have my change go in first.  I'm hoping
that we'll be able to get a lot of mileage with batch mode in Git For
Windows early on.

Thanks,
Neeraj

On Thu, Oct 7, 2021 at 11:52 PM Elijah Newren <newren@gmail.com> wrote:
>
> On Thu, Oct 7, 2021 at 3:01 PM Junio C Hamano <gitster@pobox.com> wrote:
> >
> > Neeraj Singh <nksingh85@gmail.com> writes:
> >
> > > Elijah,
> > >
> > > Here's a branch of your changes based on the amalgamated tmp-objdir code:
> > > https://github.com/neerajsi-msft/git/commits/neerajsi/remerge-diff
> > >
> > > This commit adapts your code to use the amalgamated API:
> > > https://github.com/neerajsi-msft/git/commit/725328fe1d8be8326d2ddef78e164ca21450b100
> >
> > It seems that the discussion petered out at this point.
> >
> > Right now I have a version of ns/remerge-diff before this adjustment
> > in 'seen', and Neeraj's latest version is kept out of 'seen' as they
> > do not play well together without an adjustment like that.
> >
> > What's the good way forward?  I do not deeply care which one goes
> > first, but I have a feeling that the need by remerge-diff that wants
> > to discard temporary objects would involve more work to make it safe
> > than the need by batched fsync where newly created objects will not
> > be discarded but merely moved to the primary store before the end of
> > the operation, so from that point of view, it seems simpler and
> > safer to queue ns/batched-fsync topic first (especially given that
> > it is a no-op until the end-user opts into the experiment), and have
> > a remerge-diff that uses the infrastructure from Neeraj's topic.
> >
> > What's your take on the rebase Neeraj made, Elijah (at the URL
> > above)?
>
> I meant to dig further, but nearly all my git time in the last week
> and a half was attempting to keep up with other patch reviews.  My git
> time is fast disappearing in the near term, and it's not clear how
> much, if any, time I'll have to work on patches (or even continued
> reviewing) before, say, mid-November.  I most likely won't be able to
> do any discussion-prep work in advance of the Git Contributor's
> Summit, and might not even be able to attend anymore.
>
> I had looked over Neeraj's patches and they looked reasonable.  I
> thought there might be some tweaks that I could try out, but at this
> point, just take what he has and keep my topic as expecting an update.
> I'll circle back eventually.
>
> Sorry for the delay.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: What's cooking in git.git (Sep 2021, #08; Mon, 27)
  2021-10-08  6:51               ` Elijah Newren
  2021-10-08 22:30                 ` Neeraj Singh
@ 2021-10-08 23:01                 ` Junio C Hamano
  1 sibling, 0 replies; 40+ messages in thread
From: Junio C Hamano @ 2021-10-08 23:01 UTC (permalink / raw)
  To: Elijah Newren
  Cc: Neeraj Singh, Ævar Arnfjörð Bjarmason,
	Git Mailing List, Jeff King, Jonathan Nieder, Neeraj Singh

Elijah Newren <newren@gmail.com> writes:

> I had looked over Neeraj's patches and they looked reasonable.  I
> thought there might be some tweaks that I could try out, but at this
> point, just take what he has and keep my topic as expecting an update.
> I'll circle back eventually.

Thanks.

I've fetched Neeraj's remerge-diff rebase, and structured three
topics: 

 . 'ns/tmp-objdir' topic is to hold the bottommost two tmp-objdir
   patches.

 . 'ns/batched-fsync' topic forks from 'master', merges
   'ns/tmp-objdir', and then applies its own patches.

 . 'ns/remerge-diff' topic forks from 'master', merges
   'ns/tmp-objdir', and then applies its own patches.


^ permalink raw reply	[flat|nested] 40+ messages in thread

end of thread, other threads:[~2021-10-08 23:01 UTC | newest]

Thread overview: 40+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-28  0:52 What's cooking in git.git (Sep 2021, #08; Mon, 27) Junio C Hamano
2021-09-28  1:57 ` Ævar Arnfjörð Bjarmason
2021-09-28 20:52   ` Junio C Hamano
2021-09-28  6:46 ` Elijah Newren
2021-09-28  7:45   ` Ævar Arnfjörð Bjarmason
2021-09-28 17:25     ` Junio C Hamano
2021-09-28 21:00       ` Neeraj Singh
2021-09-28 23:34         ` Junio C Hamano
2021-09-28 23:53           ` Neeraj Singh
2021-10-07 22:01             ` Junio C Hamano
2021-10-08  6:51               ` Elijah Newren
2021-10-08 22:30                 ` Neeraj Singh
2021-10-08 23:01                 ` Junio C Hamano
2021-09-28  8:07   ` Ævar Arnfjörð Bjarmason
2021-09-28 17:27     ` Junio C Hamano
2021-09-28 13:31   ` Derrick Stolee
2021-09-28 17:33     ` Junio C Hamano
2021-09-28 20:16       ` Derrick Stolee
2021-09-28 17:16   ` Junio C Hamano
2021-09-29  6:42     ` Elijah Newren
2021-09-28 23:40   ` Jeff King
2021-09-28 23:49     ` Jeff King
2021-09-29 18:43     ` Neeraj Singh
2021-09-30  8:16       ` Jeff King
2021-10-01  7:50         ` Elijah Newren
2021-10-01 17:02           ` Junio C Hamano
2021-10-01 17:39             ` Neeraj Singh
2021-10-01 18:15               ` Elijah Newren
2021-10-01 18:12             ` Elijah Newren
2021-10-01 22:02               ` Junio C Hamano
2021-10-01 23:05                 ` Elijah Newren
2021-10-04 13:45     ` Elijah Newren
2021-09-28  8:22 ` da/difftool (was: Re: What's cooking in git.git (Sep 2021, #08; Mon, 27)) Ævar Arnfjörð Bjarmason
2021-09-28  8:23 ` ns/batched-fsync & en/remerge-diff (was " Ævar Arnfjörð Bjarmason
2021-09-28  8:31 ` sg/test-split-index-fix " Ævar Arnfjörð Bjarmason
2021-09-28  8:35 ` hn/reftable (Re: " Ævar Arnfjörð Bjarmason
2021-09-28 12:18   ` Han-Wen Nienhuys
2021-09-30  5:06     ` Carlo Arenas
2021-09-29  8:12 ` What's cooking in git.git (Sep 2021, #08; Mon, 27) Fabian Stelzer
2021-09-30 21:26   ` Junio C Hamano

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.