{"thread":{"id":"46379","subject":"What's cooking in git.git (Jul 2017, #04; Thu, 13)","startedAt":"2017-07-13T23:44:20Z","lastAt":"2017-07-18T21:38:40Z","messageCount":13,"participants":["Junio C Hamano","Santiago Torres","Ævar Arnfjörð Bjarmason"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"324467","messageId":"xmqqo9snsy0k.fsf@gitster.mtv.corp.google.com","threadId":"46379","inReplyTo":null,"subject":"What's cooking in git.git (Jul 2017, #04; Thu, 13)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-07-13T23:44:11Z","receivedAt":"2017-07-13T23:44:20Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Here are the topics that have been cooking.  Commits prefixed with\n'-' are only in 'pu' (proposed updates) while commits prefixed with\n'+' are in 'next'.  The ones marked with '.' do not appear in any of\nthe integration branches, but I am still holding onto them.\n\nA maintenance release for 2.13.x series, and the first preview for\n2.14 series -rc0, have been tagged.  Let's close the 'master' except\nfor obvious fixes and clean-ups and concentrate on regressions from\nnow on.\n\nYou can find the changes described here in the integration branches\nof the repositories listed at\n\n    http://git-blame.blogspot.com/p/git-public-repositories.html\n\n--------------------------------------------------\n[Graduated to \"master\"]\n\n* ab/grep-lose-opt-regflags (2017-06-30) 6 commits\n  (merged to 'next' on 2017-07-05 at 375c0b92ea)\n + grep: remove redundant REG_NEWLINE when compiling fixed regex\n + grep: remove regflags from the public grep_opt API\n + grep: remove redundant and verbose re-assignments to 0\n + grep: remove redundant \"fixed\" field re-assignment to 0\n + grep: adjust a redundant grep pattern type assignment\n + grep: remove redundant double assignment to 0\n\n Code cleanup.\n\n\n* jk/build-with-asan (2017-07-10) 5 commits\n  (merged to 'next' on 2017-07-10 at 49757e1119)\n + Makefile: disable unaligned loads with UBSan\n + Makefile: turn off -fomit-frame-pointer with sanitizers\n + Makefile: add helper for compiling with -fsanitize\n + test-lib: turn on ASan abort_on_error by default\n + test-lib: set ASAN_OPTIONS variable before we run git\n\n The build procedure has been improved to allow building and testing\n Git with address sanitizer more easily.\n\n\n* kn/ref-filter-branch-list (2017-07-10) 4 commits\n  (merged to 'next' on 2017-07-10 at 35fd25c0dd)\n + ref-filter.c: drop return from void function\n + branch: set remote color in ref-filter branch immediately\n + branch: use BRANCH_COLOR_LOCAL in ref-filter format\n + branch: only perform HEAD check for local branches\n\n The rewrite of \"git branch --list\" using for-each-ref's internals\n that happened in v2.13 regressed its handling of color.branch.local;\n this has been fixed.\n\n\n* ks/fix-rebase-doc-picture (2017-07-10) 1 commit\n  (merged to 'next' on 2017-07-10 at 3acb856b17)\n + doc: correct a mistake in an illustration\n\n Doc update.\n\n\n* rs/apply-avoid-over-reading (2017-07-09) 1 commit\n  (merged to 'next' on 2017-07-10 at 2d8191ec3f)\n + apply: use strcmp(3) for comparing strings in gitdiff_verify_name()\n\n Code cleanup.\n\n\n* rs/urlmatch-cleanup (2017-07-09) 1 commit\n  (merged to 'next' on 2017-07-10 at 2dd3e7cab0)\n + urlmatch: use hex2chr() in append_normalized_escapes()\n\n Code cleanup.\n\n\n* rs/use-div-round-up (2017-07-10) 1 commit\n  (merged to 'next' on 2017-07-10 at accb7919da)\n + use DIV_ROUND_UP\n\n Code cleanup.\n\n\n* rs/wt-status-cleanup (2017-07-10) 1 commit\n  (merged to 'next' on 2017-07-10 at d8939f683a)\n + wt-status: use separate variable for result of shorten_unambiguous_ref\n\n Code cleanup.\n\n\n* sb/hashmap-customize-comparison (2017-06-30) 3 commits\n  (merged to 'next' on 2017-07-06 at cc420805f3)\n + hashmap: migrate documentation from Documentation/technical into header\n + patch-ids.c: use hashmap correctly\n + hashmap.h: compare function has access to a data field\n (this branch is used by sb/diff-color-move and sb/hashmap-cleanup.)\n\n Update the hashmap API so that data to customize the behaviour of\n the comparison function can be specified at the time a hashmap is\n initialized.\n\n\n* sb/pull-rebase-submodule (2017-06-27) 4 commits\n  (merged to 'next' on 2017-07-09 at 48d2c3a51c)\n + builtin/fetch cleanup: always set default value for submodule recursing\n + pull: optionally rebase submodules (remote submodule changes only)\n + builtin/fetch: parse recurse-submodules-default at default options parsing\n + builtin/fetch: factor submodule recurse parsing out to submodule config\n\n \"git pull --rebase --recurse-submodules\" learns to rebase the\n branch in the submodules to an updated base.\n\n\n* sb/submodule-doc (2017-06-22) 1 commit\n  (merged to 'next' on 2017-07-09 at fda0ceec31)\n + submodules: overhaul documentation\n\n Doc update.\n\n--------------------------------------------------\n[New Topics]\n\n* jn/hooks-pre-rebase-sample-fix (2017-07-11) 1 commit\n  (merged to 'next' on 2017-07-12 at ed86887454)\n + pre-rebase hook: capture documentation in a <<here document\n\n Code clean-up, that makes us in sync with Debian by one patch.\n\n Will merge to 'master'.\n\n\n* jk/gc-pre-detach-under-hook (2017-07-12) 1 commit\n  (merged to 'next' on 2017-07-12 at 9cf44c2b07)\n + gc: run pre-detach operations under lock\n\n We run an early part of \"git gc\" that deals with refs before\n daemonising (and not under lock) even when running a background\n auto-gc, which caused multiple gc processes attempting to run the\n early part at the same time.  This is now prevented by running the\n early part also under the GC lock.\n\n Will merge to 'master'.\n\n\n* jk/ref-filter-colors (2017-07-13) 15 commits\n - ref-filter: consult want_color() before emitting colors\n - pretty: respect color settings for %C placeholders\n - rev-list: pass diffopt->use_colors through to pretty-print\n - for-each-ref: load config earlier\n - color: check color.ui in git_default_config()\n - ref-filter: pass ref_format struct to atom parsers\n - ref-filter: factor out the parsing of sorting atoms\n - ref-filter: make parse_ref_filter_atom a private function\n - ref-filter: provide a function for parsing sort options\n - ref-filter: move need_color_reset_at_eol into ref_format\n - ref-filter: abstract ref format into its own struct\n - ref-filter: simplify automatic color reset\n - t: use test_decode_color rather than literal ANSI codes\n - docs/for-each-ref: update pointer to color syntax\n - check return value of verify_ref_format()\n\n \"%C(color name)\" in the pretty print format always produced ANSI\n color escape codes, which was an early design mistake.  They now\n honor the configuration (e.g. \"color.ui = never\") and also tty-ness\n of the output medium.\n\n Will merge to and cook in 'next'.\n\n\n* sb/object-id (2017-07-13) 2 commits\n - tag: convert gpg_verify_tag to use struct object_id\n - commit: convert lookup_commit_graft to struct object_id\n\n Conversion from uchar[20] to struct object_id continues.\n\n Will merge to 'next'.\n\n--------------------------------------------------\n[Stalled]\n\n* mg/status-in-progress-info (2017-05-10) 2 commits\n - status --short --inprogress: spell it as --in-progress\n - status: show in-progress info for short status\n\n \"git status\" learns an option to report various operations\n (e.g. \"merging\") that the user is in the middle of.\n\n cf. <xmqqmvakcdqw.fsf@gitster.mtv.corp.google.com>\n\n\n* nd/worktree-move (2017-04-20) 6 commits\n - worktree remove: new command\n - worktree move: refuse to move worktrees with submodules\n - worktree move: accept destination as directory\n - worktree move: new command\n - worktree.c: add update_worktree_location()\n - worktree.c: add validate_worktree()\n\n \"git worktree\" learned move and remove subcommands.\n\n Expecting a reroll.\n cf. <20170420101024.7593-1-pclouds@gmail.com>\n cf. <20170421145916.mknekgqzhxffu7di@sigill.intra.peff.net>\n cf. <d0e81b1e-5869-299e-f462-4d43dc997bd1@ramsayjones.plus.com>\n\n\n* sg/clone-refspec-from-command-line-config (2017-06-16) 2 commits\n - Documentation/clone: document ignored configuration variables\n - clone: respect additional configured fetch refspecs during initial fetch\n (this branch is used by sg/remote-no-string-refspecs.)\n\n \"git clone -c var=val\" is a way to set configuration variables in\n the resulting repository, but it is more useful to also make these\n variables take effect while the initial clone is happening,\n e.g. these configuration variables could be fetch refspecs.\n\n Waiting for a response.\n cf. <20170617112228.vugswym4o4owf6wj@sigill.intra.peff.net>\n cf. <xmqqmv8zhdap.fsf@gitster.mtv.corp.google.com>\n\n\n* js/rebase-i-final (2017-06-15) 10 commits\n - rebase -i: rearrange fixup/squash lines using the rebase--helper\n - t3415: test fixup with wrapped oneline\n - rebase -i: skip unnecessary picks using the rebase--helper\n - rebase -i: check for missing commits in the rebase--helper\n - t3404: relax rebase.missingCommitsCheck tests\n - rebase -i: also expand/collapse the SHA-1s via the rebase--helper\n - rebase -i: do not invent onelines when expanding/collapsing SHA-1s\n - rebase -i: remove useless indentation\n - rebase -i: generate the script via rebase--helper\n - t3415: verify that an empty instructionFormat is handled as before\n\n The final batch to \"git rebase -i\" updates to move more code from\n the shell script to C.\n\n Expecting a reroll.\n This is at its v5.\n cf. <cover.1497444257.git.johannes.schindelin@gmx.de>\n\n--------------------------------------------------\n[Cooking]\n\n* rs/progress-overall-throughput-at-the-end (2017-07-09) 1 commit\n  (merged to 'next' on 2017-07-11 at f5168e975b)\n + progress: show overall rate in last update\n\n The progress meter did not give a useful output when we haven't had\n 0.5 seconds to measure the throughput during the interval.  Instead\n show the overall throughput rate at the end, which is a much more\n useful number.\n\n Will merge to 'master'.\n\n\n* rs/sha1-file-micro-optim (2017-07-09) 2 commits\n - SQUASH???\n - sha1_file: add slash once in for_each_file_in_obj_subdir()\n\n Code cleanup.\n\n Perhaps drop.\n cf. <f59c8256-716b-9305-2a4f-d4fe49f666ff@web.de>\n\n\n* ks/prepare-commit-msg-sample (2017-07-12) 4 commits\n - hook: add a simple first example\n - hook: add sign-off using \"interpret-trailers\"\n - hook: name the positional variables\n - hook: cleanup script\n\n Remove an example that is now obsolete from a sample hook,\n and improve an old example in it that added a sign-off manually\n to use the interpret-trailers command.\n\n\n* jc/allow-lazy-cas (2017-07-06) 1 commit\n - push: disable lazy --force-with-lease by default\n\n Because \"git push --force-with-lease[=<ref>]\" that relies on the\n stability of remote-tracking branches is unsafe when something\n fetches into the repository behind user's back, it is now disabled\n by default.  A new configuration variable can be used to enable it\n by users who know what they are doing.  This would pave the way to\n possibly turn `--force` into `--force-with-lease`.\n\n Will wait for feedback, then merge to and cook in 'next'.\n\n Independent from disabling the feature by default, this stirred up\n a discussion to replace the DWIM heuristics with a better one, which\n deserves to be its own topic.\n cf. <alpine.DEB.2.21.1.1707131435220.4193@virtualbox>\n\n\n* bc/object-id (2017-07-04) 12 commits\n - sha1_name: convert GET_SHA1* flags to GET_OID*\n - sha1_name: convert get_sha1* to get_oid*\n - Convert remaining callers of get_sha1 to get_oid.\n - builtin/verify-tag: convert to struct object_id\n - builtin/unpack-file: convert to struct object_id\n - bisect: convert bisect_checkout to struct object_id\n - builtin/update_ref: convert to struct object_id\n - sequencer: convert to struct object_id\n - remote: convert struct push_cas to struct object_id\n - submodule: convert submodule config lookup to use object_id\n - builtin/merge-tree: convert remaining caller of get_sha1 to object_id\n - builtin/fsck: convert remaining caller of get_sha1 to object_id\n\n Conversion from uchar[20] to struct object_id continues.\n\n Expecting a reroll.\n cf. <20170707212201.ofdgjaips2tw3koy@genre.crustytoothpaste.net>\n\n\n* jk/reflog-walk (2017-07-09) 9 commits\n  (merged to 'next' on 2017-07-09 at 7449e964c6)\n + reflog-walk: apply --since/--until to reflog dates\n + reflog-walk: stop using fake parents\n + rev-list: check reflog_info before showing usage\n + get_revision_1(): replace do-while with an early return\n + log: do not free parents when walking reflog\n + log: clarify comment about reflog cycles\n + revision: disallow reflog walking with revs->limited\n + t1414: document some reflog-walk oddities\n + Merge branch 'jk/reflog-walk-maint' into jk/reflog-walk\n\n Numerous bugs in walking of reflogs via \"log -g\" and friends have\n been fixed.\n\n Will cook in 'next'.\n\n\n* sb/hashmap-cleanup (2017-07-05) 10 commits\n  (merged to 'next' on 2017-07-12 at ce31d06165)\n + t/helper/test-hashmap: use custom data instead of duplicate cmp functions\n + name-hash.c: drop hashmap_cmp_fn cast\n + submodule-config.c: drop hashmap_cmp_fn cast\n + remote.c: drop hashmap_cmp_fn cast\n + patch-ids.c: drop hashmap_cmp_fn cast\n + convert/sub-process: drop cast to hashmap_cmp_fn\n + config.c: drop hashmap_cmp_fn cast\n + builtin/describe: drop hashmap_cmp_fn cast\n + builtin/difftool.c: drop hashmap_cmp_fn cast\n + attr.c: drop hashmap_cmp_fn cast\n\n Many uses of comparision callback function the hashmap API uses\n cast the callback function type when registering it to\n hashmap_init(), which defeats the compile time type checking when\n the callback interface changes (e.g. gaining more parameters).\n The callback implementations have been updated to take \"void *\"\n pointers and cast them to the type they expect instead.\n\n Will cook in 'next'.\n\n\n* tb/push-to-cygwin-unc-path (2017-07-05) 1 commit\n  (merged to 'next' on 2017-07-11 at 4d9c3f82bd)\n + cygwin: allow pushing to UNC paths\n\n On Cygwin, similar to Windows, \"git push //server/share/repository\"\n ought to mean a repository on a network share that can be accessed\n locally, but this did not work correctly due to stripping the double\n slashes at the beginning.\n\n This may need to be heavily tested before it gets unleashed to the\n wild, as the change is at a fairly low-level code and would affect\n not just the code to decide if the push destination is local.  There\n may be unexpected fallouts in the path normalization.\n\n Will merge to 'master'.\n\n\n* mt/p4-parse-G-output (2017-07-13) 3 commits\n - git-p4: filter for {'code':'info'} in p4CmdList\n - git-p4: parse marshal output \"p4 -G\" in p4 changes\n - git-p4: git-p4 tests with p4 triggers\n\n Use \"p4 -G\" to make \"p4 changes\" output more Python-friendly\n to parse.\n\n Wait to see Travis is OK with this and merge to 'next'\n\n\n* ex/deprecate-empty-pathspec-as-match-all (2017-06-23) 2 commits\n  (merged to 'next' on 2017-06-26 at d026281517)\n + pathspec: die on empty strings as pathspec\n + t0027: do not use an empty string as a pathspec element\n\n The final step to make an empty string as a pathspec element\n illegal.  We started this by first deprecating and warning a\n pathspec that has such an element in 2.11 (Nov 2016).\n\n Hopefully we can merge this down to the 'master' by the end of the\n year?  A deprecation warning period that is about 1 year does not\n sound too bad.\n\n Will cook in 'next'.\n\n\n* mh/packed-ref-store (2017-07-03) 30 commits\n  (merged to 'next' on 2017-07-05 at 6c68c603cc)\n + read_packed_refs(): die if `packed-refs` contains bogus data\n + t3210: add some tests of bogus packed-refs file contents\n + repack_without_refs(): don't lock or unlock the packed refs\n + commit_packed_refs(): remove call to `packed_refs_unlock()`\n + clear_packed_ref_cache(): don't protest if the lock is held\n + packed_refs_unlock(), packed_refs_is_locked(): new functions\n + packed_refs_lock(): report errors via a `struct strbuf *err`\n + packed_refs_lock(): function renamed from lock_packed_refs()\n + commit_packed_refs(): use a staging file separate from the lockfile\n + commit_packed_refs(): report errors rather than dying\n + packed_ref_store: make class into a subclass of `ref_store`\n + packed-backend: new module for handling packed references\n + packed_read_raw_ref(): new function, replacing `resolve_packed_ref()`\n + packed_ref_store: support iteration\n + packed_peel_ref(): new function, extracted from `files_peel_ref()`\n + repack_without_refs(): take a `packed_ref_store *` parameter\n + get_packed_ref(): take a `packed_ref_store *` parameter\n + rollback_packed_refs(): take a `packed_ref_store *` parameter\n + commit_packed_refs(): take a `packed_ref_store *` parameter\n + lock_packed_refs(): take a `packed_ref_store *` parameter\n + add_packed_ref(): take a `packed_ref_store *` parameter\n + get_packed_refs(): take a `packed_ref_store *` parameter\n + get_packed_ref_cache(): take a `packed_ref_store *` parameter\n + validate_packed_ref_cache(): take a `packed_ref_store *` parameter\n + clear_packed_ref_cache(): take a `packed_ref_store *` parameter\n + packed_ref_store: move `packed_refs_lock` member here\n + packed_ref_store: move `packed_refs_path` here\n + packed_ref_store: new struct\n + add_packed_ref(): teach function to overwrite existing refs\n + t1408: add a test of stale packed refs covered by loose refs\n\n The \"ref-store\" code reorganization continues.\n\n Will cook in 'next'.\n\n\n* sd/branch-copy (2017-06-18) 3 commits\n - branch: add a --copy (-c) option to go with --move (-m)\n - branch: add test for -m renaming multiple config sections\n - config: create a function to format section headers\n\n \"git branch\" learned \"-c/-C\" to create and switch to a new branch\n by copying an existing one.\n\n Undecided.\n\n I personally do not think \"branch --copy master backup\" while on\n \"master\" that switches to \"backup\" is a good UI, and I *will* say\n \"I told you so\" when users complain after we merge this down to\n 'next' and eventually to 'master'.\n\n\n* ls/filter-process-delayed (2017-06-30) 7 commits\n  (merged to 'next' on 2017-07-05 at a35e644082)\n + convert: add \"status=delayed\" to filter process protocol\n + convert: refactor capabilities negotiation\n + convert: move multiple file filter error handling to separate function\n + convert: put the flags field before the flag itself for consistent style\n + t0021: write \"OUT <size>\" only on success\n + t0021: make debug log file name configurable\n + t0021: keep filter log files on comparison\n\n The filter-process interface learned to allow a process with long\n latency give a \"delayed\" response.\n\n Will cook in 'next'.\n\n\n* bp/fsmonitor (2017-06-12) 6 commits\n - fsmonitor: add a sample query-fsmonitor hook script for Watchman\n - fsmonitor: add documentation for the fsmonitor extension.\n - fsmonitor: add test cases for fsmonitor extension\n - fsmonitor: teach git to optionally utilize a file system monitor to speed up detecting new or changed files.\n - dir: make lookup_untracked() available outside of dir.c\n - bswap: add 64 bit endianness helper get_be64\n\n We learned to talk to watchman to speed up \"git status\".\n\n Expecting a reroll.\n cf. <bade1166-e646-b05a-f65b-adb8da8ba0a7@gmail.com>\n\n\n* sb/diff-color-move (2017-06-30) 26 commits\n  (merged to 'next' on 2017-07-06 at 758ed40e4f)\n + diff: document the new --color-moved setting\n + diff.c: add dimming to moved line detection\n + diff.c: color moved lines differently, plain mode\n + diff.c: color moved lines differently\n + diff.c: buffer all output if asked to\n + diff.c: emit_diff_symbol learns about DIFF_SYMBOL_SUMMARY\n + diff.c: emit_diff_symbol learns about DIFF_SYMBOL_STAT_SEP\n + diff.c: convert word diffing to use emit_diff_symbol\n + diff.c: convert show_stats to use emit_diff_symbol\n + diff.c: convert emit_binary_diff_body to use emit_diff_symbol\n + submodule.c: migrate diff output to use emit_diff_symbol\n + diff.c: emit_diff_symbol learns DIFF_SYMBOL_REWRITE_DIFF\n + diff.c: emit_diff_symbol learns about DIFF_SYMBOL_BINARY_FILES\n + diff.c: emit_diff_symbol learns DIFF_SYMBOL_HEADER\n + diff.c: emit_diff_symbol learns DIFF_SYMBOL_FILEPAIR_{PLUS, MINUS}\n + diff.c: emit_diff_symbol learns DIFF_SYMBOL_CONTEXT_INCOMPLETE\n + diff.c: emit_diff_symbol learns DIFF_SYMBOL_WORDS[_PORCELAIN]\n + diff.c: migrate emit_line_checked to use emit_diff_symbol\n + diff.c: emit_diff_symbol learns DIFF_SYMBOL_NO_LF_EOF\n + diff.c: emit_diff_symbol learns DIFF_SYMBOL_CONTEXT_FRAGINFO\n + diff.c: emit_diff_symbol learns DIFF_SYMBOL_CONTEXT_MARKER\n + diff.c: introduce emit_diff_symbol\n + diff.c: factor out diff_flush_patch_all_file_pairs\n + diff.c: move line ending check into emit_hunk_header\n + diff.c: readability fix\n + Merge branch 'sb/hashmap-customize-comparison' into sb/diff-color-move\n\n \"git diff\" has been taught to optionally paint new lines that are\n the same as deleted lines elsewhere differently from genuinely new\n lines.\n\n Will cook in 'next'.\n\n--------------------------------------------------\n[Discarded]\n\n* mh/packed-ref-store-prep-extra (2017-06-18) 1 commit\n . prefix_ref_iterator_advance(): relax the check of trim length\n\n Split out of mh/packed-ref-store-prep.\n\n\n* nd/prune-in-worktree (2017-04-24) 12 commits\n . rev-list: expose and document --single-worktree\n . revision.c: --reflog add HEAD reflog from all worktrees\n . files-backend: make reflog iterator go through per-worktree reflog\n . revision.c: --all adds HEAD from all worktrees\n . refs: remove dead for_each_*_submodule()\n . revision.c: use refs_for_each*() instead of for_each_*_submodule()\n . refs: add refs_head_ref()\n . refs: move submodule slash stripping code to get_submodule_ref_store\n . refs.c: refactor get_submodule_ref_store(), share common free block\n . revision.c: --indexed-objects add objects from all worktrees\n . revision.c: refactor add_index_objects_to_pending()\n . revision.h: new flag in struct rev_info wrt. worktree-related refs\n\n \"git gc\" and friends when multiple worktrees are used off of a\n single repository did not consider the index and per-worktree refs\n of other worktrees as the root for reachability traversal, making\n objects that are in use only in other worktrees to be subject to\n garbage collection.\n"},{"id":"324485","messageId":"20170714002754.jyck5qmykbmuado7@LykOS.localdomain","threadId":"46379","inReplyTo":"xmqqo9snsy0k.fsf@gitster.mtv.corp.google.com","subject":"Re: What's cooking in git.git (Jul 2017, #04; Thu, 13)","fromName":"Santiago Torres","fromEmail":"santiago@nyu.edu","sentAt":"2017-07-14T00:27:55Z","receivedAt":"2017-07-14T00:28:03Z","isPatch":false,"sender":{"key":"santiago@nyu.edu","avatar":"https://avatars.githubusercontent.com/u/3579933?v=4"},"body":"> Here are the topics that have been cooking.  \n\nHi, \n\nI sent (a patch almost a week ago) that would probably[1] be labeled\nas \"uninteresting\" (as per the notes from the maintainer), but I wanted\nto make sure it wasn't lost in the noise -- I see that theres a lot of\nactive development lately. I checked the latest iterations of \"what's\ncooking\" to see if it was going to be discarded or so, but I see no\nmention of it.\n\nThanks!\n-Santiago\n\n[1] https://public-inbox.org/git/20170707220729.a3xrsju3rf4guyzs@LykOS.localdomain/T/#t\n"},{"id":"324488","messageId":"xmqqfudzsq7u.fsf@gitster.mtv.corp.google.com","threadId":"46379","inReplyTo":"20170714002754.jyck5qmykbmuado7@LykOS.localdomain","subject":"Re: What's cooking in git.git (Jul 2017, #04; Thu, 13)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-07-14T02:32:37Z","receivedAt":"2017-07-14T02:32:45Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Santiago Torres <santiago@nyu.edu> writes:\n\n>> Here are the topics that have been cooking.  \n>\n> I sent (a patch almost a week ago) that would probably[1] be labeled\n> as \"uninteresting\" (as per the notes from the maintainer), but I wanted\n> to make sure it wasn't lost in the noise -- I see that theres a lot of\n> active development lately. I checked the latest iterations of \"what's\n> cooking\" to see if it was going to be discarded or so, but I see no\n> mention of it.\n\nI postponed it when I saw it the first time to see if anybody\ncomments on it, and then it turns out nobody was interested, and it\nremained uninteresting to the list to this day.  \n\nNow, after looking at the message again, from the patch description,\nI would believe you that you experienced _some_ trouble when the\ngpg-agent that is auto-spawned by gpg gets left behind (as I do not\nsee any hits from \"git grep gpg-agent t/\", we are not deliberately\nusing the agent).  However, I could not convince myself that the\nsolution is credible.  Here is an excerpt from the patch:\n\n> diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh\n> index ec2aa8f68..22ef2fa87 100755\n> --- a/t/lib-gpg.sh\n> +++ b/t/lib-gpg.sh\n> @@ -31,6 +31,7 @@ then\n>  \t\tchmod 0700 ./gpghome &&\n>  \t\tGNUPGHOME=\"$(pwd)/gpghome\" &&\n>  \t\texport GNUPGHOME &&\n> +\t\tgpgconf --kill gpg-agent &&\n>  \t\tgpg --homedir \"${GNUPGHOME}\" 2>/dev/null --import \\\n>  \t\t\t\"$TEST_DIRECTORY\"/lib-gpg/keyring.gpg &&\n>  \t\tgpg --homedir \"${GNUPGHOME}\" 2>/dev/null --import-ownertrust \\\n> -- \n\nbut the current directory of this part is the $TRASH_DIRECTORY,\nwhich is always created anew from the beginning in a test.  What\nagent process could possibly be running there immedately after\ncreating ./gpghome (which happens with \"mkdir gpghome &&\" without\n\"-p\" just before the context of this hunk---if the agent was running\nalready, the directory would have been there, and mkdir would have\nfailed, which would have caused \"test_set_prereq GPG\" at the end of\nthe \"&&\" cascade to be skipped.  In other words, it is unclear to\nme, and your log message does not sufficiently explain, why this is\nthe right solution (or what the exact cause of the symptom is, for\nthat matter).\n\nOr perhaps the gpg-agent socket is created somewhere outside the\nGNUPGHOME and stays behind even after a previous run of the same\ntest finishes and $TRASH_DIRECTORY gets cleared (I am guessing the\n\"what the exact cause is\" part, as the log message did not explain\nit)?  If that is the case, it makes me wonder if either of the two\nalternative may be a more robust solution: (1) running gpg tests\ntelling gpg never to use agent, or (2) telling gpg and gpg-agent to\nuse a socket inside GNUPGHOME.\n\nAfter all, \"kill\"ing agent blindly like the above patch would mean\nyou do not know what other party is relying on the proper operation\nof the thing you are killing.  That sounds more like a workaround\nthat a solution (unless it is explained with a solid reason why that\nis the right way to run more than one instances of GPG).\n\nPerhaps everybody else is running these gpg tests without having to\nworry about gpg-agent already because their environment is more\nvanilla, but you have some configuration or environment that cause\ngpg to use agent, and that is the reason why nobody is interested\n(because they have never seen the symptom)?  It is possible that the\npart of t/test-lib.sh that tries to cleanse environment variables\nand other \"external influence\" to give us a predictable and\nrepeatable test is unaware of such a knob that only some developers\n(including you) have and the rest of us were merely lucky.  Perhaps\nwe need to throw GPG_AGENT_INFO SSH_AUTH_SOCK etc. into the list of\nenvirionment variables to nuke there?\n\nCombined with the unknown-ness of the root cause of the issue, I can\nonly say that the patch may be raising an issue worth addressing,\nbut it is too sketchy to tell if it is a right solution or what the\nexact problem being solved is.\n"},{"id":"324507","messageId":"20170714140241.mqg5256ml3grmrn6@LykOS.localdomain","threadId":"46379","inReplyTo":"xmqqfudzsq7u.fsf@gitster.mtv.corp.google.com","subject":"Re: What's cooking in git.git (Jul 2017, #04; Thu, 13)","fromName":"Santiago Torres","fromEmail":"santiago@nyu.edu","sentAt":"2017-07-14T14:02:42Z","receivedAt":"2017-07-14T14:02:50Z","isPatch":false,"sender":{"key":"santiago@nyu.edu","avatar":"https://avatars.githubusercontent.com/u/3579933?v=4"},"body":"Hi, Junio. Thanks for replying.\n\n> I postponed it when I saw it the first time to see if anybody\n> comments on it, and then it turns out nobody was interested, and it\n> remained uninteresting to the list to this day.  \n> \n\nTrue, that's what I was afraid of, but I wanted to give it some closure.\n\n> Now, after looking at the message again, from the patch description,\n> I would believe you that you experienced _some_ trouble when the\n> gpg-agent that is auto-spawned by gpg gets left behind (as I do not\n> see any hits from \"git grep gpg-agent t/\", we are not deliberately\n> using the agent). \n\nTrue, this is what I could gather from asking people over at #gnupg. The\nagent spawns a socket for a GNUPGHOME and leaves it open outside of the\nhome folder (and it caches the inode for the olddir or so).\n\n> However, I could not convince myself that the\n> solution is credible.  \n\nI think you're right on this. I'd rather have more people reproduce the\nissue (some of my colleagues were able to do so, but we all were running\nthe latest GPG, vanilla conf) and find the root of the issue too.\n\n> but the current directory of this part is the $TRASH_DIRECTORY,\n> which is always created anew from the beginning in a test.  What\n> agent process could possibly be running there immedately after\n> creating ./gpghome (which happens with \"mkdir gpghome &&\" without\n> \"-p\" just before the context of this hunk---if the agent was running\n> already, the directory would have been there, and mkdir would have\n> failed, which would have caused \"test_set_prereq GPG\" at the end of\n> the \"&&\" cascade to be skipped.  In other words, it is unclear to\n> me, and your log message does not sufficiently explain, why this is\n> the right solution (or what the exact cause of the symptom is, for\n> that matter).\n\nI see. What is causing this (as far as my current understanding goes)\nis:\n\n1) First iteration of the tests is run, the .gpghome is created and a\n    unix socket is created too. The keychain is imported etc. Tests run\n    normally.\n\n2) The test ends, and the trash directory (along with the .gpghome) are\n    flushed, but the agent is not aware of this. The socket is still\n    open.\n\n3) The second iteration of the tests is run, the new .gpghome is\n    created, but when the keychain fails to import and the agent errors\n    out with ENOENT. The and-chain fails and test_set_preqreq GPG is\n    skipped (as you pointed out).\n\nThis last bit is apparently a result of the agent trying to be clever\nwith the paths. \n\n> \n> Or perhaps the gpg-agent socket is created somewhere outside the\n> GNUPGHOME and stays behind even after a previous run of the same\n> test finishes and $TRASH_DIRECTORY gets cleared (I am guessing the\n> \"what the exact cause is\" part, as the log message did not explain\n> it)?  If that is the case, it makes me wonder if either of the two\n> alternative may be a more robust solution: (1) running gpg tests\n> telling gpg never to use agent, or (2) telling gpg and gpg-agent to\n> use a socket inside GNUPGHOME.\n\nI agree. In hindsight this solution seems rather naive. I'll dig into\nthe root cause, as well as to try to isolate the issue from a\ngpg-version and gpg-config viewpoint.\n\n> After all, \"kill\"ing agent blindly like the above patch would mean\n> you do not know what other party is relying on the proper operation\n> of the thing you are killing.  That sounds more like a workaround\n> that a solution (unless it is explained with a solid reason why that\n> is the right way to run more than one instances of GPG).\n\nI agree. It is probably better to seek the solutions that you suggested\nabove.\n\n> \n> Perhaps everybody else is running these gpg tests without having to\n> worry about gpg-agent already because their environment is more\n> vanilla, but you have some configuration or environment that cause\n> gpg to use agent, and that is the reason why nobody is interested\n> (because they have never seen the symptom)?  It is possible that the\n> part of t/test-lib.sh that tries to cleanse environment variables\n> and other \"external influence\" to give us a predictable and\n> repeatable test is unaware of such a knob that only some developers\n> (including you) have and the rest of us were merely lucky.  Perhaps\n> we need to throw GPG_AGENT_INFO SSH_AUTH_SOCK etc. into the list of\n> envirionment variables to nuke there?\n> \n> Combined with the unknown-ness of the root cause of the issue, I can\n> only say that the patch may be raising an issue worth addressing,\n> but it is too sketchy to tell if it is a right solution or what the\n> exact problem being solved is.\n\nI'll dig into this. This sounds a way more reasonable approach.\n\nThanks for the feedback!\n-Santiago.\n"},{"id":"324530","messageId":"xmqqwp7bqbvg.fsf@gitster.mtv.corp.google.com","threadId":"46379","inReplyTo":"20170714140241.mqg5256ml3grmrn6@LykOS.localdomain","subject":"Re: What's cooking in git.git (Jul 2017, #04; Thu, 13)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-07-14T15:25:23Z","receivedAt":"2017-07-14T15:25:31Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Santiago Torres <santiago@nyu.edu> writes:\n\n>> Combined with the unknown-ness of the root cause of the issue, I can\n>> only say that the patch may be raising an issue worth addressing,\n>> but it is too sketchy to tell if it is a right solution or what the\n>> exact problem being solved is.\n>\n> I'll dig into this. This sounds a way more reasonable approach.\n\nThanks.  Another thing that may help, if it turns out that we do\nwant to let agent run when it wants to, may be to study and mimick\nthe way our webserver tests arrange how the servers are killed at\nthe end of the test.\n"},{"id":"324572","messageId":"87iniu2bty.fsf@gmail.com","threadId":"46379","inReplyTo":"xmqqo9snsy0k.fsf@gitster.mtv.corp.google.com","subject":"Re: What's cooking in git.git (Jul 2017, #04; Thu, 13)","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-07-15T11:12:25Z","receivedAt":"2017-07-15T11:12:34Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Jul 13 2017, Junio C. Hamano jotted:\n\n> Here are the topics that have been cooking.  Commits prefixed with\n> [...]\n>\n> * jc/allow-lazy-cas (2017-07-06) 1 commit\n>  - push: disable lazy --force-with-lease by default\n>\n>  Because \"git push --force-with-lease[=<ref>]\" that relies on the\n>  stability of remote-tracking branches is unsafe when something\n>  fetches into the repository behind user's back, it is now disabled\n>  by default.  A new configuration variable can be used to enable it\n>  by users who know what they are doing.  This would pave the way to\n>  possibly turn `--force` into `--force-with-lease`.\n>\n>  Will wait for feedback, then merge to and cook in 'next'.\n\nAside from the feedback I just sent now 87k23a2d28.fsf@gmail.com,\nthere's my outstanding comment on the config variable name in\n877ezkbn6x.fsf@gmail.com which is waiting on your feedback.\n\n> * sd/branch-copy (2017-06-18) 3 commits\n>  - branch: add a --copy (-c) option to go with --move (-m)\n>  - branch: add test for -m renaming multiple config sections\n>  - config: create a function to format section headers\n>\n>  \"git branch\" learned \"-c/-C\" to create and switch to a new branch\n>  by copying an existing one.\n>\n>  Undecided.\n>\n>  I personally do not think \"branch --copy master backup\" while on\n>  \"master\" that switches to \"backup\" is a good UI, and I *will* say\n>  \"I told you so\" when users complain after we merge this down to\n>  'next' and eventually to 'master'.\n\nFor anyone following along with this. The reason Sahil & I hacked it up\nlike that is because that's analogous to what the --move option does.\n\nLet's see what *nix does:\n\n    $ rm -rf /tmp/{master,backup}; mkdir /tmp/master && cd /tmp/master && mv /tmp/{master,backup} ; file /tmp/{master,backup}\n    /tmp/master: cannot open `/tmp/master' (No such file or directory)\n    /tmp/backup: directory\n\nSimilarly to that, when you're on \"master\" \"git branch --move backup\"\ncould have left you on an orphan branch, but it doesn't, it's the\nequivalent of \"mv && cd\" in *nix terms.\n\nSo since our --move is really --move-and-checkout I think it would be\nconfusing to introduce a --copy sister option that has the semantics of\n--copy-no-checkout instead of a corresponding --copy-and-checkout.\n\nI think it's easier to explain & use an option that's \"like --move\nexcept the old branch doesn't get deleted\", which is what this does,\ninstead of \"actually not analogous to --move at all\".\n\nI happen to want to use this for something where the semantics on that\ntopic work better for me, but I recognize that that's just a matter of\ntaste, if we were green-fielding this I wouldn't mind either way.\n\nBut since we're not, and especially with all the confusion around\ncheckout/branch (some of the hairiest UX in git) I think our --move &\n--copy should be symmetric in the same way that mv(1) an cp(1) are\nsymmetric. Let's not add yet another special case to whether a ref is\ncreated/checked out when being manipulated by some mode of\ncheckout/branch.\n"},{"id":"324655","messageId":"xmqqtw2bgd8q.fsf@gitster.mtv.corp.google.com","threadId":"46379","inReplyTo":"87iniu2bty.fsf@gmail.com","subject":"Re: What's cooking in git.git (Jul 2017, #04; Thu, 13)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-07-17T17:55:01Z","receivedAt":"2017-07-17T17:55:22Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n\n> Let's see what *nix does:\n>\n>     $ rm -rf /tmp/{master,backup}; mkdir /tmp/master && cd /tmp/master && mv /tmp/{master,backup} ; file /tmp/{master,backup}\n>\n> Similarly to that, when you're on \"master\" \"git branch --move backup\"\n> could have left you on an orphan branch, but it doesn't, it's the\n> equivalent of \"mv && cd\" in *nix terms.\n\nAnd with the same analogy,\n\n\tmkdir /tmp/master\n\tcd /tmp/master\n\tcp -R /tmp/master /tmp/backup\n\tpwd\n\nwould show you that you are still in /tmp/master.  Which is quite\ndifferent from what happens with \"mv\".  So what's your point?\n\nIn any case, you already realize that it is a crazy and convoluted\nexample nobody does in real life to go in to one directory *and*\nrename it while you are inside it, don't you?\n\nThe reason why I do not think it is wise to move to the backup\nbranch when you did\n\n\tgit checkout master\n\tgit branch --copy master backup\n\nis not because of that crazy analogy, though.  It is primarily that\nI think one of the major reasons people want to do a \"copy\" to a\nbackup is to keep the pre-modification state of a thing that they\nmay be able to later come back to when it turns out to be needed.\nFor that workflow, with the above \"branch --copy\", your user is\nplanning to make a change that is not usually kept track of by Git\nto the \"master\" branch (perhaps branch.*.description is changed,\nperhaps the remote-tracking branch it is set to integrate with is\nchanged, etc.), but the user is unsuare, and preparing a backup that\ncan be used with \"gir branch -M backup master\" to come back to.  \n\nYou can do all that even if your \"branch --copy\" initially takes you\nto the \"backup\" branch you just created, but then the user has to\nimmediately come back to \"master\" to start doing the major surgery\non \"master\", for which the \"copy\" was a preparation to recover.\n\n> So since our --move is really --move-and-checkout I think it would be\n> confusing to introduce a --copy sister option that has the semantics of\n> --copy-no-checkout instead of a corresponding --copy-and-checkout.\n\nOur --move is *NOT* --move-and-checkout.\n\n\tgit chekckout master\n\tgit branch naster\n\tgit branch -m naster aster\n\nwill *not* move \"naster\" to \"aster\" and check it out.  During the\nwhole exercise, you will stay on \"master\".  \n\nWhen you rename the current branch so that the name of the current\nbranch will no longer exist in the system, you _could_ detach the\nHEAD, or you _could_ move the current branch to the new name.  There\nis no sensible alternative other than these two, but either way, you\nneed to have a special case for renaming the current branch.  It's\njust the latter is more useful in practice and that is the only\nreason why it moves _you_ along with the current branch if you\nhappen to be on that branch being renamed.\n\nI do not see much reason why such a special case has to apply to\n\"copy\".  The source of the copy is not going away, and in the\n\"backup\" scenario, moving away from the thing that is backed up, in\npreparation for further work that may have to be reverted, is\nactively counter-productive.\n\nThere however _is_ an opposite use case.  You may want to start\nworking on a _new_ branch that is more similarly set up to what your\ncurrent branch is, _and_ you want the new branch to start at your\ncurrent branch.  But I think that should be done by adding an\nenhanced mode of \"checkout -b\".  IOW, I think\n\n\tgit checkout master\n\tgit checkout -b --with-configuration naster [master]\n\nis a very sensible thing to desire; if \"master\" is set to integrate\nwith refs/remotes/origin/master, the new \"naster\" branch may want to\nintegrate with refs/remotes/origin/naster (instead of the local\n\"master\", which is what the traditional \"checkout -b\" would do), for\nexample.  Or you could do the same thing with\n\n\tgit branch --copy master naster\n\tgit checkout naster\n\nif your \"branch --copy\" does not switch out of the current branch.\n\n\n\n\n"},{"id":"324685","messageId":"20170717214241.yui75hywg2ddkfsb@LykOS.localdomain","threadId":"46379","inReplyTo":"xmqqwp7bqbvg.fsf@gitster.mtv.corp.google.com","subject":"Re: What's cooking in git.git (Jul 2017, #04; Thu, 13)","fromName":"Santiago Torres","fromEmail":"santiago@nyu.edu","sentAt":"2017-07-17T21:42:41Z","receivedAt":"2017-07-17T21:42:49Z","isPatch":false,"sender":{"key":"santiago@nyu.edu","avatar":"https://avatars.githubusercontent.com/u/3579933?v=4"},"body":"> > I'll dig into this. This sounds a way more reasonable approach.\n> \n> Thanks.  Another thing that may help, if it turns out that we do\n> want to let agent run when it wants to\n\nI did some digging on the reason as to why this was happening. It turns\nout there is a bug on gnupg. As of gpg 2.1.21, all operations require an\nagent running (BTW --no-use-agent is effectively a no-op now). \n\n\nGiven that on versions 2.19-21, they changed the way sockets are\ncreated to be stored on /run/user/UID/gnupg/... deleting the $GNUPG_HOME\ndirectory wouldn't flush the agent socket, and would leave an agent\ninstance per test running, possibly forever. E.g., make test would\nresult in the following:\n\n    santiago at ~ ✔ pgrep -a gpg-agent\n    632 gpg-agent --homedir /git/t/trash directory.t6050-replace/gpghome --use-standard-socket --daemon\n    1192 /usr/bin/gpg-agent --supervised\n    2939 gpg-agent --homedir /git/t/trash directory.t5801-remote-helpers/gpghome --use-standard-socket --daemon\n    4656 gpg-agent --homedir /git/t/trash directory.t6300-for-each-ref/gpghome --use-standard-socket --daemon\n    5427 gpg-agent --homedir /git/t/trash directory.t7510-signed-commit/gpghome --use-standard-socket --daemon\n    5898 gpg-agent --homedir /git/t/trash directory.t6302-for-each-ref-filter/gpghome --use-standard-socket --daemon\n    7747 gpg-agent --homedir /git/t/trash directory.t7003-filter-branch/gpghome --use-standard-socket --daemon\n    12922 gpg-agent --homedir /git/t/trash directory.t7600-merge/gpghome --use-standard-socket --daemon\n    13572 gpg-agent --homedir /git/t/trash directory.t7004-tag/gpghome --use-standard-socket --daemon\n    14521 gpg-agent --homedir /git/t/trash directory.t5534-push-signed/gpghome --use-standard-socket --daemon\n    16563 gpg-agent --homedir /git/t/trash directory.t5541-http-push-smart/gpghome --use-standard-socket --daemon\n    17853 gpg-agent --homedir /git/t/trash directory.t7030-verify-tag/gpghome --use-standard-socket --daemon\n    29858 gpg-agent --homedir /git/t/trash directory.t7612-merge-verify-signatures/gpghome --use-standard-socket --daemon\n    31100 gpg-agent --homedir /git/t/trash directory.t4202-log/gpghome --use-standard-socket --daemon\n\nOther projects such as notmuch opted for a solution that's simlar to\nwhat I had suggested[1], but I wonder if it's even necessary to do.\nThere is already a fix on the master branch of gnupg[2], which I imagine\nwill show up to the next version of gpg2.\n\nI don't think it would make sense to fix anything on our side, unless we\nwant to be extra sure the test suite is not leaking agents for all gpg\nversions (including these minor versions). \n\nWhat is your take?\n\nThanks!\n-Santiago.\n\n[1] https://paste.debian.net/976970/\n[2] https://dev.gnupg.org/T3218\n"},{"id":"324688","messageId":"xmqqmv82g1g7.fsf@gitster.mtv.corp.google.com","threadId":"46379","inReplyTo":"20170717214241.yui75hywg2ddkfsb@LykOS.localdomain","subject":"Re: What's cooking in git.git (Jul 2017, #04; Thu, 13)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-07-17T22:09:44Z","receivedAt":"2017-07-17T22:09:51Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Santiago Torres <santiago@nyu.edu> writes:\n\n> Other projects such as notmuch opted for a solution that's simlar to\n> what I had suggested[1], but I wonder if it's even necessary to do.\n> There is already a fix on the master branch of gnupg[2], which I imagine\n> will show up to the next version of gpg2.\n>\n> I don't think it would make sense to fix anything on our side, unless we\n> want to be extra sure the test suite is not leaking agents for all gpg\n> versions (including these minor versions). \n\nI am not sure if it is merely \"if it's even necessary\"; if there are\ntwo tests running in parallel, with their own separate\n$TRASH_DIRECTORY, and one of them say \"kill the agent\" at the\nbeginning, would it affect the other test, depending on the timing?\n\nI would imagine that the sockets are kept per GNUPGHOME and they are\nnot going to interfere, so if that is the case, I do not think we\nmind helping folks with a buggy versions of GnuPG by having a \"let's\nbe cautious and kill a leftover agent before starting to test\"\npatch, as long as the reason why we do so is clearly understood and\ndocumented.\n\nThanks for digging it to the root cause.\n\n"},{"id":"324709","messageId":"20170718175458.hadk64c3yccyv57t@LykOS.localdomain","threadId":"46379","inReplyTo":"xmqqmv82g1g7.fsf@gitster.mtv.corp.google.com","subject":"Re: What's cooking in git.git (Jul 2017, #04; Thu, 13)","fromName":"Santiago Torres","fromEmail":"santiago@nyu.edu","sentAt":"2017-07-18T17:54:59Z","receivedAt":"2017-07-18T17:55:19Z","isPatch":false,"sender":{"key":"santiago@nyu.edu","avatar":"https://avatars.githubusercontent.com/u/3579933?v=4"},"body":"On Mon, Jul 17, 2017 at 03:09:44PM -0700, Junio C Hamano wrote:\n> I am not sure if it is merely \"if it's even necessary\"; if there are\n> two tests running in parallel, with their own separate\n> $TRASH_DIRECTORY, and one of them say \"kill the agent\" at the\n> beginning, would it affect the other test, depending on the timing?\n\nThis shouldn't happen, provided their respective GNUPGHOME env vars\naren't the same. A gpg-agent process is spawned per GNUPGHOME on the\nversions that I've tested.\n\n> \n> I would imagine that the sockets are kept per GNUPGHOME and they are\n> not going to interfere, so if that is the case, I do not think we\n> mind helping folks with a buggy versions of GnuPG by having a \"let's\n> be cautious and kill a leftover agent before starting to test\"\n> patch, as long as the reason why we do so is clearly understood and\n> documented.\n\nI double checked the patch/solutions/causes just to be sure I'm not\ndoing anything crazy. Here's a v2 of the patch that kills the agent upon\ncleanup rather than startup.\n\nThanks!\n-Santiago.\n\n\nFrom 20491890b804d13f9edb0205c1cc21d080beffe2 Mon Sep 17 00:00:00 2001\nFrom: Santiago Torres <santiago@nyu.edu>\nDate: Tue, 18 Jul 2017 13:16:11 -0400\nSubject: [RFC/PATCH] t: test-lib: flush gpg agent on cleanup\n\nWhen running gpg-relevant tests, a gpg-daemon is spawned for each\nGNUPGHOME used. This daemon may stay running after the testand cache ile\ndescriptors for the trash directories, even after the trash directory is\nremoved. This leads to ENOENT errors when attempting to create files if\ntests are run multiple times.\n\nAdd a cleanup script to force flushing the gpg-agent when before\nremoving the trash directory if the test is GPG-relevant.\n\nHelped-by: Junio C Hamano <gitster@pobox.com>\nSigned-off-by: Santiago Torres <santiago@nyu.edu>\n---\n t/test-lib.sh | 5 +++++\n 1 file changed, 5 insertions(+)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 1b6e53f78..ed8796d7a 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -732,6 +732,11 @@ test_done () {\n \t\tEOF\n \tfi\n \n+\tif test_have_prereq GPG\n+\tthen\n+\t\tGNUPGHOME=\"$TRASH_DIRECTORY/gpghome\" gpgconf --kill all\n+\tfi\n+\n \tif test \"$test_fixed\" != 0\n \tthen\n \t\tsay_color error \"# $test_fixed known breakage(s) vanished; please update test(s)\"\n-- \n2.13.3\n\n"},{"id":"324713","messageId":"xmqqeftdpor5.fsf@gitster.mtv.corp.google.com","threadId":"46379","inReplyTo":"20170718175458.hadk64c3yccyv57t@LykOS.localdomain","subject":"Re: What's cooking in git.git (Jul 2017, #04; Thu, 13)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-07-18T18:46:06Z","receivedAt":"2017-07-18T18:46:20Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Santiago Torres <santiago@nyu.edu> writes:\n\n> On Mon, Jul 17, 2017 at 03:09:44PM -0700, Junio C Hamano wrote:\n>> I am not sure if it is merely \"if it's even necessary\"; if there are\n>> two tests running in parallel, with their own separate\n>> $TRASH_DIRECTORY, and one of them say \"kill the agent\" at the\n>> beginning, would it affect the other test, depending on the timing?\n>\n> This shouldn't happen, provided their respective GNUPGHOME env vars\n> aren't the same. A gpg-agent process is spawned per GNUPGHOME on the\n> versions that I've tested.\n>\n>> \n>> I would imagine that the sockets are kept per GNUPGHOME and they are\n>> not going to interfere, so if that is the case, I do not think we\n>> mind helping folks with a buggy versions of GnuPG by having a \"let's\n>> be cautious and kill a leftover agent before starting to test\"\n>> patch, as long as the reason why we do so is clearly understood and\n>> documented.\n>\n> I double checked the patch/solutions/causes just to be sure I'm not\n> doing anything crazy. Here's a v2 of the patch that kills the agent upon\n> cleanup rather than startup.\n\nThanks.  \n\nThe workaround of killing the lingering agent makes sense to me, but\nis test_done the right place to do so?  When \"sh t7004-tag.sh -i\"\ndies in the middle, I do not think the control even reaches the\ninvocation of that shell function at the end.\n\nThe httpd tests seem to do this in start_httpd shell function:\n\n\tstart_httpd () {\n\t\tprepare_httpd \n\t\ttrap 'code=$?; stop_httpd: (exit $code); die' EXIT\n\t\t...\n\nand I can see how this would avoid the \"'-i' option can stop the\ntest script in the middle\" issue.  Because the GPG tests do not have\na clear \"start agent\" step, we'd need to arrange the \"trap\" in the\ncase arm in t/lib-gpg.sh where we set GPG prerequisite if we were to\nuse the same approach.  Or killing a possibly lingering one like your\nfirst patch did may be simpler (we need to make sure &&-chain works\nwell with \"gpgconf --kill all\").\n\nOh, wait, I can run \"gpg\" just fine, but I do not seem to have\ngpgconf.\n\n\t$ type gpgconf\n\tbash: type: gpgconf: not found\n\nThe patch may need a bit more cross-version work, it seems.\n\n\n-- >8 --\nFrom: Santiago Torres <santiago@nyu.edu>\nDate: Tue, 18 Jul 2017 13:16:11 -0400\nSubject: [RFC/PATCH] t: test-lib: flush gpg agent on cleanup\n\nWhen running gpg-relevant tests, a gpg-daemon is spawned for each\nGNUPGHOME used. This daemon may stay running after the testand cache ile\ndescriptors for the trash directories, even after the trash directory is\nremoved. This leads to ENOENT errors when attempting to create files if\ntests are run multiple times.\n\nAdd a cleanup script to force flushing the gpg-agent when before\nremoving the trash directory if the test is GPG-relevant.\n\nHelped-by: Junio C Hamano <gitster@pobox.com>\nSigned-off-by: Santiago Torres <santiago@nyu.edu>\n---\n t/test-lib.sh | 5 +++++\n 1 file changed, 5 insertions(+)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 1b6e53f78..ed8796d7a 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -732,6 +732,11 @@ test_done () {\n \t\tEOF\n \tfi\n \n+\tif test_have_prereq GPG\n+\tthen\n+\t\tGNUPGHOME=\"$TRASH_DIRECTORY/gpghome\" gpgconf --kill all\n+\tfi\n+\n \tif test \"$test_fixed\" != 0\n \tthen\n \t\tsay_color error \"# $test_fixed known breakage(s) vanished; please update test(s)\"\n-- \n2.13.3\n\n\n"},{"id":"324746","messageId":"20170718211651.rcnbkhcwem4sbcb5@LykOS.localdomain","threadId":"46379","inReplyTo":"xmqqeftdpor5.fsf@gitster.mtv.corp.google.com","subject":"Re: What's cooking in git.git (Jul 2017, #04; Thu, 13)","fromName":"Santiago Torres","fromEmail":"santiago@nyu.edu","sentAt":"2017-07-18T21:16:52Z","receivedAt":"2017-07-18T21:16:58Z","isPatch":false,"sender":{"key":"santiago@nyu.edu","avatar":"https://avatars.githubusercontent.com/u/3579933?v=4"},"body":"> Oh, wait, I can run \"gpg\" just fine, but I do not seem to have\n> gpgconf.\n> \n> \t$ type gpgconf\n> \tbash: type: gpgconf: not found\n> \n> The patch may need a bit more cross-version work, it seems.\n\nRight, sorry about that. \n\nI was testing against Debian Stretch/Arch, who do ship gpg2 with\ngpgconf. It seems Debian oldstable and other variants still ship gpg1,\nwhich doesn't have it. Would it make sense to have a fallthrough branch\non the switch statement for gpg2.1 instead? something like the attached patch.\n\nThanks,\n-Santiago.\n\n\nFrom 07ab87c1ddb31197a3a5c124ad5a2462a460d4e3 Mon Sep 17 00:00:00 2001\nFrom: Santiago Torres <santiago@nyu.edu>\nDate: Tue, 18 Jul 2017 13:16:11 -0400\nSubject: [RFC/PATCH] t: lib-gpg: flush gpg agent on startup\n\nWhen running gpg-relevant tests, a gpg-daemon is spawned for each\nGNUPGHOME used. This daemon may stay running after the test and cache\nfile descriptors for the trash directories, even after the trash\ndirectory is removed. This leads to ENOENT errors when attempting to\ncreate files if tests are run multiple times.\n\nAdd a cleanup script to force flushing the gpg-agent for that GNUPGHOME\n(if any) before setting up the GPG relevant-environment.\n\nHelped-by: Junio C Hamano <gitster@pobox.com>\nSigned-off-by: Santiago Torres <santiago@nyu.edu>\n---\n t/lib-gpg.sh | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/t/lib-gpg.sh b/t/lib-gpg.sh\nindex ec2aa8f68..ffb20a438 100755\n--- a/t/lib-gpg.sh\n+++ b/t/lib-gpg.sh\n@@ -10,6 +10,8 @@ then\n \t'gpg (GnuPG) 1.0.6'*)\n \t\tsay \"Your version of gpg (1.0.6) is too buggy for testing\"\n \t\t;;\n+\t'gpg (GnuPG) 2.1'*)\n+\t\tGNUPGHOME=\"$(pwd)/gpghome\" gpgconf --kill all ;&\n \t*)\n \t\t# Available key info:\n \t\t# * Type DSA and Elgamal, size 2048 bits, no expiration date,\n-- \n2.13.3\n\n"},{"id":"324748","messageId":"xmqqvampmnmv.fsf@gitster.mtv.corp.google.com","threadId":"46379","inReplyTo":"20170718211651.rcnbkhcwem4sbcb5@LykOS.localdomain","subject":"Re: What's cooking in git.git (Jul 2017, #04; Thu, 13)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-07-18T21:38:32Z","receivedAt":"2017-07-18T21:38:40Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Santiago Torres <santiago@nyu.edu> writes:\n\n> ... It seems Debian oldstable and other variants still ship gpg1,\n> which doesn't have it. Would it make sense to have a fallthrough branch\n> on the switch statement for gpg2.1 instead? something like the attached patch.\n\nIf the problem of leftover agent is limited to a narrow versions of\nGPG2, alternatively we could unconditionally attempt to use gpgconf\nand ignore the failure ;-), but as long as we know all problematic\nones identify themselves as \"gpg (GnuPG) 2.1*\", then your patch\n(with s/;&/;;/ of course ;-)) sounds very sensible.\n\n> Thanks,\n\nThank *you* for working on this.\n\n> From 07ab87c1ddb31197a3a5c124ad5a2462a460d4e3 Mon Sep 17 00:00:00 2001\n> From: Santiago Torres <santiago@nyu.edu>\n> Date: Tue, 18 Jul 2017 13:16:11 -0400\n> Subject: [RFC/PATCH] t: lib-gpg: flush gpg agent on startup\n\nPerhaps it is about time we lost RFC/ mark from here.\n\n>\n> When running gpg-relevant tests, a gpg-daemon is spawned for each\n> GNUPGHOME used. This daemon may stay running after the test and cache\n> file descriptors for the trash directories, even after the trash\n> directory is removed. This leads to ENOENT errors when attempting to\n> create files if tests are run multiple times.\n>\n> Add a cleanup script to force flushing the gpg-agent for that GNUPGHOME\n> (if any) before setting up the GPG relevant-environment.\n>\n> Helped-by: Junio C Hamano <gitster@pobox.com>\n> Signed-off-by: Santiago Torres <santiago@nyu.edu>\n> ---\n>  t/lib-gpg.sh | 2 ++\n>  1 file changed, 2 insertions(+)\n>\n> diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh\n> index ec2aa8f68..ffb20a438 100755\n> --- a/t/lib-gpg.sh\n> +++ b/t/lib-gpg.sh\n> @@ -10,6 +10,8 @@ then\n>  \t'gpg (GnuPG) 1.0.6'*)\n>  \t\tsay \"Your version of gpg (1.0.6) is too buggy for testing\"\n>  \t\t;;\n> +\t'gpg (GnuPG) 2.1'*)\n> +\t\tGNUPGHOME=\"$(pwd)/gpghome\" gpgconf --kill all ;&\n>  \t*)\n>  \t\t# Available key info:\n>  \t\t# * Type DSA and Elgamal, size 2048 bits, no expiration date,\n"}]}